Describe the bug When configuring CSRF with a CookieCsrfTokenRepository and a XorCsrfTokenRequestAttributeHandler for use with JavaScript frameworks (according to https://docs.spring.io/spring-security/reference/5.8/migration/servlet/exploits.html#_i_am_using_angularjs_or_another_javascript_framework) the default login form generated by Spring Security doesn't accept username and password anymore.

Upon submitting the form the login page simply gets reloaded with no error message displayed to the user and no relevant log message either.

Please note: The login form does contain a hidden _csrf input parameter with a value. So, the CSRF token should be transmitted with the form.

This is related to https://github.com/spring-projects/spring-security/issues/12869.

To Reproduce 1. Configure both a CookieCsrfTokenRepository and an XorCsrfTokenRequestAttributeHandler according to https://docs.spring.io/spring-security/reference/5.8/migration/servlet/exploits.html#_i_am_using_angularjs_or_another_javascript_framework 2. Add a formLogin(Customizer.withDefaults()) to the HttpSecurity SecurityFilterChain bean in a Spring Boot application using Spring Security (e.g., via the spring-boot-starter-security starter). 3. Start the Spring Boot application. 4. Open http://localhost:8080/ 5. Try to sign in.

Expected behavior The user should be signed in.

Sample https://github.com/BjoernKW/sample-for-possible-csrf-token-bug-in-spring-security-6

Comment From: BjoernKW

As @insight720 has pointed out in this (no longer existing?) comment https://github.com/spring-projects/spring-security/issues/12915#issuecomment-1482416902 it seems to be working as expected if using CsrfTokenRequestAttributeHandler delegate = new CsrfTokenRequestAttributeHandler() instead of XorCsrfTokenRequestAttributeHandler delegate = new XorCsrfTokenRequestAttributeHandler();.

I'm not sure if that's an appropriate solution, though, or if this has other implications I'm not aware of.

Comment From: insight720

I deleted the comment because I didn't understand the difference between using delegate directly and using delegate::handle. CsrfTokenRequestAttributeHandler is the default implementation before, and it is relatively simple. In other words, it can indeed solve the problem.

Comment From: sjohnr

@insight720

CsrfTokenRequestAttributeHandler is the default implementation before, and it is relatively simple. In other words, it can indeed solve the problem.

While using CsrfTokenRequestAttributeHandler does solve the problem, keep in mind that you are disabling BREACH protection of CSRF tokens entirely. This is not necessarily recommended. You are of course free to do so if you prefer.

@BjoernKW, thanks for reaching out on this issue. As it happens, I noticed a similar issue yesterday when using the configuration from the migration guide for an Angular app that redirected to the default login form. However, I don't consider it a bug since it is just an example for working with an Angular app. Combining Angular with a Thymeleaf-style CSRF integration (which the default login form is similar to) is a different case.

You can provide (for example) the following configuration if you want to support both styles:

    @Bean
    public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
        XorCsrfTokenRequestAttributeHandler delegate = new XorCsrfTokenRequestAttributeHandler();
        // set the name of the attribute the CsrfToken will be populated on
        delegate.setCsrfRequestAttributeName("_csrf");
        CsrfTokenRequestHandler requestHandler = new CsrfTokenRequestHandler() {
            @Override
            public void handle(HttpServletRequest request, HttpServletResponse response, Supplier<CsrfToken> csrfToken) {
                delegate.handle(request, response, csrfToken);
            }

            @Override
            public String resolveCsrfTokenValue(HttpServletRequest request, CsrfToken csrfToken) {
                String tokenValue = CsrfTokenRequestHandler.super.resolveCsrfTokenValue(request, csrfToken);
                if (tokenValue.length() == 36) {
                    return tokenValue;
                }
                return delegate.resolveCsrfTokenValue(request, csrfToken);
            }
        };

        http
            // ...
            .csrf((csrf) -> csrf
                .csrfTokenRequestHandler(requestHandler)
                .csrfTokenRepository(CookieCsrfTokenRepository.withHttpOnlyFalse())
            );

        return http.build();
    }

I'm not sure this scenario is common. If it is, we could consider adding this example to the migration guide as well.

The above example should move you forward. With that, I'm going to close this issue. Feel free to add additional comments if you feel I've missed anything or would like to discuss documentation improvements.

Comment From: BjoernKW

@sjohnr Thanks a lot for providing a working configuration for both of those use cases!

It'd be great if the documentation was a little clearer on how to implement these configuration for specific common use cases. It'd be even better if there was a simple API for abstracting over the implementation details, e.g.:

http
            // ...
            .csrf(withJavaScriptCsrfHandling());
http
            // ...
            .csrf(
                        withJavaScriptCsrfHandling()
                        .and()
                        .withHttpCsrfHandling());

Comment From: sjohnr

@BjoernKW, glad the configuration worked for you!

It'd be great if the documentation was a little clearer on how to implement these configuration for specific common use cases.

As I mentioned earlier, I don't know if this scenario is common (e.g. I feel like it is not very common). Can you suggest a way that the documentation could be made clearer? I feel there are a number of examples and notes that explain the most common cases, so I'm wondering what you feel is missing? Is it just this specific example?

It'd be even better if there was a simple API for abstracting over the implementation details

I like this idea. The challenge I foresee is supporting the large variety of small variations that would be needed. I feel that building on one or two solid implementations with delegation is powerful enough and covers most other cases better than a "flavors" API might do.

Having said that, I am taking in as much feedback as possible and formulating some ideas for best supporting the AngularJS case (and similar scenarios), so I appreciate you providing feedback. Thanks!

Comment From: BjoernKW

@sjohnr Thanks for taking the time to explain this in more detail!

As I mentioned earlier, I don't know if this scenario is common (e.g. I feel like it is not very common).

My particular scenario is this: A web application using server-side rendering (with Thymeleaf, i.e. no JavaScript front-end such as Angular) for displaying views to the user. The same application, however, also provides REST API endpoints. To test these endpoints, Postman is used. To be able to account for CSRF tokens in Postman, JavaScript token handling is required. Hence, being able to use both Thymeleaf-style CSRF integration and JavaScript token handling is necessary for this application.

Can you suggest a way that the documentation could be made clearer?

This behaviour in question is described in these sections in the documentation, which didn't seem like an obvious place to look when I first encountered that behaviour: Documentation > Migrating to 6.0 > Servlet Migrations > Exploit Protection Documentation > Servlet Applications > Protection Against Exploits >Cross Site Request Forgery (CSRF) for Servlet Environments

I also had to look up what BREACH means exactly and how it might be relevant to my problem. Maybe, just putting these sections somewhere more prominent and adding more descriptive headlines or hints (in terms of common scenarios) might be very helpful already.

"flavors" API

I like the name for such an API :-)

for best supporting the AngularJS case

Changing the term to "Angular" here might be beneficial, too, since AngularJS is the - discontinued and now largely obsolete - predecessor of Angular.

Comment From: sjohnr

Thanks for the good feedback, @BjoernKW!