Skip to content

Commit ff9dedf

Browse files
committed
Improve CSRF token logging and code organization
Improve CSRF logging: consistent formatting, reduced nesting, proper encapsulation, and removal of redundant messages Signed-off-by: yybmion <[email protected]>
1 parent 1636ffd commit ff9dedf

7 files changed

+88
-18
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,9 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
119119
}
120120
CsrfToken csrfToken = deferredCsrfToken.get();
121121
String actualToken = this.requestHandler.resolveCsrfTokenValue(request, csrfToken);
122+
if (actualToken != null && this.logger.isTraceEnabled()) {
123+
this.logger.trace(LogMessage.format("Found a CSRF token in the request"));
124+
}
122125
if (!equalsConstantTime(csrfToken.getToken(), actualToken)) {
123126
boolean missingToken = deferredCsrfToken.isGenerated();
124127
this.logger

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ public void handle(HttpServletRequest request, HttpServletResponse response,
6767
: csrfToken.getParameterName();
6868
request.setAttribute(csrfAttrName, csrfToken);
6969

70-
logger.trace(LogMessage.format("CSRF token written to request attributes: %s, %s", CsrfToken.class.getName(),
71-
csrfAttrName));
70+
logger.trace(LogMessage.format("Wrote a CSRF token to the following request attributes: [%s, %s]", csrfAttrName,
71+
CsrfToken.class.getName()));
7272
}
7373

7474
@SuppressWarnings("serial")

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

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919
import jakarta.servlet.http.HttpServletRequest;
2020
import jakarta.servlet.http.HttpServletResponse;
2121
import java.util.function.Supplier;
22-
import org.apache.commons.logging.Log;
23-
import org.apache.commons.logging.LogFactory;
2422
import org.springframework.core.log.LogMessage;
2523
import org.springframework.util.Assert;
2624

@@ -38,8 +36,6 @@
3836
@FunctionalInterface
3937
public interface CsrfTokenRequestHandler extends CsrfTokenRequestResolver {
4038

41-
Log logger = LogFactory.getLog(CsrfTokenRequestHandler.class);
42-
4339
/**
4440
* Handles a request using a {@link CsrfToken}.
4541
* @param request the {@code HttpServletRequest} being handled
@@ -53,17 +49,20 @@ default String resolveCsrfTokenValue(HttpServletRequest request, CsrfToken csrfT
5349
Assert.notNull(request, "request cannot be null");
5450
Assert.notNull(csrfToken, "csrfToken cannot be null");
5551
String actualToken = request.getHeader(csrfToken.getHeaderName());
56-
if (actualToken == null) {
57-
actualToken = request.getParameter(csrfToken.getParameterName());
58-
if (actualToken != null) {
59-
logger.trace(
60-
LogMessage.format("CSRF token found in request parameter: %s", csrfToken.getParameterName()));
61-
}
52+
if (actualToken != null) {
53+
return actualToken;
6254
}
63-
else {
64-
logger.trace(LogMessage.format("CSRF token found in request header: %s", csrfToken.getHeaderName()));
55+
CsrfTokenRequestHandlerLoggerHolder.logger.trace(
56+
LogMessage.format("Did not find a CSRF token in the [%s] request header", csrfToken.getHeaderName()));
57+
58+
actualToken = request.getParameter(csrfToken.getParameterName());
59+
if (actualToken != null) {
60+
return actualToken;
6561
}
66-
return actualToken;
62+
CsrfTokenRequestHandlerLoggerHolder.logger.trace(LogMessage
63+
.format("Did not find a CSRF token in the [%s] request parameter", csrfToken.getParameterName()));
64+
65+
return null;
6766
}
6867

6968
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
* Copyright 2002-2025 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.security.web.csrf;
18+
19+
import org.apache.commons.logging.Log;
20+
import org.apache.commons.logging.LogFactory;
21+
22+
/**
23+
* Utility class for holding the logger for {@link CsrfTokenRequestHandler}
24+
*
25+
* @since 5.8
26+
*/
27+
class CsrfTokenRequestHandlerLoggerHolder {
28+
29+
static final Log logger = LogFactory.getLog(CsrfTokenRequestHandler.class);
30+
31+
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616

1717
package org.springframework.security.web.server.csrf;
1818

19+
import org.apache.commons.logging.Log;
20+
import org.apache.commons.logging.LogFactory;
21+
import org.springframework.core.log.LogMessage;
1922
import reactor.core.publisher.Mono;
2023

2124
import org.springframework.http.HttpHeaders;
@@ -35,13 +38,17 @@
3538
*/
3639
public class ServerCsrfTokenRequestAttributeHandler implements ServerCsrfTokenRequestHandler {
3740

41+
private static final Log logger = LogFactory.getLog(ServerCsrfTokenRequestAttributeHandler.class);
42+
3843
private boolean isTokenFromMultipartDataEnabled;
3944

4045
@Override
4146
public void handle(ServerWebExchange exchange, Mono<CsrfToken> csrfToken) {
4247
Assert.notNull(exchange, "exchange cannot be null");
4348
Assert.notNull(csrfToken, "csrfToken cannot be null");
4449
exchange.getAttributes().put(CsrfToken.class.getName(), csrfToken);
50+
logger.trace(LogMessage.format("Wrote a CSRF token to the following exchange attributes: [%s]",
51+
CsrfToken.class.getName()));
4552
}
4653

4754
@Override

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

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package org.springframework.security.web.server.csrf;
1818

19+
import org.springframework.core.log.LogMessage;
1920
import reactor.core.publisher.Mono;
2021

2122
import org.springframework.util.Assert;
@@ -46,9 +47,23 @@ public interface ServerCsrfTokenRequestHandler extends ServerCsrfTokenRequestRes
4647
default Mono<String> resolveCsrfTokenValue(ServerWebExchange exchange, CsrfToken csrfToken) {
4748
Assert.notNull(exchange, "exchange cannot be null");
4849
Assert.notNull(csrfToken, "csrfToken cannot be null");
49-
return exchange.getFormData()
50-
.flatMap((data) -> Mono.justOrEmpty(data.getFirst(csrfToken.getParameterName())))
51-
.switchIfEmpty(Mono.justOrEmpty(exchange.getRequest().getHeaders().getFirst(csrfToken.getHeaderName())));
50+
return exchange.getFormData().flatMap((data) -> {
51+
String token = data.getFirst(csrfToken.getParameterName());
52+
if (token != null) {
53+
return Mono.just(token);
54+
}
55+
ServerCsrfTokenRequestHandlerLoggerHolder.logger.trace(LogMessage
56+
.format("Did not find a CSRF token in the [%s] request parameter", csrfToken.getParameterName()));
57+
return Mono.empty();
58+
}).switchIfEmpty(Mono.defer(() -> {
59+
String token = exchange.getRequest().getHeaders().getFirst(csrfToken.getHeaderName());
60+
if (token != null) {
61+
return Mono.just(token);
62+
}
63+
ServerCsrfTokenRequestHandlerLoggerHolder.logger.trace(LogMessage
64+
.format("Did not find a CSRF token in the [%s] request header", csrfToken.getHeaderName()));
65+
return Mono.empty();
66+
}));
5267
}
5368

5469
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package org.springframework.security.web.server.csrf;
2+
3+
import org.apache.commons.logging.Log;
4+
import org.apache.commons.logging.LogFactory;
5+
6+
/**
7+
* Utility class for holding the logger for {@link ServerCsrfTokenRequestHandler}
8+
*
9+
* @since 5.8
10+
*/
11+
class ServerCsrfTokenRequestHandlerLoggerHolder {
12+
13+
static final Log logger = LogFactory.getLog(ServerCsrfTokenRequestHandler.class);
14+
15+
}

0 commit comments

Comments
 (0)