From 38738c47b08d9cad341c7768877fad8147605c3d Mon Sep 17 00:00:00 2001 From: Robert Stupp Date: Mon, 30 Sep 2024 11:28:12 +0200 Subject: [PATCH 1/2] OIDC: Encapsulate static/dynamic tenants maps in `TenantConfigBean` Lets `TenantConfigBean` be the sole "owner" of the static/dynamic tenants maps, adds/changes accessor methods for tenants. Also introduces a functional interface to create tenants. No functional change, only moving code around. --- .../runtime/DefaultTenantConfigResolver.java | 23 ++++----- .../io/quarkus/oidc/runtime/OidcRecorder.java | 39 +++++---------- .../oidc/runtime/StaticTenantResolver.java | 2 +- .../oidc/runtime/TenantConfigBean.java | 49 +++++++++++++++---- .../it/keycloak/ProtectedResource.java | 2 +- .../io/quarkus/it/keycloak/TenantRefresh.java | 2 +- 6 files changed, 66 insertions(+), 51 deletions(-) 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 15ae16cca5ece..781e0b6c2822b 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 @@ -112,7 +112,7 @@ public Uni apply(OidcTenantConfig oidcTenantConfig) { final String tenantId = context.get(OidcUtils.TENANT_ID_ATTRIBUTE); if (tenantId != null && !isTenantSetByAnnotation(context, tenantId)) { - TenantConfigContext tenantContext = tenantConfigBean.getDynamicTenantsConfig().get(tenantId); + TenantConfigContext tenantContext = tenantConfigBean.getDynamicTenant(tenantId); if (tenantContext != null) { return Uni.createFrom().item(tenantContext.getOidcTenantConfig()); } @@ -197,7 +197,7 @@ private boolean isTenantSetByAnnotation(RoutingContext context, String tenantId) } private TenantConfigContext getStaticTenantContext(String tenantId) { - TenantConfigContext configContext = tenantId != null ? tenantConfigBean.getStaticTenantsConfig().get(tenantId) : null; + var configContext = tenantId != null ? tenantConfigBean.getStaticTenant(tenantId) : null; if (configContext == null) { if (tenantId != null && !tenantId.isEmpty()) { LOG.debugf( @@ -261,18 +261,18 @@ private Uni getDynamicTenantContext(RoutingContext context) @Override public Uni apply(OidcTenantConfig tenantConfig) { if (tenantConfig != null) { - String tenantId = tenantConfig.getTenantId() + var tenantId = tenantConfig.getTenantId() .orElseThrow(() -> new OIDCException("Tenant configuration must have tenant id")); - TenantConfigContext tenantContext = tenantConfigBean.getDynamicTenantsConfig().get(tenantId); + var tenantContext = tenantConfigBean.getDynamicTenant(tenantId); if (tenantContext == null) { - return tenantConfigBean.getTenantConfigContextFactory().apply(tenantConfig); + return tenantConfigBean.createDynamicTenantContext(tenantConfig); } else { return Uni.createFrom().item(tenantContext); } } else { final String tenantId = context.get(OidcUtils.TENANT_ID_ATTRIBUTE); if (tenantId != null && !isTenantSetByAnnotation(context, tenantId)) { - TenantConfigContext tenantContext = tenantConfigBean.getDynamicTenantsConfig().get(tenantId); + TenantConfigContext tenantContext = tenantConfigBean.getDynamicTenant(tenantId); if (tenantContext != null) { return Uni.createFrom().item(tenantContext); } @@ -304,14 +304,11 @@ public OidcTenantConfig getResolvedConfig(String sessionTenantId) { return tenantConfigBean.getDefaultTenant().getOidcTenantConfig(); } - if (tenantConfigBean.getStaticTenantsConfig().containsKey(sessionTenantId)) { - return tenantConfigBean.getStaticTenantsConfig().get(sessionTenantId).getOidcTenantConfig(); + var tenant = tenantConfigBean.getStaticTenant(sessionTenantId); + if (tenant == null) { + tenant = tenantConfigBean.getDynamicTenant(sessionTenantId); } - - if (tenantConfigBean.getDynamicTenantsConfig().containsKey(sessionTenantId)) { - return tenantConfigBean.getDynamicTenantsConfig().get(sessionTenantId).getOidcTenantConfig(); - } - return null; + return tenant != null ? tenant.getOidcTenantConfig() : null; } public String getRootPath() { diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcRecorder.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcRecorder.java index fc968952aca15..d06efc38e43af 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcRecorder.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcRecorder.java @@ -71,7 +71,6 @@ public class OidcRecorder { private static final Logger LOG = Logger.getLogger(OidcRecorder.class); private static final String SECURITY_EVENTS_ENABLED_CONFIG_KEY = "quarkus.security.events.enabled"; - private static final Map dynamicTenantsConfig = new ConcurrentHashMap<>(); private static final Set tenantsExpectingServerAvailableEvents = ConcurrentHashMap.newKeySet(); private static volatile boolean userInfoInjectionPointDetected = false; @@ -128,42 +127,30 @@ public TenantConfigBean setup(OidcConfig config, Vertx vertxValue, OidcTlsSuppor staticTenantInitializer)); } - return new TenantConfigBean(staticTenantsConfig, dynamicTenantsConfig, defaultTenantContext, - new Function>() { + return new TenantConfigBean(staticTenantsConfig, defaultTenantContext, + new TenantConfigBean.TenantContextFactory() { @Override - public Uni apply(OidcTenantConfig config) { - return createDynamicTenantContext(vertxValue, config, config.getTenantId().get(), - tlsSupport); + public Uni create(OidcTenantConfig config) { + return createDynamicTenantContext(vertxValue, config, tlsSupport); } }); } private Uni createDynamicTenantContext(Vertx vertx, - OidcTenantConfig oidcConfig, String tenantId, OidcTlsSupport tlsSupport) { + OidcTenantConfig oidcConfig, OidcTlsSupport tlsSupport) { + var tenantId = oidcConfig.tenantId.orElseThrow(); if (oidcConfig.logout.backchannel.path.isPresent()) { throw new ConfigurationException( "BackChannel Logout is currently not supported for dynamic tenants"); } - if (!dynamicTenantsConfig.containsKey(tenantId)) { - Uni uniContext = createTenantContext(vertx, oidcConfig, false, tenantId, tlsSupport) - .onFailure().transform(new Function() { - @Override - public Throwable apply(Throwable t) { - return logTenantConfigContextFailure(t, tenantId); - } - }); - return uniContext.onItem().transform( - new Function() { - @Override - public TenantConfigContext apply(TenantConfigContext t) { - dynamicTenantsConfig.putIfAbsent(tenantId, t); - return t; - } - }); - } else { - return Uni.createFrom().item(dynamicTenantsConfig.get(tenantId)); - } + return createTenantContext(vertx, oidcConfig, false, tenantId, tlsSupport) + .onFailure().transform(new Function() { + @Override + public Throwable apply(Throwable t) { + return logTenantConfigContextFailure(t, tenantId); + } + }); } private TenantConfigContext createStaticTenantContext(Vertx vertx, diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/StaticTenantResolver.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/StaticTenantResolver.java index dbe520988612e..8f79d15a56a14 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/StaticTenantResolver.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/StaticTenantResolver.java @@ -92,7 +92,7 @@ public String resolve(RoutingContext context) { String[] pathSegments = context.request().path().split("/"); if (pathSegments.length > 0) { String lastPathSegment = pathSegments[pathSegments.length - 1]; - if (tenantConfigBean.getStaticTenantsConfig().containsKey(lastPathSegment)) { + if (tenantConfigBean.getStaticTenant(lastPathSegment) != null) { LOG.debugf( "Tenant id '%s' is selected on the '%s' request path", lastPathSegment, context.normalizedPath()); return lastPathSegment; diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigBean.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigBean.java index 073e6432fa8c1..29923013eb333 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigBean.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigBean.java @@ -1,6 +1,8 @@ package io.quarkus.oidc.runtime; +import java.util.Collections; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; import jakarta.enterprise.context.spi.CreationalContext; @@ -14,33 +16,62 @@ public class TenantConfigBean { private final Map staticTenantsConfig; private final Map dynamicTenantsConfig; private final TenantConfigContext defaultTenant; - private final Function> tenantConfigContextFactory; + private final TenantContextFactory tenantContextFactory; + private final Map unmodifiableDynamicTenants; + + @FunctionalInterface + public interface TenantContextFactory { + Uni create(OidcTenantConfig oidcTenantConfig); + } public TenantConfigBean( Map staticTenantsConfig, - Map dynamicTenantsConfig, TenantConfigContext defaultTenant, - Function> tenantConfigContextFactory) { + TenantContextFactory tenantContextFactory) { this.staticTenantsConfig = Map.copyOf(staticTenantsConfig); - this.dynamicTenantsConfig = dynamicTenantsConfig; + this.dynamicTenantsConfig = new ConcurrentHashMap<>(); + this.unmodifiableDynamicTenants = Collections.unmodifiableMap(dynamicTenantsConfig); this.defaultTenant = defaultTenant; - this.tenantConfigContextFactory = tenantConfigContextFactory; + this.tenantContextFactory = tenantContextFactory; + } + + public Uni createDynamicTenantContext(OidcTenantConfig oidcConfig) { + var tenantId = oidcConfig.tenantId.orElseThrow(); + + var tenant = dynamicTenantsConfig.get(tenantId); + if (tenant != null) { + return Uni.createFrom().item(tenant); + } + + return tenantContextFactory.create(oidcConfig).onItem().transform( + new Function() { + @Override + public TenantConfigContext apply(TenantConfigContext t) { + dynamicTenantsConfig.putIfAbsent(tenantId, t); + return t; + } + }); } public Map getStaticTenantsConfig() { return staticTenantsConfig; } + public TenantConfigContext getStaticTenant(String tenantId) { + return staticTenantsConfig.get(tenantId); + } + public TenantConfigContext getDefaultTenant() { return defaultTenant; } - public Function> getTenantConfigContextFactory() { - return tenantConfigContextFactory; + @SuppressWarnings("unused") // retained for ABI compatibility + public Map getDynamicTenantsConfig() { + return unmodifiableDynamicTenants; } - public Map getDynamicTenantsConfig() { - return dynamicTenantsConfig; + public TenantConfigContext getDynamicTenant(String tenantId) { + return dynamicTenantsConfig.get(tenantId); } public static class Destroyer implements BeanDestroyer { diff --git a/integration-tests/oidc-client-registration/src/main/java/io/quarkus/it/keycloak/ProtectedResource.java b/integration-tests/oidc-client-registration/src/main/java/io/quarkus/it/keycloak/ProtectedResource.java index 23501f0d2ef23..81ace6de744cb 100644 --- a/integration-tests/oidc-client-registration/src/main/java/io/quarkus/it/keycloak/ProtectedResource.java +++ b/integration-tests/oidc-client-registration/src/main/java/io/quarkus/it/keycloak/ProtectedResource.java @@ -62,7 +62,7 @@ public String principalNameMulti2() { } private String getClientName() { - OidcTenantConfig oidcConfig = tenantConfigBean.getDynamicTenantsConfig().get(session.getTenantId()) + OidcTenantConfig oidcConfig = tenantConfigBean.getDynamicTenant(session.getTenantId()) .getOidcTenantConfig(); return oidcConfig.getClientName().get(); } diff --git a/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/TenantRefresh.java b/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/TenantRefresh.java index c1c4646559d67..9b3773a7583a8 100644 --- a/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/TenantRefresh.java +++ b/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/TenantRefresh.java @@ -39,7 +39,7 @@ public String sessionExpired(@CookieParam("session_expired") String sessionExpir // Cookie format: jwt| String[] pair = sessionExpired.split("\\|"); - OidcTenantConfig oidcConfig = tenantConfig.getStaticTenantsConfig().get(pair[1]).getOidcTenantConfig(); + OidcTenantConfig oidcConfig = tenantConfig.getStaticTenant(pair[1]).getOidcTenantConfig(); JsonWebToken jwt = new DefaultJWTParser().decrypt(pair[0], oidcConfig.credentials.secret.get()); OidcUtils.removeCookie(context, oidcConfig, "session_expired"); From ac50eb67e711916a646c1b48a322edb74a282b2e Mon Sep 17 00:00:00 2001 From: Robert Stupp Date: Mon, 30 Sep 2024 14:39:45 +0200 Subject: [PATCH 2/2] review --- .../java/io/quarkus/oidc/runtime/TenantConfigBean.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigBean.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigBean.java index 29923013eb333..8407a9d5bdfea 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigBean.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigBean.java @@ -1,6 +1,5 @@ package io.quarkus.oidc.runtime; -import java.util.Collections; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; @@ -17,7 +16,6 @@ public class TenantConfigBean { private final Map dynamicTenantsConfig; private final TenantConfigContext defaultTenant; private final TenantContextFactory tenantContextFactory; - private final Map unmodifiableDynamicTenants; @FunctionalInterface public interface TenantContextFactory { @@ -30,7 +28,6 @@ public TenantConfigBean( TenantContextFactory tenantContextFactory) { this.staticTenantsConfig = Map.copyOf(staticTenantsConfig); this.dynamicTenantsConfig = new ConcurrentHashMap<>(); - this.unmodifiableDynamicTenants = Collections.unmodifiableMap(dynamicTenantsConfig); this.defaultTenant = defaultTenant; this.tenantContextFactory = tenantContextFactory; } @@ -65,11 +62,6 @@ public TenantConfigContext getDefaultTenant() { return defaultTenant; } - @SuppressWarnings("unused") // retained for ABI compatibility - public Map getDynamicTenantsConfig() { - return unmodifiableDynamicTenants; - } - public TenantConfigContext getDynamicTenant(String tenantId) { return dynamicTenantsConfig.get(tenantId); }