Felipe Lrz
Felipe Lrz

Reputation: 33

Express-validator does not return object in async validation

I am validating a login form with express-validator and passport.js, using a local strategy:

login: function() {
  passport.use('local-login', new LocalStrategy({
    passReqToCallback: true
  },
    function(req, username, password, done) {
      req.check('username', 'Incorrect user and/or password.').doesUserExists(password);
      req.check('password', 'Password cannot be empty.').notEmpty();

      req.asyncValidationErrors(true)
      .then(function(user) {
        return done(null, user);
      })
      .catch(function(errors) {
        if (errors) return done(null, false, req.flash('error', errors));
      });
    }
  ));
}

The function doesUserExists() is a custom asynchronous validation, which query for the user, compare the password provided with the hashed password in the database, and resolves it:

doesUserExists: function(username, password) {
  return new Promise(function(resolve, reject) {
    User.findOne({ username: username })
    .then(function(user) {
      if (user) return user;
      if (!user) reject(user);
    })
    .then(function(user) {
      user.comparePassword(password, function(error, isMatch) {
        if (isMatch) return user;
        else reject(user);
      });
      resolve(user);
    })
    .catch(function(error) {
      if (error) reject(error);
    });
  });
}

So far it is working perfectly, except when the user and the password are matched, and the promise is resolved, no object (user) is returned to the req.asyncValidationErrors() function, preventing its .then() block to redirect to the user profile.

I must add that I'm pretty new to promises, and not sure if what I'm expecting should happen. Perhaps some misunderstanding about how it works is leading me to think erroneous.

Update

For now, I decided to make another database query for the validated user/password:

req.asyncValidationErrors(true)
  .then(function() {
    User.findOne({ username: username })
    .then(function(user) {
      return done(null, user);
    });
  })
  .catch(function(errors) {
    if (errors) {
      return done(null, false, req.flash('error', errors));
    }
  });

Extra database queries are not elegant, but...

Upvotes: 2

Views: 1059

Answers (1)

Jose Hermosilla Rodrigo
Jose Hermosilla Rodrigo

Reputation: 3683

You don't need to create new Promise if you can just return the Promise returned by User.find :

doesUserExists: function(username, password) {
    return User.findOne({ username: username })
    .then(function(user) {
      if (user) {
        // Also here is a mistake because we can't return inside 
        // the comparePassword callback, and that's why user is not get back
        user.comparePassword(password, function(error, isMatch) {
          if (isMatch) return user;
          else throw new Error('my error');
        });
      }
      else throw new Error('my error');
    });
}
// Now use it as follows
req
.check('username', 'Incorrect user and/or password.')
.doesUserExists(username, password)
.then(function(user){/* user authenticated */})
.catch(function(error){/* user not atuhenticated */});

So I guess you can compare password later and problem solved :

doesUserExists: function(username, password) {
    return User.findOne({ username: username })
    .then(function(user) {
      if (user) return user;
      else throw new Error('my error');
    });
}
// Now use it as follows
req
.check('username', 'Incorrect user and/or password.')
.doesUserExists(username, password)
.then(function(user){
  /* From here, user was found */
  user.comparePassword(password, function(error, isMatch) {
    if (isMatch){ /* Do whatever with the user authenticated */ }
    else { /* Do whatever you want when password don't match */ }
  });
})
.catch(function(error){/* user not found */});

If you're doing more than one async validation, I suggest you to use Promise.all() to make to execute parallel async functions.

Upvotes: 1

Related Questions