Spring Security
  1. Spring Security
  2. SEC-1050

Add new option to create a session for a newly authenticated user

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.4
    • Fix Version/s: 3.0.0 M2
    • Component/s: Core, Namespace
    • Labels:
      None

      Description

      Currently it is possible for a user to be authenticated by something like remember-me, which doesn't create a session and for the response to be committed before the filter chain gets to the point where the SecurityContextPersistenceFilter (HttpSessionContextIntegrationFilter) attempts to create a session to store the context. HttpServletRequest.getSession() will then fail if cookies are being used to maintain the session, as the cookie information for the response will already have been streamed back to the client.

      We should possibly look at providing an additional option in the SessionFixationProtectionFilter which would allow it to create a new session when it detects that authentication has occurred since the start of the request, even if a session doesn't already exist. This would mean the session was already in place before the response was committed and the context could then safely be stored.

      The namespace would then have an additional create-session="onAuthentication" option which would transparently provide this feature.

        Issue Links

          Activity

          Hide
          Jean-Pol Landrain added a comment -

          According to the servlet specification, this is actually a bug in HttpSessionContextIntegrationFilter: calling request.getSession(true) after the response is being commited like it's done from the method storeSecurityContextInSession returns null instead of a new HttpSession instance if no session already exists.
          I can confirm it effectively happens like that in Tomcat 5.5 and 6: Tomcat follows the servlet spec and the call to request.getSession(true) made after the call to chain.doFilter(request, responseWrapper) returns null. So the SecurityContext object can't be stored in the session and, consequently, the authentication mechanism is reexecuted every time the same client calls the server. In my case, the authentication is done via an LDAP and, having the clients reauthenticate each time they make a call to the server, is a real performance killer.

          in 2.0.4, HttpSessionContextIntegrationFilter.java on line 371: "httpSession = safeGetSession(request, true);"

          Show
          Jean-Pol Landrain added a comment - According to the servlet specification, this is actually a bug in HttpSessionContextIntegrationFilter: calling request.getSession(true) after the response is being commited like it's done from the method storeSecurityContextInSession returns null instead of a new HttpSession instance if no session already exists. I can confirm it effectively happens like that in Tomcat 5.5 and 6: Tomcat follows the servlet spec and the call to request.getSession(true) made after the call to chain.doFilter(request, responseWrapper) returns null. So the SecurityContext object can't be stored in the session and, consequently, the authentication mechanism is reexecuted every time the same client calls the server. In my case, the authentication is done via an LDAP and, having the clients reauthenticate each time they make a call to the server, is a real performance killer. in 2.0.4, HttpSessionContextIntegrationFilter.java on line 371: "httpSession = safeGetSession(request, true);"
          Hide
          Luke Taylor added a comment -

          The servlet spec doesn't actually say that a call to request.getSession(true) will return null if the response has been committed - it says it will throw an IllegalStateException (and I'm pretty sure Tomcat adheres to that). Strictly speaking you could argue that the class should call response.isCommitted() rather than trapping the exception, but the net result is the same and there may be historical reasons (e.g. container bugs) which explain why it was coded the way it is. Hence I've left it alone in the past. So it's not a bug - there's no way HttpSessionContextIntegrationFilter can create a session if you have caused the response to be committed.

          Creating a session in your app should solve the problem.

          Show
          Luke Taylor added a comment - The servlet spec doesn't actually say that a call to request.getSession(true) will return null if the response has been committed - it says it will throw an IllegalStateException (and I'm pretty sure Tomcat adheres to that). Strictly speaking you could argue that the class should call response.isCommitted() rather than trapping the exception, but the net result is the same and there may be historical reasons (e.g. container bugs) which explain why it was coded the way it is. Hence I've left it alone in the past. So it's not a bug - there's no way HttpSessionContextIntegrationFilter can create a session if you have caused the response to be committed. Creating a session in your app should solve the problem.
          Hide
          Jean-Pol Landrain added a comment -

          Yes, you're right actually. The 2.4 servlet specification says about getSession(boolean create):

          "To make sure the session is properly maintained, you must call this method before the response is committed. If the container is using cookies to session integrity and is asked to create a new session when the response is committed, an IllegalStateException is thrown."

          So, sorry for my wrong comment.

          However, I'm not sure Tomcat adheres to that: I have a case tested both with Tomcat 5.5 and 6 where the line 371 "safeGetSession(request, true);" of HttpSessionContextIntegrationFilter returns null without throwing any exception. I have no other explanation for this behaviour than the fact the response is already commited when HttpSessionContextIntegrationFilter tries to create the session. Maybe, because we are in a filter, Tomcat doesn't throw the IllegalStateException it is supposed to throw in that case, but returns null instead.

          > there's no way HttpSessionContextIntegrationFilter can create a session if you have caused the response to be committed.

          My application doesn't really need to create a session. The session is only needed for Spring Security in order to store the Security Context. So I wonder if Spring Security should create the session before calling chain.doFilter() ? I mean that's probably what a filter is supposed to do if it needs an HttpSession, because after the call to chain.doFilter() it can't know if the response is already commited when it tries to instantiate the session.

          > Creating a session in your app should solve the problem.

          I'll try that as workaround. Thanks for your input.

          Show
          Jean-Pol Landrain added a comment - Yes, you're right actually. The 2.4 servlet specification says about getSession(boolean create): "To make sure the session is properly maintained, you must call this method before the response is committed. If the container is using cookies to session integrity and is asked to create a new session when the response is committed, an IllegalStateException is thrown." So, sorry for my wrong comment. However, I'm not sure Tomcat adheres to that: I have a case tested both with Tomcat 5.5 and 6 where the line 371 "safeGetSession(request, true);" of HttpSessionContextIntegrationFilter returns null without throwing any exception. I have no other explanation for this behaviour than the fact the response is already commited when HttpSessionContextIntegrationFilter tries to create the session. Maybe, because we are in a filter, Tomcat doesn't throw the IllegalStateException it is supposed to throw in that case, but returns null instead. > there's no way HttpSessionContextIntegrationFilter can create a session if you have caused the response to be committed. My application doesn't really need to create a session. The session is only needed for Spring Security in order to store the Security Context. So I wonder if Spring Security should create the session before calling chain.doFilter() ? I mean that's probably what a filter is supposed to do if it needs an HttpSession, because after the call to chain.doFilter() it can't know if the response is already commited when it tries to instantiate the session. > Creating a session in your app should solve the problem. I'll try that as workaround. Thanks for your input.
          Hide
          Pierre-Antoine Grégoire added a comment -

          If I may add something to Jean-Paul's comment:
          Creating a session in the application is an interesting work-around.
          Though Jean-Paul didn't mention the context of the error which is the Spring Remoting HttpInvoker.

          Show
          Pierre-Antoine Grégoire added a comment - If I may add something to Jean-Paul's comment: Creating a session in the application is an interesting work-around. Though Jean-Paul didn't mention the context of the error which is the Spring Remoting HttpInvoker.
          Hide
          Pierre-Antoine Grégoire added a comment -

          Sorry for my previous comment with no real final point.

          The point is that in the context of the Spring Remoting HttpInvoker, the default configuration/behaviour doesn't give access to the session.
          Still it's possible to add an Interceptor or a dedicated Filter for this purpose, though integration of Spring HttpInvoker and Spring Security should be a bit easier. maybe through a dedicated set of filters or a specific namespace?

          Cheers.

          Show
          Pierre-Antoine Grégoire added a comment - Sorry for my previous comment with no real final point. The point is that in the context of the Spring Remoting HttpInvoker, the default configuration/behaviour doesn't give access to the session. Still it's possible to add an Interceptor or a dedicated Filter for this purpose, though integration of Spring HttpInvoker and Spring Security should be a bit easier. maybe through a dedicated set of filters or a specific namespace? Cheers.
          Hide
          Luke Taylor added a comment -

          @Jean-Pol
          HttpSessionContextIntegrationFIlter doesn't need to create a session unless the user has been authenticated, and it can't tell if this will happen before the filter chain is invoked (hence the reason I have created this issue). It can be configured to eagerly create a session if you wish, but most users don't want this behaviour.

          @Pierre-Antoine I'm not sure what kind of integration you're referring to (from a Spring Security perspective) but feel free to submit a patch as a separate issue. With a stateless client you can also cache the authentication at the back end to get round performance problems.This may be preferable to creating an HTTP session.

          Show
          Luke Taylor added a comment - @Jean-Pol HttpSessionContextIntegrationFIlter doesn't need to create a session unless the user has been authenticated, and it can't tell if this will happen before the filter chain is invoked (hence the reason I have created this issue). It can be configured to eagerly create a session if you wish, but most users don't want this behaviour. @Pierre-Antoine I'm not sure what kind of integration you're referring to (from a Spring Security perspective) but feel free to submit a patch as a separate issue. With a stateless client you can also cache the authentication at the back end to get round performance problems.This may be preferable to creating an HTTP session.
          Hide
          Luke Taylor added a comment -

          After some thought this should probably just be the behaviour for the default "ifRequired" option - i.e. create a new session on authentication, unless session-creation is disabled. Session handling needs to be addressed in more depth at some point as there are now several places where flags are set indicating whether new sessions may be created and whether existing sessions should be overwritten.

          Show
          Luke Taylor added a comment - After some thought this should probably just be the behaviour for the default "ifRequired" option - i.e. create a new session on authentication, unless session-creation is disabled. Session handling needs to be addressed in more depth at some point as there are now several places where flags are set indicating whether new sessions may be created and whether existing sessions should be overwritten.
          Hide
          Luke Taylor added a comment -

          This should be supported by the changes in SEC-1211. The DefaultAuthenticatedSessionStrategy has an option called "alwaysCreateSession" which can be set to true to ensure that a session is always created for a newly authenticated user.

          Show
          Luke Taylor added a comment - This should be supported by the changes in SEC-1211 . The DefaultAuthenticatedSessionStrategy has an option called "alwaysCreateSession" which can be set to true to ensure that a session is always created for a newly authenticated user.

            People

            • Assignee:
              Luke Taylor
              Reporter:
              Luke Taylor
            • Votes:
              3 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: