Aaron Aaron - 2 months ago 9
Node.js Question

bcrypt.compareSync is always returning false

I verified that in my db I am saving the username and hash of the password. I am able to retrieve the name from the db, however when I check the password it always returns false. Not sure what is wrong.

Here is my HTML

<div ng-controller="userController">
<div class=user>
<form name="login_form">
<h2 class>Login</h2>
<h3 class = "login_page">UserName</h3>
<input ng-model="user" type="text" ng-minlength="1" required>
<h3 class = "login_page">Password</h3>
<input ng-model="password" type="password" name="password" ng-minlength="4" required>
<input type="submit" value="Login" ng-click="login()" >
<div ng-if ="login_form.$submitted" ng-messages="login_form.password.$error" style="color:maroon" role="alert">
<div ng-message="minlength">Your field is too short</div>
</div>
<p ng-if="error">Username or login is incorrect</p>
</form>
</div>
<div class=user>
<form name = "register_form">
<h2 class>Register</h2>
<h3 class = "login_page">UserName</h3>
<input ng-model="reg.name" type="text" required>
<h3 class = "login_page">Password</h3>
<input ng-model="reg.password" type="password">
<input type="submit" value="Register" ng-click="register()" required >
<div ng-if ="login_form.$submitted" ng-messages="login_form.password.$error" style="color:maroon" role="alert">
<div ng-message="minlength">Your field is too short</div>
</div>
<p ng-if="duplicate">That user name is taken, please choose another</p>
<p ng-if="correct">Registration Succesfull</p>
</form>
</div>
</div>


Here is my controller on the server side

var mongoose = require('mongoose'),
Todo = mongoose.model('Todo');
Login = mongoose.model('Login');
var bcrypt = require('bcrypt');
var name = ""

module.exports = (function(){
return {
save_name:function(req, res){
req.session.user = req.body.user
Login.findOne({name: req.body.user},
function(err, user) {
if(user){
console.log(user.password);
console.log( bcrypt.compareSync(req.body.password, user.password));
res.json({'error': false});
}else {
res.json({'error': true});
}
})
}, //end of save name method
register:function(req, res){
bcrypt.hashSync(req.body.password, bcrypt.genSaltSync(8));
login = new Login({
name:req.body.user,
password: bcrypt.genSaltSync(8)
})
login.save(function(err){
if(err){
res.json({'error': true});
} else {
res.json({'sucess': true})
}
})
} // end of register user function
}
})();

Answer

You're saving a generated salt as the password instead of the actual hash itself. Also, explicitly calling genSalt*() is unnecessary. Lastly, you really should use the async functions instead, to avoid unnecessarily blocking the event loop. So with all of this in mind, you may end up with something like:

module.exports = {
  save_name: function(req, res) {
    req.session.user = req.body.user;
    Login.findOne({ name: req.body.user },
                  function(err, user) {
      if (err)
        return res.json({ error: true });
      bcrypt.compare(req.body.password,
                     user.password,
                     function(err, valid) {
        res.json({ error: !!(err || !valid) }); 
      });
    });
  }, // end of save name method
  register: function(req, res) {
    bcrypt.hash(req.body.password, 8, function(err, hash) {
      if (err)
        return res.json({ error: true });
      login = new Login({
        name: req.body.user,
        password: hash
      })
      login.save(function(err) {
        res.json({ error: !!err });
      })
    });
  } // end of register user function
};
Comments