dcodeIO / bcrypt.js

Optimized bcrypt in plain JavaScript with zero dependencies.
Other
3.47k stars 264 forks source link

bcrypt.compare always returns false. #76

Closed thislekan closed 6 years ago

thislekan commented 6 years ago

Hello. I am pretty new to node and I've been following a tutorial. The tutor used bcrypt.compare in his code to compare a login password to the hash in the database (He uses a MAC) and it worked. When i tried to do the same, it didn't work. I have tried pasting his source code in my text editor and I still ended up with the same problem (I use a Windows PC). I'll like to know if this is a window's thing. I'll also like to know why this is an issue (querying from DB). I have tried going through closed issues to see if I can solve the problem but I ended up with the same result (no hope). Can anyone help me at least understand what the problem is?

NOTE: I use mongoDB and I checked to see if my hash in the DB was truncated: It wasn't. It returned 60 when i checked for length. My hash and passwords aren't wrong and the login password entered wasn't rehashed as the string was the value passed in to bcrypt.compare.

MY CODE ARE AS SEEN BELOW>


const mongoose = require('mongoose');
const validator = require('validator');
const Schema = mongoose.Schema;
const jwt = require('jsonwebtoken');
const _ = require('lodash');
const bcrypt = require('bcryptjs');

const UserSchema = new Schema({
    email: {
        type: String,
        required: true,
        minlength: 1,
        trim: true,
        unique: true,
        validate: {
            validator: validator.isEmail,
            message: `{value} is not a valid number`
        }
    },
    password: {
        type: String,
        required: true,
        minlength: 6
    },
    tokens: [{
        access: {
            type: String,
            required: true
        },
        token: {
            type: String,
            required: true
        }
    }]
});

UserSchema.methods.toJSON = function() {
    var user = this;
    var userObject = user.toObject();
    return _.pick(userObject, ['_id', 'email']);
}

UserSchema.methods.generateAuthToken = function() {
    var user = this;
    var access = 'auth';
    var token = jwt.sign({ _id: user._id.toHexString(), access }, 'abc123').toString();

    user.tokens.push({ access, token });

    return user.save().then(() => {
        return token;
    })
};

UserSchema.statics.findByToken = function(token) {
    var User = this;
    var decoded;

    try {
        decoded = jwt.verify(token, 'abc123');
    } catch (e) {
        return Promise.reject();
    }

    return User.findOne({
        '_id': decoded._id,
        'tokens.token': token,
        'tokens.access': 'auth'
    });
};

UserSchema.statics.findByCredentials = function(email, password) {
    var User = this;

    return User.findOne({ email }).then((user) => {
        if (!user) {
            return Promise.reject();
        }
        //console.log(user.password.length);
        return new Promise((resolve, reject) => {
            // Use bcrypt.compare to compare password and user.password
            bcrypt.compare(password, user.password, (err, res) => {
                //console.log(res);
                //console.log(` ${password}  :  ${user.password}`);
                //console.log(res);
                if (res) {
                    resolve(user);
                } else {
                    //console.log(user);
                    reject();
                }
            });
        });
    });
}

UserSchema.pre('save', function(next) {
    var user = this;
    if (user.isModified) {
        bcrypt.genSalt(10, (err, salt) => {
            bcrypt.hash(user.password, salt, (err, hash) => {
                user.password = hash;
                next();
            });
        });
    } else {
        next();
    }
})

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

module.exports = { User }
dcodeIO commented 6 years ago

There are no known issues with the library running on specific platforms. It's just JavaScript, nothing native. Hence, guessing:

The save hook looks wrong to me. Is it maybe hashing the already hashed password when a user is modified twice? That would still result in a valid hash in the database, but one that doesn't match the password anymore.

thislekan commented 6 years ago

Hello @dcodeIO Thank you for your very fast response. I appreciate it. The save function wasn't supposed to do that. I confirmed that it was indeed rehashing an already hashed password. However, I am unsure of what line of action to take to remedy this (I am pretty much a node noob). Is there any suggestion you can give me?

NOTE: 1st hash was consoled upon creating user which happened to be rehashed twice for reasons i know not. The 2nd hash is the confirmation of the hash in the DB. The error is solely from my save function.

image

thislekan commented 6 years ago

Hello @dcodeIO I am very grateful for your response and this was all my fault. I was supposed to pass an argument to user.isModified like so: user.isModified('password') and i completely missed that. It was all down to my recklessness. Thank you for your help. This can be closed now.

wendellpalazzo commented 5 years ago

Hello guys! I had a problem now with user authentication, after much debugging the codes.

I realized the following, always compare in the format STRING, if I send by example in Integer the hash not valid in the function that compares the password in plain text with the hash that came from BD.

I think this can be one of the problems of several devs who complain that they can not authenticate or that for some reason stopped working.

I think it would be interesting to put a note for example.

Thank you!

ratrateroo commented 4 years ago

I think in my case this is an issue too. I always use 123456 as a password to test my login form. In development, this password works but when I go to production integer password won't work anymore.

compare(inputPassword, passwordFromDB.toString()) does not work

What I did is before hashing I converted it first to string then save it to DB.

Hope this works also for you.

I am not yet sure why bcrypt is failing in production but not in development with integer passwords.

//mongoose schema password: { type: String, //already string required: true }, I hope someone can clear this out for us.

Thank you.