Describe the bug We are currently in the process of updating to spring-security 6 and are having trouble with our MockMvc tests that are calling CSRF-protected endpoints with enabled BREACH-protection. We are using CSRF-cookies. All requests are rejected by the CsrfFilter and status 403 Forbidden. Our configuration works when using it outside of MockMvc.
To Reproduce - We are using Cookies for CSRF via CookieCsrfTokenRepository.withHttpOnlyFalse() - We are using SecurityMockMvcRequestPostProcessors.csrf() to populate our mock-requests - We are basically using this security-configuration: https://docs.spring.io/spring-security/reference/5.8/migration/servlet/exploits.html#_i_am_using_angularjs_or_another_javascript_framework
Expected behavior Requests should not be rejected because of invalid CSRF tokens while using SecurityMockMvcRequestPostProcessors.CsrfRequestPostProcessor.
Possible solution: To me it seems that the CSRF-token in our Mock-Request is not correctly processed by XorCsrfTokenRequestAttributeHandler. The following change in SecurityMockMvcRequestPostProcessors.CsrfRequestPostProcessor fixes the issue for us:
CsrfToken token = (CsrfToken) request.getAttribute(CsrfToken.class.getName());
- String tokenValue = this.useInvalidToken ? INVALID_TOKEN_VALUE : token.getToken();
+ String tokenValue = this.useInvalidToken ? INVALID_TOKEN_VALUE : deferredCsrfToken.get().getToken();
if (this.asHeader) {
Sample There is a writeup on stackoverflow with a similar issue: https://stackoverflow.com/questions/74729765/csrf-in-tests-stopped-working-with-spring-boot-3-and-spring-security-6
Comment From: hinnerkoetting
I created a draft PR that contains a failing test to show the problem here: https://github.com/spring-projects/spring-security/pull/12785
Comment From: marcusdacoregio
Hi @hinnerkoetting, thanks for the report and for providing a reproducible test.
I think this is in part related to https://github.com/spring-projects/spring-security/issues/12813.
If you want to simulate a javascript framework calling your API, I think you should have a /csrf endpoint as mentioned here, and get the CSRF token before doing your actual POST request, something like:
MvcResult mvcResult = this.mockMvc.perform(get("/csrf")).andReturn();
Cookie cookie = mvcResult.getResponse().getCookie("XSRF-TOKEN");
this.mockMvc.perform(post("/").header("X-XSRF-TOKEN", cookie.getValue()).cookie(cookie))
.andExpect(status().is2xxSuccessful())
You can create a RequestPostProcessor for this.
Although, if the SecurityMockMvcRequestPostProcessor$CsrfRequestPostProcessor#postProcessRequest is changed to include repository.saveToken(token, request, response); your original test also passes, I am not sure if this should be added since it was removed here. I'll page @sjohnr to get his input on this.
Comment From: hinnerkoetting
Hi @marcusdacoregio,
thanks for your support.
In our tests we are already using a PostProcessor that does something similar.
Still I think that it would be great, if SecurityMockMvcRequestPostProcessors.csrf() could provide this functionality out of the box since it seems to me that we are using an approach that is documented by spring boot.
Comment From: sjohnr
Hi @hinnerkoetting, thanks for reaching out! It’s been invaluable to hear about these issues and get first-hand reports with good details.
Our configuration works when using it outside of MockMvc.
Correct me if I'm wrong, but I believe this is because you've customized CSRF protection to work for the "AngularJS" case which allows raw tokens to be submitted, but returns hashed tokens when the _csrf request attribute is used.
The issue you're facing is then related to the fact that this customized CSRF protection (which is documented as an option in the migration guide for Angular users) does not automatically work with SecurityMockMvcRequestPostProcessors.csrf().
To me it seems that the CSRF-token in our Mock-Request is not correctly processed by XorCsrfTokenRequestAttributeHandler.
As it sits now (unless we decide a change is needed), I don't believe this is accurate. The csrf() helper that handles providing a CSRF token would not know about your customized CSRF support. It currently supports the case where the token returned from the request attribute is what is used to validate the token (the XorCsrfTokenRequestAttributeHandler) since it's doing the job of retrieving a token and providing it in the request. In other words, it requires a symmetrical flow between the response (where the client obtains the CSRF token) and the request (where the client presents the CSRF token). Since the customization recommended for Angular makes the flow asymmetrical (hashed tokens are returned, raw tokens are expected), this particular post processor doesn't work.
By way of example, imagine a customization that simply adds the letter "x" to the end of every CSRF token, and expects that token to be submitted by the client without the letter "x". I know this is a silly example, but the point is: How would the csrf() handler know what to do in this case? It wouldn't, right?
You would need to provide a custom post-processor as @marcusdacoregio suggested. I see the issue you bring up here as falling into this case. Let me know if you see this differently once you've read the above explanation.
To me, it also sounds like a documentation enhancement could help with this. Perhaps adding something in the Testing with CSRF Protection section would be helpful?
Comment From: hinnerkoetting
Hi @sjohnr, thanks for your insightful answer. It really helped me understand the issue better. I believe the existing documentation would really benefit from this explanation. 👍
So in my opinion, I still believe that it might make sense that there is a built-in Post-Processor that supports this usecase. I understand your point that SecurityMockMvcRequestPostProcessors.csrf() cannot support all cases automatically. But maybe there could be either a parameter or a separate PostProcessor that supports this case.
It might make migration to v3 easier for others, since I assume it's not some exotic use-case.
As for a documentation-update, I agree that that an update in the Testing with CSRF Protection makes sense. Furthermore a hint, that changes for MvcTests are necessary might also help others in the migration documentation (at least I did not look into the Testing with CSRF Protection section at all).
Anyway, thanks again for your explanation. Should I close this issue or keep it open (if you want to, you can also close it)?
Comment From: sjohnr
@hinnerkoetting,
Anyway, thanks again for your explanation. Should I close this issue or keep it open (if you want to, you can also close it)?
I'll let @marcusdacoregio decide that. I'll open an item to address docs including the migration guide.
So in my opinion, I still believe that it might make sense that there is a built-in Post-Processor that supports this usecase.
I hear you, and I think this is something we can definitely keep considering! However, I'm going to start with an example in the docs and see if we can get feedback on it. Reason being, while it's not an exotic use case, it may not be very useful if most folks with your case simply disable BREACH protection altogether. If enough folks need this built-in, that's when we could add it, I think.
Comment From: marcusdacoregio
I think we can close this for now and improve the docs addressing the use case. Given we have a workaround that is pretty much what the current post processor does, I'd choose to wait and keep an eye on the feedback and if it becomes common enough we can consider adding built-in support.
Comment From: mpalourdio
For all of those coming here from SO , I have drafted a RequestPostProcessor for MockMvcTests to make tests work again, without disabling csrf support. It's a drop-in replacement for the Spring Security csrf() RequestPostProcessor that does not work anymore.
Easily upgradable if you want to customize the header and cookie name (if needed)
https://gist.github.com/mpalourdio/6a6afabcc4824b08e8f580d32e11caef
You can use it like this
@Test
void postRequestTest() throws Exception {
mockMvc.perform(post("/api/post")
.with(xorCsrf()))
.andReturn();