Skip to content
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

CsrfAuthenticationStrategy does not regenerate CsrfToken with CookieCsrfTokenRepository #12141

Closed
sjohnr opened this issue Nov 4, 2022 · 7 comments
Assignees
Labels
in: web An issue in web modules (web, webmvc) type: bug A general bug
Milestone

Comments

@sjohnr
Copy link
Member

sjohnr commented Nov 4, 2022

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);
	}

}
@sjohnr sjohnr added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Nov 4, 2022
@sjohnr sjohnr self-assigned this Nov 4, 2022
@sjohnr sjohnr added in: web An issue in web modules (web, webmvc) and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 4, 2022
@sjohnr sjohnr added this to the 5.8.0 milestone Nov 4, 2022
@sjohnr sjohnr closed this as completed in 6b0ed02 Nov 5, 2022
sjohnr added a commit that referenced this issue Nov 9, 2022
@mraible
Copy link
Contributor

mraible commented Nov 20, 2022

@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. jhipster/generator-jhipster#19791 (comment)

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
Copy link
Contributor

mraible commented Nov 20, 2022

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
Copy link
Member Author

sjohnr commented Nov 21, 2022

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
Copy link
Contributor

mraible commented Nov 21, 2022

@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)

mraible pushed a commit to jhipster/generator-jhipster that referenced this issue Nov 21, 2022
mraible pushed a commit to jhipster/generator-jhipster that referenced this issue Nov 21, 2022
@sjohnr
Copy link
Member Author

sjohnr commented Nov 21, 2022

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
Copy link
Contributor

mraible commented Nov 21, 2022

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

@hannah23280
Copy link

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants