Browse Source

Restore Previous Behavior for Servlet 5

Closes gh-16173
Josh Cummings 8 months ago
parent
commit
96a9cf0d2d

+ 14 - 3
web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2023 the original author or authors.
+ * Copyright 2012-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -23,6 +23,7 @@ import jakarta.servlet.http.Cookie;
 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;
@@ -97,8 +98,18 @@ public final class CookieCsrfTokenRepository implements CsrfTokenRepository {
 
 		this.cookieCustomizer.accept(cookieBuilder);
 
-		Cookie cookie = mapToCookie(cookieBuilder.build());
-		response.addCookie(cookie);
+		ResponseCookie responseCookie = cookieBuilder.build();
+		if (!StringUtils.hasLength(responseCookie.getSameSite())) {
+			Cookie cookie = mapToCookie(responseCookie);
+			response.addCookie(cookie);
+		}
+		else if (request.getServletContext().getMajorVersion() > 5) {
+			Cookie cookie = mapToCookie(responseCookie);
+			response.addCookie(cookie);
+		}
+		else {
+			response.addHeader(HttpHeaders.SET_COOKIE, responseCookie.toString());
+		}
 
 		// Set request attribute to signal that response has blank cookie value,
 		// which allows loadToken to return null when token has been removed

+ 43 - 1
web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,16 +17,20 @@
 package org.springframework.security.web.csrf;
 
 import jakarta.servlet.http.Cookie;
+import jakarta.servlet.http.HttpServletResponse;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import org.springframework.http.HttpHeaders;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.mock.web.MockServletContext;
 
 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.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.springframework.security.web.csrf.CsrfTokenAssert.assertThatCsrfToken;
@@ -447,6 +451,44 @@ class CookieCsrfTokenRepositoryTests {
 		assertThat(tokenCookie.isHttpOnly()).isEqualTo(Boolean.FALSE);
 	}
 
+	// gh-16173
+	@Test
+	void saveTokenWhenSameSiteAndServletVersion5ThenUsesAddHeader() {
+		HttpServletResponse response = mock(HttpServletResponse.class);
+		((MockServletContext) this.request.getServletContext()).setMajorVersion(5);
+		this.repository.setCookieCustomizer((builder) -> builder.sameSite("Strict"));
+		CsrfToken token = this.repository.generateToken(this.request);
+		this.repository.saveToken(token, this.request, response);
+		verify(response, never()).addCookie(any(Cookie.class));
+		verify(response).addHeader(any(), any());
+	}
+
+	// gh-16173
+	@Test
+	void saveTokenWhenSameSiteAndServletVersion6OrHigherThenUsesAddCookie() {
+		HttpServletResponse response = mock(HttpServletResponse.class);
+		this.repository.setCookieCustomizer((builder) -> builder.sameSite("Strict"));
+		CsrfToken token = this.repository.generateToken(this.request);
+		this.repository.saveToken(token, this.request, response);
+		verify(response).addCookie(any(Cookie.class));
+		verify(response, never()).addHeader(any(), any());
+	}
+
+	// gh-16173
+	@Test
+	void saveTokenWhenNoSameSiteThenUsesAddCookie() {
+		HttpServletResponse response = mock(HttpServletResponse.class);
+		CsrfToken token = this.repository.generateToken(this.request);
+		this.repository.saveToken(token, this.request, response);
+		verify(response).addCookie(any(Cookie.class));
+		verify(response, never()).addHeader(any(), any());
+		((MockServletContext) this.request.getServletContext()).setMajorVersion(5);
+		response = mock(HttpServletResponse.class);
+		this.repository.saveToken(token, this.request, response);
+		verify(response).addCookie(any(Cookie.class));
+		verify(response, never()).addHeader(any(), any());
+	}
+
 	@Test
 	void setCookieNameNullIllegalArgumentException() {
 		assertThatIllegalArgumentException().isThrownBy(() -> this.repository.setCookieName(null));