From 274c28a9235b010b453964964710143030a91756 Mon Sep 17 00:00:00 2001 From: SleepWalker Date: Wed, 4 Jan 2017 07:52:46 +0200 Subject: [PATCH] #246: remove logout from user/actions. Use logoutAll from account/actions everywhere --- src/components/accounts/actions.js | 16 ++- src/components/auth/actions.js | 17 +-- src/components/user/actions.js | 39 ++---- .../middlewares/refreshTokenMiddleware.js | 15 +- tests/components/accounts/actions.test.js | 23 ++- tests/components/user/actions.test.js | 132 ------------------ 6 files changed, 67 insertions(+), 175 deletions(-) diff --git a/src/components/accounts/actions.js b/src/components/accounts/actions.js index 8d955a3..a3bb5ab 100644 --- a/src/components/accounts/actions.js +++ b/src/components/accounts/actions.js @@ -1,6 +1,8 @@ +import { routeActions } from 'react-router-redux'; + import authentication from 'services/api/authentication'; import accounts from 'services/api/accounts'; -import { updateUser, logout } from 'components/user/actions'; +import { updateUser, setGuest } from 'components/user/actions'; import { setLocale } from 'components/i18n/actions'; import logger from 'services/logger'; @@ -25,7 +27,7 @@ export function authenticate({token, refreshToken}) { authentication.validateToken({token, refreshToken}) .catch(() => { // TODO: log this case - dispatch(logout()); + dispatch(logoutAll()); return Promise.reject(); }) @@ -80,17 +82,23 @@ export function revoke(account) { }); } - return dispatch(logout()); + return dispatch(logoutAll()); }; } export function logoutAll() { return (dispatch, getState) => { + dispatch(setGuest()); + const {accounts: {available}} = getState(); available.forEach((account) => authentication.logout(account)); dispatch(reset()); + + dispatch(routeActions.push('/login')); + + return Promise.resolve(); }; } @@ -120,7 +128,7 @@ export function logoutStrangers() { return dispatch(authenticate(accountToReplace)); } - dispatch(logout()); + dispatch(logoutAll()); return Promise.resolve(); }; diff --git a/src/components/auth/actions.js b/src/components/auth/actions.js index fb91af0..e5a47af 100644 --- a/src/components/auth/actions.js +++ b/src/components/auth/actions.js @@ -1,12 +1,16 @@ import { routeActions } from 'react-router-redux'; -import { updateUser, logout, acceptRules as userAcceptRules } from 'components/user/actions'; -import { authenticate } from 'components/accounts/actions'; +import logger from 'services/logger'; +import { updateUser, acceptRules as userAcceptRules } from 'components/user/actions'; +import { authenticate, logoutAll } from 'components/accounts/actions'; import authentication from 'services/api/authentication'; import oauth from 'services/api/oauth'; import signup from 'services/api/signup'; import dispatchBsod from 'components/ui/bsod/dispatchBsod'; +export { updateUser } from 'components/user/actions'; +export { authenticate, logoutAll as logout } from 'components/accounts/actions'; + export function login({login = '', password = '', rememberMe = false}) { const PASSWORD_REQUIRED = 'error.password_required'; const LOGIN_REQUIRED = 'error.login_required'; @@ -24,9 +28,9 @@ export function login({login = '', password = '', rememberMe = false}) { } else if (resp.errors.login === ACTIVATION_REQUIRED) { return dispatch(needActivation()); } else if (resp.errors.login === LOGIN_REQUIRED && password) { - // TODO: log this case to backend - // return to the first step - return dispatch(logout()); + logger.warn('No login on password panel'); + + return dispatch(logoutAll()); } } @@ -144,9 +148,6 @@ export function clearErrors() { return setErrors(null); } -export { logout, updateUser } from 'components/user/actions'; -export { authenticate } from 'components/accounts/actions'; - /** * @param {object} oauthData * @param {string} oauthData.clientId diff --git a/src/components/user/actions.js b/src/components/user/actions.js index 0bdcd36..2342dd5 100644 --- a/src/components/user/actions.js +++ b/src/components/user/actions.js @@ -1,7 +1,4 @@ -import { routeActions } from 'react-router-redux'; - import accounts from 'services/api/accounts'; -import { logoutAll } from 'components/accounts/actions'; import { setLocale } from 'components/i18n/actions'; export const UPDATE = 'USER_UPDATE'; @@ -18,6 +15,20 @@ export function updateUser(payload) { }; } +export const SET = 'USER_SET'; +/** + * Replace current user's state with a new one + * + * @param {User} payload + * @return {object} - action definition + */ +export function setUser(payload) { + return { + type: SET, + payload + }; +} + export const CHANGE_LANG = 'USER_CHANGE_LANG'; export function changeLang(lang) { return (dispatch, getState) => dispatch(setLocale(lang)) @@ -37,32 +48,12 @@ export function changeLang(lang) { }); } -export const SET = 'USER_SET'; -/** - * Replace current user's state with a new one - * - * @param {User} payload - * @return {object} - action definition - */ -export function setUser(payload) { - return { - type: SET, - payload - }; -} - -export function logout() { +export function setGuest() { return (dispatch, getState) => { dispatch(setUser({ lang: getState().user.lang, isGuest: true })); - - dispatch(logoutAll()); - - dispatch(routeActions.push('/login')); - - return Promise.resolve(); }; } diff --git a/src/components/user/middlewares/refreshTokenMiddleware.js b/src/components/user/middlewares/refreshTokenMiddleware.js index 491f88a..b48cb04 100644 --- a/src/components/user/middlewares/refreshTokenMiddleware.js +++ b/src/components/user/middlewares/refreshTokenMiddleware.js @@ -1,6 +1,6 @@ import authentication from 'services/api/authentication'; -import { updateToken } from 'components/accounts/actions'; -import { logout } from '../actions'; +import logger from 'services/logger'; +import { updateToken, logoutAll } from 'components/accounts/actions'; /** * Ensures, that all user's requests have fresh access token @@ -41,8 +41,11 @@ export default function refreshTokenMiddleware({dispatch, getState}) { return requestAccessToken(refreshToken, dispatch).then(() => req); } } catch (err) { - // console.error('Bad token', err); // TODO: it would be cool to log such things to backend - return dispatch(logout()).then(() => req); + logger.warn('Refresh token error: bad token', { + token + }); + + return dispatch(logoutAll()).then(() => req); } return Promise.resolve(req); @@ -58,7 +61,7 @@ export default function refreshTokenMiddleware({dispatch, getState}) { return requestAccessToken(refreshToken, dispatch).then(restart); } - return dispatch(logout()).then(() => Promise.reject(resp)); + return dispatch(logoutAll()).then(() => Promise.reject(resp)); } return Promise.reject(resp); @@ -76,7 +79,7 @@ function requestAccessToken(refreshToken, dispatch) { return promise .then(({token}) => dispatch(updateToken(token))) - .catch(() => dispatch(logout())); + .catch(() => dispatch(logoutAll())); } diff --git a/tests/components/accounts/actions.test.js b/tests/components/accounts/actions.test.js index 4544a32..2413ab8 100644 --- a/tests/components/accounts/actions.test.js +++ b/tests/components/accounts/actions.test.js @@ -1,6 +1,8 @@ import expect from 'unexpected'; import sinon from 'sinon'; +import { routeActions } from 'react-router-redux'; + import accounts from 'services/api/accounts'; import authentication from 'services/api/authentication'; import { @@ -15,7 +17,7 @@ import { } from 'components/accounts/actions'; import { SET_LOCALE } from 'components/i18n/actions'; -import { updateUser } from 'components/user/actions'; +import { updateUser, setUser } from 'components/user/actions'; const account = { id: 1, @@ -257,6 +259,25 @@ describe('components/accounts/actions', () => { reset() ]); }); + + it('should redirect to /login', () => + logoutAll()(dispatch, getState).then(() => { + expect(dispatch, 'to have a call satisfying', [ + routeActions.push('/login') + ]); + }) + ); + + it('should change user to guest', () => + logoutAll()(dispatch, getState).then(() => { + expect(dispatch, 'to have a call satisfying', [ + setUser({ + lang: user.lang, + isGuest: true + }) + ]); + }) + ); }); describe('#logoutStrangers', () => { diff --git a/tests/components/user/actions.test.js b/tests/components/user/actions.test.js index 479acdd..94c5805 100644 --- a/tests/components/user/actions.test.js +++ b/tests/components/user/actions.test.js @@ -1,134 +1,2 @@ -import expect from 'unexpected'; -import sinon from 'sinon'; - -import { routeActions } from 'react-router-redux'; - -import request from 'services/request'; -import { reset, RESET } from 'components/accounts/actions'; - -import { - logout, - setUser -} from 'components/user/actions'; - - describe('components/user/actions', () => { - const getState = sinon.stub().named('store.getState'); - const dispatch = sinon.spy((arg) => - typeof arg === 'function' ? arg(dispatch, getState) : arg - ).named('store.dispatch'); - - const callThunk = function(fn, ...args) { - const thunk = fn(...args); - - return thunk(dispatch, getState); - }; - - beforeEach(() => { - dispatch.reset(); - getState.reset(); - getState.returns({}); - sinon.stub(request, 'get').named('request.get'); - sinon.stub(request, 'post').named('request.post'); - }); - - afterEach(() => { - request.get.restore(); - request.post.restore(); - }); - - describe('#logout()', () => { - beforeEach(() => { - request.post.returns(Promise.resolve()); - }); - - describe('user with jwt', () => { - const token = 'iLoveRockNRoll'; - - beforeEach(() => { - getState.returns({ - user: { - lang: 'foo' - }, - accounts: { - active: {token}, - available: [{token}] - } - }); - }); - - it('should post to /api/authentication/logout with user jwt', () => - callThunk(logout).then(() => { - expect(request.post, 'to have a call satisfying', [ - '/api/authentication/logout', {}, { - token: expect.it('not to be empty') - } - ]); - }) - ); - - testChangedToGuest(); - testAccountsReset(); - testRedirectedToLogin(); - }); - - describe('user without jwt', () => { - // (a guest with partially filled user's state) - // DEPRECATED - beforeEach(() => { - getState.returns({ - user: { - lang: 'foo' - }, - accounts: { - active: null, - available: [] - } - }); - }); - - it('should not post to /api/authentication/logout', () => - callThunk(logout).then(() => { - expect(request.post, 'was not called'); - }) - ); - - testChangedToGuest(); - testAccountsReset(); - testRedirectedToLogin(); - }); - - function testChangedToGuest() { - it('should change user to guest', () => - callThunk(logout).then(() => { - expect(dispatch, 'to have a call satisfying', [ - setUser({ - lang: 'foo', - isGuest: true - }) - ]); - }) - ); - } - - function testRedirectedToLogin() { - it('should redirect to /login', () => - callThunk(logout).then(() => { - expect(dispatch, 'to have a call satisfying', [ - routeActions.push('/login') - ]); - }) - ); - } - - function testAccountsReset() { - it(`should dispatch ${RESET}`, () => - callThunk(logout).then(() => { - expect(dispatch, 'to have a call satisfying', [ - reset() - ]); - }) - ); - } - }); });