Improve CSRF token handling and logging

Align log messages between servlet and reactive implementations

Signed-off-by: yybmion <[email protected]>

Author: yybmion

web/src/main/java/org/springframework/security/web/csrf/CsrfTokenRequestAttributeHandler.java

@@ -20,9 +20,9 @@
 
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
-
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+
 import org.springframework.core.log.LogMessage;
 import org.springframework.util.Assert;
 

web/src/main/java/org/springframework/security/web/csrf/CsrfTokenRequestHandler.java

@@ -16,9 +16,11 @@
 
 package org.springframework.security.web.csrf;
 
+import java.util.function.Supplier;
+
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
-import java.util.function.Supplier;
+
 import org.springframework.core.log.LogMessage;
 import org.springframework.util.Assert;
 

web/src/main/java/org/springframework/security/web/csrf/CsrfTokenRequestHandlerLoggerHolder.java

@@ -21,11 +21,12 @@
 
 /**
  * Utility class for holding the logger for {@link CsrfTokenRequestHandler}
- *
- * @since 5.8
  */
-class CsrfTokenRequestHandlerLoggerHolder {
+final class CsrfTokenRequestHandlerLoggerHolder {
 
 	static final Log logger = LogFactory.getLog(CsrfTokenRequestHandler.class);
 
+	private CsrfTokenRequestHandlerLoggerHolder() {
+	}
+
 }

web/src/main/java/org/springframework/security/web/csrf/XorCsrfTokenRequestAttributeHandler.java

@@ -16,13 +16,15 @@
 
 package org.springframework.security.web.csrf;
 
-import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.http.HttpServletResponse;
 import java.security.SecureRandom;
 import java.util.Base64;
 import java.util.function.Supplier;
+
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+
 import org.springframework.core.log.LogMessage;
 import org.springframework.security.crypto.codec.Utf8;
 import org.springframework.util.Assert;
@@ -77,9 +79,7 @@ public String resolveCsrfTokenValue(HttpServletRequest request, CsrfToken csrfTo
 		if (actualToken == null) {
 			return null;
 		}
-		String tokenValue = getTokenValue(actualToken, csrfToken.getToken());
-
-		return tokenValue;
+		return getTokenValue(actualToken, csrfToken.getToken());
 	}
 
 	private static String getTokenValue(String actualToken, String token) {

web/src/main/java/org/springframework/security/web/server/csrf/ServerCsrfTokenRequestAttributeHandler.java

@@ -18,9 +18,9 @@
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-import org.springframework.core.log.LogMessage;
 import reactor.core.publisher.Mono;
 
+import org.springframework.core.log.LogMessage;
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.MediaType;
 import org.springframework.http.codec.multipart.FormFieldPart;
@@ -48,8 +48,7 @@ public void handle(ServerWebExchange exchange, Mono<CsrfToken> csrfToken) {
 		Assert.notNull(exchange, "exchange cannot be null");
 		Assert.notNull(csrfToken, "csrfToken cannot be null");
 		exchange.getAttributes().put(CsrfToken.class.getName(), csrfToken);
-		logger.trace(LogMessage.format("Wrote a CSRF token to the following exchange attributes: [%s]",
-				CsrfToken.class.getName()));
+		logger.trace(LogMessage.format("Wrote a CSRF token to the [%s] exchange attribute", CsrfToken.class.getName()));
 	}
 
 	@Override

web/src/main/java/org/springframework/security/web/server/csrf/ServerCsrfTokenRequestHandler.java

@@ -16,9 +16,9 @@
 
 package org.springframework.security.web.server.csrf;
 
-import org.springframework.core.log.LogMessage;
 import reactor.core.publisher.Mono;
 
+import org.springframework.core.log.LogMessage;
 import org.springframework.util.Assert;
 import org.springframework.web.server.ServerWebExchange;
 
@@ -47,21 +47,25 @@ public interface ServerCsrfTokenRequestHandler extends ServerCsrfTokenRequestRes
 	default Mono<String> resolveCsrfTokenValue(ServerWebExchange exchange, CsrfToken csrfToken) {
 		Assert.notNull(exchange, "exchange cannot be null");
 		Assert.notNull(csrfToken, "csrfToken cannot be null");
+
+		String headerName = csrfToken.getHeaderName();
+		String parameterName = csrfToken.getParameterName();
+
 		return exchange.getFormData().flatMap((data) -> {
-			String token = data.getFirst(csrfToken.getParameterName());
+			String token = data.getFirst(parameterName);
 			if (token != null) {
 				return Mono.just(token);
 			}
-			ServerCsrfTokenRequestHandlerLoggerHolder.logger.trace(LogMessage
-				.format("Did not find a CSRF token in the [%s] request parameter", csrfToken.getParameterName()));
+			ServerCsrfTokenRequestHandlerLoggerHolder.logger
+				.trace(LogMessage.format("Did not find a CSRF token in the [%s] request parameter", parameterName));
 			return Mono.empty();
 		}).switchIfEmpty(Mono.defer(() -> {
-			String token = exchange.getRequest().getHeaders().getFirst(csrfToken.getHeaderName());
+			String token = exchange.getRequest().getHeaders().getFirst(headerName);
 			if (token != null) {
 				return Mono.just(token);
 			}
-			ServerCsrfTokenRequestHandlerLoggerHolder.logger.trace(LogMessage
-				.format("Did not find a CSRF token in the [%s] request header", csrfToken.getHeaderName()));
+			ServerCsrfTokenRequestHandlerLoggerHolder.logger
+				.trace(LogMessage.format("Did not find a CSRF token in the [%s] request header", headerName));
 			return Mono.empty();
 		}));
 	}

web/src/main/java/org/springframework/security/web/server/csrf/ServerCsrfTokenRequestHandlerLoggerHolder.java

@@ -21,11 +21,12 @@
 
 /**
  * Utility class for holding the logger for {@link ServerCsrfTokenRequestHandler}
- *
- * @since 5.8
  */
-class ServerCsrfTokenRequestHandlerLoggerHolder {
+final class ServerCsrfTokenRequestHandlerLoggerHolder {
 
 	static final Log logger = LogFactory.getLog(ServerCsrfTokenRequestHandler.class);
 
+	private ServerCsrfTokenRequestHandlerLoggerHolder() {
+	}
+
 }

web/src/main/java/org/springframework/security/web/server/csrf/XorServerCsrfTokenRequestAttributeHandler.java

@@ -21,9 +21,9 @@
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-import org.springframework.core.log.LogMessage;
 import reactor.core.publisher.Mono;
 
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.crypto.codec.Utf8;
 import org.springframework.util.Assert;
 import org.springframework.web.server.ServerWebExchange;
@@ -78,14 +78,16 @@ private static String getTokenValue(String actualToken, String token) {
 			actualBytes = Base64.getUrlDecoder().decode(actualToken);
 		}
 		catch (Exception ex) {
-			logger.trace(LogMessage.format("Failed to find CSRF token since Base64 decoding failed"), ex);
+			logger.trace(LogMessage.format("Not returning the CSRF token since it's not Base64-encoded"), ex);
 			return null;
 		}
 
 		byte[] tokenBytes = Utf8.encode(token);
 		int tokenSize = tokenBytes.length;
 		if (actualBytes.length != tokenSize * 2) {
-			logger.trace(LogMessage.format("Failed to validate CSRF token since token length is invalid"));
+			logger.trace(LogMessage.format(
+					"Not returning the CSRF token since its Base64-decoded length (%d) is not equal to (%d)",
+					actualBytes.length, tokenSize * 2));
 			return null;
 		}