Wrong logging for CsrfFilter in trace level

[everflux]

Describe the bug When trace logging is active a simple GET request that does not require CSRF protection logs the following:

Did not protect against CSRF since request did not match CsrfNotRequired [TRACE, HEAD, GET, OPTIONS]

But it is indeed a GET request.

To Reproduce Enable spring security, use trace level logging, perform GET request.

Expected behavior Log message should state the correct condition.

Sample Problem is in https://github.com/spring-projects/spring-security/blob/e1d8033ee39d04bde0f58d70bce3a262ecbd1d9d/web/src/main/java/org/springframework/security/web/csrf/CsrfFilter.java#L114

The logic

if (!this.requireCsrfProtectionMatcher.matches(request)) {
			if (this.logger.isTraceEnabled()) {
				this.logger.trace("Did not protect against CSRF since request did not match "
						+ this.requireCsrfProtectionMatcher);
			}
			filterChain.doFilter(request, response);
			return;
		}

matches the intended log message, but the log message uses the toString method of DefaultRequiresCsrfMatcher which references allowed methods and the matcher again negates the condition, leading to a mismatch between output and behaviour.

		@Override
		public String toString() {
			return "CsrfNotRequired " + this.allowedMethods;
		}

[jzheaux]

Hi, @everflux, thanks for reaching out. I agree that this message could be confusing and could use a change.

I believe the issue is that it correctly states that it won't protect, but it's reasoning is hard to parse. The following would be more correct:

Did not protect against CSRF since request did not match IsNotHttpMethod [TRACE, HEAD, GET, OPTIONS]

In this way, if someone configures a custom request matcher, the message can still make sense:

Did not protect against CSRF since request did not match HttpMethod [POST]

However, since there are a lot of negations in the default, we could use a simpler error message when the class is using the default DefaultRequestCsrfMatcher:

Did not protect against CSRF since IsNotHttpMethod [TRACE, HEAD, GET, OPTIONS]

Are you able to contribute a PR that simplifies the log message when the filter is using its default request matcher? It would also be nice to change the toString to use IsNotHttpMethod.

[dineshgupta630]

Since @everflux raised the issue, I completely understand if they’d like to take the lead — but if they’re unavailable or prefer not to, I’d be happy to help with a PR. Thanks!

[everflux]

Please do, I am not eager to be responsible at the moment.

[jzheaux]

Thanks @everflux for raising the issue and @dineshgupta630 for volunteering a PR!

I'll close this in favor of reviewing https://github.com/spring-projects/spring-security/pull/17425