Feat/webhook auth by harsh-aghara · Pull Request #3454 · bluewave-labs/Checkmate

harsh-aghara · 2026-04-01T10:12:57Z

Describe your changes

Add optional Basic Auth and Bearer Token authentication support for Webhook notifications.

Server:

Added WebhookAuthType type definitions (none | basic | bearer)
Extended Mongoose Notification schema with authType, authUsername, authPassword, authToken fields
Added Zod validation with conditional requirements (username/password for Basic, token for Bearer)
Updated WebhookProvider.buildAuthHeaders() to inject Authorization header into outgoing HTTP requests

Client:

Added auth type selector dropdown (None / Basic Auth / Bearer Token) in webhook notification form
Conditional form fields: username + password for Basic Auth, token for Bearer
Client-side Zod validation with superRefine for conditional requirements
Added all i18n translation keys, no hardcoded strings

Write your issue number after "Fixes "

Fixes #2369

Please ensure all items are checked off before requesting a review. "Checked off" means you need to add an "x" character between brackets so they turn into checkmarks.

(Do not skip this or your PR will be closed) I deployed the application locally.
(Do not skip this or your PR will be closed) I have performed a self-review and testing of my code.
I have included the issue # in the PR.
I have added i18n support to visible strings (instead of <div>Add</div>, use):

const { t } = useTranslation();
<div>{t('add')}</div>

I have not included any files that are not related to my pull request, including package-lock and package-json if dependencies have not changed
I didn't use any hardcoded values (otherwise it will not scale, and will make it difficult to maintain consistency across the application).
I made sure font sizes, color choices etc are all referenced from the theme. I don't have any hardcoded dimensions.
My PR is granular and targeted to one specific feature.
I ran npm run format in server and client directories, which automatically formats your code.
I took a screenshot or a video and attached to this PR if there is a UI change.

None

Basic Auth

Bearer Token

ajhollid

In general, it looks decent. There are some type casting issues to resolve, and credentials should be stripped so they are not returned from the API server.

ajhollid · 2026-04-01T17:17:43Z

 		const defaults = buildDefaults(data);
-		return { schema: notificationSchema, defaults };
+		return {
+			schema: notificationSchema as unknown as typeof baseNotificationSchema,


This removes all type safety, please revert this

ajhollid · 2026-04-01T17:19:05Z

+		authPassword: { type: String },
+		authToken: { type: String },


These two fields should stripped before being returned to the front end, credentials should not be sent from the API to an unknown consumer.

Reverted the type casts in the client hook and server schema.
For security I've implemented sanitization at the Controller layer this ensures the WebhookProvider still has access to credentials for outgoing requests while stripping them from all API responses sent to the client.

ajhollid · 2026-04-01T17:19:56Z

 		accessToken: { type: String },
+		authType: {
+			type: String,
+			enum: ["none", "basic", "bearer"] as WebhookAuthType[],


No casting please, use the correct type here

ajhollid

Thanks for making the requested changes, looking pretty good! We can simplify that helper method to be more performant though, so let's do that. Should be good to go after that!

ajhollid · 2026-04-03T18:27:24Z

+	private sanitizeNotification = (notification: any) => {
+		if (!notification) return notification;
+		const sanitized = { ...notification };
+		delete sanitized.authPassword;
+		delete sanitized.authToken;
+		delete sanitized.accessToken;
+		return sanitized;
+	};
+


Let's avoid using delete, we can just spread the properties and discard the unused ones

const {authPassword, authToken, accessToken, ...sanitized} = notification return sanitized

That should be sufficient

Done.
Thanks for the suggestion.

pauln17 · 2026-04-05T04:08:18Z

Hey @harsh-aghara, I am working on an issue with slightly similar tasks, do you have a discord or something I could reach you on if you wouldn't mind?

harsh-aghara · 2026-04-05T09:02:47Z

Hey @harsh-aghara, I am working on an issue with slightly similar tasks, do you have a discord or something I could reach you on if you wouldn't mind?

Sure.
You can mail me!

harsh-aghara · 2026-04-06T04:17:02Z

Thanks for making the requested changes, looking pretty good! We can simplify that helper method to be more performant though, so let's do that. Should be good to go after that!

@ajhollid I have simplified the helper method.
Please review it.

harsh-aghara · 2026-04-17T05:45:17Z

@ajhollid Just pushed an update to clear out the branch rot, sync up with develop.

What i changed:

Rebase & feat: add Pushover notification channel #3530: Got everything rebased and made sure the new Pushover channel plays nicely with the shared validation schemas and UI.
Build fix: Caught a TS type mismatch in the client that broke the build after recent merges. Fixed it and verified with a local npm run build.
Review feedback: Swapped in the lighter helper method you suggested and added credential sanitization to the controller.
All unit and integration tests are passing locally again.

Could you approve the CI workflow when you have a sec and give it another look?

harsh-aghara · 2026-04-21T14:49:14Z

Hey @ajhollid, I just completed another rebase to resolve the fresh conflicts from the recently merged Twilio(#3534) and Pushover(#3530) PRs.

To make your review easier, here is what I've ensured in this update:

Conflict Resolution: Integrated the new Twilio/Pushover fields while keeping my Webhook Auth logic strictly isolated to avoid regressions.

Maintainer Feedback: Applied the performance-optimized sanitization (object destructuring) we discussed and ensured all credentials (including Webhook tokens) are stripped from API responses.

Validation: Verified both Client and Server builds locally, and all 995 tests are passing.

Since we were good to go after the last review, I'd love to get this merged before any more notification channels cause new conflicts. Let me know if there's anything else you need!

Br0wnHammer

Overall, a nice and clean implementation. The PR was easy to follow through. However, I have some questions and nitpicks. Please look at them.

Br0wnHammer · 2026-05-25T16:28:55Z

 		}
 	};

+	private sanitizeNotification = (notification: any) => {


Nit - type this as Notification instead of any.

Done!
Typed it to Notification.

Br0wnHammer · 2026-05-25T16:34:28Z

 import got from "got";

 export class WebhookProvider extends NotificationProvider {
+	private buildAuthHeaders(notification: Partial<Notification>): Record<string, string> {


There's already a webhookProvider.test.ts but nothing covers this. buildAuthHeaders is pure and security-sensitive, please add cases.

Good call. I have added test cases covering all the auth types and edge cases for missing credentials and empty strings.

Br0wnHammer · 2026-05-25T16:38:13Z


+	private sanitizeNotification = (notification: any) => {
+		if (!notification) return notification;
+		const { authPassword, authToken, accessToken, ...sanitized } = notification;


This strips accessToken from every notification, but that's the live credential for Telegram/Twilio/Pushover/Matrix too, won't this leave their token fields empty and block re-saving on edit?

Ah I didn't realize that.
I've removed accessToken from the destructuring list so it's no longer stripped

… authentication Add WebhookAuthType union type ('none' | 'basic' | 'bearer') and WebhookAuthTypes const array to both client and server type modules. Extend the Notification interface with optional authType, authUsername, authPassword, and authToken fields. Ref bluewave-labs#2369

…epository Add authType (enum: none/basic/bearer, default: none), authUsername, authPassword, and authToken fields to the Notification Mongoose schema. Map the new fields in MongoNotificationsRepository document-to-domain conversion. Ref bluewave-labs#2369

Add authType, authUsername, authPassword, and authToken to the webhook notification Zod validation schema. Use superRefine to conditionally require username/password for Basic Auth and token for Bearer Auth. Ref bluewave-labs#2369

Add buildAuthHeaders method to WebhookProvider that constructs Basic (base64-encoded username:password) or Bearer token Authorization headers based on the notification's authType. Apply to both sendMessage and sendTestMessage flows. Ref bluewave-labs#2369

Extend webhookSchema with authType, authUsername, authPassword, and authToken fields. Extract baseNotificationSchema and add superRefine to conditionally validate credentials based on selected auth type. Ref bluewave-labs#2369

…orm fields Add Authentication ConfigBox to webhook notification create/edit page with auth type selector (None/Basic/Bearer), conditional username/password fields for Basic Auth, and token field for Bearer Auth. Update useNotificationForm hook with auth field defaults. Add i18n translation keys for all new webhook auth UI strings. Ref bluewave-labs#2369

…fety - Strip authPassword, authToken, and accessToken from notification API responses in the controller to ensure security at the API boundary. - Remove redundant type casting from authType enum in the Mongoose schema. - Revert unnecessary type casting in useNotificationForm hook to maintain proper type safety. Ref bluewave-labs#2369

…place the delete operator with object destructuring in sanitizeNotification to improve performance and avoid object de-optimization. Ref bluewave-labs#2369

… mismatches

…typing and test coverage Ref bluewave-labs#2369

Br0wnHammer · 2026-05-25T18:29:58Z


+	private sanitizeNotification = (notification: Notification | null | undefined): Notification | null | undefined => {
+		if (!notification) return notification;
+		const { authPassword, authToken, ...sanitized } = notification;


Since sanitizeNotification strips authPassword/authToken from responses, the edit form loads them empty, but superRefine still requires them, so editing a Basic/Bearer webhook seems to force re-entering credentials every time (and an empty submit would $set over the stored secret).

I was focused on the sanitization that's why I didn't think through the "Edit" UX. It’s definitely a pain if a user has to re-enter their token just to rename a webhook.
I can fix this by updating the Zod schema to allow empty strings on update and then tweak the service logic so it only $sets those fields if the user actually types something new. That way, leaving them blank would just keep the existing secret in the DB. Does that sound like the right path to you?

@ajhollid what are your thoughts on this?

@harsh-aghara, I was going through your proposed solution and I agree, leave blank to keep the secret is the right call. Just heads up that create and edit share the same validation, so loosening it would also let someone create a Basic auth webhook with no password. Probably want a separate lenient edit schema, and when authType changes, clear the old secret fields instead of skipping them so they don't linger in the DB.

@Br0wnHammer Thanks for the heads up.

I've just pushed a fix for this. I split out a lenient edit schema on both the client and server so users can safely leave secrets blank to keep their existing ones.

I also updated the service layer to explicitly $unset stale credentials if the authType (or the main channel type) changes, so we never leave lingering data in the DB.

@harsh-aghara have your pushed the changes? I am not seeing any latest commits in the PR.

@Br0wnHammer Apologies for the confusion. I had to force-push to clean up an accidental commit that included unrelated files.

…or webhook auth - Split create/edit validation schemas (client + server) so secrets are optional on update, strict on create - Service layer skips empty secret fields on edit to preserve DB values - Explicit $unset of stale credentials when authType or channel changes - Repository supports $unset alongside $set for clean field removal - Edit UX: 'Leave blank to keep existing' placeholder on secret fields - Test button validates against strict schema with inline error feedback - Added i18n key for keepExisting placeholder Ref bluewave-labs#2369

Br0wnHammer

LGMT! Great work on this @harsh-aghara 🚀

Br0wnHammer · 2026-06-10T12:22:58Z

Hi @harsh-aghara,
We discussed your PR with the team, and we've decided that we can't merge it in its current form due to a number of related changes that still need to be addressed.

Given the scope of the code changes, it would be best to open a new PR and incorporate the following feedback there:

The edit schema is largely a duplicate of the create schema (roughly 90% overlap), so we'd like to avoid maintaining two nearly identical implementations.
There are a few type casts that undermine type safety and should be refactored.
It looks like notification testing may no longer work as expected, since some fields are being stripped before processing.

Once these concerns are addressed in a new PR, we can take another look.
Thankyou again for all your hardwork! 🚀

harsh-aghara requested review from Br0wnHammer, Owaiseimdad, ajhollid and karenvicent as code owners April 1, 2026 10:12

harsh-aghara closed this Apr 1, 2026

harsh-aghara deleted the feat/webhook-auth branch April 1, 2026 10:39

harsh-aghara restored the feat/webhook-auth branch April 1, 2026 10:44

harsh-aghara reopened this Apr 1, 2026

ajhollid requested changes Apr 1, 2026

View reviewed changes

harsh-aghara force-pushed the feat/webhook-auth branch 2 times, most recently from 864f403 to 2496576 Compare April 2, 2026 17:49

harsh-aghara requested a review from ajhollid April 2, 2026 18:25

ajhollid requested changes Apr 3, 2026

View reviewed changes

harsh-aghara requested a review from ajhollid April 3, 2026 19:17

pauln17 mentioned this pull request Apr 5, 2026

feat: add ntfy support #3462

Closed

10 tasks

harsh-aghara force-pushed the feat/webhook-auth branch from 0feb663 to 012c068 Compare April 17, 2026 05:15

harsh-aghara force-pushed the feat/webhook-auth branch from cf36d2f to 681feac Compare April 21, 2026 13:47

harsh-aghara mentioned this pull request May 22, 2026

Add bearer/basic auth option to webhooks #2369

Open

Br0wnHammer requested changes May 25, 2026

View reviewed changes

Br0wnHammer removed the request for review from Owaiseimdad May 25, 2026 16:40

Br0wnHammer added the enhancement New feature or request label May 25, 2026

harsh-aghara added 4 commits May 25, 2026 23:30

harsh-aghara added 8 commits May 25, 2026 23:30

feat(notifications): add max length constraints to webhook auth fields

6b3d2ca

refactor(notifications): use object destructuring for sanitization Re…

ecadb67

…place the delete operator with object destructuring in sanitizeNotification to improve performance and avoid object de-optimization. Ref bluewave-labs#2369

fix(notifications): resolve build errors and type mismatches from merge

9ad1547

fix(notifications): make webhook auth fields optional to resolve type…

5a79115

… mismatches

refactor(notifications): address maintainer feedback on sanitization …

3319894

…typing and test coverage Ref bluewave-labs#2369

harsh-aghara force-pushed the feat/webhook-auth branch from 681feac to 3319894 Compare May 25, 2026 18:08

Br0wnHammer self-requested a review May 25, 2026 18:16

Br0wnHammer requested changes May 25, 2026

View reviewed changes

harsh-aghara requested a review from Br0wnHammer May 27, 2026 18:30

harsh-aghara force-pushed the feat/webhook-auth branch from 9137344 to 3319894 Compare May 27, 2026 18:52

Br0wnHammer approved these changes May 28, 2026

View reviewed changes

Br0wnHammer closed this Jun 10, 2026

Uh oh!

Uh oh!

Conversation

harsh-aghara commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes

Write your issue number after "Fixes "

Please ensure all items are checked off before requesting a review. "Checked off" means you need to add an "x" character between brackets so they turn into checkmarks.

Uh oh!

ajhollid left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ajhollid left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pauln17 commented Apr 5, 2026

Uh oh!

harsh-aghara commented Apr 5, 2026

Uh oh!

harsh-aghara commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

harsh-aghara commented Apr 17, 2026

Uh oh!

harsh-aghara commented Apr 21, 2026

Uh oh!

Br0wnHammer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harsh-aghara May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Br0wnHammer left a comment

Choose a reason for hiding this comment

Uh oh!

Br0wnHammer commented Jun 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

harsh-aghara commented Apr 1, 2026 •

edited

Loading

harsh-aghara commented Apr 6, 2026 •

edited

Loading

harsh-aghara May 25, 2026 •

edited

Loading