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

Session regeneration during persistent is fragile, causes session loss #43

Open
rvm-peercode opened this issue Oct 2, 2023 · 3 comments
Labels
Bug Something isn't working

Comments

@rvm-peercode
Copy link

Bug Report

Versions: 1.11.0

Summary

We've received reports of users losing their session, seemingly at random. After debugging, the cause proved to be that if a request is done that changes the session, and the response is not received by the client, the session is lost.

Current behavior

In the current behavior, the session id is regenerated if the session has changed. The old session is destroyed. If, for some reason, the response is not received by the client, either because the client cancels the request, or because the application encounters a fatal error after persisting the new session, the client never receives the Set-Cookie header with the new session id. That means that the client still has the old session id, while the server no longer has that id. At this point, the client is no longer logged in.
This also happens when the network connection is not in perfect order: if one of the responses gets lost the issue occurs.
It also occurs if the response takes long, and a user clicks a second time on a (possibly different) element, where session persistence happens before the second click, but the click is done before the response of the first click is received.
In practice, every situation where the session is changed and the response is not received by the client triggers this problem.

How to reproduce

One way to reproduce this is in an application. Click on an element that does a request that causes the session to change, and then very quickly cancel the request by pressing Escape. You are now logged out.

Expected behavior

The expected behavior is that the session is still valid.

Possible solutions

I would like to propose a solution where a constructor variable of the CacheSessionPersistence class can be used to configure whether a change in the session data results in session regeneration. This can be true by default to retain the current behavior.
If this is acceptable, I can supply a pull request to implement this.

@rvm-peercode rvm-peercode added the Bug Something isn't working label Oct 2, 2023
@Ocramius
Copy link
Member

Ocramius commented Oct 2, 2023

The expected behavior is that the session is still valid.

The point of session ID re-generation is precisely to prevent this.

@rvm-peercode
Copy link
Author

Well, it means that any network interruption or request cancellation by a user invalidates the session and logs the user out, which is not very user friendly.

Are you open to making this configurable, as I proposed?

@Ocramius
Copy link
Member

Ocramius commented Oct 3, 2023

can be used to configure whether a change in the session data results in session regeneration.

Hmm, ID re-generation is usually to be performed by the upper layers, not by the storage itself:

// - the session has changed (data is different)
if ('' === $id || $session->isRegenerated() || $session->hasChanged()) {

I didn't expect this decision to be done in this repo at all 🤔

rvm-peercode added a commit to rvm-peercode/mezzio-session-cache that referenced this issue Oct 4, 2023
This is the responsibility of upper layers.

Closes: mezzio#43
rvm-peercode added a commit to rvm-peercode/mezzio-session-cache that referenced this issue Oct 4, 2023
This is the responsibility of upper layers.

Closes: mezzio#43

Signed-off-by: Roel van Meer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants