diff --git a/src/receivers/HTTPReceiver.ts b/src/receivers/HTTPReceiver.ts index bd08b19c2..b7207e63d 100644 --- a/src/receivers/HTTPReceiver.ts +++ b/src/receivers/HTTPReceiver.ts @@ -34,6 +34,12 @@ import type { ParamsIncomingMessage } from './ParamsIncomingMessage'; import { type CustomRoute, type ReceiverRoutes, buildReceiverRoutes } from './custom-routes'; import { verifyRedirectOpts } from './verify-redirect-opts'; +export interface HTTPReceiverInvalidRequestSignatureHandlerArgs { + rawBody: string; + signature: string | undefined; + ts: number | undefined; +} + // Option keys for tls.createServer() and tls.createSecureContext(), exclusive of those for http.createServer() const httpsOptionKeys = [ 'ALPNProtocols', @@ -81,6 +87,7 @@ export interface HTTPReceiverOptions { logLevel?: LogLevel; processBeforeResponse?: boolean; signatureVerification?: boolean; + invalidRequestSignatureHandler?: (args: HTTPReceiverInvalidRequestSignatureHandlerArgs) => void; clientId?: string; clientSecret?: string; stateSecret?: InstallProviderOptions['stateSecret']; // required when using default stateStore @@ -137,6 +144,8 @@ export default class HTTPReceiver implements Receiver { private signatureVerification: boolean; + private invalidRequestSignatureHandler: (args: HTTPReceiverInvalidRequestSignatureHandlerArgs) => void; + private app?: App; public requestListener: RequestListener; @@ -178,6 +187,7 @@ export default class HTTPReceiver implements Receiver { logLevel = LogLevel.INFO, processBeforeResponse = false, signatureVerification = true, + invalidRequestSignatureHandler, clientId = undefined, clientSecret = undefined, stateSecret = undefined, @@ -195,6 +205,8 @@ export default class HTTPReceiver implements Receiver { this.signingSecret = signingSecret; this.processBeforeResponse = processBeforeResponse; this.signatureVerification = signatureVerification; + this.invalidRequestSignatureHandler = + invalidRequestSignatureHandler ?? this.defaultInvalidRequestSignatureHandler.bind(this); this.logger = logger ?? (() => { @@ -448,6 +460,13 @@ export default class HTTPReceiver implements Receiver { const e = err as Error; if (this.signatureVerification) { this.logger.warn(`Failed to parse and verify the request data: ${e.message}`); + const requestWithRawBody = req as IncomingMessage & { rawBody?: string }; + const rawBody = typeof requestWithRawBody.rawBody === 'string' ? requestWithRawBody.rawBody : ''; + this.invalidRequestSignatureHandler({ + rawBody, + signature: req.headers['x-slack-signature'] as string | undefined, + ts: req.headers['x-slack-request-timestamp'] ? Number(req.headers['x-slack-request-timestamp']) : undefined, + }); } else { this.logger.warn(`Failed to parse the request body: ${e.message}`); } @@ -565,4 +584,8 @@ export default class HTTPReceiver implements Receiver { installer.handleCallback(req, res, installCallbackOptions).catch(errorHandler); } } + + private defaultInvalidRequestSignatureHandler(_args: HTTPReceiverInvalidRequestSignatureHandlerArgs): void { + // noop - signature verification failure is already logged and a 401 is returned + } } diff --git a/test/unit/receivers/HTTPReceiver.spec.ts b/test/unit/receivers/HTTPReceiver.spec.ts index 3d9ab1b43..d97715e10 100644 --- a/test/unit/receivers/HTTPReceiver.spec.ts +++ b/test/unit/receivers/HTTPReceiver.spec.ts @@ -571,6 +571,125 @@ describe('HTTPReceiver', () => { }); }); + describe('invalidRequestSignatureHandler', () => { + it('should call the custom handler when signature verification fails', async () => { + const spy = sinon.spy(); + const fakeParseAndVerify = sinon.fake.rejects(new Error('Signature mismatch')); + const fakeBuildNoBodyResponse = sinon.fake(); + + const overridesWithFakeVerify = mergeOverrides(overrides, { + './HTTPModuleFunctions': { + parseAndVerifyHTTPRequest: fakeParseAndVerify, + parseHTTPRequestBody: sinon.fake(), + buildNoBodyResponse: fakeBuildNoBodyResponse, + '@noCallThru': true, + }, + }); + + const HTTPReceiver = importHTTPReceiver(overridesWithFakeVerify); + const receiver = new HTTPReceiver({ + signingSecret: 'secret', + logger: noopLogger, + invalidRequestSignatureHandler: spy, + }); + assert.isNotNull(receiver); + + const fakeReq = sinon.createStubInstance(IncomingMessage) as unknown as IncomingMessage; + fakeReq.url = '/slack/events'; + fakeReq.method = 'POST'; + fakeReq.headers = { + 'x-slack-signature': 'v0=bad', + 'x-slack-request-timestamp': '1234567890', + }; + (fakeReq as IncomingMessage & { rawBody?: string }).rawBody = '{"token":"test"}'; + + const fakeRes = sinon.createStubInstance(ServerResponse) as unknown as ServerResponse; + + receiver.requestListener(fakeReq, fakeRes); + + // Wait for the async closure inside handleIncomingEvent to settle + await new Promise((resolve) => setTimeout(resolve, 50)); + + assert(spy.calledOnce, 'invalidRequestSignatureHandler should be called once'); + const args = spy.firstCall.args[0]; + assert.equal(args.rawBody, '{"token":"test"}'); + assert.equal(args.signature, 'v0=bad'); + assert.equal(args.ts, 1234567890); + }); + + it('should use the default noop handler when no custom handler is provided', async () => { + const fakeParseAndVerify = sinon.fake.rejects(new Error('Signature mismatch')); + const fakeBuildNoBodyResponse = sinon.fake(); + + const overridesWithFakeVerify = mergeOverrides(overrides, { + './HTTPModuleFunctions': { + parseAndVerifyHTTPRequest: fakeParseAndVerify, + parseHTTPRequestBody: sinon.fake(), + buildNoBodyResponse: fakeBuildNoBodyResponse, + '@noCallThru': true, + }, + }); + + const HTTPReceiver = importHTTPReceiver(overridesWithFakeVerify); + const receiver = new HTTPReceiver({ + signingSecret: 'secret', + logger: noopLogger, + }); + + const fakeReq = sinon.createStubInstance(IncomingMessage) as unknown as IncomingMessage; + fakeReq.url = '/slack/events'; + fakeReq.method = 'POST'; + fakeReq.headers = {}; + + const fakeRes = sinon.createStubInstance(ServerResponse) as unknown as ServerResponse; + + // Should not throw even without a custom handler + receiver.requestListener(fakeReq, fakeRes); + await new Promise((resolve) => setTimeout(resolve, 50)); + + sinon.assert.calledOnce(fakeBuildNoBodyResponse); + sinon.assert.calledWith(fakeBuildNoBodyResponse, fakeRes, 401); + }); + + it('should pass undefined for signature and ts when headers are missing', async () => { + const spy = sinon.spy(); + const fakeParseAndVerify = sinon.fake.rejects(new Error('Signature mismatch')); + const fakeBuildNoBodyResponse = sinon.fake(); + + const overridesWithFakeVerify = mergeOverrides(overrides, { + './HTTPModuleFunctions': { + parseAndVerifyHTTPRequest: fakeParseAndVerify, + parseHTTPRequestBody: sinon.fake(), + buildNoBodyResponse: fakeBuildNoBodyResponse, + '@noCallThru': true, + }, + }); + + const HTTPReceiver = importHTTPReceiver(overridesWithFakeVerify); + const receiver = new HTTPReceiver({ + signingSecret: 'secret', + logger: noopLogger, + invalidRequestSignatureHandler: spy, + }); + + const fakeReq = sinon.createStubInstance(IncomingMessage) as unknown as IncomingMessage; + fakeReq.url = '/slack/events'; + fakeReq.method = 'POST'; + fakeReq.headers = {}; + + const fakeRes = sinon.createStubInstance(ServerResponse) as unknown as ServerResponse; + + receiver.requestListener(fakeReq, fakeRes); + await new Promise((resolve) => setTimeout(resolve, 50)); + + assert(spy.calledOnce); + const args = spy.firstCall.args[0]; + assert.equal(args.rawBody, ''); + assert.isUndefined(args.signature); + assert.isUndefined(args.ts); + }); + }); + it("should throw if request doesn't match install path, redirect URI path, or custom routes", async () => { const installProviderStub = sinon.createStubInstance(InstallProvider); const HTTPReceiver = importHTTPReceiver(overrides);