Merge pull request quarkusio#45072 from michalvavrik/feature/disable-…

…inclusive-auh-refactoring

Prioritize OIDC mechanism when inclusive authentication is disabled to simplify using mTLS and OIDC together

docs/src/main/asciidoc/security-authentication-mechanisms.adoc

@@ -602,7 +602,11 @@ quarkus.http.auth.inclusive=true
 If the authentication is inclusive then `SecurityIdentity` created by the first authentication mechanism can be
 injected into the application code.
 For example, if both <<mutual-tls>> and basic authentication mechanism authentications are required,
-the <<mutual-tls>> authentication mechanism will create `SecurityIdentity` first.
+the <<mutual-tls>> mechanism will create `SecurityIdentity` first.
+
+NOTE: The <<mutual-tls>> mechanism has the highest priority when inclusive authentication is enabled, to ensure
+that an injected `SecurityIdentity` always represents <<mutual-tls>> and can be used to get access to `SecurityIdentity`
+identities provided by other authentication mechanisms.
 
 Additional `SecurityIdentity` instances can be accessed as a `quarkus.security.identities` attribute on the first
 `SecurityIdentity`, however, accessing these extra identities directly may not be necessary, for example,

extensions/oidc/deployment/pom.xml

@@ -93,6 +93,11 @@
             <artifactId>quarkus-elytron-security-properties-file-deployment</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>io.smallrye.certs</groupId>
+            <artifactId>smallrye-certificate-generator-junit5</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/OidcMtlsDisabledInclusiveAuthTest.java

@@ -0,0 +1,107 @@
+package io.quarkus.oidc.test;
+
+import static org.hamcrest.Matchers.is;
+
+import java.io.File;
+
+import jakarta.inject.Inject;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+
+import org.jboss.shrinkwrap.api.asset.StringAsset;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.oidc.BearerTokenAuthentication;
+import io.quarkus.security.Authenticated;
+import io.quarkus.security.identity.SecurityIdentity;
+import io.quarkus.test.QuarkusDevModeTest;
+import io.quarkus.test.common.QuarkusTestResource;
+import io.quarkus.test.keycloak.server.KeycloakTestResourceLifecycleManager;
+import io.quarkus.vertx.http.runtime.security.annotation.MTLSAuthentication;
+import io.restassured.RestAssured;
+import io.restassured.specification.RequestSpecification;
+import io.smallrye.certs.Format;
+import io.smallrye.certs.junit5.Certificate;
+import io.smallrye.certs.junit5.Certificates;
+
+/**
+ * This test ensures OIDC runs before mTLS authentication mechanism when inclusive authentication is not enabled.
+ */
+@QuarkusTestResource(KeycloakTestResourceLifecycleManager.class)
+@Certificates(baseDir = "target/certs", certificates = @Certificate(name = "mtls-test", password = "secret", formats = {
+        Format.PKCS12, Format.PEM }, client = true))
+public class OidcMtlsDisabledInclusiveAuthTest {
+
+    private static final String BASE_URL = "https://localhost:8443/mtls-bearer/";
+    private static final String CONFIGURATION = """
+            quarkus.tls.key-store.pem.0.cert=server.crt
+            quarkus.tls.key-store.pem.0.key=server.key
+            quarkus.tls.trust-store.pem.certs=ca.crt
+            quarkus.http.ssl.client-auth=REQUIRED
+            quarkus.http.insecure-requests=disabled
+            quarkus.oidc.auth-server-url=${keycloak.url}/realms/quarkus
+            quarkus.oidc.client-id=quarkus-service-app
+            quarkus.oidc.credentials.secret=secret
+            quarkus.http.auth.proactive=false
+            """;
+
+    @RegisterExtension
+    static final QuarkusDevModeTest config = new QuarkusDevModeTest()
+            .withApplicationRoot((jar) -> jar
+                    .addClasses(MtlsBearerResource.class)
+                    .addAsResource(new StringAsset(CONFIGURATION), "application.properties")
+                    .addAsResource(new File("target/certs/mtls-test.key"), "server.key")
+                    .addAsResource(new File("target/certs/mtls-test.crt"), "server.crt")
+                    .addAsResource(new File("target/certs/mtls-test-server-ca.crt"), "ca.crt"));
+
+    @Test
+    public void testOidcHasHighestPriority() {
+        givenWithCerts().get(BASE_URL + "only-mtls").then().statusCode(200).body(is("CN=localhost"));
+        givenWithCerts().auth().oauth2(getAccessToken()).get(BASE_URL + "only-bearer").then().statusCode(200).body(is("alice"));
+        // this needs to be OIDC because when inclusive auth is disabled, OIDC has higher priority
+        givenWithCerts().auth().oauth2(getAccessToken()).get(BASE_URL + "both").then().statusCode(200).body(is("alice"));
+        // OIDC must run first and thus authentication fails over invalid credentials
+        givenWithCerts().auth().oauth2("invalid-token").get(BASE_URL + "both").then().statusCode(401);
+        // mTLS authentication mechanism still runs when OIDC doesn't produce the identity
+        givenWithCerts().get(BASE_URL + "both").then().statusCode(200).body(is("CN=localhost"));
+    }
+
+    private static RequestSpecification givenWithCerts() {
+        return RestAssured.given()
+                .keyStore("target/certs/mtls-test-client-keystore.p12", "secret")
+                .trustStore("target/certs/mtls-test-client-truststore.p12", "secret");
+    }
+
+    private static String getAccessToken() {
+        return KeycloakTestResourceLifecycleManager.getAccessToken("alice");
+    }
+
+    @Path("mtls-bearer")
+    public static class MtlsBearerResource {
+
+        @Inject
+        SecurityIdentity securityIdentity;
+
+        @GET
+        @Authenticated
+        @Path("both")
+        public String both() {
+            return securityIdentity.getPrincipal().getName();
+        }
+
+        @GET
+        @MTLSAuthentication
+        @Path("only-mtls")
+        public String onlyMTLS() {
+            return securityIdentity.getPrincipal().getName();
+        }
+
+        @GET
+        @BearerTokenAuthentication
+        @Path("only-bearer")
+        public String onlyBearer() {
+            return securityIdentity.getPrincipal().getName();
+        }
+    }
+}

extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/security/InclusiveAuthValidationTest.java

@@ -89,7 +89,7 @@ public Set<Class<? extends AuthenticationRequest>> getCredentialTypes() {
 
         @Override
         public int getPriority() {
-            return MtlsAuthenticationMechanism.PRIORITY + 1;
+            return MtlsAuthenticationMechanism.INCLUSIVE_AUTHENTICATION_PRIORITY + 1;
         }
     }
 }

extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/AuthConfig.java

@@ -43,6 +43,8 @@ public class AuthConfig {
      * authentication, for example, OIDC bearer token authentication, must succeed.
      * In such cases, `SecurityIdentity` created by the first mechanism, mTLS, can be injected, identities created
      * by other mechanisms will be available on `SecurityIdentity`.
+     * The mTLS mechanism is always the first mechanism, because its priority is elevated when inclusive authentication
+     * is enabled.
      * The identities can be retrieved using utility method as in the example below:
      *
      * <pre>

extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/HttpAuthenticator.java

@@ -160,7 +160,7 @@ public int compare(HttpAuthenticationMechanism mech1, HttpAuthenticationMechanis
                                     the highest priority. Please lower priority of the '%s' authentication mechanism under '%s'.
                                     """.formatted(MtlsAuthenticationMechanism.class.getName(),
                                     topMechanism.getClass().getName(),
-                                    MtlsAuthenticationMechanism.PRIORITY));
+                                    MtlsAuthenticationMechanism.INCLUSIVE_AUTHENTICATION_PRIORITY));
                 }
             }
         }

extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/MtlsAuthenticationMechanism.java

@@ -17,6 +17,8 @@
  */
 package io.quarkus.vertx.http.runtime.security;
 
+import static io.quarkus.vertx.http.runtime.security.HttpAuthenticationMechanism.DEFAULT_PRIORITY;
+
 import java.security.cert.Certificate;
 import java.security.cert.X509Certificate;
 import java.util.Collections;
@@ -25,6 +27,8 @@
 
 import javax.net.ssl.SSLPeerUnverifiedException;
 
+import org.eclipse.microprofile.config.inject.ConfigProperty;
+
 import io.netty.handler.codec.http.HttpResponseStatus;
 import io.quarkus.security.credential.CertificateCredential;
 import io.quarkus.security.identity.IdentityProviderManager;
@@ -39,10 +43,15 @@
  * The authentication handler responsible for mTLS client authentication
  */
 public class MtlsAuthenticationMechanism implements HttpAuthenticationMechanism {
-    public static final int PRIORITY = 3000;
+    public static final int INCLUSIVE_AUTHENTICATION_PRIORITY = 3000;
     private static final String ROLES_MAPPER_ATTRIBUTE = "roles_mapper";
+    private final boolean inclusiveAuthentication;
     private Function<X509Certificate, Set<String>> certificateToRoles = null;
 
+    MtlsAuthenticationMechanism(@ConfigProperty(name = "quarkus.http.auth.inclusive") boolean inclusiveAuthentication) {
+        this.inclusiveAuthentication = inclusiveAuthentication;
+    }
+
     @Override
     public Uni<SecurityIdentity> authenticate(RoutingContext context,
             IdentityProviderManager identityProviderManager) {
@@ -86,7 +95,7 @@ public Uni<HttpCredentialTransport> getCredentialTransport(RoutingContext contex
 
     @Override
     public int getPriority() {
-        return PRIORITY;
+        return inclusiveAuthentication ? INCLUSIVE_AUTHENTICATION_PRIORITY : DEFAULT_PRIORITY;
     }
 
     void setCertificateToRolesMapper(Function<X509Certificate, Set<String>> certificateToRoles) {