node.jsexpresspassport-localbcryptjs

Why is my Node.js app not correctly validating passwords at login (using passport-local and bcryptjs)?


I'm building a Node.js application and having trouble with login validation. I'm using the passport-local strategy with bcryptjs but whilst it's correctly identifying when the username does not exist in the database, it is logging in users even when an incorrect password is provided.

I'm probably making a very stupid mistake but some pointers would be great!

passport.js:

const passport = require("passport");
const LocalStrategy = require("passport-local").Strategy;
const bcrypt = require("bcryptjs");
const connection = require("./database");
const User = connection.models.User;

const verifyUser = (username, password, done) => {
  User.findOne({ username: username }).then((user) => {
    if (user == null) {
      return done(null, false, { message: "No user with this username" });
    }

    try {
      if (bcrypt.compare(password, user.password)) {
        return done(null, user);
      } else {
        return done(null, false, { message: "Incorrect password" });
      }
    } catch (err) {
      return done(err);
    }
  });
};

passport.use(
  new LocalStrategy(
    { usernameField: "username", passwordField: "password" },
    verifyUser
  )
);

passport.serializeUser((user, done) => {
  done(null, user.id);
});

passport.deserializeUser((userId, done) => {
  User.findById(userId)
    .then((user) => {
      done(null, user);
    })
    .catch((err) => done(err));
});

Register and login post requests (auth.js):

router.post("/register", async (req, res, next) => {
  const { error } = registerValidation(req.body);

  if (error) {
    return res.status(400).send({ message: error["details"][0]["message"] }); // Message to be tidied but works
  }

  const userExists_1 = await User.findOne({ username: req.body.username });
  if (userExists_1) {
    return res
      .status(400)
      .send({ message: "An account with the same username already exists" }); // Message to be tidied but works
  }

  const userExists_2 = await User.findOne({ email: req.body.email });
  if (userExists_2) {
    return res.status(400).send({
      message: "An account is already registered to this email address", // Message to be tidied but works
    });
  }

  try {
    const hashedPassword = await bcrypt.hash(req.body.password, 10);
    const user = new User({
      username: req.body.username,
      email: req.body.email,
      password: hashedPassword,
    });

    user.save().then((user) => {
      console.log(user);
    });

    res.redirect("./login");
  } catch {
    res.redirect("./register");
  }
});

// ----------------------------------------------------

router.post(
  "/login",
  passport.authenticate("local", {
    failureRedirect: "/login",
    successRedirect: "/home",
    failureFlash: true,
  })
);

Solution

  • Looks like compare bcrypt method uses a callback or a promise.

    Cf documentation:

    // As of bcryptjs 2.4.0, compare returns a promise if callback is omitted:
    bcrypt.compare("B4c0/\/", hash).then((res) => {
        // res === true
    });
    

    Please try to update your db callback function to async and await for the response of bcrypt

    User.findOne({ username: username }).then(async (user) => {
      // ..
    
      if (await bcrypt.compare(password, user.password)) {
      // ..
    
      }
    }