Skip to content

Commit 921afba

Browse files
Use addCookie instead of addHeader in CookieCsrfTokenRepository
By using addCookie we make sure that configured Tomcat's CookieProcessors are invoked Closes gh-14131
1 parent cf76864 commit 921afba

File tree

2 files changed

+33
-7
lines changed

2 files changed

+33
-7
lines changed

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import jakarta.servlet.http.HttpServletRequest;
2424
import jakarta.servlet.http.HttpServletResponse;
2525

26-
import org.springframework.http.HttpHeaders;
2726
import org.springframework.http.ResponseCookie;
2827
import org.springframework.util.Assert;
2928
import org.springframework.util.StringUtils;
@@ -98,7 +97,8 @@ public void saveToken(CsrfToken token, HttpServletRequest request, HttpServletRe
9897

9998
this.cookieCustomizer.accept(cookieBuilder);
10099

101-
response.addHeader(HttpHeaders.SET_COOKIE, cookieBuilder.build().toString());
100+
Cookie cookie = mapToCookie(cookieBuilder.build());
101+
response.addCookie(cookie);
102102

103103
// Set request attribute to signal that response has blank cookie value,
104104
// which allows loadToken to return null when token has been removed
@@ -187,6 +187,21 @@ private String createNewToken() {
187187
return UUID.randomUUID().toString();
188188
}
189189

190+
private Cookie mapToCookie(ResponseCookie responseCookie) {
191+
Cookie cookie = new Cookie(responseCookie.getName(), responseCookie.getValue());
192+
cookie.setSecure(responseCookie.isSecure());
193+
cookie.setPath(responseCookie.getPath());
194+
cookie.setMaxAge((int) responseCookie.getMaxAge().getSeconds());
195+
cookie.setHttpOnly(responseCookie.isHttpOnly());
196+
if (StringUtils.hasLength(responseCookie.getDomain())) {
197+
cookie.setDomain(responseCookie.getDomain());
198+
}
199+
if (StringUtils.hasText(responseCookie.getSameSite())) {
200+
cookie.setAttribute("SameSite", responseCookie.getSameSite());
201+
}
202+
return cookie;
203+
}
204+
190205
/**
191206
* Set the path that the Cookie will be created with. This will override the default
192207
* functionality which uses the request context as the path.

web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,14 @@
2121
import org.junit.jupiter.api.Test;
2222

2323
import org.springframework.http.HttpHeaders;
24-
import org.springframework.mock.web.MockCookie;
2524
import org.springframework.mock.web.MockHttpServletRequest;
2625
import org.springframework.mock.web.MockHttpServletResponse;
2726

2827
import static org.assertj.core.api.Assertions.assertThat;
2928
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
29+
import static org.mockito.ArgumentMatchers.any;
30+
import static org.mockito.Mockito.spy;
31+
import static org.mockito.Mockito.verify;
3032
import static org.springframework.security.web.csrf.CsrfTokenAssert.assertThatCsrfToken;
3133

3234
/**
@@ -85,6 +87,15 @@ void saveToken() {
8587
assertThat(tokenCookie.isHttpOnly()).isTrue();
8688
}
8789

90+
// gh-14131
91+
@Test
92+
void saveTokenShouldUseResponseAddCookie() {
93+
CsrfToken token = this.repository.generateToken(this.request);
94+
MockHttpServletResponse spyResponse = spy(this.response);
95+
this.repository.saveToken(token, this.request, spyResponse);
96+
verify(spyResponse).addCookie(any(Cookie.class));
97+
}
98+
8899
@Test
89100
void saveTokenSecure() {
90101
this.request.setSecure(true);
@@ -268,7 +279,7 @@ void saveTokenWithSameSiteNull() {
268279
CsrfToken token = this.repository.generateToken(this.request);
269280
this.repository.saveToken(token, this.request, this.response);
270281
Cookie tokenCookie = this.response.getCookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME);
271-
assertThat(((MockCookie) tokenCookie).getSameSite()).isNull();
282+
assertThat(tokenCookie.getAttribute("SameSite")).isNull();
272283
}
273284

274285
@Test
@@ -278,7 +289,7 @@ void saveTokenWithSameSiteStrict() {
278289
CsrfToken token = this.repository.generateToken(this.request);
279290
this.repository.saveToken(token, this.request, this.response);
280291
Cookie tokenCookie = this.response.getCookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME);
281-
assertThat(((MockCookie) tokenCookie).getSameSite()).isEqualTo(sameSitePolicy);
292+
assertThat(tokenCookie.getAttribute("SameSite")).isEqualTo(sameSitePolicy);
282293
}
283294

284295
@Test
@@ -288,7 +299,7 @@ void saveTokenWithSameSiteLax() {
288299
CsrfToken token = this.repository.generateToken(this.request);
289300
this.repository.saveToken(token, this.request, this.response);
290301
Cookie tokenCookie = this.response.getCookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME);
291-
assertThat(((MockCookie) tokenCookie).getSameSite()).isEqualTo(sameSitePolicy);
302+
assertThat(tokenCookie.getAttribute("SameSite")).isEqualTo(sameSitePolicy);
292303
}
293304

294305
// gh-13075
@@ -420,7 +431,7 @@ void cookieCustomizer() {
420431
assertThat(tokenCookie.getDomain()).isEqualTo(domainName);
421432
assertThat(tokenCookie.getPath()).isEqualTo(customPath);
422433
assertThat(tokenCookie.isHttpOnly()).isEqualTo(Boolean.TRUE);
423-
assertThat(((MockCookie) tokenCookie).getSameSite()).isEqualTo(sameSitePolicy);
434+
assertThat(tokenCookie.getAttribute("SameSite")).isEqualTo(sameSitePolicy);
424435
}
425436

426437
// gh-13659

0 commit comments

Comments
 (0)