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

feat: add options for custom password hashing/verifying functions #374

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ User.plugin(passportLocalMongoose, options);
```
* `usernameQueryFields`: specifies alternative fields of the model for identifying a user (e.g. email).
* `findByUsername`: Specifies a query function that is executed with query parameters to restrict the query with extra query parameters. For example query only users with field "active" set to `true`. Default: `function(model, queryParameters) { return model.findOne(queryParameters); }`. See the examples section for a use case.
* `generatePasswordHashAsync`: specifies a custom function for generating the password hash. Default: see `lib/pbkdfs.js`.
* `verifyPasswordHashAsync`: specifies a custom function for verifying the password hash. Default: see `lib/pbkdfs.js`.

**_Attention!_** Changing any of the hashing options (saltlen, iterations or keylen) in a production environment will prevent existing users from authenticating!

Expand Down Expand Up @@ -294,4 +296,4 @@ The default digest algorithm was changed due to security implications from **sha

## License

Passport-Local Mongoose is licensed under the [MIT license](http://opensource.org/licenses/MIT).
Passport-Local Mongoose is licensed under the [MIT license](http://opensource.org/licenses/MIT).
5 changes: 4 additions & 1 deletion index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ declare module 'mongoose' {
encoding?: string | undefined;
digestAlgorithm?: string | undefined;
passwordValidator?: ((password: string, cb: (err: any) => void) => void) | undefined;
verifyPasswordHashAsync: (password: string) => Promise<{ hash: string, salt: string }>;
generatePasswordHashAsync: (password: string, user: Document, options: PassportLocalOptions) => Promise<boolean>;


usernameField?: string | undefined;
usernameUnique?: boolean | undefined;
Expand Down Expand Up @@ -110,4 +113,4 @@ declare module 'passport-local-mongoose' {
import mongoose = require('mongoose');
var _: (schema: mongoose.Schema<any, any, any, any>, options?: Object) => void;
export = _;
}
}
32 changes: 13 additions & 19 deletions index.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
const crypto = require('crypto');
const LocalStrategy = require('passport-local').Strategy;

const pbkdf2 = require('./lib/pbkdf2');
const errors = require('./lib/errors');
const authenticate = require('./lib/authenticate');

const { defaultPasswordHashGeneratorAsync, defaultPasswordHashVerifierAsync } = require('./lib/pbkdf2');

module.exports = function (schema, options) {
options = options || {};
options.saltlen = options.saltlen || 32;
Expand All @@ -23,6 +23,9 @@ module.exports = function (schema, options) {
});
}

options.verifyPasswordHashAsync = options.verifyPasswordHashAsync || defaultPasswordHashVerifierAsync;
options.generatePasswordHashAsync = options.generatePasswordHashAsync || defaultPasswordHashGeneratorAsync;

options.passwordValidator = options.passwordValidator || defaultPasswordValidator;
options.passwordValidatorAsync = options.passwordValidatorAsync || defaultPasswordValidatorAsync;

Expand Down Expand Up @@ -104,16 +107,14 @@ module.exports = function (schema, options) {
}
})
.then(() => options.passwordValidatorAsync(password))
.then(() => randomBytes(options.saltlen))
.then((saltBuffer) => saltBuffer.toString(options.encoding))
.then((salt) => {
this.set(options.saltField, salt);
.then(() => options.generatePasswordHashAsync(password, options))
.then(({ hash, salt }) => {
if (!hash) {
throw new Error('Failed to generate valid password hash');
}

return salt;
})
.then((salt) => pbkdf2Promisified(password, salt, options))
.then((hashRaw) => {
this.set(options.hashField, Buffer.from(hashRaw, 'binary').toString(options.encoding));
this.set(options.saltField, salt);
this.set(options.hashField, hash);
})
.then(() => this);

Expand Down Expand Up @@ -299,6 +300,7 @@ module.exports = function (schema, options) {
.exec()
.then((user) => cb(null, user))
.catch(cb);

return;
}

Expand All @@ -310,12 +312,4 @@ module.exports = function (schema, options) {
};
};

function pbkdf2Promisified(password, salt, options) {
return new Promise((resolve, reject) => pbkdf2(password, salt, options, (err, hashRaw) => (err ? reject(err) : resolve(hashRaw))));
}

function randomBytes(saltlen) {
return new Promise((resolve, reject) => crypto.randomBytes(saltlen, (err, saltBuffer) => (err ? reject(err) : resolve(saltBuffer))));
}

module.exports.errors = errors;
12 changes: 2 additions & 10 deletions lib/authenticate.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
const scmp = require('scmp');

const pbkdf2 = require('./pbkdf2');
const errors = require('./errors');

// authenticate function needs refactoring - to avoid bugs we wrapped a bit dirty
Expand Down Expand Up @@ -38,7 +35,6 @@ function authenticate(user, password, options, cb) {
if (options.unlockInterval && Date.now() - user.get(options.lastLoginField) > options.unlockInterval) {
user.set(options.lastLoginField, Date.now());
user.set(options.attemptsField, 0);

promise = user.save();
} else {
return cb(null, false, new errors.TooManyAttemptsError(options.errorMessages.TooManyAttemptsError));
Expand All @@ -51,12 +47,8 @@ function authenticate(user, password, options, cb) {
return cb(null, false, new errors.NoSaltValueStoredError(options.errorMessages.NoSaltValueStoredError));
}

pbkdf2(password, user.get(options.saltField), options, function (err, hashBuffer) {
if (err) {
return cb(err);
}

if (scmp(hashBuffer, Buffer.from(user.get(options.hashField), options.encoding))) {
options.verifyPasswordHashAsync(password, user, options).then((isCorrect) => {
if (isCorrect === true) {
if (options.limitAttempts) {
user.set(options.lastLoginField, Date.now());
user.set(options.attemptsField, 0);
Expand Down
36 changes: 34 additions & 2 deletions lib/pbkdf2.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,37 @@
const crypto = require('crypto');

module.exports = function pbkdf2(password, salt, options, callback) {
crypto.pbkdf2(password, salt, options.iterations, options.keylen, options.digestAlgorithm, callback);
const defaultPasswordHashGeneratorAsync = async (password, options) => {
const saltBuffer = await new Promise((resolve, reject) =>
crypto.randomBytes(options.saltlen, (err, saltBuffer) => (err ? reject(err) : resolve(saltBuffer)))
);

const salt = saltBuffer.toString(options.encoding);

const hashBuffer = await new Promise((resolve, reject) =>
crypto.pbkdf2(password, salt, options.iterations, options.keylen, options.digestAlgorithm, (err, hashBuffer) =>
err ? reject(err) : resolve(hashBuffer)
)
);

const hash = Buffer.from(hashBuffer, 'binary').toString(options.encoding);

return { hash, salt };
};

const defaultPasswordHashVerifierAsync = async (password, user, options) => {
const userHash = user.get(options.hashField);
const userSalt = user.get(options.saltField);

const hashBuffer = await new Promise((resolve, reject) =>
crypto.pbkdf2(password, userSalt, options.iterations, options.keylen, options.digestAlgorithm, (err, hashBuffer) =>
err ? reject(err) : resolve(hashBuffer)
)
);

return crypto.timingSafeEqual(Buffer.from(userHash, options.encoding), hashBuffer);
};

module.exports = {
defaultPasswordHashGeneratorAsync,
defaultPasswordHashVerifierAsync,
};
13 changes: 1 addition & 12 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@
},
"dependencies": {
"generaterr": "^1.5.0",
"passport-local": "^1.0.0",
"scmp": "^2.1.0"
"passport-local": "^1.0.0"
},
"devDependencies": {
"@types/passport": "1.0.7",
Expand Down Expand Up @@ -50,4 +49,4 @@
"tsd": {
"directory": "test/types"
}
}
}
69 changes: 69 additions & 0 deletions test/passport-local-mongoose.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const Schema = mongoose.Schema;
const expect = require('chai').expect;
const dropMongodbCollections = require('drop-mongodb-collections');
const debug = require('debug')('passport:local:mongoose');
const crypto = require('crypto');

const errors = require('../lib/errors.js');
const passportLocalMongoose = require('../');
Expand Down Expand Up @@ -1845,6 +1846,74 @@ describe('passportLocalMongoose', function () {
});
});

describe('Password Hashing', function () {
beforeEach(async () => await dropMongodbCollections(connectionString));

beforeEach(() =>
mongoose.connect(connectionString, { bufferCommands: false, autoIndex: false, useNewUrlParser: true, useUnifiedTopology: true })
);

afterEach(() => mongoose.disconnect());

it('should authenticate with custom hash function', async function () {
const UserSchema = new Schema({});
UserSchema.plugin(passportLocalMongoose, {
verifyPasswordHashAsync: async (password, user, options) => {
const hash = crypto
.createHash('sha256')
.update(password + user.get(options.saltField))
.digest('base64');

return user.get(options.hashField) === hash;
},
generatePasswordHashAsync: async (password) => {
const saltBuffer = await new Promise((resolve, reject) =>
crypto.randomBytes(200, (err, saltBuffer) => (err ? reject(err) : resolve(saltBuffer)))
);

const salt = saltBuffer.toString('utf-8');

// don't use this for password hashing, it's insecure and only used for testing
const hash = crypto
.createHash('sha256')
.update(password + salt)
.digest('base64');

return { hash, salt };
},
});

const User = mongoose.model('CustomHashRegisterAndNotAuthenticateUser', UserSchema);

await User.register({ username: 'hugo' }, 'secret');
const { user, error } = await User.authenticate()('hugo', 'wrong_password');
expect(user).to.equal(false);
expect(error).to.exist;

const { user: user2, error: error2 } = await User.authenticate()('hugo', 'secret');
expect(user2).to.not.equal(false);
expect(error2).to.not.exist;
});

it('should not authenticate if custom hash function throws error', async function () {
const UserSchema = new Schema({});
UserSchema.plugin(passportLocalMongoose, {
verifyPasswordHashAsync: async () => {
throw new Error('');
},
generatePasswordHashAsync: async (password) => ({
hash: password === 'secret' ? 'hash' : 'someotherhash',
salt: 'mysalt',
}),
});

const User = mongoose.model('CustomInvalidHashRegisterAndNotAuthenticateUser', UserSchema);

await User.register({ username: 'hugo' }, 'secret');
expect(async () => await User.authenticate()('hugo', 'secret')).to.throw;
});
});

function setPasswordAndAuthenticate(user, passwordToSet, passwordToAuthenticate, cb) {
user.setPassword(passwordToSet, function (err) {
if (err) {
Expand Down