Have Passport Emit an Event on Authentication Failure

[jacklund]

My motivation for this is a use case we have in using Passport: we want to use it to respond automatically with a 401 on authentication failure, but we also want to be notified of authentication failures, e.g., for logging or for being able to detect attacks (brute force or DOS). I couldn't find any way to do this without registering a custom callback, which would mean, basically, handling the error ourselves, which is what we want Passport for.

I understand that this means making Passport an EventEmitter, which might be considered an intrusive change. If there's a different way of accomplishing the same objective, that would be fine; I just need some notification mechanism.

[coveralls]

Coverage increased (+0.14%) when pulling 0411a10 on jacklund:master into 4a8230c on jaredhanson:master.

[jaredhanson]

I pretty biased against making Passport an EventEmitter. That said, I think I have a decent solution for you. In master (to be released as passport 0.2.0), there is a failWithError option. See here and here for sample usage.

You could use this to centralize the logic for handling authentication errors like so:

app.post('/login',
  passport.authenticate('local', { successRedirect: '/home', failWithError: true }),
  function(err, req, res, next) {
    if (err.status == 401) {
      console.log('authentication error');
      // track potential brute force attacks
      return;
    }
    // pass other errors to app-level error handler
    next(err);
  }
);

Will that sufficiently address your issue?

[jacklund]

Yep, I believe that should do it. Thanks!

Just out of curiosity, why are you against it being an EventEmitter? I'm new enough to node not to have a strong opinion one way or the other, but I did find it interesting that you hadn't made it one.

[jaredhanson]

Don't get me wrong, I do like EventEmitters and use them many many places. Those places tend to be more "stateful" however, with objects that have a more complicated lifecycle (about which they emit events).

For handling HTTP requests, I find the "function chain" style of Express and middleware to be ideal. And since those chains flow naturally into application-specific code, it is easy to keep common middleware focused yet "extensible" by simply nexting in the appropriate places. And that's how I'd prefer to keep Passport operating.

Also, it keeps the ecosystem consistent. For instance, both the logging and brute-force limiting functionality could be implemented as middleware themselves. If they hooked into Passport events, however, that would not be possible.