From d4e38a90b86f26647e56fb18053e4919efbb1f93 Mon Sep 17 00:00:00 2001 From: habara keigo Date: Thu, 18 Sep 2025 18:21:38 +0900 Subject: [PATCH 1/3] Allow to skip signature verification --- lib/middleware.ts | 9 +++- lib/types.ts | 8 +++ test/helpers/test-server.ts | 38 +++++++++++--- test/middleware.spec.ts | 102 +++++++++++++++++++++++++++++++++++- 4 files changed, 148 insertions(+), 9 deletions(-) diff --git a/lib/middleware.ts b/lib/middleware.ts index fa9ba349b..489299573 100644 --- a/lib/middleware.ts +++ b/lib/middleware.ts @@ -62,7 +62,14 @@ export default function middleware(config: Types.MiddlewareConfig): Middleware { } })(); - if (!validateSignature(body, secret, signature)) { + // Check if signature verification should be skipped + const shouldSkipVerification = + config.skipSignatureVerification && config.skipSignatureVerification(); + + if ( + !shouldSkipVerification && + !validateSignature(body, secret, signature) + ) { next( new SignatureValidationFailed("signature validation failed", { signature, diff --git a/lib/types.ts b/lib/types.ts index 532987958..dc77d4512 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -15,6 +15,14 @@ export interface ClientConfig extends Config { export interface MiddlewareConfig extends Config { channelSecret: string; + + // skipSignatureValidation is a function that determines whether to skip + // webhook signature verification. + // + // If the function returns true, the signature verification step is skipped. + // This can be useful in scenarios such as when you're in the process of updating + // the channel secret and need to temporarily bypass verification to avoid disruptions. + skipSignatureVerification?: () => boolean; } export type Profile = { diff --git a/test/helpers/test-server.ts b/test/helpers/test-server.ts index 2487ff38d..d4f67e8b5 100644 --- a/test/helpers/test-server.ts +++ b/test/helpers/test-server.ts @@ -10,7 +10,8 @@ import { } from "../../lib/exceptions.js"; import * as finalhandler from "finalhandler"; -let server: Server | null = null; +// Use a map to store multiple server instances +let servers: Map = new Map(); function listen(port: number, middleware?: express.RequestHandler) { const app = express(); @@ -77,17 +78,40 @@ function listen(port: number, middleware?: express.RequestHandler) { ); return new Promise(resolve => { - server = app.listen(port, () => resolve(undefined)); + const server = app.listen(port, () => resolve(undefined)); + servers.set(port, server); }); } -function close() { +function close(port?: number) { return new Promise(resolve => { - if (!server) { - return resolve(undefined); - } + if (port !== undefined) { + const server = servers.get(port); + if (!server) { + return resolve(undefined); + } + + server.close(() => { + servers.delete(port); + resolve(undefined); + }); + } else { + // Close all servers if no port is specified + if (servers.size === 0) { + return resolve(undefined); + } + + const promises = Array.from(servers.entries()).map(([port, server]) => { + return new Promise(resolveServer => { + server.close(() => { + servers.delete(port); + resolveServer(undefined); + }); + }); + }); - server.close(() => resolve(undefined)); + Promise.all(promises).then(() => resolve(undefined)); + } }); } diff --git a/test/middleware.spec.ts b/test/middleware.spec.ts index ec8911dad..9f1ac1e35 100644 --- a/test/middleware.spec.ts +++ b/test/middleware.spec.ts @@ -13,6 +13,19 @@ const TEST_PORT = parseInt(process.env.TEST_PORT || "1234", 10); const m = middleware({ channelSecret: "test_channel_secret" }); +// Middleware with skipSignatureVerification function (always true) +const mWithSkipAlwaysTrue = middleware({ + channelSecret: "test_channel_secret", + skipSignatureVerification: () => true, +}); + +// Middleware with skipSignatureVerification function (dynamic behavior based on environment variable) +let shouldSkipSignature = false; +const mWithDynamicSkip = middleware({ + channelSecret: "test_channel_secret", + skipSignatureVerification: () => shouldSkipSignature, +}); + const getRecentReq = (): { body: Types.WebhookRequestBody } => JSON.parse(readFileSync(join(__dirname, "helpers/request.json")).toString()); @@ -53,8 +66,95 @@ describe("middleware test", () => { beforeAll(() => { listen(TEST_PORT, m); }); + + describe("With skipSignatureVerification functionality", () => { + // Port for always-true skip function + let alwaysTruePort: number; + // Port for dynamic skip function + let dynamicSkipPort: number; + + beforeAll(() => { + alwaysTruePort = TEST_PORT + 1; + dynamicSkipPort = TEST_PORT + 2; + listen(alwaysTruePort, mWithSkipAlwaysTrue); + return listen(dynamicSkipPort, mWithDynamicSkip); + }); + + afterAll(() => { + close(alwaysTruePort); + return close(dynamicSkipPort); + }); + + it("should skip signature verification when skipSignatureVerification returns true", async () => { + const client = new HTTPClient({ + baseURL: `http://localhost:${alwaysTruePort}`, + defaultHeaders: { + "X-Line-Signature": "invalid_signature", + }, + }); + + // This should work even with invalid signature because verification is skipped + await client.post("/webhook", { + events: [webhook], + destination: DESTINATION, + }); + + const req = getRecentReq(); + deepEqual(req.body.destination, DESTINATION); + deepEqual(req.body.events, [webhook]); + }); + + it("should respect dynamic skipSignatureVerification behavior - when true", async () => { + // Set to skip verification + shouldSkipSignature = true; + + const client = new HTTPClient({ + baseURL: `http://localhost:${dynamicSkipPort}`, + defaultHeaders: { + "X-Line-Signature": "invalid_signature", + }, + }); + + // This should work even with invalid signature because verification is skipped + await client.post("/webhook", { + events: [webhook], + destination: DESTINATION, + }); + + const req = getRecentReq(); + deepEqual(req.body.destination, DESTINATION); + deepEqual(req.body.events, [webhook]); + }); + + it("should respect dynamic skipSignatureVerification behavior - when false", async () => { + // Set to NOT skip verification + shouldSkipSignature = false; + + const client = new HTTPClient({ + baseURL: `http://localhost:${dynamicSkipPort}`, + defaultHeaders: { + "X-Line-Signature": "invalid_signature", + }, + }); + + try { + // This should fail because signature verification is not skipped + await client.post("/webhook", { + events: [webhook], + destination: DESTINATION, + }); + ok(false, "Expected to throw an error due to invalid signature"); + } catch (err) { + if (err instanceof HTTPError) { + equal(err.statusCode, 401); + } else { + throw err; + } + } + }); + }); afterAll(() => { - close(); + close(TEST_PORT); }); describe("Succeeds on parsing valid request", () => { From 067d4e9308f46916ec30dd9ea222c8d27eb31c04 Mon Sep 17 00:00:00 2001 From: habara keigo Date: Fri, 19 Sep 2025 16:41:27 +0900 Subject: [PATCH 2/3] Remove redundant comments --- lib/middleware.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/middleware.ts b/lib/middleware.ts index 489299573..1e6fefdbd 100644 --- a/lib/middleware.ts +++ b/lib/middleware.ts @@ -62,7 +62,6 @@ export default function middleware(config: Types.MiddlewareConfig): Middleware { } })(); - // Check if signature verification should be skipped const shouldSkipVerification = config.skipSignatureVerification && config.skipSignatureVerification(); From 779513761f7578d12f68ec8f02fe4b5e9e840f18 Mon Sep 17 00:00:00 2001 From: habara keigo Date: Fri, 19 Sep 2025 16:42:05 +0900 Subject: [PATCH 3/3] Define mWith~ within test description --- test/middleware.spec.ts | 86 ++++++++++++----------------------------- 1 file changed, 25 insertions(+), 61 deletions(-) diff --git a/test/middleware.spec.ts b/test/middleware.spec.ts index 9f1ac1e35..fcc21b06f 100644 --- a/test/middleware.spec.ts +++ b/test/middleware.spec.ts @@ -13,19 +13,6 @@ const TEST_PORT = parseInt(process.env.TEST_PORT || "1234", 10); const m = middleware({ channelSecret: "test_channel_secret" }); -// Middleware with skipSignatureVerification function (always true) -const mWithSkipAlwaysTrue = middleware({ - channelSecret: "test_channel_secret", - skipSignatureVerification: () => true, -}); - -// Middleware with skipSignatureVerification function (dynamic behavior based on environment variable) -let shouldSkipSignature = false; -const mWithDynamicSkip = middleware({ - channelSecret: "test_channel_secret", - skipSignatureVerification: () => shouldSkipSignature, -}); - const getRecentReq = (): { body: Types.WebhookRequestBody } => JSON.parse(readFileSync(join(__dirname, "helpers/request.json")).toString()); @@ -68,54 +55,32 @@ describe("middleware test", () => { }); describe("With skipSignatureVerification functionality", () => { - // Port for always-true skip function - let alwaysTruePort: number; - // Port for dynamic skip function - let dynamicSkipPort: number; - - beforeAll(() => { - alwaysTruePort = TEST_PORT + 1; - dynamicSkipPort = TEST_PORT + 2; - listen(alwaysTruePort, mWithSkipAlwaysTrue); - return listen(dynamicSkipPort, mWithDynamicSkip); - }); - - afterAll(() => { - close(alwaysTruePort); - return close(dynamicSkipPort); - }); + let serverPort: number; - it("should skip signature verification when skipSignatureVerification returns true", async () => { - const client = new HTTPClient({ - baseURL: `http://localhost:${alwaysTruePort}`, + const createClient = (port: number) => + new HTTPClient({ + baseURL: `http://localhost:${port}`, defaultHeaders: { "X-Line-Signature": "invalid_signature", }, }); - // This should work even with invalid signature because verification is skipped - await client.post("/webhook", { - events: [webhook], - destination: DESTINATION, - }); - - const req = getRecentReq(); - deepEqual(req.body.destination, DESTINATION); - deepEqual(req.body.events, [webhook]); + afterEach(() => { + if (serverPort) { + close(serverPort); + } }); - it("should respect dynamic skipSignatureVerification behavior - when true", async () => { - // Set to skip verification - shouldSkipSignature = true; - - const client = new HTTPClient({ - baseURL: `http://localhost:${dynamicSkipPort}`, - defaultHeaders: { - "X-Line-Signature": "invalid_signature", - }, + it("should skip signature verification when skipSignatureVerification returns true", async () => { + serverPort = TEST_PORT + 1; + const m = middleware({ + channelSecret: "test_channel_secret", + skipSignatureVerification: () => true, }); + await listen(serverPort, m); + + const client = createClient(serverPort); - // This should work even with invalid signature because verification is skipped await client.post("/webhook", { events: [webhook], destination: DESTINATION, @@ -126,19 +91,17 @@ describe("middleware test", () => { deepEqual(req.body.events, [webhook]); }); - it("should respect dynamic skipSignatureVerification behavior - when false", async () => { - // Set to NOT skip verification - shouldSkipSignature = false; - - const client = new HTTPClient({ - baseURL: `http://localhost:${dynamicSkipPort}`, - defaultHeaders: { - "X-Line-Signature": "invalid_signature", - }, + it("should skip signature verification when skipSignatureVerification returns false", async () => { + serverPort = TEST_PORT + 2; + const m = middleware({ + channelSecret: "test_channel_secret", + skipSignatureVerification: () => false, }); + await listen(serverPort, m); + + const client = createClient(serverPort); try { - // This should fail because signature verification is not skipped await client.post("/webhook", { events: [webhook], destination: DESTINATION, @@ -153,6 +116,7 @@ describe("middleware test", () => { } }); }); + afterAll(() => { close(TEST_PORT); });