[AGAIN] Fix premature redirect when used with express-session

[unknowndomain]

Just spent most of my evening trying to fix this issue with regular ol'passport only to realise it's been depreciated and this is the closest thing to a maintained branch (thanks for that btw), it's crazy that some of the biggest packages in the Node ecosystem like this have died and not been turned over to a community to look after.

I switched packages but found while the login issues are fixed the logout issue remains, and the only fix was to again use req.session.save on the redirect here. This is a slightly more complex issue because there is no built in redirect functionality in the logout function.

It seems to me like what is needed is for req.logout to take a redirect path or a callback but it doesn't currently do either.

[rwky]

Can you provide sample code that triggers this?

[unknowndomain]

A simplified example is:

app

app.get('/', auth.isLoggedIn, (req, res) => {
  req.controller.getRoot(req, res);
});

controller (without fix)

getRoot(req, res) {
    req.logout();
    res.redirect('/login');
  }

controller (with fix)

getRoot(req, res) {
    req.logout();
    req.session.save( function(err) {
      if ( err ) throw new Error(err);
      res.redirect('/login');
    } )
  }

Without the fix it would require you to manually refresh the browser to get the login form because the first redirect saw the user was logged in already and sent them back to the main screen.

[rwky]

Note to self https://github.com/passport-next/passport/blob/master/lib/http/request.js#L69 needs to have a signature similar to https://github.com/passport-next/passport/blob/master/lib/http/request.js#L32 so done is called once the user is actually logged out.

[rwky]

The fix for this has now been deployed to npm!

[unknowndomain]

Awesome, will have to check it out!