Bcrypt-NodeJS compare() returns false whatever the password

Question

I know that question has already been asked a few times (like here, here or there, or even on Github, but none of the answers actually worked for me...

I am trying to develop authentication for a NodeJS app using Mongoose and Passport, and using Bcrypt-NodeJS to hash the users' passwords.

Everything was working without any problem before I decided to refactor the User schema and to use the async methods of bcrypt. The hashing still works while creating a new user but I am now unable to verify a password against its hash stored in MongoDB.

What do I know?

1. bcrypt.compare() always returns false whatever the password is correct or not, and whatever the password (I tried several strings).
2. The password is only hashed once (so the hash is not re-hashed) on user's creation.
3. The password and the hash given to the compare method are the right ones, in the right order.
4. The password and the hash are of type "String".
5. The hash isn't truncated when stored in the database (60 characters long string).
6. The hash fetched in the database is the same as the one stored on user's creation.

Code

User schema

Some fields have been stripped to keep it clear, but I kept the relevant parts.

var userSchema = mongoose.Schema({

    // Local authentication
    password: {
        hash: {
            type: String,
            select: false
        },
        modified: {
            type: Date,
            default: Date.now
        }
    },

    // User data
    profile: {
        email: {
            type: String,
            required: true,
            unique: true
        }
    },

    // Dates
    lastSignedIn: {
        type: Date,
        default: Date.now
    }
});

Password hashing

userSchema.statics.hashPassword = function(password, callback) {
    bcrypt.hash(password, bcrypt.genSaltSync(12), null, function(err, hash) {
        if (err) return callback(err);
        callback(null, hash);
    });
}

Password comparison

userSchema.methods.comparePassword = function(password, callback) {
    // Here, `password` is the string entered in the login form
    // and `this.password.hash` is the hash stored in the database
    // No problem so far
    bcrypt.compare(password, this.password.hash, function(err, match) {
        // Here, `err == null` and `match == false` whatever the password
        if (err) return callback(err);
        callback(null, match);
    });
}

User authentication

userSchema.statics.authenticate = function(email, password, callback) {
    this.findOne({ 'profile.email': email })
        .select('+password.hash')
        .exec(function(err, user) {
            if (err) return callback(err);
            if (!user) return callback(null, false);

            user.comparePassword(password, function(err, match) {
                // Here, `err == null` and `match == false`
                if (err) return callback(err);
                if (!match) return callback(null, false);

                // Update the user
                user.lastSignedIn = Date.now();
                user.save(function(err) {
                    if (err) return callback(err);
                    user.password.hash = undefined;
                    callback(null, user);
                });
            });
        });
}

It may be a "simple" mistake I made but I wasn't able to find anything wrong in a few hours... May you have any idea to make that method work, I would be glad to read it.

Thank you guys.

Edit:

When running this bit of code, match is actually equal to true. So I know my methods are correct. I suspect this has something to do with the storage of the hash in the database, but I really have no idea of what can cause this error to occur.

var pwd = 'TestingPwd01!';
mongoose.model('User').hashPassword(pwd, function(err, hash) {
    console.log('Password: ' + pwd);
    console.log('Hash: ' + hash);
    user.password.hash = hash;
    user.comparePassword(pwd, function(err, match) {
        console.log('Match: ' + match);
    });
});

Edit 2 (and solution) :

I put it there in case it could be helpful to someone one day...

I found the error in my code, which was occurring during the user's registration (and actually the only piece of code I didn't post here). I was hashing the user.password object instead of user.password.plaintext...

It's only by changing my dependencies from "brcypt-nodejs" to "bcryptjs" that I was able to find the error because bcryptjs throws an error when asked to hash an object, while brcypt-nodejs just hashes the object as if it were a string.

Navarrette

You shouldn't mark best answer if it doesn't answer your question – Cowbell

@Cowbell I found the solution because of the comments on that answer, doesn't that make it the best answer? No? Okay, I'll remember that. – Navarrette

Answer 1

bcrypt.hash() has 3 arguments... you have 4 for some reason.

Instead of

bcrypt.hash(password, bcrypt.genSaltSync(12), null, function(err, hash) {

it should be

bcrypt.hash(password, bcrypt.genSaltSync(12), function(err, hash) {

Since you were hashing only during user creation, then you might not have been hashing properly. You may need to re-create the users.

Chartreuse

I'm using the bcrypt-nodejs module (I'm working on a Windows machine and don't really want to install several gigs of dependencies...) which hash() method has 4 arguments, the third one being a callback used to signify progress during the hash. Your suggestion makes my app throw an error since no callback was given. Thanks anyway, I'll probably switch to the "real" bcrypt package (the one you mentioned here) or even better, use soething else than Windows to develop my app. – Navarrette

@AntoineVanServeyt My bad. Too many bcrypt modules out there e.g. bcrypt, bcrypt-nodejs, bcryptjs. If you are looking for an alternative, I'd probably recommend the last one though as that usually works on all platforms. – Chartreuse

@Mickey No problem. I'll take a look at bcryptjs then, thanks for the suggestion. – Navarrette

Answer 2

I know the solution has been found but just in case you are landing here out of google search and have the same issue, especially if you are using a schema.pre("save") function, sometimes there is a tendency of saving the same model several times, hence re-hashing the password each time. This is especially true if you are using references in mongoDB to create schema relationship. Here is what my registration function looked like:

Signup Code

User.create(newUser, (err, user) => {
            if (err || !user) {
                console.warn("Error at stage 1");
                return res.json(transformedApiRes(err, "Signup error", false)).status(400);
            }
            let personData: PersonInterface = <PersonInterface>{};
            personData.firstName = req.body.first_name;
            personData.lastName = req.body.last_name;
            personData.user = user._id;
            Person.create(personData, function (err1: Error, person: any): any {
                if (err1 || !person) {
                    return res.json(transformedApiRes(err1, "Error while saving to Persons", false));
                }
                /* One-to-One relationship */
                user.person = person;
                user.save(function (err, user) {
                    if (err || !user) {
                        return res.json({error: err}, "Error while linking user and person models", false);
                    }
                    emitter.emit("userRegistered", user);
                    return res.json(transformedApiRes(user, `Signup Successful`, true));
                });
            });
        });

As you can see there is a nested save on User because I had to link the User model with Person model (one-to-one). As a result, I had the mismatch error because I was using a pre-save function and every time I triggered User.create or User.save, the function would be called and it would re-hash the existing password. A console statement inside pre-save gave me the following, showing that indeed that password was re-hashed:

Console debug after a single signup call

{ plain: 'passwd',
  hash: '$2b$10$S2g9jIcmjGxE0aT1ASd6lujHqT87kijqXTss1XtUHJCIkAlk0Vi0S' }
{ plain: '$2b$10$S2g9jIcmjGxE0aT1ASd6lujHqT87kijqXTss1XtUHJCIkAlk0Vi0S',
  hash: '$2b$10$KRkVY3M8a8KX9FcZRX.l8.oTSupI/Fg0xij9lezgOxN8Lld7RCHXm' }

The Fix, The Solution

To fix this, you have to modify your pre("save") code to ensure the password is only hashed if it is the first time it is being saved to the db or if it has been modified. To do this, surround your pre-save code in these blocks:

if (user.isModified("password") || user.isNew) {
    //Perform password hashing here
} else {
    return next();
}

Here is how the whole of my pre-save function looks like

UsersSchema.pre("save", function (next: NextFunction): any {
    let user: any = this;
    if (user.isModified("password") || user.isNew) {
        bcrypt.genSalt(10, function (err: Error, salt: string): any {
            if (err) {
                return next(err);
            }
            bcrypt.hash(user.password, salt, function (err: Error, hash: string) {
                if (err) {
                    console.log(err);
                    return next(err);
                }
                console.warn({plain: user.password, hash: hash});
                user.password = hash;
                next();
            });
        });
    } else {
        return next();
    }
});

Hopefully this helps someone.

Cecilia

Answer 3

I am dropping this here because it might help someone someday.

In my own case, the reason why I was having bcrypt.compare as false even when I supplied the right authentication details was because of the constraints on the datatype in the model. So each time the hash was saved in the DB, it was truncated in order to fit into the 50 characters constraints.

I had

    'password': {
      type: DataTypes.STRING(50),
      allowNull: false,
      comment: "null"
    },

The string could only contain 50 characters but the result of bcrypt.hash was more than that.

FIX

I modified the model thus DataTypes.STRING(255)

Thrips

Answer 4

None of the above answers solved my issue. bycrpt.compare() would always return false for me also. I finally discovered the solution. I had a value of lowercase: true,1 in the password field of my user schema. I removed this value and problem solved!

Syllogistic

Answer 5

Tip: If you are switching

then().then()

Block always check return value.

Dennison

Answer 6

You can always check the max length for the password field in the database. Make sure it is large. In my case, I have set it to 500. And then the code worked flawlessly!

Thirtytwo

Answer 7

TS version

const { phone, password } = loginDto;
            const user = await this.usersService.findUserByPhone(phone);
            const match = await compare(password, user.password);
            if (user && match){
                return user
            }else{
                throw new UnauthorizedException();
            } 

JS version

const { phone, password } = loginDto;
                const user = await this.usersService.findUserByPhone(phone);
                const match = await bcrypt.compare(password, user.password);
                if (user && match){
                    return user
                }else{
                    throw new UnauthorizedException();
                } 

Roundtheclock