Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

SEC-1229: Redesign Concurrent Session Control implementation #1477

spring-issuemaster opened this Issue Aug 24, 2009 · 4 comments


None yet
1 participant

Luke Taylor (Migrated from SEC-1229) said:

There are several problems with the concurrent session control code, mostly stemming from the fact that it is largely to do with Http sessions (although in theory the session could be something else) but the checks are applied through the AuthenticationManager and not in the web layer. This requires that a session already be created eagerly, wasting resources, before the ConcurrentSessionController is invoked. The creation of a separate internal AuthenticationManager for use by the namespace block is also incompatible with the current approach (SEC-1227).

Ideally the sequence should be:

  1. the AuthenticationManager is called first to authenticate the request.
  2. If successful, the ConcurrentSessionController is invoked to determine whether the authentication will exceed the allowed sessions
  3. A new session is created to store the authentication and provide an Id to add to the SessionRegistry
  4. The ConcurrentSessionController is invoked again to register the authentication with the SessionRegistry

This creates some issues:

  1. The control cannot be enacted from a single point (the AuthenticationManager). It could be integrated with the existing SessionManagementFilter and the AuthenticatedSessionStrategy interface.
  2. Event generation will not be as simple as before. the AuthenticationManager will generate a success event and subsequently, the AuthenticatedSessionStrategy (with the concurrent checks built in) will reject the authentication.

Luke Taylor said:

Some more thoughts on an alternative implementation:

A ConcurrentSessionControlAuthenticatedSessionStrategy could extend the DefaultAuthenticatedSessionStrategy. This would be injected into AbstractAuthenticatonProcessingFilter instances and the SessionManagementFilter - basically the only two locations where session-fixation protection is implemented. It would implement the call to the ConcurrentSessionController to check the authentication. It shouldn't even be necessary to create a session Id first - just assume that one will be created, if it doesn't already exist. ConcurrentSessionControllerImpl checks to see if the current request is already attached to a registered session, but we don't need to do that if the session Id is null, since the current request has no session. So change the checkAuthenticationAllowed method to take an Id argument and to only perform the registry check if the Id is not null.

In fact, it would probably be better if the ConcurrentSessionController was dropped altogether. The checkAuthenticationAllowed behaviour can be added to the ConcurrentSessionControlAuthenticatedSessionStrategy and the existing registerSuccessfulAuthentication method is just a delegated call to the SessionRegistry. Configuration would be simpler without having the extra bean to inject. The addition of the session-controller-ref attribute in the namespace should also be rolled back.

Luke Taylor said:

Concurrent session control is now handled through a specialized AuthenticatedSessionStrategy implementation "ConcurrentSessionControlAuthenticatedSessionStrategy", injected into the AbstractAuthenticationProcessingFilter and the SessionManagementFilter (the latter handles non-interactive authentication situations). The legacy classes and namespace syntax have been removed.

The namepspace config is now enabled through the new "session-management" element (which also now handles session-fixation-protection and other properties related to the SessionManagementFilter). The child element enables concurrency checking and has the attributes max-sessions, error-if-max-exceeded (NB "error", not "exception" as it was in the old element) and error-url. The latter allows a URL to be supplied to which the user will be sent if they exceed their allocated sessions (and error-if-max-exceeded is 'true').

Luke Taylor said:

Docs have been updated to match the code changes.

@spring-issuemaster spring-issuemaster added this to the 3.0.0 RC1 milestone Feb 5, 2016

This issue supersedes #1459
This issue supersedes #1476

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment