-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: Add bearer/basic auth option to webhooks #3785
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,10 @@ export interface Notification { | |
| accountSid?: string; | ||
| twilioPhoneNumber?: string; | ||
| topic?: string; | ||
| webhookAuthType?: 'none' | 'basic' | 'bearer'; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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: |
||
| webhookUsername?: string; | ||
| webhookPassword?: string; | ||
| webhookToken?: string; | ||
| createdAt: string; | ||
| updatedAt: string; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,10 @@ const discordSchema = baseSchema.extend({ | |
| 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(), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. inline enum duplication here as mentioned in |
||
| webhookUsername: z.string().optional(), | ||
| webhookPassword: z.string().optional(), | ||
| webhookToken: z.string().optional(), | ||
| }); | ||
|
|
||
| const pagerDutySchema = baseSchema.extend({ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,10 @@ export const createNotificationBodyValidation = z.discriminatedUnion("type", [ | |
| 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(), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. I was able to create this, which I should not be able to. |
||
| webhookUsername: z.union([z.string(), z.literal("")]).optional(), | ||
| webhookPassword: z.union([z.string(), z.literal("")]).optional(), | ||
| webhookToken: z.union([z.string(), z.literal("")]).optional(), | ||
| }), | ||
| // Slack notification | ||
| z.object({ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,6 +52,10 @@ const NotificationSchema = new Schema<NotificationDocument>( | |
| accountSid: { type: String }, | ||
| twilioPhoneNumber: { type: String }, | ||
| topic: { type: String }, | ||
| webhookAuthType: { type: String, enum: ['none', 'basic', 'bearer'] }, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is failing formatting |
||
| webhookUsername: { type: String }, | ||
| webhookPassword: { type: String }, | ||
| webhookToken: { type: String }, | ||
| }, | ||
| { | ||
| timestamps: true, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,10 @@ export interface Notification { | |
| accountSid?: string; | ||
| twilioPhoneNumber?: string; | ||
| topic?: string; | ||
| webhookAuthType?: 'none' | 'basic' | 'bearer'; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, format failure |
||
| webhookUsername?: string; | ||
| webhookPassword?: string; | ||
| webhookToken?: string; | ||
| createdAt: string; | ||
| updatedAt: string; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,25 @@ import { getTestMessage } from "@/domain/notifications/providers/utils.js"; | |
| import got from "got"; | ||
|
|
||
| export class WebhookProvider extends NotificationProvider { | ||
| /** | ||
| * Build authorization header based on webhook auth configuration | ||
| */ | ||
| private buildAuthHeaders = (notification: Notification): Record<string, string> => { | ||
| const headers: Record<string, string> = {}; | ||
|
|
||
| if (notification.webhookAuthType === "basic") { | ||
| const username = notification.webhookUsername || ""; | ||
| const password = notification.webhookPassword || ""; | ||
| const encoded = Buffer.from(username + ":" + password).toString("base64"); | ||
| headers["Authorization"] = "Basic " + encoded; | ||
| } else if (notification.webhookAuthType === "bearer") { | ||
| const token = notification.webhookToken || ""; | ||
| headers["Authorization"] = "Bearer " + token; | ||
| } | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing tests |
||
| return headers; | ||
| }; | ||
|
|
||
| sendMessage = async (notification: Notification, message: NotificationMessage): Promise<boolean> => { | ||
| if (!notification.address) { | ||
| return false; | ||
|
|
@@ -19,6 +38,7 @@ export class WebhookProvider extends NotificationProvider { | |
| json: payload, | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| ...this.buildAuthHeaders(notification), | ||
| }, | ||
| ...this.gotRequestOptions(), | ||
| }); | ||
|
|
@@ -101,6 +121,7 @@ export class WebhookProvider extends NotificationProvider { | |
| json: { text: getTestMessage() }, | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| ...this.buildAuthHeaders(notification as Notification), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| }, | ||
| ...this.gotRequestOptions(), | ||
| }); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's hoist this like the other
watchcalls, this shouldn't really be inline. It's very easy to miss that this is even here