Fix JAX-RS default security checks for inherited / transformed endpoints

extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/RestPathAnnotationProcessor.java

@@ -182,7 +182,7 @@ static Optional<AnnotationInstance> searchPathAnnotationOnInterfaces(CombinedInd
      * @param resultAcc accumulator for tail-recursion
      * @return Collection of all interfaces und their parents. Never null.
      */
-    private static Collection<ClassInfo> getAllClassInterfaces(
+    static Collection<ClassInfo> getAllClassInterfaces(
             CombinedIndexBuildItem index,
             Collection<ClassInfo> classInfos,
             List<ClassInfo> resultAcc) {

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<ClassInfo> interfaces = getAllClassInterfaces(index, List.of(classInfo), new ArrayList<>());
+                if (!interfaces.isEmpty()) {
+                    final List<MethodInfo> 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<DotName> 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<MethodInfo> 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<MethodInfo> methods) {
+        if (!hasSecurityAnnotation(classInfo)) {
+            for (MethodInfo methodInfo : classInfo.methods()) {
+                if (isRestEndpointMethod(index, methodInfo) && !hasSecurityAnnotation(methodInfo)) {
+                    methods.add(methodInfo);
+                }
+            }
+        }
+    }
+
     /**
      * Install the JAX-RS security provider.
      */

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";

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"));

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";

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";
+    }
+
+}

extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredResource.java

@@ -10,13 +10,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

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";
+    }
+
+}

extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/EagerSecurityFilter.java

@@ -1,15 +1,11 @@
 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;
@@ -18,19 +14,13 @@
 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;
 
@@ -46,7 +36,6 @@ public void accept(RoutingContext routingContext) {
     };
     private final Map<MethodDescription, Consumer<RoutingContext>> cache = new HashMap<>();
     private final EagerSecurityInterceptorStorage interceptorStorage;
-    private final SecurityEventHelper<AuthorizationSuccessEvent, AuthorizationFailureEvent> eventHelper;
 
     @Context
     ResourceInfo resourceInfo;
@@ -66,11 +55,6 @@ public void accept(RoutingContext routingContext) {
     public EagerSecurityFilter() {
         var interceptorStorageHandle = Arc.container().instance(EagerSecurityInterceptorStorage.class);
         this.interceptorStorage = interceptorStorageHandle.isAvailable() ? interceptorStorageHandle.get() : null;
-        Event<Object> 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
@@ -87,50 +71,22 @@ public void filter(ContainerRequestContext requestContext) throws IOException {
     private void applySecurityChecks(MethodDescription description) {
         SecurityCheck check = securityCheckStorage.getSecurityCheck(description);
         if (check != null) {
-            if (check.isPermitAll()) {
-                fireEventOnAuthZSuccess(check, null);
-            } else {
+            if (!check.isPermitAll()) {
                 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());
+                check.apply(identityAssociation.getIdentity(), description, null);
             }
             // 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)));
-    }
-
-    private void fireEventOnAuthZSuccess(SecurityCheck check, SecurityIdentity securityIdentity) {
-        if (eventHelper.fireEventOnSuccess()) {
-            eventHelper.fireSuccessEvent(new AuthorizationSuccessEvent(securityIdentity,
-                    check.getClass().getName(), Map.of(RoutingContext.class.getName(), routingContext)));
-        }
-    }
-
     private void applyEagerSecurityInterceptors(MethodDescription description) {
         var interceptor = cache.get(description);
         if (interceptor != NULL_SENTINEL) {