From 671ed8819883e360d69fdb7bfe45c1901fbb1fc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Vav=C5=99=C3=ADk?= <mvavrik@redhat.com> Date: Thu, 12 Dec 2024 14:10:30 +0100 Subject: [PATCH] fix(oidc,security): OIDC must auth before mTLS when disabled inclusive auth --- .../security-authentication-mechanisms.adoc | 6 +- extensions/oidc/deployment/pom.xml | 5 + .../OidcMtlsDisabledInclusiveAuthTest.java | 107 ++++++++++++++++++ .../security/InclusiveAuthValidationTest.java | 2 +- .../vertx/http/runtime/AuthConfig.java | 2 + .../runtime/security/HttpAuthenticator.java | 2 +- .../security/MtlsAuthenticationMechanism.java | 13 ++- 7 files changed, 132 insertions(+), 5 deletions(-) create mode 100644 extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/OidcMtlsDisabledInclusiveAuthTest.java diff --git a/docs/src/main/asciidoc/security-authentication-mechanisms.adoc b/docs/src/main/asciidoc/security-authentication-mechanisms.adoc index 95a32393ed97b..f5cc1a250b9e0 100644 --- a/docs/src/main/asciidoc/security-authentication-mechanisms.adoc +++ b/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, diff --git a/extensions/oidc/deployment/pom.xml b/extensions/oidc/deployment/pom.xml index 8691bf75f1911..8e8796c16474f 100644 --- a/extensions/oidc/deployment/pom.xml +++ b/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> diff --git a/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/OidcMtlsDisabledInclusiveAuthTest.java b/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/OidcMtlsDisabledInclusiveAuthTest.java new file mode 100644 index 0000000000000..94b913a6b3c5a --- /dev/null +++ b/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(); + } + } +} diff --git a/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/security/InclusiveAuthValidationTest.java b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/security/InclusiveAuthValidationTest.java index 83285ca98d6ef..186b7a4b6dfc3 100644 --- a/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/security/InclusiveAuthValidationTest.java +++ b/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; } } } diff --git a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/AuthConfig.java b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/AuthConfig.java index 08107d606038f..d251a34e732b7 100644 --- a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/AuthConfig.java +++ b/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> diff --git a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/HttpAuthenticator.java b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/HttpAuthenticator.java index 0dfbc8f392975..da171acff63df 100644 --- a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/HttpAuthenticator.java +++ b/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)); } } } diff --git a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/MtlsAuthenticationMechanism.java b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/MtlsAuthenticationMechanism.java index fa7d77c449dec..e6316f0e1bc0e 100644 --- a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/MtlsAuthenticationMechanism.java +++ b/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) {