From e8d052ab4f0cadceeef089e3f15e535e4d5eeef1 Mon Sep 17 00:00:00 2001
From: Eric Kryski <e.kryski@gmail.com>
Date: Tue, 22 Nov 2016 23:54:29 -0700
Subject: [PATCH] checking strategy options on sockets. Now supporting entity
 id for JWT payload. Only auth passed strategy. Closes #346. Closes #348.

---
 README.md                          |  3 +-
 docs/migrating.md                  | 53 ++++++++++++++++++++----------
 example/app.js                     | 13 +-------
 src/passport/authenticate.js       | 19 ++++++-----
 src/passport/initialize.js         | 15 +++++++++
 src/service.js                     |  5 +--
 src/socket/handler.js              | 14 ++++----
 src/utils.js                       |  3 +-
 test/fixtures/server.js            | 24 ++++++++------
 test/integration/primus.test.js    | 27 ++++++++++++---
 test/integration/rest.test.js      |  9 ++---
 test/integration/socketio.test.js  | 31 ++++++++++++++---
 test/passport/authenticate.test.js | 10 +++---
 13 files changed, 148 insertions(+), 78 deletions(-)

diff --git a/README.md b/README.md
index 1f2201ba..9d94b525 100644
--- a/README.md
+++ b/README.md
@@ -147,8 +147,7 @@ app.service('authentication').hooks({
   before: {
     create: [
       // You can chain multiple strategies
-      auth.hooks.authenticate(['jwt', 'local']),
-      customizeJWTPayload()
+      auth.hooks.authenticate(['jwt', 'local'])
     ],
     remove: [
       auth.hooks.authenticate('jwt')
diff --git a/docs/migrating.md b/docs/migrating.md
index 5560363d..fc5afb3a 100644
--- a/docs/migrating.md
+++ b/docs/migrating.md
@@ -91,29 +91,13 @@ app.use('/users', memory())
       Strategy: FacebookStrategy
     }));
 
-// This hook customizes your payload.
-function customizeJWTPayload() {
-  return function(hook) {
-    console.log('Customizing JWT Payload');
-    hook.data.payload = {
-      // You need to make sure you have the right id.
-      // You can put whatever you want to be encoded in
-      // the JWT access token.
-      id: hook.params.user.id
-    };
-
-    return Promise.resolve(hook);
-  };
-}
-
 // Authenticate the user using the a JWT or
 // email/password strategy and if successful
 // return a new JWT access token.
 app.service('authentication').hooks({
   before: {
     create: [
-      auth.hooks.authenticate(['jwt', 'local']),
-      customizeJWTPayload()
+      auth.hooks.authenticate(['jwt', 'local'])
     ]
   }
 });
@@ -125,7 +109,7 @@ There are a number of breaking changes since the services have been removed:
 
 - Change `auth.token` -> `auth.jwt` in your config
 - Move `auth.token.secret` -> `auth.secret`
-- `auth.token.payload` option has been removed. See [customizing JWT payload]() for how to do this.
+- `auth.token.payload` option has been removed. See [customizing JWT payload](#customizing-jwt-payload) for how to do this.
 - `auth.idField` has been removed. It is now included in all services so we can pull it internally without you needing to specify it.
 - `auth.shouldSetupSuccessRoute` has been removed. Success redirect middleware is registered automatically but only triggers if you explicitly set a redirect. [See redirecting]() for more details.
 - `auth.shouldSetupFailureRoute` has been removed. Failure redirect middleware is registered automatically but only triggers if you explicitly set a redirect. [See redirecting]() for more details.
@@ -233,6 +217,39 @@ app.authenticate({
 
 We previously made the poor assumption that you are always authenticating a user. This is not always the case, or your app may not care about the current user as you already have their id in the accessToken payload or can encode some additional details in the JWT accessToken.  Therefore, if you need to get the current user you need to request it explicitly after authentication or populate it yourself in an after hook server side. See the new usage above for how to fetch your user.
 
+## Customizing JWT Payload
+
+By default the payload for your JWT is simply your entity id (ie. `{ userId }`). However, you can customize your JWT payloads however you wish by adding a `before` hook to the authentication service. For example:
+
+```js
+// This hook customizes your payload.
+function customizeJWTPayload() {
+  return function(hook) {
+    console.log('Customizing JWT Payload');
+    hook.data.payload = {
+      // You need to make sure you have the right id.
+      // You can put whatever you want to be encoded in
+      // the JWT access token.
+      customId: hook.params.user.id
+    };
+
+    return Promise.resolve(hook);
+  };
+}
+
+// Authenticate the user using the a JWT or
+// email/password strategy and if successful
+// return a new JWT access token.
+app.service('authentication').hooks({
+  before: {
+    create: [
+      auth.hooks.authenticate(['jwt', 'local']),
+      customizeJWTPayload()
+    ]
+  }
+});
+``` 
+
 ## JWT Parsing
 
 The JWT is only parsed from the header and body by default now. It is no longer pulled from the query string unless you explicitly tell `feathers-authentication-jwt` to do so.
diff --git a/example/app.js b/example/app.js
index 0d78f17c..392f6902 100644
--- a/example/app.js
+++ b/example/app.js
@@ -10,16 +10,6 @@ const local = require('feathers-authentication-local');
 const jwt = require('feathers-authentication-jwt');
 const auth = require('../lib/index');
 
-function customizeJWTPayload() {
-  return function(hook) {
-    hook.data.payload = {
-      id: hook.params.user.id
-    };
-
-    return Promise.resolve(hook);
-  };
-}
-
 const app = feathers();
 app.configure(rest())
   .configure(socketio())
@@ -36,8 +26,7 @@ app.service('authentication').hooks({
   before: {
     create: [
       // You can chain multiple strategies
-      auth.hooks.authenticate(['jwt', 'local']),
-      customizeJWTPayload()
+      auth.hooks.authenticate(['jwt', 'local'])
     ],
     remove: [
       auth.hooks.authenticate('jwt')
diff --git a/src/passport/authenticate.js b/src/passport/authenticate.js
index e7ac244b..2da5d1d8 100644
--- a/src/passport/authenticate.js
+++ b/src/passport/authenticate.js
@@ -13,9 +13,7 @@ export default function authenticate (options = {}) {
   debug('Initializing custom passport authenticate', options);
 
   // This function is bound by passport and called by passport.authenticate()
-  return function (passport, name, strategyOptions = {}, callback = () => {}) {
-    debug('passport.authenticate called with the following options', passport, name, strategyOptions, callback);
-    
+  return function (passport, name, strategyOptions = {}, callback = () => {}) {    
     // This is called by the feathers middleware, hook or socket. The request object
     // is a mock request derived from an http request, socket object, or hook.
     return function (request = {}) {
@@ -26,7 +24,7 @@ export default function authenticate (options = {}) {
         // Default is hook.params.user, req.user and socket.user.
         const entity = strategyOptions.entity || strategyOptions.assignProperty || options.entity;
         let failures = [];
-        let strategies = [name];
+        let strategies;
 
         // Cast `name` to an array, allowing authentication to pass through a chain of
         // strategies.  The first name to succeed, redirect, or error will halt
@@ -38,8 +36,12 @@ export default function authenticate (options = {}) {
         // It is not feasible to construct a chain of multiple strategies that involve
         // redirection (for example both Facebook and Twitter), since the first one to
         // redirect will halt the chain.
-        if (Array.isArray(name)) {
+        if (request.strategy) {
+          strategies = [request.strategy];
+        } else if (Array.isArray(name)) {
           strategies = name;
+        } else {
+          strategies = [name];
         }
 
         function attempt(index) {
@@ -60,7 +62,6 @@ export default function authenticate (options = {}) {
           if (!prototype) {
             return reject(new Error(`Unknown authentication strategy '${layer}'`));
           }
-          
 
           // Implement required passport methods that
           // can be called by a passport strategy.
@@ -88,13 +89,13 @@ export default function authenticate (options = {}) {
             reject(error);
           };
 
-          strategy.success = (data, info) => {
-            debug(`'${layer}' authentication strategy succeeded`, data, info);
+          strategy.success = (data, payload) => {
+            debug(`'${layer}' authentication strategy succeeded`, data, payload);
             resolve({
               success: true,
               data: {
                 [entity]: data,
-                info
+                payload
               }
             });
           };
diff --git a/src/passport/initialize.js b/src/passport/initialize.js
index b36c43d1..5e0a93fc 100644
--- a/src/passport/initialize.js
+++ b/src/passport/initialize.js
@@ -15,7 +15,22 @@ export default function initialize (options = {}) {
     // app.configure(authentication()).
     
     // Expose our JWT util functions globally
+    passport._feathers = {};
     passport.createJWT = createJWT;
     passport.verifyJWT = verifyJWT;
+    passport.options = function(name, strategyOptions) {
+      if (!name) {
+        return passport._feathers;
+      }
+
+      if (typeof name === 'string' && !strategyOptions) {
+        return passport._feathers[name];
+      }
+
+      if (typeof name === 'string' && strategyOptions) {
+        debug(`Setting ${name} strategy options`, strategyOptions);
+        passport._feathers[name] = Object.assign({}, strategyOptions);
+      }
+    };
   };
 }
\ No newline at end of file
diff --git a/src/service.js b/src/service.js
index 51455958..3deaf17b 100644
--- a/src/service.js
+++ b/src/service.js
@@ -10,15 +10,16 @@ class Service {
     this.passport = app.passport;
   }
 
-  create(data, params) {
+  create(data = {}, params = {}) {
     const defaults = this.app.get('auth');
+    const payload = merge(data.payload, params.payload);
 
     // create accessToken
     // TODO (EK): Support refresh tokens
     // TODO (EK): This should likely be a hook
     // TODO (EK): This service can be datastore backed to support blacklists :)
     return this.passport
-      .createJWT(data.payload, merge(defaults, params))
+      .createJWT(payload, merge(defaults, params))
       .then(accessToken => {
         return { accessToken };
       });
diff --git a/src/socket/handler.js b/src/socket/handler.js
index 9a92672a..7307cd1b 100644
--- a/src/socket/handler.js
+++ b/src/socket/handler.js
@@ -76,17 +76,19 @@ export default function setupSocketHandler(app, options, { feathersParams, provi
         return callback(normalizeError(error));
       }
 
-      const promise = app.authenticate(strategy, options[strategy])(socket._feathers)
+      const stategyOptions = app.passport.options(strategy);
+
+      const promise = app.authenticate(strategy, stategyOptions)(socket._feathers)
         .then(result => {
           if (result.success) {
             // NOTE (EK): I don't think we need to support
             // custom redirects. We can emit this to the client
             // and let the client redirect.
-            // if (options.successRedirect) {
+            // if (stategyOptions.successRedirect) {
             //   return {
             //     redirect: true,
             //     status: 302,
-            //     url: options.successRedirect
+            //     url: stategyOptions.successRedirect
             //   };
             // }
             return Promise.resolve(result.data);
@@ -96,16 +98,16 @@ export default function setupSocketHandler(app, options, { feathersParams, provi
             // NOTE (EK): I don't think we need to support
             // custom redirects. We can emit this to the client
             // and let the client redirect.
-            // if (options.failureRedirect) {
+            // if (stategyOptions.failureRedirect) {
             //   return {
             //     redirect: true,
             //     status: 302,
-            //     url: options.failureRedirect
+            //     url: stategyOptions.failureRedirect
             //   };
             // }
 
             const { challenge } = result;
-            const message = options.failureMessage || (challenge && challenge.message);
+            const message = stategyOptions.failureMessage || (challenge && challenge.message);
             
             return Promise.reject(new errors[401](message, challenge));
           }
diff --git a/src/utils.js b/src/utils.js
index c1f2f252..f6c6cc61 100644
--- a/src/utils.js
+++ b/src/utils.js
@@ -1,5 +1,6 @@
 import Debug from 'debug';
 import pick from 'lodash.pick';
+import omit from 'lodash.omit';
 import jwt from 'jsonwebtoken';
 
 const debug = Debug('feathers-authentication:authentication:utils');
@@ -32,7 +33,7 @@ export function createJWT (payload = {}, options = {}) {
     }
 
     // TODO (EK): Support jwtids. Maybe auto-generate a uuid
-    jwt.sign(payload, secret, pick(settings, VALID_KEYS), function(error, token) {
+    jwt.sign(omit(payload, VALID_KEYS), secret, pick(settings, VALID_KEYS), function(error, token) {
       if (error) {
         debug('Error signing JWT', error);
         return reject(error);
diff --git a/test/fixtures/server.js b/test/fixtures/server.js
index c3f445c1..375d5cb1 100644
--- a/test/fixtures/server.js
+++ b/test/fixtures/server.js
@@ -17,15 +17,15 @@ const User = {
   permissions: ['*']
 };
 
-function customizeJWTPayload() {
-  return function(hook) {
-    hook.data.payload = {
-      id: hook.params.user.id
-    };
+// function customizeJWTPayload() {
+//   return function(hook) {
+//     hook.data.payload = {
+//       id: hook.params.user.id
+//     };
 
-    return Promise.resolve(hook);
-  };
-}
+//     return Promise.resolve(hook);
+//   };
+// }
 
 export default function(settings, socketProvider) {
   const app = feathers();
@@ -39,6 +39,10 @@ export default function(settings, socketProvider) {
     .use(bodyParser.urlencoded({ extended: true }))
     .configure(auth(settings))
     .configure(local())
+    .configure(local({
+      name: 'org-local',
+      entity: 'org'
+    }))
     .configure(jwt())
     .use('/users', memory())
     .use('/', feathers.static(__dirname + '/public'));
@@ -46,8 +50,8 @@ export default function(settings, socketProvider) {
   app.service('authentication').hooks({
     before: {
       create: [
-        auth.hooks.authenticate(['jwt', 'local']),
-        customizeJWTPayload()
+        auth.hooks.authenticate(['jwt', 'local', 'org-local']),
+        // customizeJWTPayload()
       ],
       remove: [
         auth.hooks.authenticate('jwt')
diff --git a/test/integration/primus.test.js b/test/integration/primus.test.js
index f8ed92e2..2755c267 100644
--- a/test/integration/primus.test.js
+++ b/test/integration/primus.test.js
@@ -19,6 +19,7 @@ describe('Primus authentication', function() {
   let server;
   let socket;
   let Socket;
+  let serverSocket;
   let ExpiringSocket;
   let expiringServer;
   let expiringSocket;
@@ -30,7 +31,7 @@ describe('Primus authentication', function() {
     app.passport.createJWT({}, options)
       .then(token => {
         expiredToken = token;
-        return app.passport.createJWT({ id: 0 }, app.get('auth'));
+        return app.passport.createJWT({ userId: 0 }, app.get('auth'));
       })
       .then(token => {
         accessToken = token;
@@ -40,6 +41,7 @@ describe('Primus authentication', function() {
           server = app.listen(port);
           server.once('listening', () => {
             Socket = app.primus.Socket;
+            app.primus.on('connection', s => serverSocket = s);
             done();
           });
         });
@@ -78,11 +80,28 @@ describe('Primus authentication', function() {
             app.passport.verifyJWT(response.accessToken, app.get('auth')).then(payload => {
               expect(payload).to.exist;
               expect(payload.iss).to.equal('feathers');
-              expect(payload.id).to.equal(0);
+              expect(payload.userId).to.equal(0);
               done();
             });
           });
         });
+
+        it('sets the user on the socket', done => {
+          socket.send('authenticate', data, (error, response) => {
+            expect(response.accessToken).to.exist;
+            expect(serverSocket.request.feathers.user).to.not.equal(undefined);
+            done();
+          });
+        });
+
+        it('sets entity specified in strategy', done => {
+          data.strategy = 'org-local';
+          socket.send('authenticate', data, (error, response) => {
+            expect(response.accessToken).to.exist;
+            expect(serverSocket.request.feathers.org).to.not.equal(undefined);
+            done();
+          });
+        });
       });
 
       describe('when using invalid credentials', () => {
@@ -132,7 +151,7 @@ describe('Primus authentication', function() {
             app.passport.verifyJWT(response.accessToken, app.get('auth')).then(payload => {
               expect(payload).to.exist;
               expect(payload.iss).to.equal('feathers');
-              expect(payload.id).to.equal(0);
+              expect(payload.userId).to.equal(0);
               done();
             });
           });
@@ -148,7 +167,7 @@ describe('Primus authentication', function() {
             app.passport.verifyJWT(response.accessToken, app.get('auth')).then(payload => {
               expect(payload).to.exist;
               expect(payload.iss).to.equal('feathers');
-              expect(payload.id).to.equal(0);
+              expect(payload.userId).to.equal(0);
               done();
             });
           });
diff --git a/test/integration/rest.test.js b/test/integration/rest.test.js
index d1c92a0c..04a84e63 100644
--- a/test/integration/rest.test.js
+++ b/test/integration/rest.test.js
@@ -20,7 +20,7 @@ describe('REST authentication', function() {
     app.passport.createJWT({}, options)
       .then(token => {
         expiredToken = token;
-        return app.passport.createJWT({ id: 0 }, app.get('auth'));
+        return app.passport.createJWT({ userId: 0 }, app.get('auth'));
       })
       .then(token => {
         accessToken = token;
@@ -37,6 +37,7 @@ describe('REST authentication', function() {
 
       beforeEach(() => {
         data = {
+          strategy: 'local',
           email: 'admin@feathersjs.com',
           password: 'admin'
         };
@@ -53,7 +54,7 @@ describe('REST authentication', function() {
             }).then(payload => {
               expect(payload).to.exist;
               expect(payload.iss).to.equal('feathers');
-              expect(payload.id).to.equal(0);
+              expect(payload.userId).to.equal(0);
             });
         });
       });
@@ -108,7 +109,7 @@ describe('REST authentication', function() {
             }).then(payload => {
               expect(payload).to.exist;
               expect(payload.iss).to.equal('feathers');
-              expect(payload.id).to.equal(0);
+              expect(payload.userId).to.equal(0);
             });
         });
       });
@@ -124,7 +125,7 @@ describe('REST authentication', function() {
             }).then(payload => {
               expect(payload).to.exist;
               expect(payload.iss).to.equal('feathers');
-              expect(payload.id).to.equal(0);
+              expect(payload.userId).to.equal(0);
             });
         });
       });
diff --git a/test/integration/socketio.test.js b/test/integration/socketio.test.js
index bb3ca089..feed6663 100644
--- a/test/integration/socketio.test.js
+++ b/test/integration/socketio.test.js
@@ -18,6 +18,7 @@ describe('Socket.io authentication', function() {
 
   let server;
   let socket;
+  let serverSocket;
   let expiringServer;
   let expiringSocket;
   let expiredToken;
@@ -28,14 +29,17 @@ describe('Socket.io authentication', function() {
     app.passport.createJWT({}, options)
       .then(token => {
         expiredToken = token;
-        return app.passport.createJWT({ id: 0 }, app.get('auth'));
+        return app.passport.createJWT({ userId: 0 }, app.get('auth'));
       })
       .then(token => {
         accessToken = token;
         expiringServer = expiringApp.listen(1336);
         expiringServer.once('listening', () => {
           server = app.listen(port);
-          server.once('listening', () => done());
+          server.once('listening', () => {
+            app.io.on('connect', s => serverSocket = s);
+            done();
+          });
         });
       });
   });
@@ -69,11 +73,28 @@ describe('Socket.io authentication', function() {
             app.passport.verifyJWT(response.accessToken, app.get('auth')).then(payload => {
               expect(payload).to.exist;
               expect(payload.iss).to.equal('feathers');
-              expect(payload.id).to.equal(0);
+              expect(payload.userId).to.equal(0);
               done();
             });
           });
         });
+
+        it('sets the user on the socket', done => {
+          socket.emit('authenticate', data, (error, response) => {
+            expect(response.accessToken).to.exist;
+            expect(serverSocket.feathers.user).to.not.equal(undefined);
+            done();
+          });
+        });
+
+        it('sets entity specified in strategy', done => {
+          data.strategy = 'org-local';
+          socket.emit('authenticate', data, (error, response) => {
+            expect(response.accessToken).to.exist;
+            expect(serverSocket.feathers.org).to.not.equal(undefined);
+            done();
+          });
+        });
       });
 
       describe('when using invalid credentials', () => {
@@ -123,7 +144,7 @@ describe('Socket.io authentication', function() {
             app.passport.verifyJWT(response.accessToken, app.get('auth')).then(payload => {
               expect(payload).to.exist;
               expect(payload.iss).to.equal('feathers');
-              expect(payload.id).to.equal(0);
+              expect(payload.userId).to.equal(0);
               done();
             });
           });
@@ -139,7 +160,7 @@ describe('Socket.io authentication', function() {
             app.passport.verifyJWT(response.accessToken, app.get('auth')).then(payload => {
               expect(payload).to.exist;
               expect(payload.iss).to.equal('feathers');
-              expect(payload.id).to.equal(0);
+              expect(payload.userId).to.equal(0);
               done();
             });
           });
diff --git a/test/passport/authenticate.test.js b/test/passport/authenticate.test.js
index b66eb081..39bcd115 100644
--- a/test/passport/authenticate.test.js
+++ b/test/passport/authenticate.test.js
@@ -141,15 +141,15 @@ describe('passport:authenticate', () => {
       });
 
       describe('success', () => {
-        let info;
+        let payload;
         let user;
         let organization;
 
         beforeEach(() => {
-          info = { platform: 'feathers' };
+          payload = { platform: 'feathers' };
           user = { name: 'Bob' };
           organization = { name: 'Apple' };
-          verifier = (cb) => cb(null, user, info);
+          verifier = (cb) => cb(null, user, payload);
           passport.use(new MockStrategy({}, verifier));
           authenticator = authenticate({ entity: 'user' })(passport, 'mock');
         });
@@ -166,9 +166,9 @@ describe('passport:authenticate', () => {
           });
         });
 
-        it('returns the passport info', () => {
+        it('returns the passport payload', () => {
           return authenticator(req).then(result => {
-            expect(result.data.info).to.deep.equal(info);
+            expect(result.data.payload).to.deep.equal(payload);
           });
         });