feat: Add bearer/basic auth option to webhooks - #3785
Conversation
Add optional authentication type selection (None, Basic Auth, Bearer Token) to webhook notifications. When Basic Auth is selected, username and password fields appear; when Bearer Token is selected, a token field appears. Backend changes: - Add webhookAuthType, webhookUsername, webhookPassword, webhookToken fields to Notification type, model, and validation - Implement buildAuthHeaders method in WebhookProvider to send Authorization headers based on the selected auth type Frontend changes: - Add webhook auth UI section to the notification create/edit page with auth type dropdown and conditional credential fields - Update Notification type, validation schema, form hook, and translations Closes bluewave-labs#2369
ajhollid
left a comment
ajhollid
left a comment
There was a problem hiding this comment.
The concept here is sound, but the implementaiton needs to be hardened somewhat before it is ready to go.
Most important here is validation, I can create invalid authentication schemas without them being rejected, so let's get that sorted out.
Other than that there are some formatting issues and other minor issues to address.
Thanks for your contribution thus far, looking forward to seeing the revised code!
| accountSid?: string; | ||
| twilioPhoneNumber?: string; | ||
| topic?: string; | ||
| webhookAuthType?: 'none' | 'basic' | 'bearer'; |
There was a problem hiding this comment.
These should not be inlined as it requires duplicate maintenance whenever this is updated, both here and in the validation schema.
This should be declared and exported as all other enums in the applicaiton:
export const WebhookAuthTypes = ["none", "basic", "bearer"] as const;
export type WebhookAuthType = (typeof WebhookAuthTypes)[number];
| const webhookSchema = baseSchema.extend({ | ||
| type: z.literal("webhook"), | ||
| address: z.string().min(1, "Webhook URL is required").url("Please enter a valid URL"), | ||
| webhookAuthType: z.enum(["none", "basic", "bearer"]).optional(), |
There was a problem hiding this comment.
inline enum duplication here as mentioned in notification.ts
| accountSid: { type: String }, | ||
| twilioPhoneNumber: { type: String }, | ||
| topic: { type: String }, | ||
| webhookAuthType: { type: String, enum: ['none', 'basic', 'bearer'] }, |
There was a problem hiding this comment.
This is failing formatting
| accountSid?: string; | ||
| twilioPhoneNumber?: string; | ||
| topic?: string; | ||
| webhookAuthType?: 'none' | 'basic' | 'bearer'; |
There was a problem hiding this comment.
Same here, format failure
| homeserverUrl: z.union([z.string(), z.literal("")]).optional(), | ||
| roomId: z.union([z.string(), z.literal("")]).optional(), | ||
| accessToken: z.union([z.string(), z.literal("")]).optional(), | ||
| webhookAuthType: z.enum(["none", "basic", "bearer"]).optional(), |
There was a problem hiding this comment.
There needs to be a super refine applied here to properly validate webhook authentication.
For example, if I select basic with an empty username/password it passes validation on both client and server, but is clearly not a valid basic auth schema.
{
"_id": {
"$oid": "6a5e5048a3a78419627a835e"
},
"userId": {
"$oid": "6a25a150a2c194721064fb15"
},
"teamId": {
"$oid": "6a25a150a2c194721064fb13"
},
"type": "webhook",
"notificationName": "test",
"address": "https://www.google.ca",
"webhookAuthType": "basic",
"webhookUsername": "",
"webhookPassword": "",
"createdAt": {
"$date": "2026-07-20T16:43:52.067Z"
},
"updatedAt": {
"$date": "2026-07-20T16:43:52.067Z"
},
"__v": 0
}
I was able to create this, which I should not be able to.
| const token = notification.webhookToken || ""; | ||
| headers["Authorization"] = "Bearer " + token; | ||
| } | ||
|
|
| </Select> | ||
| )} | ||
| /> | ||
| {watch("webhookAuthType") === "basic" && ( |
There was a problem hiding this comment.
Let's hoist this like the other watch calls, this shouldn't really be inline. It's very easy to miss that this is even here
| json: { text: getTestMessage() }, | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| ...this.buildAuthHeaders(notification as Notification), |
There was a problem hiding this comment.
No casting please, we should not have to lie to the compiler in order for the code to compile. Everything should always be properly typed.
2 participants
Description
Closes #2369
This PR adds optional authentication support for webhook notifications. Users can now select an authentication type when configuring a webhook:
Changes
Backend
Frontend
Testing