Merge pull request #37725 from sberyozkin/csrf_refresh_cookie

Reset CSRF cookie to minimize a risk of failures due to its expiry
quarkusio · Dec 26, 2023 · 7d15333 · 7d15333
2 parents 4904645 + 5a95be2
commit 7d15333
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 11 deletions.
diff --git a/...f-reactive/runtime/src/main/java/io/quarkus/csrf/reactive/runtime/CsrfReactiveConfig.java b/...f-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/...ime/src/main/java/io/quarkus/csrf/reactive/runtime/CsrfRequestResponseReactiveFilter.java b/...ime/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
@@ -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();
         }