Skip to content

Commit 3319894

Browse files
committed
refactor(notifications): address maintainer feedback on sanitization typing and test coverage
Ref #2369
1 parent 5a79115 commit 3319894

2 files changed

Lines changed: 110 additions & 3 deletions

File tree

server/src/controllers/notificationController.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { AppError } from "@/utils/AppError.js";
1212
import { INotificationsService } from "@/service/index.js";
1313
import { requireTeamId, requireUserId } from "./controllerUtils.js";
1414
import { IMonitorsRepository } from "@/repositories/index.js";
15+
import type { Notification } from "@/types/notification.js";
1516

1617
const SERVICE_NAME = "NotificationController";
1718

@@ -47,10 +48,10 @@ class NotificationController implements INotificationController {
4748
}
4849
};
4950

50-
private sanitizeNotification = (notification: any) => {
51+
private sanitizeNotification = (notification: Notification | null | undefined): Notification | null | undefined => {
5152
if (!notification) return notification;
52-
const { authPassword, authToken, accessToken, ...sanitized } = notification;
53-
return sanitized;
53+
const { authPassword, authToken, ...sanitized } = notification;
54+
return sanitized as Notification;
5455
};
5556

5657
createNotification = async (req: Request, res: Response, next: NextFunction) => {

server/test/unit/providers/notifications/webhookProvider.test.ts

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,98 @@ describe("WebhookProvider", () => {
4545
const { provider } = createProvider();
4646
expect(await provider.sendTestAlert(makeNotification())).toBe(false);
4747
});
48+
49+
it("omits Authorization header when authType is none", async () => {
50+
const { provider } = createProvider();
51+
await provider.sendTestAlert(makeNotification({ authType: "none" }));
52+
const headers = mockGotPost.mock.calls[0][1].headers;
53+
expect(headers.Authorization).toBeUndefined();
54+
expect(headers["Content-Type"]).toBe("application/json");
55+
});
56+
57+
it("sends Basic Authorization when authType is basic", async () => {
58+
const { provider } = createProvider();
59+
await provider.sendTestAlert(
60+
makeNotification({
61+
authType: "basic",
62+
authUsername: "user",
63+
authPassword: "secret",
64+
})
65+
);
66+
const headers = mockGotPost.mock.calls[0][1].headers;
67+
expect(headers.Authorization).toBe(`Basic ${Buffer.from("user:secret").toString("base64")}`);
68+
});
69+
70+
it("sends Bearer Authorization when authType is bearer", async () => {
71+
const { provider } = createProvider();
72+
await provider.sendTestAlert(
73+
makeNotification({
74+
authType: "bearer",
75+
authToken: "my-bearer-token",
76+
})
77+
);
78+
const headers = mockGotPost.mock.calls[0][1].headers;
79+
expect(headers.Authorization).toBe("Bearer my-bearer-token");
80+
});
81+
82+
it("omits Authorization when basic auth is missing password", async () => {
83+
const { provider } = createProvider();
84+
await provider.sendTestAlert(
85+
makeNotification({
86+
authType: "basic",
87+
authUsername: "user",
88+
authPassword: undefined,
89+
})
90+
);
91+
const headers = mockGotPost.mock.calls[0][1].headers;
92+
expect(headers.Authorization).toBeUndefined();
93+
});
94+
95+
it("omits Authorization when bearer auth is missing token", async () => {
96+
const { provider } = createProvider();
97+
await provider.sendTestAlert(
98+
makeNotification({
99+
authType: "bearer",
100+
authToken: undefined,
101+
})
102+
);
103+
const headers = mockGotPost.mock.calls[0][1].headers;
104+
expect(headers.Authorization).toBeUndefined();
105+
});
106+
107+
it("omits Authorization when basic auth is missing username", async () => {
108+
const { provider } = createProvider();
109+
await provider.sendTestAlert(
110+
makeNotification({
111+
authType: "basic",
112+
authUsername: undefined,
113+
authPassword: "secret",
114+
})
115+
);
116+
const headers = mockGotPost.mock.calls[0][1].headers;
117+
expect(headers.Authorization).toBeUndefined();
118+
});
119+
120+
it("handles empty strings for auth fields by omitting Authorization", async () => {
121+
const { provider } = createProvider();
122+
123+
await provider.sendTestAlert(
124+
makeNotification({
125+
authType: "basic",
126+
authUsername: "",
127+
authPassword: "",
128+
})
129+
);
130+
expect(mockGotPost.mock.calls[0][1].headers.Authorization).toBeUndefined();
131+
132+
await provider.sendTestAlert(
133+
makeNotification({
134+
authType: "bearer",
135+
authToken: "",
136+
})
137+
);
138+
expect(mockGotPost.mock.calls[1][1].headers.Authorization).toBeUndefined();
139+
});
48140
});
49141

50142
describe("sendMessage", () => {
@@ -98,5 +190,19 @@ describe("WebhookProvider", () => {
98190
expect(text).not.toContain("Additional Information");
99191
expect(text).not.toContain("View Incident");
100192
});
193+
194+
it("sends Basic Authorization on sendMessage when authType is basic", async () => {
195+
const { provider } = createProvider();
196+
await provider.sendMessage(
197+
makeNotification({
198+
authType: "basic",
199+
authUsername: "user",
200+
authPassword: "secret",
201+
}) as any,
202+
makeMessage()
203+
);
204+
const headers = mockGotPost.mock.calls[0][1].headers;
205+
expect(headers.Authorization).toBe(`Basic ${Buffer.from("user:secret").toString("base64")}`);
206+
});
101207
});
102208
});

0 commit comments

Comments
 (0)