From bf2976c0091a1e3c0f71a714c7743fc2661d0016 Mon Sep 17 00:00:00 2001 From: SleepWalker Date: Sat, 17 Feb 2018 21:59:35 +0200 Subject: [PATCH] #389: allow switch accounts, when refreshToken is invalid. Fix oauth in case, when refreshToken is invalid --- src/components/accounts/actions.js | 6 +- src/components/auth/actions.js | 38 ++++++-- .../auth/chooseAccount/ChooseAccountBody.js | 12 +-- src/components/auth/reducer.js | 23 +++-- src/pages/auth/AuthPage.js | 1 + src/services/authFlow/AuthFlow.js | 5 ++ src/services/authFlow/AuthFlow.test.js | 2 + src/services/authFlow/ChooseAccountState.js | 9 +- .../authFlow/ChooseAccountState.test.js | 19 ++++ src/services/authFlow/PasswordState.js | 20 ++++- src/services/authFlow/PasswordState.test.js | 86 +++++++++++++++++++ 11 files changed, 187 insertions(+), 34 deletions(-) diff --git a/src/components/accounts/actions.js b/src/components/accounts/actions.js index 7d58d78..f646c62 100644 --- a/src/components/accounts/actions.js +++ b/src/components/accounts/actions.js @@ -1,9 +1,8 @@ // @flow import { getJwtPayload } from 'functions'; -import { browserHistory } from 'services/history'; import { sessionStorage } from 'services/localStorage'; import authentication from 'services/api/authentication'; -import { setLogin } from 'components/auth/actions'; +import { relogin as navigateToLogin } from 'components/auth/actions'; import { updateUser, setGuest } from 'components/user/actions'; import { setLocale } from 'components/i18n/actions'; import { setAccountSwitcher } from 'components/auth/actions'; @@ -238,8 +237,7 @@ export function relogin(email?: string) { email = activeAccount.email; } - email && dispatch(setLogin(email)); - browserHistory.push('/login'); + dispatch(navigateToLogin(email || null)); }; } diff --git a/src/components/auth/actions.js b/src/components/auth/actions.js index 51b22c8..34a1a7d 100644 --- a/src/components/auth/actions.js +++ b/src/components/auth/actions.js @@ -16,6 +16,7 @@ import ContactForm from 'components/contact/ContactForm'; export { updateUser } from 'components/user/actions'; export { authenticate, logoutAll as logout } from 'components/accounts/actions'; +import { getCredentials } from './reducer'; /** * Reoutes user to the previous page if it is possible @@ -224,19 +225,40 @@ export function setLogin(login: ?string) { }; } +export function relogin(login: ?string) { + return (dispatch: (Function | Object) => void) => { + dispatch({ + type: SET_CREDENTIALS, + payload: { + login, + returnUrl: location.pathname + location.search, + isRelogin: true, + }, + }); + + browserHistory.push('/login'); + }; +} + function requestTotp({login, password, rememberMe}: { login: string, password: string, rememberMe: bool }) { - return { - type: SET_CREDENTIALS, - payload: { - login, - password, - rememberMe, - isTotpRequired: true - } + return (dispatch: (Function | Object) => void, getState: () => Object) => { + // merging with current credentials to propogate returnUrl + const credentials = getCredentials(getState()); + + dispatch({ + type: SET_CREDENTIALS, + payload: { + ...credentials, + login, + password, + rememberMe, + isTotpRequired: true + } + }); }; } diff --git a/src/components/auth/chooseAccount/ChooseAccountBody.js b/src/components/auth/chooseAccount/ChooseAccountBody.js index b80ba1b..0317a25 100644 --- a/src/components/auth/chooseAccount/ChooseAccountBody.js +++ b/src/components/auth/chooseAccount/ChooseAccountBody.js @@ -19,11 +19,13 @@ export default class ChooseAccountBody extends BaseAuthBody {
{this.renderErrors()} -
- {client.name} - }} /> -
+ {client && ( +
+ {client.name} + }} /> +
+ )}
+ diff --git a/src/services/authFlow/AuthFlow.js b/src/services/authFlow/AuthFlow.js index 5dba009..0d52d4e 100644 --- a/src/services/authFlow/AuthFlow.js +++ b/src/services/authFlow/AuthFlow.js @@ -11,6 +11,7 @@ import ForgotPasswordState from './ForgotPasswordState'; import RecoverPasswordState from './RecoverPasswordState'; import ActivationState from './ActivationState'; import CompleteState from './CompleteState'; +import ChooseAccountState from './ChooseAccountState'; import ResendActivationState from './ResendActivationState'; import type AbstractState from './AbstractState'; @@ -219,6 +220,10 @@ export default class AuthFlow implements AuthContext { this.setState(new ResendActivationState()); break; + case '/choose-account': + this.setState(new ChooseAccountState()); + break; + case '/': case '/login': case '/password': diff --git a/src/services/authFlow/AuthFlow.test.js b/src/services/authFlow/AuthFlow.test.js index 3927717..748f32a 100644 --- a/src/services/authFlow/AuthFlow.test.js +++ b/src/services/authFlow/AuthFlow.test.js @@ -13,6 +13,7 @@ import ActivationState from 'services/authFlow/ActivationState'; import ResendActivationState from 'services/authFlow/ResendActivationState'; import LoginState from 'services/authFlow/LoginState'; import CompleteState from 'services/authFlow/CompleteState'; +import ChooseAccountState from 'services/authFlow/ChooseAccountState'; describe('AuthFlow', () => { let flow; @@ -275,6 +276,7 @@ describe('AuthFlow', () => { '/oauth2/v1': OAuthState, '/oauth2': OAuthState, '/register': RegisterState, + '/choose-account': ChooseAccountState, '/recover-password': RecoverPasswordState, '/recover-password/key123': RecoverPasswordState, '/forgot-password': ForgotPasswordState, diff --git a/src/services/authFlow/ChooseAccountState.js b/src/services/authFlow/ChooseAccountState.js index b85f3f5..3fd0f46 100644 --- a/src/services/authFlow/ChooseAccountState.js +++ b/src/services/authFlow/ChooseAccountState.js @@ -4,7 +4,13 @@ import CompleteState from './CompleteState'; export default class ChooseAccountState extends AbstractState { enter(context) { - context.navigate('/oauth/choose-account'); + const { auth } = context.getState(); + + if (auth.oauth) { + context.navigate('/oauth/choose-account'); + } else { + context.navigate('/choose-account'); + } } resolve(context, payload) { @@ -12,6 +18,7 @@ export default class ChooseAccountState extends AbstractState { context.setState(new CompleteState()); } else { context.navigate('/login'); + context.run('setLogin', null); context.setState(new LoginState()); } } diff --git a/src/services/authFlow/ChooseAccountState.test.js b/src/services/authFlow/ChooseAccountState.test.js index 12ffae4..726cffb 100644 --- a/src/services/authFlow/ChooseAccountState.test.js +++ b/src/services/authFlow/ChooseAccountState.test.js @@ -23,10 +23,28 @@ describe('ChooseAccountState', () => { describe('#enter', () => { it('should navigate to /oauth/choose-account', () => { + context.getState.returns({ + auth: { + oauth: {}, + }, + }); + expectNavigate(mock, '/oauth/choose-account'); state.enter(context); }); + + it('should navigate to /choose-account if not oauth', () => { + context.getState.returns({ + auth: { + oauth: null, + }, + }); + + expectNavigate(mock, '/choose-account'); + + state.enter(context); + }); }); describe('#resolve', () => { @@ -38,6 +56,7 @@ describe('ChooseAccountState', () => { it('should transition to login if user wants to add new account', () => { expectNavigate(mock, '/login'); + expectRun(mock, 'setLogin', null); expectState(mock, LoginState); state.resolve(context, {}); diff --git a/src/services/authFlow/PasswordState.js b/src/services/authFlow/PasswordState.js index 38de0fb..a598882 100644 --- a/src/services/authFlow/PasswordState.js +++ b/src/services/authFlow/PasswordState.js @@ -3,6 +3,7 @@ import logger from 'services/logger'; import { getCredentials } from 'components/auth/reducer'; import AbstractState from './AbstractState'; +import ChooseAccountState from './ChooseAccountState'; import CompleteState from './CompleteState'; import ForgotPasswordState from './ForgotPasswordState'; import LoginState from './LoginState'; @@ -31,7 +32,7 @@ export default class PasswordState extends AbstractState { rememberMe: bool } ) { - const {login} = getCredentials(context.getState()); + const { login, returnUrl } = getCredentials(context.getState()); return context.run('login', { password, @@ -39,12 +40,17 @@ export default class PasswordState extends AbstractState { login }) .then(() => { - const {isTotpRequired} = getCredentials(context.getState()); + const { isTotpRequired } = getCredentials(context.getState()); if (isTotpRequired) { return context.setState(new MfaState()); } + if (returnUrl) { + context.navigate(returnUrl); + return; + } + return context.setState(new CompleteState()); }) .catch((err = {}) => @@ -57,7 +63,13 @@ export default class PasswordState extends AbstractState { } goBack(context: AuthContext) { - context.run('setLogin', null); - context.setState(new LoginState()); + const { isRelogin } = getCredentials(context.getState()); + + if (isRelogin) { + context.setState(new ChooseAccountState()); + } else { + context.run('setLogin', null); + context.setState(new LoginState()); + } } } diff --git a/src/services/authFlow/PasswordState.test.js b/src/services/authFlow/PasswordState.test.js index a73b54a..30a3aaf 100644 --- a/src/services/authFlow/PasswordState.test.js +++ b/src/services/authFlow/PasswordState.test.js @@ -3,8 +3,10 @@ import sinon from 'sinon'; import PasswordState from 'services/authFlow/PasswordState'; import CompleteState from 'services/authFlow/CompleteState'; +import MfaState from 'services/authFlow/MfaState'; import LoginState from 'services/authFlow/LoginState'; import ForgotPasswordState from 'services/authFlow/ForgotPasswordState'; +import ChooseAccountState from 'services/authFlow/ChooseAccountState'; import { bootstrap, expectState, expectNavigate, expectRun } from './helpers'; @@ -82,6 +84,67 @@ describe('PasswordState', () => { return expect(state.resolve(context, payload), 'to be fulfilled'); }); + + it('should go to MfaState if totp required', () => { + const expectedLogin = 'foo'; + const expectedPassword = 'bar'; + const expectedRememberMe = true; + + context.getState.returns({ + auth: { + credentials: { + login: expectedLogin, + isTotpRequired: true, + } + } + }); + + expectRun( + mock, + 'login', + sinon.match({ + login: expectedLogin, + password: expectedPassword, + rememberMe: expectedRememberMe, + }) + ).returns(Promise.resolve()); + expectState(mock, MfaState); + + const payload = {password: expectedPassword, rememberMe: expectedRememberMe}; + + return expect(state.resolve(context, payload), 'to be fulfilled'); + }); + + it('should navigate to returnUrl if any', () => { + const expectedLogin = 'foo'; + const expectedPassword = 'bar'; + const expectedReturnUrl = '/returnUrl'; + const expectedRememberMe = true; + + context.getState.returns({ + auth: { + credentials: { + login: expectedLogin, + returnUrl: expectedReturnUrl, + } + } + }); + + expectRun( + mock, + 'login', + sinon.match({ + login: expectedLogin, + password: expectedPassword, + rememberMe: expectedRememberMe, + }) + ).returns(Promise.resolve()); + expectNavigate(mock, expectedReturnUrl); + + const payload = {password: expectedPassword, rememberMe: expectedRememberMe}; + + return expect(state.resolve(context, payload), 'to be fulfilled'); + }); }); describe('#reject', () => { @@ -94,10 +157,33 @@ describe('PasswordState', () => { describe('#goBack', () => { it('should transition to login state', () => { + context.getState.returns({ + auth: { + credentials: { + login: 'foo' + } + } + }); + expectRun(mock, 'setLogin', null); expectState(mock, LoginState); state.goBack(context); }); + + it('should transition to ChooseAccountState if this is relogin', () => { + context.getState.returns({ + auth: { + credentials: { + login: 'foo', + isRelogin: true, + } + } + }); + + expectState(mock, ChooseAccountState); + + state.goBack(context); + }); }); });