From 5142d65b39fbd8713fa4b20aa13f1a4ce4496564 Mon Sep 17 00:00:00 2001 From: SleepWalker Date: Tue, 15 Nov 2016 07:55:15 +0200 Subject: [PATCH] #48: call authentication.logout for each revoked account --- src/components/accounts/actions.js | 16 +- src/components/user/actions.js | 24 +-- src/services/api/authentication.js | 13 +- tests/components/accounts/actions.test.js | 154 +++++++++++++----- tests/components/user/actions.test.js | 17 +- .../refreshTokenMiddleware.test.js | 28 ++-- tests/services/api/authentication.test.js | 35 ++++ 7 files changed, 214 insertions(+), 73 deletions(-) diff --git a/src/components/accounts/actions.js b/src/components/accounts/actions.js index 59f7fa9..37fe539 100644 --- a/src/components/accounts/actions.js +++ b/src/components/accounts/actions.js @@ -59,7 +59,10 @@ export function revoke(account) { if (accountToReplace) { return dispatch(authenticate(accountToReplace)) - .then(() => dispatch(remove(account))); + .then(() => { + authentication.logout(account); + dispatch(remove(account)); + }); } return dispatch(logout()); @@ -111,8 +114,19 @@ export function activate(account) { }; } +export function logoutAll() { + return (dispatch, getState) => { + const {accounts: {available}} = getState(); + + available.forEach((account) => authentication.logout(account)); + + dispatch(reset()); + }; +} export const RESET = 'accounts:reset'; /** + * @api private + * * @return {object} - action definition */ export function reset() { diff --git a/src/components/user/actions.js b/src/components/user/actions.js index a19f6b3..4079147 100644 --- a/src/components/user/actions.js +++ b/src/components/user/actions.js @@ -1,7 +1,7 @@ import { routeActions } from 'react-router-redux'; import accounts from 'services/api/accounts'; -import { reset as resetAccounts } from 'components/accounts/actions'; +import { logoutAll } from 'components/accounts/actions'; import authentication from 'services/api/authentication'; import { setLocale } from 'components/i18n/actions'; @@ -54,24 +54,16 @@ export function setUser(payload) { export function logout() { return (dispatch, getState) => { - if (getState().user.token) { - authentication.logout(); - } + dispatch(setUser({ + lang: getState().user.lang, + isGuest: true + })); - return new Promise((resolve) => { - setTimeout(() => { // a tiny timeout to allow logout before user's token will be removed - dispatch(setUser({ - lang: getState().user.lang, - isGuest: true - })); + dispatch(logoutAll()); - dispatch(resetAccounts()); + dispatch(routeActions.push('/login')); - dispatch(routeActions.push('/login')); - - resolve(); - }, 0); - }); + return Promise.resolve(); }; } diff --git a/src/services/api/authentication.js b/src/services/api/authentication.js index 5a3e935..b495f5b 100644 --- a/src/services/api/authentication.js +++ b/src/services/api/authentication.js @@ -13,8 +13,17 @@ const authentication = { ); }, - logout() { - return request.post('/api/authentication/logout'); + /** + * @param {object} options + * @param {object} [options.token] - an optional token to overwrite headers + * in middleware and disable token auto-refresh + * + * @return {Promise} + */ + logout(options = {}) { + return request.post('/api/authentication/logout', {}, { + token: options.token + }); }, forgotPassword({ diff --git a/tests/components/accounts/actions.test.js b/tests/components/accounts/actions.test.js index 3c48e56..297dbdd 100644 --- a/tests/components/accounts/actions.test.js +++ b/tests/components/accounts/actions.test.js @@ -8,7 +8,8 @@ import { add, ADD, activate, ACTIVATE, remove, - reset + reset, + logoutAll } from 'components/accounts/actions'; import { SET_LOCALE } from 'components/i18n/actions'; @@ -116,58 +117,129 @@ describe('components/accounts/actions', () => { }); describe('#revoke()', () => { - it('should switch next account if available', () => { + beforeEach(() => { + sinon.stub(authentication, 'logout').named('authentication.logout'); + }); + + afterEach(() => { + authentication.logout.restore(); + }); + + describe('when one account available', () => { + beforeEach(() => { + getState.returns({ + accounts: { + active: account, + available: [account] + }, + user + }); + }); + + it('should dispatch reset action', () => + revoke(account)(dispatch, getState).then(() => + expect(dispatch, 'to have a call satisfying', [ + reset() + ]) + ) + ); + + it('should call logout api method in background', () => + revoke(account)(dispatch, getState).then(() => + expect(authentication.logout, 'to have a call satisfying', [ + account + ]) + ) + ); + + it('should update user state', () => + revoke(account)(dispatch, getState).then(() => + expect(dispatch, 'to have a call satisfying', [ + {payload: {isGuest: true}} + // updateUser({isGuest: true}) + ]) + // expect(dispatch, 'to have calls satisfying', [ + // [remove(account)], + // [expect.it('to be a function')] + // // [logout()] // TODO: this is not a plain action. How should we simplify its testing? + // ]) + ) + ); + }); + + describe('when multiple accounts available', () => { const account2 = {...account, id: 2}; + beforeEach(() => { + getState.returns({ + accounts: { + active: account2, + available: [account, account2] + }, + user + }); + }); + + it('should switch to the next account', () => + revoke(account2)(dispatch, getState).then(() => + expect(dispatch, 'to have a call satisfying', [ + activate(account) + ]) + ) + ); + + it('should remove current account', () => + revoke(account2)(dispatch, getState).then(() => + expect(dispatch, 'to have a call satisfying', [ + remove(account2) + ]) + ) + ); + + it('should call logout api method in background', () => + revoke(account2)(dispatch, getState).then(() => + expect(authentication.logout, 'to have a call satisfying', [ + account2 + ]) + ) + ); + }); + }); + + describe('#logoutAll()', () => { + const account2 = {...account, id: 2}; + + beforeEach(() => { getState.returns({ accounts: { active: account2, - available: [account] + available: [account, account2] }, user }); - return revoke(account2)(dispatch, getState).then(() => { - expect(dispatch, 'to have a call satisfying', [ - remove(account2) - ]); - expect(dispatch, 'to have a call satisfying', [ - activate(account) - ]); - expect(dispatch, 'to have a call satisfying', [ - updateUser({...user, isGuest: false}) - ]); - // expect(dispatch, 'to have calls satisfying', [ - // [remove(account2)], - // [expect.it('to be a function')] - // // [authenticate(account2)] // TODO: this is not a plain action. How should we simplify its testing? - // ]) - }); + sinon.stub(authentication, 'logout').named('authentication.logout'); }); - it('should logout if no other accounts available', () => { - getState.returns({ - accounts: { - active: account, - available: [] - }, - user - }); + afterEach(() => { + authentication.logout.restore(); + }); - revoke(account)(dispatch, getState).then(() => { - expect(dispatch, 'to have a call satisfying', [ - {payload: {isGuest: true}} - // updateUser({isGuest: true}) - ]); - expect(dispatch, 'to have a call satisfying', [ - reset() - ]); - // expect(dispatch, 'to have calls satisfying', [ - // [remove(account)], - // [expect.it('to be a function')] - // // [logout()] // TODO: this is not a plain action. How should we simplify its testing? - // ]) - }); + it('should call logout api method for each account', () => { + logoutAll()(dispatch, getState); + + expect(authentication.logout, 'to have calls satisfying', [ + [account], + [account2] + ]); + }); + + it('should dispatch reset', () => { + logoutAll()(dispatch, getState); + + expect(dispatch, 'to have a call satisfying', [ + reset() + ]); }); }); }); diff --git a/tests/components/user/actions.test.js b/tests/components/user/actions.test.js index 450cefa..693dce4 100644 --- a/tests/components/user/actions.test.js +++ b/tests/components/user/actions.test.js @@ -42,11 +42,16 @@ describe('components/user/actions', () => { }); describe('user with jwt', () => { + const token = 'iLoveRockNRoll'; + beforeEach(() => { getState.returns({ user: { - token: 'iLoveRockNRoll', lang: 'foo' + }, + accounts: { + active: {token}, + available: [{token}] } }); }); @@ -65,7 +70,7 @@ describe('components/user/actions', () => { return callThunk(logout).then(() => { expect(request.post, 'to have a call satisfying', [ - '/api/authentication/logout' + '/api/authentication/logout', {}, {} ]); }); }); @@ -75,11 +80,17 @@ describe('components/user/actions', () => { testRedirectedToLogin(); }); - describe('user without jwt', () => { // (a guest with partially filled user's state) + describe('user without jwt', () => { + // (a guest with partially filled user's state) + // DEPRECATED beforeEach(() => { getState.returns({ user: { lang: 'foo' + }, + accounts: { + active: null, + available: [] } }); }); diff --git a/tests/components/user/middlewares/refreshTokenMiddleware.test.js b/tests/components/user/middlewares/refreshTokenMiddleware.test.js index 6068872..9b9aa3d 100644 --- a/tests/components/user/middlewares/refreshTokenMiddleware.test.js +++ b/tests/components/user/middlewares/refreshTokenMiddleware.test.js @@ -17,6 +17,7 @@ describe('refreshTokenMiddleware', () => { beforeEach(() => { sinon.stub(authentication, 'requestToken').named('authentication.requestToken'); + sinon.stub(authentication, 'logout').named('authentication.logout'); getState = sinon.stub().named('store.getState'); dispatch = sinon.spy((arg) => @@ -28,6 +29,7 @@ describe('refreshTokenMiddleware', () => { afterEach(() => { authentication.requestToken.restore(); + authentication.logout.restore(); }); it('must be till 2100 to test with validToken', () => @@ -37,12 +39,14 @@ describe('refreshTokenMiddleware', () => { describe('#before', () => { describe('when token expired', () => { beforeEach(() => { + const account = { + token: expiredToken, + refreshToken + }; getState.returns({ accounts: { - active: { - token: expiredToken, - refreshToken - } + active: account, + available: [account] }, user: {} }); @@ -104,12 +108,14 @@ describe('refreshTokenMiddleware', () => { }); it('should if token can not be parsed', () => { + const account = { + token: 'realy bad token', + refreshToken + }; getState.returns({ accounts: { - active: { - token: 'realy bad token', - refreshToken - } + active: account, + available: [account] }, user: {} }); @@ -140,7 +146,8 @@ describe('refreshTokenMiddleware', () => { beforeEach(() => { getState.returns({ accounts: { - active: null + active: null, + available: [] }, user: { token: expiredToken, @@ -216,7 +223,8 @@ describe('refreshTokenMiddleware', () => { beforeEach(() => { getState.returns({ accounts: { - active: {refreshToken} + active: {refreshToken}, + available: [{refreshToken}] }, user: {} }); diff --git a/tests/services/api/authentication.test.js b/tests/services/api/authentication.test.js index 67025a8..6f054f9 100644 --- a/tests/services/api/authentication.test.js +++ b/tests/services/api/authentication.test.js @@ -1,5 +1,6 @@ import expect from 'unexpected'; +import request from 'services/request'; import authentication from 'services/api/authentication'; import accounts from 'services/api/accounts'; @@ -88,4 +89,38 @@ describe('authentication api', () => { }); }); }); + + describe('#logout', () => { + beforeEach(() => { + sinon.stub(request, 'post').named('request.post'); + }); + + afterEach(() => { + request.post.restore(); + }); + + it('should request logout api', () => { + authentication.logout(); + + expect(request.post, 'to have a call satisfying', [ + '/api/authentication/logout', {}, {} + ]); + }); + + it('returns a promise', () => { + request.post.returns(Promise.resolve()); + + return expect(authentication.logout(), 'to be fulfilled'); + }); + + it('overrides token if provided', () => { + const token = 'foo'; + + authentication.logout({token}); + + expect(request.post, 'to have a call satisfying', [ + '/api/authentication/logout', {}, {token} + ]); + }); + }); });