Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make bcrypt rounds configurable #9044

Closed
wants to merge 14 commits into from
Closed
4 changes: 4 additions & 0 deletions History.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
## v.NEXT

* `Accounts.config` now supports a `bcryptRounds` option that
overrides the default 10 rounds currently used to secure passwords.
[PR #9044](https://github.com/meteor/meteor/pull/9044)

* Enables passing `false` to `cursor.count()` on the client to prevent skip
and limit from having an effect on the result.
[Issue #1201](https://github.com/meteor/meteor/issues/1201)
Expand Down
5 changes: 4 additions & 1 deletion packages/accounts-base/accounts_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ export class AccountsCommon {
// - ambiguousErrorMessages {Boolean}
// Return ambiguous error messages from login failures to prevent
// user enumeration.
// - bcryptRounds {Number}
// Allows override of number of bcrypt rounds (aka work factor) used
// to store passwords.

/**
* @summary Set global accounts options.
Expand Down Expand Up @@ -135,7 +138,7 @@ export class AccountsCommon {
// validate option keys
var VALID_KEYS = ["sendVerificationEmail", "forbidClientAccountCreation", "passwordEnrollTokenExpirationInDays",
"restrictCreationByEmailDomain", "loginExpirationInDays", "passwordResetTokenExpirationInDays",
"ambiguousErrorMessages"];
"ambiguousErrorMessages", "bcryptRounds"];
_.each(_.keys(options), function (key) {
if (!_.contains(VALID_KEYS, key)) {
throw new Error("Accounts.config: Invalid key: " + key);
Expand Down
2 changes: 1 addition & 1 deletion packages/accounts-base/package.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Package.describe({
summary: "A user account system",
version: "1.3.4"
version: "1.3.6"
});

Package.onUse(function (api) {
Expand Down
2 changes: 1 addition & 1 deletion packages/accounts-password/package.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Package.describe({
// release process, so versions 2.0.0-beta.2 through -beta.5 and -rc.0
// have already been published. The next time this package reaches 2.x
// territory, I would recommend jumping straight to 2.1.0.
version: "1.4.1"
version: "1.4.2"
});

Package.onUse(function(api) {
Expand Down
32 changes: 28 additions & 4 deletions packages/accounts-password/password_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ var bcryptCompare = Meteor.wrapAsync(bcrypt.compare);
// "sha-256" and then passes the digest to bcrypt.


Accounts._bcryptRounds = 10;
Accounts._bcryptRounds = Accounts._options.bcryptRounds || 10;

// Given a 'password' from the client, extract the string that we should
// bcrypt. 'password' can be one of:
Expand Down Expand Up @@ -52,6 +52,18 @@ var hashPassword = function (password) {
return bcryptHash(password, Accounts._bcryptRounds);
};

// Extract the number of rounds used in the specified bcrypt hash.
const getRoundsFromBcryptHash = hash => {
let rounds;
if (hash) {
const hashSegments = hash.split('$');
if (hashSegments.length > 2) {
rounds = parseInt(hashSegments[2], 10);
}
}
return rounds;
};

// 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
Expand All @@ -63,10 +75,22 @@ 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 (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(formattedPassword, Accounts._bcryptRounds)
}
});
});
}

return result;
Expand All @@ -78,7 +102,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
Expand Down
47 changes: 47 additions & 0 deletions packages/accounts-password/password_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use parseInt here to avoid interpreting leading 0s as octal.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done - and I've adjusted rounds extraction to use split string segments. Thanks!

}

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

}) ();