Commits on Oct 16, 2015

1. Make passport.authenticate() callbacks more useful …

   The `callback` argument is challenging to use because passport treats it as an
   alternative to its default behavior for Strategy#succes, #fail and #error, but
   the callback doesn't have access to the same middleware variables (req, res,
   next, multi and failures) that passport does.
   
   This is change moves all passport login logic into a function, and uses that
   function as the default value of `callback`. The only API change is that the
   signature of the `callback` function changes from:
   
       function (err, user, info, status) {}
   
   To
   
       function (err, user, info, status, req, res, next) {}
   
   This changes code that previously needed to do this:
   
   ```javascript
   app.get('/protected', function(req, res, next) {
     passport.authenticate('local', function(err, user, info) {
       if (err) { return next(err) }
       if (!user) { return res.redirect('/signin') }
       res.redirect('/account');
     })(req, res, next);
   });
   ```
   
   To be able to do this:
   
   ```javascript
   app.get('/protected',
     passport.authenticate('local',
       function(err, user, info, status, req, res, next) {
         if (err) { return next(err) }
         if (!user) { return res.redirect('/signin') }
         res.redirect('/account');
       }
     )
   );
   ```
   
   This has three main benefits:
   
   * It avoids having to recreate the middleware function on every request
   * It makes the syntax more readable to engineers new to JavaScript, Node or
     passport
   * It makes the strategy augmentation logic more understandable to strategy
     developers
   
   I feel this is valuable because the only reason you'd want to use the callback
   argument instead of default passport handling is if your application was
   delegating session management outside of Express (e.g. to another service), and
   performance was a major consideration.
   
   Given the existing workarounds for the issue, the change should also not have
   any negative security implications.

   

   kara-ryli committed Oct 16, 2015
   Configuration menu

   • View commit details
   • Copy full SHA for db99684
   • Browse repository at this point

   Copy the full SHA

   db99684 View commit details

   Browse the repository at this point in the history