-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
Do Not Invalidate Current Session When Its Registered #15066
Conversation
this change prevents from deleting the current session to, creating the need to log in two times.
Hi @joaquinjsb, can you elaborate more on what is the problem and how this fixes it? As far as I can tell the invalidation is performed before the current session is registered in the |
what happens in my case is: what my fix does is, removes from the registeredSessions, the current session, so that you don't end up deleting both browser 1 and browser 2 sessions, which prevents the login flow from being "standard" on the user side. |
There is a test that verifies that behavior here, can you create a new test that reproduces the problem? |
I tried running it with a limit of 1, which behaves correctly as you describe, would the problem be probably on SpringSessionBackedReactiveSessionRegistry? do you have any suggestions on how to proceed? |
The best approach would be to create a minimal, reproducible sample for the issues, with tests preferably. Then we can identify exactly where the problem is |
here you have both minimal reproducible, and a test which is failing. |
I dived in further, the answer to our question is here: the delegate.getSession() creates a new session, then it gets saved in the repository, which means that the getSessions() will have both the currentSession and the older ones. so why it doesn't reflect in the tests is due to the fact that both InMemoryReactiveSessionRegistry & WebSessionStore has their own sessionsMaps. I can apply my change in the handler, so filters don't need to do that work, or as I did, or do you recommend another path? |
Thanks @joaquinjsb, this has been closed via 927840f |
this change prevents from deleting the current session, which ends up creating the need to log in two times.