passport: req.isAuthenticated occasionally fails - race condition?

I have set up passport to use a custom authentication scheme, but for arguments sake, imagine it authenticates any user to the app. I have a ‘landing’ page with a login button that issues a post, handled as follows:

app.post('/:id/landing', function (req, res, next) {
  passport.authenticate('myAuth', function (err, user) {
    if (err || !user) {
      return res.redirect('/' + req.params.id + '/landing' + req.body.redirectTo);
    } else {
      req.logIn(user, function () {
        return res.redirect('/' + req.params.id + '/' + req.body.redirectTo);
      });
    }
  })(req, res, next);
});

later, a route handler matches the redirect:

app.all('/:id/', function (req, res) {
  if (req.isAuthenticated()) {
    res.sendFile('index.html', {root: __dirname + '/../frontend/'});
  } else {
    // Instead of redirecting to /landing, simply render it.
    // this gets round safari issue with losing url fragments during a redirect:
    // https://bugs.webkit.org/show_bug.cgi?id=24175
    //
    res.render('landing', {message: '', previousID: '/' + req.params.id});
  }
});

9 times out of ten, this all works fine, but occasionally req.isAuthenticated will return false. If I put some logging in, I can see that before the redirect I have a valid req.user object, but then in the second route handler following the redirect, req.user is undefined. Sometimes it works, sometimes it doesn’t! (When it does work, req.user IS defined in the second route handler) Is this issue the same as others have reported around the user not being serialised correctly?

About this issue

Original URL
State: closed
Created 10 years ago
Comments: 15

Commits related to this issue

workaround for passport bug to ensure redir auth see: + https://github.com/jaredhanson/passport/issues/401 + https://github.com/jaredhanson/passport/issues/306 — committed to dxwc/CRUD-JSFree_en by dxwc 5 years ago
workaround for passport bug to ensure redir auth see: + https://github.com/jaredhanson/passport/issues/401 + https://github.com/jaredhanson/passport/issues/306 — committed to dxwc/CRUD-JSFree by dxwc 5 years ago

Most upvoted comments

@smartmouse You are probably seeing it on some machines and not others as the saving of the session is ASYNC. When you debug your app you subtly change the timings of your functions, which is perhaps why it now works. If you want to confirm, try the work around I proposed above, manually saving the session before doing any post-authentication redirect:

      req.logIn(user, function () {
        // Manually save session before redirect. See bug https://github.com/expressjs/session/pull/69
        req.session.save(function(){
            res.redirect('/someURL');
        });
      });

The other issue you linked to is unrelated I believe.

benheymink on Mar 17, 2015

FYI - Just experienced this problem (September 2017) and this fix still works!

bradocchs on Sep 29, 2017

@tkalfigo Weird, it should save it 100% of the time. Are you sure you are only finishing the request/performing a redirect AFTER the session.save callback has completed? Doing the following won’t work:

req.session.save()
req.redirect()

Make sure you are finishing the request in the callback:

req.session.save(function(){
  req.redirect()
});

benheymink on May 19, 2015

@ChristopherGS The issue was that req.logIn is async, and hadn’t finished flushing to the session store by the time the next request had come in for the redirect resource. I got around it by manually saving the season before the redirect:

          req.logIn(user, function () {
            // Manually save session before redirect. See bug https://github.com/expressjs/session/pull/69
            req.session.save(function(){
                res.redirect('/someURL');
            });
          });

Hope this helps!

benheymink on Feb 23, 2015

If anyone is using cookie-session, the above/following might not work (as the cookie-session’s save() does not support callbacks):

req.session.save(function(){
  req.redirect();
});

Managed to get it to work by setting some value on the session before redirecting:

req.session.timestamp = new Date();
req.redirect();

tilleps on Sep 7, 2018