Do not make OIDC state cookie name unique if multiple code flows are …

…not allowed
quarkusio · Jul 17, 2023 · 8007ddf · 8007ddf
1 parent de112b9
commit 8007ddf
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 5 deletions.
diff --git a/...sions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java b/...sions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java
@@ -154,8 +154,10 @@ private Uni<SecurityIdentity> processRedirectFromOidc(RoutingContext context, Oi
             return stateParamIsMissing(oidcTenantConfig, context, cookies, stateQueryParam.size() > 1);
         }
 
+        String stateCookieNameSuffix = oidcTenantConfig.authentication.allowMultipleCodeFlows ? "_" + stateQueryParam.get(0)
+                : "";
         final Cookie stateCookie = context.request().getCookie(
-                getStateCookieName(oidcTenantConfig) + "_" + stateQueryParam.get(0));
+                getStateCookieName(oidcTenantConfig) + stateCookieNameSuffix);
 
         if (stateCookie == null) {
             return stateCookieIsMissing(oidcTenantConfig, context, cookies);
@@ -971,8 +973,9 @@ private String generateCodeFlowState(RoutingContext context, TenantConfigContext
             extraStateValue.setRestorePath("?" + context.request().query());
             cookieValue += (COOKIE_DELIM + encodeExtraStateValue(extraStateValue, configContext));
         }
+        String stateCookieNameSuffix = configContext.oidcConfig.authentication.allowMultipleCodeFlows ? "_" + uuid : "";
         createCookie(context, configContext.oidcConfig,
-                getStateCookieName(configContext.oidcConfig) + "_" + uuid, cookieValue, 60 * 30);
+                getStateCookieName(configContext.oidcConfig) + stateCookieNameSuffix, cookieValue, 60 * 30);
         return uuid;
     }
 

diff --git a/integration-tests/oidc-code-flow/src/main/resources/application.properties b/integration-tests/oidc-code-flow/src/main/resources/application.properties
@@ -43,6 +43,7 @@ quarkus.oidc.tenant-jwt.client-id=quarkus-app-jwt
 quarkus.oidc.tenant-jwt.credentials.jwt.secret=AyM1SysPpbyDfgZld3umj1qzKObwVMkoqQ-EstJQLr_T-1qS0gZH75aKtMN3Yj0iPS4hcgUuTwjAzZr1Z9CAow
 quarkus.oidc.tenant-jwt.token.issuer=${quarkus.oidc.auth-server-url}
 quarkus.oidc.tenant-jwt.authentication.redirect-path=/web-app/callback-jwt-after-redirect
+quarkus.oidc.tenant-jwt.authentication.allow-multiple-code-flows=false
 quarkus.oidc.tenant-jwt.application-type=web-app
 
 # Tenant with client which needs to use client_secret_jwt but uses client_secret_post

diff --git a/integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java b/integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java
@@ -59,6 +59,7 @@ public void testCodeFlowNoConsent() throws IOException {
 
             Cookie stateCookie = getStateCookie(webClient, null);
             assertNotNull(stateCookie);
+            assertEquals(stateCookie.getName(), "q_auth_Default_test_" + getStateCookieStateParam(stateCookie));
             assertNull(stateCookie.getSameSite());
 
             webClient.getCookieManager().clearCookies();
@@ -673,9 +674,10 @@ public void testIdTokenInjectionJwtMethod() throws IOException, InterruptedExcep
             WebResponse webResponse = webClient
                     .loadWebResponse(
                             new WebRequest(URI.create("http://localhost:8081/web-app/callback-jwt-before-redirect").toURL()));
-            assertNotNull(getStateCookie(webClient, "tenant-jwt"));
-            assertNotNull(getStateCookieStateParam(webClient, "tenant-jwt"));
-            assertNull(getStateCookieSavedPath(webClient, "tenant-jwt"));
+            Cookie stateCookie = getNonUniqueStateCookie(webClient, "tenant-jwt");
+            assertEquals(stateCookie.getName(), "q_auth_tenant-jwt");
+            assertNotNull(getStateCookieStateParam(stateCookie));
+            assertNull(getStateCookieSavedPath(stateCookie));
 
             HtmlPage page = webClient.getPage(webResponse.getResponseHeaderValue("location"));
             assertEquals("Sign in to quarkus", page.getTitleText());
@@ -1265,15 +1267,29 @@ private Cookie getStateCookie(WebClient webClient, String tenantId) {
         return null;
     }
 
+    private Cookie getNonUniqueStateCookie(WebClient webClient, String tenantId) {
+        String cookieName = "q_auth" + (tenantId == null ? "_Default_test" : "_" + tenantId);
+        return webClient.getCookieManager().getCookie(cookieName);
+    }
+
     private String getStateCookieStateParam(WebClient webClient, String tenantId) {
         return getStateCookie(webClient, tenantId).getValue().split("\\|")[0];
     }
 
+    private String getStateCookieStateParam(Cookie stateCookie) {
+        return stateCookie.getValue().split("\\|")[0];
+    }
+
     private String getStateCookieSavedPath(WebClient webClient, String tenantId) {
         String[] parts = getStateCookie(webClient, tenantId).getValue().split("\\|");
         return parts.length == 2 ? parts[1] : null;
     }
 
+    private String getStateCookieSavedPath(Cookie stateCookie) {
+        String[] parts = stateCookie.getValue().split("\\|");
+        return parts.length == 2 ? parts[1] : null;
+    }
+
     private Cookie getSessionCookie(WebClient webClient, String tenantId) {
         return webClient.getCookieManager().getCookie("q_session" + (tenantId == null ? "_Default_test" : "_" + tenantId));
     }