A solution for more specific error handling

Question

I immediately got confused when Dave manually created another error in the index.js in the callback of the POST/login route. This renders the "user not found" error from user.js superflous. We'll never see that error, because the POST/login callback is just testing for any error and then saying, "Wrong email or password." I also don't like the user experience of saying "Wrong email or password." Which one was wrong? Mostly, it just doesn't feel clean to have one error handler in user.js, and another in the callback of the POST/login route.

Here is what I came up with; I personally just like the flow of this a lot more.

In user.js:

UserSchema.statics.authenticate = function (email, password, callback) {
    User.findOne( { email: email })
        .exec( (err, user) => { //when findOne has found one (or not), execute this function.
            if (err) return callback(err); //if there is an error with the query operation itself
            else if (!user) {
                let error = new Error('User not found with that email address.');
                error.status = 401;
                return callback(error);
            }
            bcrypt.compare(password, user.password, (err, result) => {
                if (result === true) return callback(null, user);
                if (result === false) {
                    let error = new Error("Incorrect password.");
                    error.status = 401;
                    return callback(error);
                }
            })
        });
}

I explicitly test true and false just for semantic/readability reasons. Also, I tried simply returning callback(err) after the the first if block (no test, no creating a new error) and this didn't work; not sure why. Anyway, I would prefer to create a new error and simply say "Incorrect password" than use whatever error bcrypt.compare would send along.

In index.js:

//POST /login
router.post('/login', (req, res, next) => {
  if (req.body.email && req.body.password) {
    User.authenticate(req.body.email, req.body.password, (err, user) => {
      if (err) return next(err);
      req.session.id = user._id;
      return res.redirect("/profile");
    });
  } else {
    var err = new Error('Email and password are required!');
    err.status = 401;
    return next(err);
  }
});

It works, and it gives me a nice specific error saying whether the email wasn't found, or the password was wrong.

Answer 1 · 2017-02-10T02:55:20Z

February 10, 2017 2:55am

Hello,

While I do see that you have come up with an alternative solution that, by a simple overview, appears that it will work well, I would like to point out something with the 'usability' statement you have made.

The reason behind simply saying "Wrong email or password", giving the user an unhelpful user experience is for security. You will generally find nowadays, that many websites will not specifically tell you whether you have incorrectly entered a password or username/email address. By telling the user that the password does not match, it let's them know that the username/email does. Therefore, they know they can attempt again to 'guess' by trying or programmatically badger the server until it either crashes or they are allowed in. (If you haven't guessed by now, I am talking about a malicious user). The next thought may be to only allow them to try a certain amount of times. This is helpful but is generally only temporary.

As for the username/email, this is generally an email. Saying the password is incorrect tells a malicious user the location at which an email lives, where they can go attack another site. This also gives way to a malicious user to have user enumeration, a list of valid users to which they can potentially try to enter the account of on either the site they found it out or the one associated with their email. Unfortunately, since most places have an email as a username, if a malicious user gains access to one account, they might be able to gain access to multiple (being that many users use the same email & password)

There are many articles across the internet about login/authentication security, so take a search on Google and learn more, maybe even apply it to what you have written here.

Just wanted to share some insight, and again, kudos on venturing off to write you own implementation, normally the developers that take their own initiative make the best developers!

Welcome to the Treehouse Community

Looking to learn something new?

Brendan Moran

Brendan Moran

A solution for more specific error handling

1 Answer

Joe Beltramo

Joe Beltramo

Brendan Moran

Brendan Moran