Merge branch 'bcrypt-change-password' into release-0.8.2

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,29 @@ 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.
+          srpUpgradePath({
+            upgradeError: error,
+            userSelector: { id: Meteor.userId() },
+            plaintextPassword: oldPassword
+          }, function (err) {
+            if (err) {
+              callback(err);
+            } else {
+              // Now that we've successfully migrated from srp to
+              // bcrypt, try changing the password again.
+              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();
       }

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;

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
+  ]);
 }) ();
 
 

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");
   }
 });