-
Notifications
You must be signed in to change notification settings - Fork 6k
SessionDestroyedEvent not getting triggered under certain conditions #12140
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
Comments
Hi @attitudemattrs, thanks for the report. In order to help us to debug the issue, can you provide a minimal, reproducible sample where we can start the application and go through those steps? |
Hi @marcusdacoregio, appreciate your response. I believe I have been able to create a small Spring Boot example based on a slightly modified version of the Security a Web Application project found here. I have provided a zip file A few things to note about the application:
To build the project just execute The steps to reproduce are identical to the ones I provided earlier, though I will repeat them here just to add some clarifying statements and to show exactly what I did. Expected BehaviorMake sure you do steps 4 - 6 within the
Unexpected BehaviorMake sure you do steps 6 - 9 within the
Hopefully this gives you what you need and that I haven't left anything out. If you have any questions or would like me to provide a pre-built executable jar, please let me know. Thanks again for looking into this issue. Apologize the missteps for the attachments. I didn't see them embedded in the middle of my text so I made multiple attempts |
Thanks for putting the effort into the sample @attitudemattrs. Once the team finishes the Spring Security 6 release we can start looking into this, I appreciate your patience. |
Hi @marcusdacoregio, In researching to see if there was a work-around for this issue, I came across something that I believe addresses our problem. If you recall from my earlier posts (and my demo application), the session management was defined in our application configuration files as follows:
The demo application I uploaded had a similar configuration, but in I created a This got me researching further how the sessions were being managed and took me to the section of the Spring Security documentation on Session Fixation Attack Protection and the description of the different session fixation settings. Unfortunately, I'm not well versed on the differences for some of these, especially
In the demo application I changed the
In both cases I reran my tests and they worked exactly as we need it to act. In fact, even better. Results documented in the scenario below (previously Unexpected Behavior): Make sure you do steps 6 - 9 within the
At this point the SessionDestroyedEvent is immediately triggered for the user1 session and it is cleaned up and a new session created for the user2 session
Based on my testing so far, it seems that using the session fixation setting of So, at this point I don't know if what I logged before is "expected" behavior when using Before I proceed forward with using Thanks in advance for your help! |
Hi @attitudemattrs, thanks for your patience. The strategy that Spring Security uses by default for session fixation is changing the session id, see
The With In summary, your scenario is working fine and the session is being destroyed correctly, the public final class MyAuthenticationStrategy extends AbstractSessionFixationProtectionStrategy {
private final SessionRegistry sessionRegistry;
@Override
HttpSession applySessionFixation(HttpServletRequest request) {
String oldSessionId = request.getSession(false).getId()
request.changeSessionId();
this.sessionRegistry.removeSessionInformation(oldSessionId);
return request.getSession();
}
} Does that makes sense for you? |
Hi @marcusdacoregio, Thank you for the explanation. I believe I understand the reasons for the behavior we were seeing by not specifically setting the session fixation configuration and using the default. If I read what you responded with correctly, since my original configuration of Assuming this is correct, I have to apologize because I am still a little unclear on a few things and ask for a little more of your time. Looking at the code for If my assumptions are correct (which is likely a bad assumption), I am still unsure why, when I changed the configuration to If you could clarify this for me it would be much appreciated. Additionally, if I can impose on your time for a couple more questions...
I am basically trying to determine whether the best resolution to our problem is simply to use Again, thank you so much for your time and your patience with me on this topic. This has definitely been a learning experience for me and your time and help is greatly appreciated. |
That is correct, however, there is no old session, the session continues the same but with a different id. The session seems to be there only because
Yes
Not really, accordingly to the Spring Security documentation:
When I tried with
No, the code that I showed you was just to clarify that the
You can have this answer in the documentation that I've linked above.
I don't think you have a problem there, as I've said before, when you are using the change id strategy, there is no such thing like old and new session, it's the same session but a different id. Therefore, it is expected that only a single session is destroyed. The information present in |
Hi @marcusdacoregio, Makes sense. I have closed this issue based on the discussion above. Thank you for your time, attention, and patience on this issue. |
I think that should not be the case since Version 5.4.0 released in 2020, have a look at #5439 (SessionRegistryImpl is now aware of SessionIdChangedEvent)
@attitudemattrs Reopen this issue? ;-) |
Thanks @drahkrub for the comments. I'll defer to @marcusdacoregio as to whether this should be reopened :-). For our situation I am getting around the issue by using a session-fixation-protection setting of newSession as shown below:
This seems to have gotten us around the problem, at least for this situation. |
Hi @drahkrub, Even if that is true, I don't think that it proves that the |
Hi @marcusdacoregio, sorry, you're absolutely right on this point - in this respect there is no need to reopen this issue. But there is no need (or there should be no need) to remove the old session id as you've done it in I came here because I have a similiar problem: In my web app I also found #10242 which looks similiar. |
Hi @drahkrub,
You are right.
Feel free to contribute to the #10242 discussion, or open a new issue if it is not so related. |
We currently have
class SessionDestroyedListener implements ApplicationListener<SessionDestroyedEvent>
defined which handles logout auditing and application session cleanup within theonApplicationEvent(SessionDestroyedEvent evt)
method.Within the web.xml we have the
HttpSessionEventPublisher
defined as below:Within the
<sec:http>
definition we have<sec:session-management invalid-session-url="/login"/>
defined, allowing multiple logins for the same user principal. Have also tried the configuration below:When a user logs into the application we use
session.setMaxInactiveInterval(time)
to set the max inactive interval for the session to 3 minutes (for example).Under normal circumstances, if the user logs into the application and logs out or closes the browser completely we get the SessionDestoryedEvent as expected, and we can do our auditing and cleanup. However, in the described scenario below we are seeing an issue.
The issue is best explained using the steps to reproduce below.
Expected Behavior
Unexpected Behavior
As you can see, in the Unexpected Behavior scenario, only the 2nd session triggers the SessionDestroyedEvent and gets cleaned up. The first one remains out there and seems to never expire. Looking through the
SessionRegistry
in the debugger in subsequent requests, the last activity time remains static at the last activity time before the original browser was closed and the session expired flag remains false for the remaining session. Even if I close the remaining browser windows the session never gets cleaned up until the application server is shutdown. Though in my Un-Expected Behavior scenario I used the same user, this exact behavior can be repeated even if the second user login is done using a completely unique principal. See below:Note, I even changed the user principal to be unique within the application code by adding a unique UUID to it and it didn't matter, despite the fact that even with the same user logging in, the principal was always unique. Just the existence of another instance of the same browser (even browsing a different site) seems to affect the behavior.
One final remark. In the Unexpected Behavior scenario, if I stop at step 5 and wait, without logging in again, the SessionDestroyedEvent does get triggered and the session is cleaned up as expected.
This behavior was repeated with Chrome, Edge, and Firefox.
I'm not ruling out configuration issues on the application side, but this certainly seems like odd and incorrect behavior to me that seems outside the control of the application or its configuration.
Any assistance in this matter would be greatly appreciated!
Thanks
The text was updated successfully, but these errors were encountered: