Jarrod Carlson (Migrated from SEC-1241) said:
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.
I have a form handler registered at:
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:
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.
Any other suggestions are welcome, and I am happy to provide more concrete code examples if necessary...
Luke Taylor said:
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).
Jarrod Carlson said:
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?
Any situation where the original request is a POST is a legitimate case:
In this case what we want is for the original "POST" request to be substituted for the current request, therefore a match is required.
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)?
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.
Ildar Nurislamov said:
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 definition. This will completely exclude SecurityContextHolderAwareRequestFilter(SERVLET_API_SUPPORT_FILTER) from filter chain. But who really need it?!
ohad redlich said:
I understand the problem, and the solution.
However, I have a problem due to this solution. I wrote it in the Spring Security forum: