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 f67d7f851f41d..0568683eda84a 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 @@ -44,6 +44,7 @@ import io.quarkus.runtime.TlsConfig; import io.quarkus.runtime.annotations.Recorder; import io.quarkus.runtime.configuration.ConfigurationException; +import io.quarkus.security.AuthenticationFailedException; import io.quarkus.security.identity.AuthenticationRequestContext; import io.quarkus.security.identity.SecurityIdentity; import io.quarkus.security.identity.request.TokenAuthenticationRequest; @@ -600,6 +601,30 @@ public Consumer apply(String tenantId) { return new Consumer() { @Override public void accept(RoutingContext routingContext) { + OidcTenantConfig tenantConfig = routingContext.get(OidcTenantConfig.class.getName()); + if (tenantConfig != null) { + // authentication has happened before @Tenant annotation was matched with the HTTP request + String tenantUsedForAuth = tenantConfig.tenantId.orElse(null); + if (tenantId.equals(tenantUsedForAuth)) { + // @Tenant selects the same tenant as already selected + return; + } else { + // @Tenant selects the different tenant than already selected + throw new AuthenticationFailedException( + """ + The '%1$s' selected with the @Tenant annotation must be used to authenticate + the request but it was already authenticated with the '%2$s' tenant. It + can happen if the '%1$s' is selected with an annotation but '%2$s' is + resolved during authentication required by the HTTP Security Policy which + is enforced before the JAX-RS chain is run. In such cases, please set the + 'quarkus.http.auth.permission."permissions".applies-to=JAXRS' to all HTTP + Security Policies which secure the same REST endpoints as the ones + where the '%1$s' tenant is resolved by the '@Tenant' annotation. + """ + .formatted(tenantId, tenantUsedForAuth)); + } + } + LOG.debugf("@Tenant annotation set a '%s' tenant id on the %s request path", tenantId, routingContext.request().path()); routingContext.put(OidcUtils.TENANT_ID_SET_BY_ANNOTATION, tenantId); diff --git a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/JakartaRestResourceHttpPermissionTest.java b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/JakartaRestResourceHttpPermissionTest.java index a8324f72cfc2c..0eb468a1bcdc0 100644 --- a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/JakartaRestResourceHttpPermissionTest.java +++ b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/JakartaRestResourceHttpPermissionTest.java @@ -12,10 +12,12 @@ import org.jboss.shrinkwrap.api.asset.StringAsset; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import io.quarkus.security.identity.SecurityIdentity; import io.quarkus.security.test.utils.TestIdentityController; import io.quarkus.security.test.utils.TestIdentityProvider; import io.quarkus.test.QuarkusUnitTest; @@ -40,18 +42,23 @@ public class JakartaRestResourceHttpPermissionTest { "quarkus.http.auth.permission.root.paths=/\n" + "quarkus.http.auth.permission.root.policy=authenticated\n" + "quarkus.http.auth.permission.dot.paths=dot,dot/\n" + - "quarkus.http.auth.permission.dot.policy=authenticated\n"; + "quarkus.http.auth.permission.dot.policy=authenticated\n" + + "quarkus.http.auth.permission.jax-rs.paths=jax-rs\n" + + "quarkus.http.auth.permission.jax-rs.policy=admin-role\n" + + "quarkus.http.auth.policy.admin-role.roles-allowed=admin"; @RegisterExtension static QuarkusUnitTest runner = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar .addClasses(TestIdentityProvider.class, TestIdentityController.class, ApiResource.class, - RootResource.class, PublicResource.class) + RootResource.class, PublicResource.class, JaxRsResource.class) .addAsResource(new StringAsset(APP_PROPS), "application.properties")); @BeforeAll public static void setup() { - TestIdentityController.resetRoles().add("test", "test", "test"); + TestIdentityController.resetRoles() + .add("admin", "admin", "admin") + .add("test", "test", "test"); } @TestHTTPResource @@ -97,6 +104,15 @@ public void testSecuredNotFound(String path) { assurePathAuthenticated(path, 404); } + @Test + public void testJaxRsRolesHttpSecurityPolicy() { + // insufficient role, expected admin + assurePath("/jax-rs", 401); + assurePath("///jax-rs///", 401); + + assurePath("/jax-rs", 200, "admin", true, "admin:admin"); + } + private static String getLastNonEmptySegmentContent(String path) { while (path.endsWith("/") || path.endsWith(".")) { path = path.substring(0, path.length() - 1); @@ -104,6 +120,18 @@ private static String getLastNonEmptySegmentContent(String path) { return path.substring(path.lastIndexOf('/') + 1); } + @Path("jax-rs") + public static class JaxRsResource { + + @Inject + SecurityIdentity identity; + + @GET + public String getPrincipalName() { + return identity.getPrincipal().getName(); + } + } + @Path("/api") public static class ApiResource { @@ -201,13 +229,17 @@ private void assurePathAuthenticated(String path, String body) { } private void assurePath(String path, int expectedStatusCode, String body, boolean auth) { + assurePath(path, expectedStatusCode, body, auth, "test:test"); + } + + private void assurePath(String path, int expectedStatusCode, String body, boolean auth, String credentials) { var httpClient = vertx.createHttpClient(); try { httpClient .request(HttpMethod.GET, url.getPort(), url.getHost(), path) .map(r -> { if (auth) { - r.putHeader("Authorization", "Basic " + encodeBase64URLSafeString("test:test".getBytes())); + r.putHeader("Authorization", "Basic " + encodeBase64URLSafeString(credentials.getBytes())); } return r; }) diff --git a/extensions/resteasy-reactive/rest/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/JakartaRestResourceHttpPermissionTest.java b/extensions/resteasy-reactive/rest/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/JakartaRestResourceHttpPermissionTest.java index 220102abd6348..e6ff7c5e18e7b 100644 --- a/extensions/resteasy-reactive/rest/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/JakartaRestResourceHttpPermissionTest.java +++ b/extensions/resteasy-reactive/rest/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/JakartaRestResourceHttpPermissionTest.java @@ -14,10 +14,12 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import io.quarkus.security.identity.SecurityIdentity; import io.quarkus.security.test.utils.TestIdentityController; import io.quarkus.security.test.utils.TestIdentityProvider; import io.quarkus.test.QuarkusUnitTest; @@ -40,19 +42,24 @@ public class JakartaRestResourceHttpPermissionTest { "quarkus.http.auth.permission.root.paths=/\n" + "quarkus.http.auth.permission.root.policy=authenticated\n" + "quarkus.http.auth.permission.fragment.paths=/#stuff,/#stuff/\n" + - "quarkus.http.auth.permission.fragment.policy=authenticated\n"; + "quarkus.http.auth.permission.fragment.policy=authenticated\n" + + "quarkus.http.auth.permission.jax-rs.paths=jax-rs\n" + + "quarkus.http.auth.permission.jax-rs.policy=admin-role\n" + + "quarkus.http.auth.policy.admin-role.roles-allowed=admin"; private static WebClient client; @RegisterExtension static QuarkusUnitTest runner = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar .addClasses(TestIdentityProvider.class, TestIdentityController.class, ApiResource.class, - RootResource.class, PublicResource.class) + RootResource.class, PublicResource.class, JaxRsResource.class) .addAsResource(new StringAsset(APP_PROPS), "application.properties")); @BeforeAll public static void setup() { - TestIdentityController.resetRoles().add("test", "test", "test"); + TestIdentityController.resetRoles() + .add("admin", "admin", "admin") + .add("test", "test", "test"); } @AfterAll @@ -106,6 +113,15 @@ public void testNotSecuredPaths(String path) { assurePathAuthenticated(path); } + @Test + public void testJaxRsRolesHttpSecurityPolicy() { + // insufficient role, expected admin + assurePath("/jax-rs", 401); + assurePath("///jax-rs///", 401); + + assurePath("/jax-rs", 200, "admin", true, "admin"); + } + private static String getLastNonEmptySegmentContent(String path) { while (path.endsWith("/") || path.endsWith(".")) { path = path.substring(0, path.length() - 1); @@ -113,6 +129,18 @@ private static String getLastNonEmptySegmentContent(String path) { return path.substring(path.lastIndexOf('/') + 1); } + @Path("jax-rs") + public static class JaxRsResource { + + @Inject + SecurityIdentity identity; + + @GET + public String getPrincipalName() { + return identity.getPrincipal().getName(); + } + } + @Path("/api") public static class ApiResource { @@ -212,9 +240,13 @@ private void assurePathAuthenticated(String path, String body) { } private void assurePath(String path, int expectedStatusCode, String body, boolean auth) { + assurePath(path, expectedStatusCode, body, auth, "test"); + } + + private void assurePath(String path, int expectedStatusCode, String body, boolean auth, String username) { var req = getClient().get(url.getPort(), url.getHost(), path); if (auth) { - req.basicAuthentication("test", "test"); + req.basicAuthentication(username, username); } var result = req.send(); await().atMost(REQUEST_TIMEOUT).until(result::isComplete); diff --git a/integration-tests/oidc-wiremock/src/main/java/io/quarkus/it/keycloak/TenantEchoResource.java b/integration-tests/oidc-wiremock/src/main/java/io/quarkus/it/keycloak/TenantEchoResource.java index 5636556933e8c..4440ddf1a8e4e 100644 --- a/integration-tests/oidc-wiremock/src/main/java/io/quarkus/it/keycloak/TenantEchoResource.java +++ b/integration-tests/oidc-wiremock/src/main/java/io/quarkus/it/keycloak/TenantEchoResource.java @@ -54,6 +54,19 @@ public String getHrTenantIdentityAugmentation() { return getTenantInternal(); } + @Path("/http-security-policy-applies-all-diff") + @GET + public String httpSecurityPolicyAppliesAllDiff() { + throw new IllegalStateException("An exception should have been thrown because authentication happened" + + " before Tenant was selected with the @Tenant annotation"); + } + + @Path("/http-security-policy-applies-all-same") + @GET + public String httpSecurityPolicyAppliesAllSame() { + return getTenantInternal(); + } + private String getTenantInternal() { return OidcUtils.TENANT_ID_ATTRIBUTE + "=" + routingContext.get(OidcUtils.TENANT_ID_ATTRIBUTE) + ", static.tenant.id=" + routingContext.get("static.tenant.id") diff --git a/integration-tests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/AnnotationBasedTenantTest.java b/integration-tests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/AnnotationBasedTenantTest.java index a072936710d3e..eac6beda6d99e 100644 --- a/integration-tests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/AnnotationBasedTenantTest.java +++ b/integration-tests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/AnnotationBasedTenantTest.java @@ -28,6 +28,7 @@ public Map getConfigOverrides() { 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"), + Map.entry("quarkus.oidc.hr.tenant-paths", "/api/tenant-echo/http-security-policy-applies-all-same"), Map.entry("quarkus.oidc.hr.token.audience", "http://hr.service"), Map.entry("quarkus.http.auth.policy.roles1.roles-allowed", "role1"), Map.entry("quarkus.http.auth.policy.roles2.roles-allowed", "role2"), @@ -59,7 +60,11 @@ public Map getConfigOverrides() { Map.entry("quarkus.http.auth.permission.identity-augmentation.paths", "/api/tenant-echo/hr-identity-augmentation"), Map.entry("quarkus.http.auth.permission.identity-augmentation.policy", "roles3"), - Map.entry("quarkus.http.auth.permission.identity-augmentation.applies-to", "JAXRS")); + Map.entry("quarkus.http.auth.permission.identity-augmentation.applies-to", "JAXRS"), + Map.entry("quarkus.http.auth.permission.tenant-annotation-applies-all.paths", + "/api/tenant-echo/http-security-policy-applies-all-diff,/api/tenant-echo/http-security-policy-applies-all-same"), + Map.entry("quarkus.http.auth.permission.tenant-annotation-applies-all.policy", "admin-role"), + Map.entry("quarkus.http.auth.policy.admin-role.roles-allowed", "admin")); } } @@ -204,9 +209,7 @@ public void testClassicHttpSecurityPolicyWithRbac() { token = getTokenWithRole("role1"); RestAssured.given().auth().oauth2(token) .when().get("/api/tenant-echo2/hr-classic-perm-check") - .then().statusCode(200) - .body(Matchers.equalTo(("tenant-id=hr, static.tenant.id=null, name=alice, " - + OidcUtils.TENANT_ID_SET_BY_ANNOTATION + "=hr"))); + .then().statusCode(401); token = getTokenWithRole("wrong-role"); RestAssured.given().auth().oauth2(token) @@ -239,21 +242,18 @@ public void testJaxRsAndClassicHttpSecurityPolicyNoRbac() { token = getTokenWithRole("role2"); RestAssured.given().auth().oauth2(token) .when().get("/api/tenant-echo/hr-classic-and-jaxrs-perm-check") - .then().statusCode(403); + .then().statusCode(401); // roles allowed security check (created for @RolesAllowed) fails over missing role "role3" token = getTokenWithRole("role2", "role1"); RestAssured.given().auth().oauth2(token) .when().get("/api/tenant-echo/hr-classic-and-jaxrs-perm-check") - .then().statusCode(403); + .then().statusCode(401); token = getTokenWithRole("role3", "role2", "role1"); RestAssured.given().auth().oauth2(token) .when().get("/api/tenant-echo/hr-classic-and-jaxrs-perm-check") - .then().statusCode(200) - // static tenant is null as the permission check "combined-part1" happened before @Tenant - .body(Matchers.equalTo(("tenant-id=hr, static.tenant.id=null, name=alice, " - + OidcUtils.TENANT_ID_SET_BY_ANNOTATION + "=hr"))); + .then().statusCode(401); } finally { server.stop(); } @@ -282,15 +282,12 @@ public void testJaxRsAndClassicHttpSecurityPolicyWithRbac() { token = getTokenWithRole("role2"); RestAssured.given().auth().oauth2(token) .when().get("/api/tenant-echo2/hr-classic-and-jaxrs-perm-check") - .then().statusCode(403); + .then().statusCode(401); token = getTokenWithRole("role2", "role1"); RestAssured.given().auth().oauth2(token) .when().get("/api/tenant-echo2/hr-classic-and-jaxrs-perm-check") - .then().statusCode(200) - // static tenant is null as the permission check "combined-part1" happened before @Tenant - .body(Matchers.equalTo(("tenant-id=hr, static.tenant.id=null, name=alice, " - + OidcUtils.TENANT_ID_SET_BY_ANNOTATION + "=hr"))); + .then().statusCode(401); } finally { server.stop(); } @@ -325,6 +322,31 @@ public void testJaxRsIdentityAugmentation() { } } + @Test + public void testPolicyAppliedBeforeTenantAnnotationMatched() { + WiremockTestResource server = new WiremockTestResource(); + server.start(); + try { + // policy applied before @Tenant annotation has been matched and different tenant has been used for auth + // than the one that @Tenant annotation selects + var token = getNonHrTenantAccessToken(Set.of("admin")); + RestAssured.given().auth().oauth2(token) + .when().get("/api/tenant-echo/http-security-policy-applies-all-diff") + .then().statusCode(401); + + // policy applied before @Tenant annotation has been matched and different tenant has been used for auth + // than the one that @Tenant annotation selects + token = getTokenWithRole("admin"); + RestAssured.given().auth().oauth2(token) + .when().get("/api/tenant-echo/http-security-policy-applies-all-same") + .then().statusCode(200) + .body(Matchers + .equalTo("tenant-id=null, static.tenant.id=hr, name=alice, tenant-id-set-by-annotation=null")); + } finally { + server.stop(); + } + } + private static String getTokenWithRole(String... roles) { return Jwt.preferredUserName("alice") .groups(Set.of(roles)) @@ -333,4 +355,14 @@ private static String getTokenWithRole(String... roles) { .keyId("1") .sign("privateKey.jwk"); } + + private String getNonHrTenantAccessToken(Set groups) { + return Jwt.preferredUserName("alice") + .groups(groups) + .issuer("https://server.example.com") + .audience("https://service.example.com") + .jws() + .keyId("1") + .sign(); + } }