Spring Security
  1. Spring Security
  2. SEC-1241

SavedRequest not destroyed after successful authentication

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 3.0.0 M2
    • Fix Version/s: 3.0.0 RC1
    • Component/s: Web
    • Labels:
      None
    • Environment:
      Tomcat 6.0.18, Java 1.6, OS X 10.5

      Description

      SUMMARY:

      After an anonymous user authenticates in order to access a protected resource, the RequestCacheAwareFilter does not destroy the saved request.

      After successfully replaying the original request for the protected resource, the saved request is no longer valid/useful and should be destroyed.

      BACKGROUND:

      I have a form handler registered at:
      http://foo.com/app/form

      A handler is defined with two methods (one for GET and one for POST). The form posts back to the same url as the GET request for the form (a relatively common practice?).

      So for example:

      GET: /app/form

      <form action="/app/form" method="POST"> ... </form>

      The URL "/app/form" is protected by Spring Security and requires "IS_AUTHENTICATED_FULLY".

      When an anonymous user attempts to access the protected resource, they are correctly redirected to the login form. Once authentication is successful, the user is directed back to the protected resource.

      Upon submitting the form, though, the POST fails, because the session still contains a cached copy of the original GET request.

      Because the session still contains a copy of the original GET request, SavedRequest.doesRequestMatch (line 166) erroneously returns true. However, the requests do NOT match: the current request is a POST, whereas the saved request was a GET.

      Additionally, after successfully completing the saved request after authentication, RequestCacheAwareFilter.doFilter (line 36) does not remove the saved request from the cache.

      As a result, when the user attempts to POST the form, the result is the user being directed back to the GET view of the form, with no form-processing actions taking place. Debugging confirms that the requests are mapped to the handler's GET method.

      I would consider this a critical if not blocking issue, as it prevents normal form processing to occur after authenticating.

      I am still looking, but I have not yet discovered a way to configure Spring Security to use a custom class in-place-of the RequestCacheAwareFilter - so I cannot easily extend or remove the Filter.

      As a work around, I can change the form handler's POST methods to map to a different URL than the GET method - however, this breaks any RESTful contracts I might have wished to adhere to.

      Additionally, I can configure a custom AuthenticationSuccessHandler (namely, using the SimpleUrlAuthenticationSuccessHandler instead of the SavedRequestAwareAuthenticationSuccessHandler). Doing so effectively stops the Security layer from trying to replay the original request, but this has a jarring impact on the user experience in that users always get redirected back to the default URL after authenticating.

      CONCLUSION

      Any other suggestions are welcome, and I am happy to provide more concrete code examples if necessary...

        Activity

        Hide
        Luke Taylor added a comment -

        Thanks for the report. This looks like a failure in HttpSessionRequestCache to remove the saved request after a match.

        Please note that this area is still under development (see SEC-1167) and the intention is to make the saved request handling system pluggable.

        Regarding "the requests do NOT match: the current request is a POST, whereas the saved request was a GET", that is intentional in order to match requests which were orginally POSTs (it is only possible to redirect a request as a GET).

        Show
        Luke Taylor added a comment - Thanks for the report. This looks like a failure in HttpSessionRequestCache to remove the saved request after a match. Please note that this area is still under development (see SEC-1167 ) and the intention is to make the saved request handling system pluggable. Regarding "the requests do NOT match: the current request is a POST, whereas the saved request was a GET", that is intentional in order to match requests which were orginally POSTs (it is only possible to redirect a request as a GET).
        Hide
        Jarrod Carlson added a comment -

        I'm not sure I understand your statement about matching requests... My point was that the SavedRequest.doesRequestMatch() method will return TRUE when the current request is a POST and the saved request is a GET. It is unclear to me why this method would condsider the two requests as a match. Could you describe a legitimate case where the saved request is a GET and the current request is a POST?

        I would expect that in the case of the current request being a POST and the saved request being a GET, the method would return false (the saved request cannot be used).

        Hopefully, with the fix applied in this ticket, this scenario will not arise, but I'm still curious as to why the two requests being compared could still be considered a match?

        Show
        Jarrod Carlson added a comment - I'm not sure I understand your statement about matching requests... My point was that the SavedRequest.doesRequestMatch() method will return TRUE when the current request is a POST and the saved request is a GET. It is unclear to me why this method would condsider the two requests as a match. Could you describe a legitimate case where the saved request is a GET and the current request is a POST? I would expect that in the case of the current request being a POST and the saved request being a GET, the method would return false (the saved request cannot be used). Hopefully, with the fix applied in this ticket, this scenario will not arise, but I'm still curious as to why the two requests being compared could still be considered a match?
        Hide
        Luke Taylor added a comment -

        Any situation where the original request is a POST is a legitimate case:

        1. User submits a POST request to /secure
        2. Request is saved and user redirected to login
        3. User logs in and is redirected to the original URL "/secure"
        4. Browser issues a "GET" request for "/secure"

        In this case what we want is for the original "POST" request to be substituted for the current request, therefore a match is required.

        Show
        Luke Taylor added a comment - Any situation where the original request is a POST is a legitimate case: 1. User submits a POST request to /secure 2. Request is saved and user redirected to login 3. User logs in and is redirected to the original URL "/secure" 4. Browser issues a "GET" request for "/secure" In this case what we want is for the original "POST" request to be substituted for the current request, therefore a match is required.
        Hide
        Jarrod Carlson added a comment -

        Ah, but the case you describe is the reverse of the problem I experienced. In your example, the saved request is a POST and the current request is a GET.

        My problem specifically focused on the opposite scenario, where the saved request is a GET and the current request is a POST.

        Like I mentioned, this scenario shouldn't happen if the saved requests are properly destroyed when their usefulness expires, but my question remains: are there cases such as the one I describe, and should these be detected as NOT a match (if they can in fact legitimately occur)?

        Show
        Jarrod Carlson added a comment - Ah, but the case you describe is the reverse of the problem I experienced. In your example, the saved request is a POST and the current request is a GET. My problem specifically focused on the opposite scenario, where the saved request is a GET and the current request is a POST. Like I mentioned, this scenario shouldn't happen if the saved requests are properly destroyed when their usefulness expires, but my question remains: are there cases such as the one I describe, and should these be detected as NOT a match (if they can in fact legitimately occur)?
        Hide
        Luke Taylor added a comment -

        No, you're probably right. I can't think of any situation where an incoming POST request should match a saved GET request, so the default SavedRequest implementation should probably take that into account. The wonderful thing about working on Spring Security though is that you're never quite sure what some people will end up using it for and what scenarios they'll come up with .

        Changing SavedRequest to an interface and making RequestCache pluggable will allow all this to be customized if necessary.

        Show
        Luke Taylor added a comment - No, you're probably right. I can't think of any situation where an incoming POST request should match a saved GET request, so the default SavedRequest implementation should probably take that into account. The wonderful thing about working on Spring Security though is that you're never quite sure what some people will end up using it for and what scenarios they'll come up with . Changing SavedRequest to an interface and making RequestCache pluggable will allow all this to be customized if necessary.
        Hide
        Ildar Nurislamov added a comment -

        2.0.5 version also has this bug. It is very important because most production projects still use 2.0.5.
        2.0.5 version has another class hierarchy. So SecurityContextHolderAwareRequestFilter and SecurityContextHolderAwareRequestWrapper are responsible for such behavior in 2.0.
        You can easily workaround this problem by setting servlet-api-provision="false" attribute in <http> definition. This will completely exclude SecurityContextHolderAwareRequestFilter(SERVLET_API_SUPPORT_FILTER) from filter chain. But who really need it?!

        Show
        Ildar Nurislamov added a comment - 2.0.5 version also has this bug. It is very important because most production projects still use 2.0.5. 2.0.5 version has another class hierarchy. So SecurityContextHolderAwareRequestFilter and SecurityContextHolderAwareRequestWrapper are responsible for such behavior in 2.0. You can easily workaround this problem by setting servlet-api-provision="false" attribute in <http> definition. This will completely exclude SecurityContextHolderAwareRequestFilter(SERVLET_API_SUPPORT_FILTER) from filter chain. But who really need it?!
        Hide
        ohad redlich added a comment -

        I understand the problem, and the solution.

        However, I have a problem due to this solution. I wrote it in the Spring Security forum:
        http://forum.springsource.org/showthread.php?133566-2-opened-tabs-after-login-Spring-does-not-redirect-to-the-protected-resource

        Any ideas?

        Show
        ohad redlich added a comment - I understand the problem, and the solution. However, I have a problem due to this solution. I wrote it in the Spring Security forum: http://forum.springsource.org/showthread.php?133566-2-opened-tabs-after-login-Spring-does-not-redirect-to-the-protected-resource Any ideas?

          People

          • Assignee:
            Luke Taylor
            Reporter:
            Jarrod Carlson
          • Votes:
            1 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: