Description
I encountered an intermittent issue in my app where two sessions were unexpectedly created for the same user between log in and log out. I wrote a test script to repeatedly execute the log in and log out code over and over so I could catch it reliably. Upon examining the most recent release's change I think a potential race condition has been missed in the 0.6.0
upgrade.
The source of the potential race condition can be found here: https://github.com/jaredhanson/passport/blob/master/lib/sessionmanager.js#L87-L90
The callback is called immediately after merge(req.session, prevSession);
but in this case req.session
may not yet be saved when the callback is called.
Fix
Add an additional req.session.save()
call for the case where keepSessionInfo
is true
. The other code path can run the cb();
straight away as it currently does when there is no change to req.session
.
Happy to provide a PR.
Expected behavior
If keepSessionInfo
is set on the login and logout function, and a random number is persisted to the session, then you repeatedly log in and log out over and over again, you would expect only one session to exist with the random number you persisted.
Actual behavior
With an async database backend (such as postgres) and after logging in and out hundreds of times, one can observe the situation where a session is sometimes duplicated.
Steps to reproduce
Use an async database backend, persist a random number to a session, then log in and out many times. Eventually two sessions will be created that have the same random number in their session data.
Environment
- Operating System: Ubuntu Linux
- Node version: 16
- passport version: 0.6.0
- Sessions database backend: Postgres