spring-security icon indicating copy to clipboard operation
spring-security copied to clipboard

LogoutConfigurer forces POST even if CSRF is disabled for /logout

Open erizzo opened this issue 10 months ago • 3 comments

With a configuration that includes CSRF protection (even customized), LogoutConfigurer assumes that /logout will only match POST requests (code in question here as of the time of this writing). That's not a problem in general, but if /logout has been configured to NOT match CSRF protection, it causes confusion and violates the principal of least surprise.

For example, with a custom CsrfConfigurer like csrf.ignoringRequestMatchers("/logout"); it's very likely that the developer intends to allow GET requests for /logout (what other reason would there be for ignoring /logout in CSRF?). That was certainly my expectation in configuring it that way.

However the logic in LogoutConfigurer.createLogoutRequestMatcher(H) requires POST if any CsrfConfigurer is present, ignoring the fact that CSRF is ignored for /logout.

private RequestMatcher createLogoutRequestMatcher(H http) {
	RequestMatcher post = createLogoutRequestMatcher("POST");
	if (http.getConfigurer(CsrfConfigurer.class) != null) {
		return post;
	}
	...
}

The workaround is to specify a request matcher for the LogoutConfigurer, but that's not obvious (I spent a couple of hours debugging through the Spring Security code to find what was happening and where it came from - a situation made more painful by the fact that you have to know which log categories to set to TRACE to get any info about why /logout isn't "available").

I'm not sure if this is considered a bug or if it was an intentional choice (and thus an enhancement request), but either way it seems like it would be easy to adjust the logic to look in the CSRF config to see if /logout (or whatever the configured logout URL is) is ignored and, if so, allow other HTTP methods for it. Alternatively, add something to the Logout DSL to make it simple to accomplish.

erizzo avatar Apr 15 '24 20:04 erizzo