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

spring-security/web/src/main/java/org/springframework/security/web/csrf/CsrfFilter.java

Line 114 in e1d8033

this.logger.trace("Did not protect against CSRF since request did not match "

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.