RESTEasy Reactive - prevent repeating of standard security checks

fix: quarkusio#26536

extensions/resteasy-reactive/quarkus-resteasy-reactive-common/deployment/src/main/java/io/quarkus/resteasy/reactive/common/deployment/ResteasyReactiveCommonProcessor.java

@@ -90,7 +90,7 @@ void setUpDenyAllJaxRs(CombinedIndexBuildItem index,
                 }
             }
 
-            additionalSecuredClasses.produce(new AdditionalSecuredClassesBuildItem(classes));
+            additionalSecuredClasses.produce(new AdditionalSecuredClassesBuildItem(classes, false));
         } else if (defaultRolesAllowedConfig.isPresent() && resteasyDeployment.isPresent()) {
 
             final List<ClassInfo> classes = new ArrayList<>();
@@ -102,7 +102,7 @@ void setUpDenyAllJaxRs(CombinedIndexBuildItem index,
                 }
             }
             additionalSecuredClasses
-                    .produce(new AdditionalSecuredClassesBuildItem(classes, defaultRolesAllowedConfig));
+                    .produce(new AdditionalSecuredClassesBuildItem(classes, defaultRolesAllowedConfig, false));
         }
     }
 

extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java

@@ -4,6 +4,7 @@
 import static io.quarkus.resteasy.reactive.common.deployment.QuarkusResteasyReactiveDotNames.HTTP_SERVER_RESPONSE;
 import static io.quarkus.resteasy.reactive.common.deployment.QuarkusResteasyReactiveDotNames.ROUTING_CONTEXT;
 import static java.util.stream.Collectors.toList;
+import static org.jboss.resteasy.reactive.common.processor.EndpointIndexer.findEndpoints;
 import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.DATE_FORMAT;
 
 import java.io.File;
@@ -168,6 +169,7 @@
 import io.quarkus.security.AuthenticationCompletionException;
 import io.quarkus.security.AuthenticationRedirectException;
 import io.quarkus.security.ForbiddenException;
+import io.quarkus.security.spi.runtime.SecurityCheckStorage;
 import io.quarkus.vertx.http.deployment.RouteBuildItem;
 import io.quarkus.vertx.http.runtime.HttpBuildTimeConfig;
 import io.quarkus.vertx.http.runtime.VertxHttpRecorder;
@@ -324,12 +326,17 @@ void registerCustomExceptionMappers(BuildProducer<CustomExceptionMapperBuildItem
     @BuildStep
     public void unremovableBeans(Optional<ResourceScanningResultBuildItem> resourceScanningResultBuildItem,
             BuildProducer<UnremovableBeanBuildItem> unremovableBeans) {
+
         if (!resourceScanningResultBuildItem.isPresent()) {
             return;
         }
         Set<String> beanParams = resourceScanningResultBuildItem.get().getResult()
                 .getBeanParams();
         unremovableBeans.produce(UnremovableBeanBuildItem.beanClassNames(beanParams.toArray(EMPTY_STRING_ARRAY)));
+
+        // Necessary as the Quarkus Security doesn't need the SecurityCheckStorage bean in cases
+        // when only security checks are for endpoints and these are handled exclusively by EagerSecurityHandler
+        unremovableBeans.produce(UnremovableBeanBuildItem.beanTypes(SecurityCheckStorage.class));
     }
 
     @BuildStep
@@ -1061,6 +1068,96 @@ public List<HandlerChainCustomizer> scan(MethodInfo method, ClassInfo actualEndp
         });
     }
 
+    /**
+     * Prevent repeating {@link io.quarkus.security.spi.runtime.SecurityCheck}s in the Quarkus Security on methods
+     * checked by {@link EagerSecurityHandler}.
+     */
+    @BuildStep
+    io.quarkus.arc.deployment.AnnotationsTransformerBuildItem removeStandardSecurityAnnotationsFromResources(
+            Capabilities capabilities,
+            Optional<ResourceScanningResultBuildItem> resourceScanningResultBuildItem,
+            ApplicationResultBuildItem applicationResultBuildItem, BeanArchiveIndexBuildItem beanArchiveIndexBuildItem) {
+
+        if (!capabilities.isPresent(Capability.SECURITY) || resourceScanningResultBuildItem.isEmpty()) {
+            return null;
+        }
+
+        final IndexView index = beanArchiveIndexBuildItem.getIndex();
+
+        // collect all endpoints
+        final Set<MethodInfo> endpoints = findEndpoints(resourceScanningResultBuildItem.get().getResult(), index,
+                applicationResultBuildItem.getResult());
+
+        final Set<DotName> visitedClasses = new HashSet<>();
+        final Set<DotName> annotatedClasses = new HashSet<>();
+        final Set<MethodDescriptor> annotatedMethods = new HashSet<>();
+        for (MethodInfo endpoint : endpoints) {
+
+            // collect endpoints annotated with a security annotation
+            if (SecurityTransformerUtils.hasStandardSecurityAnnotation(endpoint)) {
+                annotatedMethods.add(MethodDescriptor.of(endpoint));
+            }
+
+            // collect resource (super) classes annotated with a security annotation
+            ClassInfo c = endpoint.declaringClass();
+            while (c.superName() != null) {
+                if (!visitedClasses.contains(c.name())) {
+                    visitedClasses.add(c.name());
+                    if (SecurityTransformerUtils.hasStandardSecurityAnnotation(c)) {
+                        annotatedClasses.add(c.name());
+                    }
+                }
+                c = index.getClassByName(c.superName());
+            }
+        }
+
+        final boolean appliesToClasses = !annotatedClasses.isEmpty();
+        final boolean appliesToMethods = !annotatedMethods.isEmpty();
+        final Predicate<AnnotationTarget.Kind> appliesTo;
+        if (appliesToClasses && appliesToMethods) {
+            appliesTo = Predicate.<AnnotationTarget.Kind> isEqual(AnnotationTarget.Kind.METHOD)
+                    .or(Predicate.isEqual(AnnotationTarget.Kind.CLASS));
+        } else if (appliesToMethods) {
+            appliesTo = Predicate.isEqual(AnnotationTarget.Kind.METHOD);
+        } else if (appliesToClasses) {
+            appliesTo = Predicate.isEqual(AnnotationTarget.Kind.CLASS);
+        } else {
+            // found no security annotations
+            return null;
+        }
+
+        final Predicate<AnnotationInstance> isSecurityAnnotation = new Predicate<AnnotationInstance>() {
+            @Override
+            public boolean test(AnnotationInstance annotationInstance) {
+                return SecurityTransformerUtils.SECURITY_BINDINGS.contains(annotationInstance.name());
+            }
+        };
+        // remove security annotations
+        return new io.quarkus.arc.deployment.AnnotationsTransformerBuildItem(
+                new io.quarkus.arc.processor.AnnotationsTransformer() {
+
+                    @Override
+                    public boolean appliesTo(AnnotationTarget.Kind kind) {
+                        return appliesTo.test(kind);
+                    }
+
+                    @Override
+                    public void transform(TransformationContext context) {
+                        if (appliesToMethods && context.getTarget().kind() == AnnotationTarget.Kind.METHOD) {
+                            if (annotatedMethods.contains(MethodDescriptor.of(context.getTarget().asMethod()))) {
+                                removeSecurityAnnotations(context);
+                            }
+                        } else if (annotatedClasses.contains(context.getTarget().asClass().name())) {
+                            removeSecurityAnnotations(context);
+                        }
+                    }
+
+                    private void removeSecurityAnnotations(TransformationContext context) {
+                        context.transform().remove(isSecurityAnnotation).done();
+                    }
+                });
+    }
+
     /**
      * This results in adding {@link AllWriteableMarker} to user provided {@link MessageBodyWriter} classes
      * that handle every class

extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/AssureNoDuplicateSecurityChecksInterceptor.java

@@ -0,0 +1,23 @@
+package io.quarkus.resteasy.reactive.server.test.security;
+
+import javax.annotation.Priority;
+import javax.annotation.security.RolesAllowed;
+import javax.interceptor.AroundInvoke;
+import javax.interceptor.Interceptor;
+import javax.interceptor.InvocationContext;
+
+import org.junit.jupiter.api.Assertions;
+
+import io.quarkus.security.runtime.interceptor.SecurityHandler;
+
+@Interceptor
+@RolesAllowed("")
+@Priority(Interceptor.Priority.PLATFORM_AFTER)
+public class AssureNoDuplicateSecurityChecksInterceptor {
+
+    @AroundInvoke
+    public Object intercept(InvocationContext ic) throws Exception {
+        Assertions.assertNull(ic.getContextData().get(SecurityHandler.class.getName()));
+        return ic.proceed();
+    }
+}

extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DefaultRolesAllowedJaxRsTest.java

@@ -19,7 +19,7 @@ public class DefaultRolesAllowedJaxRsTest {
                     .addClasses(PermitAllResource.class, UnsecuredResource.class,
                             TestIdentityProvider.class,
                             TestIdentityController.class,
-                            UnsecuredSubResource.class)
+                            UnsecuredSubResource.class, AssureNoDuplicateSecurityChecksInterceptor.class)
                     .addAsResource(new StringAsset("quarkus.security.jaxrs.default-roles-allowed=admin\n"),
                             "application.properties"));
 

extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DefaultRolesAllowedStarJaxRsTest.java

@@ -19,7 +19,7 @@ public class DefaultRolesAllowedStarJaxRsTest {
                     .addClasses(PermitAllResource.class, UnsecuredResource.class,
                             TestIdentityProvider.class,
                             TestIdentityController.class,
-                            UnsecuredSubResource.class)
+                            UnsecuredSubResource.class, AssureNoDuplicateSecurityChecksInterceptor.class)
                     .addAsResource(new StringAsset("quarkus.security.jaxrs.default-roles-allowed = **\n"),
                             "application.properties"));
 

extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/RolesAllowedJaxRsTestCase.java

@@ -32,7 +32,7 @@ public class RolesAllowedJaxRsTestCase {
                             SerializationEntity.class, SerializationRolesResource.class,
                             TestIdentityProvider.class,
                             TestIdentityController.class,
-                            UnsecuredSubResource.class));
+                            UnsecuredSubResource.class, AssureNoDuplicateSecurityChecksInterceptor.class));
 
     @BeforeAll
     public static void setupUsers() {
@@ -103,4 +103,5 @@ public SerializationEntity readFrom(Class<SerializationEntity> type, Type generi
             return entity;
         }
     }
+
 }

extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/security/EagerSecurityHandler.java

@@ -80,7 +80,7 @@ public void handle(ResteasyReactiveRequestContext requestContext) throws Excepti
             Uni<SecurityIdentity> deferredIdentity = getCurrentIdentityAssociation().get().getDeferredIdentity();
 
             // if proactive auth is disabled, then accessing SecurityIdentity is a blocking operation for synchronous methods
-            // setting identity here will enable SecurityInterceptors registered in Quarkus Security Deployment to run checks
+            // setting identity here will allow users to access SecurityIdentity in a synchronous manner
             if (isProactiveAuthDisabled && lazyMethod.isNonBlocking) {
                 deferredIdentity = deferredIdentity.call(securityIdentity -> {
                     if (securityIdentity != null) {

extensions/security/deployment/src/main/java/io/quarkus/security/deployment/DenyingUnannotatedTransformer.java

@@ -3,9 +3,11 @@
 import static io.quarkus.security.deployment.SecurityTransformerUtils.DENY_ALL;
 
 import java.util.List;
+import java.util.Set;
 
 import org.jboss.jandex.AnnotationTarget;
 import org.jboss.jandex.ClassInfo;
+import org.jboss.jandex.DotName;
 import org.jboss.jandex.MethodInfo;
 
 import io.quarkus.arc.processor.AnnotationsTransformer;
@@ -15,6 +17,12 @@
  */
 public class DenyingUnannotatedTransformer implements AnnotationsTransformer {
 
+    private final Set<DotName> classesThatShouldNotBeAnnotated;
+
+    public DenyingUnannotatedTransformer(Set<DotName> classesThatShouldNotBeAnnotated) {
+        this.classesThatShouldNotBeAnnotated = classesThatShouldNotBeAnnotated;
+    }
+
     @Override
     public boolean appliesTo(AnnotationTarget.Kind kind) {
         return kind == AnnotationTarget.Kind.CLASS;
@@ -23,6 +31,10 @@ public boolean appliesTo(AnnotationTarget.Kind kind) {
     @Override
     public void transform(TransformationContext transformationContext) {
         ClassInfo classInfo = transformationContext.getTarget().asClass();
+        if (classesThatShouldNotBeAnnotated.contains(classInfo.name())) {
+            return;
+        }
+
         List<MethodInfo> methods = classInfo.methods();
         if (!SecurityTransformerUtils.hasStandardSecurityAnnotation(classInfo)
                 && methods.stream().anyMatch(SecurityTransformerUtils::hasStandardSecurityAnnotation)) {

extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java

@@ -424,15 +424,23 @@ void registerSecurityInterceptors(BuildProducer<InterceptorBindingRegistrarBuild
     void transformSecurityAnnotations(BuildProducer<AnnotationsTransformerBuildItem> transformers,
             List<AdditionalSecuredClassesBuildItem> additionalSecuredClasses,
             SecurityBuildTimeConfig config) {
-        if (config.denyUnannotated) {
-            transformers.produce(new AnnotationsTransformerBuildItem(new DenyingUnannotatedTransformer()));
-        }
+        final Set<DotName> classesThatShouldNotBeAnnotated = new HashSet<>();
         if (!additionalSecuredClasses.isEmpty()) {
             for (AdditionalSecuredClassesBuildItem securedClasses : additionalSecuredClasses) {
+
                 Set<String> additionalSecured = new HashSet<>();
                 for (ClassInfo additionalSecuredClass : securedClasses.additionalSecuredClasses) {
-                    additionalSecured.add(additionalSecuredClass.name().toString());
+                    if (securedClasses.addAnnotation) {
+                        additionalSecured.add(additionalSecuredClass.name().toString());
+                    } else {
+                        classesThatShouldNotBeAnnotated.add(additionalSecuredClass.name());
+                    }
                 }
+
+                if (!securedClasses.addAnnotation) {
+                    continue;
+                }
+
                 if (securedClasses.rolesAllowed.isPresent()) {
                     transformers.produce(
                             new AnnotationsTransformerBuildItem(new AdditionalRolesAllowedTransformer(additionalSecured,
@@ -444,6 +452,10 @@ void transformSecurityAnnotations(BuildProducer<AnnotationsTransformerBuildItem>
                 }
             }
         }
+        if (config.denyUnannotated) {
+            transformers.produce(
+                    new AnnotationsTransformerBuildItem(new DenyingUnannotatedTransformer(classesThatShouldNotBeAnnotated)));
+        }
     }
 
     @BuildStep

extensions/security/spi/src/main/java/io/quarkus/security/spi/AdditionalSecuredClassesBuildItem.java

@@ -14,17 +14,30 @@
  */
 public final class AdditionalSecuredClassesBuildItem extends MultiBuildItem {
 
+    /**
+     * If true, each secured class is going to be annotated with a security annotation.
+     */
+    public final boolean addAnnotation;
     public final Collection<ClassInfo> additionalSecuredClasses;
     public final Optional<List<String>> rolesAllowed;
 
+    public AdditionalSecuredClassesBuildItem(Collection<ClassInfo> additionalSecuredClasses, boolean addAnnotation) {
+        this(additionalSecuredClasses, Optional.empty(), addAnnotation);
+    }
+
     public AdditionalSecuredClassesBuildItem(Collection<ClassInfo> additionalSecuredClasses) {
-        this.additionalSecuredClasses = Collections.unmodifiableCollection(additionalSecuredClasses);
-        rolesAllowed = Optional.empty();
+        this(additionalSecuredClasses, true);
     }
 
     public AdditionalSecuredClassesBuildItem(Collection<ClassInfo> additionalSecuredClasses,
             Optional<List<String>> rolesAllowed) {
+        this(additionalSecuredClasses, rolesAllowed, true);
+    }
+
+    public AdditionalSecuredClassesBuildItem(Collection<ClassInfo> additionalSecuredClasses,
+            Optional<List<String>> rolesAllowed, boolean addAnnotation) {
         this.additionalSecuredClasses = Collections.unmodifiableCollection(additionalSecuredClasses);
         this.rolesAllowed = rolesAllowed;
+        this.addAnnotation = addAnnotation;
     }
 }