From a2afac867a3213028c7096d78b7f376216550f62 Mon Sep 17 00:00:00 2001 From: SleepWalker Date: Tue, 8 Nov 2016 08:30:53 +0200 Subject: [PATCH] #48: fix the bug, whent account.current was requested without disabling refreshToken middleware --- src/components/accounts/actions.js | 21 ++++++++++++++----- .../middlewares/refreshTokenMiddleware.js | 4 ++-- src/services/api/accounts.js | 7 +++---- src/services/api/authentication.js | 2 +- tests/components/accounts/actions.test.js | 8 +++++++ .../refreshTokenMiddleware.test.js | 8 +++---- tests/services/api/authentication.test.js | 2 +- 7 files changed, 35 insertions(+), 17 deletions(-) diff --git a/src/components/accounts/actions.js b/src/components/accounts/actions.js index db8a22c..00f2b94 100644 --- a/src/components/accounts/actions.js +++ b/src/components/accounts/actions.js @@ -16,10 +16,12 @@ import { setLocale } from 'components/i18n/actions'; * @param {Account|object} account * @param {string} account.token * @param {string} account.refreshToken + * + * @return {function} */ export function authenticate({token, refreshToken}) { - return (dispatch) => { - return authentication.validateToken({token, refreshToken}) + return (dispatch) => + authentication.validateToken({token, refreshToken}) .then(({token, refreshToken}) => accounts.current({token}) .then((user) => ({ @@ -44,11 +46,12 @@ export function authenticate({token, refreshToken}) { return dispatch(setLocale(user.lang)) .then(() => account); }); - }; } /** * @param {Account} account + * + * @return {function} */ export function revoke(account) { return (dispatch, getState) => { @@ -56,9 +59,9 @@ export function revoke(account) { if (getState().accounts.length) { return dispatch(authenticate(getState().accounts[0])); - } else { - return dispatch(logout()); } + + return dispatch(logout()); }; } @@ -67,6 +70,8 @@ export const ADD = 'accounts:add'; * @api private * * @param {Account} account + * + * @return {object} - action definition */ export function add(account) { return { @@ -80,6 +85,8 @@ export const REMOVE = 'accounts:remove'; * @api private * * @param {Account} account + * + * @return {object} - action definition */ export function remove(account) { return { @@ -93,6 +100,8 @@ export const ACTIVATE = 'accounts:activate'; * @api private * * @param {Account} account + * + * @return {object} - action definition */ export function activate(account) { return { @@ -104,6 +113,8 @@ export function activate(account) { export const UPDATE_TOKEN = 'accounts:updateToken'; /** * @param {string} token + * + * @return {object} - action definition */ export function updateToken(token) { return { diff --git a/src/components/user/middlewares/refreshTokenMiddleware.js b/src/components/user/middlewares/refreshTokenMiddleware.js index 5bbcd93..a0eb3c8 100644 --- a/src/components/user/middlewares/refreshTokenMiddleware.js +++ b/src/components/user/middlewares/refreshTokenMiddleware.js @@ -29,7 +29,7 @@ export default function refreshTokenMiddleware({dispatch, getState}) { refreshToken = user.refreshToken; } - if (!token || isRefreshTokenRequest || req.options.autoRefreshToken === false) { + if (!token || req.options.token || isRefreshTokenRequest) { return req; } @@ -49,7 +49,7 @@ export default function refreshTokenMiddleware({dispatch, getState}) { }, catch(resp, req, restart) { - if (resp && resp.status === 401 && req.options.autoRefreshToken !== false) { + if (resp && resp.status === 401 && !req.options.token) { const {user, accounts} = getState(); const {refreshToken} = accounts.active ? accounts.active : user; diff --git a/src/services/api/accounts.js b/src/services/api/accounts.js index 014ec76..05ce95c 100644 --- a/src/services/api/accounts.js +++ b/src/services/api/accounts.js @@ -3,15 +3,14 @@ import request from 'services/request'; export default { /** * @param {object} options - * @param {object} [options.token] - an optional token to overwrite headers in middleware - * @param {bool} [options.autoRefreshToken=true] - disable token auto refresh during request + * @param {object} [options.token] - an optional token to overwrite headers + * in middleware and disable token auto-refresh * * @return {Promise} */ current(options = {}) { return request.get('/api/accounts/current', {}, { - token: options.token, - autoRefreshToken: options.autoRefreshToken + token: options.token }); }, diff --git a/src/services/api/authentication.js b/src/services/api/authentication.js index 0d2c2cb..5a3e935 100644 --- a/src/services/api/authentication.js +++ b/src/services/api/authentication.js @@ -59,7 +59,7 @@ const authentication = { resolve(); }) - .then(() => accounts.current({token, autoRefreshToken: false})) + .then(() => accounts.current({token})) .then(() => ({token, refreshToken})) .catch((resp) => { if (resp.message === 'Token expired') { diff --git a/tests/components/accounts/actions.test.js b/tests/components/accounts/actions.test.js index 9c26c5f..94d363f 100644 --- a/tests/components/accounts/actions.test.js +++ b/tests/components/accounts/actions.test.js @@ -1,6 +1,7 @@ import expect from 'unexpected'; import accounts from 'services/api/accounts'; +import authentication from 'services/api/authentication'; import { authenticate, revoke, add, activate, remove, ADD, REMOVE, ACTIVATE } from 'components/accounts/actions'; import { SET_LOCALE } from 'components/i18n/actions'; @@ -36,11 +37,18 @@ describe('Accounts actions', () => { user: {} }); + sinon.stub(authentication, 'validateToken').named('authentication.validateToken'); + authentication.validateToken.returns(Promise.resolve({ + token: account.token, + refreshToken: account.refreshToken + })); + sinon.stub(accounts, 'current').named('accounts.current'); accounts.current.returns(Promise.resolve(user)); }); afterEach(() => { + authentication.validateToken.restore(); accounts.current.restore(); }); diff --git a/tests/components/user/middlewares/refreshTokenMiddleware.test.js b/tests/components/user/middlewares/refreshTokenMiddleware.test.js index 7421738..82b1114 100644 --- a/tests/components/user/middlewares/refreshTokenMiddleware.test.js +++ b/tests/components/user/middlewares/refreshTokenMiddleware.test.js @@ -76,10 +76,10 @@ describe('refreshTokenMiddleware', () => { expect(authentication.requestToken, 'was not called'); }); - it('should not apply if options.autoRefreshToken === false', () => { + it('should not auto refresh token if options.token specified', () => { const data = { url: 'foo', - options: {autoRefreshToken: false} + options: {token: 'foo'} }; middleware.before(data); @@ -257,10 +257,10 @@ describe('refreshTokenMiddleware', () => { ) ); - it('should pass the request through if options.autoRefreshToken === false', () => { + it('should pass the request through if options.token specified', () => { const promise = middleware.catch(expiredResponse, { options: { - autoRefreshToken: false + token: 'foo' } }, restart); diff --git a/tests/services/api/authentication.test.js b/tests/services/api/authentication.test.js index ef21005..67025a8 100644 --- a/tests/services/api/authentication.test.js +++ b/tests/services/api/authentication.test.js @@ -21,7 +21,7 @@ describe('authentication api', () => { expect(authentication.validateToken(validTokens), 'to be fulfilled') .then(() => { expect(accounts.current, 'to have a call satisfying', [ - {token: 'foo', autoRefreshToken: false} + {token: 'foo'} ]); }) );