Fix named-tenant user info acquisition / access token verification de…

…tection
quarkusio · Mar 15, 2024 · 0e80675 · 0e80675
1 parent 17ec2f9
commit 0e80675
Show file tree

Hide file tree

Showing 8 changed files with 57 additions and 27 deletions.
diff --git a/extensions/oidc/deployment/src/main/java/io/quarkus/oidc/deployment/OidcBuildStep.java b/extensions/oidc/deployment/src/main/java/io/quarkus/oidc/deployment/OidcBuildStep.java
@@ -303,6 +303,8 @@ void detectUserInfoRequired(BeanRegistrationPhaseBuildItem beanRegistrationPhase
         if (isInjected(beanRegistrationPhaseBuildItem, USER_INFO_NAME, null)) {
             runtimeConfigDefaultProducer.produce(
                     new RunTimeConfigurationDefaultBuildItem("quarkus.oidc.authentication.user-info-required", "true"));
+            runtimeConfigDefaultProducer.produce(
+                    new RunTimeConfigurationDefaultBuildItem("quarkus.oidc.*.authentication.user-info-required", "true"));
         }
     }
 
@@ -312,6 +314,8 @@ void detectAccessTokenVerificationRequired(BeanRegistrationPhaseBuildItem beanRe
         if (isInjected(beanRegistrationPhaseBuildItem, JSON_WEB_TOKEN_NAME, ID_TOKEN_NAME)) {
             runtimeConfigDefaultProducer.produce(
                     new RunTimeConfigurationDefaultBuildItem("quarkus.oidc.authentication.verify-access-token", "true"));
+            runtimeConfigDefaultProducer.produce(
+                    new RunTimeConfigurationDefaultBuildItem("quarkus.oidc.*.authentication.verify-access-token", "true"));
         }
     }
 

diff --git a/...ons/oidc/deployment/src/test/java/io/quarkus/oidc/test/UserInfoRequiredDetectionTest.java b/...ons/oidc/deployment/src/test/java/io/quarkus/oidc/test/UserInfoRequiredDetectionTest.java
@@ -34,7 +34,8 @@ public class UserInfoRequiredDetectionTest {
                                     """
                                             quarkus.oidc.tenant-paths=/user-info/default-tenant
                                             quarkus.oidc.user-info-path=http://${quarkus.http.host}:${quarkus.http.port}/user-info-endpoint
-                                            quarkus.oidc.named.authentication.user-info-required=true
+                                            quarkus.oidc.named.auth-server-url=${quarkus.oidc.auth-server-url}
+                                            quarkus.oidc.named.tenant-paths=/user-info/named-tenant
                                             quarkus.oidc.named.user-info-path=http://${quarkus.http.host}:${quarkus.http.port}/user-info-endpoint
                                             quarkus.http.auth.proactive=false
                                             """),

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
@@ -217,7 +217,8 @@ private Uni<TenantConfigContext> createTenantContext(Vertx vertx, OidcTenantConf
                                 .item(new TenantConfigContext(new OidcProvider(null, null, null, null), oidcConfig));
                     }
                 }
-                throw new ConfigurationException("'quarkus.oidc.auth-server-url' property must be configured");
+                throw new ConfigurationException(
+                        "'" + getConfigPropertyForTenant(tenantId, "auth-server-url") + "' property must be configured");
             }
             OidcCommonUtils.verifyEndpointUrl(oidcConfig.getAuthServerUrl().get());
             OidcCommonUtils.verifyCommonConfiguration(oidcConfig, OidcUtils.isServiceApp(oidcConfig), true);
@@ -242,10 +243,13 @@ private Uni<TenantConfigContext> createTenantContext(Vertx vertx, OidcTenantConf
         if (!oidcConfig.discoveryEnabled.orElse(true)) {
             if (!OidcUtils.isServiceApp(oidcConfig)) {
                 if (!oidcConfig.authorizationPath.isPresent() || !oidcConfig.tokenPath.isPresent()) {
+                    String authorizationPathProperty = getConfigPropertyForTenant(tenantId, "authorization-path");
+                    String tokenPathProperty = getConfigPropertyForTenant(tenantId, "token-path");
                     throw new ConfigurationException(
-                            "'web-app' applications must have 'authorization-path' and 'token-path' properties "
+                            "'web-app' applications must have '" + authorizationPathProperty + "' and '" + tokenPathProperty
+                                    + "' properties "
                                     + "set when the discovery is disabled.",
-                            Set.of("quarkus.oidc.authorization-path", "quarkus.oidc.token-path"));
+                            Set.of(authorizationPathProperty, tokenPathProperty));
                 }
             }
             // JWK and introspection endpoints have to be set for both 'web-app' and 'service' applications
@@ -260,31 +264,35 @@ private Uni<TenantConfigContext> createTenantContext(Vertx vertx, OidcTenantConf
                 }
             }
             if (oidcConfig.authentication.userInfoRequired.orElse(false) && !oidcConfig.userInfoPath.isPresent()) {
+                String configProperty = getConfigPropertyForTenant(tenantId, "user-info-path");
                 throw new ConfigurationException(
-                        "UserInfo is required but 'quarkus.oidc.user-info-path' is not configured.",
-                        Set.of("quarkus.oidc.user-info-path"));
+                        "UserInfo is required but '" + configProperty + "' is not configured.",
+                        Set.of(configProperty));
             }
         }
 
         if (OidcUtils.isServiceApp(oidcConfig)) {
             if (oidcConfig.token.refreshExpired) {
                 throw new ConfigurationException(
-                        "The 'token.refresh-expired' property can only be enabled for " + ApplicationType.WEB_APP
+                        "The '" + getConfigPropertyForTenant(tenantId, "token.refresh-expired")
+                                + "' property can only be enabled for " + ApplicationType.WEB_APP
                                 + " application types");
             }
             if (!oidcConfig.token.refreshTokenTimeSkew.isEmpty()) {
                 throw new ConfigurationException(
-                        "The 'token.refresh-token-time-skew' property can only be enabled for " + ApplicationType.WEB_APP
+                        "The '" + getConfigPropertyForTenant(tenantId, "token.refresh-token-time-skew")
+                                + "' property can only be enabled for " + ApplicationType.WEB_APP
                                 + " application types");
             }
             if (oidcConfig.logout.path.isPresent()) {
                 throw new ConfigurationException(
-                        "The 'logout.path' property can only be enabled for " + ApplicationType.WEB_APP
-                                + " application types");
+                        "The '" + getConfigPropertyForTenant(tenantId, "logout.path") + "' property can only be enabled for "
+                                + ApplicationType.WEB_APP + " application types");
             }
             if (oidcConfig.roles.source.isPresent() && oidcConfig.roles.source.get() == Source.idtoken) {
                 throw new ConfigurationException(
-                        "The 'roles.source' property can only be set to 'idtoken' for " + ApplicationType.WEB_APP
+                        "The '" + getConfigPropertyForTenant(tenantId, "roles.source")
+                                + "' property can only be set to 'idtoken' for " + ApplicationType.WEB_APP
                                 + " application types");
             }
         } else {
@@ -320,10 +328,12 @@ private Uni<TenantConfigContext> createTenantContext(Vertx vertx, OidcTenantConf
         }
 
         if (!oidcConfig.token.isIssuedAtRequired() && oidcConfig.token.getAge().isPresent()) {
+            String tokenIssuedAtRequired = getConfigPropertyForTenant(tenantId, "token.issued-at-required");
+            String tokenAge = getConfigPropertyForTenant(tenantId, "token.age");
             throw new ConfigurationException(
-                    "The 'token.issued-at-required' can only be set to false if 'token.age' is not set." +
-                            " Either set 'token.issued-at-required' to true or do not set 'token.age'.",
-                    Set.of("quarkus.oidc.token.issued-at-required", "quarkus.oidc.token.age"));
+                    "The '" + tokenIssuedAtRequired + "' can only be set to false if '" + tokenAge + "' is not set." +
+                            " Either set '" + tokenIssuedAtRequired + "' to true or do not set '" + tokenAge + "'.",
+                    Set.of(tokenIssuedAtRequired, tokenAge));
         }
 
         return createOidcProvider(oidcConfig, tlsConfig, vertx)
@@ -335,6 +345,14 @@ public TenantConfigContext apply(OidcProvider p) {
                 });
     }
 
+    private static String getConfigPropertyForTenant(String tenantId, String configSubKey) {
+        if (DEFAULT_TENANT_ID.equals(tenantId)) {
+            return "quarkus.oidc." + configSubKey;
+        } else {
+            return "quarkus.oidc." + tenantId + "." + configSubKey;
+        }
+    }
+
     private static boolean enableUserInfo(OidcTenantConfig oidcConfig) {
         Optional<Boolean> userInfoRequired = oidcConfig.authentication.isUserInfoRequired();
         if (userInfoRequired.isPresent()) {

diff --git a/integration-tests/oidc-code-flow/src/main/resources/application.properties b/integration-tests/oidc-code-flow/src/main/resources/application.properties
@@ -143,19 +143,25 @@ quarkus.oidc.tenant-idtoken-only.client-id=quarkus-app
 quarkus.oidc.tenant-idtoken-only.credentials.secret=secret
 quarkus.oidc.tenant-idtoken-only.token-state-manager.strategy=id-token
 quarkus.oidc.tenant-idtoken-only.application-type=web-app
+quarkus.oidc.tenant-idtoken-only.authentication.user-info-required=false
+quarkus.oidc.tenant-idtoken-only.authentication.verify-access-token=false
 
 quarkus.oidc.tenant-id-refresh-token.auth-server-url=${quarkus.oidc.auth-server-url}
 quarkus.oidc.tenant-id-refresh-token.client-id=quarkus-app
 quarkus.oidc.tenant-id-refresh-token.credentials.secret=secret
 quarkus.oidc.tenant-id-refresh-token.token-state-manager.strategy=id-refresh-tokens
 quarkus.oidc.tenant-id-refresh-token.application-type=web-app
+quarkus.oidc.tenant-id-refresh-token.authentication.user-info-required=false
+quarkus.oidc.tenant-id-refresh-token.authentication.verify-access-token=false
 
 quarkus.oidc.tenant-split-id-refresh-token.auth-server-url=${quarkus.oidc.auth-server-url}
 quarkus.oidc.tenant-split-id-refresh-token.client-id=quarkus-app
 quarkus.oidc.tenant-split-id-refresh-token.credentials.secret=secret
 quarkus.oidc.tenant-split-id-refresh-token.token-state-manager.strategy=id-refresh-tokens
 quarkus.oidc.tenant-split-id-refresh-token.token-state-manager.split-tokens=true
 quarkus.oidc.tenant-split-id-refresh-token.application-type=web-app
+quarkus.oidc.tenant-split-id-refresh-token.authentication.user-info-required=false
+quarkus.oidc.tenant-split-id-refresh-token.authentication.verify-access-token=false
 
 quarkus.oidc.tenant-split-tokens.auth-server-url=${quarkus.oidc.auth-server-url}
 quarkus.oidc.tenant-split-tokens.client-id=quarkus-app

diff --git a/integration-tests/oidc-wiremock/src/main/resources/application.properties b/integration-tests/oidc-wiremock/src/main/resources/application.properties
@@ -1,10 +1,13 @@
+
 # Configuration file
 quarkus.oidc.auth-server-url=http://localhost:8180/auth/realms/quarkus2/
 quarkus.oidc.client-id=quarkus-app
 quarkus.oidc.credentials.secret=secret
 quarkus.oidc.authentication.scopes=profile,email,phone
+quarkus.oidc.authentication.user-info-required=false
 
 quarkus.oidc.no-discovery.auth-server-url=http://localhost:8180/auth/realms/quarkus2/
+quarkus.oidc.no-discovery.authentication.user-info-required=false
 quarkus.oidc.no-discovery.discovery-enabled=false
 quarkus.oidc.no-discovery.jwks-path=protocol/openid-connect/certs
 quarkus.oidc.no-discovery.client-id=quarkus-app
@@ -13,6 +16,8 @@ quarkus.oidc.no-discovery.authentication.scopes=profile,email,phone
 
 quarkus.oidc.code-flow.auth-server-url=${keycloak.url}/realms/quarkus/
 quarkus.oidc.code-flow.client-id=quarkus-web-app
+quarkus.oidc.code-flow.authentication.user-info-required=false
+quarkus.oidc.code-flow.authentication.verify-access-token=false
 quarkus.oidc.code-flow.logout.path=/code-flow/logout
 quarkus.oidc.code-flow.logout.post-logout-path=/code-flow/post-logout
 quarkus.oidc.code-flow.logout.post-logout-uri-param=returnTo
@@ -40,6 +45,7 @@ quarkus.oidc.code-flow-encrypted-id-token-pem.token.decryption-key-location=priv
 quarkus.oidc.code-flow-encrypted-id-token-pem.token.audience=any
 
 quarkus.oidc.code-flow-form-post.auth-server-url=${keycloak.url}/realms/quarkus-form-post/
+quarkus.oidc.code-flow-form-post.authentication.user-info-required=false
 quarkus.oidc.code-flow-form-post.client-id=quarkus-web-app
 quarkus.oidc.code-flow-form-post.credentials.secret=AyM1SysPpbyDfgZld3umj1qzKObwVMkoqQ-EstJQLr_T-1qS0gZH75aKtMN3Yj0iPS4hcgUuTwjAzZr1Z9CAow
 quarkus.oidc.code-flow-form-post.application-type=web-app
@@ -61,6 +67,7 @@ quarkus.oidc.code-flow-user-info-only.authorization-path=/
 quarkus.oidc.code-flow-user-info-only.token-path=access_token
 quarkus.oidc.code-flow-user-info-only.user-info-path=protocol/openid-connect/userinfo
 quarkus.oidc.code-flow-user-info-only.authentication.id-token-required=false
+quarkus.oidc.code-flow-user-info-only.authentication.verify-access-token=false
 quarkus.oidc.code-flow-user-info-only.code-grant.extra-params.extra-param=extra-param-value
 quarkus.oidc.code-flow-user-info-only.code-grant.headers.X-Custom=XCustomHeaderValue
 quarkus.oidc.code-flow-user-info-only.client-id=quarkus-web-app
@@ -69,6 +76,7 @@ quarkus.oidc.code-flow-user-info-only.application-type=web-app
 
 quarkus.oidc.code-flow-user-info-github.provider=github
 quarkus.oidc.code-flow-user-info-github.authentication.internal-id-token-lifespan=7H
+quarkus.oidc.code-flow-user-info-github.authentication.verify-access-token=false
 quarkus.oidc.code-flow-user-info-github.auth-server-url=${keycloak.url}/realms/quarkus/
 quarkus.oidc.code-flow-user-info-github.authorization-path=/
 quarkus.oidc.code-flow-user-info-github.user-info-path=protocol/openid-connect/userinfo
@@ -146,6 +154,7 @@ quarkus.oidc.bearer-required-algorithm.credentials.secret=secret
 quarkus.oidc.bearer-required-algorithm.token.signature-algorithm=PS256
 
 quarkus.oidc.bearer-azure.provider=microsoft
+quarkus.oidc.bearer-azure.authentication.user-info-required=false
 quarkus.oidc.bearer-azure.application-type=service
 quarkus.oidc.bearer-azure.discovery-enabled=false
 quarkus.oidc.bearer-azure.jwks-path=${keycloak.url}/azure/jwk
@@ -180,6 +189,7 @@ quarkus.oidc.bearer-certificate-full-chain-root-only-wrongcname.certificate-chai
 quarkus.oidc.bearer-certificate-full-chain-root-only-wrongcname.certificate-chain.leaf-certificate-name=www.quarkusio.com
 
 quarkus.oidc.bearer-key-without-kid-thumbprint.auth-server-url=${keycloak.url}/realms/quarkus/
+quarkus.oidc.bearer-key-without-kid-thumbprint.authentication.user-info-required=false
 quarkus.oidc.bearer-key-without-kid-thumbprint.discovery-enabled=false
 quarkus.oidc.bearer-key-without-kid-thumbprint.jwks-path=single-key-without-kid-thumbprint
 quarkus.oidc.bearer-key-without-kid-thumbprint.client-id=quarkus-app

diff --git a/...n-tests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/AnnotationBasedTenantTest.java b/...n-tests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/AnnotationBasedTenantTest.java
@@ -23,7 +23,7 @@ public class AnnotationBasedTenantTest {
     public static class NoProactiveAuthTestProfile implements QuarkusTestProfile {
         public Map<String, String> getConfigOverrides() {
             return Map.ofEntries(Map.entry("quarkus.http.auth.proactive", "false"),
-                    Map.entry("quarkus.oidc.authentication.user-info-required", "false"),
+                    Map.entry("quarkus.oidc.hr.authentication.user-info-required", "false"),
                     Map.entry("quarkus.oidc.hr.auth-server-url", "http://localhost:8180/auth/realms/quarkus2/"),
                     Map.entry("quarkus.oidc.hr.client-id", "quarkus-app"),
                     Map.entry("quarkus.oidc.hr.credentials.secret", "secret"),

diff --git a/...ests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/BearerTokenOidcRecoveredTest.java b/...ests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/BearerTokenOidcRecoveredTest.java
@@ -4,7 +4,6 @@
 
 import java.util.Arrays;
 import java.util.HashSet;
-import java.util.Map;
 import java.util.Set;
 
 import org.hamcrest.Matchers;
@@ -14,23 +13,13 @@
 import org.junit.jupiter.api.TestMethodOrder;
 
 import io.quarkus.test.junit.QuarkusTest;
-import io.quarkus.test.junit.QuarkusTestProfile;
-import io.quarkus.test.junit.TestProfile;
 import io.restassured.RestAssured;
 import io.smallrye.jwt.build.Jwt;
 
 @TestMethodOrder(MethodOrderer.OrderAnnotation.class)
 @QuarkusTest
-@TestProfile(BearerTokenOidcRecoveredTest.DisabledUserInfoTestProfile.class)
 public class BearerTokenOidcRecoveredTest {
 
-    public static class DisabledUserInfoTestProfile implements QuarkusTestProfile {
-        @Override
-        public Map<String, String> getConfigOverrides() {
-            return Map.of("quarkus.oidc.authentication.user-info-required", "false");
-        }
-    }
-
     @Order(1)
     @Test
     public void testOidcRecoveredWithDiscovery() {

diff --git a/...n-tests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/CodeFlowAuthorizationTest.java b/...n-tests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/CodeFlowAuthorizationTest.java
@@ -239,7 +239,7 @@ public void testCodeFlowFormPostAndFrontChannelLogout() throws Exception {
     @Test
     public void testCodeFlowUserInfo() throws Exception {
         defineCodeFlowAuthorizationOauth2TokenStub();
-
+        wireMockServer.resetRequests();
         doTestCodeFlowUserInfo("code-flow-user-info-only", 300);
         clearCache();
         doTestCodeFlowUserInfo("code-flow-user-info-github", 25200);
@@ -315,6 +315,7 @@ public void testCodeFlowTokenIntrospection() throws Exception {
     private void doTestCodeFlowUserInfo(String tenantId, long internalIdTokenLifetime) throws Exception {
         try (final WebClient webClient = createWebClient()) {
             webClient.getOptions().setRedirectEnabled(true);
+            wireMockServer.verify(0, getRequestedFor(urlPathMatching("/auth/realms/quarkus/protocol/openid-connect/userinfo")));
             HtmlPage page = webClient.getPage("http://localhost:8081/" + tenantId);
 
             HtmlForm form = page.getFormByName("form");
@@ -344,6 +345,7 @@ private void doTestCodeFlowUserInfo(String tenantId, long internalIdTokenLifetim
             assertTrue(date.toInstant().getEpochSecond() - issuedAt <= internalIdTokenLifetime + 3);
 
             webClient.getCookieManager().clearCookies();
+            wireMockServer.resetRequests();
         }
     }