diff --git a/src/ExpressReceiver.spec.ts b/src/ExpressReceiver.spec.ts index c7a75e64d..2e5ba4212 100644 --- a/src/ExpressReceiver.spec.ts +++ b/src/ExpressReceiver.spec.ts @@ -19,6 +19,17 @@ describe('ExpressReceiver', () => { setName(_name: string): void { }, }; + function buildResponseToVerify(result: any): Response { + return { + status: (code: number) => { + result.code = code; + return { + send: () => { result.sent = true; } + } as Response; + }, + } as Response; + } + describe('verifySignatureAndParseBody', () => { let clock: SinonFakeTimers; @@ -45,7 +56,8 @@ describe('ExpressReceiver', () => { reqAsStream.push(null); // indicate EOF (reqAsStream as { [key: string]: any }).headers = { 'x-slack-signature': signature, - 'x-slack-request-timestamp': requestTimestamp + 'x-slack-request-timestamp': requestTimestamp, + 'content-type': 'application/x-www-form-urlencoded', }; const req = reqAsStream as Request; return req; @@ -56,7 +68,8 @@ describe('ExpressReceiver', () => { rawBody: body, headers: { 'x-slack-signature': signature, - 'x-slack-request-timestamp': requestTimestamp + 'x-slack-request-timestamp': requestTimestamp, + 'content-type': 'application/x-www-form-urlencoded', } }; const req = untypedReq as Request; @@ -66,46 +79,45 @@ describe('ExpressReceiver', () => { // ---------------------------- // runWithValidRequest - async function runWithValidRequest(req: Request, errorResult: any) { + async function runWithValidRequest(req: Request): Promise { // Arrange - const resp = {} as Response; - const next = (error: any) => { errorResult = error; }; + const result: any = {}; + const resp = buildResponseToVerify(result); + const next = sinon.fake(); // Act const verifier = verifySignatureAndParseBody(noopLogger, signingSecret); await verifier(req, resp, next); - return errorResult; + + // Assert + assert.isUndefined(result.code); + assert.isUndefined(result.sent); } it('should verify requests', async () => { - let errorResult: any; - runWithValidRequest(buildExpressRequest(), errorResult); - // Assert - assert.isUndefined(errorResult); + await runWithValidRequest(buildExpressRequest()); }); it('should verify requests on GCP', async () => { - let errorResult: any; - runWithValidRequest(buildGCPRequest(), errorResult); - // Assert - assert.isUndefined(errorResult); + await runWithValidRequest(buildGCPRequest()); }); // ---------------------------- // verifyMissingHeaderDetection - function verifyMissingHeaderDetection(req: Request): Promise { + async function verifyMissingHeaderDetection(req: Request): Promise { // Arrange - const resp = {} as Response; - let errorResult: any; - const next = (error: any) => { errorResult = error; }; + const result: any = {}; + const resp = buildResponseToVerify(result); + const next = sinon.fake(); // Act const verifier = verifySignatureAndParseBody(noopLogger, signingSecret); - return verifier(req, resp, next).then((_: any) => { - // Assert - assert.equal(errorResult, 'Error: Slack request signing verification failed. Some headers are missing.'); - }) + await verifier(req, resp, next); + + // Assert + assert.equal(401, result.code); + assert.equal(true, result.sent); } it('should detect headers missing signature', async () => { @@ -144,18 +156,19 @@ describe('ExpressReceiver', () => { // ---------------------------- // verifyInvalidTimestampError - function verifyInvalidTimestampError(req: Request): Promise { + async function verifyInvalidTimestampError(req: Request): Promise { // Arrange - const resp = {} as Response; - let errorResult: any; - const next = (error: any) => { errorResult = error; }; + const result: any = {}; + const resp = buildResponseToVerify(result); + const next = sinon.fake(); // Act const verifier = verifySignatureAndParseBody(noopLogger, signingSecret); - return verifier(req, resp, next).then((_: any) => { - // Assert - assert.equal(errorResult, 'Error: Slack request signing verification failed. Timestamp is invalid.'); - }); + await verifier(req, resp, next); + + // Assert + assert.equal(401, result.code); + assert.equal(true, result.sent); } it('should detect invalid timestamp header', async () => { @@ -172,21 +185,22 @@ describe('ExpressReceiver', () => { // ---------------------------- // verifyTooOldTimestampError - function verifyTooOldTimestampError(req: Request): Promise { + async function verifyTooOldTimestampError(req: Request): Promise { // Arrange // restore the valid clock clock.restore(); - const resp = {} as Response; - let errorResult: any; - const next = (error: any) => { errorResult = error; }; + const result: any = {}; + const resp = buildResponseToVerify(result); + const next = sinon.fake(); // Act const verifier = verifySignatureAndParseBody(noopLogger, signingSecret); - return verifier(req, resp, next).then((_: any) => { - // Assert - assert.equal(errorResult, 'Error: Slack request signing verification failed. Timestamp is too old.'); - }); + await verifier(req, resp, next); + + // Assert + assert.equal(401, result.code); + assert.equal(true, result.sent); } it('should detect too old timestamp', async () => { @@ -200,19 +214,20 @@ describe('ExpressReceiver', () => { // ---------------------------- // verifySignatureMismatch - function verifySignatureMismatch(req: Request): Promise { + async function verifySignatureMismatch(req: Request): Promise { // Arrange - const resp = {} as Response; - let errorResult: any; - const next = (error: any) => { errorResult = error; }; + const result: any = {}; + const resp = buildResponseToVerify(result); + const next = sinon.fake(); // Act const verifier = verifySignatureAndParseBody(noopLogger, signingSecret); verifier(req, resp, next); - return verifier(req, resp, next).then((_: any) => { - // Assert - assert.equal(errorResult, 'Error: Slack request signing verification failed. Signature mismatch.'); - }); + await verifier(req, resp, next); + + // Assert + assert.equal(401, result.code); + assert.equal(true, result.sent); } it('should detect signature mismatch', async () => { diff --git a/src/ExpressReceiver.ts b/src/ExpressReceiver.ts index 33a541cdb..a221abd9b 100644 --- a/src/ExpressReceiver.ts +++ b/src/ExpressReceiver.ts @@ -176,7 +176,7 @@ export function verifySignatureAndParseBody( logger: Logger, signingSecret: string, ): RequestHandler { - return async (req, _res, next) => { + return async (req, res, next) => { try { // *** Request verification *** let stringBody: string; @@ -211,8 +211,15 @@ export function verifySignatureAndParseBody( req.body = parseRequestBody(logger, stringBody, contentType); return next(); + } catch (error) { - return next(error); + // Deny the request as something wrong with the signature + if ('code' in error) { + logger.warn(`Request verification failed (code: ${error.code}, message: ${error.message})`); + } else { + logger.warn(`Request verification failed (error: ${error})`); + } + return res.status(401).send(); } }; }