From 6cd1598c3bd014e8c1492537b81d97ae114a9943 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Vav=C5=99=C3=ADk?= Date: Tue, 23 Jan 2024 22:22:04 +0100 Subject: [PATCH 1/3] Move RESTEasy Classic RBAC security checks to JAX-RS filter (cherry picked from commit 4a82745c000813b4153a1775329776672af95cfa) --- .../deployment/ResteasyBuiltinsProcessor.java | 20 +- .../test/security/EagerSecurityCheckTest.java | 176 ++++++++++++++++++ .../resteasy/runtime/EagerSecurityFilter.java | 117 ++++++++++-- .../StandardSecurityCheckInterceptor.java | 85 +++++++++ 4 files changed, 376 insertions(+), 22 deletions(-) create mode 100644 extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/EagerSecurityCheckTest.java create mode 100644 extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/StandardSecurityCheckInterceptor.java diff --git a/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyBuiltinsProcessor.java b/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyBuiltinsProcessor.java index df5f2f1a1ff3e..2001747fbb72b 100644 --- a/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyBuiltinsProcessor.java +++ b/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyBuiltinsProcessor.java @@ -7,7 +7,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; -import java.util.Optional; import java.util.stream.Collectors; import org.jboss.jandex.ClassInfo; @@ -34,6 +33,7 @@ import io.quarkus.resteasy.runtime.JaxRsSecurityConfig; import io.quarkus.resteasy.runtime.NotFoundExceptionMapper; import io.quarkus.resteasy.runtime.SecurityContextFilter; +import io.quarkus.resteasy.runtime.StandardSecurityCheckInterceptor; import io.quarkus.resteasy.runtime.UnauthorizedExceptionMapper; import io.quarkus.resteasy.runtime.vertx.JsonArrayReader; import io.quarkus.resteasy.runtime.vertx.JsonArrayWriter; @@ -41,7 +41,6 @@ import io.quarkus.resteasy.runtime.vertx.JsonObjectWriter; import io.quarkus.resteasy.server.common.deployment.ResteasyDeploymentBuildItem; import io.quarkus.security.spi.AdditionalSecuredMethodsBuildItem; -import io.quarkus.vertx.http.deployment.EagerSecurityInterceptorBuildItem; import io.quarkus.vertx.http.deployment.HttpRootPathBuildItem; import io.quarkus.vertx.http.deployment.devmode.NotFoundPageDisplayableEndpointBuildItem; import io.quarkus.vertx.http.deployment.devmode.RouteDescriptionBuildItem; @@ -91,8 +90,7 @@ void setUpDenyAllJaxRs(CombinedIndexBuildItem index, */ @BuildStep void setUpSecurity(BuildProducer providers, - BuildProducer additionalBeanBuildItem, Capabilities capabilities, - Optional eagerSecurityInterceptors) { + BuildProducer additionalBeanBuildItem, Capabilities capabilities) { providers.produce(new ResteasyJaxrsProviderBuildItem(UnauthorizedExceptionMapper.class.getName())); providers.produce(new ResteasyJaxrsProviderBuildItem(ForbiddenExceptionMapper.class.getName())); providers.produce(new ResteasyJaxrsProviderBuildItem(AuthenticationFailedExceptionMapper.class.getName())); @@ -102,10 +100,16 @@ void setUpSecurity(BuildProducer providers, if (capabilities.isPresent(Capability.SECURITY)) { providers.produce(new ResteasyJaxrsProviderBuildItem(SecurityContextFilter.class.getName())); additionalBeanBuildItem.produce(AdditionalBeanBuildItem.unremovableOf(SecurityContextFilter.class)); - if (eagerSecurityInterceptors.isPresent()) { - providers.produce(new ResteasyJaxrsProviderBuildItem(EagerSecurityFilter.class.getName())); - additionalBeanBuildItem.produce(AdditionalBeanBuildItem.unremovableOf(EagerSecurityFilter.class)); - } + providers.produce(new ResteasyJaxrsProviderBuildItem(EagerSecurityFilter.class.getName())); + additionalBeanBuildItem.produce(AdditionalBeanBuildItem.unremovableOf(EagerSecurityFilter.class)); + additionalBeanBuildItem.produce( + AdditionalBeanBuildItem.unremovableOf(StandardSecurityCheckInterceptor.RolesAllowedInterceptor.class)); + additionalBeanBuildItem.produce(AdditionalBeanBuildItem + .unremovableOf(StandardSecurityCheckInterceptor.PermissionsAllowedInterceptor.class)); + additionalBeanBuildItem.produce( + AdditionalBeanBuildItem.unremovableOf(StandardSecurityCheckInterceptor.PermitAllInterceptor.class)); + additionalBeanBuildItem.produce( + AdditionalBeanBuildItem.unremovableOf(StandardSecurityCheckInterceptor.AuthenticatedInterceptor.class)); } } diff --git a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/EagerSecurityCheckTest.java b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/EagerSecurityCheckTest.java new file mode 100644 index 0000000000000..d71665b4f1292 --- /dev/null +++ b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/EagerSecurityCheckTest.java @@ -0,0 +1,176 @@ +package io.quarkus.resteasy.test.security; + +import jakarta.annotation.security.DenyAll; +import jakarta.annotation.security.PermitAll; +import jakarta.annotation.security.RolesAllowed; +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.MediaType; + +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.security.Authenticated; +import io.quarkus.security.test.utils.TestIdentityController; +import io.quarkus.security.test.utils.TestIdentityProvider; +import io.quarkus.test.QuarkusUnitTest; +import io.restassured.RestAssured; +import io.restassured.http.ContentType; +import io.restassured.response.Response; +import io.vertx.core.json.JsonObject; + +/** + * Tests that {@link io.quarkus.security.spi.runtime.SecurityCheck}s are executed by Jakarta REST filters. + */ +public class EagerSecurityCheckTest { + + @RegisterExtension + static QuarkusUnitTest runner = new QuarkusUnitTest() + .withApplicationRoot((jar) -> jar + .addClasses(TestIdentityProvider.class, TestIdentityController.class, JsonResource.class, + AbstractJsonResource.class, JsonSubResource.class)); + + @BeforeAll + public static void setupUsers() { + TestIdentityController.resetRoles() + .add("admin", "admin", "admin") + .add("user", "user", "user"); + } + + @Test + public void testAuthenticated() { + testPostJson("auth", "admin", true).then().statusCode(400); + testPostJson("auth", null, true).then().statusCode(401); + testPostJson("auth", "admin", false).then().statusCode(200); + testPostJson("auth", null, false).then().statusCode(401); + } + + @Test + public void testRolesAllowed() { + testPostJson("roles", "admin", true).then().statusCode(400); + testPostJson("roles", "user", true).then().statusCode(403); + testPostJson("roles", "admin", false).then().statusCode(200); + testPostJson("roles", "user", false).then().statusCode(403); + } + + @Test + public void testRolesAllowedOverriddenMethod() { + testPostJson("/roles-overridden", "admin", true).then().statusCode(400); + testPostJson("/roles-overridden", "user", true).then().statusCode(403); + testPostJson("/roles-overridden", "admin", false).then().statusCode(200); + testPostJson("/roles-overridden", "user", false).then().statusCode(403); + } + + @Test + public void testDenyAll() { + testPostJson("deny", "admin", true).then().statusCode(403); + testPostJson("deny", null, true).then().statusCode(401); + testPostJson("deny", "admin", false).then().statusCode(403); + testPostJson("deny", null, false).then().statusCode(401); + } + + @Test + public void testDenyAllClassLevel() { + testPostJson("/sub-resource/deny-class-level-annotation", "admin", true).then().statusCode(403); + testPostJson("/sub-resource/deny-class-level-annotation", null, true).then().statusCode(401); + testPostJson("/sub-resource/deny-class-level-annotation", "admin", false).then().statusCode(403); + testPostJson("/sub-resource/deny-class-level-annotation", null, false).then().statusCode(401); + } + + @Test + public void testPermitAll() { + testPostJson("permit", "admin", true).then().statusCode(400); + testPostJson("permit", null, true).then().statusCode(400); + testPostJson("permit", "admin", false).then().statusCode(200); + testPostJson("permit", null, false).then().statusCode(200); + } + + @Test + public void testSubResource() { + testPostJson("/sub-resource/roles", "admin", true).then().statusCode(400); + testPostJson("/sub-resource/roles", "user", true).then().statusCode(403); + testPostJson("/sub-resource/roles", "admin", false).then().statusCode(200); + testPostJson("/sub-resource/roles", "user", false).then().statusCode(403); + } + + private static Response testPostJson(String path, String username, boolean invalid) { + var req = RestAssured.given(); + if (username != null) { + req = req.auth().preemptive().basic(username, username); + } + return req + .contentType(ContentType.JSON) + .body((invalid ? "}" : "") + "{\"simple\": \"obj\"}").post(path); + } + + @Path("/") + @Produces(MediaType.APPLICATION_JSON) + @Consumes(MediaType.APPLICATION_JSON) + public static class JsonResource extends AbstractJsonResource { + + @Authenticated + @Path("/auth") + @POST + public JsonObject auth(JsonObject array) { + return array.put("test", "testval"); + } + + @RolesAllowed("admin") + @Path("/roles") + @POST + public JsonObject roles(JsonObject array) { + return array.put("test", "testval"); + } + + @PermitAll + @Path("/permit") + @POST + public JsonObject permit(JsonObject array) { + return array.put("test", "testval"); + } + + @PermitAll + @Path("/sub-resource") + public JsonSubResource subResource() { + return new JsonSubResource(); + } + + @RolesAllowed("admin") + @Override + public JsonObject rolesOverridden(JsonObject array) { + return array.put("test", "testval"); + } + } + + @DenyAll + public static class JsonSubResource { + @RolesAllowed("admin") + @Path("/roles") + @POST + public JsonObject roles(JsonObject array) { + return array.put("test", "testval"); + } + + @Path("/deny-class-level-annotation") + @POST + public JsonObject denyClassLevelAnnotation(JsonObject array) { + return array.put("test", "testval"); + } + } + + public static abstract class AbstractJsonResource { + @DenyAll + @Path("/deny") + @POST + public JsonObject deny(JsonObject array) { + return array.put("test", "testval"); + } + + @Path("/roles-overridden") + @POST + public abstract JsonObject rolesOverridden(JsonObject array); + } +} diff --git a/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/EagerSecurityFilter.java b/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/EagerSecurityFilter.java index 81a138aaa974e..e05df7c8dc751 100644 --- a/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/EagerSecurityFilter.java +++ b/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/EagerSecurityFilter.java @@ -1,11 +1,15 @@ package io.quarkus.resteasy.runtime; +import static io.quarkus.security.spi.runtime.SecurityEventHelper.AUTHORIZATION_FAILURE; +import static io.quarkus.security.spi.runtime.SecurityEventHelper.AUTHORIZATION_SUCCESS; + import java.io.IOException; import java.util.HashMap; import java.util.Map; import java.util.function.Consumer; import jakarta.annotation.Priority; +import jakarta.enterprise.event.Event; import jakarta.inject.Inject; import jakarta.ws.rs.Priorities; import jakarta.ws.rs.container.ContainerRequestContext; @@ -14,7 +18,19 @@ import jakarta.ws.rs.core.Context; import jakarta.ws.rs.ext.Provider; +import org.eclipse.microprofile.config.ConfigProvider; + +import io.quarkus.arc.Arc; +import io.quarkus.security.UnauthorizedException; +import io.quarkus.security.identity.CurrentIdentityAssociation; +import io.quarkus.security.identity.SecurityIdentity; +import io.quarkus.security.spi.runtime.AuthorizationController; +import io.quarkus.security.spi.runtime.AuthorizationFailureEvent; +import io.quarkus.security.spi.runtime.AuthorizationSuccessEvent; import io.quarkus.security.spi.runtime.MethodDescription; +import io.quarkus.security.spi.runtime.SecurityCheck; +import io.quarkus.security.spi.runtime.SecurityCheckStorage; +import io.quarkus.security.spi.runtime.SecurityEventHelper; import io.quarkus.vertx.http.runtime.security.EagerSecurityInterceptorStorage; import io.vertx.ext.web.RoutingContext; @@ -29,33 +45,106 @@ public void accept(RoutingContext routingContext) { } }; private final Map> cache = new HashMap<>(); + private final EagerSecurityInterceptorStorage interceptorStorage; + private final SecurityEventHelper eventHelper; @Context ResourceInfo resourceInfo; @Inject - EagerSecurityInterceptorStorage interceptorStorage; + RoutingContext routingContext; @Inject - RoutingContext routingContext; + SecurityCheckStorage securityCheckStorage; + + @Inject + CurrentIdentityAssociation identityAssociation; + + @Inject + AuthorizationController authorizationController; + + public EagerSecurityFilter() { + var interceptorStorageHandle = Arc.container().instance(EagerSecurityInterceptorStorage.class); + this.interceptorStorage = interceptorStorageHandle.isAvailable() ? interceptorStorageHandle.get() : null; + Event event = Arc.container().beanManager().getEvent(); + this.eventHelper = new SecurityEventHelper<>(event.select(AuthorizationSuccessEvent.class), + event.select(AuthorizationFailureEvent.class), AUTHORIZATION_SUCCESS, + AUTHORIZATION_FAILURE, Arc.container().beanManager(), + ConfigProvider.getConfig().getOptionalValue("quarkus.security.events.enabled", Boolean.class).orElse(false)); + } @Override public void filter(ContainerRequestContext requestContext) throws IOException { - var description = MethodDescription.ofMethod(resourceInfo.getResourceMethod()); - var interceptor = cache.get(description); + if (authorizationController.isAuthorizationEnabled()) { + var description = MethodDescription.ofMethod(resourceInfo.getResourceMethod()); + if (interceptorStorage != null) { + applyEagerSecurityInterceptors(description); + } + applySecurityChecks(description); + } + } + + private void applySecurityChecks(MethodDescription description) { + SecurityCheck check = securityCheckStorage.getSecurityCheck(description); + if (check != null) { + if (check.isPermitAll()) { + fireEventOnAuthZSuccess(check, null); + } else { + if (check.requiresMethodArguments()) { + if (identityAssociation.getIdentity().isAnonymous()) { + var exception = new UnauthorizedException(); + if (eventHelper.fireEventOnFailure()) { + fireEventOnAuthZFailure(exception, check); + } + throw exception; + } + // security check will be performed by CDI interceptor + return; + } + if (eventHelper.fireEventOnFailure()) { + try { + check.apply(identityAssociation.getIdentity(), description, null); + } catch (Exception e) { + fireEventOnAuthZFailure(e, check); + throw e; + } + } else { + check.apply(identityAssociation.getIdentity(), description, null); + } + fireEventOnAuthZSuccess(check, identityAssociation.getIdentity()); + } + // prevent repeated security checks + routingContext.put(EagerSecurityFilter.class.getName(), resourceInfo.getResourceMethod()); + } + } + + private void fireEventOnAuthZFailure(Exception exception, SecurityCheck check) { + eventHelper.fireFailureEvent(new AuthorizationFailureEvent( + identityAssociation.getIdentity(), exception, check.getClass().getName(), + Map.of(RoutingContext.class.getName(), routingContext))); + } - if (interceptor == NULL_SENTINEL) { - return; - } else if (interceptor != null) { - interceptor.accept(routingContext); + private void fireEventOnAuthZSuccess(SecurityCheck check, SecurityIdentity securityIdentity) { + if (eventHelper.fireEventOnSuccess()) { + eventHelper.fireSuccessEvent(new AuthorizationSuccessEvent(securityIdentity, + check.getClass().getName(), Map.of(RoutingContext.class.getName(), routingContext))); } + } - interceptor = interceptorStorage.getInterceptor(description); - if (interceptor == null) { - cache.put(description, NULL_SENTINEL); - } else { - cache.put(description, interceptor); - interceptor.accept(routingContext); + private void applyEagerSecurityInterceptors(MethodDescription description) { + var interceptor = cache.get(description); + if (interceptor != NULL_SENTINEL) { + if (interceptor != null) { + interceptor.accept(routingContext); + } else { + interceptor = interceptorStorage.getInterceptor(description); + if (interceptor == null) { + cache.put(description, NULL_SENTINEL); + } else { + cache.put(description, interceptor); + interceptor.accept(routingContext); + } + } } } } diff --git a/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/StandardSecurityCheckInterceptor.java b/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/StandardSecurityCheckInterceptor.java new file mode 100644 index 0000000000000..036ab8fcf3b74 --- /dev/null +++ b/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/StandardSecurityCheckInterceptor.java @@ -0,0 +1,85 @@ +package io.quarkus.resteasy.runtime; + +import static io.quarkus.security.spi.runtime.SecurityHandlerConstants.EXECUTED; +import static io.quarkus.security.spi.runtime.SecurityHandlerConstants.SECURITY_HANDLER; + +import java.lang.reflect.Method; + +import jakarta.annotation.Priority; +import jakarta.annotation.security.DenyAll; +import jakarta.annotation.security.PermitAll; +import jakarta.annotation.security.RolesAllowed; +import jakarta.inject.Inject; +import jakarta.interceptor.AroundInvoke; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InvocationContext; + +import io.quarkus.security.Authenticated; +import io.quarkus.security.PermissionsAllowed; +import io.quarkus.security.spi.runtime.AuthorizationController; +import io.vertx.ext.web.RoutingContext; + +/** + * Security checks for RBAC annotations on endpoints are done by the {@link EagerSecurityFilter}, this interceptor + * propagates the information to the SecurityHandler to prevent repeated checks. The {@link DenyAll} security check + * is performed just once. + */ +public abstract class StandardSecurityCheckInterceptor { + + @Inject + AuthorizationController controller; + + @Inject + RoutingContext routingContext; + + @AroundInvoke + public Object intercept(InvocationContext ic) throws Exception { + if (controller.isAuthorizationEnabled()) { + Method method = routingContext.get(EagerSecurityFilter.class.getName()); + if (method != null && method.equals(ic.getMethod())) { + ic.getContextData().put(SECURITY_HANDLER, EXECUTED); + } + } + return ic.proceed(); + } + + /** + * Prevent the SecurityHandler from performing {@link RolesAllowed} security checks + */ + @Interceptor + @RolesAllowed("") + @Priority(Interceptor.Priority.PLATFORM_BEFORE) + public static final class RolesAllowedInterceptor extends StandardSecurityCheckInterceptor { + + } + + /** + * Prevent the SecurityHandler from performing {@link PermissionsAllowed} security checks + */ + @Interceptor + @PermissionsAllowed("") + @Priority(Interceptor.Priority.PLATFORM_BEFORE) + public static final class PermissionsAllowedInterceptor extends StandardSecurityCheckInterceptor { + + } + + /** + * Prevent the SecurityHandler from performing {@link PermitAll} security checks + */ + @Interceptor + @PermitAll + @Priority(Interceptor.Priority.PLATFORM_BEFORE) + public static final class PermitAllInterceptor extends StandardSecurityCheckInterceptor { + + } + + /** + * Prevent the SecurityHandler from performing {@link Authenticated} security checks + */ + @Interceptor + @Authenticated + @Priority(Interceptor.Priority.PLATFORM_BEFORE) + public static final class AuthenticatedInterceptor extends StandardSecurityCheckInterceptor { + + } +} From d802748128cd1932279b7c334f3792d481814ef5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Vav=C5=99=C3=ADk?= Date: Tue, 23 Jan 2024 16:48:20 +0100 Subject: [PATCH 2/3] Fix JAX-RS default security checks for inherited / transformed endpoints (cherry picked from commit 6f3d752d157cc8eb22cc86c521cd6f029f67f42f) --- .../RestPathAnnotationProcessor.java | 2 +- .../deployment/ResteasyBuiltinsProcessor.java | 66 ++++++++++++++- .../security/AbstractSecurityEventTest.java | 3 +- .../DefaultRolesAllowedJaxRsTest.java | 16 +++- .../DefaultRolesAllowedStarJaxRsTest.java | 4 +- .../test/security/DenyAllJaxRsTest.java | 16 +++- .../security/UnsecuredParentResource.java | 14 ++++ .../test/security/UnsecuredResource.java | 7 +- .../security/UnsecuredResourceInterface.java | 14 ++++ .../ResteasyReactiveCommonProcessor.java | 53 ++---------- .../security/AbstractSecurityEventTest.java | 3 +- .../DefaultRolesAllowedJaxRsTest.java | 16 +++- .../DefaultRolesAllowedStarJaxRsTest.java | 4 +- .../test/security/DenyAllJaxRsTest.java | 84 ++++++++++++++++++- .../security/UnsecuredParentResource.java | 14 ++++ .../test/security/UnsecuredResource.java | 2 +- .../security/UnsecuredResourceInterface.java | 14 ++++ .../security/EagerSecurityHandler.java | 16 +++- .../deployment/SecurityProcessor.java | 11 +++ .../spi/runtime/SecurityCheckStorage.java | 5 ++ .../runtime/SecurityCheckRecorder.java | 4 + .../SecurityCheckStorageBuilder.java | 13 +++ .../spi/DefaultSecurityCheckBuildItem.java | 28 +++++++ .../common/processor/EndpointIndexer.java | 19 ----- 24 files changed, 339 insertions(+), 89 deletions(-) create mode 100644 extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredParentResource.java create mode 100644 extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredResourceInterface.java create mode 100644 extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredParentResource.java create mode 100644 extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResourceInterface.java create mode 100644 extensions/security/spi/src/main/java/io/quarkus/security/spi/DefaultSecurityCheckBuildItem.java diff --git a/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/RestPathAnnotationProcessor.java b/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/RestPathAnnotationProcessor.java index 5959d9e09c199..94b3a0993718a 100644 --- a/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/RestPathAnnotationProcessor.java +++ b/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/RestPathAnnotationProcessor.java @@ -182,7 +182,7 @@ static Optional searchPathAnnotationOnInterfaces(CombinedInd * @param resultAcc accumulator for tail-recursion * @return Collection of all interfaces und their parents. Never null. */ - private static Collection getAllClassInterfaces( + static Collection getAllClassInterfaces( CombinedIndexBuildItem index, Collection classInfos, List resultAcc) { diff --git a/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyBuiltinsProcessor.java b/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyBuiltinsProcessor.java index 2001747fbb72b..e525ee2caddfd 100644 --- a/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyBuiltinsProcessor.java +++ b/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyBuiltinsProcessor.java @@ -1,17 +1,21 @@ package io.quarkus.resteasy.deployment; import static io.quarkus.deployment.annotations.ExecutionTime.STATIC_INIT; +import static io.quarkus.resteasy.deployment.RestPathAnnotationProcessor.getAllClassInterfaces; import static io.quarkus.resteasy.deployment.RestPathAnnotationProcessor.isRestEndpointMethod; import static io.quarkus.security.spi.SecurityTransformerUtils.hasSecurityAnnotation; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Objects; +import java.util.function.Predicate; import java.util.stream.Collectors; import org.jboss.jandex.ClassInfo; import org.jboss.jandex.DotName; import org.jboss.jandex.MethodInfo; +import org.jboss.logging.Logger; import io.quarkus.arc.deployment.AdditionalBeanBuildItem; import io.quarkus.deployment.Capabilities; @@ -50,6 +54,7 @@ public class ResteasyBuiltinsProcessor { protected static final String META_INF_RESOURCES = "META-INF/resources"; + private static final Logger LOG = Logger.getLogger(ResteasyBuiltinsProcessor.class); @BuildStep void setUpDenyAllJaxRs(CombinedIndexBuildItem index, @@ -65,10 +70,42 @@ void setUpDenyAllJaxRs(CombinedIndexBuildItem index, ClassInfo classInfo = index.getIndex().getClassByName(DotName.createSimple(className)); if (classInfo == null) throw new IllegalStateException("Unable to find class info for " + className); - if (!hasSecurityAnnotation(classInfo)) { - for (MethodInfo methodInfo : classInfo.methods()) { - if (isRestEndpointMethod(index, methodInfo) && !hasSecurityAnnotation(methodInfo)) { - methods.add(methodInfo); + // add unannotated class endpoints as well as parent class unannotated endpoints + addAllUnannotatedEndpoints(index, classInfo, methods); + + // interface endpoints implemented on resources are already in, now we need to resolve default interface + // methods as there, CDI interceptors won't work, therefore neither will our additional secured methods + Collection interfaces = getAllClassInterfaces(index, List.of(classInfo), new ArrayList<>()); + if (!interfaces.isEmpty()) { + final List interfaceEndpoints = new ArrayList<>(); + for (ClassInfo anInterface : interfaces) { + addUnannotatedEndpoints(index, anInterface, interfaceEndpoints); + } + // look for implementors as implementors on resource classes are secured by CDI interceptors + if (!interfaceEndpoints.isEmpty()) { + interfaceBlock: for (MethodInfo interfaceEndpoint : interfaceEndpoints) { + if (interfaceEndpoint.isDefault()) { + for (MethodInfo endpoint : methods) { + boolean nameParamsMatch = endpoint.name().equals(interfaceEndpoint.name()) + && (interfaceEndpoint.parameterTypes().equals(endpoint.parameterTypes())); + if (nameParamsMatch) { + // whether matched method is declared on class that implements interface endpoint + Predicate isEndpointInterface = interfaceEndpoint.declaringClass() + .name()::equals; + if (endpoint.declaringClass().interfaceNames().stream().anyMatch(isEndpointInterface)) { + continue interfaceBlock; + } + } + } + String configProperty = config.denyJaxRs ? "quarkus.security.jaxrs.deny-unannotated-endpoints" + : "quarkus.security.jaxrs.default-roles-allowed"; + // this is logging only as I'm a bit worried about false positives and breaking things + // for what is very much edge case + LOG.warn("Default interface method '" + interfaceEndpoint + + "' cannot be secured with the '" + configProperty + + "' configuration property. Please implement this method for CDI " + + "interceptor binding to work"); + } } } } @@ -85,6 +122,27 @@ void setUpDenyAllJaxRs(CombinedIndexBuildItem index, } } + private static void addAllUnannotatedEndpoints(CombinedIndexBuildItem index, ClassInfo classInfo, + List methods) { + if (classInfo == null) { + return; + } + addUnannotatedEndpoints(index, classInfo, methods); + if (classInfo.superClassType() != null && !classInfo.superClassType().name().equals(DotName.OBJECT_NAME)) { + addAllUnannotatedEndpoints(index, index.getIndex().getClassByName(classInfo.superClassType().name()), methods); + } + } + + private static void addUnannotatedEndpoints(CombinedIndexBuildItem index, ClassInfo classInfo, List methods) { + if (!hasSecurityAnnotation(classInfo)) { + for (MethodInfo methodInfo : classInfo.methods()) { + if (isRestEndpointMethod(index, methodInfo) && !hasSecurityAnnotation(methodInfo)) { + methods.add(methodInfo); + } + } + } + } + /** * Install the JAX-RS security provider. */ diff --git a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/AbstractSecurityEventTest.java b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/AbstractSecurityEventTest.java index ee7c11733afc8..007fbb74b414e 100644 --- a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/AbstractSecurityEventTest.java +++ b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/AbstractSecurityEventTest.java @@ -37,7 +37,8 @@ public abstract class AbstractSecurityEventTest { protected static final Class[] TEST_CLASSES = { RolesAllowedResource.class, TestIdentityProvider.class, TestIdentityController.class, - UnsecuredResource.class, UnsecuredSubResource.class, EventObserver.class + UnsecuredResource.class, UnsecuredSubResource.class, EventObserver.class, UnsecuredResourceInterface.class, + UnsecuredParentResource.class }; @Inject diff --git a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DefaultRolesAllowedJaxRsTest.java b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DefaultRolesAllowedJaxRsTest.java index fb13a3d50cbf6..b9e18eed5475c 100644 --- a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DefaultRolesAllowedJaxRsTest.java +++ b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DefaultRolesAllowedJaxRsTest.java @@ -22,8 +22,8 @@ public class DefaultRolesAllowedJaxRsTest { static QuarkusUnitTest runner = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar .addClasses(PermitAllResource.class, UnsecuredResource.class, - TestIdentityProvider.class, - TestIdentityController.class, + TestIdentityProvider.class, UnsecuredResourceInterface.class, + TestIdentityController.class, UnsecuredParentResource.class, UnsecuredSubResource.class, HelloResource.class) .addAsResource(new StringAsset("quarkus.security.jaxrs.default-roles-allowed = admin\n"), "application.properties")); @@ -41,6 +41,18 @@ public void shouldDenyUnannotated() { assertStatus(path, 200, 403, 401); } + @Test + public void shouldDenyUnannotatedOnParentClass() { + String path = "/unsecured/defaultSecurityParent"; + assertStatus(path, 200, 403, 401); + } + + @Test + public void shouldDenyUnannotatedOnInterface() { + String path = "/unsecured/defaultSecurityInterface"; + assertStatus(path, 200, 403, 401); + } + @Test public void shouldDenyDenyAllMethod() { String path = "/unsecured/denyAll"; diff --git a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DefaultRolesAllowedStarJaxRsTest.java b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DefaultRolesAllowedStarJaxRsTest.java index 4e0fd8c7dd808..ddcc31f5ae87e 100644 --- a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DefaultRolesAllowedStarJaxRsTest.java +++ b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DefaultRolesAllowedStarJaxRsTest.java @@ -17,8 +17,8 @@ public class DefaultRolesAllowedStarJaxRsTest { static QuarkusUnitTest runner = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar .addClasses(PermitAllResource.class, UnsecuredResource.class, - TestIdentityProvider.class, - TestIdentityController.class, + TestIdentityProvider.class, UnsecuredParentResource.class, + TestIdentityController.class, UnsecuredResourceInterface.class, UnsecuredSubResource.class) .addAsResource(new StringAsset("quarkus.security.jaxrs.default-roles-allowed = **\n"), "application.properties")); diff --git a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DenyAllJaxRsTest.java b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DenyAllJaxRsTest.java index 90cd2a9f77390..8ed4d8cbf445c 100644 --- a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DenyAllJaxRsTest.java +++ b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DenyAllJaxRsTest.java @@ -26,8 +26,8 @@ public class DenyAllJaxRsTest { static QuarkusUnitTest runner = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar .addClasses(PermitAllResource.class, UnsecuredResource.class, - TestIdentityProvider.class, - TestIdentityController.class, + TestIdentityProvider.class, UnsecuredParentResource.class, + TestIdentityController.class, UnsecuredResourceInterface.class, UnsecuredSubResource.class, HelloResource.class) .addAsResource(new StringAsset("quarkus.security.jaxrs.deny-unannotated-endpoints = true\n"), "application.properties")); @@ -58,6 +58,18 @@ public void shouldDenyUnannotated() { assertStatus(path, 403, 401); } + @Test + public void shouldDenyUnannotatedOnParentClass() { + String path = "/unsecured/defaultSecurityParent"; + assertStatus(path, 403, 401); + } + + @Test + public void shouldDenyUnannotatedOnInterface() { + String path = "/unsecured/defaultSecurityInterface"; + assertStatus(path, 403, 401); + } + @Test public void shouldDenyDenyAllMethod() { String path = "/unsecured/denyAll"; diff --git a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredParentResource.java b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredParentResource.java new file mode 100644 index 0000000000000..abf5b385e9a0d --- /dev/null +++ b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredParentResource.java @@ -0,0 +1,14 @@ +package io.quarkus.resteasy.test.security; + +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; + +public class UnsecuredParentResource { + + @Path("/defaultSecurityParent") + @GET + public String defaultSecurityParent() { + return "defaultSecurityParent"; + } + +} diff --git a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredResource.java b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredResource.java index 116f041ba538d..7339c9c1eda07 100644 --- a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredResource.java +++ b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredResource.java @@ -12,13 +12,18 @@ * @author Michal Szynkiewicz, michal.l.szynkiewicz@gmail.com */ @Path("/unsecured") -public class UnsecuredResource { +public class UnsecuredResource extends UnsecuredParentResource implements UnsecuredResourceInterface { @Path("/defaultSecurity") @GET public String defaultSecurity() { return "defaultSecurity"; } + @Override + public String defaultSecurityInterface() { + return UnsecuredResourceInterface.super.defaultSecurityInterface(); + } + @Path("/permitAllPathParam/{index}") @GET @PermitAll diff --git a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredResourceInterface.java b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredResourceInterface.java new file mode 100644 index 0000000000000..d2498d46a8c63 --- /dev/null +++ b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredResourceInterface.java @@ -0,0 +1,14 @@ +package io.quarkus.resteasy.test.security; + +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; + +public interface UnsecuredResourceInterface { + + @Path("/defaultSecurityInterface") + @GET + default String defaultSecurityInterface() { + return "defaultSecurityInterface"; + } + +} diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive-common/deployment/src/main/java/io/quarkus/resteasy/reactive/common/deployment/ResteasyReactiveCommonProcessor.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive-common/deployment/src/main/java/io/quarkus/resteasy/reactive/common/deployment/ResteasyReactiveCommonProcessor.java index ff52973878de5..272d25e5b98f1 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive-common/deployment/src/main/java/io/quarkus/resteasy/reactive/common/deployment/ResteasyReactiveCommonProcessor.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive-common/deployment/src/main/java/io/quarkus/resteasy/reactive/common/deployment/ResteasyReactiveCommonProcessor.java @@ -1,13 +1,9 @@ package io.quarkus.resteasy.reactive.common.deployment; -import static io.quarkus.security.spi.SecurityTransformerUtils.hasSecurityAnnotation; import static org.jboss.resteasy.reactive.common.model.ResourceInterceptor.FILTER_SOURCE_METHOD_METADATA_KEY; -import static org.jboss.resteasy.reactive.common.processor.EndpointIndexer.collectClassEndpoints; import java.io.ByteArrayInputStream; import java.io.IOException; -import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -76,7 +72,7 @@ import io.quarkus.resteasy.reactive.spi.MessageBodyWriterOverrideBuildItem; import io.quarkus.resteasy.reactive.spi.ReaderInterceptorBuildItem; import io.quarkus.resteasy.reactive.spi.WriterInterceptorBuildItem; -import io.quarkus.security.spi.AdditionalSecuredMethodsBuildItem; +import io.quarkus.security.spi.DefaultSecurityCheckBuildItem; public class ResteasyReactiveCommonProcessor { @@ -134,46 +130,13 @@ void searchForProviders(Capabilities capabilities, } @BuildStep - void setUpDenyAllJaxRs( - CombinedIndexBuildItem index, - JaxRsSecurityConfig securityConfig, - Optional resteasyDeployment, - BeanArchiveIndexBuildItem beanArchiveIndexBuildItem, - ApplicationResultBuildItem applicationResultBuildItem, - BuildProducer additionalSecuredClasses) { - - if (resteasyDeployment.isPresent() - && (securityConfig.denyJaxRs() || securityConfig.defaultRolesAllowed().isPresent())) { - final List methods = new ArrayList<>(); - Map httpAnnotationToMethod = resteasyDeployment.get().getResult().getHttpAnnotationToMethod(); - Set resourceClasses = resteasyDeployment.get().getResult().getScannedResourcePaths().keySet(); - - for (DotName className : resourceClasses) { - ClassInfo classInfo = index.getIndex().getClassByName(className); - if (classInfo == null) - throw new IllegalStateException("Unable to find class info for " + className); - if (!hasSecurityAnnotation(classInfo)) { - // collect class endpoints - Collection classEndpoints = collectClassEndpoints(classInfo, httpAnnotationToMethod, - beanArchiveIndexBuildItem.getIndex(), applicationResultBuildItem.getResult()); - - // add endpoints - for (MethodInfo classEndpoint : classEndpoints) { - if (!hasSecurityAnnotation(classEndpoint)) { - methods.add(classEndpoint); - } - } - } - } - - if (!methods.isEmpty()) { - if (securityConfig.denyJaxRs()) { - additionalSecuredClasses.produce(new AdditionalSecuredMethodsBuildItem(methods)); - } else { - additionalSecuredClasses - .produce(new AdditionalSecuredMethodsBuildItem(methods, securityConfig.defaultRolesAllowed())); - } - } + void setUpDenyAllJaxRs(JaxRsSecurityConfig securityConfig, + BuildProducer defaultSecurityCheckProducer) { + if (securityConfig.denyJaxRs()) { + defaultSecurityCheckProducer.produce(DefaultSecurityCheckBuildItem.denyAll()); + } else if (securityConfig.defaultRolesAllowed().isPresent()) { + defaultSecurityCheckProducer + .produce(DefaultSecurityCheckBuildItem.rolesAllowed(securityConfig.defaultRolesAllowed().get())); } } diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/AbstractSecurityEventTest.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/AbstractSecurityEventTest.java index cc8d3a9db7199..1d3366fd2220a 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/AbstractSecurityEventTest.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/AbstractSecurityEventTest.java @@ -40,7 +40,8 @@ public abstract class AbstractSecurityEventTest { protected static final Class[] TEST_CLASSES = { RolesAllowedResource.class, RolesAllowedBlockingResource.class, TestIdentityProvider.class, TestIdentityController.class, UnsecuredResource.class, UnsecuredSubResource.class, RolesAllowedService.class, - RolesAllowedServiceResource.class, EventObserver.class + RolesAllowedServiceResource.class, EventObserver.class, UnsecuredResourceInterface.class, + UnsecuredParentResource.class }; @Inject diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DefaultRolesAllowedJaxRsTest.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DefaultRolesAllowedJaxRsTest.java index 4590ee9a9d02f..ca34474ad9052 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DefaultRolesAllowedJaxRsTest.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DefaultRolesAllowedJaxRsTest.java @@ -18,9 +18,9 @@ public class DefaultRolesAllowedJaxRsTest { static QuarkusUnitTest runner = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar .addClasses(PermitAllResource.class, UnsecuredResource.class, - TestIdentityProvider.class, + TestIdentityProvider.class, UnsecuredResourceInterface.class, TestIdentityController.class, - UnsecuredSubResource.class, HelloResource.class) + UnsecuredSubResource.class, HelloResource.class, UnsecuredParentResource.class) .addAsResource(new StringAsset("quarkus.security.jaxrs.default-roles-allowed=admin\n"), "application.properties")); @@ -37,6 +37,18 @@ public void shouldDenyUnannotated() { assertStatus(path, 200, 403, 401); } + @Test + public void shouldDenyUnannotatedParent() { + String path = "/unsecured/defaultSecurityParent"; + assertStatus(path, 200, 403, 401); + } + + @Test + public void shouldDenyUnannotatedInterface() { + String path = "/unsecured/defaultSecurityInterface"; + assertStatus(path, 200, 403, 401); + } + @Test public void shouldDenyDenyAllMethod() { String path = "/unsecured/denyAll"; diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DefaultRolesAllowedStarJaxRsTest.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DefaultRolesAllowedStarJaxRsTest.java index dcad10e88e0ce..6f77ef2fff88e 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DefaultRolesAllowedStarJaxRsTest.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DefaultRolesAllowedStarJaxRsTest.java @@ -17,8 +17,8 @@ public class DefaultRolesAllowedStarJaxRsTest { static QuarkusUnitTest runner = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar .addClasses(PermitAllResource.class, UnsecuredResource.class, - TestIdentityProvider.class, - TestIdentityController.class, + TestIdentityProvider.class, UnsecuredResourceInterface.class, + TestIdentityController.class, UnsecuredParentResource.class, UnsecuredSubResource.class) .addAsResource(new StringAsset("quarkus.security.jaxrs.default-roles-allowed = **\n"), "application.properties")); diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DenyAllJaxRsTest.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DenyAllJaxRsTest.java index 2e10fb07015e4..76f23ddcc8056 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DenyAllJaxRsTest.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DenyAllJaxRsTest.java @@ -3,12 +3,25 @@ import static io.restassured.RestAssured.given; import static io.restassured.RestAssured.when; +import java.lang.reflect.Modifier; + +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; + import org.hamcrest.Matchers; +import org.jboss.jandex.AnnotationTarget; +import org.jboss.jandex.AnnotationValue; +import org.jboss.jandex.ClassInfo; +import org.jboss.jandex.MethodInfo; +import org.jboss.resteasy.reactive.common.processor.transformation.AnnotationsTransformer; 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 io.quarkus.builder.BuildContext; +import io.quarkus.builder.BuildStep; +import io.quarkus.resteasy.reactive.server.spi.AnnotationsTransformerBuildItem; import io.quarkus.security.test.utils.TestIdentityController; import io.quarkus.security.test.utils.TestIdentityProvider; import io.quarkus.test.QuarkusUnitTest; @@ -21,11 +34,43 @@ public class DenyAllJaxRsTest { static QuarkusUnitTest runner = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar .addClasses(PermitAllResource.class, UnsecuredResource.class, - TestIdentityProvider.class, - TestIdentityController.class, - UnsecuredSubResource.class, HelloResource.class) + TestIdentityProvider.class, UnsecuredResourceInterface.class, + TestIdentityController.class, SpecialResource.class, + UnsecuredSubResource.class, HelloResource.class, UnsecuredParentResource.class) .addAsResource(new StringAsset("quarkus.security.jaxrs.deny-unannotated-endpoints = true\n"), - "application.properties")); + "application.properties")) + .addBuildChainCustomizer(builder -> { + builder.addBuildStep(new BuildStep() { + @Override + public void execute(BuildContext context) { + // Here we add an AnnotationsTransformer in order to make sure that the security layer + // uses the proper set of transformers + context.produce( + new AnnotationsTransformerBuildItem( + AnnotationsTransformer.builder().appliesTo(AnnotationTarget.Kind.METHOD) + .transform(transformerContext -> { + // This transformer auto-adds @GET and @Path if missing, thus emulating Renarde + MethodInfo methodInfo = transformerContext.getTarget().asMethod(); + ClassInfo declaringClass = methodInfo.declaringClass(); + if (declaringClass.name().toString().equals(SpecialResource.class.getName()) + && !methodInfo.isConstructor() + && !Modifier.isStatic(methodInfo.flags())) { + if (methodInfo.declaredAnnotation(GET.class.getName()) == null) { + // auto-add it + transformerContext.transform().add(GET.class).done(); + } + if (methodInfo.declaredAnnotation(Path.class.getName()) == null) { + // auto-add it + transformerContext.transform().add(Path.class, + AnnotationValue.createStringValue("value", + methodInfo.name())) + .done(); + } + } + }))); + } + }).produces(AnnotationsTransformerBuildItem.class).build(); + }); @BeforeAll public static void setupUsers() { @@ -40,6 +85,18 @@ public void shouldDenyUnannotated() { assertStatus(path, 403, 401); } + @Test + public void shouldDenyUnannotatedOnParentClass() { + String path = "/unsecured/defaultSecurityParent"; + assertStatus(path, 403, 401); + } + + @Test + public void shouldDenyUnannotatedOnInterface() { + String path = "/unsecured/defaultSecurityInterface"; + assertStatus(path, 403, 401); + } + @Test public void shouldDenyUnannotatedNonBlocking() { String path = "/unsecured/defaultSecurityNonBlocking"; @@ -90,6 +147,14 @@ public void testServerExceptionMapper() { .body(Matchers.equalTo("unauthorizedExceptionMapper")); } + @Test + public void shouldDenyUnannotatedWithAnnotationTransformer() { + String path = "/special/explicit"; + assertStatus(path, 403, 401); + path = "/special/implicit"; + assertStatus(path, 403, 401); + } + private void assertStatus(String path, int status, int anonStatus) { given().auth().preemptive() .basic("admin", "admin").get(path) @@ -105,4 +170,15 @@ private void assertStatus(String path, int status, int anonStatus) { } + @Path("/special") + public static class SpecialResource { + @GET + public String explicit() { + return "explicit"; + } + + public String implicit() { + return "implicit"; + } + } } diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredParentResource.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredParentResource.java new file mode 100644 index 0000000000000..8250d5a9bf9a6 --- /dev/null +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredParentResource.java @@ -0,0 +1,14 @@ +package io.quarkus.resteasy.reactive.server.test.security; + +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; + +public class UnsecuredParentResource { + + @Path("/defaultSecurityParent") + @GET + public String defaultSecurityParent() { + return "defaultSecurityParent"; + } + +} diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResource.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResource.java index bca5025db0a5f..82622c3e585de 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResource.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResource.java @@ -12,7 +12,7 @@ * @author Michal Szynkiewicz, michal.l.szynkiewicz@gmail.com */ @Path("/unsecured") -public class UnsecuredResource { +public class UnsecuredResource extends UnsecuredParentResource implements UnsecuredResourceInterface { @Path("/defaultSecurity") @GET public String defaultSecurity() { diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResourceInterface.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResourceInterface.java new file mode 100644 index 0000000000000..7be652f15ef8d --- /dev/null +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResourceInterface.java @@ -0,0 +1,14 @@ +package io.quarkus.resteasy.reactive.server.test.security; + +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; + +public interface UnsecuredResourceInterface { + + @Path("/defaultSecurityInterface") + @GET + default String defaultSecurityInterface() { + return "defaultSecurityInterface"; + } + +} diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/security/EagerSecurityHandler.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/security/EagerSecurityHandler.java index 00b25676a4bd4..f94c369070cf0 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/security/EagerSecurityHandler.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/security/EagerSecurityHandler.java @@ -66,9 +66,14 @@ public void handle(ResteasyReactiveRequestContext requestContext) throws Excepti ResteasyReactiveResourceInfo lazyMethod = requestContext.getTarget().getLazyMethod(); MethodDescription methodDescription = lazyMethodToMethodDescription(lazyMethod); if (check == null) { - check = Arc.container().instance(SecurityCheckStorage.class).get().getSecurityCheck(methodDescription); + SecurityCheckStorage storage = Arc.container().instance(SecurityCheckStorage.class).get(); + check = storage.getSecurityCheck(methodDescription); if (check == null) { - check = NULL_SENTINEL; + if (storage.getDefaultSecurityCheck() == null || isRequestAlreadyChecked(requestContext)) { + check = NULL_SENTINEL; + } else { + check = storage.getDefaultSecurityCheck(); + } } this.check = check; } @@ -193,6 +198,13 @@ private void preventRepeatedSecurityChecks(ResteasyReactiveRequestContext reques requestContext.setProperty(STANDARD_SECURITY_CHECK_INTERCEPTOR, methodDescription); } + private boolean isRequestAlreadyChecked(ResteasyReactiveRequestContext requestContext) { + // when request has already been checked at least once (by another instance of this handler) + // then default security checks, like denied access to all JAX-RS resources by default + // shouldn't be applied; this doesn't mean security checks registered for methods shouldn't be applied + return requestContext.getProperty(STANDARD_SECURITY_CHECK_INTERCEPTOR) != null; + } + private InjectableInstance getCurrentIdentityAssociation() { InjectableInstance identityAssociation = this.currentIdentityAssociation; if (identityAssociation == null) { diff --git a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java b/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java index d781085f835ac..8aa777f640b67 100644 --- a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java +++ b/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java @@ -105,6 +105,7 @@ import io.quarkus.security.runtime.interceptor.SecurityHandler; import io.quarkus.security.spi.AdditionalSecuredClassesBuildItem; import io.quarkus.security.spi.AdditionalSecuredMethodsBuildItem; +import io.quarkus.security.spi.DefaultSecurityCheckBuildItem; import io.quarkus.security.spi.RolesAllowedConfigExpResolverBuildItem; import io.quarkus.security.spi.runtime.AuthorizationController; import io.quarkus.security.spi.runtime.DevModeDisabledAuthorizationController; @@ -527,6 +528,7 @@ void gatherSecurityChecks(BuildProducer syntheticBeans, BuildProducer configBuilderProducer, List additionalSecuredMethods, SecurityCheckRecorder recorder, + Optional defaultSecurityCheckBuildItem, BuildProducer reflectiveClassBuildItemBuildProducer, List additionalSecurityChecks, SecurityBuildTimeConfig config) { classPredicate.produce(new ApplicationClassPredicateBuildItem(new SecurityCheckStorageAppPredicate())); @@ -559,6 +561,15 @@ void gatherSecurityChecks(BuildProducer syntheticBeans, recorder.addMethod(builder, method.declaringClass().name().toString(), method.name(), params, methodEntry.getValue()); } + + if (defaultSecurityCheckBuildItem.isPresent()) { + var roles = defaultSecurityCheckBuildItem.get().getRolesAllowed(); + if (roles == null) { + recorder.registerDefaultSecurityCheck(builder, recorder.denyAll()); + } else { + recorder.registerDefaultSecurityCheck(builder, recorder.rolesAllowed(roles.toArray(new String[0]))); + } + } recorder.create(builder); syntheticBeans.produce( diff --git a/extensions/security/runtime-spi/src/main/java/io/quarkus/security/spi/runtime/SecurityCheckStorage.java b/extensions/security/runtime-spi/src/main/java/io/quarkus/security/spi/runtime/SecurityCheckStorage.java index 3cfeaa2b44b80..2d72f5f8dc2e2 100644 --- a/extensions/security/runtime-spi/src/main/java/io/quarkus/security/spi/runtime/SecurityCheckStorage.java +++ b/extensions/security/runtime-spi/src/main/java/io/quarkus/security/spi/runtime/SecurityCheckStorage.java @@ -10,4 +10,9 @@ default SecurityCheck getSecurityCheck(Method method) { SecurityCheck getSecurityCheck(MethodDescription methodDescription); + /** + * {@link SecurityCheck} that should be applied when there is no other check applied on incoming request. + */ + SecurityCheck getDefaultSecurityCheck(); + } diff --git a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityCheckRecorder.java b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityCheckRecorder.java index 8661d06f3f4bd..545b3249cd9b0 100644 --- a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityCheckRecorder.java +++ b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityCheckRecorder.java @@ -351,4 +351,8 @@ private Class loadClass(String className) { throw new RuntimeException("Unable to load class '" + className + "' for creating permission", e); } } + + public void registerDefaultSecurityCheck(RuntimeValue builder, SecurityCheck securityCheck) { + builder.getValue().registerDefaultSecurityCheck(securityCheck); + } } diff --git a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/interceptor/SecurityCheckStorageBuilder.java b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/interceptor/SecurityCheckStorageBuilder.java index 6be2f134e0057..25cecdb398e78 100644 --- a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/interceptor/SecurityCheckStorageBuilder.java +++ b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/interceptor/SecurityCheckStorageBuilder.java @@ -9,6 +9,7 @@ public class SecurityCheckStorageBuilder { private final Map securityChecks = new HashMap<>(); + private SecurityCheck defaultSecurityCheck; public void registerCheck(String className, String methodName, @@ -17,12 +18,24 @@ public void registerCheck(String className, securityChecks.put(new MethodDescription(className, methodName, parameterTypes), securityCheck); } + public void registerDefaultSecurityCheck(SecurityCheck defaultSecurityCheck) { + if (this.defaultSecurityCheck != null) { + throw new IllegalStateException("Default SecurityCheck has already been registered"); + } + this.defaultSecurityCheck = defaultSecurityCheck; + } + public SecurityCheckStorage create() { return new SecurityCheckStorage() { @Override public SecurityCheck getSecurityCheck(MethodDescription methodDescription) { return securityChecks.get(methodDescription); } + + @Override + public SecurityCheck getDefaultSecurityCheck() { + return defaultSecurityCheck; + } }; } } diff --git a/extensions/security/spi/src/main/java/io/quarkus/security/spi/DefaultSecurityCheckBuildItem.java b/extensions/security/spi/src/main/java/io/quarkus/security/spi/DefaultSecurityCheckBuildItem.java new file mode 100644 index 0000000000000..ed3dafe18de0d --- /dev/null +++ b/extensions/security/spi/src/main/java/io/quarkus/security/spi/DefaultSecurityCheckBuildItem.java @@ -0,0 +1,28 @@ +package io.quarkus.security.spi; + +import java.util.List; +import java.util.Objects; + +import io.quarkus.builder.item.SimpleBuildItem; + +public final class DefaultSecurityCheckBuildItem extends SimpleBuildItem { + + public final List rolesAllowed; + + private DefaultSecurityCheckBuildItem(List rolesAllowed) { + this.rolesAllowed = rolesAllowed; + } + + public static DefaultSecurityCheckBuildItem denyAll() { + return new DefaultSecurityCheckBuildItem(null); + } + + public static DefaultSecurityCheckBuildItem rolesAllowed(List rolesAllowed) { + Objects.requireNonNull(rolesAllowed); + return new DefaultSecurityCheckBuildItem(List.copyOf(rolesAllowed)); + } + + public List getRolesAllowed() { + return rolesAllowed; + } +} diff --git a/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointIndexer.java b/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointIndexer.java index 41f8bcaa557ef..a16267eab7b54 100644 --- a/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointIndexer.java +++ b/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointIndexer.java @@ -422,25 +422,6 @@ protected List createEndpoints(ClassInfo currentClassInfo, return ret; } - /** - * Return endpoints defined directly on classInfo. - * - * @param classInfo resource class - * @return classInfo endpoint method info - */ - public static Collection collectClassEndpoints(ClassInfo classInfo, - Map httpAnnotationToMethod, IndexView index, ApplicationScanningResult applicationScanningResult) { - Collection endpoints = collectEndpoints(classInfo, classInfo, new HashSet<>(), new HashSet<>(), true, - httpAnnotationToMethod, index, applicationScanningResult, new AnnotationStore(null)); - Collection ret = new HashSet<>(); - for (FoundEndpoint endpoint : endpoints) { - if (endpoint.classInfo.equals(classInfo)) { - ret.add(endpoint.methodInfo); - } - } - return ret; - } - private static List collectEndpoints(ClassInfo currentClassInfo, ClassInfo actualEndpointInfo, Set seenMethods, Set existingClassNameBindings, boolean considerApplication, Map httpAnnotationToMethod, IndexView index, ApplicationScanningResult applicationScanningResult, From 3dcf570da9722e155c959b1a9c7ced4a8597411f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Vav=C5=99=C3=ADk?= Date: Fri, 26 Jan 2024 18:22:49 +0100 Subject: [PATCH 3/3] Do not require RoutingContext outside or RESTEasy handler (cherry picked from commit 36bfe4cda3129e242f95d7e2b14703d531aa29e0) --- .../runtime/StandardSecurityCheckInterceptor.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/StandardSecurityCheckInterceptor.java b/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/StandardSecurityCheckInterceptor.java index 036ab8fcf3b74..10ad1effe0416 100644 --- a/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/StandardSecurityCheckInterceptor.java +++ b/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/StandardSecurityCheckInterceptor.java @@ -17,7 +17,7 @@ import io.quarkus.security.Authenticated; import io.quarkus.security.PermissionsAllowed; import io.quarkus.security.spi.runtime.AuthorizationController; -import io.vertx.ext.web.RoutingContext; +import io.quarkus.vertx.http.runtime.CurrentVertxRequest; /** * Security checks for RBAC annotations on endpoints are done by the {@link EagerSecurityFilter}, this interceptor @@ -30,12 +30,14 @@ public abstract class StandardSecurityCheckInterceptor { AuthorizationController controller; @Inject - RoutingContext routingContext; + CurrentVertxRequest currentVertxRequest; @AroundInvoke public Object intercept(InvocationContext ic) throws Exception { - if (controller.isAuthorizationEnabled()) { - Method method = routingContext.get(EagerSecurityFilter.class.getName()); + // RoutingContext can be null if RESTEasy is used together with other stacks that do not rely on it (e.g. gRPC) + // and this is not invoked from RESTEasy route handler + if (controller.isAuthorizationEnabled() && currentVertxRequest.getCurrent() != null) { + Method method = currentVertxRequest.getCurrent().get(EagerSecurityFilter.class.getName()); if (method != null && method.equals(ic.getMethod())) { ic.getContextData().put(SECURITY_HANDLER, EXECUTED); }