From 137129b35804ff7011fa271b374436282a24ab53 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Thu, 19 Jun 2014 14:45:49 -0700 Subject: [PATCH] Upgrade from SRP to bcrypt on password change --- packages/accounts-password/password_client.js | 86 ++++++++++++++----- packages/accounts-password/password_server.js | 25 +++++- packages/accounts-password/password_tests.js | 45 ++++++++++ .../accounts-password/password_tests_setup.js | 8 ++ 4 files changed, 139 insertions(+), 25 deletions(-) diff --git a/packages/accounts-password/password_client.js b/packages/accounts-password/password_client.js index 2a3a6e088a..2cd4731f61 100644 --- a/packages/accounts-password/password_client.js +++ b/packages/accounts-password/password_client.js @@ -34,16 +34,11 @@ Meteor.loginWithPassword = function (selector, password, callback) { // the password without requiring a full SRP flow, as well as // SHA256(password), which the server bcrypts and stores in // place of the old SRP information for this user. - var details; - try { - details = EJSON.parse(error.details); - } catch (e) {} - if (!(details && details.format === 'srp')) - callback(new Meteor.Error(400, - "Password is old. Please reset your " + - "password.")); - else - srpUpgradePath(selector, password, details.identity, callback); + srpUpgradePath({ + upgradeError: error, + userSelector: selector, + plaintextPassword: password + }, callback); } else if (error) { callback(error); @@ -61,18 +56,32 @@ var hashPassword = function (password) { }; }; +// XXX COMPAT WITH 0.8.1.3 // The server requested an upgrade from the old SRP password format, -// so supply the needed SRP identity to login. -var srpUpgradePath = function (selector, plaintextPassword, - identity, callback) { - Accounts.callLoginMethod({ - methodArguments: [{ - user: selector, - srp: SHA256(identity + ":" + plaintextPassword), - password: hashPassword(plaintextPassword) - }], - userCallback: callback - }); +// so supply the needed SRP identity to login. Options: +// - upgradeError: the error object that the server returned to tell +// us to upgrade from SRP to bcrypt. +// - userSelector: selector to retrieve the user object +// - plaintextPassword: the password as a string +var srpUpgradePath = function (options, callback) { + var details; + try { + details = EJSON.parse(options.upgradeError.details); + } catch (e) {} + if (!(details && details.format === 'srp')) { + callback(new Meteor.Error(400, + "Password is old. Please reset your " + + "password.")); + } else { + Accounts.callLoginMethod({ + methodArguments: [{ + user: options.userSelector, + srp: SHA256(details.identity + ":" + options.plaintextPassword), + password: hashPassword(options.plaintextPassword) + }], + userCallback: callback + }); + } }; @@ -113,8 +122,39 @@ Accounts.changePassword = function (oldPassword, newPassword, callback) { [oldPassword ? hashPassword(oldPassword) : null, hashPassword(newPassword)], function (error, result) { if (error || !result) { - callback && callback( - error || new Error("No result from changePassword.")); + if (error && error.error === 400 && + error.reason === 'old password format') { + // XXX COMPAT WITH 0.8.1.3 + // The server is telling us to upgrade from SRP to bcrypt, as + // in Meteor.loginWithPassword. + var userSelector = {}; + if (Meteor.user().username) { + userSelector = { username: Meteor.user().username }; + } else if (Meteor.user().emails && Meteor.user().emails.length) { + userSelector = { email: Meteor.user().emails[0].address }; + } else { + callback(new Error( + "Cannot upgrade password format without " + + "username or email address")); + return; + } + + srpUpgradePath({ + upgradeError: error, + userSelector: userSelector, + plaintextPassword: oldPassword + }, function (err) { + if (err) { + callback(err); + } else { + Accounts.changePassword(oldPassword, newPassword, callback); + } + }); + } else { + // A normal error, not an error telling us to upgrade to bcrypt + callback && callback( + error || new Error("No result from changePassword.")); + } } else { callback && callback(); } diff --git a/packages/accounts-password/password_server.js b/packages/accounts-password/password_server.js index b054bb1f76..39d84518c0 100644 --- a/packages/accounts-password/password_server.js +++ b/packages/accounts-password/password_server.js @@ -254,7 +254,20 @@ Accounts.registerLoginHandler("password", function (options) { /// // Let the user change their own password if they know the old -// password. +// password. `oldPassword` and `newPassword` should be objects with keys +// `digest` and `algorithm` (representing the SHA256 of the password). +// +// XXX COMPAT WITH 0.8.1.3 +// Like the login method, if the user hasn't been upgraded from SRP to +// bcrypt yet, then this method will throw an 'old password format' +// error. The client should call the SRP upgrade login handler and then +// retry this method again. +// +// UNLIKE the login method, there is no way to avoid getting SRP upgrade +// errors thrown. The reasoning for this is that clients using this +// method directly will need to be updated anyway because we no longer +// support the SRP flow that they would have been doing to use this +// method previously. Meteor.methods({changePassword: function (oldPassword, newPassword) { check(oldPassword, passwordValidator); check(newPassword, passwordValidator); @@ -266,9 +279,17 @@ Meteor.methods({changePassword: function (oldPassword, newPassword) { if (!user) throw new Meteor.Error(403, "User not found"); - if (!user.services || !user.services.password || !user.services.password.bcrypt) + if (!user.services || !user.services.password || + (!user.services.password.bcrypt && !user.services.password.srp)) throw new Meteor.Error(403, "User has no password set"); + if (! user.services.password.bcrypt) { + throw new Meteor.Error(400, "old password format", EJSON.stringify({ + format: 'srp', + identity: user.services.password.srp.identity + })); + } + var result = checkPassword(user, oldPassword); if (result.error) throw result.error; diff --git a/packages/accounts-password/password_tests.js b/packages/accounts-password/password_tests.js index d79654e23f..e1e51d4aac 100644 --- a/packages/accounts-password/password_tests.js +++ b/packages/accounts-password/password_tests.js @@ -767,6 +767,51 @@ if (Meteor.isClient) (function () { })); } ]); + + testAsyncMulti("passwords - srp to bcrypt upgrade via password change", [ + logoutStep, + // Create user with old SRP credentials in the database. + function (test, expect) { + var self = this; + Meteor.call("testCreateSRPUser", expect(function (error, result) { + test.isFalse(error); + self.username = result; + })); + }, + // Log in with the plaintext password handler, which should NOT upgrade us to bcrypt. + function (test, expect) { + Accounts.callLoginMethod({ + methodName: "login", + methodArguments: [ { user: { username: this.username }, password: "abcdef" } ], + userCallback: expect(function (err) { + test.isFalse(err); + }) + }); + }, + function (test, expect) { + Meteor.call("testNoSRPUpgrade", this.username, expect(function (error) { + test.isFalse(error); + })); + }, + // Changing our password should upgrade us to bcrypt. + function (test, expect) { + Accounts.changePassword("abcdef", "abcdefg", expect(function (error) { + test.isFalse(error); + })); + }, + function (test, expect) { + Meteor.call("testSRPUpgrade", this.username, expect(function (error) { + test.isFalse(error); + })); + }, + // And after the upgrade we should be able to change our password again. + function (test, expect) { + Accounts.changePassword("abcdefg", "abcdef", expect(function (error) { + test.isFalse(error); + })); + }, + logoutStep + ]); }) (); diff --git a/packages/accounts-password/password_tests_setup.js b/packages/accounts-password/password_tests_setup.js index 0993a706b6..fa4432f097 100644 --- a/packages/accounts-password/password_tests_setup.js +++ b/packages/accounts-password/password_tests_setup.js @@ -141,5 +141,13 @@ Meteor.methods({ throw new Error("srp wasn't removed"); if (!(user.services && user.services.password && user.services.password.bcrypt)) throw new Error("bcrypt wasn't added"); + }, + + testNoSRPUpgrade: function (username) { + var user = Meteor.users.findOne({username: username}); + if (user.services && user.services.password && user.services.password.bcrypt) + throw new Error("bcrypt was added"); + if (user.services && user.services.password && ! user.services.password.srp) + throw new Error("srp was removed"); } });