Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Edge case: Unable to login with 2 session cookies stored in browser #881

Open
matusferko opened this issue Mar 30, 2022 · 2 comments
Open

Comments

@matusferko
Copy link

When I reconfigured session storage to share cookie/session between subdomains (added {domain: ".domain.com"}
I have encountered on multiple time the issue where user ended up with old and new session cookie in the browser.
when express-session is getting cookies it was always returning the older session id.
In this case following code is executed:

 // generate the session object
    debug('fetching %s', req.sessionID);
    store.get(req.sessionID, function(err, sess){
      // error handling
      if (err && err.code !== 'ENOENT') {
        debug('error %j', err);
        next(err)
        return
      }

      try {
        if (err || !sess) {
          debug('no session found')

It is an edge case but express session behaves inconsistently. It creates/writes one session id and it reads the other id.
It creates conditions where it's not possible to log in without end user cleaning up their cookies.
It creates following loop

  1. User tries to log in.
  2. Express session gets incorrect session id from header
  3. session storage returns null because session expired
  4. Express session creates new session id (user is not logged in)
  5. User is returned to login page ..

The way I resolved it was that I implemented middleware where I manually parse cookie header , detect multiple session cookies and expiring them manually.

In the above code snippet I would suggest to cleanup req.sessionID cookie in the case sess is not found in the session store.

@dougwilson
Copy link
Contributor

Hello, we can take a look. Would you be able to provide server and client code and explain how to set it up to produce the scenario you are speaking of so we can try to help fix? Otherwise you are welcome to make a pull request with the fix you are thinking of.

@zbryikt
Copy link

zbryikt commented Jul 11, 2022

We constantly have users complaining about login issue; some recently reported that they are logged out instantly after a successfully login. Some of them have expired session but I can't reproduce this issue by shortening the session's maxAge.

Duplicate the session id can reproduce a similar effect. Say you have this cookie:

connect.sid=somevalidtoken; domain=my.domain

Add a random new one with an invalid token and slightly modified domain:

connect.sid=someunsignedinvalidtoken; domain=.my.domain

Then delete the old one and reload. Now you have two cookies named connect.sid. However, express-session will always get the invalid one if the cookies are in the right order, causing it to generate a new id every time the express-session middleware is run.

While this can be reproduced intentionally, users usually won't want to mess up with their own session cookie so I don't know if this is the root cause for users who suffered from similar issues. But I think there must be something to do with cookie since incognito mode seems to be able to fix this issue for them.

Duplicated cookies are totally legit and always possible due to browser bug, browser extensions or any other reasons, so I think we should still take care of this properly. Perhaps an event or exception for developers to take care will be sufficient.

For example, in

debug('cookie signature invalid');
we probably should next(err) since it's kinda weird to have an id with invalid cookie signature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants