0

I'm using passport-local and I've already hashed my passwords in my mongo database. I can't figure out why my bcrypt.compare() isn't working correctly. It says 'cb is not a function' but it is. I just saved the callback as a variable in the same file instead of saving it in my User schema. Anyone ran across this problem before or see any errors in my code??

passport.use(new LocalStrategy((username, password, done) => {
User.findOne({ username: username }).exec().then((user, err) => {
if (err) return done(err)
if (!user) return done(null, false, { message: 'Incorrect username.' })

const comparePassword = function (candidatePassword, hashedPassword, cb) {
  bcrypt.compare(candidatePassword, hashedPassword, function (err, isMatch) {
    if (err) return cb(err)
    return cb(null, isMatch)
  })
}
 comparePassword(password, user.password, function (err, isMatch) {
   if (err) return done(err)
   if (!isMatch) return done(null, false, { message: 'Incorrect password.' })
   return done(null, user)
 })
})
}))
3
  • 1
    Why are you using that separate comparePassword function at all (instead of calling bcrypt.compare directly)?
    – Bergi
    Jun 15, 2016 at 1:23
  • 2
    This is unrelated, but for security purposes you generally shouldn't differentiate between bad username and bad password. Just return a same generic message, that way a malicious user won't know if they found a valid username or not. That's my 2 cents anyway.
    – mscdex
    Jun 15, 2016 at 2:13
  • I can't get it to work with just bcrypt.compare either :/ Jun 15, 2016 at 3:43

4 Answers 4

1

So after hours of struggling with bcrypt.compare, I just decided to clear out my database and create new users from scratch. I ended up using bcrypt.compareSync() and it finally verified my passwords! So make sure you have all of your methods defined on your model BEFORE you add users/whatever to your database. I thought I had already added the compare function but I guess I was wrong. Thanks for all of your help!

1

I also had the same error, just reinstall npm and bcrypt, be careful! see what version of node you are working that is also important. Regards

0

I encountered something like this before. It has to do with async handling and the fact that all your code is already in the .then() handler of a promise (user.findOne()). It's generally not a good idea to mix promises and the callback pattern.

What I ended up doing is wrapping my comparePassword function in a promise and then including it in the promise chain. My code was quite different, but in your case it would look something like this:

passport.use(new LocalStrategy((username, password, done) => {

function comparePassword(candidatePassword, hashedPassword) {
return new Promise((resolve, reject) => {
  bcrypt.compare(candidatePassword, hashedPassword,
    (err, isMatch) => {
      if (err) return reject(err);
      else if (!isMatch) reject(new Error('Incorrect Password'));
      else resolve();
    });
  });
})

User.findOne({ username: username }).exec()
.then((user, err) => {
if (err) return done(err)
if (!user) return done(null, false, { message: 'Incorrect username.' })
return comparePassword(password, user.password)
})
.then(/*do stuff if password is a match*/)
.catch(/*do stuff iff not a match*/)
}))

You could also try to just

return comparePassword(password, user.password, function (err, isMatch) {

at the end of your .then(), and I'd declare comparePassword outside of the .then() like in my code above.

-1

The code looks alright to me. Eventhough I didn't test it.

Does your password comparison has to be aSync? Have you considered using bcrypt.compareSync?

Example:

MyPasswordTool.prototype.validateHashedPassword = function(challlenger, hashedPassword) {
    return bcrypt.compareSync(challlenger, hashedPassword);
};
1

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.