SEC-2002: Add SessionFixationProtectionEvent

[spring-projects-issues]

Nick Williams (Migrated from SEC-2002) said:

Both types of successful authentication events (AuthenticationSuccessEvent AND InteractiveAuthenticationSuccessEvent) are fired BEFORE the session fixation protection migrates the session data. This is INVALID, as the session ID contained within the data provided by the event is no longer valid immediately after the event completes processing.

So, for example, one cannot use ApplicationListeners for AuthenticationSuccessEvent and SessionDestroyedEvent to maintain a log of login/logout activity, because the session ID changes between when the success event is called and the destroyed event is called.

There was a discussion about this in the Spring forums (attached). The conclusion reached in this discussion is, I believe, also invalid. Spring Security classes shouldn't have to be overridden in order to make them behave correctly. It seems a trivial matter for the authentication event to be called AFTER the session has been migrated, which would provide accurate information in objects provided, through events, to a consuming application.

[spring-projects-issues]

Nick Williams said:

I meant for the title of this bug to be "AuthenticationSuccessEvents called before session is migrated" but apparently messed that up somehow. Since I can't seem to find an edit function, please affect this change for me.

[spring-projects-issues]

Nick Williams said:

Correction:

In order to work around this issue, I changed session-fixation-protection from "none" to "migrateSession." It made no difference. The session ID still gets changed after the event gets called. So, this is just plain broke; session fixation protection doesn't matter.

[spring-projects-issues]

Rob Winch said:

As Luke mentioned this is not a bug. The intent is that the Authentication is the result of what caused the event (as per the javadoc). The event was caused by successfully authenticating the Authentication with the WebAuthenticationDetails that has a JSESSIONID before the session was changed. Therefore the JSESSIONID will be different than the JSESSIONID that is present in these events.

[spring-projects-issues]

Nick Williams said:

Respectfully, I disagree. How is an AuthenticationSuccessEvent any less valid after the session is migrated than before it is migrated? (Answer: it isn't.) Therefore, if the AuthenticationSuccessEvent would have more accurate information after session migration than it does before session migration, then why not issue the event after migrating the session.

The answer is, the AuthenticationSuccessEvent should be issued AFTER the session is migrated. It hurts nothing to do so, and doing so results in more accurate information at event issue time. There are no downsides to this. Why not fix it?

[spring-projects-issues]

Rob Winch said:

The behavior is working as documented and this feature has long since shipped. The information is accurate as documented. Specifically the Authentication is contains details about the session id at time of authentication. These details are not intended to be the current session. We cannot change the behavior since it is working as documented and others are currently using it. While it makes little sense for your use case, there are many Spring Security users and we cannot make assumptions about how they are using the framework or what their requirements are. For example, they may want to know the JSESSIONID and ipaddress of the user at time authentication to track where the original authentication came from (this is the intent behind this feature).

I can see how the session id after session migration can be valuable to others as well and so we can consider an enhancement to the code base and/or documentation to make this easier. However, the existing behavior must stay the same for the reasons listed above. One option to obtain the current HttpServletRequest information is using Spring's RequestContextHolder. In order to use the RequestContextHolder outside of the DispatcherServlet (i.e. Spring MVC controllers) ensure that you register the RequestContextListener or RequestContextFilter in your web.xml.

[spring-projects-issues]

Nick Williams said:

Valid point. As hard as it is for me to imagine a scenario where someone would be relying on the event being published before the session is migrated, I wouldn't want to break functionality developers are currently relying on. So, instead of changing the existing authentication event, here are some alternatives:

• Create a SessionMigratedEvent with the previous session ID and the new session object available as event properties; OR,
• Create an AuthenticationSuccessAfterMigrationEvent to provide the same details as the AuthenticationSuccessEvent, but published after session migration instead of for.

What do you think of these alternatives?

[spring-projects-issues]

Rob Winch said:

Rather than discussing how it is to be done, I'd like to first try to figure out what you need done. For example, it sounded as though I could summarize your requirements as

• You need the post migrated JSESSIONID at the time it is migrated
• You need the post migrated JSESSIONID at the time of logout

Are there other things you would expect? For example, what about the ipaddress at these times (which would also be stale on the WebAuthenticationDetails)? There may be other things you are looking for and I want to make sure we iron out these details before we decide how we do it.

Why doesn't the JSESSIONID from when the user submitted their username and password not work for you? If you have additional attributes why do you need those vs what is already provided. I'm not doubting your needs, but is important that I understand the whole scenario so that we can come to the best outcome.

Once we know everything that needs to be done, we can discuss ways we can accomplish the requirements.

Cheers,
Rob

[spring-projects-issues]

Nick Williams said:

I'm trying to track login/logout history. So, I at AuthenticationSuccessEvent time, I insert a record into the database with user ID, session ID, IP address and login time. Then, at SessionDestroyedEvent time, I update the record with the matching session ID and no logout date to indicate when they logged out (that way, even if their session just times out instead of them clicking the "logout" button, I still close their login history record).

A SessionMigratedEvent that contains the old and new session IDs would solve my problem, because it would allow me to find the record with the old session ID and update it to the new session ID, but that isn't necessarily ideal because then I have to (arguably unnecessarily) update a record I just inserted. However, a SessionMigratedEvent might be more useful for the general community.

An AuthenticationSuccessAfterMigrationEvent would also solve my problem as long as it still contained the WebAuthenticationDetails object (because I need IP address, session ID and principal).

Does this help?

[spring-projects-issues]

Rob Winch said:

Thank you this helps. I will need to sit on it a bit to figure out how to ensure this works for the general population.

[spring-projects-issues]

Nick Williams said:

Okeydokey.

[spring-projects-issues]

Nick Williams said:

Rob,

It's been several months since I've seen an update on this. I noticed that Spring 3.2 is in Milestone 1. I don't know whether M2 is next, or RC1, or what, but I would really love to see this resolved in 3.2.

I would be willing to contribute a patch if possible, if it will make the difference between it getting done in 3.2 or not. If this will work, let me know. Meanwhile, I'll investigate the source code to see what it'll take.

Thanks,

Nick

P.S. I just realized that one of the comments made in this bug was never addressed (possibly because I accidentally stated it incorrectly). I tried to work around this problem by disabling session migration. I know that ultimately session migration is the most secure, but this application is only available through a VPN, so I wasn't as concerned with that. I changed session-fixation-protection from "migrateSession" to "none." It made no difference. The session ID is STILL getting changed after the event gets called. The documentation (http://static.springsource.org/spring-security/site/docs/3.1.x/reference/ns-config.html) clearly states that setting session-fixation-protection to "none" disables session migration ("none - Don't do anything. The original session will be retained.), so since my sessions are still getting migrated, I believe at least that part is actually a bug.

[spring-projects-issues]

Rob Winch said:

Nick,

I think these are two distinct issues. The first is that you would like to be able to access the session id and the second is that session-fixation-protection="none" is not working for you. Please feel free to submit a new JIRA (as a bug) with a sample application that demonstrates this issue with clear instructions on how to reproduce it.

If you are able to put together a pull request, this will certainly help to ensure this issue is included in M2 (the next release). In fact, sending a pull request will assist with getting any issue resolved faster. Please be sure to read the Contributor Guidelines to make sure the pull request can be merged as easily as possible https://github.com/SpringSource/spring-security/wiki/Contributor-Guidelines

[spring-projects-issues]

Nick Williams said:

I have submitted a pull request. Includes two new classes, one new method, one changed method, three changed unit tests and two new unit tests. Please let me know if there is anything else I need to do.

#30

[spring-projects-issues]

Nick Williams said:

By the way, I have signed and agree to the terms of the SpringSource Individual Contributor License Agreement. I forgot to add that comment to my pull request. I have added the comment now.

[spring-projects-issues]

Nick Williams said:

Is there anything more I can do? Did I follow all of the contributor guidelines sufficiently? Is my code formatted correctly? :-)