Merge pull request #39458 from michalvavrik/feature/oidc-required-pro…

…ps-detect-injection Enforce OIDC UserInfo acquisition and authorization code flow access token verification if UserInfo and JsonWebToken beans are injected
quarkusio · Mar 15, 2024 · 0ff6395 · 0ff6395
2 parents 5a38253 + c7ec8ee
commit 0ff6395
Show file tree

Hide file tree

Showing 7 changed files with 191 additions and 2 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
@@ -17,6 +17,7 @@
 import jakarta.inject.Singleton;
 
 import org.eclipse.microprofile.jwt.Claim;
+import org.eclipse.microprofile.jwt.JsonWebToken;
 import org.jboss.jandex.AnnotationInstance;
 import org.jboss.jandex.AnnotationTarget;
 import org.jboss.jandex.DotName;
@@ -26,6 +27,7 @@
 
 import io.quarkus.arc.deployment.AdditionalBeanBuildItem;
 import io.quarkus.arc.deployment.BeanDiscoveryFinishedBuildItem;
+import io.quarkus.arc.deployment.BeanRegistrationPhaseBuildItem;
 import io.quarkus.arc.deployment.QualifierRegistrarBuildItem;
 import io.quarkus.arc.deployment.SyntheticBeanBuildItem;
 import io.quarkus.arc.processor.InjectionPointInfo;
@@ -40,11 +42,14 @@
 import io.quarkus.deployment.annotations.Record;
 import io.quarkus.deployment.builditem.CombinedIndexBuildItem;
 import io.quarkus.deployment.builditem.ExtensionSslNativeSupportBuildItem;
+import io.quarkus.deployment.builditem.RunTimeConfigurationDefaultBuildItem;
 import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
+import io.quarkus.oidc.IdToken;
 import io.quarkus.oidc.Tenant;
 import io.quarkus.oidc.TenantFeature;
 import io.quarkus.oidc.TenantIdentityProvider;
 import io.quarkus.oidc.TokenIntrospectionCache;
+import io.quarkus.oidc.UserInfo;
 import io.quarkus.oidc.UserInfoCache;
 import io.quarkus.oidc.runtime.BackChannelLogoutHandler;
 import io.quarkus.oidc.runtime.DefaultTenantConfigResolver;
@@ -77,6 +82,9 @@ public class OidcBuildStep {
     private static final DotName TENANT_FEATURE_NAME = DotName.createSimple(TenantFeature.class);
     private static final DotName TENANT_IDENTITY_PROVIDER_NAME = DotName.createSimple(TenantIdentityProvider.class);
     private static final Logger LOG = Logger.getLogger(OidcBuildStep.class);
+    private static final DotName USER_INFO_NAME = DotName.createSimple(UserInfo.class);
+    private static final DotName JSON_WEB_TOKEN_NAME = DotName.createSimple(JsonWebToken.class);
+    private static final DotName ID_TOKEN_NAME = DotName.createSimple(IdToken.class);
 
     @BuildStep
     public void provideSecurityInformation(BuildProducer<SecurityInformationBuildItem> securityInformationProducer) {
@@ -289,6 +297,35 @@ public void produceTenantResolverInterceptors(CombinedIndexBuildItem indexBuildI
         }
     }
 
+    @BuildStep
+    void detectUserInfoRequired(BeanRegistrationPhaseBuildItem beanRegistrationPhaseBuildItem,
+            BuildProducer<RunTimeConfigurationDefaultBuildItem> runtimeConfigDefaultProducer) {
+        if (isInjected(beanRegistrationPhaseBuildItem, USER_INFO_NAME, null)) {
+            runtimeConfigDefaultProducer.produce(
+                    new RunTimeConfigurationDefaultBuildItem("quarkus.oidc.authentication.user-info-required", "true"));
+        }
+    }
+
+    @BuildStep
+    void detectAccessTokenVerificationRequired(BeanRegistrationPhaseBuildItem beanRegistrationPhaseBuildItem,
+            BuildProducer<RunTimeConfigurationDefaultBuildItem> runtimeConfigDefaultProducer) {
+        if (isInjected(beanRegistrationPhaseBuildItem, JSON_WEB_TOKEN_NAME, ID_TOKEN_NAME)) {
+            runtimeConfigDefaultProducer.produce(
+                    new RunTimeConfigurationDefaultBuildItem("quarkus.oidc.authentication.verify-access-token", "true"));
+        }
+    }
+
+    private static boolean isInjected(BeanRegistrationPhaseBuildItem beanRegistrationPhaseBuildItem, DotName requiredType,
+            DotName withoutQualifier) {
+        for (InjectionPointInfo injectionPoint : beanRegistrationPhaseBuildItem.getInjectionPoints()) {
+            if (requiredType.equals(injectionPoint.getRequiredType().name())
+                    && (withoutQualifier == null || injectionPoint.getRequiredQualifier(withoutQualifier) == null)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     private static String toTargetName(AnnotationTarget target) {
         if (target.kind() == CLASS) {
             return target.asClass().name().toString();

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
@@ -20,7 +20,10 @@
 
 import org.awaitility.core.ThrowingRunnable;
 import org.eclipse.microprofile.config.spi.ConfigSource;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Order;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
 import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
@@ -35,6 +38,7 @@
 import io.quarkus.test.common.QuarkusTestResource;
 import io.quarkus.test.keycloak.server.KeycloakTestResourceLifecycleManager;
 
+@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
 @QuarkusTestResource(KeycloakTestResourceLifecycleManager.class)
 public class CodeFlowDevModeTestCase {
 
@@ -54,6 +58,7 @@ public class CodeFlowDevModeTestCase {
                     .addAsResource("application-dev-mode.properties", "application.properties")
                     .addAsServiceProvider(ConfigSource.class, OidcConfigSource.class));
 
+    @Order(1)
     @Test
     public void testAccessAndRefreshTokenInjectionDevMode() throws IOException, InterruptedException {
         // Default tenant is disabled, check that having TenantConfigResolver is enough
@@ -123,6 +128,27 @@ public void testAccessAndRefreshTokenInjectionDevMode() throws IOException, Inte
 
     }
 
+    @Order(2)
+    @Test
+    public void testAccessTokenVerified() throws IOException {
+        try (final WebClient webClient = createWebClient()) {
+            test.modifyResourceFile("application.properties", s -> s.replace("tenant-enabled=false", "tenant-enabled=true")
+                    .replace("secret-from-vault-typo", "secret-from-vault"));
+            HtmlPage page = webClient.getPage("http://localhost:8080/protected/access-token-name");
+
+            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("alice", page.getBody().asNormalizedText());
+        }
+    }
+
     private void useTenantConfigResolver() throws IOException, InterruptedException {
         try (final WebClient webClient = createWebClient()) {
             HtmlPage page = webClient.getPage("http://localhost:8080/protected/tenant/tenant-config-resolver");

diff --git a/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/ProtectedResource.java b/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/ProtectedResource.java
@@ -8,6 +8,7 @@
 import org.eclipse.microprofile.jwt.JsonWebToken;
 
 import io.quarkus.oidc.IdToken;
+import io.quarkus.oidc.runtime.OidcConfig;
 import io.quarkus.security.Authenticated;
 
 @Path("/protected")
@@ -18,6 +19,12 @@ public class ProtectedResource {
     @IdToken
     JsonWebToken idToken;
 
+    @Inject
+    JsonWebToken accessToken;
+
+    @Inject
+    OidcConfig config;
+
     @GET
     public String getName() {
         return idToken.getName();
@@ -34,4 +41,13 @@ public String getTenantName(@PathParam("id") String tenantId) {
     public void logout() {
         throw new RuntimeException("Logout must be handled by CodeAuthenticationMechanism");
     }
+
+    @Path("access-token-name")
+    @GET
+    public String accessTokenName() {
+        if (!config.defaultTenant.authentication.verifyAccessToken) {
+            throw new IllegalStateException("Access token verification should be enabled");
+        }
+        return accessToken.getName();
+    }
 }
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
@@ -0,0 +1,98 @@
+package io.quarkus.oidc.test;
+
+import java.util.List;
+
+import jakarta.enterprise.event.Observes;
+import jakarta.inject.Inject;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+
+import org.hamcrest.Matchers;
+import org.jboss.shrinkwrap.api.asset.StringAsset;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.oidc.UserInfo;
+import io.quarkus.oidc.runtime.OidcConfig;
+import io.quarkus.security.PermissionsAllowed;
+import io.quarkus.test.QuarkusDevModeTest;
+import io.quarkus.test.common.QuarkusTestResource;
+import io.quarkus.test.keycloak.client.KeycloakTestClient;
+import io.quarkus.test.keycloak.server.KeycloakTestResourceLifecycleManager;
+import io.restassured.RestAssured;
+import io.vertx.ext.web.Router;
+
+@QuarkusTestResource(KeycloakTestResourceLifecycleManager.class)
+public class UserInfoRequiredDetectionTest {
+
+    @RegisterExtension
+    static final QuarkusDevModeTest test = new QuarkusDevModeTest()
+            .withApplicationRoot((jar) -> jar
+                    .addClasses(UserInfoResource.class, UserInfoEndpoint.class)
+                    .addAsResource(
+                            new StringAsset(
+                                    """
+                                            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.user-info-path=http://${quarkus.http.host}:${quarkus.http.port}/user-info-endpoint
+                                            quarkus.http.auth.proactive=false
+                                            """),
+                            "application.properties"));
+
+    @Test
+    public void testDefaultTenant() {
+        RestAssured.given().auth().oauth2(getAccessToken()).get("/user-info/default-tenant").then().statusCode(200)
+                .body(Matchers.is("alice"));
+    }
+
+    @Test
+    public void testNamedTenant() {
+        RestAssured.given().auth().oauth2(getAccessToken()).get("/user-info/named-tenant").then().statusCode(200)
+                .body(Matchers.is("alice"));
+    }
+
+    private static String getAccessToken() {
+        return new KeycloakTestClient().getAccessToken("alice", "alice", "quarkus-service-app", "secret", List.of("openid"));
+    }
+
+    public static class UserInfoEndpoint {
+        void observe(@Observes Router router) {
+            router.route("/user-info-endpoint").order(1).handler(rc -> rc.response().setStatusCode(200).end("{" +
+                    "   \"sub\": \"123456789\"," +
+                    "   \"preferred_username\": \"alice\"" +
+                    "  }"));
+        }
+    }
+
+    @Path("user-info")
+    public static class UserInfoResource {
+
+        @Inject
+        OidcConfig config;
+
+        @Inject
+        UserInfo userInfo;
+
+        @PermissionsAllowed("openid")
+        @Path("default-tenant")
+        @GET
+        public String getDefaultTenantName() {
+            if (!config.defaultTenant.authentication.userInfoRequired.orElse(false)) {
+                throw new IllegalStateException("Default tenant user info should be required");
+            }
+            return userInfo.getPreferredUserName();
+        }
+
+        @PermissionsAllowed("openid")
+        @Path("named-tenant")
+        @GET
+        public String getNamedTenantName() {
+            if (!config.namedTenants.get("named").authentication.userInfoRequired.orElse(false)) {
+                throw new IllegalStateException("Named tenant user info should be required");
+            }
+            return userInfo.getPreferredUserName();
+        }
+    }
+
+}
diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java
@@ -946,7 +946,7 @@ public enum ResponseMode {
          *
          * Bearer access tokens are always verified.
          */
-        @ConfigItem(defaultValue = "false")
+        @ConfigItem(defaultValueDocumentation = "true when access token is injected as the JsonWebToken bean, false otherwise")
         public boolean verifyAccessToken;
 
         /**
@@ -1087,7 +1087,7 @@ public enum ResponseMode {
          * or `quarkus.oidc.authentication.id-token-required` is set to `false`,
          * you do not need to enable this property manually in these cases.
          */
-        @ConfigItem(defaultValueDocumentation = "false")
+        @ConfigItem(defaultValueDocumentation = "true when UserInfo bean is injected, false otherwise")
         public Optional<Boolean> userInfoRequired = Optional.empty();
 
         /**

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,6 +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.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,6 +4,7 @@
 
 import java.util.Arrays;
 import java.util.HashSet;
+import java.util.Map;
 import java.util.Set;
 
 import org.hamcrest.Matchers;
@@ -13,13 +14,23 @@
 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() {