Fix premature redirect when used with express-session

[zypA13510]

Are you implementing a new feature?

I'm not sure if you will call this "a new feature", it will solve a lot of issues involving express-session not saving the session in time before redirects happen, by calling req.session.save().

Is this a security patch?

No

Detail

By calling req.session.save() before res.redirect(), it ensures the session is properly stored in the session store, avoiding issues like #306, #401, #477, #482 (and possibly #254, #314, #521). To ensure compatibility, the code will check req.session.save && typeof req.session.save == 'function' before calling req.session.save(). Reference: expressjs/session#74

Note

As I only tested this with express, express-session and a specific session store for express-session, I'm not sure if this would cause issue with Connect or other middlewares. A more thorough test is probably required.

Checklist

• I have read the CONTRIBUTING guidelines.
• I have added test cases which verify the correct operation of this feature or patch.
• I have added documentation pertaining to this feature or patch.
• The automated test suite ($ make test) executes successfully.
• The automated code linting ($ make lint) executes successfully.

[coveralls]

Coverage decreased (-0.5%) to 98.329% when pulling 1db59e7 on zypA13510:master into 2327a36 on jaredhanson:master.

[mckenzieja]

Is Passport not being maintained anymore?

[rwky]

@zypA13510 would you like to create this PR against https://github.com/passport-next/passport and I'll take a look.

[zypA13510]

@jaredhanson Since this is neither closed nor commented, unlike #686, I take it that you would consider this if the checklist items are finished?

[rwky]

@zypA13510 this isn't maintained anymore, if you make a PR against the fork I'll look at it.

[zypA13510]

@rwky PR created. But you know, I'm reluctant to do this because it is a middleware I used that use passport. I'm not using passport directly. So even if you are able to fix this in your fork, the middleware has to switch the dependency to your fork for this to work (probably never happen).

[rwky]

@zypA13510 is the middleware on github?

[zypA13510]

@rwky okta/okta-oidc-js/packages/oidc-middleware

[rwky]

@zypA13510 I created a PR okta/okta-oidc-js#250 in that repo which switches to the fork.