From 8e681123d836fc0717b6ff2c504f475cd3d02563 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 20 Apr 2026 23:52:29 +0100 Subject: [PATCH] refactor: dedupe synology chat tests --- .../src/channel.integration.test.ts | 61 +++---- extensions/synology-chat/src/channel.test.ts | 87 +++++----- extensions/synology-chat/src/client.test.ts | 13 +- .../synology-chat/src/webhook-handler.test.ts | 152 +++++++++--------- 4 files changed, 157 insertions(+), 156 deletions(-) diff --git a/extensions/synology-chat/src/channel.integration.test.ts b/extensions/synology-chat/src/channel.integration.test.ts index fb21351e9c9..a1d2e2c2e65 100644 --- a/extensions/synology-chat/src/channel.integration.test.ts +++ b/extensions/synology-chat/src/channel.integration.test.ts @@ -15,6 +15,16 @@ type _RegisteredRoute = { }; let createSynologyChatPlugin: typeof import("./channel.js").createSynologyChatPlugin; + +function makeStartContext(cfg: T, accountId: string, abortSignal: AbortSignal) { + return { + cfg, + accountId, + log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + abortSignal, + }; +} + describe("Synology channel wiring integration", () => { beforeAll(async () => { ({ createSynologyChatPlugin } = await import("./channel.js")); @@ -30,30 +40,27 @@ describe("Synology channel wiring integration", () => { it("registers real webhook handler with resolved account config and enforces allowlist", async () => { const plugin = createSynologyChatPlugin(); const abortController = new AbortController(); - const ctx = { - cfg: { - channels: { - "synology-chat": { - enabled: true, - accounts: { - alerts: { - enabled: true, - token: "valid-token", - incomingUrl: "https://nas.example.com/incoming", - webhookPath: "/webhook/synology-alerts", - dmPolicy: "allowlist", - allowedUserIds: ["456"], - }, + const cfg = { + channels: { + "synology-chat": { + enabled: true, + accounts: { + alerts: { + enabled: true, + token: "valid-token", + incomingUrl: "https://nas.example.com/incoming", + webhookPath: "/webhook/synology-alerts", + dmPolicy: "allowlist", + allowedUserIds: ["456"], }, }, }, }, - accountId: "alerts", - log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, - abortSignal: abortController.signal, }; - const started = plugin.gateway.startAccount(ctx); + const started = plugin.gateway.startAccount( + makeStartContext(cfg, "alerts", abortController.signal), + ); expect(registerPluginHttpRouteMock).toHaveBeenCalledTimes(1); const firstCall = registerPluginHttpRouteMock.mock.calls[0]; @@ -115,18 +122,12 @@ describe("Synology channel wiring integration", () => { }, }; - const alphaStarted = plugin.gateway.startAccount({ - cfg, - accountId: "alpha", - log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, - abortSignal: alphaAbortController.signal, - }); - const betaStarted = plugin.gateway.startAccount({ - cfg, - accountId: "beta", - log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, - abortSignal: betaAbortController.signal, - }); + const alphaStarted = plugin.gateway.startAccount( + makeStartContext(cfg, "alpha", alphaAbortController.signal), + ); + const betaStarted = plugin.gateway.startAccount( + makeStartContext(cfg, "beta", betaAbortController.signal), + ); expect(registerPluginHttpRouteMock).toHaveBeenCalledTimes(2); const alphaRoute = registerPluginHttpRouteMock.mock.calls[0]?.[0]; diff --git a/extensions/synology-chat/src/channel.test.ts b/extensions/synology-chat/src/channel.test.ts index 2f516304ccb..862b953e370 100644 --- a/extensions/synology-chat/src/channel.test.ts +++ b/extensions/synology-chat/src/channel.test.ts @@ -2,26 +2,27 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { createPluginSetupWizardStatus } from "../../../test/helpers/plugins/setup-wizard.js"; import type { ResolvedSynologyChatAccount } from "./types.js"; +const securityAccountDefaults: ResolvedSynologyChatAccount = { + accountId: "default", + enabled: true, + token: "t", + incomingUrl: "https://nas/incoming", + nasHost: "h", + webhookPath: "/w", + webhookPathSource: "default" as const, + dangerouslyAllowNameMatching: false, + dangerouslyAllowInheritedWebhookPath: false, + dmPolicy: "allowlist" as const, + allowedUserIds: [], + rateLimitPerMinute: 30, + botName: "Bot", + allowInsecureSsl: false, +}; + function makeSecurityAccount( overrides: Partial = {}, ): ResolvedSynologyChatAccount { - return { - accountId: "default", - enabled: true, - token: "t", - incomingUrl: "https://nas/incoming", - nasHost: "h", - webhookPath: "/w", - webhookPathSource: "default" as const, - dangerouslyAllowNameMatching: false, - dangerouslyAllowInheritedWebhookPath: false, - dmPolicy: "allowlist" as const, - allowedUserIds: [], - rateLimitPerMinute: 30, - botName: "Bot", - allowInsecureSsl: false, - ...overrides, - }; + return { ...securityAccountDefaults, ...overrides }; } const clientModule = await import("./client.js"); @@ -227,6 +228,26 @@ describe("createSynologyChatPlugin", () => { }); describe("security.collectWarnings", () => { + function makeSharedWebhookConfig(alertsOverrides: Record = {}) { + return { + channels: { + "synology-chat": { + token: "base-token", + webhookPath: "/webhook/shared", + accounts: { + alerts: { + token: "alerts-token", + incomingUrl: "https://nas/alerts", + dmPolicy: "allowlist", + allowedUserIds: ["123"], + ...alertsOverrides, + }, + }, + }, + }, + }; + } + it("warns when token is missing", () => { const plugin = createSynologyChatPlugin(); const account = makeSecurityAccount({ token: "" }); @@ -277,22 +298,7 @@ describe("createSynologyChatPlugin", () => { it("warns when named multi-account routes inherit a shared webhookPath", () => { const plugin = createSynologyChatPlugin(); - const cfg = { - channels: { - "synology-chat": { - token: "base-token", - webhookPath: "/webhook/shared", - accounts: { - alerts: { - token: "alerts-token", - incomingUrl: "https://nas/alerts", - dmPolicy: "allowlist", - allowedUserIds: ["123"], - }, - }, - }, - }, - }; + const cfg = makeSharedWebhookConfig(); const account = plugin.config.resolveAccount(cfg, "alerts"); const warnings = plugin.security.collectWarnings({ cfg, account }); expect(warnings.some((w: string) => w.includes("must set an explicit webhookPath"))).toBe( @@ -302,23 +308,16 @@ describe("createSynologyChatPlugin", () => { it("warns when enabled accounts share the same exact webhookPath", () => { const plugin = createSynologyChatPlugin(); + const base = makeSharedWebhookConfig({ webhookPath: "/webhook/shared" }).channels[ + "synology-chat" + ]; const cfg = { channels: { "synology-chat": { - token: "base-token", + ...base, incomingUrl: "https://nas/default", - webhookPath: "/webhook/shared", dmPolicy: "allowlist", allowedUserIds: ["123"], - accounts: { - alerts: { - token: "alerts-token", - incomingUrl: "https://nas/alerts", - webhookPath: "/webhook/shared", - dmPolicy: "allowlist", - allowedUserIds: ["123"], - }, - }, }, }, }; diff --git a/extensions/synology-chat/src/client.test.ts b/extensions/synology-chat/src/client.test.ts index 833ce755f78..ee4bbfc3d93 100644 --- a/extensions/synology-chat/src/client.test.ts +++ b/extensions/synology-chat/src/client.test.ts @@ -4,15 +4,16 @@ import { describe, it, expect, vi, beforeAll, beforeEach, afterEach } from "vite // Mock http and https modules before importing the client vi.mock("node:https", () => { - const mockRequest = vi.fn(); - const mockGet = vi.fn(); - return { default: { request: mockRequest, get: mockGet }, request: mockRequest, get: mockGet }; + const httpsRequest = vi.fn(); + const httpsGet = vi.fn(); + const httpsModule = { request: httpsRequest, get: httpsGet }; + return { default: httpsModule, request: httpsRequest, get: httpsGet }; }); vi.mock("node:http", () => { - const mockRequest = vi.fn(); - const mockGet = vi.fn(); - return { default: { request: mockRequest, get: mockGet }, request: mockRequest, get: mockGet }; + const httpRequest = vi.fn(); + const httpGet = vi.fn(); + return { default: { request: httpRequest, get: httpGet }, request: httpRequest, get: httpGet }; }); const https = await import("node:https"); diff --git a/extensions/synology-chat/src/webhook-handler.test.ts b/extensions/synology-chat/src/webhook-handler.test.ts index f2cbb3488bf..ff89a14bf9f 100644 --- a/extensions/synology-chat/src/webhook-handler.test.ts +++ b/extensions/synology-chat/src/webhook-handler.test.ts @@ -115,6 +115,69 @@ describe("createWebhookHandler", () => { expect(deliver).not.toHaveBeenCalled(); } + function makeTestHandler(params: { + accountIdSuffix: string; + deliver?: WebhookHandlerDeps["deliver"]; + account?: Partial; + }) { + const deliver = params.deliver ?? vi.fn().mockResolvedValue(null); + return { + deliver, + handler: createWebhookHandler({ + account: makeAccount({ + accountId: `${params.accountIdSuffix}-${Date.now()}`, + ...params.account, + }), + deliver, + log, + }), + }; + } + + async function postToWebhook( + handler: ReturnType, + body = validBody, + options?: Parameters[2], + ) { + const req = makeReq("POST", body, options); + const res = makeRes(); + await handler(req, res); + return res; + } + + async function expectTokenlessBodyAccepted(params: { + accountIdSuffix: string; + options: Parameters[2]; + }) { + const { deliver, handler } = makeTestHandler({ accountIdSuffix: params.accountIdSuffix }); + const res = await postToWebhook( + handler, + makeFormBody({ user_id: "123", username: "testuser", text: "hello" }), + params.options, + ); + expect(res._status).toBe(204); + expect(deliver).toHaveBeenCalled(); + } + + async function runValidReply(params: { accountIdSuffix: string; reply?: string }) { + const { deliver, handler } = makeTestHandler({ + accountIdSuffix: params.accountIdSuffix, + deliver: vi.fn().mockResolvedValue(params.reply ?? "Bot reply"), + }); + const res = await postToWebhook(handler); + expect(res._status).toBe(204); + return { deliver, res }; + } + + function expectBotReplySentTo(chatUserId: string) { + expect(sendMessage).toHaveBeenCalledWith( + "https://nas.example.com/incoming", + "Bot reply", + chatUserId, + true, + ); + } + it("rejects non-POST methods with 405", async () => { const handler = createWebhookHandler({ account: makeAccount(), @@ -358,51 +421,25 @@ describe("createWebhookHandler", () => { }); it("accepts token from query when body token is absent", async () => { - const deliver = vi.fn().mockResolvedValue(null); - const handler = createWebhookHandler({ - account: makeAccount({ accountId: "query-token-test-" + Date.now() }), - deliver, - log, - }); - - const req = makeReq( - "POST", - makeFormBody({ user_id: "123", username: "testuser", text: "hello" }), - { + await expectTokenlessBodyAccepted({ + accountIdSuffix: "query-token-test", + options: { headers: { "content-type": "application/x-www-form-urlencoded" }, url: "/webhook/synology?token=valid-token", }, - ); - const res = makeRes(); - await handler(req, res); - - expect(res._status).toBe(204); - expect(deliver).toHaveBeenCalled(); + }); }); it("accepts token from authorization header when body token is absent", async () => { - const deliver = vi.fn().mockResolvedValue(null); - const handler = createWebhookHandler({ - account: makeAccount({ accountId: "header-token-test-" + Date.now() }), - deliver, - log, - }); - - const req = makeReq( - "POST", - makeFormBody({ user_id: "123", username: "testuser", text: "hello" }), - { + await expectTokenlessBodyAccepted({ + accountIdSuffix: "header-token-test", + options: { headers: { "content-type": "application/x-www-form-urlencoded", authorization: "Bearer valid-token", }, }, - ); - const res = makeRes(); - await handler(req, res); - - expect(res._status).toBe(204); - expect(deliver).toHaveBeenCalled(); + }); }); it("returns 403 for unauthorized user with allowlist policy", async () => { @@ -484,18 +521,7 @@ describe("createWebhookHandler", () => { }); it("responds 204 immediately and delivers async", async () => { - const deliver = vi.fn().mockResolvedValue("Bot reply"); - const handler = createWebhookHandler({ - account: makeAccount({ accountId: "async-test-" + Date.now() }), - deliver, - log, - }); - - const req = makeReq("POST", validBody); - const res = makeRes(); - await handler(req, res); - - expect(res._status).toBe(204); + const { deliver, res } = await runValidReply({ accountIdSuffix: "async-test" }); expect(res._body).toBe(""); expect(deliver).toHaveBeenCalledWith( expect.objectContaining({ @@ -510,18 +536,7 @@ describe("createWebhookHandler", () => { }); it("keeps replies bound to payload.user_id by default", async () => { - const deliver = vi.fn().mockResolvedValue("Bot reply"); - const handler = createWebhookHandler({ - account: makeAccount({ accountId: "stable-id-test-" + Date.now() }), - deliver, - log, - }); - - const req = makeReq("POST", validBody); - const res = makeRes(); - await handler(req, res); - - expect(res._status).toBe(204); + const { deliver } = await runValidReply({ accountIdSuffix: "stable-id-test" }); expect(resolveLegacyWebhookNameToChatUserId).not.toHaveBeenCalled(); expect(deliver).toHaveBeenCalledWith( expect.objectContaining({ @@ -529,12 +544,7 @@ describe("createWebhookHandler", () => { chatUserId: "123", }), ); - expect(sendMessage).toHaveBeenCalledWith( - "https://nas.example.com/incoming", - "Bot reply", - "123", - true, - ); + expectBotReplySentTo("123"); }); it("only resolves reply recipient by username when break-glass mode is enabled", async () => { @@ -548,12 +558,7 @@ describe("createWebhookHandler", () => { chatUserId: "456", }), ); - expect(sendMessage).toHaveBeenCalledWith( - "https://nas.example.com/incoming", - "Bot reply", - "456", - true, - ); + expectBotReplySentTo("456"); }); it("falls back to payload.user_id when break-glass resolution does not find a match", async () => { @@ -569,12 +574,7 @@ describe("createWebhookHandler", () => { chatUserId: "123", }), ); - expect(sendMessage).toHaveBeenCalledWith( - "https://nas.example.com/incoming", - "Bot reply", - "123", - true, - ); + expectBotReplySentTo("123"); }); it("sanitizes input before delivery", async () => {