Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

Commit

Permalink
fix(client): Check the user's old password before attempting a change…
Browse files Browse the repository at this point in the history
… password.

* Add 'checkPassword' to fxa-client.js
* Check the user's pasword before checking whether the passwords are the same.

fixes #1118
  • Loading branch information
Shane Tomlinson committed Jun 23, 2014
1 parent 0aec5fe commit b78bfaa
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 27 deletions.
10 changes: 10 additions & 0 deletions app/scripts/lib/fxa-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,16 @@ function (FxaClient, $, p, Session, AuthErrors, Constants) {
return defer.promise;
},

/**
* Check the user's current password without affecting session state.
*/
checkPassword: function (email, password) {
return this._getClientAsync()
.then(function (client) {
return client.signIn(email, password);
});
},

signIn: function (originalEmail, password, options) {
var email = trim(originalEmail);
var self = this;
Expand Down
42 changes: 24 additions & 18 deletions app/scripts/views/change_password.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,27 +46,33 @@ function (_, BaseView, FormView, Template, Session, PasswordMixin, FloatingPlace
var oldPassword = this.$('#old_password').val();
var newPassword = this.$('#new_password').val();

if (oldPassword === newPassword) {
var err = AuthErrors.toError('PASSWORDS_MUST_BE_DIFFERENT');
return this.displayError(err);
}

this.hideError();

var self = this;
return this.fxaClient.changePassword(email, oldPassword, newPassword)
.then(function () {
self.navigate('settings', {
success: t('Password changed')
});
}, function (err) {
if (AuthErrors.is(err, 'UNVERIFIED_ACCOUNT')) {
err.forceMessage = t('Unverified account. <a href="#" id="resend">Resend verification email</a>.');
return self.displayErrorUnsafe(err);
}

throw err;
});
// Try to sign the user in before checking whether the
// passwords are the same. If the user typed the incorrect old
// password, they should know that first.
return this.fxaClient.checkPassword(email, oldPassword)
.then(function () {
if (oldPassword === newPassword) {
throw AuthErrors.toError('PASSWORDS_MUST_BE_DIFFERENT');
}

return self.fxaClient.changePassword(
email, oldPassword, newPassword);
})
.then(function () {
self.navigate('settings', {
success: t('Password changed')
});
}, function (err) {
if (AuthErrors.is(err, 'UNVERIFIED_ACCOUNT')) {
err.forceMessage = t('Unverified account. <a href="#" id="resend">Resend verification email</a>.');
return self.displayErrorUnsafe(err);
}

throw err;
});
},

resendVerificationEmail: BaseView.preventDefaultThen(function () {
Expand Down
24 changes: 24 additions & 0 deletions app/tests/spec/lib/fxa-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,29 @@ function (chai, $, ChannelMock, testHelpers,
});
});

describe('checkPassword', function () {
it('returns error if password is incorrect', function () {
email = trim(email);
return realClient.signUp(email, password, { preverified: true })
.then(function() {
return client.checkPassword(email, 'badpassword');
})
.then(function () {
assert.ok(false, 'unexpected success');
}, function(err) {
assert.isTrue(AuthErrors.is(err, 'INCORRECT_PASSWORD'));
});
});

it('succeeds if password is correct', function () {
email = trim(email);
return realClient.signUp(email, password, { preverified: true })
.then(function() {
return client.checkPassword(email, password);
});
});
});

describe('changePassword', function () {
it('changes the user\'s password', function () {
return client.signUp(email, password, {preVerified: true})
Expand Down Expand Up @@ -479,6 +502,7 @@ function (chai, $, ChannelMock, testHelpers,
});
});
});

});
});

30 changes: 21 additions & 9 deletions app/tests/spec/views/change_password.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ define([
'chai',
'underscore',
'jquery',
'lib/auth-errors',
'views/change_password',
'../../mocks/router',
'../../lib/helpers'
],
function (chai, _, $, View, RouterMock, TestHelpers) {
function (chai, _, $, AuthErrors, View, RouterMock, TestHelpers) {
var assert = chai.assert;
var wrapAssertion = TestHelpers.wrapAssertion;

Expand Down Expand Up @@ -115,17 +116,28 @@ function (chai, _, $, View, RouterMock, TestHelpers) {
});

describe('submit', function () {
it('prints an error message if both passwords are the same', function (done) {
it('prints incorrect password error message if old password is incorrect (event if passwords are the same)', function () {
$('#old_password').val('bad_password');
$('#new_password').val('bad_password');

return view.submit()
.then(function () {
assert.ok(false, 'unexpected success');
}, function (err) {
assert.isTrue(AuthErrors.is(err, 'INCORRECT_PASSWORD'));
});
});

it('prints passwords must be different message if both passwords are the same and the first password is correct', function () {
$('#old_password').val('password');
$('#new_password').val('password');

view.on('error', function (msg) {
wrapAssertion(function () {
assert.ok(msg);
}, done);
});

view.submit();
return view.submit()
.then(function () {
assert.ok(false, 'unexpected success');
}, function (err) {
assert.ok(AuthErrors.is(err, 'PASSWORDS_MUST_BE_DIFFERENT'));
});
});

it('changes from old to new password, redirects user to /settings', function () {
Expand Down

0 comments on commit b78bfaa

Please sign in to comment.