From 5a95be2b559d2ccb320a3eb24a043ab857269fe1 Mon Sep 17 00:00:00 2001 From: Sergey Beryozkin Date: Wed, 13 Dec 2023 22:55:09 +0000 Subject: [PATCH] Reset CSRF cookie to minimize a risk of failures due to its expiry --- .../reactive/runtime/CsrfReactiveConfig.java | 2 +- .../CsrfRequestResponseReactiveFilter.java | 22 +++++++++++-------- .../io/quarkus/it/csrf/CsrfReactiveTest.java | 12 +++++++++- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/extensions/csrf-reactive/runtime/src/main/java/io/quarkus/csrf/reactive/runtime/CsrfReactiveConfig.java b/extensions/csrf-reactive/runtime/src/main/java/io/quarkus/csrf/reactive/runtime/CsrfReactiveConfig.java index e525994361f12..6d7717e0a8d3f 100644 --- a/extensions/csrf-reactive/runtime/src/main/java/io/quarkus/csrf/reactive/runtime/CsrfReactiveConfig.java +++ b/extensions/csrf-reactive/runtime/src/main/java/io/quarkus/csrf/reactive/runtime/CsrfReactiveConfig.java @@ -34,7 +34,7 @@ public class CsrfReactiveConfig { /** * CSRF cookie max age. */ - @ConfigItem(defaultValue = "10M") + @ConfigItem(defaultValue = "2H") public Duration cookieMaxAge; /** diff --git a/extensions/csrf-reactive/runtime/src/main/java/io/quarkus/csrf/reactive/runtime/CsrfRequestResponseReactiveFilter.java b/extensions/csrf-reactive/runtime/src/main/java/io/quarkus/csrf/reactive/runtime/CsrfRequestResponseReactiveFilter.java index b75764772a912..25df381ac7b59 100644 --- a/extensions/csrf-reactive/runtime/src/main/java/io/quarkus/csrf/reactive/runtime/CsrfRequestResponseReactiveFilter.java +++ b/extensions/csrf-reactive/runtime/src/main/java/io/quarkus/csrf/reactive/runtime/CsrfRequestResponseReactiveFilter.java @@ -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. @@ -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 @@ -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"); @@ -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; } } @@ -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()) { @@ -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) { @@ -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()); diff --git a/integration-tests/csrf-reactive/src/test/java/io/quarkus/it/csrf/CsrfReactiveTest.java b/integration-tests/csrf-reactive/src/test/java/io/quarkus/it/csrf/CsrfReactiveTest.java index 770d36441e300..a8a937e1906a8 100644 --- a/integration-tests/csrf-reactive/src/test/java/io/quarkus/it/csrf/CsrfReactiveTest.java +++ b/integration-tests/csrf-reactive/src/test/java/io/quarkus/it/csrf/CsrfReactiveTest.java @@ -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()); @@ -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(); }