Browse Source

Use addCookie instead of addHeader in CookieCsrfTokenRepository

By using addCookie we make sure that configured Tomcat's CookieProcessors are invoked

Closes gh-14131
pull/14119/head
Marcus Hert Da Coregio 2 years ago
parent
commit
921afba134
  1. 19
      web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java
  2. 21
      web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java

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

@ -23,7 +23,6 @@ import jakarta.servlet.http.Cookie; @@ -23,7 +23,6 @@ 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;
@ -98,7 +97,8 @@ public final class CookieCsrfTokenRepository implements CsrfTokenRepository { @@ -98,7 +97,8 @@ public final class CookieCsrfTokenRepository implements CsrfTokenRepository {
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 @@ public final class CookieCsrfTokenRepository implements CsrfTokenRepository { @@ -187,6 +187,21 @@ public final class CookieCsrfTokenRepository implements CsrfTokenRepository {
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.

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

@ -21,12 +21,14 @@ import org.junit.jupiter.api.BeforeEach; @@ -21,12 +21,14 @@ import org.junit.jupiter.api.BeforeEach;
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 @@ class CookieCsrfTokenRepositoryTests { @@ -85,6 +87,15 @@ class CookieCsrfTokenRepositoryTests {
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 @@ class CookieCsrfTokenRepositoryTests { @@ -268,7 +279,7 @@ class CookieCsrfTokenRepositoryTests {
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 @@ class CookieCsrfTokenRepositoryTests { @@ -278,7 +289,7 @@ class CookieCsrfTokenRepositoryTests {
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 @@ class CookieCsrfTokenRepositoryTests { @@ -288,7 +299,7 @@ class CookieCsrfTokenRepositoryTests {
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 @@ class CookieCsrfTokenRepositoryTests { @@ -420,7 +431,7 @@ class CookieCsrfTokenRepositoryTests {
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

Loading…
Cancel
Save