Merge branch '6.1.x'

Closes gh-14138
spring-projects · Nov 14, 2023 · e920bfb · e920bfb
2 parents a1cab16 + 921afba
commit e920bfb
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 7 deletions.
diff --git a/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java b/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java
@@ -23,7 +23,6 @@
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
 
-import org.springframework.http.HttpHeaders;
 import org.springframework.http.ResponseCookie;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
@@ -98,7 +97,8 @@ public void saveToken(CsrfToken token, HttpServletRequest request, HttpServletRe
 
 		this.cookieCustomizer.accept(cookieBuilder);
 
-		response.addHeader(HttpHeaders.SET_COOKIE, cookieBuilder.build().toString());
+		Cookie cookie = mapToCookie(cookieBuilder.build());
+		response.addCookie(cookie);
 
 		// Set request attribute to signal that response has blank cookie value,
 		// which allows loadToken to return null when token has been removed
@@ -187,6 +187,21 @@ private String createNewToken() {
 		return UUID.randomUUID().toString();
 	}
 
+	private Cookie mapToCookie(ResponseCookie responseCookie) {
+		Cookie cookie = new Cookie(responseCookie.getName(), responseCookie.getValue());
+		cookie.setSecure(responseCookie.isSecure());
+		cookie.setPath(responseCookie.getPath());
+		cookie.setMaxAge((int) responseCookie.getMaxAge().getSeconds());
+		cookie.setHttpOnly(responseCookie.isHttpOnly());
+		if (StringUtils.hasLength(responseCookie.getDomain())) {
+			cookie.setDomain(responseCookie.getDomain());
+		}
+		if (StringUtils.hasText(responseCookie.getSameSite())) {
+			cookie.setAttribute("SameSite", responseCookie.getSameSite());
+		}
+		return cookie;
+	}
+
 	/**
 	 * Set the path that the Cookie will be created with. This will override the default
 	 * functionality which uses the request context as the path.

diff --git a/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java b/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java
@@ -21,12 +21,14 @@
 import org.junit.jupiter.api.Test;
 
 import org.springframework.http.HttpHeaders;
-import org.springframework.mock.web.MockCookie;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
 import static org.springframework.security.web.csrf.CsrfTokenAssert.assertThatCsrfToken;
 
 /**
@@ -85,6 +87,15 @@ void saveToken() {
 		assertThat(tokenCookie.isHttpOnly()).isTrue();
 	}
 
+	// gh-14131
+	@Test
+	void saveTokenShouldUseResponseAddCookie() {
+		CsrfToken token = this.repository.generateToken(this.request);
+		MockHttpServletResponse spyResponse = spy(this.response);
+		this.repository.saveToken(token, this.request, spyResponse);
+		verify(spyResponse).addCookie(any(Cookie.class));
+	}
+
 	@Test
 	void saveTokenSecure() {
 		this.request.setSecure(true);
@@ -268,7 +279,7 @@ void saveTokenWithSameSiteNull() {
 		CsrfToken token = this.repository.generateToken(this.request);
 		this.repository.saveToken(token, this.request, this.response);
 		Cookie tokenCookie = this.response.getCookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME);
-		assertThat(((MockCookie) tokenCookie).getSameSite()).isNull();
+		assertThat(tokenCookie.getAttribute("SameSite")).isNull();
 	}
 
 	@Test
@@ -278,7 +289,7 @@ void saveTokenWithSameSiteStrict() {
 		CsrfToken token = this.repository.generateToken(this.request);
 		this.repository.saveToken(token, this.request, this.response);
 		Cookie tokenCookie = this.response.getCookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME);
-		assertThat(((MockCookie) tokenCookie).getSameSite()).isEqualTo(sameSitePolicy);
+		assertThat(tokenCookie.getAttribute("SameSite")).isEqualTo(sameSitePolicy);
 	}
 
 	@Test
@@ -288,7 +299,7 @@ void saveTokenWithSameSiteLax() {
 		CsrfToken token = this.repository.generateToken(this.request);
 		this.repository.saveToken(token, this.request, this.response);
 		Cookie tokenCookie = this.response.getCookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME);
-		assertThat(((MockCookie) tokenCookie).getSameSite()).isEqualTo(sameSitePolicy);
+		assertThat(tokenCookie.getAttribute("SameSite")).isEqualTo(sameSitePolicy);
 	}
 
 	// gh-13075
@@ -420,7 +431,7 @@ void cookieCustomizer() {
 		assertThat(tokenCookie.getDomain()).isEqualTo(domainName);
 		assertThat(tokenCookie.getPath()).isEqualTo(customPath);
 		assertThat(tokenCookie.isHttpOnly()).isEqualTo(Boolean.TRUE);
-		assertThat(((MockCookie) tokenCookie).getSameSite()).isEqualTo(sameSitePolicy);
+		assertThat(tokenCookie.getAttribute("SameSite")).isEqualTo(sameSitePolicy);
 	}
 
 	// gh-13659