Merge pull request #67 from feathersjs/issue-64

Removing assigning token to params.query for sockets. Closes #65.

src/client/hooks.js

@@ -16,25 +16,12 @@ export let populateHeader = function(options = {}) {
 
   return function(hook) {
     if (hook.params.token) {
-      hook.params.headers = Object.assign({}, hook.params.headers, {
-        [options.header]: hook.params.token
-      });
-    }
-  };
-};
-
-export let populateSocketParams = function() {
-  return function(hook) {
-    if (hook.params.token) {
-      hook.params.query = Object.assign({}, hook.params.query, {
-        token: hook.params.token
-      });
+      hook.params.headers = Object.assign({}, { [options.header]: hook.params.token }, hook.params.headers);
     }
   };
 };
 
 export default {
   populateParams,
-  populateHeader,
-  populateSocketParams
+  populateHeader
 };

src/client/index.js

@@ -104,17 +104,17 @@ export default function(options = {}) {
       utils.clearUser();
     };
 
-    // Set up hook that adds adds token to data sent to server over sockets
+    // Set up hook that adds adds token and user to params so that
+    // it they can be accessed by client side hooks and services
     app.mixins.push(function(service) {
       service.before(hooks.populateParams());
     });
-
-    // Set up hook that adds authorization header
+    
+    // Set up hook that adds authorization header for REST provider
     if (app.rest) {
       app.mixins.push(function(service) {
         service.before(hooks.populateHeader());
       });
     }
-
   };
 }

src/hooks/index.js

@@ -7,7 +7,6 @@ import setUserId from './set-user-id';
 import toLowerCase from './to-lower-case';
 import verifyToken from './verify-token';
 import populateUser from './populate-user';
-import normalizeAuthToken from './normalize-auth-token';
 
 let hooks = {
   hashPassword,
@@ -18,8 +17,7 @@ let hooks = {
   setUserId,
   toLowerCase,
   verifyToken,
-  populateUser,
-  normalizeAuthToken
+  populateUser
 };
 
 export default hooks;

src/hooks/verify-token.js

@@ -10,6 +10,11 @@ import errors from 'feathers-errors';
 export default function(options = {}){
   const secret = options.secret;
 
+  if (!secret) {
+    console.log('no secret', options);
+    throw new Error('You need to pass `options.secret` to the verifyToken() hook.');
+  }
+
   return function(hook) {
     const token = hook.params.token;
 

src/index.js

@@ -19,11 +19,6 @@ export default function(providers) {
     const app = this;
     let _super = app.setup;
 
-    // Add mixin to normalize the auth token on the params
-    app.mixins.push(function(service){
-      service.before(hooks.normalizeAuthToken());
-    });
-
     // REST middleware
     if (app.rest) {
       debug('registering REST authentication middleware');
@@ -96,7 +91,10 @@ export default function(providers) {
 
       app.configure( provider(options) );
     });
+
+    // TODO (EK): We might want to also support a failRedirect for HTML
 
+    // TODO (EK): Don't register this route handler if a custom success redirect is passed in
     app.get(authOptions.successRedirect, function(req, res){
       res.sendFile(path.resolve(__dirname, 'public', 'auth-success.html'));
     });

src/middleware/index.js

@@ -19,15 +19,18 @@ export let exposeConnectMiddleware = function(req, res, next) {
   next();
 };
 
-// Make the authenticated passport user also available for REST services
+// Make the authenticated passport user also available for REST services.
+// We might need this for when we have session supported auth.
 // export let exposeAuthenticatedUser = function(options = {}) {
 //   return function(req, res, next) {
 //     req.feathers.user = req.user;
 //     next();
 //   };
 // };
 
-// Make the authenticated passport user also available for REST services
+// Make sure than an auth token passed in is available for hooks
+// and services. This gracefully falls back from
+// header -> cookie -> body -> query string
 export let normalizeAuthToken = function(options = {}) {
   const defaults = {
     header: 'authorization',
@@ -125,6 +128,7 @@ export let setupSocketIOAuthentication = function(app, options = {}) {
         // The token gets normalized in hook.params for REST so we'll stay with
         // convention and pass it as params using sockets.
         app.service(options.tokenEndpoint).create({}, data).then(response => {
+          socket.feathers.token = response.token;
           socket.feathers.user = response.data;
           socket.emit('authenticated', response);
         }).catch(errorHandler);
@@ -141,6 +145,7 @@ export let setupSocketIOAuthentication = function(app, options = {}) {
         params.req.body = data;
 
         app.service(options.localEndpoint).create(data, params).then(response => {
+          socket.feathers.token = response.token;
           socket.feathers.user = response.data;
           socket.emit('authenticated', response);
         }).catch(errorHandler);
@@ -177,6 +182,7 @@ export let setupPrimusAuthentication = function(app, options = {}) {
         // The token gets normalized in hook.params for REST so we'll stay with
         // convention and pass it as params using sockets.
         app.service(options.tokenEndpoint).create({}, data).then(response => {
+          socket.request.feathers.token = response.token;
           socket.request.feathers.user = response.data;
           socket.send('authenticated', response);
         }).catch(errorHandler);
@@ -193,6 +199,7 @@ export let setupPrimusAuthentication = function(app, options = {}) {
         params.req.body = data;
 
         app.service(options.localEndpoint).create(data, params).then(response => {
+          socket.request.feathers.token = response.token;
           socket.request.feathers.user = response.data;
           socket.send('authenticated', response);
         }).catch(errorHandler);

test/integration/primus.test.js

@@ -201,4 +201,44 @@ describe('Primus authentication', function() {
     // TODO (EK): This isn't really possible with primus unless
     // you are sending auth_tokens from your OAuth2 provider
   });
+
+  describe('Authorization', () => {
+    describe('when authenticated', () => {
+      it('returns data from protected route', (done) => {
+        const data = { token: validToken };
+
+        primus.on('authenticated', function() {
+          primus.send('messages::get', 1, {}, (error, data) => {
+            assert.equal(data.id, 1);
+            done();
+          });
+        });
+
+        primus.send('authenticate', data);
+      });
+    });
+
+    describe('when not authenticated', () => {
+      it('returns 401 from protected route', (done) => {
+        primus.send('messages::get', 1, {}, (error) => {
+          assert.equal(error.code, 401);
+          done();
+        });
+      });
+
+      it('does not return data from protected route', (done) => {
+        primus.send('messages::get', 1, {}, (error, data) => {
+          assert.equal(data, undefined);
+          done();
+        });
+      });
+
+      it('returns data from unprotected route', (done) => {
+        primus.send('users::find', {}, (error, data) => {
+          assert.notEqual(data, undefined);
+          done();
+        });
+      });
+    });
+  });
 });