Skip to content

Commit a913f1c

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 a913f1c

9 files changed

+112
-36
lines changed

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

+3
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

+2-2
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

+12-13
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
}
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/csrf/XorCsrfTokenRequestAttributeHandler.java

+5-10
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ public void handle(HttpServletRequest request, HttpServletResponse response,
5959
Assert.notNull(response, "response cannot be null");
6060
Assert.notNull(deferredCsrfToken, "deferredCsrfToken cannot be null");
6161
Supplier<CsrfToken> updatedCsrfToken = deferCsrfTokenUpdate(deferredCsrfToken);
62-
logger.trace(LogMessage.format("XOR CSRF token created and will be written to request attributes"));
6362
super.handle(request, response, updatedCsrfToken);
6463
}
6564

@@ -76,16 +75,10 @@ private Supplier<CsrfToken> deferCsrfTokenUpdate(Supplier<CsrfToken> csrfTokenSu
7675
public String resolveCsrfTokenValue(HttpServletRequest request, CsrfToken csrfToken) {
7776
String actualToken = super.resolveCsrfTokenValue(request, csrfToken);
7877
if (actualToken == null) {
79-
logger.trace(LogMessage.format("No CSRF token value found in request"));
8078
return null;
8179
}
8280
String tokenValue = getTokenValue(actualToken, csrfToken.getToken());
83-
if (tokenValue == null) {
84-
logger.trace(LogMessage.format("CSRF token validation failed"));
85-
}
86-
else {
87-
logger.trace(LogMessage.format("CSRF token successfully validated"));
88-
}
81+
8982
return tokenValue;
9083
}
9184

@@ -95,14 +88,16 @@ private static String getTokenValue(String actualToken, String token) {
9588
actualBytes = Base64.getUrlDecoder().decode(actualToken);
9689
}
9790
catch (Exception ex) {
98-
logger.trace(LogMessage.format("Failed to find CSRF token since Base64 decoding failed"), ex);
91+
logger.trace(LogMessage.format("Not returning the CSRF token since it's not Base64-encoded"), ex);
9992
return null;
10093
}
10194

10295
byte[] tokenBytes = Utf8.encode(token);
10396
int tokenSize = tokenBytes.length;
10497
if (actualBytes.length != tokenSize * 2) {
105-
logger.trace(LogMessage.format("Failed to validate CSRF token since token length is invalid"));
98+
logger.trace(LogMessage.format(
99+
"Not returning the CSRF token since its Base64-decoded length (%d) is not equal to (%d)",
100+
actualBytes.length, tokenSize * 2));
106101
return null;
107102
}
108103

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

+9-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -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;
@@ -31,17 +34,22 @@
3134
* resolving the token value as either a form data value or header of the request.
3235
*
3336
* @author Steve Riesenberg
37+
* @author Yoobin Yoon
3438
* @since 5.8
3539
*/
3640
public class ServerCsrfTokenRequestAttributeHandler implements ServerCsrfTokenRequestHandler {
3741

42+
private static final Log logger = LogFactory.getLog(ServerCsrfTokenRequestAttributeHandler.class);
43+
3844
private boolean isTokenFromMultipartDataEnabled;
3945

4046
@Override
4147
public void handle(ServerWebExchange exchange, Mono<CsrfToken> csrfToken) {
4248
Assert.notNull(exchange, "exchange cannot be null");
4349
Assert.notNull(csrfToken, "csrfToken cannot be null");
4450
exchange.getAttributes().put(CsrfToken.class.getName(), csrfToken);
51+
logger.trace(LogMessage.format("Wrote a CSRF token to the following exchange attributes: [%s]",
52+
CsrfToken.class.getName()));
4553
}
4654

4755
@Override

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

+18-3
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,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.server.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 ServerCsrfTokenRequestHandler}
24+
*
25+
* @since 5.8
26+
*/
27+
class ServerCsrfTokenRequestHandlerLoggerHolder {
28+
29+
static final Log logger = LogFactory.getLog(ServerCsrfTokenRequestHandler.class);
30+
31+
}

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

+1-7
Original file line numberDiff line numberDiff line change
@@ -63,19 +63,13 @@ public void handle(ServerWebExchange exchange, Mono<CsrfToken> csrfToken) {
6363
createXoredCsrfToken(this.secureRandom, token.getToken())))
6464
.cast(CsrfToken.class)
6565
.cache();
66-
logger.trace(LogMessage.format("XOR CSRF token created and will be written to exchange attributes"));
6766
super.handle(exchange, updatedCsrfToken);
6867
}
6968

7069
@Override
7170
public Mono<String> resolveCsrfTokenValue(ServerWebExchange exchange, CsrfToken csrfToken) {
7271
return super.resolveCsrfTokenValue(exchange, csrfToken)
73-
.flatMap((actualToken) -> Mono.justOrEmpty(getTokenValue(actualToken, csrfToken.getToken())))
74-
.doOnNext(tokenValue -> {
75-
if (tokenValue != null) {
76-
logger.trace(LogMessage.format("CSRF token successfully validated"));
77-
}
78-
});
72+
.flatMap((actualToken) -> Mono.justOrEmpty(getTokenValue(actualToken, csrfToken.getToken())));
7973
}
8074

8175
private static String getTokenValue(String actualToken, String token) {

0 commit comments

Comments
 (0)