From a1b55f53881f6dbb0c995b7c76db083a18ca9502 Mon Sep 17 00:00:00 2001 From: Sergey Beryozkin Date: Wed, 20 Dec 2023 20:25:17 +0000 Subject: [PATCH] Split OIDC session cookie if its size is more than 4KB --- .../runtime/CodeAuthenticationMechanism.java | 68 ++++++++-------- .../runtime/DefaultTokenStateManager.java | 4 +- .../quarkus/oidc/runtime/OidcSessionImpl.java | 21 ++--- .../io/quarkus/oidc/runtime/OidcUtils.java | 77 ++++++++++++++++++- .../quarkus/oidc/runtime/OidcUtilsTest.java | 55 +++++++++++++ .../it/keycloak/CustomTenantResolver.java | 6 +- .../io/quarkus/it/keycloak/TenantNonce.java | 1 + .../io/quarkus/it/keycloak/CodeFlowTest.java | 66 +++++++++++++--- 8 files changed, 230 insertions(+), 68 deletions(-) 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 b451675273551..08a1829ba87a1 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 @@ -65,12 +65,10 @@ public class CodeAuthenticationMechanism extends AbstractOidcAuthenticationMecha static final String AMP = "&"; static final String EQ = "="; static final String COMMA = ","; - static final String UNDERSCORE = "_"; static final String COOKIE_DELIM = "|"; static final Pattern COOKIE_PATTERN = Pattern.compile("\\" + COOKIE_DELIM); static final String STATE_COOKIE_RESTORE_PATH = "restore-path"; static final Uni VOID_UNI = Uni.createFrom().voidItem(); - static final Integer MAX_COOKIE_VALUE_LENGTH = 4096; static final String NO_OIDC_COOKIES_AVAILABLE = "no_oidc_cookies"; private static final String INTERNAL_IDTOKEN_HEADER = "internal"; @@ -88,19 +86,17 @@ public CodeAuthenticationMechanism(BlockingSecurityExecutor blockingExecutor) { public Uni authenticate(RoutingContext context, IdentityProviderManager identityProviderManager, OidcTenantConfig oidcTenantConfig) { final Map cookies = context.request().cookieMap(); - - final Cookie sessionCookie = cookies.get(getSessionCookieName(oidcTenantConfig)); + final String sessionCookieValue = OidcUtils.getSessionCookie(context.data(), cookies, oidcTenantConfig); // If the session is already established then try to re-authenticate - if (sessionCookie != null) { + if (sessionCookieValue != null) { LOG.debug("Session cookie is present, starting the reauthentication"); - context.put(OidcUtils.SESSION_COOKIE_NAME, sessionCookie.getName()); Uni resolvedContext = resolver.resolveContext(context); return resolvedContext.onItem() .transformToUni(new Function>() { @Override public Uni apply(TenantConfigContext tenantContext) { - return reAuthenticate(sessionCookie, context, identityProviderManager, tenantContext); + return reAuthenticate(sessionCookieValue, context, identityProviderManager, tenantContext); } }); } @@ -299,14 +295,14 @@ private String getRequestParametersAsQuery(URI requestUri, MultiMap requestParam } } - private Uni reAuthenticate(Cookie sessionCookie, + private Uni reAuthenticate(String sessionCookie, RoutingContext context, IdentityProviderManager identityProviderManager, TenantConfigContext configContext) { context.put(TenantConfigContext.class.getName(), configContext); return resolver.getTokenStateManager().getTokens(context, configContext.oidcConfig, - sessionCookie.getValue(), getTokenStateRequestContext) + sessionCookie, getTokenStateRequestContext) .onFailure(AuthenticationCompletionException.class) .recoverWithUni( new Function>() { @@ -958,14 +954,14 @@ public Uni apply(Void t) { @Override public Void apply(String cookieValue) { - String sessionCookie = createCookie(context, configContext.oidcConfig, - getSessionCookieName(configContext.oidcConfig), - 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." - + " Browsers may ignore this cookie which will cause a new challenge for the authenticated users." - + " Recommendations: 1. Set 'quarkus.oidc.token-state-manager.split-tokens=true'" + String sessionName = OidcUtils.getSessionCookieName(configContext.oidcConfig); + LOG.debugf("Session cookie length for the tenant %s is %d bytes.", + configContext.oidcConfig.tenantId.get(), cookieValue.length()); + if (cookieValue.length() > OidcUtils.MAX_COOKIE_VALUE_LENGTH) { + LOG.debugf( + "Session cookie length is greater than %d bytes." + + " The cookie will be split to chunks to avoid browsers ignoring it." + + " Alternative recommendations: 1. Set 'quarkus.oidc.token-state-manager.split-tokens=true'" + " to have the ID, access and refresh tokens stored in separate cookies." + " 2. Set 'quarkus.oidc.token-state-manager.strategy=id-refresh-tokens' if you do not need to use the access token" + " as a source of roles or to request UserInfo or propagate it to the downstream services." @@ -973,7 +969,22 @@ public Void apply(String cookieValue) { + " but only if it is considered to be safe in your application's network." + " 4. Register a custom 'quarkus.oidc.TokenStateManager' CDI bean with the alternative priority set to 1.", configContext.oidcConfig.tenantId.get(), - MAX_COOKIE_VALUE_LENGTH); + OidcUtils.MAX_COOKIE_VALUE_LENGTH); + for (int sessionIndex = 1, + currentPos = 0; currentPos < cookieValue.length(); sessionIndex++) { + int nextPos = currentPos + OidcUtils.MAX_COOKIE_VALUE_LENGTH; + int nextValueUpperPos = nextPos < cookieValue.length() ? nextPos + : cookieValue.length(); + String nextValue = cookieValue.substring(currentPos, nextValueUpperPos); + // q_session_session_chunk_1, etc + String nextName = sessionName + OidcUtils.SESSION_COOKIE_CHUNK + sessionIndex; + createCookie(context, configContext.oidcConfig, nextName, nextValue, + sessionMaxAge, true); + currentPos = nextPos; + } + } else { + createCookie(context, configContext.oidcConfig, sessionName, cookieValue, + sessionMaxAge, true); } fireEvent(SecurityEvent.Type.OIDC_LOGIN, securityIdentity); return null; @@ -1307,30 +1318,15 @@ public Void apply(Void t) { } private static String getStateCookieName(OidcTenantConfig oidcConfig) { - return OidcUtils.STATE_COOKIE_NAME + getCookieSuffix(oidcConfig); + return OidcUtils.STATE_COOKIE_NAME + OidcUtils.getCookieSuffix(oidcConfig); } private static String getPostLogoutCookieName(OidcTenantConfig oidcConfig) { - return OidcUtils.POST_LOGOUT_COOKIE_NAME + getCookieSuffix(oidcConfig); - } - - private static String getSessionCookieName(OidcTenantConfig oidcConfig) { - return OidcUtils.SESSION_COOKIE_NAME + getCookieSuffix(oidcConfig); + return OidcUtils.POST_LOGOUT_COOKIE_NAME + OidcUtils.getCookieSuffix(oidcConfig); } private Uni removeSessionCookie(RoutingContext context, OidcTenantConfig oidcConfig) { - String cookieName = getSessionCookieName(oidcConfig); - return OidcUtils.removeSessionCookie(context, oidcConfig, cookieName, resolver.getTokenStateManager()); - } - - static String getCookieSuffix(OidcTenantConfig oidcConfig) { - String tenantId = oidcConfig.tenantId.get(); - boolean cookieSuffixConfigured = oidcConfig.authentication.cookieSuffix.isPresent(); - String tenantIdSuffix = (cookieSuffixConfigured || !"Default".equals(tenantId)) ? UNDERSCORE + tenantId : ""; - - return cookieSuffixConfigured - ? (tenantIdSuffix + UNDERSCORE + oidcConfig.authentication.cookieSuffix.get()) - : tenantIdSuffix; + return OidcUtils.removeSessionCookie(context, oidcConfig, resolver.getTokenStateManager()); } private class LogoutCall implements Function> { diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/DefaultTokenStateManager.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/DefaultTokenStateManager.java index fcc50bfe52a5f..bf23c7bf5f496 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/DefaultTokenStateManager.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/DefaultTokenStateManager.java @@ -136,12 +136,12 @@ private static ServerCookie getRefreshTokenCookie(RoutingContext routingContext, } private static String getAccessTokenCookieName(OidcTenantConfig oidcConfig) { - String cookieSuffix = CodeAuthenticationMechanism.getCookieSuffix(oidcConfig); + String cookieSuffix = OidcUtils.getCookieSuffix(oidcConfig); return SESSION_AT_COOKIE_NAME + cookieSuffix; } private static String getRefreshTokenCookieName(OidcTenantConfig oidcConfig) { - String cookieSuffix = CodeAuthenticationMechanism.getCookieSuffix(oidcConfig); + String cookieSuffix = OidcUtils.getCookieSuffix(oidcConfig); return SESSION_RT_COOKIE_NAME + cookieSuffix; } diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcSessionImpl.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcSessionImpl.java index 1d9ca56847639..5972902066f09 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcSessionImpl.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcSessionImpl.java @@ -35,20 +35,15 @@ public String getTenantId() { @Override public Uni logout() { - String sessionCookieName = routingContext.get(OidcUtils.SESSION_COOKIE_NAME); - if (sessionCookieName != null) { - Uni oidcConfigUni = resolver.resolveConfig(routingContext); - return oidcConfigUni.onItem().transformToUni(new Function>() { + Uni oidcConfigUni = resolver.resolveConfig(routingContext); + return oidcConfigUni.onItem().transformToUni(new Function>() { + @Override + public Uni apply(OidcTenantConfig oidcConfig) { + return OidcUtils.removeSessionCookie(routingContext, oidcConfig, + resolver.getTokenStateManager()); + } + }); - @Override - public Uni apply(OidcTenantConfig oidcConfig) { - return OidcUtils.removeSessionCookie(routingContext, oidcConfig, sessionCookieName, - resolver.getTokenStateManager()); - } - - }); - } - return Uni.createFrom().voidItem(); } @Override diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcUtils.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcUtils.java index 8fcc88f5c15ce..94220d432211f 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcUtils.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcUtils.java @@ -13,10 +13,13 @@ import java.util.Base64; import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.SortedMap; import java.util.StringTokenizer; +import java.util.TreeMap; import java.util.function.Consumer; import java.util.function.Function; import java.util.regex.Pattern; @@ -56,6 +59,7 @@ import io.smallrye.mutiny.subscription.UniEmitter; import io.vertx.core.Handler; import io.vertx.core.MultiMap; +import io.vertx.core.http.Cookie; import io.vertx.core.http.HttpHeaders; import io.vertx.core.http.HttpMethod; import io.vertx.core.http.impl.ServerCookie; @@ -72,8 +76,11 @@ public final class OidcUtils { public static final String TENANT_ID_ATTRIBUTE = "tenant-id"; public static final String DEFAULT_TENANT_ID = "Default"; public static final String SESSION_COOKIE_NAME = "q_session"; + public static final String SESSION_COOKIE_CHUNK = "_chunk_"; public static final String STATE_COOKIE_NAME = "q_auth"; + public static final Integer MAX_COOKIE_VALUE_LENGTH = 4096; public static final String POST_LOGOUT_COOKIE_NAME = "q_post_logout"; + static final String UNDERSCORE = "_"; static final Uni VOID_UNI = Uni.createFrom().voidItem(); static final BlockingTaskRunner deleteTokensRequestContext = new BlockingTaskRunner(); @@ -88,6 +95,64 @@ private OidcUtils() { } + public static String getSessionCookie(Map context, Map cookies, + OidcTenantConfig oidcTenantConfig) { + if (cookies.isEmpty()) { + return null; + } + final String sessionCookieName = OidcUtils.getSessionCookieName(oidcTenantConfig); + + if (cookies.containsKey(sessionCookieName)) { + context.put(OidcUtils.SESSION_COOKIE_NAME, List.of(sessionCookieName)); + return cookies.get(sessionCookieName).getValue(); + } else { + final String sessionChunkPrefix = sessionCookieName + OidcUtils.SESSION_COOKIE_CHUNK; + + SortedMap sessionCookies = new TreeMap<>(new Comparator() { + + @Override + public int compare(String s1, String s2) { + // at this point it is guaranteed cookie names end with `chunk_` + int lastUnderscoreIndex1 = s1.lastIndexOf(UNDERSCORE); + int lastUnderscoreIndex2 = s2.lastIndexOf(UNDERSCORE); + Integer pos1 = Integer.valueOf(s1.substring(lastUnderscoreIndex1 + 1)); + Integer pos2 = Integer.valueOf(s2.substring(lastUnderscoreIndex2 + 1)); + return pos1.compareTo(pos2); + } + + }); + for (String cookieName : cookies.keySet()) { + if (cookieName.startsWith(sessionChunkPrefix)) { + sessionCookies.put(cookieName, cookies.get(cookieName).getValue()); + } + } + if (!sessionCookies.isEmpty()) { + context.put(OidcUtils.SESSION_COOKIE_NAME, new ArrayList(sessionCookies.keySet())); + + StringBuilder sessionCookieValue = new StringBuilder(); + for (String value : sessionCookies.values()) { + sessionCookieValue.append(value); + } + return sessionCookieValue.toString(); + } + } + return null; + } + + public static String getSessionCookieName(OidcTenantConfig oidcConfig) { + return OidcUtils.SESSION_COOKIE_NAME + getCookieSuffix(oidcConfig); + } + + public static String getCookieSuffix(OidcTenantConfig oidcConfig) { + String tenantId = oidcConfig.tenantId.get(); + boolean cookieSuffixConfigured = oidcConfig.authentication.cookieSuffix.isPresent(); + String tenantIdSuffix = (cookieSuffixConfigured || !"Default".equals(tenantId)) ? UNDERSCORE + tenantId : ""; + + return cookieSuffixConfigured + ? (tenantIdSuffix + UNDERSCORE + oidcConfig.authentication.cookieSuffix.get()) + : tenantIdSuffix; + } + public static boolean isServiceApp(OidcTenantConfig oidcConfig) { return ApplicationType.SERVICE.equals(oidcConfig.applicationType.orElse(ApplicationType.SERVICE)); } @@ -380,11 +445,15 @@ public static void validatePrimaryJwtTokenType(OidcTenantConfig.Token tokenConfi } } - static Uni removeSessionCookie(RoutingContext context, OidcTenantConfig oidcConfig, String cookieName, + static Uni removeSessionCookie(RoutingContext context, OidcTenantConfig oidcConfig, TokenStateManager tokenStateManager) { - String cookieValue = removeCookie(context, oidcConfig, cookieName); - if (cookieValue != null) { - return tokenStateManager.deleteTokens(context, oidcConfig, cookieValue, + List cookieNames = context.get(SESSION_COOKIE_NAME); + if (cookieNames != null) { + StringBuilder cookieValue = new StringBuilder(); + for (String cookieName : cookieNames) { + cookieValue.append(removeCookie(context, oidcConfig, cookieName)); + } + return tokenStateManager.deleteTokens(context, oidcConfig, cookieValue.toString(), deleteTokensRequestContext); } else { return VOID_UNI; diff --git a/extensions/oidc/runtime/src/test/java/io/quarkus/oidc/runtime/OidcUtilsTest.java b/extensions/oidc/runtime/src/test/java/io/quarkus/oidc/runtime/OidcUtilsTest.java index b2ac3a6788151..3770db039424a 100644 --- a/extensions/oidc/runtime/src/test/java/io/quarkus/oidc/runtime/OidcUtilsTest.java +++ b/extensions/oidc/runtime/src/test/java/io/quarkus/oidc/runtime/OidcUtilsTest.java @@ -12,8 +12,11 @@ import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; import java.security.Permission; +import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.stream.Collectors; import javax.crypto.SecretKey; @@ -30,10 +33,62 @@ import io.quarkus.oidc.runtime.providers.KnownOidcProviders; import io.smallrye.jwt.algorithm.SignatureAlgorithm; import io.smallrye.jwt.build.Jwt; +import io.vertx.core.http.Cookie; +import io.vertx.core.http.impl.CookieImpl; import io.vertx.core.json.JsonObject; public class OidcUtilsTest { + @Test + public void testGetSingleSessionCookie() throws Exception { + + OidcTenantConfig oidcConfig = new OidcTenantConfig(); + oidcConfig.setTenantId("test"); + Map context = new HashMap<>(); + String sessionCookieValue = OidcUtils.getSessionCookie(context, + Map.of("q_session_test", new CookieImpl("q_session_test", "tokens")), oidcConfig); + assertEquals("tokens", sessionCookieValue); + @SuppressWarnings({ "rawtypes", "unchecked" }) + List names = (List) context.get(OidcUtils.SESSION_COOKIE_NAME); + assertEquals(1, names.size()); + assertEquals("q_session_test", names.get(0)); + } + + @Test + public void testGetMultipleSessionCookies() throws Exception { + + OidcTenantConfig oidcConfig = new OidcTenantConfig(); + oidcConfig.setTenantId("test"); + + char[] alphabet = "abcdefghijklmnopqrstuvwxyz".toCharArray(); + + StringBuilder expectedCookieValue = new StringBuilder(); + Map cookies = new HashMap<>(); + for (int i = 0; i < alphabet.length; i++) { + char[] data = new char[OidcUtils.MAX_COOKIE_VALUE_LENGTH]; + Arrays.fill(data, alphabet[i]); + String cookieName = "q_session_test_chunk_" + (i + 1); + String nextChunk = new String(data); + expectedCookieValue.append(nextChunk); + cookies.put(cookieName, new CookieImpl(cookieName, nextChunk)); + } + String lastChunk = String.valueOf("tokens"); + expectedCookieValue.append(lastChunk); + String lastCookieName = "q_session_test_chunk_" + (alphabet.length + 1); + cookies.put(lastCookieName, new CookieImpl(lastCookieName, lastChunk)); + + Map context = new HashMap<>(); + String sessionCookieValue = OidcUtils.getSessionCookie(context, cookies, oidcConfig); + assertEquals(expectedCookieValue.toString(), sessionCookieValue); + + @SuppressWarnings({ "rawtypes", "unchecked" }) + List names = (List) context.get(OidcUtils.SESSION_COOKIE_NAME); + assertEquals(alphabet.length + 1, names.size()); + for (int i = 0; i < names.size(); i++) { + assertEquals("q_session_test_chunk_" + (i + 1), names.get(i)); + } + } + @Test public void testAcceptGitHubProperties() throws Exception { OidcTenantConfig tenant = new OidcTenantConfig(); diff --git a/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/CustomTenantResolver.java b/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/CustomTenantResolver.java index 8763bf2ef5ebf..2915157d827e8 100644 --- a/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/CustomTenantResolver.java +++ b/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/CustomTenantResolver.java @@ -57,7 +57,8 @@ public String resolve(RoutingContext context) { } if (path.contains("tenant-https")) { - if (context.getCookie("q_session_tenant-https_test") != null) { + if (context.getCookie("q_session_tenant-https_test_chunk_1") != null + && context.getCookie("q_session_tenant-https_test_chunk_2") != null) { context.put("reauthenticated", "true"); return context.get(OidcUtils.TENANT_ID_ATTRIBUTE); } else { @@ -66,7 +67,8 @@ public String resolve(RoutingContext context) { } if (path.contains("tenant-nonce")) { - if (context.getCookie("q_session_tenant-nonce") != null) { + if (context.getCookie("q_session_tenant-nonce_chunk_1") != null + && context.getCookie("q_session_tenant-nonce_chunk_2") != null) { context.put("reauthenticated", "true"); return context.get(OidcUtils.TENANT_ID_ATTRIBUTE); } else { diff --git a/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/TenantNonce.java b/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/TenantNonce.java index ec3a17cbf2df2..fa6843a367221 100644 --- a/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/TenantNonce.java +++ b/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/TenantNonce.java @@ -19,6 +19,7 @@ public class TenantNonce { @GET @Authenticated public String getTenant() { + session.logout().await().indefinitely(); return session.getTenantId() + (routingContext.get("reauthenticated") != null ? ":reauthenticated" : ""); } } 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 8d55c8193dc06..f1b544141b972 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 @@ -13,7 +13,11 @@ import java.net.URI; import java.nio.charset.StandardCharsets; import java.time.Duration; +import java.util.ArrayList; import java.util.Base64; +import java.util.List; +import java.util.SortedMap; +import java.util.TreeMap; import java.util.concurrent.Callable; import java.util.concurrent.TimeUnit; @@ -24,6 +28,7 @@ import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import com.gargoylesoftware.htmlunit.CookieManager; import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; import com.gargoylesoftware.htmlunit.SilentCssErrorHandler; import com.gargoylesoftware.htmlunit.WebClient; @@ -221,9 +226,11 @@ public void testCodeFlowForceHttpsRedirectUriAndPkce() throws Exception { page = webClient.getPage(endpointLocationWithoutQueryUri.toURL()); assertEquals("tenant-https:reauthenticated", page.getBody().asNormalizedText()); - Cookie sessionCookie = getSessionCookie(webClient, "tenant-https_test"); - assertNotNull(sessionCookie); - assertEquals("strict", sessionCookie.getSameSite()); + + List sessionCookies = verifyTenantHttpTestCookies(webClient); + + assertEquals("strict", sessionCookies.get(0).getSameSite()); + assertEquals("strict", sessionCookies.get(1).getSameSite()); webClient.getCookieManager().clearCookies(); } } @@ -333,15 +340,19 @@ public void testCodeFlowForceHttpsRedirectUriWithQueryAndPkce() throws Exception URI endpointLocationWithoutQueryUri = URI.create(endpointLocationWithoutQuery); assertEquals("code=b", endpointLocationWithoutQueryUri.getRawQuery()); - Cookie sessionCookie = getSessionCookie(webClient, "tenant-https_test"); - assertNotNull(sessionCookie); + List sessionCookies = verifyTenantHttpTestCookies(webClient); + + StringBuilder sessionCookieValue = new StringBuilder(); + for (Cookie c : sessionCookies) { + sessionCookieValue.append(c.getValue()); + } SecretKey key = new SecretKeySpec(OidcUtils .getSha256Digest("secret".getBytes(StandardCharsets.UTF_8)), "AES"); - String sessionCookieValue = OidcUtils.decryptString(sessionCookie.getValue(), key); + String decryptedSessionCookieValue = OidcUtils.decryptString(sessionCookieValue.toString(), key); - String encodedIdToken = sessionCookieValue.split("\\|")[0]; + String encodedIdToken = decryptedSessionCookieValue.split("\\|")[0]; JsonObject idToken = OidcUtils.decodeJwtContent(encodedIdToken); String expiresAt = idToken.getInteger("exp").toString(); @@ -351,12 +362,22 @@ public void testCodeFlowForceHttpsRedirectUriWithQueryAndPkce() throws Exception response.startsWith("tenant-https:reauthenticated?code=b&expiresAt=" + expiresAt + "&expiresInDuration=")); Integer duration = Integer.valueOf(response.substring(response.length() - 1)); assertTrue(duration > 1 && duration < 5); - sessionCookie = getSessionCookie(webClient, "tenant-https_test"); - assertNotNull(sessionCookie); + + verifyTenantHttpTestCookies(webClient); + webClient.getCookieManager().clearCookies(); } } + private List verifyTenantHttpTestCookies(WebClient webClient) { + List sessionCookies = getSessionCookies(webClient, "tenant-https_test"); + assertNotNull(sessionCookies); + assertEquals(2, sessionCookies.size()); + assertEquals("q_session_tenant-https_test_chunk_1", sessionCookies.get(0).getName()); + assertEquals("q_session_tenant-https_test_chunk_2", sessionCookies.get(1).getName()); + return sessionCookies; + } + @Test public void testCodeFlowNonce() throws Exception { try (final WebClient webClient = createWebClient()) { @@ -391,13 +412,23 @@ public void testCodeFlowNonce() throws Exception { assertEquals(302, webResponse.getStatusCode()); assertNull(getStateCookie(webClient, "tenant-nonce")); + // At this point the session cookie is already available, this 2nd redirect only drops + // OIDC code flow parameters such as `code` and `state` + List sessionCookies = getSessionCookies(webClient, "tenant-nonce"); + assertNotNull(sessionCookies); + assertEquals(2, sessionCookies.size()); + assertEquals("q_session_tenant-nonce_chunk_1", sessionCookies.get(0).getName()); + assertEquals("q_session_tenant-nonce_chunk_2", sessionCookies.get(1).getName()); + String endpointLocationWithoutQuery = webResponse.getResponseHeaderValue("location"); URI endpointLocationWithoutQueryUri = URI.create(endpointLocationWithoutQuery); + // This request will reach the `TenantNonce` endpoint which will also clear the session. page = webClient.getPage(endpointLocationWithoutQueryUri.toURL()); assertEquals("tenant-nonce:reauthenticated", page.getBody().asNormalizedText()); - Cookie sessionCookie = getSessionCookie(webClient, "tenant-nonce"); - assertNotNull(sessionCookie); + + // both cookies should be gone now. + assertNull(getSessionCookies(webClient, "tenant-nonce")); webClient.getCookieManager().clearCookies(); } } @@ -1473,6 +1504,19 @@ private Cookie getSessionCookie(WebClient webClient, String tenantId) { return webClient.getCookieManager().getCookie("q_session" + (tenantId == null ? "_Default_test" : "_" + tenantId)); } + private List getSessionCookies(WebClient webClient, String tenantId) { + String sessionCookieNameChunk = "q_session" + (tenantId == null ? "_Default_test" : "_" + tenantId) + "_chunk_"; + CookieManager cookieManager = webClient.getCookieManager(); + SortedMap sessionCookies = new TreeMap<>(); + for (Cookie cookie : cookieManager.getCookies()) { + if (cookie.getName().startsWith(sessionCookieNameChunk)) { + sessionCookies.put(cookie.getName(), cookie); + } + } + + return sessionCookies.isEmpty() ? null : new ArrayList(sessionCookies.values()); + } + private Cookie getSessionAtCookie(WebClient webClient, String tenantId) { return webClient.getCookieManager().getCookie("q_session_at" + (tenantId == null ? "_Default_test" : "_" + tenantId)); }