spring-projects / spring-projects/spring-security

LogoutConfigurer forces POST even if CSRF is disabled for /logout

Open
#14,913 3 comments 0 reactions 1 assignee View on GitHub

@sjohnr is already working on this.

Since Jun 13, 2024.

in: config status: feedback-provided type: enhancement
Dominant language
Java
Stars
9.6k
Forks
6.3k
Avg merge
2d 11h
Merged PRs (30d)
52

Description

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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.