-
Notifications
You must be signed in to change notification settings - Fork 6k
A way to re-enable CSRF for OAuth2 bearer token requests #8668
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Thanks for the report, @lightoze. Can you tell me more about your use case? I'm asking because, since your proxy is the thing that has the client's session, I'd expect that to be where the CSRF token is generated/stored and where CSRF denials would be performed. |
In this setup both the proxy and each individual service have their own session. When CSRF is to be used, each service will use it's own session. If we ever want to have cross-service requests possible (not likely at the moment) we'll implement custom shared token repository, but decision when to use CSRF and when not to will be still on individual service side. |
Okay, thanks @lightoze for the extra background.
Perhaps, though I wonder if applications already setting I'll keep this ticket open while I continue to research ways to accommodate your use case. In the meantime, I believe it will work to set the matcher directly on the filter using an .csrf(csrf -> csrf
.withObjectPostProcessor(new ObjectPostProcessor<CsrfFilter>() {
@Override
public <O extends CsrfFilter> O postProcess(O object) {
object.setRequireCsrfProtectionMatcher(CsrfFilter.DEFAULT_CSRF_MATCHER);
return object;
}
})
) |
@jzheaux Thanks for the workaround! |
Hi @jzheaux public String cookieTokenExtractor(HttpServletRequest request) {
String header = request.getHeader(HttpHeaders.AUTHORIZATION);
if (header != null) {
return header.replace("Bearer ", "");
}
Cookie cookie = WebUtils.getCookie(request, "access_token");
return cookie != null ? cookie.getValue() : null;
} This leads to the need of having csrf validation in place. I modified your workaround (thanks for that one!) a bit, so it doesn't unnecessarily check GET requests to this one: .csrf(csrf -> csrf
.withObjectPostProcessor(new ObjectPostProcessor<CsrfFilter>() {
@Override
public <O extends CsrfFilter> O postProcess(O object) {
object.setRequireCsrfProtectionMatcher(request -> {
try {
return cookieTokenExtractor(request) != null;
}
catch (OAuth2AuthenticationException ex) {
return false;
}
});
return object;
}
})
) but basically reimplementing the BearerTokenRequestMatcher really only feels like a workaround. It would be great to have a configuration option to enable csrf also for requests that contain a JWT. If it defaults to being disabled, existing users would not feel a disruption, too. |
TL;DR In a stateful JWT environment, the need for CSRF protection is often reduced due to inherent features such as token storage in HttpOnly cookies, adherence to the same origin policy, use of the Bearer token scheme, and stateful JWT verification. Additional mechanisms such as the default CSRF countermeasures in modern browsers and proper implementation of CORS further mitigate the risk. While these factors together provide a robust defence against CSRF attacks, it is important to recognise that security is a multifaceted discipline. Personal statement: There's no need for CSRF in a stateful environment. Detailed explanation Cross-Site Request Forgery (CSRF) is an attack that tricks the victim into submitting a malicious request. This attack is specifically designed to change the state of requests, not steal data, as the attacker has no way of seeing the response to the fraudulent request. In the context of web applications that use JWT (JSON Web Tokens) for session management, the need for CSRF protection may be questioned. Here's why CSRF may not be necessary in a stateful JWT environment:
As you can see, there are many mechanics which protecting against man-in-the-middle attacks when using a stateless environment. Thus, the need for CSRF is not present. |
The proposed workaround above has one minor flaw. In case of a CSRF token is invalid, the http status should be 403, but spring returns 401 in this case. |
I'd like to take a moment to refresh my understanding of the situation here. When the token is stored in a cookie or in the session, then Spring Security's default CSRF defense is needed and should not be shut off by resource server configuration. There are a few ways that we could do this:
Option 4 could be made passive possibly by introducing a sub-interface to The first three options leave it up to the application to decide, so I don't want to only do that. I currently like a combination of options 3 and 4 the best since option 3 still gives the application full control should it feel that Spring Security is making the wrong decision and option 4 passively makes |
In my particular opinion, locate where the CsrfConfigurer contribution was being made by BearerTokenRequestMatcher was very tricky. I think it could be an option to do this:
At startup OAuth2ResourceServerConfigurer will disable CSRF for OAuth2 scenarios but if developers want to re-enable it throw CsrfConfigurer will take precedence over OAuth2ResourceServerConfigurer If none of the options works for you, I propose to update Spring Security documentation telling that in OAuth2 scenarios CSRF will be disabled by default. By the way, something didn´t work as spected with MockMvc and SpringBootTest because in the default scenario CSRF filter applies all the times, in my particular case I only found this problem in runtime, nor in integrated test, probably because there is no real token and instead of that there is a @WithMockUser |
We have user-facing services which are accessed via OAuth2 proxy, so they are configured as resource server and bearer token is added to request headers by the proxy (from client session).
Currently
OAuth2ResourceServerConfigurer.registerDefaultCsrfOverride
is called unconditionally and it makes it impossible to use conventional CSRF configuration.On reactive side there seems to be a solution (not tested though): when custom
requireCsrfProtectionMatcher
is specified, the override is not applied. Similar behaviour could be implemented for servlet security by introducingspecifiedRequireCsrfProtectionMatcher
flag onCsrfConfigurer
.The text was updated successfully, but these errors were encountered: