From b1774d8354ca35b6cfdb596af079e9f4da4e96a0 Mon Sep 17 00:00:00 2001 From: SleepWalker Date: Sun, 28 Mar 2021 18:07:46 +0300 Subject: [PATCH] Allow escape from activation step back to login page --- packages/app/components/auth/actions.ts | 25 +++---- packages/app/components/auth/reducer.ts | 1 + .../app/components/profile/Profile.story.tsx | 1 - packages/app/components/user/reducer.ts | 2 - packages/app/services/api/signup.ts | 2 +- .../authFlow/AuthFlow.functional.test.ts | 2 +- .../services/authFlow/CompleteState.test.ts | 69 +++++++++++-------- .../app/services/authFlow/CompleteState.ts | 8 +-- .../cypress/integration/auth/register.test.ts | 49 +++++++++++++ 9 files changed, 105 insertions(+), 54 deletions(-) diff --git a/packages/app/components/auth/actions.ts b/packages/app/components/auth/actions.ts index 7f163ad..1a60685 100644 --- a/packages/app/components/auth/actions.ts +++ b/packages/app/components/auth/actions.ts @@ -92,7 +92,7 @@ export function login({ if (resp.errors.password === PASSWORD_REQUIRED) { return dispatch(setLogin(login)); } else if (resp.errors.login === ACTIVATION_REQUIRED) { - return dispatch(needActivation()); + return dispatch(needActivation({ login })); } else if (resp.errors.totp === TOTP_REQUIRED) { return dispatch( requestTotp({ @@ -174,14 +174,7 @@ export function register({ captcha, }) .then(() => { - dispatch( - updateUser({ - username, - email, - }), - ); - - dispatch(needActivation()); + dispatch(needActivation({ login: email || username })); browserHistory.push('/activation'); }) @@ -232,8 +225,6 @@ function setCredentials(payload: Credentials | null): SetCredentialsAction { /** * Sets login in credentials state * Resets the state, when `null` is passed - * - * @param login */ export function setLogin(login: string | null): SetCredentialsAction { return setCredentials(login ? { login } : null); @@ -532,11 +523,11 @@ export function resetOAuth(): AppAction { * Resets all temporary state related to auth */ export function resetAuth(): AppAction { - return (dispatch, getSate): Promise => { + return (dispatch, getState): Promise => { dispatch(setLogin(null)); dispatch(resetOAuth()); // ensure current account is valid - const activeAccount = getActiveAccount(getSate()); + const activeAccount = getActiveAccount(getState()); if (activeAccount) { return Promise.resolve(dispatch(authenticate(activeAccount))) @@ -610,10 +601,10 @@ function wrapInLoader(fn: AppAction>): AppAction> { export type LoadingAction = SetLoadingAction; -function needActivation() { - return updateUser({ - isActive: false, - isGuest: false, +function needActivation({ login }: { login: string }) { + return setCredentials({ + login, + isActivationRequired: true, }); } diff --git a/packages/app/components/auth/reducer.ts b/packages/app/components/auth/reducer.ts index 0adcd0d..1862399 100644 --- a/packages/app/components/auth/reducer.ts +++ b/packages/app/components/auth/reducer.ts @@ -20,6 +20,7 @@ export interface Credentials { returnUrl?: string; isRelogin?: boolean; isTotpRequired?: boolean; + isActivationRequired?: boolean; } type Error = Record< diff --git a/packages/app/components/profile/Profile.story.tsx b/packages/app/components/profile/Profile.story.tsx index 4539646..64489ba 100644 --- a/packages/app/components/profile/Profile.story.tsx +++ b/packages/app/components/profile/Profile.story.tsx @@ -20,7 +20,6 @@ storiesOf('Components/Profile', module).add('Profile', () => ( username: 'ErickSkrauch', email: 'erickskrauch@ely.by', hasMojangUsernameCollision: true, - isActive: true, isGuest: false, isDeleted: false, isOtpEnabled: true, diff --git a/packages/app/components/user/reducer.ts b/packages/app/components/user/reducer.ts index 06bcef1..72659dc 100644 --- a/packages/app/components/user/reducer.ts +++ b/packages/app/components/user/reducer.ts @@ -9,7 +9,6 @@ export interface User { avatar: string; lang: string; isGuest: boolean; - isActive: boolean; isDeleted: boolean; isOtpEnabled: boolean; passwordChangedAt: number; @@ -31,7 +30,6 @@ const defaults: State = { maskedEmail: '', avatar: '', lang: '', - isActive: false, isDeleted: false, isOtpEnabled: false, shouldAcceptRules: false, // whether user need to review updated rules diff --git a/packages/app/services/api/signup.ts b/packages/app/services/api/signup.ts index 72d5031..2ec97cc 100644 --- a/packages/app/services/api/signup.ts +++ b/packages/app/services/api/signup.ts @@ -20,7 +20,7 @@ export function register({ rulesAgreement = false, lang = '', captcha = '', -}: RegisterParams): Promise> { +}: RegisterParams): Promise> { return request.post( '/api/signup', { email, username, password, rePassword, rulesAgreement, lang, captcha }, diff --git a/packages/app/services/authFlow/AuthFlow.functional.test.ts b/packages/app/services/authFlow/AuthFlow.functional.test.ts index a9c7290..376b379 100644 --- a/packages/app/services/authFlow/AuthFlow.functional.test.ts +++ b/packages/app/services/authFlow/AuthFlow.functional.test.ts @@ -95,10 +95,10 @@ describe('AuthFlow.functional', () => { Object.assign(state, { user: { isGuest: false, - isActive: true, }, auth: { + credentials: {}, oauth: { clientId: 123, prompt: [], diff --git a/packages/app/services/authFlow/CompleteState.test.ts b/packages/app/services/authFlow/CompleteState.test.ts index 748e889..85d3576 100644 --- a/packages/app/services/authFlow/CompleteState.test.ts +++ b/packages/app/services/authFlow/CompleteState.test.ts @@ -33,10 +33,11 @@ describe('CompleteState', () => { it('should navigate to / for authenticated', () => { context.getState.returns({ user: { - isActive: true, isGuest: false, }, - auth: {}, + auth: { + credentials: {}, + }, }); expectNavigate(mock, '/'); @@ -49,7 +50,9 @@ describe('CompleteState', () => { user: { isGuest: true, }, - auth: {}, + auth: { + credentials: {}, + }, }); expectState(mock, LoginState); @@ -62,7 +65,11 @@ describe('CompleteState', () => { user: { isGuest: false, }, - auth: {}, + auth: { + credentials: { + isActivationRequired: true, + }, + }, }); expectState(mock, ActivationState); @@ -74,11 +81,12 @@ describe('CompleteState', () => { context.getState.returns({ user: { isGuest: false, - isActive: true, shouldAcceptRules: true, isDeleted: true, }, - auth: {}, + auth: { + credentials: {}, + }, }); expectNavigate(mock, '/'); @@ -90,10 +98,11 @@ describe('CompleteState', () => { context.getState.returns({ user: { shouldAcceptRules: true, - isActive: true, isGuest: false, }, - auth: {}, + auth: { + credentials: {}, + }, }); expectState(mock, AcceptRulesState); @@ -107,7 +116,11 @@ describe('CompleteState', () => { shouldAcceptRules: true, isGuest: false, }, - auth: {}, + auth: { + credentials: { + isActivationRequired: true, + }, + }, }); expectState(mock, ActivationState); @@ -119,10 +132,10 @@ describe('CompleteState', () => { it('should transition to finish state if code is present', () => { context.getState.returns({ user: { - isActive: true, isGuest: false, }, auth: { + credentials: {}, oauth: { clientId: 'ely.by', code: 'XXX', @@ -139,10 +152,10 @@ describe('CompleteState', () => { it('should transition to permissions state if acceptRequired', () => { context.getState.returns({ user: { - isActive: true, isGuest: false, }, auth: { + credentials: {}, oauth: { clientId: 'ely.by', acceptRequired: true, @@ -158,10 +171,10 @@ describe('CompleteState', () => { it('should transition to permissions state if prompt=consent', () => { context.getState.returns({ user: { - isActive: true, isGuest: false, }, auth: { + credentials: {}, oauth: { clientId: 'ely.by', prompt: ['consent'], @@ -179,7 +192,6 @@ describe('CompleteState', () => { it('should transition to ChooseAccountState if user has multiple accs and switcher enabled', () => { context.getState.returns({ user: { - isActive: true, isGuest: false, }, accounts: { @@ -187,6 +199,7 @@ describe('CompleteState', () => { active: 1, }, auth: { + credentials: {}, isSwitcherEnabled: true, oauth: { clientId: 'ely.by', @@ -203,7 +216,6 @@ describe('CompleteState', () => { it('should transition to ChooseAccountState if user isDeleted', () => { context.getState.returns({ user: { - isActive: true, isDeleted: true, isGuest: false, }, @@ -213,6 +225,7 @@ describe('CompleteState', () => { }, auth: { isSwitcherEnabled: true, + credentials: {}, oauth: { clientId: 'ely.by', prompt: [], @@ -228,7 +241,6 @@ describe('CompleteState', () => { it('should NOT transition to ChooseAccountState if user has multiple accs and switcher disabled', () => { context.getState.returns({ user: { - isActive: true, isGuest: false, }, accounts: { @@ -237,6 +249,7 @@ describe('CompleteState', () => { }, auth: { isSwitcherEnabled: false, + credentials: {}, oauth: { clientId: 'ely.by', prompt: [], @@ -252,7 +265,6 @@ describe('CompleteState', () => { it('should transition to ChooseAccountState if prompt=select_account and switcher enabled', () => { context.getState.returns({ user: { - isActive: true, isGuest: false, }, accounts: { @@ -261,6 +273,7 @@ describe('CompleteState', () => { }, auth: { isSwitcherEnabled: true, + credentials: {}, oauth: { clientId: 'ely.by', prompt: ['select_account'], @@ -276,7 +289,6 @@ describe('CompleteState', () => { it('should NOT transition to ChooseAccountState if prompt=select_account and switcher disabled', () => { context.getState.returns({ user: { - isActive: true, isGuest: false, }, accounts: { @@ -285,6 +297,7 @@ describe('CompleteState', () => { }, auth: { isSwitcherEnabled: false, + credentials: {}, oauth: { clientId: 'ely.by', prompt: ['select_account'], @@ -304,10 +317,10 @@ describe('CompleteState', () => { it('should run oAuthComplete', () => { context.getState.returns({ user: { - isActive: true, isGuest: false, }, auth: { + credentials: {}, oauth: { clientId: 'ely.by', prompt: [], @@ -325,10 +338,10 @@ describe('CompleteState', () => { it('should listen for auth success/failure', () => { context.getState.returns({ user: { - isActive: true, isGuest: false, }, auth: { + credentials: {}, oauth: { clientId: 'ely.by', prompt: [], @@ -352,10 +365,10 @@ describe('CompleteState', () => { context.getState.returns({ user: { - isActive: true, isGuest: false, }, auth: { + credentials: {}, oauth: { clientId: 'ely.by', prompt: [], @@ -381,10 +394,10 @@ describe('CompleteState', () => { context.getState.returns({ user: { - isActive: true, isGuest: false, }, auth: { + credentials: {}, oauth: { clientId: 'ely.by', prompt: [], @@ -425,7 +438,6 @@ describe('CompleteState', () => { context.getState.returns({ user: { - isActive: true, isGuest: false, }, accounts: { @@ -433,6 +445,7 @@ describe('CompleteState', () => { active: 100, }, auth: { + credentials: {}, oauth: { clientId: 'ely.by', loginHint: account[field], @@ -463,7 +476,6 @@ describe('CompleteState', () => { context.getState.returns({ user: { - isActive: true, isGuest: false, }, accounts: { @@ -471,6 +483,7 @@ describe('CompleteState', () => { active: account.id, }, auth: { + credentials: {}, oauth: { clientId: 'ely.by', loginHint: account.id, @@ -496,7 +509,6 @@ describe('CompleteState', () => { context.getState.returns({ user: { - isActive: true, isGuest: false, }, accounts: { @@ -504,6 +516,7 @@ describe('CompleteState', () => { active: 1, }, auth: { + credentials: {}, oauth: { clientId: 'ely.by', loginHint: account.id, @@ -542,10 +555,10 @@ describe('CompleteState', () => { state = new CompleteState(expected); context.getState.returns({ user: { - isActive: true, isGuest: false, }, auth: { + credentials: {}, oauth: { clientId: 'ely.by', prompt: [], @@ -567,10 +580,10 @@ describe('CompleteState', () => { state = new CompleteState(expected); context.getState.returns({ user: { - isActive: true, isGuest: false, }, auth: { + credentials: {}, oauth: { clientId: 'ely.by', prompt: [], @@ -593,10 +606,10 @@ describe('CompleteState', () => { state = new CompleteState(expected); context.getState.returns({ user: { - isActive: true, isGuest: false, }, auth: { + credentials: {}, oauth: { clientId: 'ely.by', prompt: [], @@ -620,10 +633,10 @@ describe('CompleteState', () => { state = new CompleteState(expected); context.getState.returns({ user: { - isActive: true, isGuest: false, }, auth: { + credentials: {}, oauth: { clientId: 'ely.by', prompt: [], diff --git a/packages/app/services/authFlow/CompleteState.ts b/packages/app/services/authFlow/CompleteState.ts index a432e9f..8678077 100644 --- a/packages/app/services/authFlow/CompleteState.ts +++ b/packages/app/services/authFlow/CompleteState.ts @@ -26,14 +26,14 @@ export default class CompleteState extends AbstractState { enter(context: AuthContext): Promise | void { const { - auth: { oauth }, + auth: { credentials, oauth }, user, } = context.getState(); - if (user.isGuest) { - context.setState(new LoginState()); - } else if (!user.isActive) { + if (credentials.isActivationRequired) { context.setState(new ActivationState()); + } else if (user.isGuest) { + context.setState(new LoginState()); } else if (user.shouldAcceptRules && !user.isDeleted) { context.setState(new AcceptRulesState()); } else if (oauth && oauth.clientId) { diff --git a/tests-e2e/cypress/integration/auth/register.test.ts b/tests-e2e/cypress/integration/auth/register.test.ts index 0ea3413..ef0ce40 100644 --- a/tests-e2e/cypress/integration/auth/register.test.ts +++ b/tests-e2e/cypress/integration/auth/register.test.ts @@ -70,6 +70,55 @@ describe('Register', () => { cy.location('pathname').should('eq', '/'); }); + it('should allow return to login page from activation', () => { + const username = `test${Date.now()}`; + const email = `${Date.now()}@gmail.com`; + const password = String(Date.now()); + const captchaCode = 'captchaCode'; + + cy.server(); + cy.route({ + method: 'POST', + url: '/api/signup', + response: { + success: true, + }, + }).as('signup'); + cy.login({ + accounts: ['default'], + updateState: false, + rawApiResp: true, + }); + cy.visit('/'); + + cy.findByTestId('toolbar').contains('Join').click(); + + cy.location('pathname').should('eq', '/register'); + + cy.get('[name=username]').type(username); + cy.get('[name=email]').type(email); + cy.get('[name=password]').type(password); + cy.get('[name=rePassword]').type(password); + cy.get('[name=rulesAgreement]').should('not.be.checked'); + cy.get('[name=rulesAgreement]').parent().click(); + cy.get('[name=rulesAgreement]').should('be.checked'); + cy.window().should('have.property', 'e2eCaptchaSetCode'); + cy.window().then((win) => { + // fake captcha response + // @ts-ignore + win.e2eCaptchaSetCode(captchaCode); + }); + cy.get('[type=submit]').click(); + + cy.wait('@signup'); + + cy.location('pathname').should('eq', '/activation'); + + cy.findByTestId('home-page').click(); + + cy.location('pathname').should('eq', '/login'); + }); + it('should allow activation', () => { const activationKey = 'activationKey';