From 053bd79a49204b7e170cb19903527c47be6efe4f Mon Sep 17 00:00:00 2001
From: Hugh Willson <hugh@octonary.com>
Date: Tue, 17 Oct 2017 07:47:55 -0400
Subject: [PATCH] Code cleanup; add tests

---
 packages/accounts-password/password_server.js | 26 +++++-----
 packages/accounts-password/password_tests.js  | 47 +++++++++++++++++++
 2 files changed, 61 insertions(+), 12 deletions(-)

diff --git a/packages/accounts-password/password_server.js b/packages/accounts-password/password_server.js
index f0282b515f..beb8abcdf0 100644
--- a/packages/accounts-password/password_server.js
+++ b/packages/accounts-password/password_server.js
@@ -52,6 +52,10 @@ var hashPassword = function (password) {
   return bcryptHash(password, Accounts._bcryptRounds);
 };
 
+// Extract the number of rounds used in the specified bcrypt hash.
+const getRoundsFromBcryptHash =
+  hash => hash ? Number(hash.substring(4, 6)) : null;
+
 // Check whether the provided password matches the bcrypt'ed password in
 // the database user record. `password` can be a string (in which case
 // it will be run through SHA256 before bcrypt) or an object with
@@ -63,21 +67,19 @@ Accounts._checkPassword = function (user, password) {
     userId: user._id
   };
 
-  password = getPasswordString(password);
+  const formattedPassword = getPasswordString(password);
+  const hash = user.services.password.bcrypt;
+  const hashRounds = getRoundsFromBcryptHash(hash);
 
-  if (! bcryptCompare(password, user.services.password.bcrypt)) {
+  if (! bcryptCompare(formattedPassword, hash)) {
     result.error = handleError("Incorrect password", false);
-  } else if (
-    user.services.password.bcrypt && 
-    Accounts._bcryptRounds > 
-      Number(user.services.password.bcrypt.substring(4, 6))
-  ) {
-    // password checks out, but user bcrypt may need update
+  } else if (hash && Accounts._bcryptRounds != hashRounds) {
+    // The password checks out, but the user's bcrypt hash needs to be updated.
     Meteor.defer(() => {
       Meteor.users.update({ _id: user._id }, {
-        $set: { 
-          'services.password.bcrypt': 
-            bcryptHash(password, Accounts._bcryptRounds)
+        $set: {
+          'services.password.bcrypt':
+            bcryptHash(formattedPassword, Accounts._bcryptRounds)
         }
       });
     });
@@ -92,7 +94,7 @@ var checkPassword = Accounts._checkPassword;
 ///
 const handleError = (msg, throwError = true) => {
   const error = new Meteor.Error(
-    403, 
+    403,
     Accounts._options.ambiguousErrorMessages
       ? "Something went wrong. Please check your credentials."
       : msg
diff --git a/packages/accounts-password/password_tests.js b/packages/accounts-password/password_tests.js
index a34b3b9a7d..cb1115298e 100644
--- a/packages/accounts-password/password_tests.js
+++ b/packages/accounts-password/password_tests.js
@@ -1886,4 +1886,51 @@ if (Meteor.isServer) (function () {
     ]);
   });
 
+  Tinytest.addAsync(
+    'passwords - allow custom bcrypt rounds',
+    function (test, done) {
+      function getUserHashRounds(user) {
+        return Number(user.services.password.bcrypt.substring(4, 6));
+      }
+
+      // Verify that a bcrypt hash generated for a new account uses the
+      // default number of rounds.
+      let username = Random.id();
+      const password = 'abc123';
+      const userId1 = Accounts.createUser({ username, password });
+      let user1 = Meteor.users.findOne(userId1);
+      let rounds = getUserHashRounds(user1);
+      test.equal(rounds, Accounts._bcryptRounds);
+
+      // When a custom number of bcrypt rounds is set via Accounts.config,
+      // and an account was already created using the default number of rounds,
+      // make sure that a new hash is created (and stored) using the new number
+      // of rounds, the next time the password is checked.
+      const defaultRounds = Accounts._bcryptRounds;
+      const customRounds = 11;
+      Accounts._bcryptRounds = customRounds;
+      Accounts._checkPassword(user1, password);
+      Meteor.setTimeout(() => {
+        user1 = Meteor.users.findOne(userId1);
+        rounds = getUserHashRounds(user1);
+        test.equal(rounds, customRounds);
+        Accounts._options.bcryptRounds = null;
+
+        // When a custom number of bcrypt rounds is set, make sure it's
+        // used for new bcrypt password hashes.
+        username = Random.id();
+        const userId2 = Accounts.createUser({ username, password });
+        const user2 = Meteor.users.findOne(userId2);
+        rounds = getUserHashRounds(user2);
+        test.equal(rounds, customRounds);
+
+        // Cleanup
+        Accounts._bcryptRounds = defaultRounds;
+        Meteor.users.remove(userId1);
+        Meteor.users.remove(userId2);
+        done();
+      }, 5000);
+    }
+  );
+
 }) ();