spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.82k stars 5.9k forks source link

CsrfAuthenticationStrategy does not regenerate CsrfToken with CookieCsrfTokenRepository #12141

Closed sjohnr closed 2 years ago

sjohnr commented 2 years ago

Describe the bug Using the new DeferredCsrfToken, CsrfAuthenticationStrategy does not regenerate CsrfToken with CookieCsrfTokenRepository within the same request. A null token cookie is added, but it does not generate a new one.

Note: In 6.0, eagerly loading the token is no longer the default, and requires accessing the request attribute (request.getAttribute(CsrfToken.class.getName())) manually as in the AuthenticationSuccessHandler below. Related gh-12094.

To Reproduce See sample.

  1. Navigate to http://localhost:8080 in the browser and log in with user:password.
  2. Observe that POST /login contains the same token in request and response.
POST /login HTTP/1.1
Host: localhost:8080
...
Cookie: JSESSIONID=EAA9296337F0588BC18C9C1BC0F8DBA1; XSRF-TOKEN=f1f89231-50e7-4eed-a15d-45c184c34e56
HTTP/1.1 200 
Set-Cookie: JSESSIONID=5528D2BC31D24431AF4F5A1D61AB0F1A; Path=/; HttpOnly
Set-Cookie: XSRF-TOKEN=; Max-Age=0; Expires=Thu, 01-Jan-1970 00:00:10 GMT; Path=/
X-XSRF-TOKEN: f1f89231-50e7-4eed-a15d-45c184c34e56
...

Expected behavior When eagerly regenerating the CsrfToken, the cookie (and header in this example) should reflect the updated token after a successful login.

Sample

Spring Security Version: 5.8.0-RC1

@SpringBootApplication
public class DemoApplication {

    public static void main(String[] args) {
        SpringApplication.run(DemoApplication.class, args);
    }

}
@Configuration
@EnableWebSecurity
public class SecurityConfig {

    @Bean
    public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
        // @formatter:off
        http
            .authorizeHttpRequests((authorize) -> authorize
                .anyRequest().authenticated()
            )
            .csrf((csrf) -> csrf
                .csrfTokenRepository(CookieCsrfTokenRepository.withHttpOnlyFalse())
            )
            .formLogin((formLogin) -> formLogin
                .successHandler((request, response, authentication) -> {
                    CsrfToken csrfToken = (CsrfToken) request.getAttribute(CsrfToken.class.getName());
                    response.setHeader(csrfToken.getHeaderName(), csrfToken.getToken());
                })
            );
        // @formatter:on
        return http.build();
    }

    @Bean 
    public UserDetailsService userDetailsService() {
        // @formatter:off
        UserDetails userDetails = User.withDefaultPasswordEncoder()
                .username("user")
                .password("password")
                .roles("USER")
                .build();
        // @formatter:on
        return new InMemoryUserDetailsManager(userDetails);
    }

}
mraible commented 1 year ago

@sjohnr I noticed a similar issue when trying to upgrade JHipster to use Spring Boot 3 and Spring Security 6. It seems that an XSRF-TOKEN cookie is not returned after an oauth2 login to Keycloak. With Spring Boot 2.x, a cookie is returned. https://github.com/jhipster/generator-jhipster/pull/19791#issuecomment-1321203742

I tried the workaround suggested here:

.oauth2Login(login -> login
    .successHandler((request, response, authentication) -> {
        CsrfToken csrfToken = (CsrfToken) request.getAttribute(CsrfToken.class.getName());
        response.setHeader(csrfToken.getHeaderName(), csrfToken.getToken());
    })
)

However, this causes the redirect back to the originating URL to not happen. It just stalls at:

http://localhost:8080/login/oauth2/code/oidc?state=7gD10TUEhzUnpH8AJHDR94aRuWARBy3a0j7t8nNQlyI%3D&session_state=9bb0ebfa-751a-4733-b989-ac8110ca44dd&code=6005c0b2-acfe-4a8f-9ee5-15922e176d24.9bb0ebfa-751a-4733-b989-ac8110ca44dd.6e8deddb-b4d6-4e2e-b389-b397d3f74fcd

Is it possible to configure the success handler so it calls the default one after adding the cookie?

mraible commented 1 year ago

The following seems to work. Not sure if it's the best solution:

.oauth2Login(login -> login
    .successHandler((request, response, authentication) -> {
        CsrfToken csrfToken = (CsrfToken) request.getAttribute(CsrfToken.class.getName());
        response.setHeader(csrfToken.getHeaderName(), csrfToken.getToken());
        SavedRequestAwareAuthenticationSuccessHandler handler = new SavedRequestAwareAuthenticationSuccessHandler();
        handler.onAuthenticationSuccess(request, response, authentication);
    })
)
sjohnr commented 1 year ago

Hi @mraible! Yes, your workaround is a fine approach. See this comment for some additional context as well.

Actually, @jzheaux and I were just working through some issues with XSRF-TOKEN cookies.

The other thing you might try is to actually add a Filter instead of a success handler, to ensure the cookie is returned as needed. This is very much similar to how it's done on the reactive side to subscribe to the Mono<CsrfToken> (see opt-out steps with AngularJS for an example of that).

Here's an example on the servlet side:

@Bean
public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
    http
        // ...
        .oauth2Login(Customizer.withDefaults())
        .addFilterAfter(new CsrfCookieFilter(), BasicAuthenticationFilter.class);

    return http.build();
}

private static final class CsrfCookieFilter extends OncePerRequestFilter {

    @Override
    protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
            throws ServletException, IOException {
        CsrfToken csrfToken = (CsrfToken) request.getAttribute(CsrfToken.class.getName());
        response.setHeader(csrfToken.getHeaderName(), csrfToken.getToken());
        filterChain.doFilter(request, response);
    }

}

Note: The placement of the filter is not exact, but after BasicAuthenticationFilter seems to work best in my testing. However, I have not tested it with an OAuth2 flow as you are doing.

mraible commented 1 year ago

@sjohnr I can confirm that your suggested solution works. Thank you!

It seems strange that Spring Security's WebFlux support ships with a CookieCsrfFilter class, but Spring MVC does not. For Spring WebFlux, we've found we have to do the following to make things works with Spring Security 6:

.csrf(csrf -> csrf
    .csrfTokenRepository(CookieServerCsrfTokenRepository.withHttpOnlyFalse())
    // See https://stackoverflow.com/q/74447118/65681
    .csrfTokenRequestHandler(new ServerCsrfTokenRequestAttributeHandler()))
// See https://github.com/spring-projects/spring-security/issues/5766
.addFilterAt(new CookieCsrfFilter(), SecurityWebFiltersOrder.REACTOR_CONTEXT)
sjohnr commented 1 year ago

Thanks for the feedback @mraible!

It seems strange that Spring Security's WebFlux support ships with a CookieCsrfFilter class, but Spring MVC does not.

I'm not sure I understand your comment. I can't find such a class in Spring Security. Perhaps it was added in your project for a very similar reason?

mraible commented 1 year ago

@sjohnr You are correct. The CookieCsrfFilter class is provided by JHipster. Sorry for the confusion.

hannah23280 commented 1 year ago

Hi @mraible! Yes, your workaround is a fine approach. See this comment for some additional context as well.

Actually, @jzheaux and I were just working through some issues with XSRF-TOKEN cookies.

The other thing you might try is to actually add a Filter instead of a success handler, to ensure the cookie is returned as needed. This is very much similar to how it's done on the reactive side to subscribe to the Mono<CsrfToken> (see opt-out steps with AngularJS for an example of that).

Here's an example on the servlet side:

@Bean
public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
    http
        // ...
        .oauth2Login(Customizer.withDefaults())
        .addFilterAfter(new CsrfCookieFilter(), BasicAuthenticationFilter.class);

    return http.build();
}

private static final class CsrfCookieFilter extends OncePerRequestFilter {

    @Override
    protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
            throws ServletException, IOException {
        CsrfToken csrfToken = (CsrfToken) request.getAttribute(CsrfToken.class.getName());
        response.setHeader(csrfToken.getHeaderName(), csrfToken.getToken());
        filterChain.doFilter(request, response);
    }

}

Note: The placement of the filter is not exact, but after BasicAuthenticationFilter seems to work best in my testing. However, I have not tested it with an OAuth2 flow as you are doing.

The solutioin to add the custom CsrfCookieFilter works. I was hoping that Spring Boot engineer can consider providing a built-in CsrfCookieFilter for us. In fact, just provide an argument to the method call , something such as shown below, to automatically add a built-in CsrfCookieFilter for us, if the argument is true.

.csrfTokenRepository(CookieServerCsrfTokenRepository.withHttpOnlyFalse() ,true)