1
\$\begingroup\$

I'm using passport to setup login in an express site. The site is just a playground for me to learn. The code inside login was exhibiting the arrow anti pattern so I decided it was time to learn to use promises.

But I don't know if what I've arrived at is good, bad, idiomatic, or idiotic. Any and all feedback appreciated!

var passport = require('passport');
var LocalStrategy = require('passport-local').Strategy;
var Promise = require('bluebird');
var bcrypt = require('bcrypt');
var db = require('./db').db; //users are stored in mongo

//I'm using bluebird.js for promises
var users = Promise.promisifyAll(db.users);
var compare = Promise.promisify(bcrypt.compare);

//I think this helps when responding to the db user load completing
function NoMatchedUserError(message) {
  this.name = "NoMatchedUserError";
  this.message = message || "Incorrect username.";
}
NoMatchedUserError.prototype = new Error();
NoMatchedUserError.prototype.constructor = NoMatchedUserError;

//this strategy is used by passport to handle logins
module.exports.localStrategy = new LocalStrategy(function(username, password, done) {
  var matchedUser;

  var comparePassword = function(user){
    if(!user) {
      throw new NoMatchedUserError();
    }

    //memoise the loaded user so it can be returned below
    matchedUser = user;
    return compare(password, matchedUser.password);
  };

  users.findOneAsync({ username: username })
    .then(comparePassword)
    .then(function(isMatch) {
      return isMatch
        ? done(null, matchedUser)
        : done(null, false, { message: 'Incorrect password.' });
    })
    .catch(NoMatchedUserError, function() {
      return done(null, false, { message: 'Incorrect username.' });
    }) 
    .error(function(err) {
      return done(err);
    });
});
\$\endgroup\$
0

1 Answer 1

3
\$\begingroup\$

Define all your error types globally in some file for convenience, like the built-in errors are defined.

Mixing promises and callbacks is a big anti-pattern, just think in promises and let .nodeify handle the mapping to callback equivalent at the end.

var passport = require('passport');
var LocalStrategy = require('passport-local').Strategy;
var Promise = require('bluebird');
var bcrypt = require('bcrypt');
var db = require('./db').db; //users are stored in mongo

//I'm using bluebird.js for promises
var users = Promise.promisifyAll(db.users);
var compare = Promise.promisify(bcrypt.compare);


// This strategy is used by passport to handle logins
module.exports.localStrategy = new LocalStrategy(function(username, password, done) {
  users.findOneAsync({username: username}).bind({})
    .then(function(user) {
        if (!user) {
          throw new NoMatchedUserError('Incorrect username.');
        }
        this.user = user;
        return compare(password, user.password);
    })
    .then(function(isMatch) {
      if (isMatch) {
        return this.user;
      }
      else {
        throw { message: 'Incorrect password.' };
      }
    })
    .nodeify(done);
});
\$\endgroup\$
1

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.