The Wayback Machine - https://web.archive.org/web/20201206085557/https://github.com/madhums/node-express-mongoose-demo/issues/214
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bcrypt instead of crypto #214

Open
juanda99 opened this issue Sep 30, 2016 · 4 comments
Open

bcrypt instead of crypto #214

juanda99 opened this issue Sep 30, 2016 · 4 comments

Comments

@juanda99
Copy link

@juanda99 juanda99 commented Sep 30, 2016

It would be better to use bcrypt, beause its more secure as it's slower (uses more computing cycles).
Your code could also be better:

You wouldn't need salt field in User model, because it's saved into the same field as password does.

For authentication, something like:

var mongoose = require('mongoose'),
  bcrypt = require('bcryptjs');

var userSchema = mongoose.Schema({
  email: String,
  pw: String,
});

userSchema.methods.validPassword = function(password) {
  return bcrypt.compareSync(password, this.pw);
}; 

I can make a PR

@PazzaVlad
Copy link

@PazzaVlad PazzaVlad commented Oct 7, 2016

+1

@madhums
Copy link
Owner

@madhums madhums commented Oct 14, 2016

+1 for bcrypt!

@Husamui
Copy link

@Husamui Husamui commented Nov 1, 2016

+1

@mylastore
Copy link

@mylastore mylastore commented Oct 10, 2017

please +1

@madhums madhums added the help wanted label Nov 14, 2017
@Raywire Raywire mentioned this issue Sep 15, 2019
5 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
You can’t perform that action at this time.