Skip to content

Commit

Permalink
Reset CSRF cookie to minimize a risk of failures due to its expiry
Browse files Browse the repository at this point in the history
  • Loading branch information
sberyozkin committed Dec 26, 2023
1 parent 4904645 commit 5a95be2
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public class CsrfReactiveConfig {
/**
* CSRF cookie max age.
*/
@ConfigItem(defaultValue = "10M")
@ConfigItem(defaultValue = "2H")
public Duration cookieMaxAge;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public class CsrfRequestResponseReactiveFilter {
*/
private static final String CSRF_TOKEN_KEY = "csrf_token";
private static final String CSRF_TOKEN_BYTES_KEY = "csrf_token_bytes";
private static final String NEW_COOKIE_REQUIRED = "true";

/**
* CSRF token verification status.
Expand Down Expand Up @@ -85,12 +86,14 @@ public void filter(ResteasyReactiveContainerRequestContext requestContext, Routi

if (requestMethodIsSafe(requestContext)) {
// safe HTTP method, tolerate the absence of a token
if (cookieToken == null && isCsrfTokenRequired(routing, config)) {
if (isCsrfTokenRequired(routing, config)) {
// Set the CSRF cookie with a randomly generated value
byte[] tokenBytes = new byte[config.tokenSize];
secureRandom.nextBytes(tokenBytes);
routing.put(CSRF_TOKEN_BYTES_KEY, tokenBytes);
routing.put(CSRF_TOKEN_KEY, Base64.getUrlEncoder().withoutPadding().encodeToString(tokenBytes));

routing.put(NEW_COOKIE_REQUIRED, true);
}
} else if (config.verifyToken) {
// unsafe HTTP method, token is required
Expand Down Expand Up @@ -129,7 +132,6 @@ public void filter(ResteasyReactiveContainerRequestContext requestContext, Routi
String csrfTokenFormParam = (String) rrContext.getFormParameter(config.formFieldName, true, false);
LOG.debugf("CSRF token found in the form parameter");
verifyCsrfToken(requestContext, routing, config, cookieToken, csrfTokenFormParam);
return;

} else if (cookieToken == null) {
LOG.debug("CSRF token is not found");
Expand Down Expand Up @@ -159,6 +161,8 @@ private void verifyCsrfToken(ResteasyReactiveContainerRequestContext requestCont
} else {
routing.put(CSRF_TOKEN_KEY, csrfToken);
routing.put(CSRF_TOKEN_VERIFIED, true);
// reset the cookie
routing.put(NEW_COOKIE_REQUIRED, true);
return;
}
}
Expand Down Expand Up @@ -195,9 +199,9 @@ private static Response badClientRequest() {
@ServerResponseFilter
public void filter(ContainerRequestContext requestContext,
ContainerResponseContext responseContext, RoutingContext routing) {
final CsrfReactiveConfig config = configInstance.get();
if (requestContext.getMethod().equals("GET") && isCsrfTokenRequired(routing, config)
&& getCookieToken(routing, config) == null) {
if (routing.get(NEW_COOKIE_REQUIRED) != null) {

final CsrfReactiveConfig config = configInstance.get();

String cookieValue = null;
if (config.tokenSignatureKey.isPresent()) {
Expand Down Expand Up @@ -230,7 +234,7 @@ && getCookieToken(routing, config) == null) {
*
* @return An Optional containing the token, or an empty Optional if the token cookie is not present or is invalid
*/
private String getCookieToken(RoutingContext routing, CsrfReactiveConfig config) {
private static String getCookieToken(RoutingContext routing, CsrfReactiveConfig config) {
Cookie cookie = routing.getCookie(config.cookieName);

if (cookie == null) {
Expand All @@ -241,14 +245,14 @@ private String getCookieToken(RoutingContext routing, CsrfReactiveConfig config)
return cookie.getValue();
}

private boolean isCsrfTokenRequired(RoutingContext routing, CsrfReactiveConfig config) {
private static boolean isCsrfTokenRequired(RoutingContext routing, CsrfReactiveConfig config) {
return config.createTokenPath
.map(value -> value.contains(routing.normalizedPath())).orElse(true);
}

private void createCookie(String csrfToken, RoutingContext routing, CsrfReactiveConfig config) {
private static void createCookie(String cookieTokenValue, RoutingContext routing, CsrfReactiveConfig config) {

ServerCookie cookie = new CookieImpl(config.cookieName, csrfToken);
ServerCookie cookie = new CookieImpl(config.cookieName, cookieTokenValue);
cookie.setHttpOnly(config.cookieHttpOnly);
cookie.setSecure(config.cookieForceSecure || routing.request().isSSL());
cookie.setMaxAge(config.cookieMaxAge.toSeconds());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ public void testCsrfTokenInForm() throws Exception {
try (final WebClient webClient = createWebClient()) {
webClient.addRequestHeader("Authorization", basicAuth("alice", "alice"));
HtmlPage htmlPage = webClient.getPage("http://localhost:8081/service/csrfTokenForm");
htmlPage = webClient.getPage("http://localhost:8081/service/csrfTokenForm");
assertNotNull(htmlPage.getWebResponse().getResponseHeaderValue("Set-Cookie"));

assertEquals("CSRF Token Form Test", htmlPage.getTitleText());

Expand All @@ -51,11 +53,19 @@ public void testCsrfTokenInForm() throws Exception {
assertNotNull(webClient.getCookieManager().getCookie("csrftoken"));

TextPage textPage = loginForm.getInputByName("submit").click();

assertNotNull(htmlPage.getWebResponse().getResponseHeaderValue("Set-Cookie"));
assertEquals("alice:true:tokenHeaderIsSet=false", textPage.getContent());

// This request which returns String is not CSRF protected
textPage = webClient.getPage("http://localhost:8081/service/hello");
assertEquals("hello", textPage.getContent());
// therefore no Set-Cookie header is expected
assertNull(textPage.getWebResponse().getResponseHeaderValue("Set-Cookie"));

// Repeat a form submission
textPage = loginForm.getInputByName("submit").click();
assertNotNull(htmlPage.getWebResponse().getResponseHeaderValue("Set-Cookie"));
assertEquals("alice:true:tokenHeaderIsSet=false", textPage.getContent());

webClient.getCookieManager().clearCookies();
}
Expand Down

0 comments on commit 5a95be2

Please sign in to comment.