From a6858139347b11af26a04c9e360ad8667807e1ed Mon Sep 17 00:00:00 2001 From: SleepWalker Date: Tue, 31 Jan 2017 08:05:36 +0200 Subject: [PATCH] #245: add register link to account chooser auth panel --- src/components/accounts/actions.js | 13 +++++- .../chooseAccount/ChooseAccount.intl.json | 1 + .../auth/chooseAccount/ChooseAccount.jsx | 6 ++- src/services/authFlow/ChooseAccountState.js | 17 +++++--- src/services/authFlow/PasswordState.js | 6 +-- src/services/authFlow/RegisterState.js | 8 +--- src/services/history.js | 2 +- tests/components/accounts/actions.test.js | 43 +++++++++++++++---- .../authFlow/ChooseAccountState.test.js | 13 ++++-- tests/services/authFlow/PasswordState.test.js | 1 - tests/services/authFlow/RegisterState.test.js | 10 ----- 11 files changed, 77 insertions(+), 43 deletions(-) diff --git a/src/components/accounts/actions.js b/src/components/accounts/actions.js index be39f57..e60c0e1 100644 --- a/src/components/accounts/actions.js +++ b/src/components/accounts/actions.js @@ -3,6 +3,7 @@ import { routeActions } from 'react-router-redux'; import authentication from 'services/api/authentication'; import { updateUser, setGuest } from 'components/user/actions'; import { setLocale } from 'components/i18n/actions'; +import { setAccountSwitcher } from 'components/auth/actions'; import logger from 'services/logger'; import { @@ -32,7 +33,7 @@ export { updateToken }; * @return {function} */ export function authenticate({token, refreshToken}) { - return (dispatch) => + return (dispatch, getState) => authentication.validateToken({token, refreshToken}) .catch((resp) => { logger.warn('Error validating token during auth', { @@ -56,6 +57,8 @@ export function authenticate({token, refreshToken}) { } })) .then(({user, account}) => { + const {auth} = getState(); + dispatch(add(account)); dispatch(activate(account)); dispatch(updateUser(user)); @@ -68,6 +71,14 @@ export function authenticate({token, refreshToken}) { sessionStorage.setItem(`stranger${account.id}`, 1); } + if (auth && auth.oauth && auth.oauth.clientId) { + // if we authenticating during oauth, we disable account chooser + // because user probably has made his choise now + // this may happen, when user registers, logs in or uses account + // chooser panel during oauth + dispatch(setAccountSwitcher(false)); + } + return dispatch(setLocale(user.lang)) .then(() => account); }); diff --git a/src/components/auth/chooseAccount/ChooseAccount.intl.json b/src/components/auth/chooseAccount/ChooseAccount.intl.json index 9b207ca..d614678 100644 --- a/src/components/auth/chooseAccount/ChooseAccount.intl.json +++ b/src/components/auth/chooseAccount/ChooseAccount.intl.json @@ -2,5 +2,6 @@ "chooseAccountTitle": "Choose an account", "addAccount": "Log into another account", "logoutAll": "Log out from all accounts", + "createNewAccount": "Create new account", "description": "You have logged in into multiple accounts. Please choose the one, you want to use to authorize {appName}" } diff --git a/src/components/auth/chooseAccount/ChooseAccount.jsx b/src/components/auth/chooseAccount/ChooseAccount.jsx index 0aaa6de..39af5e3 100644 --- a/src/components/auth/chooseAccount/ChooseAccount.jsx +++ b/src/components/auth/chooseAccount/ChooseAccount.jsx @@ -10,7 +10,11 @@ export default factory({ }, links: [ { - label: messages.logoutAll + label: messages.createNewAccount + }, + { + label: messages.logoutAll, + payload: {logout: true} } ] }); diff --git a/src/services/authFlow/ChooseAccountState.js b/src/services/authFlow/ChooseAccountState.js index ffa57b6..92de6b8 100644 --- a/src/services/authFlow/ChooseAccountState.js +++ b/src/services/authFlow/ChooseAccountState.js @@ -1,6 +1,7 @@ import AbstractState from './AbstractState'; import LoginState from './LoginState'; import CompleteState from './CompleteState'; +import RegisterState from './RegisterState'; export default class ChooseAccountState extends AbstractState { enter(context) { @@ -8,9 +9,6 @@ export default class ChooseAccountState extends AbstractState { } resolve(context, payload) { - // do not ask again after user adds account, or chooses an existed one - context.run('setAccountSwitcher', false); - if (payload.id) { context.setState(new CompleteState()); } else { @@ -19,7 +17,16 @@ export default class ChooseAccountState extends AbstractState { } } - reject(context) { - context.run('logout'); + /** + * @param {object} context + * @param {object} payload + * @param {bool} [payload.logout=false] + */ + reject(context, payload = {}) { + if (payload.logout) { + context.run('logout'); + } else { + context.setState(new RegisterState()); + } } } diff --git a/src/services/authFlow/PasswordState.js b/src/services/authFlow/PasswordState.js index 027f9d3..f6d056b 100644 --- a/src/services/authFlow/PasswordState.js +++ b/src/services/authFlow/PasswordState.js @@ -24,11 +24,7 @@ export default class PasswordState extends AbstractState { rememberMe, login }) - .then(() => { - context.run('setAccountSwitcher', false); - - context.setState(new CompleteState()); - }) + .then(() => context.setState(new CompleteState())) .catch((err = {}) => err.errors || logger.warn(err)); } diff --git a/src/services/authFlow/RegisterState.js b/src/services/authFlow/RegisterState.js index af0bd4c..1503bbd 100644 --- a/src/services/authFlow/RegisterState.js +++ b/src/services/authFlow/RegisterState.js @@ -7,13 +7,7 @@ import ResendActivationState from './ResendActivationState'; export default class RegisterState extends AbstractState { enter(context) { - const {user} = context.getState(); - - if (user.isGuest) { - context.navigate('/register'); - } else { - context.setState(new CompleteState()); - } + context.navigate('/register'); } resolve(context, payload) { diff --git a/src/services/history.js b/src/services/history.js index a8712e1..8828b31 100644 --- a/src/services/history.js +++ b/src/services/history.js @@ -14,4 +14,4 @@ export default { return document.referrer.includes(`${location.protocol}//${location.host}`) || this.initialLength < window.history.length; } -} +}; diff --git a/tests/components/accounts/actions.test.js b/tests/components/accounts/actions.test.js index 236e3f7..c532db5 100644 --- a/tests/components/accounts/actions.test.js +++ b/tests/components/accounts/actions.test.js @@ -19,6 +19,7 @@ import { import { SET_LOCALE } from 'components/i18n/actions'; import { updateUser, setUser } from 'components/user/actions'; +import { setAccountSwitcher } from 'components/auth/actions'; const account = { id: 1, @@ -67,7 +68,7 @@ describe('components/accounts/actions', () => { describe('#authenticate()', () => { it('should request user state using token', () => - authenticate(account)(dispatch).then(() => + authenticate(account)(dispatch, getState).then(() => expect(authentication.validateToken, 'to have a call satisfying', [ {token: account.token, refreshToken: account.refreshToken} ]) @@ -75,7 +76,7 @@ describe('components/accounts/actions', () => { ); it(`dispatches ${ADD} action`, () => - authenticate(account)(dispatch).then(() => + authenticate(account)(dispatch, getState).then(() => expect(dispatch, 'to have a call satisfying', [ add(account) ]) @@ -83,7 +84,7 @@ describe('components/accounts/actions', () => { ); it(`dispatches ${ACTIVATE} action`, () => - authenticate(account)(dispatch).then(() => + authenticate(account)(dispatch, getState).then(() => expect(dispatch, 'to have a call satisfying', [ activate(account) ]) @@ -91,7 +92,7 @@ describe('components/accounts/actions', () => { ); it(`dispatches ${SET_LOCALE} action`, () => - authenticate(account)(dispatch).then(() => + authenticate(account)(dispatch, getState).then(() => expect(dispatch, 'to have a call satisfying', [ {type: SET_LOCALE, payload: {locale: 'be'}} ]) @@ -99,7 +100,7 @@ describe('components/accounts/actions', () => { ); it('should update user state', () => - authenticate(account)(dispatch).then(() => + authenticate(account)(dispatch, getState).then(() => expect(dispatch, 'to have a call satisfying', [ updateUser({...user, isGuest: false}) ]) @@ -107,7 +108,7 @@ describe('components/accounts/actions', () => { ); it('resolves with account', () => - authenticate(account)(dispatch).then((resp) => + authenticate(account)(dispatch, getState).then((resp) => expect(resp, 'to equal', account) ) ); @@ -115,7 +116,7 @@ describe('components/accounts/actions', () => { it('rejects when bad auth data', () => { authentication.validateToken.returns(Promise.reject({})); - return expect(authenticate(account)(dispatch), 'to be rejected').then(() => { + return expect(authenticate(account)(dispatch, getState), 'to be rejected').then(() => { expect(dispatch, 'to have a call satisfying', [ {payload: {isGuest: true}}, ]); @@ -134,11 +135,37 @@ describe('components/accounts/actions', () => { sessionStorage.removeItem(expectedKey); - return authenticate(account)(dispatch).then(() => { + return authenticate(account)(dispatch, getState).then(() => { expect(sessionStorage.getItem(expectedKey), 'not to be null'); sessionStorage.removeItem(expectedKey); }); }); + + describe('when user authenticated during oauth', () => { + beforeEach(() => { + getState.returns({ + accounts: { + available: [], + active: null + }, + user: {}, + auth: { + oauth: { + clientId: 'ely.by', + prompt: [] + } + } + }); + }); + + it('should dispatch setAccountSwitcher', () => + authenticate(account)(dispatch, getState).then(() => + expect(dispatch, 'to have a call satisfying', [ + setAccountSwitcher(false) + ]) + ) + ); + }); }); describe('#revoke()', () => { diff --git a/tests/services/authFlow/ChooseAccountState.test.js b/tests/services/authFlow/ChooseAccountState.test.js index 58f2021..f600eef 100644 --- a/tests/services/authFlow/ChooseAccountState.test.js +++ b/tests/services/authFlow/ChooseAccountState.test.js @@ -1,6 +1,7 @@ import ChooseAccountState from 'services/authFlow/ChooseAccountState'; import CompleteState from 'services/authFlow/CompleteState'; import LoginState from 'services/authFlow/LoginState'; +import RegisterState from 'services/authFlow/RegisterState'; import { bootstrap, expectState, expectNavigate, expectRun } from './helpers'; @@ -31,14 +32,12 @@ describe('ChooseAccountState', () => { describe('#resolve', () => { it('should transition to complete if existed account was choosen', () => { - expectRun(mock, 'setAccountSwitcher', false); expectState(mock, CompleteState); state.resolve(context, {id: 123}); }); it('should transition to login if user wants to add new account', () => { - expectRun(mock, 'setAccountSwitcher', false); expectNavigate(mock, '/login'); expectState(mock, LoginState); @@ -47,10 +46,16 @@ describe('ChooseAccountState', () => { }); describe('#reject', () => { - it('should logout', () => { - expectRun(mock, 'logout'); + it('should transition to register', () => { + expectState(mock, RegisterState); state.reject(context); }); + + it('should logout', () => { + expectRun(mock, 'logout'); + + state.reject(context, {logout: true}); + }); }); }); diff --git a/tests/services/authFlow/PasswordState.test.js b/tests/services/authFlow/PasswordState.test.js index 18d2c57..ad1b906 100644 --- a/tests/services/authFlow/PasswordState.test.js +++ b/tests/services/authFlow/PasswordState.test.js @@ -61,7 +61,6 @@ describe('PasswordState', () => { } }); - expectRun(mock, 'setAccountSwitcher', false); expectRun( mock, 'login', diff --git a/tests/services/authFlow/RegisterState.test.js b/tests/services/authFlow/RegisterState.test.js index 94db655..1b2a895 100644 --- a/tests/services/authFlow/RegisterState.test.js +++ b/tests/services/authFlow/RegisterState.test.js @@ -34,16 +34,6 @@ describe('RegisterState', () => { state.enter(context); }); - - it('should transition to complete if not guest', () => { - context.getState.returns({ - user: {isGuest: false} - }); - - expectState(mock, CompleteState); - - state.enter(context); - }); }); describe('#resolve', () => {