From c4c40bee92ed6301e4841e832c55ceb896b0a39b Mon Sep 17 00:00:00 2001 From: Sergey Beryozkin Date: Sun, 29 Jan 2023 17:10:14 +0000 Subject: [PATCH] Set SameSite Strict only on OIDC session cookie --- .../main/java/io/quarkus/oidc/OidcTenantConfig.java | 4 ++-- .../oidc/runtime/CodeAuthenticationMechanism.java | 11 +++++++++-- .../java/io/quarkus/it/keycloak/CodeFlowTest.java | 13 ++++++------- 3 files changed, 17 insertions(+), 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 index e25c963ca59bd..ebe214e2d7ea3 100644 --- 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 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/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java index 4dd98f1943255..fdf6a4fb4df6a 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java @@ -799,7 +799,7 @@ public Uni 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 index e35ee29a41bc9..67f549bef42b0 100644 --- 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(); } }