diff --git a/packages/authentication-client/lib/core.js b/packages/authentication-client/lib/core.js index 6e367efb25..335edcb6bc 100644 --- a/packages/authentication-client/lib/core.js +++ b/packages/authentication-client/lib/core.js @@ -68,14 +68,10 @@ exports.AuthenticationClient = class AuthenticationClient { } reset () { - // Reset the internal authentication state - // but not the accessToken from storage - const authResult = this.app.get('authentication'); - this.app.set('authentication', null); this.authenticated = false; - return authResult; + return Promise.resolve(null); } reauthenticate (force = false) { @@ -88,12 +84,12 @@ exports.AuthenticationClient = class AuthenticationClient { if (!accessToken) { throw new NotAuthenticated('No accessToken found in storage'); } - + return this.authenticate({ strategy: this.options.jwtStrategy, accessToken }); - }); + }).catch(error => this.removeJwt().then(() => Promise.reject(error))); } return authPromise; @@ -109,9 +105,11 @@ exports.AuthenticationClient = class AuthenticationClient { const { accessToken } = authResult; this.authenticated = true; + this.app.emit('login', authResult); + this.app.emit('authenticated', authResult); return this.setJwt(accessToken).then(() => authResult); - }); + }).catch(error => this.reset().then(() => Promise.reject(error))); this.app.set('authentication', promise); @@ -121,7 +119,13 @@ exports.AuthenticationClient = class AuthenticationClient { logout () { return this.app.get('authentication') .then(() => this.service.remove(null)) - .then(() => this.removeJwt()) - .then(() => this.reset()); + .then(authResult => this.removeJwt() + .then(this.reset()) + .then(() => { + this.app.emit('logout', authResult); + + return authResult; + }) + ); } }; diff --git a/packages/authentication-client/package.json b/packages/authentication-client/package.json index a4bf5e570e..5debcb1862 100644 --- a/packages/authentication-client/package.json +++ b/packages/authentication-client/package.json @@ -26,7 +26,7 @@ "node": ">= 6" }, "scripts": { - "test": "../../node_modules/.bin/nyc mocha --opts ../../mocha.opts" + "test": "mocha --opts ../../mocha.opts" }, "directories": { "lib": "lib" diff --git a/packages/authentication-client/test/index.test.js b/packages/authentication-client/test/index.test.js index 0f12eb28c5..93a277b80c 100644 --- a/packages/authentication-client/test/index.test.js +++ b/packages/authentication-client/test/index.test.js @@ -53,12 +53,18 @@ describe('@feathersjs/authentication-client', () => { .then(res => assert.strictEqual(res, undefined)); }); - it('authenticate and authentication hook', () => { + it('authenticate, authentication hook, login event', () => { const data = { strategy: 'testing' }; - return app.authenticate(data).then(result => { + const promise = new Promise(resolve => { + app.once('login', resolve); + }); + + app.authenticate(data); + + return promise.then(result => { assert.deepStrictEqual(result, { accessToken, data, @@ -80,6 +86,18 @@ describe('@feathersjs/authentication-client', () => { }); }); + it('logout event', () => { + const promise = new Promise(resolve => app.once('logout', resolve)); + + app.authenticate({ + strategy: 'testing' + }).then(() => app.logout()); + + return promise.then(result => { + assert.deepStrictEqual(result, { id: null }); + }); + }); + describe('reauthenticate', () => { it('fails when no token in storage', () => { return app.authentication.reauthenticate().then(() => { diff --git a/packages/authentication-client/test/integration/commons.js b/packages/authentication-client/test/integration/commons.js index 1f7aae1da0..271db77cf0 100644 --- a/packages/authentication-client/test/integration/commons.js +++ b/packages/authentication-client/test/integration/commons.js @@ -28,7 +28,7 @@ module.exports = (getApp, getClient, { }); }); - it('authentication with wrong credentials fails', () => { + it('authentication with wrong credentials fails, does not maintain state', () => { return client.authenticate({ strategy: 'local', email, @@ -38,6 +38,7 @@ module.exports = (getApp, getClient, { .catch(error => { assert.strictEqual(error.name, 'NotAuthenticated'); assert.strictEqual(error.message, 'Invalid login'); + assert.ok(!client.get('authentication'), 'Reset client state'); }); }); @@ -91,7 +92,12 @@ module.exports = (getApp, getClient, { email, password }).then(() => client.logout()) - .then(() => client.service('dummy').find()) + .then(result => { + assert.ok(result.accessToken); + assert.ok(result.user); + + return client.service('dummy').find(); + }) .then(() => assert.fail('Should never get here')) .catch(error => { assert.strictEqual(error.name, 'NotAuthenticated'); diff --git a/packages/authentication-local/lib/index.js b/packages/authentication-local/lib/index.js index 2ab9462a0d..448965674e 100644 --- a/packages/authentication-local/lib/index.js +++ b/packages/authentication-local/lib/index.js @@ -3,5 +3,4 @@ const hashPassword = require('./hooks/hash-password'); const protect = require('./hooks/protect'); exports.LocalStrategy = LocalStrategy; -exports.protect = protect; -exports.hashPassword = hashPassword; +exports.hooks = { protect, hashPassword }; diff --git a/packages/authentication-local/test/fixture.js b/packages/authentication-local/test/fixture.js index 8c8964e44e..e5e7bb596b 100644 --- a/packages/authentication-local/test/fixture.js +++ b/packages/authentication-local/test/fixture.js @@ -2,7 +2,8 @@ const feathers = require('@feathersjs/feathers'); const memory = require('feathers-memory'); const { AuthenticationService, JWTStrategy } = require('@feathersjs/authentication'); -const { LocalStrategy, hashPassword, protect } = require('../lib'); +const { LocalStrategy, hooks } = require('../lib'); +const { hashPassword, protect } = hooks; module.exports = (app = feathers()) => { const authentication = new AuthenticationService(app); diff --git a/packages/authentication-local/test/hooks/hash-password.test.js b/packages/authentication-local/test/hooks/hash-password.test.js index db57af297f..3abed9ba5f 100644 --- a/packages/authentication-local/test/hooks/hash-password.test.js +++ b/packages/authentication-local/test/hooks/hash-password.test.js @@ -1,7 +1,7 @@ const assert = require('assert'); const getApp = require('../fixture'); -const { hashPassword } = require('../../lib'); +const { hooks: { hashPassword } } = require('../../lib'); describe('@feathersjs/authentication-local/hooks/hash-password', () => { let app; diff --git a/packages/authentication-local/test/hooks/protect.test.js b/packages/authentication-local/test/hooks/protect.test.js index 8defea60df..4007e3d439 100644 --- a/packages/authentication-local/test/hooks/protect.test.js +++ b/packages/authentication-local/test/hooks/protect.test.js @@ -1,5 +1,5 @@ const assert = require('assert'); -const { protect } = require('../../lib'); +const { hooks: { protect } } = require('../../lib'); function testOmit (title, property) { describe(title, () => { diff --git a/packages/authentication/lib/core.js b/packages/authentication/lib/core.js index 26e39a1324..cbee0d178f 100644 --- a/packages/authentication/lib/core.js +++ b/packages/authentication/lib/core.js @@ -104,6 +104,7 @@ module.exports = class AuthenticationBase { } const { strategy } = authentication; + const authParams = Object.assign(params, { authentication: true }); // Throw an error is a `strategy` is indicated but not in the allowed strategies if (strategy && !allowed.includes(strategy)) { @@ -116,7 +117,7 @@ module.exports = class AuthenticationBase { const promise = strategies.reduce((acc, authStrategy) => { return acc.then(({ result, error }) => { if (!result) { - return authStrategy.authenticate(authentication, params) + return authStrategy.authenticate(authentication, authParams) // Set result .then(newResult => ({ result: newResult })) // Use caught error or previous error if it already exists diff --git a/packages/authentication/lib/hooks/authenticate.js b/packages/authentication/lib/hooks/authenticate.js index 7de452de14..ba949b9092 100644 --- a/packages/authentication/lib/hooks/authenticate.js +++ b/packages/authentication/lib/hooks/authenticate.js @@ -40,8 +40,8 @@ module.exports = (_settings, ..._strategies) => { ); } - if (authentication) { - const authParams = omit(params, 'provider'); + if (authentication && authentication !== true) { + const authParams = omit(params, 'provider', 'authentication'); debug('Authenticating with', authentication, strategies); diff --git a/packages/authentication/lib/hooks/connection.js b/packages/authentication/lib/hooks/connection.js index 1deab628be..9376dbdf46 100644 --- a/packages/authentication/lib/hooks/connection.js +++ b/packages/authentication/lib/hooks/connection.js @@ -2,11 +2,8 @@ const debug = require('debug')('@feathersjs/authentication/hooks/connection'); module.exports = (strategy = 'jwt') => { return context => { - const { - method, - result: { accessToken }, - params: { connection } - } = context; + const { method, result, params: { connection } } = context; + const { accessToken, ...rest } = result; if (!connection) { return context; @@ -19,7 +16,9 @@ module.exports = (strategy = 'jwt') => { delete connection.authentication; } else if (method === 'create' && accessToken) { debug('Adding authentication information to real-time connection'); - connection.authentication = { strategy, accessToken }; + Object.assign(connection, rest, { + authentication: { strategy, accessToken } + }); } return context; diff --git a/packages/authentication/test/core.test.js b/packages/authentication/test/core.test.js index 8e836396e2..2aacf53d4f 100644 --- a/packages/authentication/test/core.test.js +++ b/packages/authentication/test/core.test.js @@ -99,12 +99,17 @@ describe('authentication/core', () => { }); it('returns second success', () => { - return auth.authenticate({ + const authentication = { strategy: 'second', v2: true, password: 'supersecret' - }, {}, 'first', 'second').then(result => { - assert.deepStrictEqual(result, Strategy2.result); + }; + + return auth.authenticate(authentication, {}, 'first', 'second').then(result => { + assert.deepStrictEqual(result, Object.assign({}, Strategy2.result, { + authentication, + params: { authentication: true } + })); }); }); @@ -112,13 +117,19 @@ describe('authentication/core', () => { const params = { some: 'thing' }; - - return auth.authenticate({ + const authentication = { strategy: 'second', v2: true, password: 'supersecret' - }, params, 'first', 'second').then(result => { - assert.deepStrictEqual(result, Object.assign({}, Strategy2.result, params)); + }; + + return auth.authenticate(authentication, params, 'first', 'second').then(result => { + assert.deepStrictEqual(result, Object.assign({ + params: Object.assign(params, { + authentication: true + }), + authentication + }, Strategy2.result)); }); }); @@ -145,11 +156,16 @@ describe('authentication/core', () => { describe('with a list of strategies and strategy not set in params', () => { it('returns first success in chain', () => { - return auth.authenticate({ + const authentication = { v2: true, password: 'supersecret' - }, {}, 'first', 'second').then(result => { - assert.deepStrictEqual(result, Strategy2.result); + }; + + return auth.authenticate(authentication, {}, 'first', 'second').then(result => { + assert.deepStrictEqual(result, Object.assign({ + authentication, + params: { authentication: true } + }, Strategy2.result)); }); }); diff --git a/packages/authentication/test/fixtures.js b/packages/authentication/test/fixtures.js index d60709d8c9..f7be188b7f 100644 --- a/packages/authentication/test/fixtures.js +++ b/packages/authentication/test/fixtures.js @@ -42,7 +42,7 @@ exports.Strategy2 = class Strategy2 { const isV2 = authentication.v2 === true && authentication.password === 'supersecret'; if (isV2 || authentication.both) { - return Promise.resolve(Object.assign({}, Strategy2.result, params)); + return Promise.resolve(Object.assign({ params, authentication }, Strategy2.result)); } return Promise.reject(new NotAuthenticated('Invalid v2 user')); diff --git a/packages/authentication/test/hooks/authenticate.test.js b/packages/authentication/test/hooks/authenticate.test.js index 9bffd7a924..a01bcfbe68 100644 --- a/packages/authentication/test/hooks/authenticate.test.js +++ b/packages/authentication/test/hooks/authenticate.test.js @@ -126,7 +126,10 @@ describe('authentication/hooks/authenticate', () => { }; return app.service('users').get(1, params).then(result => { - assert.deepStrictEqual(result, Object.assign({}, params, Strategy2.result)); + assert.deepStrictEqual(result, Object.assign({ + authentication: params.authentication, + params: { authentication: true } + }, Strategy2.result)); }); }); @@ -160,6 +163,18 @@ describe('authentication/hooks/authenticate', () => { }); }); + it('passes with authenticaiton true but external call', () => { + return app.service('users').get(1, { + provider: 'rest', + authentication: true + }).then(result => { + assert.deepStrictEqual(result, { + provider: 'rest', + authentication: true + }); + }); + }); + it('errors when used on the authentication service', () => { const auth = app.service('authentication'); diff --git a/packages/authentication/test/hooks/connection.test.js b/packages/authentication/test/hooks/connection.test.js index 2f545362b6..c7a6c0ed37 100644 --- a/packages/authentication/test/hooks/connection.test.js +++ b/packages/authentication/test/hooks/connection.test.js @@ -14,7 +14,9 @@ describe('authentication/hooks/connection', () => { } return Promise.resolve({ - accessToken: '1234' + accessToken: '1234', + authentication: { strategy: 'test' }, + additionalParams: true }); }, @@ -32,17 +34,23 @@ describe('authentication/hooks/connection', () => { it('create does nothing when there is no connection', () => { return service.create({}, {}).then(result => { assert.deepStrictEqual(result, { - accessToken: '1234' + accessToken: '1234', + authentication: { strategy: 'test' }, + additionalParams: true }); }); }); - it('create (login) updates `params.connection.authentication`', () => { + it('create (login) updates `params.connection.authentication` with all params', () => { const connection = {}; return service.create({}, { connection }).then(() => { assert.deepStrictEqual(connection, { - authentication: { strategy: 'jwt', accessToken: '1234' } + authentication: { + strategy: 'jwt', + accessToken: '1234' + }, + additionalParams: true }); }); });