Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the ability to set the SameSite policy to the CRSF Cookie #12237

Closed
wants to merge 2 commits into from
Closed

Add the ability to set the SameSite policy to the CRSF Cookie #12237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4075c5f
Add a customizer to CookieCsrfTokenRepository and CookieServerCsrfTok…
kumo829 Nov 18, 2022
853c458
JUnit 5 test cleanup for CookieCsrfTokenRepositoryTests and CookieSer…
kumo829 Nov 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 34 additions & 43 deletions web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@
*/

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.util.Assert;
import org.springframework.util.StringUtils;
import org.springframework.web.util.WebUtils;
Expand All @@ -34,6 +35,7 @@
*
* @author Rob Winch
* @author Steve Riesenberg
* @author Alex Montoya
* @since 4.1
*/
public final class CookieCsrfTokenRepository implements CsrfTokenRepository {
Expand Down Expand Up @@ -63,7 +65,17 @@ public final class CookieCsrfTokenRepository implements CsrfTokenRepository {

private int cookieMaxAge = -1;

public CookieCsrfTokenRepository() {
private Consumer<ResponseCookie.ResponseCookieBuilder> 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 cookieCustomizer consumer for a cookie builder
* @since 6.1
*/
public void setCookieCustomizer(Consumer<ResponseCookie.ResponseCookieBuilder> cookieCustomizer) {
Assert.notNull(cookieCustomizer, "cookieCustomizer must not be null");
this.cookieCustomizer = cookieCustomizer;
}

@Override
Expand All @@ -74,15 +86,17 @@ 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.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);

this.cookieCustomizer.accept(cookieBuilder);

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
Expand Down Expand Up @@ -143,11 +157,9 @@ public void setCookieName(String cookieName) {
}

/**
* Sets the HttpOnly attribute on the cookie containing the CSRF token. Defaults to
* <code>true</code>.
* @param cookieHttpOnly <code>true</code> sets the HttpOnly attribute,
* <code>false</code> does not set it
* @deprecated Use {@link #setCookieCustomizer(Consumer)} instead.
*/
@Deprecated(since = "6.1")
public void setCookieHttpOnly(boolean cookieHttpOnly) {
this.cookieHttpOnly = cookieHttpOnly;
}
Expand Down Expand Up @@ -191,51 +203,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 #setCookieCustomizer(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 #setCookieCustomizer(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.
*
* <p>
* A positive value indicates that the cookie will expire after that many seconds have
* passed. Note that the value is the <i>maximum</i> age when the cookie will expire,
* not the cookie's current age.
*
* <p>
* A negative value means that the cookie is not stored persistently and will be
* deleted when the Web browser exits.
*
* <p>
* 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 #setCookieCustomizer(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;
}

}
58 changes: 29 additions & 29 deletions ...in/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.springframework.security.web.server.csrf;

import java.util.UUID;
import java.util.function.Consumer;

import reactor.core.publisher.Mono;
import reactor.core.scheduler.Schedulers;
Expand All @@ -36,6 +37,7 @@
* @author Eric Deandrea
* @author Thomas Vitale
* @author Alonso Araya
* @author Alex Montoya
* @since 5.1
*/
public final class CookieServerCsrfTokenRepository implements ServerCsrfTokenRepository {
Expand All @@ -60,6 +62,19 @@ public final class CookieServerCsrfTokenRepository implements ServerCsrfTokenRep

private int cookieMaxAge = -1;

private Consumer<ResponseCookie.ResponseCookieBuilder> 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 cookieCustomizer consumer for a cookie builder
* @since 6.1
*/
public void setCookieCustomizer(Consumer<ResponseCookie.ResponseCookieBuilder> cookieCustomizer) {
Assert.notNull(cookieCustomizer, "cookieCustomizer must not be null");
this.cookieCustomizer = cookieCustomizer;
}

/**
* Factory method to conveniently create an instance that has
* {@link #setCookieHttpOnly(boolean)} set to false.
Expand All @@ -82,16 +97,18 @@ public Mono<Void> 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));

this.cookieCustomizer.accept(cookieBuilder);

// @formatter:on
exchange.getResponse().addCookie(cookie);
exchange.getResponse().addCookie(cookieBuilder.build());
});
}

Expand All @@ -107,9 +124,9 @@ public Mono<CsrfToken> 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 #setCookieCustomizer(Consumer)} instead.
*/
@Deprecated(since = "6.1")
public void setCookieHttpOnly(boolean cookieHttpOnly) {
this.cookieHttpOnly = cookieHttpOnly;
}
Expand Down Expand Up @@ -150,44 +167,27 @@ public void setCookiePath(String cookiePath) {
}

/**
* Sets the cookie domain
* @param cookieDomain The cookie domain
* @deprecated Use {@link #setCookieCustomizer(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 #setCookieCustomizer(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.
*
* <p>
* A positive value indicates that the cookie will expire after that many seconds have
* passed. Note that the value is the <i>maximum</i> age when the cookie will expire,
* not the cookie's current age.
*
* <p>
* A negative value means that the cookie is not stored persistently and will be
* deleted when the Web browser exits.
*
* <p>
* 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 #setCookieCustomizer(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;
Expand Down
Loading