Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Bcrypt-NodeJS compare() returns false whatever the password

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.

like image 810
Avanserv Avatar asked Sep 03 '17 11:09

Avanserv


People also ask

What does bcrypt hash return?

Asynchronous Hashing The bcrypt. hash() function takes in the plain password and a salt as input, and returns a hashed string.

Why we use bcrypt in node JS?

The bcrypt npm package is a JavaScript implementation of the bcrypt password hashing function that allows you to easily create a hash out of a password string . Unlike encryption which you can decode to get back the original password, hashing is a one-way function that can't be reversed once done.

Which is better crypto or bcrypt?

Use bcrypt where you want to do slow and computationally expensive hashing -- this will generally be for hashes where you really don't want an attacker to be able to reverse the hash, e.g. user passwords. Use native crypto for everything else.


2 Answers

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.

like image 178
Samson Maosa Avatar answered Sep 21 '22 08:09

Samson Maosa


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)

like image 23
D-Beloved Avatar answered Sep 23 '22 08:09

D-Beloved