From 4919e1d18bf9a7346e315615436f6911f44bf82e Mon Sep 17 00:00:00 2001 From: Zachary Carter Date: Fri, 10 Oct 2014 13:47:48 -0700 Subject: [PATCH] fix(signin): choosing to use a different account clears cached credentials Fixes #1721. --- app/scripts/views/sign_in.js | 46 +++++++++++++++--------------- app/tests/spec/views/sign_in.js | 25 ++++++++-------- tests/functional/sign_in_cached.js | 25 +++++++++------- 3 files changed, 49 insertions(+), 47 deletions(-) diff --git a/app/scripts/views/sign_in.js b/app/scripts/views/sign_in.js index f3b8c6ba2e..ad23405c35 100644 --- a/app/scripts/views/sign_in.js +++ b/app/scripts/views/sign_in.js @@ -37,11 +37,13 @@ function (_, p, BaseView, FormView, SignInTemplate, Session, PasswordMixin, Auth // want the last used email. this.prefillEmail = Session.prefillEmail || this.searchParam('email'); + var suggestedUser = this._suggestedUser(); + return { service: this.relier.get('service'), serviceName: this.relier.get('serviceName'), email: this.prefillEmail, - suggestedUser: this._suggestedUser(), + suggestedUser: suggestedUser, chooserAskForPassword: this._suggestedUserAskPassword(), password: Session.prefillPassword, error: this.error @@ -95,24 +97,24 @@ function (_, p, BaseView, FormView, SignInTemplate, Session, PasswordMixin, Auth p.reject(); } }) - .then(function (accountData) { - if (accountData.verified) { - return self.onSignInSuccess(); - } else { - return self.onSignInUnverified(); - } - }) - .then(null, function (err) { - if (AuthErrors.is(err, 'UNKNOWN_ACCOUNT')) { - return self._suggestSignUp(err); - } else if (AuthErrors.is(err, 'USER_CANCELED_LOGIN')) { - self.logEvent('login.canceled'); - // if user canceled login, just stop - return; - } - // re-throw error, it will be handled at a lower level. - throw err; - }); + .then(function (accountData) { + if (accountData.verified) { + return self.onSignInSuccess(); + } else { + return self.onSignInUnverified(); + } + }) + .then(null, function (err) { + if (AuthErrors.is(err, 'UNKNOWN_ACCOUNT')) { + return self._suggestSignUp(err); + } else if (AuthErrors.is(err, 'USER_CANCELED_LOGIN')) { + self.logEvent('login.canceled'); + // if user canceled login, just stop + return; + } + // re-throw error, it will be handled at a lower level. + throw err; + }); }, onSignInSuccess: function () { @@ -179,7 +181,7 @@ function (_, p, BaseView, FormView, SignInTemplate, Session, PasswordMixin, Auth * Render to a basic sign in view, used with "Use a different account" button */ useDifferentAccount: BaseView.preventDefaultThen(function () { - this.skipUserSuggestion = true; + Session.clear(); return this.render(); }), @@ -206,9 +208,7 @@ function (_, p, BaseView, FormView, SignInTemplate, Session, PasswordMixin, Auth // confirm that session token and email are present cachedSessionToken && cachedEmail && // prefilled email must be the same or absent - (this.prefillEmail === cachedEmail || ! this.prefillEmail) && - // must not be manually disabled - ! this.skipUserSuggestion + (this.prefillEmail === cachedEmail || ! this.prefillEmail) ) { return { email: cachedEmail, diff --git a/app/tests/spec/views/sign_in.js b/app/tests/spec/views/sign_in.js index a889084ca6..aa6889793c 100644 --- a/app/tests/spec/views/sign_in.js +++ b/app/tests/spec/views/sign_in.js @@ -286,7 +286,7 @@ function (chai, $, p, View, Session, AuthErrors, Metrics, FxaClient, }); describe('useLoggedInAccount', function () { - it('shows an error if session is expired', function (done) { + it('shows an error if session is expired', function () { Session.set('cachedCredentials', { sessionToken: 'abc123', email: 'a@a.com' @@ -300,12 +300,10 @@ function (chai, $, p, View, Session, AuthErrors, Metrics, FxaClient, // show password input assert.ok(view.$('#password').length); assert.equal(view.$('.error').text(), 'Session expired. Sign in to continue.'); - done(); - }) - .fail(done); + }); }); - it('signs in with a valid session', function (done) { + it('signs in with a valid session', function () { Session.set('cachedCredentials', { sessionToken: 'abc123', email: 'a@a.com' @@ -319,14 +317,12 @@ function (chai, $, p, View, Session, AuthErrors, Metrics, FxaClient, .then(function () { assert.notOk(view._isErrorVisible); assert.equal(view.$('.error').text(), ''); - done(); - }) - .fail(done); + }); }); }); describe('useDifferentAccount', function () { - it('can switch to signin with the useDifferentAccount button', function (done) { + it('can switch to signin with the useDifferentAccount button', function () { Session.set('cachedCredentials', { sessionToken: 'abc123', email: 'a@a.com' @@ -334,12 +330,15 @@ function (chai, $, p, View, Session, AuthErrors, Metrics, FxaClient, return view.useLoggedInAccount() .then(function () { - $('.use-different').click(); + assert.ok($('.use-different').length, 'has use different button'); + return view.useDifferentAccount(); + }) + .then(function () { assert.ok($('.email').length, 'should show email input'); assert.ok($('.password').length, 'should show password input'); - done(); - }) - .fail(done); + + assert.equal($('.email').val(), '', 'should have an empty email input'); + }); }); }); diff --git a/tests/functional/sign_in_cached.js b/tests/functional/sign_in_cached.js index 044f36990b..fb7b0f187d 100644 --- a/tests/functional/sign_in_cached.js +++ b/tests/functional/sign_in_cached.js @@ -218,8 +218,12 @@ define([ .click() .end() + // the form should not be prefilled .findByCssSelector('form input.email') - .clearValue() + .getAttribute('value') + .then(function (val) { + assert.equal(val, ''); + }) .click() .type(email2) .end() @@ -246,9 +250,13 @@ define([ .findByCssSelector('form input.email') .end() - .refresh() + .get(require.toUrl(PAGE_SIGNIN)) - .findByCssSelector('.use-different') + .findByCssSelector('form input.email') + .getAttribute('value') + .then(function (val) { + assert.equal(val, ''); + }) .end(); }, 'sign in with cached credentials but with an expired session': function () { @@ -480,15 +488,10 @@ define([ .end() .findByCssSelector('form input.email') - .end() - - .refresh() - - .findByCssSelector('.use-different') .end(); }, - 'sign in with desktop context then no context, desktop credentials should persist': function () { + 'sign in with desktop context then no context, desktop credentials should not persist': function () { var self = this; return this.get('remote') @@ -512,12 +515,12 @@ define([ .click() .end() + // This will clear the desktop credentials .findByCssSelector('.use-different') .click() .end() .findByCssSelector('form input.email') - .clearValue() .click() .type(email2) .end() @@ -546,7 +549,7 @@ define([ .getVisibleText() .then(function (text) { // confirm prefilled email - assert.ok(text.indexOf(email) > -1); + assert.ok(text.indexOf(email) === -1); }) .end()