Skip to content

Commit

Permalink
Adding support for pre-parsed requests
Browse files Browse the repository at this point in the history 
Issue: slackapi#758
Ported most of the work from: slackapi/node-slack-events-api#90
  • Loading branch information
@troysteinbauer
troysteinbauer committed Jun 12, 2019
1 parent 8266961 commit 0eed6de
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 37 deletions.
13 changes: 1 addition & 12 deletions packages/interactive-messages/src/adapter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ import { packageIdentifier, promiseTimeout, errorCodes as utilErrorCodes } from

const debug = debugFactory('@slack/interactive-messages:adapter');

export const errorCodes = {
BODY_PARSER_NOT_PERMITTED: 'SLACKADAPTER_BODY_PARSER_NOT_PERMITTED_FAILURE',
};

/**
* Transforms various forms of matching constraints to a single standard object shape
* @param {string|RegExp|Object} matchingConstraints - the various forms of matching constraints
Expand Down Expand Up @@ -196,14 +192,7 @@ export class SlackMessageAdapter {
*/
expressMiddleware() {
const requestListener = this.requestListener();
return (req, res, next) => {
// If parser is being used, we can't verify request signature
if (req.body) {
const error = new Error('Parsing request body prohibits request signature verification');
error.code = errorCodes.BODY_PARSER_NOT_PERMITTED;
next(error);
return;
}
return (req, res, next) => { // eslint-disable-line no-unused-vars
requestListener(req, res);
};
}
Expand Down
34 changes: 31 additions & 3 deletions packages/interactive-messages/src/http-handler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const debug = debugFactory('@slack/interactive-messages:http-handler');
export const errorCodes = {
SIGNATURE_VERIFICATION_FAILURE: 'SLACKHTTPHANDLER_REQUEST_SIGNATURE_VERIFICATION_FAILURE',
REQUEST_TIME_FAILURE: 'SLACKHTTPHANDLER_REQUEST_TIMELIMIT_FAILURE',
BODY_PARSER_NOT_PERMITTED: 'SLACKADAPTER_BODY_PARSER_NOT_PERMITTED_FAILURE', // moved constant from adapter
};

export function createHTTPHandler(adapter) {
Expand Down Expand Up @@ -72,7 +73,7 @@ export function createHTTPHandler(adapter) {

if (ts < fiveMinutesAgo) {
debug('request is older than 5 minutes');
const error = new Error('Slack request signing verification failed');
const error = new Error('Slack request signing verification outdated');
error.code = errorCodes.REQUEST_TIME_FAILURE;
throw error;
}
Expand Down Expand Up @@ -104,11 +105,38 @@ export function createHTTPHandler(adapter) {
// Function used to send response
const respond = sendResponse(res);

// If parser is being used and we don't receive the raw payload via `rawBody`,
// we can't verify request signature
if (req.body && !req.rawBody) {
const error = new Error('Parsing request body prohibits request signature verification');
error.code = errorCodes.BODY_PARSER_NOT_PERMITTED;
if (process.env.NODE_ENV === 'development') {
respond({ status: 500, content: error.message });
} else {
respond({ status: 500 });
}
return;
}

// Some serverless cloud providers (e.g. Google Firebase Cloud Functions) might populate
// the request with a bodyparser before it can be populated by the SDK.
// To prevent throwing an error here, we check the `rawBody` field before parsing the request
// through the `raw-body` module (see Issue #90 - https://github.com/slackapi/node-slack-events-api/pull/90)
let parseRawBody;
if (req.rawBody) {
debug('Parsing request with a rawBody attribute');
parseRawBody = new Promise((resolve) => {
resolve(req.rawBody);
});
} else {
debug('Parsing raw request');
parseRawBody = getRawBody(req);
}

// Builds body of the request from stream and returns the raw request body
getRawBody(req)
parseRawBody
.then((r) => {
const rawBody = r.toString();

if (verifyRequestSignature(adapter.signingSecret, req.headers, rawBody)) {
// Request signature is verified
// Parse raw body
Expand Down
22 changes: 21 additions & 1 deletion packages/interactive-messages/test/helpers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,26 @@ function createRequest(signingSecret, ts, rawBody) {
'content-type': 'application/x-www-form-urlencoded'
};
return {
body: rawBody,
headers: headers
};
}

/**
* Creates request object with proper headers and a rawBody field payload
* @param {string} signingSecret - A Slack signing secret for request verification
* @param {Integer} ts - A timestamp for request verification and header
* @param {string} rawBody - String of raw body to be put in rawBody field
* @returns {Object} pseudo request object
*/
function createRawBodyRequest(signingSecret, ts, rawBody) {
const signature = createRequestSignature(signingSecret, ts, rawBody);
const headers = {
'x-slack-signature': signature,
'x-slack-request-timestamp': ts,
'content-type': 'application/json'
};
return {
rawBody: Buffer.from(rawBody),
headers: headers
};
}
Expand Down Expand Up @@ -89,6 +108,7 @@ function delayed(ms, value, rejectionReason) {
}

module.exports.createRequest = createRequest;
module.exports.createRawBodyRequest = createRawBodyRequest;
module.exports.createRequestSignature = createRequestSignature;
module.exports.createStreamRequest = createStreamRequest;
module.exports.delayed = delayed;
Expand Down
12 changes: 0 additions & 12 deletions packages/interactive-messages/test/unit/test-adapter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ var nop = require('nop');
var getRandomPort = require('get-random-port');
var systemUnderTest = require('../../dist/adapter');
var createStreamRequest = require('../helpers').createStreamRequest;
var errorCodes = systemUnderTest.errorCodes;
var SlackMessageAdapter = systemUnderTest.default;
var delayed = require('../helpers').delayed;

Expand Down Expand Up @@ -123,17 +122,6 @@ describe('SlackMessageAdapter', function () {
var middleware = this.adapter.expressMiddleware();
assert.isFunction(middleware);
});
it('should error when body parser is used', function (done) {
var middleware = this.adapter.expressMiddleware();
var req = { body: { } };
var res = this.res;
var next = this.next;
next.callsFake(function (err) {
assert.equal(err.code, errorCodes.BODY_PARSER_NOT_PERMITTED);
done();
});
middleware(req, res, next);
});
it('should verify correctly signed request bodies', function (done) {
var ts = Math.floor(Date.now() / 1000);
var adapter = this.adapter;
Expand Down
55 changes: 46 additions & 9 deletions packages/interactive-messages/test/unit/test-http-handler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ var assert = require('chai').assert;
var sinon = require('sinon');
var proxyquire = require('proxyquire');
var createRequest = require('../helpers').createRequest;
var createRawBodyRequest = require('../helpers').createRawBodyRequest;
var correctRawBody = 'payload=%7B%22type%22%3A%22interactive_message%22%7D';
var getRawBodyStub = sinon.stub();
var systemUnderTest = proxyquire('../../dist/http-handler', {
Expand Down Expand Up @@ -33,7 +34,7 @@ describe('createHTTPHandler', function () {
var date = Math.floor(Date.now() / 1000);
var req = createRequest(correctSigningSecret, date, correctRawBody);
dispatch.resolves({ status: 200 });
getRawBodyStub.resolves(correctRawBody);
getRawBodyStub.resolves(Buffer.from(correctRawBody));
res.end.callsFake(function () {
assert(dispatch.called);
assert.equal(res.statusCode, 200);
Expand All @@ -42,11 +43,36 @@ describe('createHTTPHandler', function () {
this.requestListener(req, res);
});

it('should verify a correct signing secret for a request with rawBody attribute', function (done) {
var dispatch = this.dispatch;
var res = this.res;
var req = createRawBodyRequest(correctSigningSecret, this.correctDate, correctRawBody);
dispatch.resolves({ status: 200 });
getRawBodyStub.resolves(Buffer.from(correctRawBody));
res.end.callsFake(function () {
assert.equal(res.statusCode, 200);
done();
});
this.requestListener(req, res);
});

it('should fail request signing verification for a request with a body but no rawBody', function (done) {
var res = this.res;
var req = createRequest(correctSigningSecret, this.correctDate, correctRawBody);
req.body = {};
getRawBodyStub.resolves(Buffer.from(correctRawBody));
res.end.callsFake(function () {
assert.equal(res.statusCode, 500);
done();
});
this.requestListener(req, res);
});

it('should fail request signing verification with an incorrect signing secret', function (done) {
var dispatch = this.dispatch;
var res = this.res;
var req = createRequest('INVALID_SECRET', this.correctDate, correctRawBody);
getRawBodyStub.resolves(correctRawBody);
getRawBodyStub.resolves(Buffer.from(correctRawBody));
res.end.callsFake(function () {
assert(dispatch.notCalled);
assert.equal(res.statusCode, 404);
Expand All @@ -55,13 +81,24 @@ describe('createHTTPHandler', function () {
this.requestListener(req, res);
});

it('should fail request signing verification when a request has body and no rawBody attribute', function (done) {
var res = this.res;
var req = createRequest('INVALID_SECRET', this.correctDate, correctRawBody);
getRawBodyStub.resolves(Buffer.from(correctRawBody));
res.end.callsFake(function () {
assert.equal(res.statusCode, 404);
done();
});
this.requestListener(req, res);
});

it('should fail request signing verification with old timestamp', function (done) {
var dispatch = this.dispatch;
var res = this.res;
var sixMinutesAgo = Math.floor(Date.now() / 1000) - (60 * 6);
var req = createRequest(correctSigningSecret, sixMinutesAgo, correctRawBody);
dispatch.resolves({ status: 200 });
getRawBodyStub.resolves(correctRawBody);
getRawBodyStub.resolves(Buffer.from(correctRawBody));
res.end.callsFake(function () {
assert(dispatch.notCalled);
assert.equal(res.statusCode, 404);
Expand Down Expand Up @@ -101,7 +138,7 @@ describe('createHTTPHandler', function () {
var dispatch = this.dispatch;
var req = createRequest(correctSigningSecret, this.correctDate, correctRawBody);
dispatch.returns(undefined);
getRawBodyStub.resolves(correctRawBody);
getRawBodyStub.resolves(Buffer.from(correctRawBody));
res.end.callsFake(function () {
assert.equal(res.statusCode, 404);
done();
Expand All @@ -114,7 +151,7 @@ describe('createHTTPHandler', function () {
var res = this.res;
var req = createRequest(correctSigningSecret, this.correctDate, correctRawBody);
dispatch.resolves({ status: 200 });
getRawBodyStub.resolves(correctRawBody);
getRawBodyStub.resolves(Buffer.from(correctRawBody));
res.end.callsFake(function () {
assert(res.setHeader.calledWith('X-Slack-Powered-By'));
done();
Expand All @@ -127,7 +164,7 @@ describe('createHTTPHandler', function () {
var res = this.res;
var sslRawBody = 'payload=%7B%22ssl_check%22%3A%221%22%7D';
var req = createRequest(correctSigningSecret, this.correctDate, sslRawBody);
getRawBodyStub.resolves(sslRawBody);
getRawBodyStub.resolves(Buffer.from(sslRawBody));
res.end.callsFake(function () {
assert(dispatch.notCalled);
assert.equal(res.statusCode, 200);
Expand All @@ -148,7 +185,7 @@ describe('createHTTPHandler', function () {
p: 5
};
dispatch.resolves({ status: 200, content: content });
getRawBodyStub.resolves(correctRawBody);
getRawBodyStub.resolves(Buffer.from(correctRawBody));
res.end.callsFake(function (json) {
assert(dispatch.called);
assert.equal(res.statusCode, 200);
Expand All @@ -164,7 +201,7 @@ describe('createHTTPHandler', function () {
var res = this.res;
var req = createRequest(correctSigningSecret, this.correctDate, correctRawBody);
dispatch.resolves({ status: 500 });
getRawBodyStub.resolves(correctRawBody);
getRawBodyStub.resolves(Buffer.from(correctRawBody));
res.end.callsFake(function (body) {
assert(dispatch.called);
assert.isUndefined(body);
Expand All @@ -180,7 +217,7 @@ describe('createHTTPHandler', function () {
var req = createRequest(correctSigningSecret, this.correctDate, correctRawBody);
var content = 'hello, world';
dispatch.resolves({ status: 200, content: content });
getRawBodyStub.resolves(correctRawBody);
getRawBodyStub.resolves(Buffer.from(correctRawBody));
res.end.callsFake(function (body) {
assert(dispatch.called);
assert.equal(res.statusCode, 200);
Expand Down

0 comments on commit 0eed6de

Please sign in to comment.