From b9cd4c3421d3963b4ee426fe948c3e9823836415 Mon Sep 17 00:00:00 2001 From: Alex Montoya Date: Fri, 28 Oct 2022 23:25:30 -0700 Subject: [PATCH 1/6] Add the ability to set the SameSite policy to the CRSF Cookie Add a new public method to the class org.springframework.security.web.csrf.CookieCsrfTokenRepository that allows setting the SameSite policy as an optional parameter; with this, when sending the CSRF token in a cookie instead of a header, no warning or error should be displayed in the browser's console. Removes unnecessary default no-parameters empty constructor. Fix gh-12086 --- .../web/csrf/CookieCsrfTokenRepository.java | 37 +++++++++++++------ 1 file changed, 26 insertions(+), 11 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 index 62075e0d063..7dfa6178827 100644 --- 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,6 +23,7 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import org.springframework.http.ResponseCookie; import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.web.util.WebUtils; @@ -59,8 +60,7 @@ public final class CookieCsrfTokenRepository implements CsrfTokenRepository { private int cookieMaxAge = -1; - public CookieCsrfTokenRepository() { - } + private String sameSite = null; @Override public CsrfToken generateToken(HttpServletRequest request) { @@ -70,15 +70,18 @@ public CsrfToken generateToken(HttpServletRequest request) { @Override public void saveToken(CsrfToken token, HttpServletRequest request, HttpServletResponse response) { String tokenValue = (token != null) ? token.getToken() : ""; - Cookie cookie = new Cookie(this.cookieName, tokenValue); - cookie.setSecure((this.secure != null) ? this.secure : request.isSecure()); - cookie.setPath(StringUtils.hasLength(this.cookiePath) ? this.cookiePath : this.getRequestContext(request)); - cookie.setMaxAge((token != null) ? this.cookieMaxAge : 0); - cookie.setHttpOnly(this.cookieHttpOnly); - if (StringUtils.hasLength(this.cookieDomain)) { - cookie.setDomain(this.cookieDomain); - } - response.addCookie(cookie); + + ResponseCookie rCookie = ResponseCookie + .from(this.cookieName, tokenValue) + .secure(this.secure != null ? this.secure : request.isSecure()) + .path(StringUtils.hasLength(this.cookiePath) ? this.cookiePath : this.getRequestContext(request)) + .maxAge(token != null ? this.cookieMaxAge : 0) + .httpOnly(this.cookieHttpOnly) + .domain(this.cookieDomain) + .sameSite(this.sameSite) + .build(); + + response.setHeader(cookieName, rCookie.toString()); } @Override @@ -220,4 +223,16 @@ public void setCookieMaxAge(int cookieMaxAge) { this.cookieMaxAge = cookieMaxAge; } + /** + * Add the "SameSite" attribute to the generated cookie. + *

This limits the scope of the cookie such that it will only be + * attached to same site requests if {@code "Strict"} or cross-site + * requests if {@code "Lax"}. + * @since 6.1 + * @see RFC6265 bis + */ + public void setSameSite(String sameSite) { + this.sameSite = sameSite; + } + } From 777708d881bf412f529e597e6206d93b2ba13d5d Mon Sep 17 00:00:00 2001 From: Alex Montoya Date: Tue, 15 Nov 2022 00:49:22 -0800 Subject: [PATCH 2/6] - Remove un necessary new method to set same site policy. - Mark setCookieHttpOnly, setCookieDomain, setCookieMaxAge and setSecure as deprecated. - Add the method addCookieInitializer which allows to set properties to the ResponseCookieBuilder without having to add new setter methods. - Apply same changes to CookieServerCsrfTokenRepository. - Add unit tests to validate the use of same site policy. --- .../web/csrf/CookieCsrfTokenRepository.java | 85 +++++-------- .../csrf/CookieServerCsrfTokenRepository.java | 64 +++++----- .../csrf/CookieCsrfTokenRepositoryTests.java | 116 +++++++++++++----- .../CookieServerCsrfTokenRepositoryTests.java | 68 ++++++---- 4 files changed, 191 insertions(+), 142 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 index 7dfa6178827..ba1ddf57865 100644 --- a/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java +++ b/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java @@ -15,15 +15,16 @@ */ package org.springframework.security.web.csrf; - import java.util.UUID; +import java.util.function.Consumer; -import jakarta.servlet.ServletRequest; 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.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.web.util.WebUtils; @@ -60,7 +61,19 @@ public final class CookieCsrfTokenRepository implements CsrfTokenRepository { private int cookieMaxAge = -1; - private String sameSite = null; + @Nullable + private Consumer cookieInitializer = null; + + /** + * Add a {@link Consumer} for a {@code ResponseCookieBuilder} that will be invoked + * for each cookie being built, just before the call to {@code build()}. + * @param initializer consumer for a cookie builder + * @since 6.1 + */ + public void addCookieInitializer(Consumer initializer) { + this.cookieInitializer = this.cookieInitializer != null ? + this.cookieInitializer.andThen(initializer) : initializer; + } @Override public CsrfToken generateToken(HttpServletRequest request) { @@ -71,17 +84,18 @@ public CsrfToken generateToken(HttpServletRequest request) { public void saveToken(CsrfToken token, HttpServletRequest request, HttpServletResponse response) { String tokenValue = (token != null) ? token.getToken() : ""; - ResponseCookie rCookie = ResponseCookie - .from(this.cookieName, tokenValue) + ResponseCookie.ResponseCookieBuilder cookieBuilder = ResponseCookie.from(this.cookieName, tokenValue) .secure(this.secure != null ? this.secure : request.isSecure()) .path(StringUtils.hasLength(this.cookiePath) ? this.cookiePath : this.getRequestContext(request)) .maxAge(token != null ? this.cookieMaxAge : 0) .httpOnly(this.cookieHttpOnly) - .domain(this.cookieDomain) - .sameSite(this.sameSite) - .build(); + .domain(this.cookieDomain); + + if (this.cookieInitializer != null) { + this.cookieInitializer.accept(cookieBuilder); + } - response.setHeader(cookieName, rCookie.toString()); + response.setHeader(HttpHeaders.SET_COOKIE, cookieBuilder.build().toString()); } @Override @@ -128,11 +142,9 @@ public void setCookieName(String cookieName) { } /** - * Sets the HttpOnly attribute on the cookie containing the CSRF token. Defaults to - * true. - * @param cookieHttpOnly true sets the HttpOnly attribute, - * false does not set it + * @deprecated Use {@link #addCookieInitializer(Consumer)} instead. */ + @Deprecated(since = "6.1") public void setCookieHttpOnly(boolean cookieHttpOnly) { this.cookieHttpOnly = cookieHttpOnly; } @@ -150,7 +162,7 @@ private String getRequestContext(HttpServletRequest request) { */ public static CookieCsrfTokenRepository withHttpOnlyFalse() { CookieCsrfTokenRepository result = new CookieCsrfTokenRepository(); - result.setCookieHttpOnly(false); + result.addCookieInitializer(builder -> builder.httpOnly(false)); return result; } @@ -176,63 +188,30 @@ public String getCookiePath() { } /** - * Sets the domain of the cookie that the expected CSRF token is saved to and read - * from. - * @param cookieDomain the domain of the cookie that the expected CSRF token is saved - * to and read from + * @deprecated Use {@link #addCookieInitializer(Consumer)} instead. * @since 5.2 */ + @Deprecated(since = "6.1") public void setCookieDomain(String cookieDomain) { this.cookieDomain = cookieDomain; } /** - * Sets secure flag of the cookie that the expected CSRF token is saved to and read - * from. By default secure flag depends on {@link ServletRequest#isSecure()} - * @param secure the secure flag of the cookie that the expected CSRF token is saved - * to and read from + * @deprecated Use {@link #addCookieInitializer(Consumer)} instead. * @since 5.4 */ + @Deprecated(since = "6.1") public void setSecure(Boolean secure) { this.secure = secure; } /** - * Sets maximum age in seconds for the cookie that the expected CSRF token is saved to - * and read from. By default maximum age value is -1. - * - *

- * A positive value indicates that the cookie will expire after that many seconds have - * passed. Note that the value is the maximum age when the cookie will expire, - * not the cookie's current age. - * - *

- * A negative value means that the cookie is not stored persistently and will be - * deleted when the Web browser exits. - * - *

- * A zero value causes the cookie to be deleted immediately therefore it is not a - * valid value and in that case an {@link IllegalArgumentException} will be thrown. - * @param cookieMaxAge an integer specifying the maximum age of the cookie in seconds; - * if negative, means the cookie is not stored; if zero, the method throws an - * {@link IllegalArgumentException} + * @deprecated Use {@link #addCookieInitializer(Consumer)} instead. * @since 5.5 */ + @Deprecated(since = "6.1") public void setCookieMaxAge(int cookieMaxAge) { Assert.isTrue(cookieMaxAge != 0, "cookieMaxAge cannot be zero"); this.cookieMaxAge = cookieMaxAge; } - - /** - * Add the "SameSite" attribute to the generated cookie. - *

This limits the scope of the cookie such that it will only be - * attached to same site requests if {@code "Strict"} or cross-site - * requests if {@code "Lax"}. - * @since 6.1 - * @see RFC6265 bis - */ - public void setSameSite(String sameSite) { - this.sameSite = sameSite; - } - } diff --git a/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java b/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java index 68fac2fdadb..93eaa420168 100644 --- a/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java +++ b/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java @@ -16,8 +16,11 @@ package org.springframework.security.web.server.csrf; +import java.time.Duration; import java.util.UUID; +import java.util.function.Consumer; +import org.springframework.lang.Nullable; import reactor.core.publisher.Mono; import reactor.core.scheduler.Schedulers; @@ -60,6 +63,21 @@ public final class CookieServerCsrfTokenRepository implements ServerCsrfTokenRep private int cookieMaxAge = -1; + @Nullable + private Consumer cookieInitializer = null; + + + /** + * Add a {@link Consumer} for a {@code ResponseCookieBuilder} that will be invoked + * for each cookie being built, just before the call to {@code build()}. + * @param initializer consumer for a cookie builder + * @since 6.1 + */ + public void addCookieInitializer(Consumer initializer) { + this.cookieInitializer = this.cookieInitializer != null ? + this.cookieInitializer.andThen(initializer) : initializer; + } + /** * Factory method to conveniently create an instance that has * {@link #setCookieHttpOnly(boolean)} set to false. @@ -68,7 +86,7 @@ public final class CookieServerCsrfTokenRepository implements ServerCsrfTokenRep */ public static CookieServerCsrfTokenRepository withHttpOnlyFalse() { CookieServerCsrfTokenRepository result = new CookieServerCsrfTokenRepository(); - result.setCookieHttpOnly(false); + result.addCookieInitializer(builder -> builder.httpOnly(false)); return result; } @@ -82,16 +100,19 @@ public Mono saveToken(ServerWebExchange exchange, CsrfToken token) { return Mono.fromRunnable(() -> { String tokenValue = (token != null) ? token.getToken() : ""; // @formatter:off - ResponseCookie cookie = ResponseCookie + ResponseCookie.ResponseCookieBuilder cookieBuilder = ResponseCookie .from(this.cookieName, tokenValue) .domain(this.cookieDomain) .httpOnly(this.cookieHttpOnly) .maxAge(!tokenValue.isEmpty() ? this.cookieMaxAge : 0) .path((this.cookiePath != null) ? this.cookiePath : getRequestContext(exchange.getRequest())) - .secure((this.secure != null) ? this.secure : (exchange.getRequest().getSslInfo() != null)) - .build(); + .secure((this.secure != null) ? this.secure : (exchange.getRequest().getSslInfo() != null)); + + if (this.cookieInitializer != null) { + this.cookieInitializer.accept(cookieBuilder); + } // @formatter:on - exchange.getResponse().addCookie(cookie); + exchange.getResponse().addCookie(cookieBuilder.build()); }); } @@ -107,9 +128,9 @@ public Mono loadToken(ServerWebExchange exchange) { } /** - * Sets the HttpOnly attribute on the cookie containing the CSRF token - * @param cookieHttpOnly True to mark the cookie as http only. False otherwise. + * @deprecated Use {@link #addCookieInitializer(Consumer)} instead. */ + @Deprecated(since = "6.1") public void setCookieHttpOnly(boolean cookieHttpOnly) { this.cookieHttpOnly = cookieHttpOnly; } @@ -150,44 +171,27 @@ public void setCookiePath(String cookiePath) { } /** - * Sets the cookie domain - * @param cookieDomain The cookie domain + * @deprecated Use {@link #addCookieInitializer(Consumer)} instead. */ + @Deprecated(since = "6.1") public void setCookieDomain(String cookieDomain) { this.cookieDomain = cookieDomain; } /** - * Sets the cookie secure flag. If not set, the value depends on - * {@link ServerHttpRequest#getSslInfo()}. - * @param secure The value for the secure flag + * @deprecated Use {@link #addCookieInitializer(Consumer)} instead. * @since 5.5 */ + @Deprecated(since = "6.1") public void setSecure(boolean secure) { this.secure = secure; } /** - * Sets maximum age in seconds for the cookie that the expected CSRF token is saved to - * and read from. By default maximum age value is -1. - * - *

- * A positive value indicates that the cookie will expire after that many seconds have - * passed. Note that the value is the maximum age when the cookie will expire, - * not the cookie's current age. - * - *

- * A negative value means that the cookie is not stored persistently and will be - * deleted when the Web browser exits. - * - *

- * A zero value causes the cookie to be deleted immediately therefore it is not a - * valid value and in that case an {@link IllegalArgumentException} will be thrown. - * @param cookieMaxAge an integer specifying the maximum age of the cookie in seconds; - * if negative, means the cookie is not stored; if zero, the method throws an - * {@link IllegalArgumentException} + * @deprecated Use {@link #addCookieInitializer(Consumer)} instead. * @since 5.8 */ + @Deprecated(since = "6.1") public void setCookieMaxAge(int cookieMaxAge) { Assert.isTrue(cookieMaxAge != 0, "cookieMaxAge cannot be zero"); this.cookieMaxAge = cookieMaxAge; 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 index b2151ef99bd..ad13caf310b 100644 --- a/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java +++ b/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java @@ -20,6 +20,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.mock.web.MockCookie; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; @@ -48,7 +49,7 @@ public void setup() { } @Test - public void generateToken() { + void generateToken() { CsrfToken generateToken = this.repository.generateToken(this.request); assertThat(generateToken).isNotNull(); assertThat(generateToken.getHeaderName()).isEqualTo(CookieCsrfTokenRepository.DEFAULT_CSRF_HEADER_NAME); @@ -57,7 +58,7 @@ public void generateToken() { } @Test - public void generateTokenCustom() { + void generateTokenCustom() { String headerName = "headerName"; String parameterName = "paramName"; this.repository.setHeaderName(headerName); @@ -70,7 +71,7 @@ public void generateTokenCustom() { } @Test - public void saveToken() { + void saveToken() { 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); @@ -83,7 +84,7 @@ public void saveToken() { } @Test - public void saveTokenSecure() { + void saveTokenSecure() { this.request.setSecure(true); CsrfToken token = this.repository.generateToken(this.request); this.repository.saveToken(token, this.request, this.response); @@ -92,9 +93,9 @@ public void saveTokenSecure() { } @Test - public void saveTokenSecureFlagTrue() { + void saveTokenSecureFlagTrue() { this.request.setSecure(false); - this.repository.setSecure(Boolean.TRUE); + this.repository.addCookieInitializer(builder -> builder.secure(Boolean.TRUE)); 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); @@ -102,9 +103,9 @@ public void saveTokenSecureFlagTrue() { } @Test - public void saveTokenSecureFlagFalse() { + void saveTokenSecureFlagFalse() { this.request.setSecure(true); - this.repository.setSecure(Boolean.FALSE); + this.repository.addCookieInitializer(builder -> builder.secure(Boolean.FALSE)); 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); @@ -112,7 +113,7 @@ public void saveTokenSecureFlagFalse() { } @Test - public void saveTokenNull() { + void saveTokenNull() { this.request.setSecure(true); this.repository.saveToken(null, this.request, this.response); Cookie tokenCookie = this.response.getCookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME); @@ -124,8 +125,8 @@ public void saveTokenNull() { } @Test - public void saveTokenHttpOnlyTrue() { - this.repository.setCookieHttpOnly(true); + void saveTokenHttpOnlyTrue() { + this.repository.addCookieInitializer(builder -> builder.httpOnly(true)); 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); @@ -133,8 +134,8 @@ public void saveTokenHttpOnlyTrue() { } @Test - public void saveTokenHttpOnlyFalse() { - this.repository.setCookieHttpOnly(false); + void saveTokenHttpOnlyFalse() { + this.repository.addCookieInitializer(builder -> builder.httpOnly(false)); 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); @@ -142,7 +143,7 @@ public void saveTokenHttpOnlyFalse() { } @Test - public void saveTokenWithHttpOnlyFalse() { + void saveTokenWithHttpOnlyFalse() { this.repository = CookieCsrfTokenRepository.withHttpOnlyFalse(); CsrfToken token = this.repository.generateToken(this.request); this.repository.saveToken(token, this.request, this.response); @@ -151,7 +152,7 @@ public void saveTokenWithHttpOnlyFalse() { } @Test - public void saveTokenCustomPath() { + void saveTokenCustomPath() { String customPath = "/custompath"; this.repository.setCookiePath(customPath); CsrfToken token = this.repository.generateToken(this.request); @@ -161,7 +162,7 @@ public void saveTokenCustomPath() { } @Test - public void saveTokenEmptyCustomPath() { + void saveTokenEmptyCustomPath() { String customPath = ""; this.repository.setCookiePath(customPath); CsrfToken token = this.repository.generateToken(this.request); @@ -171,7 +172,7 @@ public void saveTokenEmptyCustomPath() { } @Test - public void saveTokenNullCustomPath() { + void saveTokenNullCustomPath() { String customPath = null; this.repository.setCookiePath(customPath); CsrfToken token = this.repository.generateToken(this.request); @@ -181,9 +182,9 @@ public void saveTokenNullCustomPath() { } @Test - public void saveTokenWithCookieDomain() { + void saveTokenWithCookieDomain() { String domainName = "example.com"; - this.repository.setCookieDomain(domainName); + this.repository.addCookieInitializer(builder -> builder.domain(domainName)); 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); @@ -191,9 +192,9 @@ public void saveTokenWithCookieDomain() { } @Test - public void saveTokenWithCookieMaxAge() { + void saveTokenWithCookieMaxAge() { int maxAge = 1200; - this.repository.setCookieMaxAge(maxAge); + this.repository.addCookieInitializer(builder -> builder.maxAge(maxAge)); 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); @@ -201,24 +202,54 @@ public void saveTokenWithCookieMaxAge() { } @Test - public void loadTokenNoCookiesNull() { + void saveTokenWithSameSiteNull() { + String sameSitePolicy = null; + this.repository.addCookieInitializer(builder -> builder.sameSite(sameSitePolicy)); + 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(null); + } + + @Test + void saveTokenWithSameSiteStrict() { + String sameSitePolicy = "Strict"; + this.repository.addCookieInitializer(builder -> builder.sameSite(sameSitePolicy)); + 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); + } + + @Test + void saveTokenWithSameSiteLax() { + String sameSitePolicy = "Lax"; + this.repository.addCookieInitializer(builder -> builder.sameSite(sameSitePolicy)); + 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); + } + + @Test + void loadTokenNoCookiesNull() { assertThat(this.repository.loadToken(this.request)).isNull(); } @Test - public void loadTokenCookieIncorrectNameNull() { + void loadTokenCookieIncorrectNameNull() { this.request.setCookies(new Cookie("other", "name")); assertThat(this.repository.loadToken(this.request)).isNull(); } @Test - public void loadTokenCookieValueEmptyString() { + void loadTokenCookieValueEmptyString() { this.request.setCookies(new Cookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, "")); assertThat(this.repository.loadToken(this.request)).isNull(); } @Test - public void loadToken() { + void loadToken() { CsrfToken generateToken = this.repository.generateToken(this.request); this.request .setCookies(new Cookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, generateToken.getToken())); @@ -230,7 +261,7 @@ public void loadToken() { } @Test - public void loadTokenCustom() { + void loadTokenCustom() { String cookieName = "cookieName"; String value = "value"; String headerName = "headerName"; @@ -247,7 +278,7 @@ public void loadTokenCustom() { } @Test - public void loadDeferredTokenWhenDoesNotExistThenGeneratedAndSaved() { + void loadDeferredTokenWhenDoesNotExistThenGeneratedAndSaved() { DeferredCsrfToken deferredCsrfToken = this.repository.loadDeferredToken(this.request, this.response); CsrfToken csrfToken = deferredCsrfToken.get(); assertThat(csrfToken).isNotNull(); @@ -263,7 +294,7 @@ public void loadDeferredTokenWhenDoesNotExistThenGeneratedAndSaved() { } @Test - public void loadDeferredTokenWhenExistsThenLoaded() { + void loadDeferredTokenWhenExistsThenLoaded() { CsrfToken generatedToken = this.repository.generateToken(this.request); this.request .setCookies(new Cookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, generatedToken.getToken())); @@ -274,23 +305,42 @@ public void loadDeferredTokenWhenExistsThenLoaded() { } @Test - public void setCookieNameNullIllegalArgumentException() { + void cookieInitializer() { + String domainName = "example.com"; + String customPath = "/custompath"; + String sameSitePolicy = "Strict"; + this.repository.addCookieInitializer(builder -> builder.domain(domainName)); + this.repository.addCookieInitializer(builder -> builder.secure(false)); + this.repository.addCookieInitializer(builder -> builder.path(customPath)); + this.repository.addCookieInitializer(builder -> builder.sameSite(sameSitePolicy)); + 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(tokenCookie).isNotNull(); + assertThat(tokenCookie.getMaxAge()).isEqualTo(-1); + assertThat(tokenCookie.getDomain()).isEqualTo(domainName); + assertThat(tokenCookie.getPath()).isEqualTo(customPath); + assertThat(tokenCookie.isHttpOnly()).isEqualTo(Boolean.TRUE); + assertThat(((MockCookie)tokenCookie).getSameSite()).isEqualTo(sameSitePolicy); + } + + @Test + void setCookieNameNullIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.repository.setCookieName(null)); } @Test - public void setParameterNameNullIllegalArgumentException() { + void setParameterNameNullIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.repository.setParameterName(null)); } @Test - public void setHeaderNameNullIllegalArgumentException() { + void setHeaderNameNullIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.repository.setHeaderName(null)); } @Test - public void setCookieMaxAgeZeroIllegalArgumentException() { + void setCookieMaxAgeZeroIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.repository.setCookieMaxAge(0)); } - } diff --git a/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java b/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java index b1906421584..6dfed54175d 100644 --- a/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java +++ b/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java @@ -61,6 +61,9 @@ public class CookieServerCsrfTokenRepositoryTests { private String expectedCookieValue = "csrfToken"; + private String expectedSameSitePolicy = null; + + @BeforeEach public void setUp() { this.csrfTokenRepository = new CookieServerCsrfTokenRepository(); @@ -68,71 +71,78 @@ public void setUp() { } @Test - public void generateTokenWhenDefaultThenDefaults() { + void generateTokenWhenDefaultThenDefaults() { generateTokenAndAssertExpectedValues(); } @Test - public void generateTokenWhenCustomHeaderThenCustomHeader() { + void generateTokenWhenCustomHeaderThenCustomHeader() { setExpectedHeaderName("someHeader"); generateTokenAndAssertExpectedValues(); } @Test - public void generateTokenWhenCustomParameterThenCustomParameter() { + void generateTokenWhenCustomParameterThenCustomParameter() { setExpectedParameterName("someParam"); generateTokenAndAssertExpectedValues(); } @Test - public void generateTokenWhenCustomHeaderAndParameterThenCustomHeaderAndParameter() { + void generateTokenWhenCustomHeaderAndParameterThenCustomHeaderAndParameter() { setExpectedHeaderName("someHeader"); setExpectedParameterName("someParam"); generateTokenAndAssertExpectedValues(); } @Test - public void saveTokenWhenNoSubscriptionThenNotWritten() { + void saveTokenWhenNoSubscriptionThenNotWritten() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); this.csrfTokenRepository.saveToken(exchange, createToken()); assertThat(exchange.getResponse().getCookies().getFirst(this.expectedCookieName)).isNull(); } @Test - public void saveTokenWhenDefaultThenDefaults() { + void saveTokenWhenDefaultThenDefaults() { saveAndAssertExpectedValues(createToken()); } @Test - public void saveTokenWhenNullThenDeletes() { + void saveTokenWhenNullThenDeletes() { saveAndAssertExpectedValues(null); } @Test - public void saveTokenWhenHttpOnlyFalseThenHttpOnlyFalse() { + void saveTokenWhenHttpOnlyFalseThenHttpOnlyFalse() { setExpectedHttpOnly(false); saveAndAssertExpectedValues(createToken()); } @Test - public void saveTokenWhenCookieMaxAgeThenCookieMaxAge() { + void saveTokenWhenCookieMaxAgeThenCookieMaxAge() { setExpectedCookieMaxAge(3600); saveAndAssertExpectedValues(createToken()); } @Test - public void saveTokenWhenCustomPropertiesThenCustomProperties() { + void saveTokenWhenSameSiteThenCookieSameSite() { + setExpectedSameSitePolicy("Lax"); + saveAndAssertExpectedValues(createToken()); + } + + @Test + void saveTokenWhenCustomPropertiesThenCustomProperties() { setExpectedDomain("spring.io"); setExpectedCookieName("csrfCookie"); setExpectedPath("/some/path"); setExpectedHeaderName("headerName"); setExpectedParameterName("paramName"); + setExpectedSameSitePolicy("Strict"); setExpectedCookieMaxAge(3600); saveAndAssertExpectedValues(createToken()); } @Test - public void saveTokenWhenSslInfoPresentThenSecure() { + void saveTokenWhenSslInfoPresentThenSecure() { this.request.sslInfo(new MockSslInfo()); MockServerWebExchange exchange = MockServerWebExchange.from(this.request); this.csrfTokenRepository.saveToken(exchange, createToken()).block(); @@ -142,7 +152,7 @@ public void saveTokenWhenSslInfoPresentThenSecure() { } @Test - public void saveTokenWhenSslInfoNullThenNotSecure() { + void saveTokenWhenSslInfoNullThenNotSecure() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); this.csrfTokenRepository.saveToken(exchange, createToken()).block(); ResponseCookie cookie = exchange.getResponse().getCookies().getFirst(this.expectedCookieName); @@ -151,9 +161,9 @@ public void saveTokenWhenSslInfoNullThenNotSecure() { } @Test - public void saveTokenWhenSecureFlagTrueThenSecure() { + void saveTokenWhenSecureFlagTrueThenSecure() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); - this.csrfTokenRepository.setSecure(true); + this.csrfTokenRepository.addCookieInitializer(builder -> builder.secure(true)); this.csrfTokenRepository.saveToken(exchange, createToken()).block(); ResponseCookie cookie = exchange.getResponse().getCookies().getFirst(this.expectedCookieName); assertThat(cookie).isNotNull(); @@ -161,9 +171,9 @@ public void saveTokenWhenSecureFlagTrueThenSecure() { } @Test - public void saveTokenWhenSecureFlagFalseThenNotSecure() { + void saveTokenWhenSecureFlagFalseThenNotSecure() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); - this.csrfTokenRepository.setSecure(false); + this.csrfTokenRepository.addCookieInitializer(builder -> builder.secure(false)); this.csrfTokenRepository.saveToken(exchange, createToken()).block(); ResponseCookie cookie = exchange.getResponse().getCookies().getFirst(this.expectedCookieName); assertThat(cookie).isNotNull(); @@ -171,10 +181,10 @@ public void saveTokenWhenSecureFlagFalseThenNotSecure() { } @Test - public void saveTokenWhenSecureFlagFalseAndSslInfoThenNotSecure() { + void saveTokenWhenSecureFlagFalseAndSslInfoThenNotSecure() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); this.request.sslInfo(new MockSslInfo()); - this.csrfTokenRepository.setSecure(false); + this.csrfTokenRepository.addCookieInitializer(builder -> builder.secure(false)); this.csrfTokenRepository.saveToken(exchange, createToken()).block(); ResponseCookie cookie = exchange.getResponse().getCookies().getFirst(this.expectedCookieName); assertThat(cookie).isNotNull(); @@ -182,12 +192,12 @@ public void saveTokenWhenSecureFlagFalseAndSslInfoThenNotSecure() { } @Test - public void loadTokenWhenCookieExistThenTokenFound() { + void loadTokenWhenCookieExistThenTokenFound() { loadAndAssertExpectedValues(); } @Test - public void loadTokenWhenCustomThenTokenFound() { + void loadTokenWhenCustomThenTokenFound() { setExpectedParameterName("paramName"); setExpectedHeaderName("headerName"); setExpectedCookieName("csrfCookie"); @@ -195,20 +205,20 @@ public void loadTokenWhenCustomThenTokenFound() { } @Test - public void loadTokenWhenNoCookiesThenNullToken() { + void loadTokenWhenNoCookiesThenNullToken() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); CsrfToken csrfToken = this.csrfTokenRepository.loadToken(exchange).block(); assertThat(csrfToken).isNull(); } @Test - public void loadTokenWhenCookieExistsWithNoValue() { + void loadTokenWhenCookieExistsWithNoValue() { setExpectedCookieValue(""); loadAndAssertExpectedValues(); } @Test - public void loadTokenWhenCookieExistsWithNullValue() { + void loadTokenWhenCookieExistsWithNullValue() { setExpectedCookieValue(null); loadAndAssertExpectedValues(); } @@ -224,7 +234,7 @@ private void setExpectedParameterName(String expectedParameterName) { } private void setExpectedDomain(String expectedDomain) { - this.csrfTokenRepository.setCookieDomain(expectedDomain); + this.csrfTokenRepository.addCookieInitializer(builder -> builder.domain(expectedDomain)); this.expectedDomain = expectedDomain; } @@ -235,7 +245,7 @@ private void setExpectedPath(String expectedPath) { private void setExpectedHttpOnly(boolean expectedHttpOnly) { this.expectedHttpOnly = expectedHttpOnly; - this.csrfTokenRepository.setCookieHttpOnly(expectedHttpOnly); + this.csrfTokenRepository.addCookieInitializer(builder -> builder.httpOnly(expectedHttpOnly)); } private void setExpectedCookieName(String expectedCookieName) { @@ -244,10 +254,15 @@ private void setExpectedCookieName(String expectedCookieName) { } private void setExpectedCookieMaxAge(int expectedCookieMaxAge) { - this.csrfTokenRepository.setCookieMaxAge(expectedCookieMaxAge); + this.csrfTokenRepository.addCookieInitializer(builder -> builder.maxAge(expectedCookieMaxAge)); this.expectedMaxAge = Duration.ofSeconds(expectedCookieMaxAge); } + private void setExpectedSameSitePolicy(String sameSitePolicy){ + this.csrfTokenRepository.addCookieInitializer(builder -> builder.sameSite(sameSitePolicy)); + this.expectedSameSitePolicy = sameSitePolicy; + } + private void setExpectedCookieValue(String expectedCookieValue) { this.expectedCookieValue = expectedCookieValue; } @@ -284,6 +299,7 @@ private void saveAndAssertExpectedValues(CsrfToken token) { assertThat(cookie.isHttpOnly()).isEqualTo(this.expectedHttpOnly); assertThat(cookie.getName()).isEqualTo(this.expectedCookieName); assertThat(cookie.getValue()).isEqualTo(this.expectedCookieValue); + assertThat(cookie.getSameSite()).isEqualTo(this.expectedSameSitePolicy); } private void generateTokenAndAssertExpectedValues() { From 825aefb31ac74e0f673fde7bac7bf4675b6ea6c3 Mon Sep 17 00:00:00 2001 From: Alex Montoya Date: Tue, 15 Nov 2022 20:34:06 -0800 Subject: [PATCH 3/6] - Remove un necessary new method to set same site policy. - Mark setCookieHttpOnly, setCookieDomain, setCookieMaxAge and setSecure as deprecated. - Add the method setCookieCustomizer which allows to set properties to the ResponseCookieBuilder without having to add new setter methods. - Apply same changes to CookieServerCsrfTokenRepository. - Add unit tests to validate the use of same site policy. --- .../web/csrf/CookieCsrfTokenRepository.java | 30 ++-- .../csrf/CookieServerCsrfTokenRepository.java | 27 ++-- .../csrf/CookieCsrfTokenRepositoryTests.java | 145 +++++++++++++----- .../CookieServerCsrfTokenRepositoryTests.java | 124 +++++++++++---- 4 files changed, 223 insertions(+), 103 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 index 9ef86b4e67f..a9047e6a1cd 100644 --- a/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java +++ b/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2022 the original author or authors. + * Copyright 2012-2016 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. @@ -24,7 +24,6 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.ResponseCookie; -import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.web.util.WebUtils; @@ -36,6 +35,7 @@ * * @author Rob Winch * @author Steve Riesenberg + * @author Alex Montoya * @since 4.1 */ public final class CookieCsrfTokenRepository implements CsrfTokenRepository { @@ -65,18 +65,16 @@ public final class CookieCsrfTokenRepository implements CsrfTokenRepository { private int cookieMaxAge = -1; - @Nullable - private Consumer cookieInitializer = null; + private Consumer cookieCustomizer = (builder) -> {}; /** * Add a {@link Consumer} for a {@code ResponseCookieBuilder} that will be invoked * for each cookie being built, just before the call to {@code build()}. - * @param initializer consumer for a cookie builder + * @param cookieCustomizer consumer for a cookie builder * @since 6.1 */ - public void addCookieInitializer(Consumer initializer) { - this.cookieInitializer = this.cookieInitializer != null ? - this.cookieInitializer.andThen(initializer) : initializer; + public void setCookieCustomizer(Consumer cookieCustomizer) { + this.cookieCustomizer = cookieCustomizer; } @Override @@ -95,11 +93,9 @@ public void saveToken(CsrfToken token, HttpServletRequest request, HttpServletRe .httpOnly(this.cookieHttpOnly) .domain(this.cookieDomain); - if (this.cookieInitializer != null) { - this.cookieInitializer.accept(cookieBuilder); - } + this.cookieCustomizer.accept(cookieBuilder); - response.setHeader(HttpHeaders.SET_COOKIE, cookieBuilder.build().toString()); + response.setHeader(HttpHeaders.SET_COOKIE, cookieBuilder.build().toString()); // Set request attribute to signal that response has blank cookie value, // which allows loadToken to return null when token has been removed @@ -160,7 +156,7 @@ public void setCookieName(String cookieName) { } /** - * @deprecated Use {@link #addCookieInitializer(Consumer)} instead. + * @deprecated Use {@link #setCookieCustomizer(Consumer)} instead. */ @Deprecated(since = "6.1") public void setCookieHttpOnly(boolean cookieHttpOnly) { @@ -180,7 +176,7 @@ private String getRequestContext(HttpServletRequest request) { */ public static CookieCsrfTokenRepository withHttpOnlyFalse() { CookieCsrfTokenRepository result = new CookieCsrfTokenRepository(); - result.addCookieInitializer(builder -> builder.httpOnly(false)); + result.setCookieCustomizer(customizer -> customizer.httpOnly(false)); return result; } @@ -206,7 +202,7 @@ public String getCookiePath() { } /** - * @deprecated Use {@link #addCookieInitializer(Consumer)} instead. + * @deprecated Use {@link #setCookieCustomizer(Consumer)} instead. * @since 5.2 */ @Deprecated(since = "6.1") @@ -215,7 +211,7 @@ public void setCookieDomain(String cookieDomain) { } /** - * @deprecated Use {@link #addCookieInitializer(Consumer)} instead. + * @deprecated Use {@link #setCookieCustomizer(Consumer)} instead. * @since 5.4 */ @Deprecated(since = "6.1") @@ -224,7 +220,7 @@ public void setSecure(Boolean secure) { } /** - * @deprecated Use {@link #addCookieInitializer(Consumer)} instead. + * @deprecated Use {@link #setCookieCustomizer(Consumer)} instead. * @since 5.5 */ @Deprecated(since = "6.1") diff --git a/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java b/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java index 93eaa420168..bf19bc32dd2 100644 --- a/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java +++ b/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java @@ -39,6 +39,7 @@ * @author Eric Deandrea * @author Thomas Vitale * @author Alonso Araya + * @author Alex Montoya * @since 5.1 */ public final class CookieServerCsrfTokenRepository implements ServerCsrfTokenRepository { @@ -63,19 +64,16 @@ public final class CookieServerCsrfTokenRepository implements ServerCsrfTokenRep private int cookieMaxAge = -1; - @Nullable - private Consumer cookieInitializer = null; - + private Consumer cookieCustomizer = (builder) -> {}; /** * Add a {@link Consumer} for a {@code ResponseCookieBuilder} that will be invoked * for each cookie being built, just before the call to {@code build()}. - * @param initializer consumer for a cookie builder + * @param cookieCustomizer consumer for a cookie builder * @since 6.1 */ - public void addCookieInitializer(Consumer initializer) { - this.cookieInitializer = this.cookieInitializer != null ? - this.cookieInitializer.andThen(initializer) : initializer; + public void setCookieCustomizer(Consumer cookieCustomizer) { + this.cookieCustomizer = cookieCustomizer; } /** @@ -86,7 +84,7 @@ public void addCookieInitializer(Consumer */ public static CookieServerCsrfTokenRepository withHttpOnlyFalse() { CookieServerCsrfTokenRepository result = new CookieServerCsrfTokenRepository(); - result.addCookieInitializer(builder -> builder.httpOnly(false)); + result.setCookieCustomizer(customizer -> customizer.httpOnly(false)); return result; } @@ -108,9 +106,8 @@ public Mono saveToken(ServerWebExchange exchange, CsrfToken token) { .path((this.cookiePath != null) ? this.cookiePath : getRequestContext(exchange.getRequest())) .secure((this.secure != null) ? this.secure : (exchange.getRequest().getSslInfo() != null)); - if (this.cookieInitializer != null) { - this.cookieInitializer.accept(cookieBuilder); - } + this.cookieCustomizer.accept(cookieBuilder); + // @formatter:on exchange.getResponse().addCookie(cookieBuilder.build()); }); @@ -128,7 +125,7 @@ public Mono loadToken(ServerWebExchange exchange) { } /** - * @deprecated Use {@link #addCookieInitializer(Consumer)} instead. + * @deprecated Use {@link #setCookieCustomizer(Consumer)} instead. */ @Deprecated(since = "6.1") public void setCookieHttpOnly(boolean cookieHttpOnly) { @@ -171,7 +168,7 @@ public void setCookiePath(String cookiePath) { } /** - * @deprecated Use {@link #addCookieInitializer(Consumer)} instead. + * @deprecated Use {@link #setCookieCustomizer(Consumer)} instead. */ @Deprecated(since = "6.1") public void setCookieDomain(String cookieDomain) { @@ -179,7 +176,7 @@ public void setCookieDomain(String cookieDomain) { } /** - * @deprecated Use {@link #addCookieInitializer(Consumer)} instead. + * @deprecated Use {@link #setCookieCustomizer(Consumer)} instead. * @since 5.5 */ @Deprecated(since = "6.1") @@ -188,7 +185,7 @@ public void setSecure(boolean secure) { } /** - * @deprecated Use {@link #addCookieInitializer(Consumer)} instead. + * @deprecated Use {@link #setCookieCustomizer(Consumer)} instead. * @since 5.8 */ @Deprecated(since = "6.1") 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 index 04a1338f97c..29157578f03 100644 --- a/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java +++ b/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java @@ -49,7 +49,7 @@ public void setup() { } @Test - void generateToken() { + public void generateToken() { CsrfToken generateToken = this.repository.generateToken(this.request); assertThat(generateToken).isNotNull(); assertThat(generateToken.getHeaderName()).isEqualTo(CookieCsrfTokenRepository.DEFAULT_CSRF_HEADER_NAME); @@ -58,7 +58,7 @@ void generateToken() { } @Test - void generateTokenCustom() { + public void generateTokenCustom() { String headerName = "headerName"; String parameterName = "paramName"; this.repository.setHeaderName(headerName); @@ -71,7 +71,7 @@ void generateTokenCustom() { } @Test - void saveToken() { + public void saveToken() { 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); @@ -84,7 +84,7 @@ void saveToken() { } @Test - void saveTokenSecure() { + public void saveTokenSecure() { this.request.setSecure(true); CsrfToken token = this.repository.generateToken(this.request); this.repository.saveToken(token, this.request, this.response); @@ -93,9 +93,9 @@ void saveTokenSecure() { } @Test - void saveTokenSecureFlagTrue() { + public void saveTokenSecureFlagTrue() { this.request.setSecure(false); - this.repository.addCookieInitializer(builder -> builder.secure(Boolean.TRUE)); + this.repository.setSecure(Boolean.TRUE); 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); @@ -103,9 +103,29 @@ void saveTokenSecureFlagTrue() { } @Test - void saveTokenSecureFlagFalse() { + public void saveTokenSecureFlagTrueUsingCustomizer() { + this.request.setSecure(false); + this.repository.setCookieCustomizer(customizer -> customizer.secure(Boolean.TRUE)); + 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(tokenCookie.getSecure()).isTrue(); + } + + @Test + public void saveTokenSecureFlagFalse() { + this.request.setSecure(true); + this.repository.setSecure(Boolean.FALSE); + 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(tokenCookie.getSecure()).isFalse(); + } + + @Test + public void saveTokenSecureFlagFalseUsingCustomizer() { this.request.setSecure(true); - this.repository.addCookieInitializer(builder -> builder.secure(Boolean.FALSE)); + this.repository.setCookieCustomizer(customizer -> customizer.secure(Boolean.FALSE)); 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); @@ -113,7 +133,7 @@ void saveTokenSecureFlagFalse() { } @Test - void saveTokenNull() { + public void saveTokenNull() { this.request.setSecure(true); this.repository.saveToken(null, this.request, this.response); Cookie tokenCookie = this.response.getCookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME); @@ -125,8 +145,17 @@ void saveTokenNull() { } @Test - void saveTokenHttpOnlyTrue() { - this.repository.addCookieInitializer(builder -> builder.httpOnly(true)); + public void saveTokenHttpOnlyTrue() { + this.repository.setCookieHttpOnly(true); + 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(tokenCookie.isHttpOnly()).isTrue(); + } + + @Test + public void saveTokenHttpOnlyTrueUsingCustomizer() { + this.repository.setCookieCustomizer(customizer -> customizer.httpOnly(true)); 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); @@ -134,8 +163,8 @@ void saveTokenHttpOnlyTrue() { } @Test - void saveTokenHttpOnlyFalse() { - this.repository.addCookieInitializer(builder -> builder.httpOnly(false)); + public void saveTokenHttpOnlyFalse() { + this.repository.setCookieHttpOnly(false); 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); @@ -143,7 +172,16 @@ void saveTokenHttpOnlyFalse() { } @Test - void saveTokenWithHttpOnlyFalse() { + public void saveTokenHttpOnlyFalseUsingCustomizer() { + this.repository.setCookieCustomizer(customizer -> customizer.httpOnly(false)); + 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(tokenCookie.isHttpOnly()).isFalse(); + } + + @Test + public void saveTokenWithHttpOnlyFalse() { this.repository = CookieCsrfTokenRepository.withHttpOnlyFalse(); CsrfToken token = this.repository.generateToken(this.request); this.repository.saveToken(token, this.request, this.response); @@ -152,7 +190,7 @@ void saveTokenWithHttpOnlyFalse() { } @Test - void saveTokenCustomPath() { + public void saveTokenCustomPath() { String customPath = "/custompath"; this.repository.setCookiePath(customPath); CsrfToken token = this.repository.generateToken(this.request); @@ -162,7 +200,7 @@ void saveTokenCustomPath() { } @Test - void saveTokenEmptyCustomPath() { + public void saveTokenEmptyCustomPath() { String customPath = ""; this.repository.setCookiePath(customPath); CsrfToken token = this.repository.generateToken(this.request); @@ -172,7 +210,7 @@ void saveTokenEmptyCustomPath() { } @Test - void saveTokenNullCustomPath() { + public void saveTokenNullCustomPath() { String customPath = null; this.repository.setCookiePath(customPath); CsrfToken token = this.repository.generateToken(this.request); @@ -182,9 +220,9 @@ void saveTokenNullCustomPath() { } @Test - void saveTokenWithCookieDomain() { + public void saveTokenWithCookieDomain() { String domainName = "example.com"; - this.repository.addCookieInitializer(builder -> builder.domain(domainName)); + this.repository.setCookieDomain(domainName); 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); @@ -192,9 +230,19 @@ void saveTokenWithCookieDomain() { } @Test - void saveTokenWithCookieMaxAge() { + public void saveTokenWithCookieDomainUsingCustomizer() { + String domainName = "example.com"; + this.repository.setCookieCustomizer(customizer -> customizer.domain(domainName)); + 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(tokenCookie.getDomain()).isEqualTo(domainName); + } + + @Test + public void saveTokenWithCookieMaxAge() { int maxAge = 1200; - this.repository.addCookieInitializer(builder -> builder.maxAge(maxAge)); + this.repository.setCookieMaxAge(maxAge); 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); @@ -202,9 +250,19 @@ void saveTokenWithCookieMaxAge() { } @Test - void saveTokenWithSameSiteNull() { + public void saveTokenWithCookieMaxAgeUsingCustomizer() { + int maxAge = 1200; + this.repository.setCookieCustomizer(customizer -> customizer.maxAge(maxAge)); + 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(tokenCookie.getMaxAge()).isEqualTo(maxAge); + } + + @Test + public void saveTokenWithSameSiteNull() { String sameSitePolicy = null; - this.repository.addCookieInitializer(builder -> builder.sameSite(sameSitePolicy)); + this.repository.setCookieCustomizer(customizer -> customizer.sameSite(sameSitePolicy)); 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); @@ -212,9 +270,9 @@ void saveTokenWithSameSiteNull() { } @Test - void saveTokenWithSameSiteStrict() { + public void saveTokenWithSameSiteStrict() { String sameSitePolicy = "Strict"; - this.repository.addCookieInitializer(builder -> builder.sameSite(sameSitePolicy)); + this.repository.setCookieCustomizer(customizer -> customizer.sameSite(sameSitePolicy)); 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); @@ -222,9 +280,9 @@ void saveTokenWithSameSiteStrict() { } @Test - void saveTokenWithSameSiteLax() { + public void saveTokenWithSameSiteLax() { String sameSitePolicy = "Lax"; - this.repository.addCookieInitializer(builder -> builder.sameSite(sameSitePolicy)); + this.repository.setCookieCustomizer(customizer -> customizer.sameSite(sameSitePolicy)); 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); @@ -232,24 +290,24 @@ void saveTokenWithSameSiteLax() { } @Test - void loadTokenNoCookiesNull() { + public void loadTokenNoCookiesNull() { assertThat(this.repository.loadToken(this.request)).isNull(); } @Test - void loadTokenCookieIncorrectNameNull() { + public void loadTokenCookieIncorrectNameNull() { this.request.setCookies(new Cookie("other", "name")); assertThat(this.repository.loadToken(this.request)).isNull(); } @Test - void loadTokenCookieValueEmptyString() { + public void loadTokenCookieValueEmptyString() { this.request.setCookies(new Cookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, "")); assertThat(this.repository.loadToken(this.request)).isNull(); } @Test - void loadToken() { + public void loadToken() { CsrfToken generateToken = this.repository.generateToken(this.request); this.request .setCookies(new Cookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, generateToken.getToken())); @@ -261,7 +319,7 @@ void loadToken() { } @Test - void loadTokenCustom() { + public void loadTokenCustom() { String cookieName = "cookieName"; String value = "value"; String headerName = "headerName"; @@ -278,7 +336,7 @@ void loadTokenCustom() { } @Test - void loadDeferredTokenWhenDoesNotExistThenGeneratedAndSaved() { + public void loadDeferredTokenWhenDoesNotExistThenGeneratedAndSaved() { DeferredCsrfToken deferredCsrfToken = this.repository.loadDeferredToken(this.request, this.response); CsrfToken csrfToken = deferredCsrfToken.get(); assertThat(csrfToken).isNotNull(); @@ -331,14 +389,16 @@ public void loadDeferredTokenWhenExistsThenLoaded() { } @Test - void cookieInitializer() { + public void cookieCustomizer() { String domainName = "example.com"; String customPath = "/custompath"; String sameSitePolicy = "Strict"; - this.repository.addCookieInitializer(builder -> builder.domain(domainName)); - this.repository.addCookieInitializer(builder -> builder.secure(false)); - this.repository.addCookieInitializer(builder -> builder.path(customPath)); - this.repository.addCookieInitializer(builder -> builder.sameSite(sameSitePolicy)); + this.repository.setCookieCustomizer(customizer -> { + customizer.domain(domainName); + customizer.secure(false); + customizer.path(customPath); + customizer.sameSite(sameSitePolicy); + }); 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); @@ -351,22 +411,23 @@ void cookieInitializer() { } @Test - void setCookieNameNullIllegalArgumentException() { + public void setCookieNameNullIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.repository.setCookieName(null)); } @Test - void setParameterNameNullIllegalArgumentException() { + public void setParameterNameNullIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.repository.setParameterName(null)); } @Test - void setHeaderNameNullIllegalArgumentException() { + public void setHeaderNameNullIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.repository.setHeaderName(null)); } @Test - void setCookieMaxAgeZeroIllegalArgumentException() { + public void setCookieMaxAgeZeroIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.repository.setCookieMaxAge(0)); } + } diff --git a/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java b/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java index 6dfed54175d..c1375a791a4 100644 --- a/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java +++ b/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java @@ -18,6 +18,7 @@ import java.security.cert.X509Certificate; import java.time.Duration; +import java.time.temporal.ChronoUnit; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -63,7 +64,6 @@ public class CookieServerCsrfTokenRepositoryTests { private String expectedSameSitePolicy = null; - @BeforeEach public void setUp() { this.csrfTokenRepository = new CookieServerCsrfTokenRepository(); @@ -71,66 +71,66 @@ public void setUp() { } @Test - void generateTokenWhenDefaultThenDefaults() { + public void generateTokenWhenDefaultThenDefaults() { generateTokenAndAssertExpectedValues(); } @Test - void generateTokenWhenCustomHeaderThenCustomHeader() { + public void generateTokenWhenCustomHeaderThenCustomHeader() { setExpectedHeaderName("someHeader"); generateTokenAndAssertExpectedValues(); } @Test - void generateTokenWhenCustomParameterThenCustomParameter() { + public void generateTokenWhenCustomParameterThenCustomParameter() { setExpectedParameterName("someParam"); generateTokenAndAssertExpectedValues(); } @Test - void generateTokenWhenCustomHeaderAndParameterThenCustomHeaderAndParameter() { + public void generateTokenWhenCustomHeaderAndParameterThenCustomHeaderAndParameter() { setExpectedHeaderName("someHeader"); setExpectedParameterName("someParam"); generateTokenAndAssertExpectedValues(); } @Test - void saveTokenWhenNoSubscriptionThenNotWritten() { + public void saveTokenWhenNoSubscriptionThenNotWritten() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); this.csrfTokenRepository.saveToken(exchange, createToken()); assertThat(exchange.getResponse().getCookies().getFirst(this.expectedCookieName)).isNull(); } @Test - void saveTokenWhenDefaultThenDefaults() { + public void saveTokenWhenDefaultThenDefaults() { saveAndAssertExpectedValues(createToken()); } @Test - void saveTokenWhenNullThenDeletes() { + public void saveTokenWhenNullThenDeletes() { saveAndAssertExpectedValues(null); } @Test - void saveTokenWhenHttpOnlyFalseThenHttpOnlyFalse() { + public void saveTokenWhenHttpOnlyFalseThenHttpOnlyFalse() { setExpectedHttpOnly(false); saveAndAssertExpectedValues(createToken()); } @Test - void saveTokenWhenCookieMaxAgeThenCookieMaxAge() { + public void saveTokenWhenCookieMaxAgeThenCookieMaxAge() { setExpectedCookieMaxAge(3600); saveAndAssertExpectedValues(createToken()); } @Test - void saveTokenWhenSameSiteThenCookieSameSite() { + public void saveTokenWhenSameSiteThenCookieSameSite() { setExpectedSameSitePolicy("Lax"); saveAndAssertExpectedValues(createToken()); } @Test - void saveTokenWhenCustomPropertiesThenCustomProperties() { + public void saveTokenWhenCustomPropertiesThenCustomProperties() { setExpectedDomain("spring.io"); setExpectedCookieName("csrfCookie"); setExpectedPath("/some/path"); @@ -142,7 +142,42 @@ void saveTokenWhenCustomPropertiesThenCustomProperties() { } @Test - void saveTokenWhenSslInfoPresentThenSecure() { + public void saveTokenWhenCustomPropertiesThenCustomPropertiesUsingCustomizer() { + String expectedDomain = "spring.io"; + int expectedMaxAge = 3600; + String expectedPath = "/some/path"; + String expectedSameSite = "Strict"; + + setExpectedCookieName("csrfCookie"); + + setExpectedHeaderName("headerName"); + setExpectedParameterName("paramName"); + + CsrfToken token = createToken(); + + this.csrfTokenRepository.setCookieCustomizer(customizer -> { + customizer.domain(expectedDomain); + customizer.maxAge(expectedMaxAge); + customizer.path(expectedPath); + customizer.sameSite(expectedSameSite); + }); + + MockServerWebExchange exchange = MockServerWebExchange.from(this.request); + this.csrfTokenRepository.saveToken(exchange, token).block(); + ResponseCookie cookie = exchange.getResponse().getCookies().getFirst(this.expectedCookieName); + assertThat(cookie).isNotNull(); + assertThat(cookie.getMaxAge()).isEqualTo(Duration.of(expectedMaxAge, ChronoUnit.SECONDS)); + assertThat(cookie.getDomain()).isEqualTo(expectedDomain); + assertThat(cookie.getPath()).isEqualTo(expectedPath); + assertThat(cookie.getSameSite()).isEqualTo(expectedSameSite); + assertThat(cookie.isSecure()).isEqualTo(this.expectedSecure); + assertThat(cookie.isHttpOnly()).isEqualTo(this.expectedHttpOnly); + assertThat(cookie.getName()).isEqualTo(this.expectedCookieName); + assertThat(cookie.getValue()).isEqualTo(this.expectedCookieValue); + } + + @Test + public void saveTokenWhenSslInfoPresentThenSecure() { this.request.sslInfo(new MockSslInfo()); MockServerWebExchange exchange = MockServerWebExchange.from(this.request); this.csrfTokenRepository.saveToken(exchange, createToken()).block(); @@ -152,7 +187,7 @@ void saveTokenWhenSslInfoPresentThenSecure() { } @Test - void saveTokenWhenSslInfoNullThenNotSecure() { + public void saveTokenWhenSslInfoNullThenNotSecure() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); this.csrfTokenRepository.saveToken(exchange, createToken()).block(); ResponseCookie cookie = exchange.getResponse().getCookies().getFirst(this.expectedCookieName); @@ -161,9 +196,9 @@ void saveTokenWhenSslInfoNullThenNotSecure() { } @Test - void saveTokenWhenSecureFlagTrueThenSecure() { + public void saveTokenWhenSecureFlagTrueThenSecure() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); - this.csrfTokenRepository.addCookieInitializer(builder -> builder.secure(true)); + this.csrfTokenRepository.setSecure(true); this.csrfTokenRepository.saveToken(exchange, createToken()).block(); ResponseCookie cookie = exchange.getResponse().getCookies().getFirst(this.expectedCookieName); assertThat(cookie).isNotNull(); @@ -171,9 +206,40 @@ void saveTokenWhenSecureFlagTrueThenSecure() { } @Test - void saveTokenWhenSecureFlagFalseThenNotSecure() { + public void saveTokenWhenSecureFlagTrueThenSecureUsingCustomizer() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); - this.csrfTokenRepository.addCookieInitializer(builder -> builder.secure(false)); + this.csrfTokenRepository.setCookieCustomizer(customizer -> customizer.secure(true)); + this.csrfTokenRepository.saveToken(exchange, createToken()).block(); + ResponseCookie cookie = exchange.getResponse().getCookies().getFirst(this.expectedCookieName); + assertThat(cookie).isNotNull(); + assertThat(cookie.isSecure()).isTrue(); + } + + @Test + public void saveTokenWhenSecureFlagFalseThenNotSecure() { + MockServerWebExchange exchange = MockServerWebExchange.from(this.request); + this.csrfTokenRepository.setSecure(false); + this.csrfTokenRepository.saveToken(exchange, createToken()).block(); + ResponseCookie cookie = exchange.getResponse().getCookies().getFirst(this.expectedCookieName); + assertThat(cookie).isNotNull(); + assertThat(cookie.isSecure()).isFalse(); + } + + @Test + public void saveTokenWhenSecureFlagFalseThenNotSecureUsingCustomizer() { + MockServerWebExchange exchange = MockServerWebExchange.from(this.request); + this.csrfTokenRepository.setCookieCustomizer(customizer -> customizer.secure(false)); + this.csrfTokenRepository.saveToken(exchange, createToken()).block(); + ResponseCookie cookie = exchange.getResponse().getCookies().getFirst(this.expectedCookieName); + assertThat(cookie).isNotNull(); + assertThat(cookie.isSecure()).isFalse(); + } + + @Test + public void saveTokenWhenSecureFlagFalseAndSslInfoThenNotSecure() { + MockServerWebExchange exchange = MockServerWebExchange.from(this.request); + this.request.sslInfo(new MockSslInfo()); + this.csrfTokenRepository.setSecure(false); this.csrfTokenRepository.saveToken(exchange, createToken()).block(); ResponseCookie cookie = exchange.getResponse().getCookies().getFirst(this.expectedCookieName); assertThat(cookie).isNotNull(); @@ -181,10 +247,10 @@ void saveTokenWhenSecureFlagFalseThenNotSecure() { } @Test - void saveTokenWhenSecureFlagFalseAndSslInfoThenNotSecure() { + public void saveTokenWhenSecureFlagFalseAndSslInfoThenNotSecureUsingCustomizer() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); this.request.sslInfo(new MockSslInfo()); - this.csrfTokenRepository.addCookieInitializer(builder -> builder.secure(false)); + this.csrfTokenRepository.setCookieCustomizer(customizer -> customizer.secure(false)); this.csrfTokenRepository.saveToken(exchange, createToken()).block(); ResponseCookie cookie = exchange.getResponse().getCookies().getFirst(this.expectedCookieName); assertThat(cookie).isNotNull(); @@ -192,12 +258,12 @@ void saveTokenWhenSecureFlagFalseAndSslInfoThenNotSecure() { } @Test - void loadTokenWhenCookieExistThenTokenFound() { + public void loadTokenWhenCookieExistThenTokenFound() { loadAndAssertExpectedValues(); } @Test - void loadTokenWhenCustomThenTokenFound() { + public void loadTokenWhenCustomThenTokenFound() { setExpectedParameterName("paramName"); setExpectedHeaderName("headerName"); setExpectedCookieName("csrfCookie"); @@ -205,20 +271,20 @@ void loadTokenWhenCustomThenTokenFound() { } @Test - void loadTokenWhenNoCookiesThenNullToken() { + public void loadTokenWhenNoCookiesThenNullToken() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); CsrfToken csrfToken = this.csrfTokenRepository.loadToken(exchange).block(); assertThat(csrfToken).isNull(); } @Test - void loadTokenWhenCookieExistsWithNoValue() { + public void loadTokenWhenCookieExistsWithNoValue() { setExpectedCookieValue(""); loadAndAssertExpectedValues(); } @Test - void loadTokenWhenCookieExistsWithNullValue() { + public void loadTokenWhenCookieExistsWithNullValue() { setExpectedCookieValue(null); loadAndAssertExpectedValues(); } @@ -234,7 +300,7 @@ private void setExpectedParameterName(String expectedParameterName) { } private void setExpectedDomain(String expectedDomain) { - this.csrfTokenRepository.addCookieInitializer(builder -> builder.domain(expectedDomain)); + this.csrfTokenRepository.setCookieDomain(expectedDomain); this.expectedDomain = expectedDomain; } @@ -245,7 +311,7 @@ private void setExpectedPath(String expectedPath) { private void setExpectedHttpOnly(boolean expectedHttpOnly) { this.expectedHttpOnly = expectedHttpOnly; - this.csrfTokenRepository.addCookieInitializer(builder -> builder.httpOnly(expectedHttpOnly)); + this.csrfTokenRepository.setCookieHttpOnly(expectedHttpOnly); } private void setExpectedCookieName(String expectedCookieName) { @@ -254,12 +320,12 @@ private void setExpectedCookieName(String expectedCookieName) { } private void setExpectedCookieMaxAge(int expectedCookieMaxAge) { - this.csrfTokenRepository.addCookieInitializer(builder -> builder.maxAge(expectedCookieMaxAge)); + this.csrfTokenRepository.setCookieMaxAge(expectedCookieMaxAge); this.expectedMaxAge = Duration.ofSeconds(expectedCookieMaxAge); } private void setExpectedSameSitePolicy(String sameSitePolicy){ - this.csrfTokenRepository.addCookieInitializer(builder -> builder.sameSite(sameSitePolicy)); + this.csrfTokenRepository.setCookieCustomizer(customizer -> customizer.sameSite(sameSitePolicy)); this.expectedSameSitePolicy = sameSitePolicy; } From 62d1b40f7f520c8df3a360cf5134dbdaf0a1b498 Mon Sep 17 00:00:00 2001 From: Alex Montoya Date: Tue, 15 Nov 2022 20:34:06 -0800 Subject: [PATCH 4/6] - Remove un necessary new method to set same site policy. - Mark setCookieHttpOnly, setCookieDomain, setCookieMaxAge and setSecure as deprecated. - Add the method setCookieCustomizer which allows to set properties to the ResponseCookieBuilder without having to add new setter methods. - Apply same changes to CookieServerCsrfTokenRepository. - Add unit tests to validate the use of same site policy. --- .../security/web/csrf/CookieCsrfTokenRepository.java | 5 +++-- .../web/server/csrf/CookieServerCsrfTokenRepository.java | 4 +--- 2 files changed, 4 insertions(+), 5 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 index a9047e6a1cd..bf63de06abc 100644 --- a/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java +++ b/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2022 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. @@ -24,6 +24,7 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.ResponseCookie; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.web.util.WebUtils; @@ -176,7 +177,7 @@ private String getRequestContext(HttpServletRequest request) { */ public static CookieCsrfTokenRepository withHttpOnlyFalse() { CookieCsrfTokenRepository result = new CookieCsrfTokenRepository(); - result.setCookieCustomizer(customizer -> customizer.httpOnly(false)); + result.setCookieHttpOnly(false); return result; } diff --git a/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java b/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java index bf19bc32dd2..9aedd3819fe 100644 --- a/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java +++ b/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java @@ -16,11 +16,9 @@ package org.springframework.security.web.server.csrf; -import java.time.Duration; import java.util.UUID; import java.util.function.Consumer; -import org.springframework.lang.Nullable; import reactor.core.publisher.Mono; import reactor.core.scheduler.Schedulers; @@ -84,7 +82,7 @@ public void setCookieCustomizer(Consumer c */ public static CookieServerCsrfTokenRepository withHttpOnlyFalse() { CookieServerCsrfTokenRepository result = new CookieServerCsrfTokenRepository(); - result.setCookieCustomizer(customizer -> customizer.httpOnly(false)); + result.setCookieHttpOnly(false); return result; } From 1b0dbb05070a0844ae59d1e3e141b5524875dab4 Mon Sep 17 00:00:00 2001 From: Alex Montoya Date: Tue, 15 Nov 2022 21:12:15 -0800 Subject: [PATCH 5/6] - Junit 5 cleanup removing public modifier in tests and classes - Changes to use assertJ semantic methods as isTrue and isNull --- .../csrf/CookieCsrfTokenRepositoryTests.java | 84 +++++++++---------- .../CookieServerCsrfTokenRepositoryTests.java | 52 ++++++------ 2 files changed, 68 insertions(+), 68 deletions(-) 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 index 29157578f03..ce942298db8 100644 --- a/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java +++ b/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java @@ -32,7 +32,7 @@ * @author Rob Winch * @since 4.1 */ -public class CookieCsrfTokenRepositoryTests { +class CookieCsrfTokenRepositoryTests { CookieCsrfTokenRepository repository; @@ -49,7 +49,7 @@ public void setup() { } @Test - public void generateToken() { + void generateToken() { CsrfToken generateToken = this.repository.generateToken(this.request); assertThat(generateToken).isNotNull(); assertThat(generateToken.getHeaderName()).isEqualTo(CookieCsrfTokenRepository.DEFAULT_CSRF_HEADER_NAME); @@ -58,7 +58,7 @@ public void generateToken() { } @Test - public void generateTokenCustom() { + void generateTokenCustom() { String headerName = "headerName"; String parameterName = "paramName"; this.repository.setHeaderName(headerName); @@ -71,7 +71,7 @@ public void generateTokenCustom() { } @Test - public void saveToken() { + void saveToken() { 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); @@ -80,11 +80,11 @@ public void saveToken() { assertThat(tokenCookie.getPath()).isEqualTo(this.request.getContextPath()); assertThat(tokenCookie.getSecure()).isEqualTo(this.request.isSecure()); assertThat(tokenCookie.getValue()).isEqualTo(token.getToken()); - assertThat(tokenCookie.isHttpOnly()).isEqualTo(true); + assertThat(tokenCookie.isHttpOnly()).isTrue(); } @Test - public void saveTokenSecure() { + void saveTokenSecure() { this.request.setSecure(true); CsrfToken token = this.repository.generateToken(this.request); this.repository.saveToken(token, this.request, this.response); @@ -93,7 +93,7 @@ public void saveTokenSecure() { } @Test - public void saveTokenSecureFlagTrue() { + void saveTokenSecureFlagTrue() { this.request.setSecure(false); this.repository.setSecure(Boolean.TRUE); CsrfToken token = this.repository.generateToken(this.request); @@ -103,7 +103,7 @@ public void saveTokenSecureFlagTrue() { } @Test - public void saveTokenSecureFlagTrueUsingCustomizer() { + void saveTokenSecureFlagTrueUsingCustomizer() { this.request.setSecure(false); this.repository.setCookieCustomizer(customizer -> customizer.secure(Boolean.TRUE)); CsrfToken token = this.repository.generateToken(this.request); @@ -113,7 +113,7 @@ public void saveTokenSecureFlagTrueUsingCustomizer() { } @Test - public void saveTokenSecureFlagFalse() { + void saveTokenSecureFlagFalse() { this.request.setSecure(true); this.repository.setSecure(Boolean.FALSE); CsrfToken token = this.repository.generateToken(this.request); @@ -123,7 +123,7 @@ public void saveTokenSecureFlagFalse() { } @Test - public void saveTokenSecureFlagFalseUsingCustomizer() { + void saveTokenSecureFlagFalseUsingCustomizer() { this.request.setSecure(true); this.repository.setCookieCustomizer(customizer -> customizer.secure(Boolean.FALSE)); CsrfToken token = this.repository.generateToken(this.request); @@ -133,7 +133,7 @@ public void saveTokenSecureFlagFalseUsingCustomizer() { } @Test - public void saveTokenNull() { + void saveTokenNull() { this.request.setSecure(true); this.repository.saveToken(null, this.request, this.response); Cookie tokenCookie = this.response.getCookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME); @@ -145,7 +145,7 @@ public void saveTokenNull() { } @Test - public void saveTokenHttpOnlyTrue() { + void saveTokenHttpOnlyTrue() { this.repository.setCookieHttpOnly(true); CsrfToken token = this.repository.generateToken(this.request); this.repository.saveToken(token, this.request, this.response); @@ -154,7 +154,7 @@ public void saveTokenHttpOnlyTrue() { } @Test - public void saveTokenHttpOnlyTrueUsingCustomizer() { + void saveTokenHttpOnlyTrueUsingCustomizer() { this.repository.setCookieCustomizer(customizer -> customizer.httpOnly(true)); CsrfToken token = this.repository.generateToken(this.request); this.repository.saveToken(token, this.request, this.response); @@ -163,7 +163,7 @@ public void saveTokenHttpOnlyTrueUsingCustomizer() { } @Test - public void saveTokenHttpOnlyFalse() { + void saveTokenHttpOnlyFalse() { this.repository.setCookieHttpOnly(false); CsrfToken token = this.repository.generateToken(this.request); this.repository.saveToken(token, this.request, this.response); @@ -172,7 +172,7 @@ public void saveTokenHttpOnlyFalse() { } @Test - public void saveTokenHttpOnlyFalseUsingCustomizer() { + void saveTokenHttpOnlyFalseUsingCustomizer() { this.repository.setCookieCustomizer(customizer -> customizer.httpOnly(false)); CsrfToken token = this.repository.generateToken(this.request); this.repository.saveToken(token, this.request, this.response); @@ -181,7 +181,7 @@ public void saveTokenHttpOnlyFalseUsingCustomizer() { } @Test - public void saveTokenWithHttpOnlyFalse() { + void saveTokenWithHttpOnlyFalse() { this.repository = CookieCsrfTokenRepository.withHttpOnlyFalse(); CsrfToken token = this.repository.generateToken(this.request); this.repository.saveToken(token, this.request, this.response); @@ -190,7 +190,7 @@ public void saveTokenWithHttpOnlyFalse() { } @Test - public void saveTokenCustomPath() { + void saveTokenCustomPath() { String customPath = "/custompath"; this.repository.setCookiePath(customPath); CsrfToken token = this.repository.generateToken(this.request); @@ -200,7 +200,7 @@ public void saveTokenCustomPath() { } @Test - public void saveTokenEmptyCustomPath() { + void saveTokenEmptyCustomPath() { String customPath = ""; this.repository.setCookiePath(customPath); CsrfToken token = this.repository.generateToken(this.request); @@ -210,7 +210,7 @@ public void saveTokenEmptyCustomPath() { } @Test - public void saveTokenNullCustomPath() { + void saveTokenNullCustomPath() { String customPath = null; this.repository.setCookiePath(customPath); CsrfToken token = this.repository.generateToken(this.request); @@ -220,7 +220,7 @@ public void saveTokenNullCustomPath() { } @Test - public void saveTokenWithCookieDomain() { + void saveTokenWithCookieDomain() { String domainName = "example.com"; this.repository.setCookieDomain(domainName); CsrfToken token = this.repository.generateToken(this.request); @@ -230,7 +230,7 @@ public void saveTokenWithCookieDomain() { } @Test - public void saveTokenWithCookieDomainUsingCustomizer() { + void saveTokenWithCookieDomainUsingCustomizer() { String domainName = "example.com"; this.repository.setCookieCustomizer(customizer -> customizer.domain(domainName)); CsrfToken token = this.repository.generateToken(this.request); @@ -240,7 +240,7 @@ public void saveTokenWithCookieDomainUsingCustomizer() { } @Test - public void saveTokenWithCookieMaxAge() { + void saveTokenWithCookieMaxAge() { int maxAge = 1200; this.repository.setCookieMaxAge(maxAge); CsrfToken token = this.repository.generateToken(this.request); @@ -250,7 +250,7 @@ public void saveTokenWithCookieMaxAge() { } @Test - public void saveTokenWithCookieMaxAgeUsingCustomizer() { + void saveTokenWithCookieMaxAgeUsingCustomizer() { int maxAge = 1200; this.repository.setCookieCustomizer(customizer -> customizer.maxAge(maxAge)); CsrfToken token = this.repository.generateToken(this.request); @@ -260,17 +260,17 @@ public void saveTokenWithCookieMaxAgeUsingCustomizer() { } @Test - public void saveTokenWithSameSiteNull() { + void saveTokenWithSameSiteNull() { String sameSitePolicy = null; this.repository.setCookieCustomizer(customizer -> customizer.sameSite(sameSitePolicy)); 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(null); + assertThat(((MockCookie)tokenCookie).getSameSite()).isNull(); } @Test - public void saveTokenWithSameSiteStrict() { + void saveTokenWithSameSiteStrict() { String sameSitePolicy = "Strict"; this.repository.setCookieCustomizer(customizer -> customizer.sameSite(sameSitePolicy)); CsrfToken token = this.repository.generateToken(this.request); @@ -280,7 +280,7 @@ public void saveTokenWithSameSiteStrict() { } @Test - public void saveTokenWithSameSiteLax() { + void saveTokenWithSameSiteLax() { String sameSitePolicy = "Lax"; this.repository.setCookieCustomizer(customizer -> customizer.sameSite(sameSitePolicy)); CsrfToken token = this.repository.generateToken(this.request); @@ -290,24 +290,24 @@ public void saveTokenWithSameSiteLax() { } @Test - public void loadTokenNoCookiesNull() { + void loadTokenNoCookiesNull() { assertThat(this.repository.loadToken(this.request)).isNull(); } @Test - public void loadTokenCookieIncorrectNameNull() { + void loadTokenCookieIncorrectNameNull() { this.request.setCookies(new Cookie("other", "name")); assertThat(this.repository.loadToken(this.request)).isNull(); } @Test - public void loadTokenCookieValueEmptyString() { + void loadTokenCookieValueEmptyString() { this.request.setCookies(new Cookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, "")); assertThat(this.repository.loadToken(this.request)).isNull(); } @Test - public void loadToken() { + void loadToken() { CsrfToken generateToken = this.repository.generateToken(this.request); this.request .setCookies(new Cookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, generateToken.getToken())); @@ -319,7 +319,7 @@ public void loadToken() { } @Test - public void loadTokenCustom() { + void loadTokenCustom() { String cookieName = "cookieName"; String value = "value"; String headerName = "headerName"; @@ -336,7 +336,7 @@ public void loadTokenCustom() { } @Test - public void loadDeferredTokenWhenDoesNotExistThenGeneratedAndSaved() { + void loadDeferredTokenWhenDoesNotExistThenGeneratedAndSaved() { DeferredCsrfToken deferredCsrfToken = this.repository.loadDeferredToken(this.request, this.response); CsrfToken csrfToken = deferredCsrfToken.get(); assertThat(csrfToken).isNotNull(); @@ -348,11 +348,11 @@ public void loadDeferredTokenWhenDoesNotExistThenGeneratedAndSaved() { assertThat(tokenCookie.getPath()).isEqualTo(this.request.getContextPath()); assertThat(tokenCookie.getSecure()).isEqualTo(this.request.isSecure()); assertThat(tokenCookie.getValue()).isEqualTo(csrfToken.getToken()); - assertThat(tokenCookie.isHttpOnly()).isEqualTo(true); + assertThat(tokenCookie.isHttpOnly()).isTrue(); } @Test - public void loadDeferredTokenWhenExistsAndNullSavedThenGeneratedAndSaved() { + void loadDeferredTokenWhenExistsAndNullSavedThenGeneratedAndSaved() { CsrfToken generatedToken = this.repository.generateToken(this.request); this.request .setCookies(new Cookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, generatedToken.getToken())); @@ -365,7 +365,7 @@ public void loadDeferredTokenWhenExistsAndNullSavedThenGeneratedAndSaved() { } @Test - public void loadDeferredTokenWhenExistsAndNullSavedAndNonNullSavedThenLoaded() { + void loadDeferredTokenWhenExistsAndNullSavedAndNonNullSavedThenLoaded() { CsrfToken generatedToken = this.repository.generateToken(this.request); this.request .setCookies(new Cookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, generatedToken.getToken())); @@ -378,7 +378,7 @@ public void loadDeferredTokenWhenExistsAndNullSavedAndNonNullSavedThenLoaded() { } @Test - public void loadDeferredTokenWhenExistsThenLoaded() { + void loadDeferredTokenWhenExistsThenLoaded() { CsrfToken generatedToken = this.repository.generateToken(this.request); this.request .setCookies(new Cookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, generatedToken.getToken())); @@ -389,7 +389,7 @@ public void loadDeferredTokenWhenExistsThenLoaded() { } @Test - public void cookieCustomizer() { + void cookieCustomizer() { String domainName = "example.com"; String customPath = "/custompath"; String sameSitePolicy = "Strict"; @@ -411,22 +411,22 @@ public void cookieCustomizer() { } @Test - public void setCookieNameNullIllegalArgumentException() { + void setCookieNameNullIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.repository.setCookieName(null)); } @Test - public void setParameterNameNullIllegalArgumentException() { + void setParameterNameNullIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.repository.setParameterName(null)); } @Test - public void setHeaderNameNullIllegalArgumentException() { + void setHeaderNameNullIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.repository.setHeaderName(null)); } @Test - public void setCookieMaxAgeZeroIllegalArgumentException() { + void setCookieMaxAgeZeroIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.repository.setCookieMaxAge(0)); } diff --git a/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java b/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java index c1375a791a4..3cd7f6d035e 100644 --- a/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java +++ b/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java @@ -38,7 +38,7 @@ * @author Alonso Araya * @since 5.1 */ -public class CookieServerCsrfTokenRepositoryTests { +class CookieServerCsrfTokenRepositoryTests { private CookieServerCsrfTokenRepository csrfTokenRepository; @@ -71,66 +71,66 @@ public void setUp() { } @Test - public void generateTokenWhenDefaultThenDefaults() { + void generateTokenWhenDefaultThenDefaults() { generateTokenAndAssertExpectedValues(); } @Test - public void generateTokenWhenCustomHeaderThenCustomHeader() { + void generateTokenWhenCustomHeaderThenCustomHeader() { setExpectedHeaderName("someHeader"); generateTokenAndAssertExpectedValues(); } @Test - public void generateTokenWhenCustomParameterThenCustomParameter() { + void generateTokenWhenCustomParameterThenCustomParameter() { setExpectedParameterName("someParam"); generateTokenAndAssertExpectedValues(); } @Test - public void generateTokenWhenCustomHeaderAndParameterThenCustomHeaderAndParameter() { + void generateTokenWhenCustomHeaderAndParameterThenCustomHeaderAndParameter() { setExpectedHeaderName("someHeader"); setExpectedParameterName("someParam"); generateTokenAndAssertExpectedValues(); } @Test - public void saveTokenWhenNoSubscriptionThenNotWritten() { + void saveTokenWhenNoSubscriptionThenNotWritten() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); this.csrfTokenRepository.saveToken(exchange, createToken()); assertThat(exchange.getResponse().getCookies().getFirst(this.expectedCookieName)).isNull(); } @Test - public void saveTokenWhenDefaultThenDefaults() { + void saveTokenWhenDefaultThenDefaults() { saveAndAssertExpectedValues(createToken()); } @Test - public void saveTokenWhenNullThenDeletes() { + void saveTokenWhenNullThenDeletes() { saveAndAssertExpectedValues(null); } @Test - public void saveTokenWhenHttpOnlyFalseThenHttpOnlyFalse() { + void saveTokenWhenHttpOnlyFalseThenHttpOnlyFalse() { setExpectedHttpOnly(false); saveAndAssertExpectedValues(createToken()); } @Test - public void saveTokenWhenCookieMaxAgeThenCookieMaxAge() { + void saveTokenWhenCookieMaxAgeThenCookieMaxAge() { setExpectedCookieMaxAge(3600); saveAndAssertExpectedValues(createToken()); } @Test - public void saveTokenWhenSameSiteThenCookieSameSite() { + void saveTokenWhenSameSiteThenCookieSameSite() { setExpectedSameSitePolicy("Lax"); saveAndAssertExpectedValues(createToken()); } @Test - public void saveTokenWhenCustomPropertiesThenCustomProperties() { + void saveTokenWhenCustomPropertiesThenCustomProperties() { setExpectedDomain("spring.io"); setExpectedCookieName("csrfCookie"); setExpectedPath("/some/path"); @@ -142,7 +142,7 @@ public void saveTokenWhenCustomPropertiesThenCustomProperties() { } @Test - public void saveTokenWhenCustomPropertiesThenCustomPropertiesUsingCustomizer() { + void saveTokenWhenCustomPropertiesThenCustomPropertiesUsingCustomizer() { String expectedDomain = "spring.io"; int expectedMaxAge = 3600; String expectedPath = "/some/path"; @@ -177,7 +177,7 @@ public void saveTokenWhenCustomPropertiesThenCustomPropertiesUsingCustomizer() { } @Test - public void saveTokenWhenSslInfoPresentThenSecure() { + void saveTokenWhenSslInfoPresentThenSecure() { this.request.sslInfo(new MockSslInfo()); MockServerWebExchange exchange = MockServerWebExchange.from(this.request); this.csrfTokenRepository.saveToken(exchange, createToken()).block(); @@ -187,7 +187,7 @@ public void saveTokenWhenSslInfoPresentThenSecure() { } @Test - public void saveTokenWhenSslInfoNullThenNotSecure() { + void saveTokenWhenSslInfoNullThenNotSecure() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); this.csrfTokenRepository.saveToken(exchange, createToken()).block(); ResponseCookie cookie = exchange.getResponse().getCookies().getFirst(this.expectedCookieName); @@ -196,7 +196,7 @@ public void saveTokenWhenSslInfoNullThenNotSecure() { } @Test - public void saveTokenWhenSecureFlagTrueThenSecure() { + void saveTokenWhenSecureFlagTrueThenSecure() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); this.csrfTokenRepository.setSecure(true); this.csrfTokenRepository.saveToken(exchange, createToken()).block(); @@ -206,7 +206,7 @@ public void saveTokenWhenSecureFlagTrueThenSecure() { } @Test - public void saveTokenWhenSecureFlagTrueThenSecureUsingCustomizer() { + void saveTokenWhenSecureFlagTrueThenSecureUsingCustomizer() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); this.csrfTokenRepository.setCookieCustomizer(customizer -> customizer.secure(true)); this.csrfTokenRepository.saveToken(exchange, createToken()).block(); @@ -216,7 +216,7 @@ public void saveTokenWhenSecureFlagTrueThenSecureUsingCustomizer() { } @Test - public void saveTokenWhenSecureFlagFalseThenNotSecure() { + void saveTokenWhenSecureFlagFalseThenNotSecure() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); this.csrfTokenRepository.setSecure(false); this.csrfTokenRepository.saveToken(exchange, createToken()).block(); @@ -226,7 +226,7 @@ public void saveTokenWhenSecureFlagFalseThenNotSecure() { } @Test - public void saveTokenWhenSecureFlagFalseThenNotSecureUsingCustomizer() { + void saveTokenWhenSecureFlagFalseThenNotSecureUsingCustomizer() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); this.csrfTokenRepository.setCookieCustomizer(customizer -> customizer.secure(false)); this.csrfTokenRepository.saveToken(exchange, createToken()).block(); @@ -236,7 +236,7 @@ public void saveTokenWhenSecureFlagFalseThenNotSecureUsingCustomizer() { } @Test - public void saveTokenWhenSecureFlagFalseAndSslInfoThenNotSecure() { + void saveTokenWhenSecureFlagFalseAndSslInfoThenNotSecure() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); this.request.sslInfo(new MockSslInfo()); this.csrfTokenRepository.setSecure(false); @@ -247,7 +247,7 @@ public void saveTokenWhenSecureFlagFalseAndSslInfoThenNotSecure() { } @Test - public void saveTokenWhenSecureFlagFalseAndSslInfoThenNotSecureUsingCustomizer() { + void saveTokenWhenSecureFlagFalseAndSslInfoThenNotSecureUsingCustomizer() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); this.request.sslInfo(new MockSslInfo()); this.csrfTokenRepository.setCookieCustomizer(customizer -> customizer.secure(false)); @@ -258,12 +258,12 @@ public void saveTokenWhenSecureFlagFalseAndSslInfoThenNotSecureUsingCustomizer() } @Test - public void loadTokenWhenCookieExistThenTokenFound() { + void loadTokenWhenCookieExistThenTokenFound() { loadAndAssertExpectedValues(); } @Test - public void loadTokenWhenCustomThenTokenFound() { + void loadTokenWhenCustomThenTokenFound() { setExpectedParameterName("paramName"); setExpectedHeaderName("headerName"); setExpectedCookieName("csrfCookie"); @@ -271,20 +271,20 @@ public void loadTokenWhenCustomThenTokenFound() { } @Test - public void loadTokenWhenNoCookiesThenNullToken() { + void loadTokenWhenNoCookiesThenNullToken() { MockServerWebExchange exchange = MockServerWebExchange.from(this.request); CsrfToken csrfToken = this.csrfTokenRepository.loadToken(exchange).block(); assertThat(csrfToken).isNull(); } @Test - public void loadTokenWhenCookieExistsWithNoValue() { + void loadTokenWhenCookieExistsWithNoValue() { setExpectedCookieValue(""); loadAndAssertExpectedValues(); } @Test - public void loadTokenWhenCookieExistsWithNullValue() { + void loadTokenWhenCookieExistsWithNullValue() { setExpectedCookieValue(null); loadAndAssertExpectedValues(); } From 58f14e404cda2ebf85ca6a25a3e1f5aa5040ece2 Mon Sep 17 00:00:00 2001 From: Alex Montoya Date: Tue, 15 Nov 2022 20:34:06 -0800 Subject: [PATCH 6/6] Add a customizer to CookieCsrfTokenRepository and CookieServerCsrfTokenRepository - Mark setCookieHttpOnly, setCookieDomain, setCookieMaxAge and setSecure as deprecated. - Add the method setCookieCustomizer which allows to set properties to the ResponseCookieBuilder without having to add new setter methods. Closes gh-12086 --- .../web/csrf/CookieCsrfTokenRepositoryTests.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) 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 index ce942298db8..15f8fe34a5b 100644 --- a/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java +++ b/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java @@ -71,7 +71,7 @@ void generateTokenCustom() { } @Test - void saveToken() { + void saveToken() { 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); @@ -113,7 +113,7 @@ void saveTokenSecureFlagTrueUsingCustomizer() { } @Test - void saveTokenSecureFlagFalse() { + void saveTokenSecureFlagFalse() { this.request.setSecure(true); this.repository.setSecure(Boolean.FALSE); CsrfToken token = this.repository.generateToken(this.request); @@ -348,11 +348,11 @@ void loadDeferredTokenWhenDoesNotExistThenGeneratedAndSaved() { assertThat(tokenCookie.getPath()).isEqualTo(this.request.getContextPath()); assertThat(tokenCookie.getSecure()).isEqualTo(this.request.isSecure()); assertThat(tokenCookie.getValue()).isEqualTo(csrfToken.getToken()); - assertThat(tokenCookie.isHttpOnly()).isTrue(); + assertThat(tokenCookie.isHttpOnly()).isEqualTo(true); } @Test - void loadDeferredTokenWhenExistsAndNullSavedThenGeneratedAndSaved() { + public void loadDeferredTokenWhenExistsAndNullSavedThenGeneratedAndSaved() { CsrfToken generatedToken = this.repository.generateToken(this.request); this.request .setCookies(new Cookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, generatedToken.getToken())); @@ -365,7 +365,7 @@ void loadDeferredTokenWhenExistsAndNullSavedThenGeneratedAndSaved() { } @Test - void loadDeferredTokenWhenExistsAndNullSavedAndNonNullSavedThenLoaded() { + public void loadDeferredTokenWhenExistsAndNullSavedAndNonNullSavedThenLoaded() { CsrfToken generatedToken = this.repository.generateToken(this.request); this.request .setCookies(new Cookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, generatedToken.getToken())); @@ -429,5 +429,4 @@ void setHeaderNameNullIllegalArgumentException() { void setCookieMaxAgeZeroIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.repository.setCookieMaxAge(0)); } - }