From e31e97afba0c3f351700162baa0842176c5883b3 Mon Sep 17 00:00:00 2001 From: Vijay Budhram Date: Fri, 13 Apr 2018 10:56:32 -0400 Subject: [PATCH] fix(email): send correct email when using unblock code (#6064), r=@philbooth, @shane-tomlinson --- app/scripts/models/account.js | 32 ++++++--- app/tests/spec/models/account.js | 40 +++++++++++ tests/functional/lib/selectors.js | 1 + tests/functional/settings_change_email.js | 81 ++++++++++++++++++++++- 4 files changed, 143 insertions(+), 11 deletions(-) diff --git a/app/scripts/models/account.js b/app/scripts/models/account.js index ef7e3fdea..3ef20e37b 100644 --- a/app/scripts/models/account.js +++ b/app/scripts/models/account.js @@ -37,6 +37,11 @@ define(function (require, exports, module) { // the user since. Is it enough to just move this field to the // 'defaults'? needsOptedInToMarketingEmail: undefined, + // This property is set when a user has changed their primary email address and + // attempts to login. Similar to logging in with a different email capitalization + // the auth-server will return the proper email to reattempt the login. When reattempting + // to login, this needs to be passed back to the auth-server. + originalLoginEmail: undefined, // password field intentionally omitted to avoid unintentional leaks permissions: undefined, profileImageId: undefined, @@ -439,8 +444,9 @@ define(function (require, exports, module) { // `originalLoginEmail` is specified when the account's primary email has changed. // This param lets the auth-server known that it should check that this email // is the current primary for the account. - if (options.originalLoginEmail) { - signinOptions.originalLoginEmail = options.originalLoginEmail; + const originalLoginEmail = this.get('originalLoginEmail'); + if (originalLoginEmail) { + signinOptions.originalLoginEmail = originalLoginEmail; } if (options.verificationMethod) { @@ -489,16 +495,26 @@ define(function (require, exports, module) { // the user's password with. if (AuthErrors.is(err, 'INCORRECT_EMAIL_CASE')) { - // Save the original email that was used for login so that the auth-server - // can verify that this is the accounts primary email address. - options.originalLoginEmail = email; + // Save the original email that was used for login. This value will be + // sent to the auth-server so that it can correctly look the account. + this.set('originalLoginEmail', email); - // The server will respond with the canonical email - // for this account. Use it hereafter. + // The email returned in the `INCORRECT_EMAIL_CASE` is either the canonical email + // address or if the primary email has changed, it is the email the account was first + // created with. this.set('email', err.email); return this.signIn(password, relier, options); - } + } else if (AuthErrors.is(err, 'THROTTLED') || + AuthErrors.is(err, 'REQUEST_BLOCKED')) { + // On a throttled or block login request, the account model's email could be storing + // a canonical email address or email the account was created with. If this is the case + // set the account model's email to the email first used for the login request. + const originalLoginEmail = this.get('originalLoginEmail'); + if (originalLoginEmail) { + this.set('email', originalLoginEmail); + } + } throw err; }); }, diff --git a/app/tests/spec/models/account.js b/app/tests/spec/models/account.js index 16fbb2ae1..32a4f41a0 100644 --- a/app/tests/spec/models/account.js +++ b/app/tests/spec/models/account.js @@ -483,9 +483,49 @@ define(function (require, exports, module) { fxaClient.signIn.calledWith(EMAIL, PASSWORD, relier, secondExpectedOptions)); assert.equal(account.get('email'), EMAIL); + assert.equal(account.get('originalLoginEmail'), upperCaseEmail); }); }); + ['REQUEST_BLOCKED', 'THROTTLED'].forEach((errorName) => { + describe(errorName, () => { + const primaryEmail = 'primaryEmail@email.com'; + const oldPrimaryEmail = EMAIL; + + beforeEach(() => { + sinon.stub(fxaClient, 'signIn').callsFake(() => { + if (fxaClient.signIn.callCount === 1) { + const err = AuthErrors.toError('INCORRECT_EMAIL_CASE'); + err.email = oldPrimaryEmail; + return Promise.reject(err); + } else if (fxaClient.signIn.callCount === 2) { + const err = AuthErrors.toError(errorName); + return Promise.reject(err); + } else { + return Promise.resolve({}); + } + }); + + account.set('email', primaryEmail); + return account.signIn(PASSWORD, relier, { + unblockCode: 'unblock code' + }).then(assert.fail, (err) => { + assert.isTrue(AuthErrors.is(err, errorName)); + }); + }); + + it('re-tries login and restores email to primary email address', () => { + assert.equal(fxaClient.signIn.callCount, 2); + let args = fxaClient.signIn.args[0]; + assert.equal(args[0], primaryEmail, 'sign-in first called with primary email'); + args = fxaClient.signIn.args[1]; + assert.equal(args[0], oldPrimaryEmail, 'sign-in then called with old primary email'); + assert.equal(account.get('email'), primaryEmail, 'primary email restored'); + }); + }); + }); + + describe('error', () => { let err; diff --git a/tests/functional/lib/selectors.js b/tests/functional/lib/selectors.js index 07fd62145..0fcfff128 100644 --- a/tests/functional/lib/selectors.js +++ b/tests/functional/lib/selectors.js @@ -194,6 +194,7 @@ module.exports = { EMAIL_FIELD: '.verification-email-message', HEADER: '#fxa-signin-unblock-header', SUBMIT: 'button[type="submit"]', + VERIFICATION: '.verification-email-message', }, SIGNUP: { AGE: '#age', diff --git a/tests/functional/settings_change_email.js b/tests/functional/settings_change_email.js index 2819a75e5..b7a82a8a0 100644 --- a/tests/functional/settings_change_email.js +++ b/tests/functional/settings_change_email.js @@ -19,6 +19,7 @@ const NEW_PASSWORD = 'password1'; let email; let secondaryEmail; +let newPrimaryEmail; const { clearBrowserState, @@ -29,12 +30,14 @@ const { fillOutCompleteResetPassword, fillOutSignUp, fillOutSignIn, + fillOutSignInUnblock, openPage, openVerificationLinkInNewTab, openVerificationLinkInSameTab, switchToWindow, testElementExists, testElementTextEquals, + testElementTextInclude, testErrorTextInclude, testSuccessWasShown, type, @@ -75,7 +78,7 @@ registerSuite('settings change email', { tests: { 'can change primary email and login': function () { return this.remote - // sign out + // sign out .then(click(selectors.SETTINGS.SIGNOUT)) .then(testElementExists(selectors.SIGNIN.HEADER)) @@ -96,7 +99,7 @@ registerSuite('settings change email', { 'can change primary email, change password and login': function () { return this.remote - // change password + // change password .then(click(selectors.CHANGE_PASSWORD.MENU_BUTTON)) .then(fillOutChangePassword(PASSWORD, NEW_PASSWORD)) @@ -143,7 +146,7 @@ registerSuite('settings change email', { 'can change primary email, change password, login, change email and login': function () { return this.remote - // change password + // change password .then(click(selectors.CHANGE_PASSWORD.MENU_BUTTON)) .then(fillOutChangePassword(PASSWORD, NEW_PASSWORD)) @@ -169,6 +172,78 @@ registerSuite('settings change email', { .then(testElementExists(selectors.SIGNIN.HEADER)) .then(fillOutSignIn(email, NEW_PASSWORD)) .then(testElementExists(selectors.SETTINGS.HEADER)); + }, + } +}); + +registerSuite('settings change email - unblock', { + beforeEach: function () { + email = TestHelpers.createEmail(); + + // Create a new primary email that is always forced through the unblock flow + newPrimaryEmail = TestHelpers.createEmail('block{id}'); + return this.remote.then(clearBrowserState()) + .then(openPage(SIGNUP_URL, selectors.SIGNUP.HEADER)) + .then(fillOutSignUp(email, PASSWORD)) + .then(testElementExists(selectors.CONFIRM_SIGNUP.HEADER)) + .then(openVerificationLinkInSameTab(email, 0)) + .then(testElementExists(selectors.SETTINGS.HEADER)) + .then(click(selectors.EMAIL.MENU_BUTTON)) + + // add secondary email, verify + .then(type(selectors.EMAIL.INPUT, newPrimaryEmail)) + .then(click(selectors.EMAIL.ADD_BUTTON)) + .then(testElementExists(selectors.EMAIL.NOT_VERIFIED_LABEL)) + .then(openVerificationLinkInSameTab(newPrimaryEmail, 0, {})) + .then(testSuccessWasShown()) + + // set new primary email + .then(openPage(SETTINGS_URL, selectors.SETTINGS.HEADER)) + .then(click(selectors.EMAIL.MENU_BUTTON)) + .then(testElementTextEquals(selectors.EMAIL.ADDRESS_LABEL, newPrimaryEmail)) + .then(testElementExists(selectors.EMAIL.VERIFIED_LABEL)) + .then(click(selectors.EMAIL.SET_PRIMARY_EMAIL_BUTTON)) + .then(visibleByQSA(selectors.EMAIL.SUCCESS)) + + // sign out + .then(click(selectors.SETTINGS.SIGNOUT)) + .then(testElementExists(selectors.SIGNIN.HEADER)); + }, + + afterEach: function () { + return this.remote.then(clearBrowserState()); + }, + + tests: { + 'can change primary email, get blocked with invalid password, redirect login page': function () { + return this.remote + // sign in + .then(openPage(SIGNIN_URL, selectors.SIGNIN.HEADER)) + .then(fillOutSignIn(newPrimaryEmail, 'INVALID_PASSWORD')) + + // fill out unblock + .then(testElementExists(selectors.SIGNIN_UNBLOCK.HEADER)) + .then(testElementTextInclude(selectors.SIGNIN_UNBLOCK.VERIFICATION, newPrimaryEmail)) + .then(fillOutSignInUnblock(newPrimaryEmail, 2)) + + // redirected to login + .then(testElementExists(selectors.SIGNIN.HEADER)); + }, + + 'can change primary email, get blocked with valid password, redirect settings page': function () { + return this.remote + // sign in + .then(openPage(SIGNIN_URL, selectors.SIGNIN.HEADER)) + .then(fillOutSignIn(newPrimaryEmail, PASSWORD)) + + // fill out unblock + .then(testElementExists(selectors.SIGNIN_UNBLOCK.HEADER)) + .then(testElementTextInclude(selectors.SIGNIN_UNBLOCK.VERIFICATION, newPrimaryEmail)) + .then(fillOutSignInUnblock(newPrimaryEmail, 2)) + + // redirected to settings + .then(testElementExists(selectors.SETTINGS.HEADER)); } } }); +