From 61529c2ffe3db6ae2ef8588159e653fdefb67244 Mon Sep 17 00:00:00 2001 From: Sergey Beryozkin Date: Thu, 28 Mar 2024 18:06:04 +0000 Subject: [PATCH] Fix OIDC cookie related tenant id and chunk calculation issues --- .../security-openid-connect-multitenancy.adoc | 414 ++++++++++++------ .../oidc/test/CodeFlowDevModeTestCase.java | 25 +- .../CodeTenantReauthenticateTestCase.java | 173 ++++++++ .../oidc/test/CustomTenantConfigResolver.java | 6 +- .../oidc/test/CustomTenantResolver.java | 19 + .../oidc/test/TenantReauthentication.java | 35 ++ ...plication-tenant-reauthenticate.properties | 11 + .../runtime/CodeAuthenticationMechanism.java | 21 +- .../runtime/DefaultTenantConfigResolver.java | 15 + .../runtime/OidcAuthenticationMechanism.java | 40 +- .../io/quarkus/oidc/runtime/OidcUtils.java | 40 +- .../quarkus/oidc/runtime/OidcUtilsTest.java | 10 + .../io/quarkus/it/keycloak/CodeFlowTest.java | 11 +- .../BearerTokenAuthorizationTest.java | 4 +- 14 files changed, 671 insertions(+), 153 deletions(-) create mode 100644 extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CodeTenantReauthenticateTestCase.java create mode 100644 extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CustomTenantResolver.java create mode 100644 extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/TenantReauthentication.java create mode 100644 extensions/oidc/deployment/src/test/resources/application-tenant-reauthenticate.properties diff --git a/docs/src/main/asciidoc/security-openid-connect-multitenancy.adoc b/docs/src/main/asciidoc/security-openid-connect-multitenancy.adoc index eacb699bcbf9f..51c2020d8a7e2 100644 --- a/docs/src/main/asciidoc/security-openid-connect-multitenancy.adoc +++ b/docs/src/main/asciidoc/security-openid-connect-multitenancy.adoc @@ -590,62 +590,20 @@ However, this time, you are going to authenticate by using a different realm. In both cases, the landing page shows the user's name and email if the user is successfully authenticated. Although `alice` exists in both tenants, the application treats them as distinct users in separate realms. -[[static-tenant-resolution]] -== Static tenant configuration resolution - -When you set multiple tenant configurations in the `application.properties` file, you only need to specify how the tenant identifier gets resolved. -To configure the resolution of the tenant identifier, use one of the following options: - -* <> -* <> -* <> -* <> +[[tenant-resolution-order]] +== Tenant resolution order -These tenant resolution options are tried in the order they are listed until the tenant id gets resolved. -If the tenant id remains unresolved (`null`), the default (unnamed) tenant configuration is selected. +OIDC tenants are resolved in the following order: +1. `io.quarkus.oidc.Tenant` annotation is checked first if the proactive authentication is disabled. +2. Dynamic tenant resolution using a custom `TenantConfigResolver`. +3. Static tenant resolution using one of these options: custom `TenantResolver`, configured tenant paths, and defaulting to the last request path segment as a tenant id. +4. Default OIDC tenant is selected if a tenant id has not been resolved after the preceeding steps. -[[tenant-resolver]] -=== Resolve with `TenantResolver` +See the following sections for more information: -The following `application.properties` example shows how you can resolve the tenant identifier of two tenants named `a` and `b` by using the `TenantResolver` method: - -[source,properties] ----- -# Tenant 'a' configuration -quarkus.oidc.a.auth-server-url=http://localhost:8180/realms/quarkus-a -quarkus.oidc.a.client-id=client-a -quarkus.oidc.a.credentials.secret=client-a-secret - -# Tenant 'b' configuration -quarkus.oidc.b.auth-server-url=http://localhost:8180/realms/quarkus-b -quarkus.oidc.b.client-id=client-b -quarkus.oidc.b.credentials.secret=client-b-secret ----- - -You can return the tenant id of either `a` or `b` from `quarkus.oidc.TenantResolver`: - -[source,java] ----- -import quarkus.oidc.TenantResolver; - -public class CustomTenantResolver implements TenantResolver { - - @Override - public String resolve(RoutingContext context) { - String path = context.request().path(); - if (path.endsWith("a")) { - return "a"; - } else if (path.endsWith("b")) { - return "b"; - } else { - // default tenant - return null; - } - } -} ----- - -In this example, the value of the last request path segment is a tenant id, but if required, you can implement a more complex tenant identifier resolution logic. +* <> +* <> +* <> [[annotations-tenant-resolver]] === Resolve with annotations @@ -698,8 +656,8 @@ quarkus.http.auth.permission.authenticated.applies-to=JAXRS <1> ---- <1> Tell Quarkus to run the HTTP permission check after the tenant has been selected with the `@Tenant` annotation. -[[configuration-based-tenant-resolver]] -=== Resolve with configuration +[[configure-tenant-paths]] +=== Configure tenant paths You can use the `quarkus.oidc.tenant-paths` configuration property for resolving the tenant identifier as an alternative to using `io.quarkus.oidc.TenantResolver`. Here is how you can select the `hr` tenant for the `sayHello` endpoint of the `HelloResource` resource used in the previous example: @@ -716,6 +674,130 @@ quarkus.oidc.b.tenant-paths=/*/hello <3> TIP: Path-matching mechanism works exactly same as in the xref:security-authorize-web-endpoints-reference.adoc#authorization-using-configuration[Authorization using configuration]. +[[tenant-config-resolver]] +== Dynamic tenant configuration resolution + +If you need a more dynamic configuration for the different tenants you want to support and don't want to end up with multiple +entries in your configuration file, you can use the `io.quarkus.oidc.TenantConfigResolver`. + +This interface allows you to dynamically create tenant configurations at runtime: + +[source,java] +---- +package io.quarkus.it.keycloak; + +import jakarta.enterprise.context.ApplicationScoped; +import java.util.function.Supplier; + +import io.smallrye.mutiny.Uni; +import io.quarkus.oidc.OidcRequestContext; +import io.quarkus.oidc.OidcTenantConfig; +import io.quarkus.oidc.TenantConfigResolver; +import io.vertx.ext.web.RoutingContext; + +@ApplicationScoped +public class CustomTenantConfigResolver implements TenantConfigResolver { + + @Override + public Uni resolve(RoutingContext context, OidcRequestContext requestContext) { + String path = context.request().path(); + String[] parts = path.split("/"); + + if (parts.length == 0) { + //Resolve to default tenant configuration + return null; + } + + if ("tenant-c".equals(parts[1])) { + // Do 'return requestContext.runBlocking(createTenantConfig());' + // if a blocking call is required to create a tenant config, + return Uni.createFromItem(createTenantConfig()); + } + + //Resolve to default tenant configuration + return null; + } + + private Supplier createTenantConfig() { + final OidcTenantConfig config = new OidcTenantConfig(); + + config.setTenantId("tenant-c"); + config.setAuthServerUrl("http://localhost:8180/realms/tenant-c"); + config.setClientId("multi-tenant-client"); + OidcTenantConfig.Credentials credentials = new OidcTenantConfig.Credentials(); + + credentials.setSecret("my-secret"); + + config.setCredentials(credentials); + + // Any other setting supported by the quarkus-oidc extension + + return () -> config; + } +} +---- + +The `OidcTenantConfig` returned by this method is the same one used to parse the `oidc` namespace configuration from the `application.properties`. +You can populate it by using any settings supported by the `quarkus-oidc` extension. + +If the dynamic tenant resolver returns `null`, a <> is attempted next. + +[[static-tenant-resolution]] +== Static tenant configuration resolution + +When you set multiple tenant configurations in the `application.properties` file, you only need to specify how the tenant identifier gets resolved. +To configure the resolution of the tenant identifier, use one of the following options: + +* <> +* <> +* <> + +These tenant resolution options are tried in the order they are listed until the tenant id gets resolved. +If the tenant id remains unresolved (`null`), the default (unnamed) tenant configuration is selected. + +[[tenant-resolver]] +=== Resolve with `TenantResolver` + +The following `application.properties` example shows how you can resolve the tenant identifier of two tenants named `a` and `b` by using the `TenantResolver` method: + +[source,properties] +---- +# Tenant 'a' configuration +quarkus.oidc.a.auth-server-url=http://localhost:8180/realms/quarkus-a +quarkus.oidc.a.client-id=client-a +quarkus.oidc.a.credentials.secret=client-a-secret + +# Tenant 'b' configuration +quarkus.oidc.b.auth-server-url=http://localhost:8180/realms/quarkus-b +quarkus.oidc.b.client-id=client-b +quarkus.oidc.b.credentials.secret=client-b-secret +---- + +You can return the tenant id of either `a` or `b` from `quarkus.oidc.TenantResolver`: + +[source,java] +---- +import quarkus.oidc.TenantResolver; + +public class CustomTenantResolver implements TenantResolver { + + @Override + public String resolve(RoutingContext context) { + String path = context.request().path(); + if (path.endsWith("a")) { + return "a"; + } else if (path.endsWith("b")) { + return "b"; + } else { + // default tenant + return null; + } + } +} +---- + +In this example, the value of the last request path segment is a tenant id, but if required, you can implement a more complex tenant identifier resolution logic. + [[default-tenant-resolver]] === Default resolution @@ -756,25 +838,37 @@ Therefore, authenticated users can access the secured application area without r Default resolution can also work for Bearer token authentication. Still, it might be less practical because a tenant identifier must always be set as the last path segment value. -[[tenant-config-resolver]] -== Dynamic tenant configuration resolution +=== Tenant resolution for OIDC web-app applications -If you need a more dynamic configuration for the different tenants you want to support and don't want to end up with multiple -entries in your configuration file, you can use the `io.quarkus.oidc.TenantConfigResolver`. +Tenant resolution for the OIDC `web-app` applications must be done at least 3 times during an authorization code flow, when the OIDC tenant-specific configuration affects how each of the following steps is run. -This interface allows you to dynamically create tenant configurations at runtime: +==== Step 1: Unauthenticated user accesses an endpoint and is redirected to OIDC provider + +When an unauthenticated user accesses a secured path, the user is redirected to the OIDC provider to authenticate and the tenant configuration is used to build the redirect URI. + +All the static and dynamic tenant resolution options listed in the <> and <> sections can be used to resolve a tenant. + +==== Step 2: The user is redirected back to the endpoint + +After the provider authentication, the user is redirected back to the Quarkus endpoint and the tenant configuration is used to complete the authorization code flow. + +All the static and dynamic tenant resolution options listed in the <> and <> sections can be used to resolve a tenant. Before the tenant resolution begins, the authorization code flow `state cookie` is used to set the already resolved tenant configuration id as a RoutingContext `tenant-id` attribute: both custom dynamic `TenantConfigResolver` and static `TenantResolver` tenant resolvers can check it. + +==== Step 3: Authenticated user accesses the secured path using the session cookie: the tenant configuration determines how the session cookie is verified and refreshed. Before the tenant resolution begins, the authorization code flow `session cookie` is used to set the already resolved tenant configuration id as a RoutingContext `tenant-id` attribute: both custom dynamic `TenantConfigResolver` and static `TenantResolver` tenant resolvers can check it. + +For example, here is how a custom `TenantConfigResolver` can avoid creating the already resolved tenant configuration, that may otherwise require blocking reads from the database or other remote sources: [source,java] ---- package io.quarkus.it.keycloak; import jakarta.enterprise.context.ApplicationScoped; -import java.util.function.Supplier; - -import io.smallrye.mutiny.Uni; import io.quarkus.oidc.OidcRequestContext; import io.quarkus.oidc.OidcTenantConfig; +import io.quarkus.oidc.OidcTenantConfig.ApplicationType; import io.quarkus.oidc.TenantConfigResolver; +import io.quarkus.oidc.runtime.OidcUtils; +import io.smallrye.mutiny.Uni; import io.vertx.ext.web.RoutingContext; @ApplicationScoped @@ -782,104 +876,180 @@ public class CustomTenantConfigResolver implements TenantConfigResolver { @Override public Uni resolve(RoutingContext context, OidcRequestContext requestContext) { - String path = context.request().path(); - String[] parts = path.split("/"); - - if (parts.length == 0) { - //Resolve to default tenant configuration + String resolvedTenantId = context.get(OidcUtils.TENANT_ID_ATTRIBUTE); + if (resolvedTenantId != null) { <1> return null; } - if ("tenant-c".equals(parts[1])) { - // Do 'return requestContext.runBlocking(createTenantConfig());' - // if a blocking call is required to create a tenant config, - return Uni.createFromItem(createTenantConfig()); + String path = context.request().path(); <2> + if (path.endsWith("tenant-a")) { + return Uni.createFromItem(createTenantConfig("tenant-a", "client-a", "secret-a")); + } else if (path.endsWith("tenant-b")) { + return Uni.createFromItem(createTenantConfig("tenant-b", "client-b", "secret-b")); } - //Resolve to default tenant configuration + // Default tenant id return null; } - private Supplier createTenantConfig() { + private OidcTenantConfig createTenantConfig(String tenantId, String clientId, String secret) { final OidcTenantConfig config = new OidcTenantConfig(); - - config.setTenantId("tenant-c"); - config.setAuthServerUrl("http://localhost:8180/realms/tenant-c"); - config.setClientId("multi-tenant-client"); - OidcTenantConfig.Credentials credentials = new OidcTenantConfig.Credentials(); - - credentials.setSecret("my-secret"); - - config.setCredentials(credentials); - - // Any other setting supported by the quarkus-oidc extension - - return () -> config; + config.setTenantId(tenantId); + config.setAuthServerUrl("http://localhost:8180/realms/" + tenantId); + config.setClientId(clientId); + config.getCredentials().setSecret(secret); + config.setApplicationType(ApplicationType.WEB_APP); + return config; } } ---- +<1> Let Quarkus use the already resolved tenant configuration if it has been resolved earlier. +<2> Check the request path to create tenant configurations. -The `OidcTenantConfig` returned by this method is the same one used to parse the `oidc` namespace configuration from the `application.properties`. -You can populate it by using any settings supported by the `quarkus-oidc` extension. +The default configuration may look like this: -If the dynamic tenant resolver returns `null`, a <> is attempted next. +[source,properties] +---- +quarkus.oidc.auth-server-url=http://localhost:8180/realms/default +quarkus.oidc.client-id=client-default +quarkus.oidc.credentials.secret=secret-default +quarkus.oidc.application-type=web-app +---- -=== Tenant resolution for OIDC web-app applications +The preceeding example assumes that the `tenant-a`, `tenant-b` and default tenants are all used to protect the same endpoint paths. In other words, after the user has authenticated with the `tenant-a` configuration, this user will not be able to choose to authenticate with the `tenant-b` or default configuration before this user logs out and has a session cookie cleared or expired. -The simplest option for resolving the OIDC `web-app` application configuration is to follow the steps described in the <> section. +The situtaion where multiple OIDC `web-app` tenants protect the tenant-specific paths is less typical and also requires an extra care. +When multiple OIDC `web-app` tenants such as `tenant-a`, `tenant-b` and default tenants are used to control access to the tenant specific paths, the users authenticated with one OIDC provider must not be able to access the paths requiring an authentication with another provider, otherwise the results can be unpredictable, most likely causing unexpected authentication failures. +For example, if the `tenant-a` authentication requires a Keycloak authentication and the `tenant-b` authentication requires an Auth0 authentication, then, if the `tenant-a` authenticated user attempts to access a path secured by the `tenant-b` configuration, then the session cookie will not be verified, since the Auth0 public verification keys can not be used to verify the tokens signed by Keycloak. +An easy, recommended way to avoid multiple `web-app` tenants conflicting with each other is to set the tenant specific session path as shown in the following example: -Try one of the options below if the default resolution strategy does not work for your application setup. +[source,java] +---- +package io.quarkus.it.keycloak; -Several options are available for selecting the tenant configuration that should be used to secure the current HTTP request for both `service` and `web-app` OIDC applications, such as: +import jakarta.enterprise.context.ApplicationScoped; +import io.quarkus.oidc.OidcRequestContext; +import io.quarkus.oidc.OidcTenantConfig; +import io.quarkus.oidc.OidcTenantConfig.ApplicationType; +import io.quarkus.oidc.TenantConfigResolver; +import io.quarkus.oidc.runtime.OidcUtils; +import io.smallrye.mutiny.Uni; +import io.vertx.ext.web.RoutingContext; -- Check the URL paths. -For example, a `tenant-service` configuration must be used for the `/service` paths, while a `tenant-manage` configuration must be used for the `/management` paths. -- Check the HTTP headers. -For example, with a URL path always being `/service`, a header such as `Realm: service` or `Realm: management` can help to select between the `tenant-service` and `tenant-manage` configurations. -- Check the URL query parameters. -It can work similarly to the way the headers are used to select the tenant configuration. +@ApplicationScoped +public class CustomTenantConfigResolver implements TenantConfigResolver { -All these options can be easily implemented with the custom `TenantResolver` and `TenantConfigResolver` implementations for the OIDC `service` applications. + @Override + public Uni resolve(RoutingContext context, OidcRequestContext requestContext) { + String resolvedTenantId = context.get(OidcUtils.TENANT_ID_ATTRIBUTE); + if (resolvedTenantId != null) { <1> + return null; + } -However, due to an HTTP redirect required to complete the code authentication flow for the OIDC `web-app` applications, a custom HTTP cookie might be needed to select the same tenant configuration before and after this redirect request because: + String path = context.request().path(); <2> + if (path.endsWith("tenant-a")) { + return Uni.createFromItem(createTenantConfig("tenant-a", "/tenant-a", "client-a", "secret-a")); + } else if (path.endsWith("tenant-b")) { + return Uni.createFromItem(createTenantConfig("tenant-b", "/tenant-b", "client-b", "secret-b")); + } -- The URL path might not be the same after the redirect request if a single redirect URL has been registered in the OIDC provider. -The original request path can be restored after the tenant configuration has been resolved. -- The HTTP headers used during the original request are unavailable after the redirect. -- The custom URL query parameters are restored after the redirect but only after the tenant configuration is resolved. + // Default tenant id + return null; + } -One option to ensure the information for resolving the tenant configurations for `web-app` applications is available before and after the redirect is to use a cookie, for example: + private OidcTenantConfig createTenantConfig(String tenantId, String cookiePath, String clientId, String secret) { + final OidcTenantConfig config = new OidcTenantConfig(); + config.setTenantId(tenantId); + config.setAuthServerUrl("http://localhost:8180/realms/" + tenantId); + config.setClientId(clientId); + config.getCredentials().setSecret(secret); + config.setApplicationType(ApplicationType.WEB_APP); + config.getAuthentication().setCookiePath(cookiePath); <3> + return config; + } +} +---- +<1> Let Quarkus use the already resolved tenant configuration if it has been resolved earlier. +<2> Check the request path to create tenant configurations. +<3> Set the tenant-specific cookie paths which makes sure the session cookie is only visible to the tenant which created it. -[source,java] +The default tenant configuration should be adjusted like this: +[source,properties] +---- +quarkus.oidc.auth-server-url=http://localhost:8180/realms/default +quarkus.oidc.client-id=client-default +quarkus.oidc.credentials.secret=secret-default +quarkus.oidc.authentication.cookie-path=/default +quarkus.oidc.application-type=web-app ---- -package org.acme.quickstart.oidc; -import java.util.List; +Having the same session cookie path when multiple OIDC `web-app` tenants protect the tenant-specific paths is not recommended and should be avoided +as it requires even more care from the custom resolvers, for example: -import jakarta.enterprise.context.ApplicationScoped; +[source,java] +---- +package io.quarkus.it.keycloak; -import io.quarkus.oidc.TenantResolver; -import io.vertx.core.http.Cookie; +import jakarta.enterprise.context.ApplicationScoped; +import io.quarkus.oidc.OidcRequestContext; +import io.quarkus.oidc.OidcTenantConfig; +import io.quarkus.oidc.OidcTenantConfig.ApplicationType; +import io.quarkus.oidc.TenantConfigResolver; +import io.quarkus.oidc.runtime.OidcUtils; +import io.smallrye.mutiny.Uni; import io.vertx.ext.web.RoutingContext; @ApplicationScoped -public class CustomTenantResolver implements TenantResolver { +public class CustomTenantConfigResolver implements TenantConfigResolver { @Override - public String resolve(RoutingContext context) { - List tenantIdQuery = context.queryParam("tenantId"); - if (!tenantIdQuery.isEmpty()) { - String tenantId = tenantIdQuery.get(0); - context.response().addCookie(Cookie.cookie("tenant", tenantId)); - return tenantId; - } else if (!context.request().cookies("tenant").isEmpty()) { - return context.request().getCookie("tenant").getValue(); + public Uni resolve(RoutingContext context, OidcRequestContext requestContext) { + + String path = context.request().path(); <1> + if (path.endsWith("tenant-a")) { + String resolvedTenantId = context.get(OidcUtils.TENANT_ID_ATTRIBUTE); + if (resolvedTenantId != null) { + if ("tenant-a".equals(resolvedTenantId)) { <2> + return null; + } else { + // Require a "tenant-a" authentication + context.remove(OidcUtils.TENANT_ID_ATTRIBUTE); <3> + } + } + return Uni.createFromItem(createTenantConfig("tenant-a", "client-a", "secret-a")); + } else if (path.endsWith("tenant-b")) { + String resolvedTenantId = context.get(OidcUtils.TENANT_ID_ATTRIBUTE); + if (resolvedTenantId != null) { + if ("tenant-b".equals(resolvedTenantId)) { <2> + return null; + } else { + // Require a "tenant-b" authentication + context.remove(OidcUtils.TENANT_ID_ATTRIBUTE); <3> + } + } + return Uni.createFromItem(createTenantConfig("tenant-b", "client-b", "secret-b")); } + // Set default tenant id + context.put(OidcUtils.TENANT_ID_ATTRIBUTE, OidcUtils.DEFAULT_TENANT_ID); <4> return null; } + + private OidcTenantConfig createTenantConfig(String tenantId, String clientId, String secret) { + final OidcTenantConfig config = new OidcTenantConfig(); + config.setTenantId(tenantId); + config.setAuthServerUrl("http://localhost:8180/realms/" + tenantId); + config.setClientId(clientId); + config.getCredentials().setSecret(secret); + config.setApplicationType(ApplicationType.WEB_APP); + return config; + } } ---- +<1> Check the request path to create tenant configurations. +<2> Let Quarkus use the already resolved tenant configuration if the already resolved tenant is expected for the current path. +<3> Remove the `tenant-id` attribute if the already resolved tenant configuration is not expected for the current path. +<4> Use the default tenant for all other paths. It is equivalent to removing the `tenant-id` attribute. [[disable-tenant]] == Disabling tenant configurations diff --git a/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CodeFlowDevModeTestCase.java b/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CodeFlowDevModeTestCase.java index c9710c7863c8b..a3c4297b51700 100644 --- a/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CodeFlowDevModeTestCase.java +++ b/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CodeFlowDevModeTestCase.java @@ -15,6 +15,10 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.SortedMap; +import java.util.TreeMap; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -26,6 +30,7 @@ import org.junit.jupiter.api.TestMethodOrder; import org.junit.jupiter.api.extension.RegisterExtension; +import com.gargoylesoftware.htmlunit.CookieManager; import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; import com.gargoylesoftware.htmlunit.SilentCssErrorHandler; import com.gargoylesoftware.htmlunit.WebClient; @@ -33,6 +38,7 @@ import com.gargoylesoftware.htmlunit.WebResponse; import com.gargoylesoftware.htmlunit.html.HtmlForm; import com.gargoylesoftware.htmlunit.html.HtmlPage; +import com.gargoylesoftware.htmlunit.util.Cookie; import io.quarkus.test.QuarkusDevModeTest; import io.quarkus.test.common.QuarkusTestResource; @@ -114,7 +120,12 @@ public void testAccessAndRefreshTokenInjectionDevMode() throws IOException, Inte assertEquals("alice", page.getBody().asNormalizedText()); - assertEquals("custom", page.getWebClient().getCookieManager().getCookie("q_session").getValue().split("\\|")[1]); + List sessionCookies = getSessionCookies(webClient, null); + assertEquals(2, sessionCookies.size()); + assertEquals("q_session_chunk_1", sessionCookies.get(0).getName()); + assertEquals("q_session_chunk_2", sessionCookies.get(1).getName()); + String sessionCookieValue = sessionCookies.get(0).getValue() + sessionCookies.get(1).getValue(); + assertEquals("custom", sessionCookieValue.split("\\|")[1]); webClient.getOptions().setRedirectEnabled(false); WebResponse webResponse = webClient @@ -217,4 +228,16 @@ public void run() throws Throwable { assertTrue(checkPassed.get(), "Can not confirm Secret key for encrypting state cookie has been generated"); } + private List getSessionCookies(WebClient webClient, String tenantId) { + String sessionCookieNameChunk = "q_session" + (tenantId == null ? "" : "_" + 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()); + } } diff --git a/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CodeTenantReauthenticateTestCase.java b/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CodeTenantReauthenticateTestCase.java new file mode 100644 index 0000000000000..98a9bdd4aa52b --- /dev/null +++ b/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CodeTenantReauthenticateTestCase.java @@ -0,0 +1,173 @@ +package io.quarkus.oidc.test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +import org.junit.jupiter.api.MethodOrderer; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestMethodOrder; +import org.junit.jupiter.api.extension.RegisterExtension; + +import com.gargoylesoftware.htmlunit.SilentCssErrorHandler; +import com.gargoylesoftware.htmlunit.TextPage; +import com.gargoylesoftware.htmlunit.WebClient; +import com.gargoylesoftware.htmlunit.html.HtmlForm; +import com.gargoylesoftware.htmlunit.html.HtmlPage; +import com.gargoylesoftware.htmlunit.util.Cookie; + +import io.quarkus.test.QuarkusUnitTest; +import io.quarkus.test.common.QuarkusTestResource; +import io.quarkus.test.keycloak.server.KeycloakTestResourceLifecycleManager; + +@TestMethodOrder(MethodOrderer.OrderAnnotation.class) +@QuarkusTestResource(KeycloakTestResourceLifecycleManager.class) +public class CodeTenantReauthenticateTestCase { + private static Class[] testClasses = { + TenantReauthentication.class, + CustomTenantResolver.class, + CustomTenantConfigResolver.class + }; + + @RegisterExtension + static final QuarkusUnitTest test = new QuarkusUnitTest() + .withApplicationRoot((jar) -> jar + .addClasses(testClasses) + .addAsResource("application-tenant-reauthenticate.properties", "application.properties")); + + @Test + public void testDefaultTenant() throws Exception { + try (final WebClient webClient = createWebClient()) { + + callTenant(webClient, null, "/protected", "alice"); + + webClient.getCookieManager().clearCookies(); + } + } + + @Test + public void testTenantResolver() throws Exception { + try (final WebClient webClient = createWebClient()) { + + callTenant(webClient, "tenant-resolver", "/protected/tenant/tenant-resolver", "tenant-resolver:alice"); + + webClient.getCookieManager().clearCookies(); + } + } + + @Test + public void testTenantConfigResolver() throws Exception { + try (final WebClient webClient = createWebClient()) { + + callTenant(webClient, "tenant-config-resolver", "/protected/tenant/tenant-config-resolver", + "tenant-config-resolver:alice"); + + webClient.getCookieManager().clearCookies(); + } + } + + @Test + public void testSwitchFromTenantResolverToDefaultTenant() throws Exception { + try (final WebClient webClient = createWebClient()) { + + callTenant(webClient, "tenant-resolver", "/protected/tenant/tenant-resolver", "tenant-resolver:alice"); + expectReauthentication(webClient, "/protected", "tenant-resolver"); + + webClient.getCookieManager().clearCookies(); + } + } + + @Test + public void testSwitchFromDefaultTenantToTenantResover() throws Exception { + try (final WebClient webClient = createWebClient()) { + + callTenant(webClient, null, "/protected", "alice"); + expectReauthentication(webClient, "/protected/tenant/tenant-resolver", null); + + webClient.getCookieManager().clearCookies(); + } + } + + @Test + public void testSwitchFromTenantConfigResolverToDefaultTenant() throws Exception { + try (final WebClient webClient = createWebClient()) { + + callTenant(webClient, "tenant-config-resolver", "/protected/tenant/tenant-config-resolver", + "tenant-config-resolver:alice"); + expectReauthentication(webClient, "/protected", "tenant-config-resolver"); + + webClient.getCookieManager().clearCookies(); + } + } + + @Test + public void testSwitchFromDefaultTenantToTenantConfigResolver() throws Exception { + try (final WebClient webClient = createWebClient()) { + + callTenant(webClient, null, "/protected", "alice"); + expectReauthentication(webClient, "/protected/tenant/tenant-config-resolver", null); + + webClient.getCookieManager().clearCookies(); + } + } + + @Test + public void testSwitchFromTenantResolverToTenantConfigResolver() throws Exception { + try (final WebClient webClient = createWebClient()) { + + callTenant(webClient, "tenant-resolver", "/protected/tenant/tenant-resolver", "tenant-resolver:alice"); + expectReauthentication(webClient, "/protected/tenant/tenant-config-resolver", "tenant-resolver"); + + webClient.getCookieManager().clearCookies(); + } + } + + @Test + public void testSwitchFromTenantConfigResolverToTenantResolver() throws Exception { + try (final WebClient webClient = createWebClient()) { + + callTenant(webClient, "tenant-config-resolver", "/protected/tenant/tenant-config-resolver", + "tenant-config-resolver:alice"); + expectReauthentication(webClient, "/protected/tenant/tenant-resolver", "tenant-config-resolver"); + + webClient.getCookieManager().clearCookies(); + } + } + + private static void callTenant(WebClient webClient, String tenant, String relativePath, String expectedResponse) + throws Exception { + HtmlPage page = webClient.getPage("http://localhost:8081" + relativePath); + + assertEquals("Sign in to quarkus", page.getTitleText()); + + HtmlForm loginForm = page.getForms().get(0); + + loginForm.getInputByName("username").setValueAttribute("alice"); + loginForm.getInputByName("password").setValueAttribute("alice"); + + page = loginForm.getInputByName("login").click(); + + assertEquals(expectedResponse, page.getBody().asNormalizedText()); + assertNotNull(getSessionCookie(webClient, tenant)); + } + + private static void expectReauthentication(WebClient webClient, String relativePath, + String oldTenant) throws Exception { + webClient.getOptions().setRedirectEnabled(false); + webClient.getOptions().setThrowExceptionOnFailingStatusCode(false); + + TextPage textPage = webClient.getPage("http://localhost:8081" + relativePath); + assertEquals(302, textPage.getWebResponse().getStatusCode()); + assertNull(getSessionCookie(webClient, oldTenant)); + } + + private static WebClient createWebClient() { + WebClient webClient = new WebClient(); + webClient.setCssErrorHandler(new SilentCssErrorHandler()); + return webClient; + } + + private static Cookie getSessionCookie(WebClient webClient, String tenantId) { + return webClient.getCookieManager().getCookie("q_session" + (tenantId == null ? "" : "_" + tenantId)); + } +} diff --git a/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CustomTenantConfigResolver.java b/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CustomTenantConfigResolver.java index 4b88cadd8a795..89289f32b478a 100644 --- a/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CustomTenantConfigResolver.java +++ b/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CustomTenantConfigResolver.java @@ -2,10 +2,13 @@ import jakarta.enterprise.context.ApplicationScoped; +import org.eclipse.microprofile.config.ConfigProvider; + import io.quarkus.oidc.OidcRequestContext; import io.quarkus.oidc.OidcTenantConfig; import io.quarkus.oidc.OidcTenantConfig.ApplicationType; import io.quarkus.oidc.TenantConfigResolver; +import io.quarkus.oidc.runtime.OidcUtils; import io.smallrye.mutiny.Uni; import io.vertx.ext.web.RoutingContext; @@ -22,6 +25,7 @@ public Uni resolve(RoutingContext context, OidcRequestContext< config.setApplicationType(ApplicationType.WEB_APP); return Uni.createFrom().item(config); } + context.remove(OidcUtils.TENANT_ID_ATTRIBUTE); if (context.request().path().endsWith("/null-tenant")) { return null; } @@ -29,6 +33,6 @@ public Uni resolve(RoutingContext context, OidcRequestContext< } private String getIssuerUrl() { - return System.getProperty("keycloak.url"); + return ConfigProvider.getConfig().getValue("keycloak.url", String.class); } } diff --git a/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CustomTenantResolver.java b/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CustomTenantResolver.java new file mode 100644 index 0000000000000..ae832b1b89070 --- /dev/null +++ b/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CustomTenantResolver.java @@ -0,0 +1,19 @@ +package io.quarkus.oidc.test; + +import jakarta.enterprise.context.ApplicationScoped; + +import io.quarkus.oidc.TenantResolver; +import io.quarkus.oidc.runtime.OidcUtils; +import io.vertx.ext.web.RoutingContext; + +@ApplicationScoped +public class CustomTenantResolver implements TenantResolver { + @Override + public String resolve(RoutingContext context) { + if (context.request().path().endsWith("/tenant-resolver")) { + return "tenant-resolver"; + } + context.put(OidcUtils.TENANT_ID_ATTRIBUTE, OidcUtils.DEFAULT_TENANT_ID); + return null; + } +} diff --git a/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/TenantReauthentication.java b/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/TenantReauthentication.java new file mode 100644 index 0000000000000..22f1b5ef5c11d --- /dev/null +++ b/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/TenantReauthentication.java @@ -0,0 +1,35 @@ +package io.quarkus.oidc.test; + +import jakarta.inject.Inject; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.PathParam; + +import org.eclipse.microprofile.jwt.JsonWebToken; + +import io.quarkus.oidc.IdToken; +import io.quarkus.oidc.OidcSession; +import io.quarkus.security.Authenticated; + +@Path("/protected") +@Authenticated +public class TenantReauthentication { + + @Inject + @IdToken + JsonWebToken idToken; + + @Inject + OidcSession session; + + @GET + public String getName() { + return idToken.getName(); + } + + @GET + @Path("tenant/{id}") + public String getTenantName(@PathParam("id") String tenantId) { + return tenantId + ":" + idToken.getName(); + } +} diff --git a/extensions/oidc/deployment/src/test/resources/application-tenant-reauthenticate.properties b/extensions/oidc/deployment/src/test/resources/application-tenant-reauthenticate.properties new file mode 100644 index 0000000000000..3e5539ef71da8 --- /dev/null +++ b/extensions/oidc/deployment/src/test/resources/application-tenant-reauthenticate.properties @@ -0,0 +1,11 @@ +quarkus.oidc.auth-server-url=${keycloak.url}/realms/quarkus +quarkus.oidc.client-id=quarkus-web-app +quarkus.oidc.credentials.secret=secret +quarkus.oidc.application-type=web-app + +quarkus.oidc.tenant-resolver.auth-server-url=${keycloak.url}/realms/quarkus +quarkus.oidc.tenant-resolver.client-id=quarkus-web-app +quarkus.oidc.tenant-resolver.credentials.secret=secret +quarkus.oidc.tenant-resolver.application-type=web-app + +quarkus.log.category."com.gargoylesoftware.htmlunit".level=ERROR 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 f81265be2cf64..bd4981b412a7a 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 @@ -560,8 +560,23 @@ public Uni apply(TenantConfigContext tenantContext) { } public Uni getChallengeInternal(RoutingContext context, TenantConfigContext configContext) { - LOG.debug("Starting an authentication challenge"); - return removeSessionCookie(context, configContext.oidcConfig) + LOG.debugf("Starting an authentication challenge for tenant %s", configContext.oidcConfig.tenantId.get()); + + OidcTenantConfig sessionCookieConfig = configContext.oidcConfig; + String sessionTenantIdSetByCookie = context.get(OidcUtils.TENANT_ID_SET_BY_SESSION_COOKIE); + + if (sessionTenantIdSetByCookie != null + && !sessionTenantIdSetByCookie.equals(sessionCookieConfig.tenantId.orElse(OidcUtils.DEFAULT_TENANT_ID))) { + // New tenant id has been chosen during the tenant resolution process + // Get the already resolved configuration, avoiding calling the tenant resolvers + OidcTenantConfig previousTenantConfig = resolver.getResolvedConfig(sessionTenantIdSetByCookie); + if (previousTenantConfig != null) { + sessionCookieConfig = previousTenantConfig; + LOG.debugf("Removing the session cookie for the previous tenant id: %s", sessionTenantIdSetByCookie); + OidcUtils.getSessionCookie(context, sessionCookieConfig); + } + } + return removeSessionCookie(context, sessionCookieConfig) .chain(new Function>() { @Override @@ -963,6 +978,8 @@ public Void apply(String cookieValue) { String nextValue = cookieValue.substring(currentPos, nextValueUpperPos); // q_session_session_chunk_1, etc String nextName = sessionName + OidcUtils.SESSION_COOKIE_CHUNK + sessionIndex; + LOG.debugf("Creating the %s session cookie chunk, size: %d", nextName, + nextValue.length()); createCookie(context, configContext.oidcConfig, nextName, nextValue, sessionMaxAge, true); currentPos = nextPos; diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/DefaultTenantConfigResolver.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/DefaultTenantConfigResolver.java index ef5dcabc81720..d87118b8221b4 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/DefaultTenantConfigResolver.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/DefaultTenantConfigResolver.java @@ -377,4 +377,19 @@ private static ImmutablePathMatcher.ImmutablePathMatcherBuilder addPath( } } + public OidcTenantConfig getResolvedConfig(String sessionTenantId) { + if (OidcUtils.DEFAULT_TENANT_ID.equals(sessionTenantId)) { + return tenantConfigBean.getDefaultTenant().getOidcTenantConfig(); + } + + if (tenantConfigBean.getStaticTenantsConfig().containsKey(sessionTenantId)) { + return tenantConfigBean.getStaticTenantsConfig().get(sessionTenantId).getOidcTenantConfig(); + } + + if (tenantConfigBean.getDynamicTenantsConfig().containsKey(sessionTenantId)) { + return tenantConfigBean.getDynamicTenantsConfig().get(sessionTenantId).getOidcTenantConfig(); + } + return null; + } + } diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcAuthenticationMechanism.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcAuthenticationMechanism.java index 6706fa49c621b..8f798a68935cf 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcAuthenticationMechanism.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcAuthenticationMechanism.java @@ -44,7 +44,6 @@ public OidcAuthenticationMechanism(DefaultTenantConfigResolver resolver, Blockin @Override public Uni authenticate(RoutingContext context, IdentityProviderManager identityProviderManager) { - setTenantIdAttribute(context); return resolve(context).chain(new Function<>() { @Override public Uni apply(OidcTenantConfig oidcConfig) { @@ -59,7 +58,6 @@ public Uni apply(OidcTenantConfig oidcConfig) { @Override public Uni getChallenge(RoutingContext context) { - setTenantIdAttribute(context); return resolve(context).chain(new Function<>() { @Override public Uni apply(OidcTenantConfig oidcTenantConfig) { @@ -73,6 +71,13 @@ public Uni apply(OidcTenantConfig oidcTenantConfig) { } private Uni resolve(RoutingContext context) { + OidcTenantConfig resolvedConfig = context.get(OidcTenantConfig.class.getName()); + if (resolvedConfig != null) { + return Uni.createFrom().item(resolvedConfig); + } + + setTenantIdAttribute(context); + return resolver.resolveConfig(context).map(new Function<>() { @Override public OidcTenantConfig apply(OidcTenantConfig oidcTenantConfig) { @@ -80,6 +85,7 @@ public OidcTenantConfig apply(OidcTenantConfig oidcTenantConfig) { throw new OIDCException("Tenant configuration has not been resolved"); } LOG.debugf("Resolved OIDC tenant id: %s", oidcTenantConfig.tenantId.orElse(OidcUtils.DEFAULT_TENANT_ID)); + context.put(OidcTenantConfig.class.getName(), oidcTenantConfig); return oidcTenantConfig; }; }); @@ -100,7 +106,6 @@ public Set> getCredentialTypes() { @Override public Uni getCredentialTransport(RoutingContext context) { - setTenantIdAttribute(context); return resolve(context).onItem().transform(new Function() { @Override public HttpCredentialTransport apply(OidcTenantConfig oidcTenantConfig) { @@ -115,29 +120,24 @@ public HttpCredentialTransport apply(OidcTenantConfig oidcTenantConfig) { } private static void setTenantIdAttribute(RoutingContext context) { - for (String cookieName : context.cookieMap().keySet()) { - if (OidcUtils.isSessionCookie(cookieName)) { - setTenantIdAttribute(context, OidcUtils.SESSION_COOKIE_NAME, cookieName, true); - } else if (cookieName.startsWith(OidcUtils.STATE_COOKIE_NAME)) { - setTenantIdAttribute(context, OidcUtils.STATE_COOKIE_NAME, cookieName, false); + if (context.get(OidcUtils.TENANT_ID_ATTRIBUTE) == null) { + for (String cookieName : context.cookieMap().keySet()) { + if (OidcUtils.isSessionCookie(cookieName)) { + setTenantIdAttribute(context, OidcUtils.SESSION_COOKIE_NAME, cookieName, true); + break; + } else if (cookieName.startsWith(OidcUtils.STATE_COOKIE_NAME)) { + setTenantIdAttribute(context, OidcUtils.STATE_COOKIE_NAME, cookieName, false); + break; + } } } } private static void setTenantIdAttribute(RoutingContext context, String cookiePrefix, String cookieName, boolean sessionCookie) { - // It has already been checked the cookieName starts with the cookiePrefix - String tenantId; - if (cookieName.length() == cookiePrefix.length()) { - tenantId = OidcUtils.DEFAULT_TENANT_ID; - context.put(OidcUtils.TENANT_ID_ATTRIBUTE, tenantId); - } else { - String suffix = cookieName.substring(cookiePrefix.length() + 1); - // It can be either a tenant_id, or a tenant_id and cookie suffix property, example, q_session_github or q_session_github_test - int index = suffix.indexOf("_"); - tenantId = index == -1 ? suffix : suffix.substring(0, index); - context.put(OidcUtils.TENANT_ID_ATTRIBUTE, tenantId); - } + String tenantId = OidcUtils.getTenantIdFromCookie(cookiePrefix, cookieName, sessionCookie); + + context.put(OidcUtils.TENANT_ID_ATTRIBUTE, tenantId); context.put(sessionCookie ? OidcUtils.TENANT_ID_SET_BY_SESSION_COOKIE : OidcUtils.TENANT_ID_SET_BY_STATE_COOKIE, tenantId); LOG.debugf("%s cookie set a '%s' tenant id on the %s request path", cookieName, tenantId, context.request().path()); 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 79df1efbaee97..a26ca21790e22 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 @@ -81,13 +81,19 @@ public final class OidcUtils { public static final String TENANT_ID_SET_BY_STATE_COOKIE = "tenant-id-set-by-state-cookie"; 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 SESSION_COOKIE_CHUNK_START = "chunk_"; + public static final String SESSION_COOKIE_CHUNK = "_" + SESSION_COOKIE_CHUNK_START; public static final String ACCESS_TOKEN_COOKIE_SUFFIX = "_at"; public static final String REFRESH_TOKEN_COOKIE_SUFFIX = "_rt"; public static final String SESSION_AT_COOKIE_NAME = SESSION_COOKIE_NAME + ACCESS_TOKEN_COOKIE_SUFFIX; public static final String SESSION_RT_COOKIE_NAME = SESSION_COOKIE_NAME + REFRESH_TOKEN_COOKIE_SUFFIX; public static final String STATE_COOKIE_NAME = "q_auth"; - public static final Integer MAX_COOKIE_VALUE_LENGTH = 4096; + + // Browsers enforce that the total Set-Cookie expression such as + // `q_session_tenant-a=,Path=/somepath,Expires=...` does not exceed 4096 + // Setting the max cookie value length to 4056 gives extra 40 bytes to cover for the name, path, expires attributes in most cases + // and can be tuned further if necessary. + public static final Integer MAX_COOKIE_VALUE_LENGTH = 4056; public static final String POST_LOGOUT_COOKIE_NAME = "q_post_logout"; public static final String DEFAULT_SCOPE_SEPARATOR = " "; static final String UNDERSCORE = "_"; @@ -107,12 +113,17 @@ private OidcUtils() { } + public static String getSessionCookie(RoutingContext context, OidcTenantConfig oidcTenantConfig) { + final Map cookies = context.request().cookieMap(); + return getSessionCookie(context.data(), cookies, oidcTenantConfig); + } + public static String getSessionCookie(Map context, Map cookies, OidcTenantConfig oidcTenantConfig) { if (cookies.isEmpty()) { return null; } - final String sessionCookieName = OidcUtils.getSessionCookieName(oidcTenantConfig); + final String sessionCookieName = getSessionCookieName(oidcTenantConfig); if (cookies.containsKey(sessionCookieName)) { context.put(OidcUtils.SESSION_COOKIE_NAME, List.of(sessionCookieName)); @@ -158,7 +169,7 @@ public static String getSessionCookieName(OidcTenantConfig 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 : ""; + String tenantIdSuffix = (cookieSuffixConfigured || !DEFAULT_TENANT_ID.equals(tenantId)) ? UNDERSCORE + tenantId : ""; return cookieSuffixConfigured ? (tenantIdSuffix + UNDERSCORE + oidcConfig.authentication.cookieSuffix.get()) @@ -423,7 +434,7 @@ public static void setBlockingApiAttribute(QuarkusSecurityIdentity.Builder build } public static void setTenantIdAttribute(QuarkusSecurityIdentity.Builder builder, OidcTenantConfig config) { - builder.addAttribute(TENANT_ID_ATTRIBUTE, config.tenantId.orElse("Default")); + builder.addAttribute(TENANT_ID_ATTRIBUTE, config.tenantId.orElse(DEFAULT_TENANT_ID)); } public static void setRoutingContextAttribute(QuarkusSecurityIdentity.Builder builder, RoutingContext routingContext) { @@ -465,6 +476,7 @@ static Uni removeSessionCookie(RoutingContext context, OidcTenantConfig oi TokenStateManager tokenStateManager) { List cookieNames = context.get(SESSION_COOKIE_NAME); if (cookieNames != null) { + LOG.debugf("Remove session cookie names: %s", cookieNames); StringBuilder cookieValue = new StringBuilder(); for (String cookieName : cookieNames) { cookieValue.append(removeCookie(context, oidcConfig, cookieName)); @@ -705,4 +717,22 @@ public static boolean isSessionCookie(String cookieName) { && !cookieName.regionMatches(SESSION_COOKIE_NAME.length(), ACCESS_TOKEN_COOKIE_SUFFIX, 0, 3) && !cookieName.regionMatches(SESSION_COOKIE_NAME.length(), REFRESH_TOKEN_COOKIE_SUFFIX, 0, 3); } + + public static String getTenantIdFromCookie(String cookiePrefix, String cookieName, boolean sessionCookie) { + // It has already been checked the cookieName starts with the cookiePrefix + if (cookieName.length() == cookiePrefix.length()) { + return OidcUtils.DEFAULT_TENANT_ID; + } else { + String suffix = cookieName.substring(cookiePrefix.length() + 1); + + if (sessionCookie && suffix.startsWith(OidcUtils.SESSION_COOKIE_CHUNK_START)) { + return OidcUtils.DEFAULT_TENANT_ID; + } else { + // It can be either a tenant_id, or a tenant_id and cookie suffix property, example, q_session_github or q_session_github_test + // or it can be a session cookie chunk like q_session_chunk_1 in which case the suffix will be chunk_1 + int index = suffix.indexOf("_", 0); + return index == -1 ? suffix : suffix.substring(0, index); + } + } + } } 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 7251c41a6fb2b..d4d85a4be26d2 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 @@ -322,6 +322,16 @@ public void testSessionCookieCheck() throws Exception { assertFalse(OidcUtils.isSessionCookie(OidcUtils.SESSION_AT_COOKIE_NAME + "1")); } + @Test + public void testGetSessionCookieTenantId() throws Exception { + assertEquals(OidcUtils.DEFAULT_TENANT_ID, + OidcUtils.getTenantIdFromCookie(OidcUtils.SESSION_COOKIE_NAME, "q_session", true)); + assertEquals(OidcUtils.DEFAULT_TENANT_ID, + OidcUtils.getTenantIdFromCookie(OidcUtils.SESSION_COOKIE_NAME, "q_session_chunk_1", true)); + assertEquals("a", OidcUtils.getTenantIdFromCookie(OidcUtils.SESSION_COOKIE_NAME, "q_session_a", true)); + assertEquals("a", OidcUtils.getTenantIdFromCookie(OidcUtils.SESSION_COOKIE_NAME, "q_session_a_chunk_1", true)); + } + public static JsonObject read(InputStream input) throws IOException { try (BufferedReader buffer = new BufferedReader(new InputStreamReader(input, StandardCharsets.UTF_8))) { return new JsonObject(buffer.lines().collect(Collectors.joining("\n"))); 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 0096a38a3401f..6cb2fdfaaa639 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 @@ -31,6 +31,7 @@ import com.gargoylesoftware.htmlunit.CookieManager; import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; import com.gargoylesoftware.htmlunit.SilentCssErrorHandler; +import com.gargoylesoftware.htmlunit.TextPage; import com.gargoylesoftware.htmlunit.WebClient; import com.gargoylesoftware.htmlunit.WebRequest; import com.gargoylesoftware.htmlunit.WebResponse; @@ -228,9 +229,17 @@ public void testCodeFlowForceHttpsRedirectUriAndPkce() throws Exception { assertEquals("tenant-https:reauthenticated", page.getBody().asNormalizedText()); List sessionCookies = verifyTenantHttpTestCookies(webClient); - assertEquals("strict", sessionCookies.get(0).getSameSite()); assertEquals("strict", sessionCookies.get(1).getSameSite()); + + // Check both session cookie chunks are removed if the new authentication is enforced + webClient.getOptions().setRedirectEnabled(false); + webClient.getOptions().setThrowExceptionOnFailingStatusCode(false); + + TextPage textPage = webClient.getPage("http://localhost:8081/index.html"); + assertEquals(302, textPage.getWebResponse().getStatusCode()); + assertNull(getSessionCookies(webClient, "tenant-https")); + webClient.getCookieManager().clearCookies(); } } diff --git a/integration-tests/oidc-tenancy/src/test/java/io/quarkus/it/keycloak/BearerTokenAuthorizationTest.java b/integration-tests/oidc-tenancy/src/test/java/io/quarkus/it/keycloak/BearerTokenAuthorizationTest.java index e1fb18f39198d..7732795c191e4 100644 --- a/integration-tests/oidc-tenancy/src/test/java/io/quarkus/it/keycloak/BearerTokenAuthorizationTest.java +++ b/integration-tests/oidc-tenancy/src/test/java/io/quarkus/it/keycloak/BearerTokenAuthorizationTest.java @@ -295,6 +295,7 @@ public void testReAuthenticateWhenSwitchingTenants() throws IOException { loginForm.getInputByName("password").setValueAttribute("alice"); page = loginForm.getInputByName("login").click(); assertEquals("tenant-web-app:alice:reauthenticated", page.getBody().asNormalizedText()); + assertNotNull(getSessionCookie(webClient, "tenant-web-app")); // tenant-web-app2 page = webClient.getPage("http://localhost:8081/tenant/tenant-web-app2/api/user/webapp2"); assertNull(getStateCookieSavedPath(webClient, "tenant-web-app2")); @@ -304,7 +305,8 @@ public void testReAuthenticateWhenSwitchingTenants() throws IOException { loginForm.getInputByName("password").setValueAttribute("alice"); page = loginForm.getInputByName("login").click(); assertEquals("tenant-web-app2:alice", page.getBody().asNormalizedText()); - + assertNull(getSessionCookie(webClient, "tenant-web-app")); + assertNotNull(getSessionCookie(webClient, "tenant-web-app2")); webClient.getCookieManager().clearCookies(); } }