Repository: feathersjs/feathers-authentication
Pull request #524 (2017-06-08 21:06:34)

Test and fix for authenticate event with invalid data
Closes https://github.com/feathersjs/feathers-security/issues/1

ekryski: Looks good! :shipit: 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c29ea69..1a68749 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,8 +1,7 @@
 # Change Log
 
-## [Unreleased](https://github.com/feathersjs/feathers-authentication/tree/HEAD)
-
-[Full Changelog](https://github.com/feathersjs/feathers-authentication/compare/v1.2.2...HEAD)
+## [v1.2.3](https://github.com/feathersjs/feathers-authentication/tree/v1.2.3) (2017-05-10)
+[Full Changelog](https://github.com/feathersjs/feathers-authentication/compare/v1.2.2...v1.2.3)
 
 **Closed issues:**
 
diff --git a/README.md b/README.md
index 55f2d7a..603b4bf 100644
--- a/README.md
+++ b/README.md
@@ -77,7 +77,7 @@ These other middleware are included and exposed but typically you don't need to
 
 ### Default Options
 
-The following default options will be mixed in with your global `auth` object from your config file. It will set the mixed options back on to the app so that they are available at any time by calling `app.get('auth')`. They can all be overridden and are depended upon by some of the authentication plugins.
+The following default options will be mixed in with your global `auth` object from your config file. It will set the mixed options back on to the app so that they are available at any time by calling `app.get('authentication')`. They can all be overridden and are depended upon by some of the authentication plugins.
 
 ```js
 {
diff --git a/docs/migrating.md b/docs/migrating.md
index 62b3d46..a87e2c6 100644
--- a/docs/migrating.md
+++ b/docs/migrating.md
@@ -50,7 +50,7 @@ For most of you, migrating your app should be fairly straight forward as there a
 const authentication = require('feathers-authentication');
 const FacebookStrategy = require('passport-facebook').Strategy;
 
-let config = app.get('auth');
+let config = app.get('authentication');
 config.facebook.strategy = FacebookStrategy;
 app.configure(authentication(config))
     .use('/users', memory()) // this use to be okay to be anywhere
@@ -83,7 +83,7 @@ const FacebookStrategy = require('passport-facebook').Strategy;
 
 // The services you are setting the `entity` param for need to be registered before authentication
 app.use('/users', memory())
-    .configure(auth(app.get('auth')))
+    .configure(auth(app.get('authentication')))
     .configure(jwt())
     .configure(local())
     .configure(oauth1())
diff --git a/src/index.js b/src/index.js
index e756757..b1bc8dc 100644
--- a/src/index.js
+++ b/src/index.js
@@ -30,6 +30,7 @@ export default function init (config = {}) {
       options.cookie.secure = false;
     }
 
+    app.set('authentication', options);
     app.set('auth', options);
 
     debug('Setting up Passport');
diff --git a/src/service.js b/src/service.js
index 1d68592..9e69fa8 100644
--- a/src/service.js
+++ b/src/service.js
@@ -11,7 +11,7 @@ class Service {
   }
 
   create (data = {}, params = {}) {
-    const defaults = this.app.get('auth');
+    const defaults = this.app.get('authentication') || this.app.get('auth');
     const payload = params.payload;
 
     // create accessToken
@@ -26,7 +26,7 @@ class Service {
   }
 
   remove (id, params) {
-    const defaults = this.app.get('auth');
+    const defaults = this.app.get('authentication') || this.app.get('auth');
     const authHeader = params.headers && params.headers[defaults.header.toLowerCase()];
     const authParams = authHeader && authHeader.match(/(\S+)\s+(\S+)/);
     const accessToken = id !== null ? id : (authParams && authParams[2]) || authHeader;
diff --git a/src/socket/handler.js b/src/socket/handler.js
index 1811fbc..f555714 100644
--- a/src/socket/handler.js
+++ b/src/socket/handler.js
@@ -19,7 +19,7 @@ function handleSocketCallback (promise, callback) {
 }
 
 export default function setupSocketHandler (app, options, { feathersParams, provider, emit, disconnect }) {
-  const authSettings = app.get('auth');
+  const authSettings = app.get('authentication') || app.get('auth');
   const service = app.service(authSettings.path);
   const entityService = app.service(authSettings.service);
   let isUpdateEntitySetup = false;
@@ -57,7 +57,15 @@ export default function setupSocketHandler (app, options, { feathersParams, prov
       }
     };
 
-    const authenticate = function (data, callback = () => {}) {
+    const authenticate = function (data = {}, callback = () => {}) {
+      if (typeof data === 'function') {
+        callback = data;
+      }
+
+      if (typeof data === 'function' || typeof data !== 'object' || data === null) {
+        data = {};
+      }
+
       const { strategy } = data;
       socket._feathers = Object.assign({
         query: {},


Test patch:
diff --git a/test/index.test.js b/test/index.test.js
index 932b16e..8e79463 100644
--- a/test/index.test.js
+++ b/test/index.test.js
@@ -62,7 +62,7 @@ describe('Feathers Authentication', () => {
     it('sets cookie to be insecure', () => {
       app.set('env', 'development');
       app.configure(authentication(config));
-      expect(app.get('auth').cookie.secure).to.equal(false);
+      expect(app.get('authentication').cookie.secure).to.equal(false);
     });
   });
 
@@ -70,7 +70,7 @@ describe('Feathers Authentication', () => {
     it('sets cookie to be insecure', () => {
       app.set('env', 'test');
       app.configure(authentication(config));
-      expect(app.get('auth').cookie.secure).to.equal(false);
+      expect(app.get('authentication').cookie.secure).to.equal(false);
     });
   });
 
@@ -78,14 +78,14 @@ describe('Feathers Authentication', () => {
     it('sets cookie to be secure', () => {
       app.set('env', 'production');
       app.configure(authentication(config));
-      expect(app.get('auth').cookie.secure).to.equal(true);
+      expect(app.get('authentication').cookie.secure).to.equal(true);
     });
   });
 
   it('sets custom config options', () => {
     config.custom = 'custom';
     app.configure(authentication(config));
-    expect(app.get('auth').custom).to.equal('custom');
+    expect(app.get('authentication').custom).to.equal('custom');
   });
 
   it('sets up feathers passport adapter', () => {
diff --git a/test/integration/primus.test.js b/test/integration/primus.test.js
index 080f9a3..79343ec 100644
--- a/test/integration/primus.test.js
+++ b/test/integration/primus.test.js
@@ -35,11 +35,11 @@ describe('Primus authentication', function () {
   let accessToken;
 
   before(done => {
-    const options = merge({}, app.get('auth'), { jwt: { expiresIn: '1ms' } });
+    const options = merge({}, app.get('authentication'), { jwt: { expiresIn: '1ms' } });
     app.passport.createJWT({}, options)
       .then(token => {
         expiredToken = token;
-        return app.passport.createJWT({ userId: 0 }, app.get('auth'));
+        return app.passport.createJWT({ userId: 0 }, app.get('authentication'));
       })
       .then(token => {
         accessToken = token;
@@ -94,7 +94,7 @@ describe('Primus authentication', function () {
           socket.send('authenticate', data, (error, response) => {
             expect(error).to.not.be.ok;
             expect(response.accessToken).to.exist;
-            app.passport.verifyJWT(response.accessToken, app.get('auth')).then(payload => {
+            app.passport.verifyJWT(response.accessToken, app.get('authentication')).then(payload => {
               expect(payload).to.exist;
               expect(payload.iss).to.equal('feathers');
               expect(payload.userId).to.equal(0);
@@ -192,7 +192,7 @@ describe('Primus authentication', function () {
           socket.send('authenticate', data, (error, response) => {
             expect(error).to.not.be.ok;
             expect(response.accessToken).to.exist;
-            app.passport.verifyJWT(response.accessToken, app.get('auth')).then(payload => {
+            app.passport.verifyJWT(response.accessToken, app.get('authentication')).then(payload => {
               expect(payload).to.exist;
               expect(payload.iss).to.equal('feathers');
               expect(payload.userId).to.equal(0);
@@ -209,7 +209,7 @@ describe('Primus authentication', function () {
           socket.send('authenticate', data, (error, response) => {
             expect(error).to.not.be.ok;
             expect(response.accessToken).to.exist;
-            app.passport.verifyJWT(response.accessToken, app.get('auth')).then(payload => {
+            app.passport.verifyJWT(response.accessToken, app.get('authentication')).then(payload => {
               expect(payload).to.exist;
               expect(payload.iss).to.equal('feathers');
               expect(payload.userId).to.equal(0);
diff --git a/test/integration/rest.test.js b/test/integration/rest.test.js
index 4cd34f8..1ae6366 100644
--- a/test/integration/rest.test.js
+++ b/test/integration/rest.test.js
@@ -18,11 +18,11 @@ describe('REST authentication', function () {
   let accessToken;
 
   before(done => {
-    const options = merge({}, app.get('auth'), { jwt: { expiresIn: '1ms' } });
+    const options = merge({}, app.get('authentication'), { jwt: { expiresIn: '1ms' } });
     app.passport.createJWT({}, options)
       .then(token => {
         expiredToken = token;
-        return app.passport.createJWT({ userId: 0 }, app.get('auth'));
+        return app.passport.createJWT({ userId: 0 }, app.get('authentication'));
       })
       .then(token => {
         accessToken = token;
@@ -52,7 +52,7 @@ describe('REST authentication', function () {
             .send(data)
             .then(response => {
               expect(response.body.accessToken).to.exist;
-              return app.passport.verifyJWT(response.body.accessToken, app.get('auth'));
+              return app.passport.verifyJWT(response.body.accessToken, app.get('authentication'));
             }).then(payload => {
               expect(payload).to.exist;
               expect(payload.iss).to.equal('feathers');
@@ -107,7 +107,7 @@ describe('REST authentication', function () {
             .send(data)
             .then(response => {
               expect(response.body.accessToken).to.exist;
-              return app.passport.verifyJWT(response.body.accessToken, app.get('auth'));
+              return app.passport.verifyJWT(response.body.accessToken, app.get('authentication'));
             }).then(payload => {
               expect(payload).to.exist;
               expect(payload.iss).to.equal('feathers');
@@ -123,7 +123,7 @@ describe('REST authentication', function () {
             .send(data)
             .then(response => {
               expect(response.body.accessToken).to.exist;
-              return app.passport.verifyJWT(response.body.accessToken, app.get('auth'));
+              return app.passport.verifyJWT(response.body.accessToken, app.get('authentication'));
             }).then(payload => {
               expect(payload).to.exist;
               expect(payload.iss).to.equal('feathers');
diff --git a/test/integration/socketio.test.js b/test/integration/socketio.test.js
index ca79f7c..fbc5583 100644
--- a/test/integration/socketio.test.js
+++ b/test/integration/socketio.test.js
@@ -34,11 +34,11 @@ describe('Socket.io authentication', function () {
   let accessToken;
 
   before(done => {
-    const options = merge({}, app.get('auth'), { jwt: { expiresIn: '1ms' } });
+    const options = merge({}, app.get('authentication'), { jwt: { expiresIn: '1ms' } });
     app.passport.createJWT({}, options)
       .then(token => {
         expiredToken = token;
-        return app.passport.createJWT({ userId: 0 }, app.get('auth'));
+        return app.passport.createJWT({ userId: 0 }, app.get('authentication'));
       })
       .then(token => {
         accessToken = token;
@@ -88,7 +88,7 @@ describe('Socket.io authentication', function () {
           socket.emit('authenticate', data, (error, response) => {
             expect(error).to.not.equal(undefined);
             expect(response.accessToken).to.exist;
-            app.passport.verifyJWT(response.accessToken, app.get('auth')).then(payload => {
+            app.passport.verifyJWT(response.accessToken, app.get('authentication')).then(payload => {
               expect(payload).to.exist;
               expect(payload.iss).to.equal('feathers');
               expect(payload.userId).to.equal(0);
@@ -149,6 +149,29 @@ describe('Socket.io authentication', function () {
             done();
           });
         });
+
+        it('returns NotAuthenticated error when strategy is invalid', done => {
+          delete data.strategy;
+
+          socket.emit('authenticate', data, error => {
+            expect(error.code).to.equal(401);
+            done();
+          });
+        });
+
+        it('returns NotAuthenticated error when data is not an object', done => {
+          socket.emit('authenticate', undefined, error => {
+            expect(error.code).to.equal(401);
+            done();
+          });
+        });
+
+        it('returns NotAuthenticated error when data is not passed', done => {
+          socket.emit('authenticate', error => {
+            expect(error.code).to.equal(401);
+            done();
+          });
+        });
       });
 
       describe('when missing credentials', () => {
@@ -186,7 +209,7 @@ describe('Socket.io authentication', function () {
           socket.emit('authenticate', data, (error, response) => {
             expect(error).to.not.be.ok;
             expect(response.accessToken).to.exist;
-            app.passport.verifyJWT(response.accessToken, app.get('auth')).then(payload => {
+            app.passport.verifyJWT(response.accessToken, app.get('authentication')).then(payload => {
               expect(payload).to.exist;
               expect(payload.iss).to.equal('feathers');
               expect(payload.userId).to.equal(0);
@@ -203,7 +226,7 @@ describe('Socket.io authentication', function () {
           socket.emit('authenticate', data, (error, response) => {
             expect(error).to.not.be.ok;
             expect(response.accessToken).to.exist;
-            app.passport.verifyJWT(response.accessToken, app.get('auth')).then(payload => {
+            app.passport.verifyJWT(response.accessToken, app.get('authentication')).then(payload => {
               expect(payload).to.exist;
               expect(payload.iss).to.equal('feathers');
               expect(payload.userId).to.equal(0);
diff --git a/test/service.test.js b/test/service.test.js
index c987514..20e4b9b 100644
--- a/test/service.test.js
+++ b/test/service.test.js
@@ -102,7 +102,7 @@ describe('/authentication service', () => {
       return app.service('authentication').create(data, params).then(result => {
         return app.service('authentication').create(data).then(result => {
           return app.passport
-            .verifyJWT(result.accessToken, app.get('auth'))
+            .verifyJWT(result.accessToken, app.get('authentication'))
             .then(payload => {
               const delta = (payload.exp - payload.iat);
               expect(delta).to.equal(24 * 60 * 60);
@@ -117,7 +117,7 @@ describe('/authentication service', () => {
 
     beforeEach(() => {
       return app.passport
-        .createJWT({ id: 1 }, app.get('auth'))
+        .createJWT({ id: 1 }, app.get('authentication'))
         .then(token => { accessToken = token; });
     });