Merge pull request #59 from shanedewael/express_decoupling

Decouples Express and adds request signing verification support

.babelrc

@@ -2,9 +2,5 @@
   "presets": [
     "es2015",
     "es2016"
-  ],
-  "plugins": [
-    "syntax-dynamic-import",
-    ["system-import-transformer", { "modules": "common" }]
   ]
 }

README.md

@@ -26,15 +26,17 @@ In order to complete the subscription, you will need a **Request URL** that can
 verification request. This module, combined with the use of a development proxy, can make this
 easier for you.
 
-0.  Force the generation of a Verification Token: If you just created your Slack App, the Basic
-Information section of your configuration will not yet have a Verification Token under App
+0.  Force the generation of a Signing Secret: If you just created your Slack App, the Basic
+Information section of your configuration will not yet have a Signing Secret under App
 Credentials. By visiting the Event Subscriptions section and putting a dummy URL into Request
-URL, you will get a verification failure, but also there will now be a **Verification Token**
+URL, you will get a verification failure, but also there will now be a **Signing Secret**
 available in the Basic Information section.
 
+> ⚠️ As of `v2.0.0`, the Events API adapter no longer accepts legacy verification tokens. You must pass a signing secret [to verify requests from Slack](https://api.slack.com/docs/verifying-requests-from-slack).
+
 1.  Start the verification tool:
-`./node_modules/.bin/slack-verify --token <token> [--path=/slack/events] [--port=3000]`. You will
-need to substitute your own Verification Token for `<token>`. You may also want to choose your own
+`./node_modules/.bin/slack-verify --token <signing-secret> [--path=/slack/events] [--port=3000]`. You will
+need to substitute your own Verification Token for `<signing-secret>`. You may also want to choose your own
 `path` and/or `port`.
 
 2.  Start your development proxy. We recommend using [ngrok](https://ngrok.com/) for its stability,
@@ -68,7 +70,7 @@ The easiest way to start using the Events API is by using the built-in HTTP serv
 ```javascript
 // Initialize using verification token from environment variables
 const createSlackEventAdapter = require('@slack/events-api').createSlackEventAdapter;
-const slackEvents = createSlackEventAdapter(process.env.SLACK_VERIFICATION_TOKEN);
+const slackEvents = createSlackEventAdapter(process.env.SLACK_SIGNING_SECRET);
 const port = process.env.PORT || 3000;
 
 // Attach listeners to events by Slack Event "type". See: https://api.slack.com/events/message.im
@@ -98,17 +100,13 @@ const http = require('http');
 
 // Initialize using verification token from environment variables
 const createSlackEventAdapter = require('@slack/events-api').createSlackEventAdapter;
-const slackEvents = createSlackEventAdapter(process.env.SLACK_VERIFICATION_TOKEN);
+const slackEvents = createSlackEventAdapter(process.env.SLACK_SIGNING_SECRET);
 const port = process.env.PORT || 3000;
 
 // Initialize an Express application
 const express = require('express');
-const bodyParser = require('body-parser');
 const app = express();
 
-// You must use a body parser for JSON before mounting the adapter
-app.use(bodyParser.json());
-
 // Mount the event handler on a route
 // NOTE: you must mount to a path that matches the Request URL that was configured earlier
 app.use('/slack/events', slackEvents.expressMiddleware());
@@ -127,6 +125,8 @@ http.createServer(app).listen(port, () => {
 });
 ```
 
+> ⚠️ As of `v2.0.0`, the Events API adapter parses raw request bodies while performing request signing verification. This means developers no longer need to use `body-parser` middleware to parse JSON-encoded requests.
+
 **NOTE**: To use the example above, you need to add a Team Event such as `message.im` in the Event
 Subscriptions section of your Slack App configuration settings.
 

examples/greet-and-react/.env.sample

@@ -1,3 +1,3 @@
 SLACK_CLIENT_ID=
 SLACK_CLIENT_SECRET=
-SLACK_VERIFICATION_TOKEN=
+SLACK_SIGNING_SECRET=

examples/greet-and-react/index.js

@@ -7,10 +7,9 @@ const passport = require('passport');
 const SlackStrategy = require('@aoberoi/passport-slack').default.Strategy;
 const http = require('http');
 const express = require('express');
-const bodyParser = require('body-parser');
 
-// *** Initialize event adapter using verification token from environment variables ***
-const slackEvents = slackEventsApi.createSlackEventAdapter(process.env.SLACK_VERIFICATION_TOKEN, {
+// *** Initialize event adapter using signing secret from environment variables ***
+const slackEvents = slackEventsApi.createSlackEventAdapter(process.env.SIGNING_SECRET, {
   includeBody: true
 });
 
@@ -44,7 +43,6 @@ passport.use(new SlackStrategy({
 
 // Initialize an Express application
 const app = express();
-app.use(bodyParser.json());
 
 // Plug the Add to Slack (OAuth) helpers into the express app
 app.use(passport.initialize());

examples/greet-and-react/package.json

@@ -12,8 +12,7 @@
   "dependencies": {
     "@aoberoi/passport-slack": "^1.0.5",
     "@slack/client": "^4.3.1",
-    "@slack/events-api": "^1.0.0",
-    "body-parser": "^1.18.3",
+    "@slack/events-api": "^2.0.0",
     "dotenv": "^4.0.0",
     "express": "^4.16.3",
     "passport": "^0.3.2"

package.json

@@ -26,36 +26,36 @@
     "prepare": "npm run build"
   },
   "optionalDependencies": {
-    "express": "^4.0.0",
-    "body-parser": "^1.4.3"
+    "express": "^4.0.0"
   },
   "devDependencies": {
     "babel-cli": "^6.18.0",
     "babel-eslint": "^7.1.1",
-    "babel-plugin-syntax-dynamic-import": "^6.18.0",
-    "babel-plugin-system-import-transformer": "^2.4.0",
     "babel-preset-es2015": "^6.18.0",
     "babel-preset-es2016": "^6.16.0",
-    "body-parser": "^1.15.2",
     "codecov": "^3.0.4",
+    "chai": "^4.1.2",
     "eslint": "^3.12.2",
     "eslint-config-airbnb": "^13.0.0",
     "eslint-config-airbnb-base": "^11.0.0",
     "eslint-plugin-import": "^2.2.0",
     "eslint-plugin-jsx-a11y": "^2.2.3",
     "eslint-plugin-react": "^6.8.0",
     "express": "^4.14.0",
+    "get-random-port": "0.0.1",
     "lodash.isfunction": "^3.0.8",
     "mocha": "^5.2.0",
     "nop": "^1.0.0",
     "nyc": "^12.0.2",
     "proxyquire": "^1.7.10",
+    "sinon": "^4.5.0",
     "superagent": "^3.3.1",
     "uncaughtException": "^1.0.0"
   },
   "dependencies": {
     "debug": "^2.6.1",
     "lodash.isstring": "^4.0.1",
+    "raw-body": "^2.3.3",
     "yargs": "^6.6.0"
   }
 }

src/.eslintrc

@@ -1,6 +1,6 @@
 {
   "parser": "babel-eslint",
-  "extends": [ "airbnb" ],
+  "extends": [ "airbnb-base" ],
   "env": {
     "node": true,
   }

src/adapter.js

@@ -2,19 +2,23 @@ import EventEmitter from 'events';
 import http from 'http';
 import isString from 'lodash.isstring';
 import debugFactory from 'debug';
-import { createExpressMiddleware } from './express-middleware';
+import { createHTTPHandler } from './http-handler';
 
 const debug = debugFactory('@slack/events-api:adapter');
 
+export const errorCodes = {
+  BODY_PARSER_NOT_PERMITTED: 'SLACKADAPTER_BODY_PARSER_NOT_PERMITTED_FAILURE',
+};
+
 export default class SlackEventAdapter extends EventEmitter {
-  constructor(verificationToken, options = {}) {
-    if (!isString(verificationToken)) {
+  constructor(signingSecret, options = {}) {
+    if (!isString(signingSecret)) {
       throw new TypeError('SlackEventAdapter needs a verification token');
     }
 
     super();
 
-    this.verificationToken = verificationToken;
+    this.signingSecret = signingSecret;
     this.includeBody = !!options.includeBody || false;
     this.includeHeaders = !!options.includeHeaders || false;
     this.waitForResponse = !!options.waitForResponse || false;
@@ -29,26 +33,10 @@ export default class SlackEventAdapter extends EventEmitter {
   // TODO: options (like https)
   createServer(path = '/slack/events') {
     // NOTE: this is a workaround for a shortcoming of the System.import() tranform
-    return Promise.resolve().then(() => Promise.all([
-      System.import('express'),
-      System.import('body-parser'),
-      // import('express'),
-      // import('body-parser'),
-
-      // The previous lines should be written as the comment following it, since `System.import()`
-      // is going to disappear after dynamic imports land (https://github.com/tc39/proposal-dynamic-import).
-      // There are no babel transforms for this syntax that seem to work at the moment. The
-      // following was meant to work but ended up not working:
-      // https://github.com/pwmckenna/babel-plugin-transform-import-commonjs.
-    ]))
-    .then(([express, bodyParser]) => {
-      const app = express();
-      app.use(bodyParser.json());
-      app.post(path, this.expressMiddleware());
-
+    return Promise.resolve().then(() => {
       debug('server created - path: %s', path);
 
-      return http.createServer(app);
+      return http.createServer(this.requestListener());
     });
   }
 
@@ -80,7 +68,21 @@ export default class SlackEventAdapter extends EventEmitter {
   }
 
   expressMiddleware(middlewareOptions = {}) {
-    return createExpressMiddleware(this, middlewareOptions);
+    const requestListener = this.requestListener(middlewareOptions);
+    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;
+      }
+      requestListener(req, res);
+    };
+  }
+
+  requestListener(middlewareOptions = {}) {
+    return createHTTPHandler(this, middlewareOptions);
   }
 
 }