Merge pull request #30722 from sberyozkin/oidc_only_session_cookie_st…

…rict Fixes #30625
quarkusio · Jan 30, 2023 · 39ecad5 · 39ecad5
2 parents 1345c2e + c4c40be
commit 39ecad5
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 11 deletions.
diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java
@@ -603,7 +603,7 @@ public static enum Source {
     public static class Authentication {
 
         /**
-         * SameSite attribute values for the session, state and post logout cookies.
+         * SameSite attribute values for the session cookie.
          */
         public enum CookieSameSite {
             STRICT,
@@ -767,7 +767,7 @@ public enum ResponseMode {
         public Optional<String> cookieDomain = Optional.empty();
 
         /**
-         * SameSite attribute for the session, state and post logout cookies.
+         * SameSite attribute for the session cookie.
          */
         @ConfigItem(defaultValue = "strict")
         public CookieSameSite cookieSameSite = CookieSameSite.STRICT;

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
@@ -799,7 +799,7 @@ public Uni<? extends Void> apply(Void t) {
                                     public Void apply(String cookieValue) {
                                         String sessionCookie = createCookie(context, configContext.oidcConfig,
                                                 getSessionCookieName(configContext.oidcConfig),
-                                                cookieValue, sessionMaxAge).getValue();
+                                                cookieValue, sessionMaxAge, true).getValue();
                                         if (sessionCookie.length() >= MAX_COOKIE_VALUE_LENGTH) {
                                             LOG.warnf(
                                                     "Session cookie length for the tenant %s is equal or greater than %d bytes."
@@ -914,6 +914,11 @@ private String generatePostLogoutState(RoutingContext context, TenantConfigConte
 
     static ServerCookie createCookie(RoutingContext context, OidcTenantConfig oidcConfig,
             String name, String value, long maxAge) {
+        return createCookie(context, oidcConfig, name, value, maxAge, false);
+    }
+
+    static ServerCookie createCookie(RoutingContext context, OidcTenantConfig oidcConfig,
+            String name, String value, long maxAge, boolean sessionCookie) {
         ServerCookie cookie = new CookieImpl(name, value);
         cookie.setHttpOnly(true);
         cookie.setSecure(oidcConfig.authentication.cookieForceSecure || context.request().isSSL());
@@ -924,7 +929,9 @@ static ServerCookie createCookie(RoutingContext context, OidcTenantConfig oidcCo
         if (auth.cookieDomain.isPresent()) {
             cookie.setDomain(auth.getCookieDomain().get());
         }
-        cookie.setSameSite(CookieSameSite.valueOf(auth.cookieSameSite.name()));
+        if (sessionCookie) {
+            cookie.setSameSite(CookieSameSite.valueOf(auth.cookieSameSite.name()));
+        }
         context.response().addCookie(cookie);
         return cookie;
     }

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
@@ -55,9 +55,9 @@ public void testCodeFlowNoConsent() throws IOException {
                     .loadWebResponse(new WebRequest(URI.create("http://localhost:8081/index.html").toURL()));
             verifyLocationHeader(webClient, webResponse.getResponseHeaderValue("location"), null, "web-app", false);
 
-            String stateCookieString = webResponse.getResponseHeaderValue("Set-Cookie");
-            assertTrue(stateCookieString.startsWith("q_auth_Default_test="));
-            assertTrue(stateCookieString.contains("SameSite=Strict"));
+            Cookie stateCookie = getStateCookie(webClient, null);
+            assertNotNull(stateCookie);
+            assertNull(stateCookie.getSameSite());
 
             webClient.getCookieManager().clearCookies();
 
@@ -95,6 +95,7 @@ public void testCodeFlowNoConsent() throws IOException {
 
             Cookie sessionCookie = getSessionCookie(webClient, null);
             assertNotNull(sessionCookie);
+            assertEquals("strict", sessionCookie.getSameSite());
 
             webClient.getCookieManager().clearCookies();
         }
@@ -176,10 +177,6 @@ public void testCodeFlowForceHttpsRedirectUriAndPkce() throws Exception {
             verifyLocationHeader(webClient, keycloakUrl, "tenant-https_test", "xforwarded%2Ftenant-https",
                     true);
 
-            String stateCookieString = webResponse.getResponseHeaderValue("Set-Cookie");
-            assertTrue(stateCookieString.startsWith("q_auth_tenant-https_test="));
-            assertTrue(stateCookieString.contains("SameSite=Lax"));
-
             HtmlPage page = webClient.getPage(keycloakUrl);
 
             assertEquals("Sign in to quarkus", page.getTitleText());
@@ -195,6 +192,7 @@ public void testCodeFlowForceHttpsRedirectUriAndPkce() throws Exception {
             String endpointLocation = webResponse.getResponseHeaderValue("location");
 
             Cookie stateCookie = getStateCookie(webClient, "tenant-https_test");
+            assertNull(stateCookie.getSameSite());
             verifyCodeVerifier(stateCookie, keycloakUrl);
 
             assertTrue(endpointLocation.startsWith("https"));
@@ -222,6 +220,7 @@ public void testCodeFlowForceHttpsRedirectUriAndPkce() throws Exception {
             assertEquals("tenant-https:reauthenticated", page.getBody().asNormalizedText());
             Cookie sessionCookie = getSessionCookie(webClient, "tenant-https_test");
             assertNotNull(sessionCookie);
+            assertEquals("lax", sessionCookie.getSameSite());
             webClient.getCookieManager().clearCookies();
         }
     }