Allow to control Same-Site attribute for OIDC state cookie

docs/src/main/asciidoc/security-oidc-expanded-configuration.adoc

@@ -552,7 +552,8 @@ The common cookie properties impact both the authorization code flow session and
 |quarkus.oidc.authentication.cookie-path |'/'| The cookie path
 |quarkus.oidc.authentication.cookie-domain || The cookie domain
 |quarkus.oidc.authentication.cookie-path-header || The cookie path header
-|quarkus.oidc.authentication.cookie-same-site |lax| The cookie SameSite status
+|quarkus.oidc.authentication.cookie-same-site |lax| The session cookie SameSite status
+|quarkus.oidc.authentication.state-cookie-same-site |lax| The state cookie SameSite status
 |quarkus.oidc.authentication.cookie-suffix || The cookie suffix
 |quarkus.oidc.authentication.cookie-force-secure |false| The cookie force secure
 |====
@@ -562,7 +563,11 @@ You may also want to restrict it when the specific endpoint paths should only be
 
 `quarkus.oidc.authentication.cookie-path-header` can be used to dynamically set the required cookie path - this property should be used with care and only when it fits your deployment requirements.
 
-`quarkus.oidc.authentication.cookie-same-site` defines a `Same-Site` attribute as `lax` by default, since setting it to `strict` proved to be breaking some deployments. However, setting it to `strict` is RECOMMENDED when it is known to work in your deployment, for example, when the OIDC provider is hosted in the same domain as the application, etc.
+`quarkus.oidc.authentication.cookie-same-site` defines a `Same-Site` attribute for a session cookie as `lax` by default, since setting it to `strict` proved to be breaking some deployments. However, setting it to `strict` is RECOMMENDED when it is known to work in your deployment, for example, when the OIDC provider is hosted in the same domain as the application, etc.
+
+Similarly, `quarkus.oidc.authentication.state-cookie-same-site` defines a `Same-Site` attribute for a state cookie as `lax` by default, but setting it to `strict` is RECOMMENDED when it is known to work in your deployment.
+
+Ideally both the session and state cookies have the same `Same-Site` attribute value, however having the same `Same-Site` `strict` value for both cookies also proved to be problematic in some cases.
 
 `quarkus.oidc.authentication.cookie-suffix` can be used to customize the state and session cookie names. For example, by setting `%test.quarkus.oidc.authentication.cookie-suffix=test` you can have the session cookie name qualified with the `_test` suffix in the test profile only.
 

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java

@@ -1431,6 +1431,13 @@ public io.quarkus.oidc.runtime.OidcTenantConfig.Authentication.CookieSameSite co
                     : io.quarkus.oidc.runtime.OidcTenantConfig.Authentication.CookieSameSite.valueOf(cookieSameSite.toString());
         }
 
+        @Override
+        public io.quarkus.oidc.runtime.OidcTenantConfig.Authentication.CookieSameSite stateCookieSameSite() {
+            return stateCookieSameSite == null ? null
+                    : io.quarkus.oidc.runtime.OidcTenantConfig.Authentication.CookieSameSite
+                            .valueOf(stateCookieSameSite.toString());
+        }
+
         @Override
         public boolean allowMultipleCodeFlows() {
             return allowMultipleCodeFlows;
@@ -1713,6 +1720,11 @@ public enum ResponseMode {
          */
         public CookieSameSite cookieSameSite = CookieSameSite.LAX;
 
+        /**
+         * SameSite attribute for the state cookie.
+         */
+        public CookieSameSite stateCookieSameSite = CookieSameSite.LAX;
+
         /**
          * If a state cookie is present, a `state` query parameter must also be present and both the state
          * cookie name suffix and state cookie value must match the value of the `state` query parameter when
@@ -2137,6 +2149,7 @@ private void addConfigMappingValues(io.quarkus.oidc.runtime.OidcTenantConfig.Aut
             cookiePathHeader = mapping.cookiePathHeader();
             cookieDomain = mapping.cookieDomain();
             cookieSameSite = CookieSameSite.valueOf(mapping.cookieSameSite().toString());
+            stateCookieSameSite = CookieSameSite.valueOf(mapping.stateCookieSameSite().toString());
             allowMultipleCodeFlows = mapping.allowMultipleCodeFlows();
             failOnMissingStateParam = mapping.failOnMissingStateParam();
             failOnUnresolvedKid = mapping.failOnUnresolvedKid();

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java

@@ -62,7 +62,9 @@
 import io.smallrye.mutiny.Uni;
 import io.vertx.core.MultiMap;
 import io.vertx.core.http.Cookie;
+import io.vertx.core.http.CookieSameSite;
 import io.vertx.core.http.HttpHeaders;
+import io.vertx.core.http.impl.ServerCookie;
 import io.vertx.core.json.JsonArray;
 import io.vertx.core.json.JsonObject;
 import io.vertx.ext.web.RoutingContext;
@@ -1299,9 +1301,12 @@ private String generateCodeFlowState(RoutingContext context, TenantConfigContext
             cookieValue += (COOKIE_DELIM + encodeExtraStateValue(extraStateValue, configContext));
         }
         String stateCookieNameSuffix = configContext.oidcConfig().authentication().allowMultipleCodeFlows() ? "_" + uuid : "";
-        OidcUtils.createCookie(context, configContext.oidcConfig(),
+        ServerCookie stateCookie = OidcUtils.createCookie(context, configContext.oidcConfig(),
                 getStateCookieName(configContext.oidcConfig()) + stateCookieNameSuffix, cookieValue,
                 configContext.oidcConfig().authentication().stateCookieAge().toSeconds());
+        stateCookie
+                .setSameSite(CookieSameSite.valueOf(configContext.oidcConfig().authentication().stateCookieSameSite().name()));
+
         return uuid;
     }
 

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcTenantConfig.java

@@ -856,6 +856,12 @@ String directive() {
         @WithDefault("lax")
         CookieSameSite cookieSameSite();
 
+        /**
+         * SameSite attribute for the state cookie.
+         */
+        @WithDefault("lax")
+        CookieSameSite stateCookieSameSite();
+
         /**
          * If a state cookie is present, a `state` query parameter must also be present and both the state
          * cookie name suffix and state cookie value must match the value of the `state` query parameter when

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcUtils.java

@@ -502,6 +502,18 @@ static void removeCookie(RoutingContext context, ServerCookie cookie, OidcTenant
             if (auth.cookieDomain().isPresent()) {
                 cookie.setDomain(auth.cookieDomain().get());
             }
+
+            CookieSameSite sameSite = null;
+            if (cookie.getName().startsWith(SESSION_COOKIE_NAME)) {
+                sameSite = CookieSameSite.valueOf(oidcConfig.authentication().cookieSameSite().name());
+            } else if (cookie.getName().startsWith(STATE_COOKIE_NAME)) {
+                sameSite = CookieSameSite.valueOf(oidcConfig.authentication().stateCookieSameSite().name());
+            }
+            if (CookieSameSite.NONE == sameSite) {
+                // browsers may want it if the same site is none
+                cookie.setSameSite(sameSite);
+                cookie.setSecure(oidcConfig.authentication().cookieForceSecure() || context.request().isSSL());
+            }
         }
     }
 

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/builders/AuthenticationConfigBuilder.java

@@ -30,7 +30,8 @@ private record AuthenticationImpl(Optional<ResponseMode> responseMode, Optional<
             Optional<List<String>> scopes, Optional<String> scopeSeparator, boolean nonceRequired,
             Optional<Boolean> addOpenidScope, Map<String, String> extraParams, Optional<List<String>> forwardParams,
             boolean cookieForceSecure, Optional<String> cookieSuffix, String cookiePath, Optional<String> cookiePathHeader,
-            Optional<String> cookieDomain, CookieSameSite cookieSameSite, Optional<Set<CacheControl>> cacheControl,
+            Optional<String> cookieDomain, CookieSameSite cookieSameSite, CookieSameSite stateCookieSameSite,
+            Optional<Set<CacheControl>> cacheControl,
             boolean allowMultipleCodeFlows, boolean failOnMissingStateParam, boolean failOnUnresolvedKid,
             Optional<Boolean> userInfoRequired, Optional<Duration> sessionAgeExtension,
             Duration stateCookieAge, boolean javaScriptAutoRedirect, Optional<Boolean> idTokenRequired,
@@ -60,6 +61,7 @@ private record AuthenticationImpl(Optional<ResponseMode> responseMode, Optional<
     private Optional<String> cookiePathHeader;
     private Optional<String> cookieDomain;
     private CookieSameSite cookieSameSite;
+    private CookieSameSite stateCookieSameSite;
     private Set<CacheControl> cacheControl = new HashSet<>();
     private boolean allowMultipleCodeFlows;
     private boolean failOnMissingStateParam;
@@ -107,6 +109,7 @@ public AuthenticationConfigBuilder(OidcTenantConfigBuilder builder) {
         this.cookiePathHeader = authentication.cookiePathHeader();
         this.cookieDomain = authentication.cookieDomain();
         this.cookieSameSite = authentication.cookieSameSite();
+        this.stateCookieSameSite = authentication.stateCookieSameSite();
         if (authentication.cacheControl().isPresent()) {
             this.cacheControl.addAll(authentication.cacheControl().get());
         }
@@ -395,6 +398,15 @@ public AuthenticationConfigBuilder cookieSameSite(CookieSameSite cookieSameSite)
         return this;
     }
 
+    /**
+     * @param stateCookieSameSite {@link Authentication#stateCookieSameSite()}
+     * @return this builder
+     */
+    public AuthenticationConfigBuilder stateCookieSameSite(CookieSameSite cookieSameSite) {
+        this.stateCookieSameSite = Objects.requireNonNull(stateCookieSameSite);
+        return this;
+    }
+
     /**
      * Sets {@link Authentication#allowMultipleCodeFlows()} to true.
      *
@@ -657,7 +669,8 @@ public Authentication build() {
         return new AuthenticationImpl(responseMode, redirectPath, restorePathAfterRedirect, removeRedirectParameters, errorPath,
                 sessionExpiredPath, verifyAccessToken, forceRedirectHttpsScheme, optionalScopes, scopeSeparator, nonceRequired,
                 addOpenidScope, Map.copyOf(extraParams), optionalForwardParams, cookieForceSecure, cookieSuffix, cookiePath,
-                cookiePathHeader, cookieDomain, cookieSameSite, optionalCacheControl, allowMultipleCodeFlows,
+                cookiePathHeader, cookieDomain, cookieSameSite, stateCookieSameSite, optionalCacheControl,
+                allowMultipleCodeFlows,
                 failOnMissingStateParam,
                 failOnUnresolvedKid,
                 userInfoRequired, sessionAgeExtension, stateCookieAge, javaScriptAutoRedirect, idTokenRequired,

extensions/oidc/runtime/src/test/java/io/quarkus/oidc/runtime/OidcTenantConfigImpl.java

@@ -143,6 +143,7 @@ enum ConfigMappingMethods {
         AUTHENTICATION_COOKIE_PATH_HEADER,
         AUTHENTICATION_COOKIE_DOMAIN,
         AUTHENTICATION_COOKIE_SAME_SITE,
+        AUTHENTICATION_STATE_COOKIE_SAME_SITE,
         AUTHENTICATION_CACHE_CONTROL,
         AUTHENTICATION_ALLOW_MULTIPLE_CODE_FLOWS,
         AUTHENTICATION_FAIL_ON_MISSING_STATE_PARAM,
@@ -776,6 +777,12 @@ public CookieSameSite cookieSameSite() {
                 return CookieSameSite.LAX;
             }
 
+            @Override
+            public CookieSameSite stateCookieSameSite() {
+                invocationsRecorder.put(ConfigMappingMethods.AUTHENTICATION_STATE_COOKIE_SAME_SITE, true);
+                return CookieSameSite.LAX;
+            }
+
             @Override
             public Optional<Set<CacheControl>> cacheControl() {
                 invocationsRecorder.put(ConfigMappingMethods.AUTHENTICATION_CACHE_CONTROL, true);

integration-tests/oidc-code-flow/src/main/resources/application.properties

@@ -137,6 +137,7 @@ quarkus.oidc.tenant-https.authentication.pkce-required=true
 quarkus.oidc.tenant-https.authentication.nonce-required=true
 quarkus.oidc.tenant-https.authentication.pkce-secret=eUk1p7UB3nFiXZGUXi0uph1Y9p34YhBU
 quarkus.oidc.tenant-https.authentication.cookie-same-site=strict
+quarkus.oidc.tenant-https.authentication.state-cookie-same-site=none
 quarkus.oidc.tenant-https.authentication.fail-on-missing-state-param=true
 
 quarkus.oidc.tenant-nonce.auth-server-url=${quarkus.oidc.auth-server-url}

integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java

@@ -75,7 +75,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());
+            assertEquals("lax", stateCookie.getSameSite());
 
             webClient.getCookieManager().clearCookies();
 
@@ -294,7 +294,7 @@ public void testCodeFlowForceHttpsRedirectUriAndPkce() throws Exception {
             String endpointLocation = webResponse.getResponseHeaderValue("location");
 
             Cookie stateCookie = getStateCookie(webClient, "tenant-https_test");
-            assertNull(stateCookie.getSameSite());
+            assertEquals("none", stateCookie.getSameSite());
             verifyCodeVerifierAndNonce(stateCookie, keycloakUrl);
 
             assertTrue(endpointLocation.startsWith("https"));
@@ -369,7 +369,7 @@ public void testStateCookieIsPresentButStateParamNot() throws Exception {
 
             // State cookie is present
             Cookie stateCookie = getStateCookie(webClient, "tenant-https_test");
-            assertNull(stateCookie.getSameSite());
+            assertEquals("none", stateCookie.getSameSite());
             verifyCodeVerifierAndNonce(stateCookie, keycloakUrl);
 
             // Make a call without an extra state query param, status is 401