search

mezzio / mezzio-session-cache

PSR-6 session persistence adapter for mezzio-session.
https://docs.mezzio.dev/mezzio-session-cache/
BSD 3-Clause "New" or "Revised" License
6 stars 10 forks source link

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

Open rvm-peercode opened 1 year ago

rvm-peercode commented 1 year ago

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.

Ocramius commented 1 year ago

The expected behavior is that the session is still valid.

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

rvm-peercode commented 1 year ago

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 commented 1 year ago

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:

https://github.com/mezzio/mezzio-session-cache/blob/e50ab7e0bd0be0245b0533481a35ad14cb7647ad/src/CacheSessionPersistence.php#L142-L143

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