From 284b4afd71ce56b2c4850903f15a12109f6de808 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Vav=C5=99=C3=ADk?= Date: Tue, 6 Feb 2024 21:31:59 +0100 Subject: [PATCH] Refactor default JAX-RS sec in RESTEasy Classic --- .../deployment/ResteasyBuiltinsProcessor.java | 105 ++---------------- .../resteasy/runtime/EagerSecurityFilter.java | 6 + .../StandardSecurityCheckInterceptor.java | 14 ++- .../AdditionalSecuredMethodsBuildItem.java | 4 + 4 files changed, 29 insertions(+), 100 deletions(-) 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 e525ee2caddfd..f7e2f2eba1b46 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,22 +1,12 @@ 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; import io.quarkus.deployment.Capability; @@ -25,7 +15,6 @@ import io.quarkus.deployment.annotations.BuildStep; import io.quarkus.deployment.annotations.Record; import io.quarkus.deployment.builditem.ApplicationArchivesBuildItem; -import io.quarkus.deployment.builditem.CombinedIndexBuildItem; import io.quarkus.resteasy.common.spi.ResteasyJaxrsProviderBuildItem; import io.quarkus.resteasy.runtime.AuthenticationCompletionExceptionMapper; import io.quarkus.resteasy.runtime.AuthenticationFailedExceptionMapper; @@ -43,8 +32,7 @@ import io.quarkus.resteasy.runtime.vertx.JsonArrayWriter; import io.quarkus.resteasy.runtime.vertx.JsonObjectReader; import io.quarkus.resteasy.runtime.vertx.JsonObjectWriter; -import io.quarkus.resteasy.server.common.deployment.ResteasyDeploymentBuildItem; -import io.quarkus.security.spi.AdditionalSecuredMethodsBuildItem; +import io.quarkus.security.spi.DefaultSecurityCheckBuildItem; import io.quarkus.vertx.http.deployment.HttpRootPathBuildItem; import io.quarkus.vertx.http.deployment.devmode.NotFoundPageDisplayableEndpointBuildItem; import io.quarkus.vertx.http.deployment.devmode.RouteDescriptionBuildItem; @@ -54,92 +42,15 @@ 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, - JaxRsSecurityConfig config, - ResteasyDeploymentBuildItem resteasyDeployment, - BuildProducer additionalSecuredClasses) { - if (resteasyDeployment != null && (config.denyJaxRs || config.defaultRolesAllowed.isPresent())) { - final List methods = new ArrayList<>(); - - // add endpoints - List resourceClasses = resteasyDeployment.getDeployment().getScannedResourceClasses(); - for (String className : resourceClasses) { - ClassInfo classInfo = index.getIndex().getClassByName(DotName.createSimple(className)); - if (classInfo == null) - throw new IllegalStateException("Unable to find class info for " + className); - // 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"); - } - } - } - } - } - - if (!methods.isEmpty()) { - if (config.denyJaxRs) { - additionalSecuredClasses.produce(new AdditionalSecuredMethodsBuildItem(methods)); - } else { - additionalSecuredClasses - .produce(new AdditionalSecuredMethodsBuildItem(methods, config.defaultRolesAllowed)); - } - } - } - } - - 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); - } - } + 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-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 e05df7c8dc751..cd1fa2d2b1295 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 @@ -44,6 +44,7 @@ public void accept(RoutingContext routingContext) { } }; + static final String SKIP_DEFAULT_CHECK = "io.quarkus.resteasy.runtime.EagerSecurityFilter#SKIP_DEFAULT_CHECK"; private final Map> cache = new HashMap<>(); private final EagerSecurityInterceptorStorage interceptorStorage; private final SecurityEventHelper eventHelper; @@ -86,6 +87,11 @@ public void filter(ContainerRequestContext requestContext) throws IOException { private void applySecurityChecks(MethodDescription description) { SecurityCheck check = securityCheckStorage.getSecurityCheck(description); + if (check == null && securityCheckStorage.getDefaultSecurityCheck() != null + && routingContext.get(EagerSecurityFilter.class.getName()) == null + && routingContext.get(SKIP_DEFAULT_CHECK) == null) { + check = securityCheckStorage.getDefaultSecurityCheck(); + } if (check != null) { if (check.isPermitAll()) { fireEventOnAuthZSuccess(check, null); 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 10ad1effe0416..657be48853e07 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 @@ -1,5 +1,6 @@ package io.quarkus.resteasy.runtime; +import static io.quarkus.resteasy.runtime.EagerSecurityFilter.SKIP_DEFAULT_CHECK; import static io.quarkus.security.spi.runtime.SecurityHandlerConstants.EXECUTED; import static io.quarkus.security.spi.runtime.SecurityHandlerConstants.SECURITY_HANDLER; @@ -18,6 +19,7 @@ import io.quarkus.security.PermissionsAllowed; import io.quarkus.security.spi.runtime.AuthorizationController; import io.quarkus.vertx.http.runtime.CurrentVertxRequest; +import io.vertx.ext.web.RoutingContext; /** * Security checks for RBAC annotations on endpoints are done by the {@link EagerSecurityFilter}, this interceptor @@ -36,9 +38,15 @@ public abstract class StandardSecurityCheckInterceptor { public Object intercept(InvocationContext ic) throws Exception { // 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())) { + RoutingContext routingContext = currentVertxRequest.getCurrent(); + if (controller.isAuthorizationEnabled() && routingContext != null) { + Method method = routingContext.get(EagerSecurityFilter.class.getName()); + if (method == null) { + // if this interceptor is run on resource method it means this is parent method for subresource + // otherwise it would already be secured, therefore security check is applied and default JAX-RS + // security needs to be skipped (for default security is only applied on unsecured requests) + routingContext.put(SKIP_DEFAULT_CHECK, true); + } else if (method.equals(ic.getMethod())) { ic.getContextData().put(SECURITY_HANDLER, EXECUTED); } } diff --git a/extensions/security/spi/src/main/java/io/quarkus/security/spi/AdditionalSecuredMethodsBuildItem.java b/extensions/security/spi/src/main/java/io/quarkus/security/spi/AdditionalSecuredMethodsBuildItem.java index 244f69b17dbda..4ffbfa638cd1a 100644 --- a/extensions/security/spi/src/main/java/io/quarkus/security/spi/AdditionalSecuredMethodsBuildItem.java +++ b/extensions/security/spi/src/main/java/io/quarkus/security/spi/AdditionalSecuredMethodsBuildItem.java @@ -12,7 +12,11 @@ /** * Contains methods that need to have {@link jakarta.annotation.security.DenyAll} or * {@link jakarta.annotation.security.RolesAllowed}. + * + * @deprecated as surplus to requirements; if this build item is of interest, please open issue in the Quarkus + * project so that we have it documented and tested */ +@Deprecated public final class AdditionalSecuredMethodsBuildItem extends MultiBuildItem { public final Collection additionalSecuredMethods;