Provide the ability to remove final flag from methods of CDI beans

We need security annotations to result in the creation of an interceptor
which is not possible when a method is final.
The solution we follow is to remove the final modifier from methods
that need intercepting.
This is now configurable with the default value true, meaning that
Arc will remove the final flag. If the value is set to false
Arc throws an exception at build time.

Fixes: #5051

extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ArcConfig.java

@@ -50,6 +50,16 @@ public class ArcConfig {
     @ConfigItem(defaultValue = "true")
     public boolean autoInjectFields;
 
+    /**
+     * If set to true, Arc will transform the bytecode of beans containing methods that need to be proxyable
+     * but have been declared as final. The transformation is simply a matter of removing final.
+     * This ensures that a proxy can be created properly.
+     * If the value is set to false, then an exception is thrown at build time indicating
+     * that a proxy could not be created because a method was final.
+     */
+    @ConfigItem(defaultValue = "true")
+    public boolean removeFinalForProxyableMethods;
+
     public final boolean isRemoveUnusedBeansFieldValid() {
         return ALLOWED_REMOVE_UNUSED_BEANS_VALUES.contains(removeUnusedBeans.toLowerCase());
     }

extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ArcProcessor.java

@@ -9,6 +9,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.function.Consumer;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
@@ -33,6 +34,7 @@
 import io.quarkus.arc.processor.BeanInfo;
 import io.quarkus.arc.processor.BeanProcessor;
 import io.quarkus.arc.processor.BuiltinScope;
+import io.quarkus.arc.processor.BytecodeTransformer;
 import io.quarkus.arc.processor.ContextConfigurator;
 import io.quarkus.arc.processor.ContextRegistrar;
 import io.quarkus.arc.processor.ReflectionRegistration;
@@ -47,6 +49,7 @@
 import io.quarkus.deployment.annotations.Record;
 import io.quarkus.deployment.builditem.ApplicationArchivesBuildItem;
 import io.quarkus.deployment.builditem.ApplicationClassPredicateBuildItem;
+import io.quarkus.deployment.builditem.BytecodeTransformerBuildItem;
 import io.quarkus.deployment.builditem.ExecutorBuildItem;
 import io.quarkus.deployment.builditem.FeatureBuildItem;
 import io.quarkus.deployment.builditem.GeneratedClassBuildItem;
@@ -214,6 +217,7 @@ public boolean test(BeanInfo bean) {
                 }
             });
         }
+        builder.setRemoveFinalFromProxyableMethods(arcConfig.removeFinalForProxyableMethods);
 
         BeanProcessor beanProcessor = builder.build();
         ContextRegistrar.RegistrationContext context = beanProcessor.registerCustomContexts();
@@ -239,7 +243,8 @@ public BeanRegistrationPhaseBuildItem registerBeans(ContextRegistrationPhaseBuil
     // PHASE 3 - initialize and validate the bean deployment
     @BuildStep
     public ValidationPhaseBuildItem validate(BeanRegistrationPhaseBuildItem beanRegistrationPhase,
-            List<BeanConfiguratorBuildItem> beanConfigurators) {
+            List<BeanConfiguratorBuildItem> beanConfigurators,
+            BuildProducer<BytecodeTransformerBuildItem> bytecodeTransformer) {
 
         for (BeanConfiguratorBuildItem beanConfigurator : beanConfigurators) {
             for (BeanConfigurator<?> value : beanConfigurator.getValues()) {
@@ -248,7 +253,12 @@ public ValidationPhaseBuildItem validate(BeanRegistrationPhaseBuildItem beanRegi
             }
         }
 
-        beanRegistrationPhase.getBeanProcessor().initialize();
+        beanRegistrationPhase.getBeanProcessor().initialize(new Consumer<BytecodeTransformer>() {
+            @Override
+            public void accept(BytecodeTransformer t) {
+                bytecodeTransformer.produce(new BytecodeTransformerBuildItem(t.getClassToTransform(), t.getVisitorFunction()));
+            }
+        });
         return new ValidationPhaseBuildItem(beanRegistrationPhase.getBeanProcessor().validate(),
                 beanRegistrationPhase.getBeanProcessor());
     }
@@ -364,4 +374,4 @@ public boolean test(T t) {
             return false;
         }
     }
-}
+}

extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/BeanWithSecuredFinalMethod.java

@@ -0,0 +1,19 @@
+package io.quarkus.security.test.cdi;
+
+import javax.annotation.security.DenyAll;
+import javax.annotation.security.RolesAllowed;
+import javax.inject.Singleton;
+
+@Singleton
+public class BeanWithSecuredFinalMethod {
+
+    @RolesAllowed("admin")
+    public final String securedMethod() {
+        return "accessibleForAdminOnly";
+    }
+
+    @DenyAll
+    public final String otherSecuredMethod(String input) {
+        return "denied";
+    }
+}

extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/SecurityAnnotationOnFinalMethodTest.java

@@ -0,0 +1,45 @@
+package io.quarkus.security.test.cdi;
+
+import static io.quarkus.security.test.cdi.SecurityTestUtils.assertFailureFor;
+import static io.quarkus.security.test.cdi.SecurityTestUtils.assertSuccess;
+import static io.quarkus.security.test.utils.IdentityMock.ADMIN;
+import static io.quarkus.security.test.utils.IdentityMock.ANONYMOUS;
+import static io.quarkus.security.test.utils.IdentityMock.USER;
+
+import javax.inject.Inject;
+
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.security.ForbiddenException;
+import io.quarkus.security.UnauthorizedException;
+import io.quarkus.security.test.utils.AuthData;
+import io.quarkus.security.test.utils.IdentityMock;
+import io.quarkus.test.QuarkusUnitTest;
+
+public class SecurityAnnotationOnFinalMethodTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest config = new QuarkusUnitTest()
+            .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
+                    .addClasses(BeanWithSecuredFinalMethod.class, IdentityMock.class,
+                            AuthData.class, SecurityTestUtils.class));
+
+    @Inject
+    BeanWithSecuredFinalMethod bean;
+
+    @Test
+    public void shouldRestrictAccessToSpecificRole() {
+        assertFailureFor(() -> bean.securedMethod(), UnauthorizedException.class, ANONYMOUS);
+        assertSuccess(() -> bean.securedMethod(), "accessibleForAdminOnly", ADMIN);
+    }
+
+    @Test
+    public void shouldFailToAccessCompletely() {
+        assertFailureFor(() -> bean.otherSecuredMethod("whatever"), UnauthorizedException.class, ANONYMOUS);
+        assertFailureFor(() -> bean.otherSecuredMethod("whatever"), ForbiddenException.class, USER, ADMIN);
+    }
+
+}

extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/SecurityAnnotationOnFinalMethodWithDisableFinalRemovalTest.java

@@ -0,0 +1,39 @@
+package io.quarkus.security.test.cdi;
+
+import static org.junit.jupiter.api.Assertions.fail;
+
+import javax.enterprise.inject.spi.DeploymentException;
+import javax.inject.Inject;
+
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.asset.StringAsset;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.security.test.utils.AuthData;
+import io.quarkus.security.test.utils.IdentityMock;
+import io.quarkus.test.QuarkusUnitTest;
+
+public class SecurityAnnotationOnFinalMethodWithDisableFinalRemovalTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest config = new QuarkusUnitTest()
+            .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
+                    .addClasses(BeanWithSecuredFinalMethod.class, IdentityMock.class,
+                            AuthData.class, SecurityTestUtils.class)
+                    .addAsResource(new StringAsset(
+                            "quarkus.arc.remove-final-for-proxyable-methods=false"),
+                            "application.properties"))
+            .setExpectedException(DeploymentException.class);
+
+    @Inject
+    BeanWithSecuredFinalMethod bean;
+
+    @Test
+    public void test() {
+        // should never be executed since the application should not be built
+        fail();
+    }
+
+}

independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java

@@ -25,6 +25,7 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
@@ -88,11 +89,12 @@ public class BeanDeployment {
     private final Map<ScopeInfo, Function<MethodCreator, ResultHandle>> customContexts;
 
     private final Collection<BeanDefiningAnnotation> beanDefiningAnnotations;
+    private final boolean removeFinalForProxyableMethods;
 
     BeanDeployment(IndexView index, Collection<BeanDefiningAnnotation> additionalBeanDefiningAnnotations,
             List<AnnotationsTransformer> annotationTransformers) {
         this(index, additionalBeanDefiningAnnotations, annotationTransformers, Collections.emptyList(), Collections.emptyList(),
-                null, false, null, Collections.emptyMap(), Collections.emptyList());
+                null, false, null, Collections.emptyMap(), Collections.emptyList(), false);
     }
 
     BeanDeployment(IndexView index, Collection<BeanDefiningAnnotation> additionalBeanDefiningAnnotations,
@@ -101,7 +103,7 @@ public class BeanDeployment {
             Collection<DotName> resourceAnnotations,
             BuildContextImpl buildContext, boolean removeUnusedBeans, List<Predicate<BeanInfo>> unusedExclusions,
             Map<DotName, Collection<AnnotationInstance>> additionalStereotypes,
-            List<InterceptorBindingRegistrar> bindingRegistrars) {
+            List<InterceptorBindingRegistrar> bindingRegistrars, boolean removeFinalForProxyableMethods) {
         this.buildContext = buildContext;
         Set<BeanDefiningAnnotation> beanDefiningAnnotations = new HashSet<>();
         if (additionalBeanDefiningAnnotations != null) {
@@ -154,6 +156,7 @@ public class BeanDeployment {
 
         this.beanResolver = new BeanResolver(this);
         this.interceptorResolver = new InterceptorResolver(this);
+        this.removeFinalForProxyableMethods = removeFinalForProxyableMethods;
     }
 
     ContextRegistrar.RegistrationContext registerCustomContexts(List<ContextRegistrar> contextRegistrars) {
@@ -203,19 +206,19 @@ BeanRegistrar.RegistrationContext registerBeans(List<BeanRegistrar> beanRegistra
         return registerSyntheticBeans(beanRegistrars, buildContext);
     }
 
-    void init() {
+    void init(Consumer<BytecodeTransformer> bytecodeTransformerConsumer) {
         long start = System.currentTimeMillis();
 
         // Collect dependency resolution errors
         List<Throwable> errors = new ArrayList<>();
         for (BeanInfo bean : beans) {
-            bean.init(errors);
+            bean.init(errors, bytecodeTransformerConsumer, removeFinalForProxyableMethods);
         }
         for (ObserverInfo observer : observers) {
             observer.init(errors);
         }
         for (InterceptorInfo interceptor : interceptors) {
-            interceptor.init(errors);
+            interceptor.init(errors, bytecodeTransformerConsumer, removeFinalForProxyableMethods);
         }
         processErrors(errors);
 

independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java

@@ -398,7 +398,8 @@ void validate(List<Throwable> errors, List<BeanDeploymentValidator> validators)
         }
     }
 
-    void init(List<Throwable> errors) {
+    void init(List<Throwable> errors, Consumer<BytecodeTransformer> bytecodeTransformerConsumer,
+            boolean removeFinalForProxyableMethods) {
         for (Injection injection : injections) {
             for (InjectionPointInfo injectionPoint : injection.injectionPoints) {
                 Beans.resolveInjectionPoint(beanDeployment, this, injectionPoint, errors);
@@ -407,7 +408,7 @@ void init(List<Throwable> errors) {
         if (disposer != null) {
             disposer.init(errors);
         }
-        interceptedMethods.putAll(initInterceptedMethods(errors));
+        interceptedMethods.putAll(initInterceptedMethods(errors, bytecodeTransformerConsumer, removeFinalForProxyableMethods));
         if (errors.isEmpty()) {
             lifecycleInterceptors.putAll(initLifecycleInterceptors());
         }
@@ -425,7 +426,8 @@ protected String getType() {
         }
     }
 
-    private Map<MethodInfo, InterceptionInfo> initInterceptedMethods(List<Throwable> errors) {
+    private Map<MethodInfo, InterceptionInfo> initInterceptedMethods(List<Throwable> errors,
+            Consumer<BytecodeTransformer> bytecodeTransformerConsumer, boolean removeFinalForProxyableMethods) {
         if (!isInterceptor() && isClassBean()) {
             Map<MethodInfo, InterceptionInfo> interceptedMethods = new HashMap<>();
             Map<MethodKey, Set<AnnotationInstance>> candidates = new HashMap<>();
@@ -439,7 +441,7 @@ private Map<MethodInfo, InterceptionInfo> initInterceptedMethods(List<Throwable>
             }
 
             Set<MethodInfo> finalMethods = Methods.addInterceptedMethodCandidates(beanDeployment, target.get().asClass(),
-                    candidates, classLevelBindings);
+                    candidates, classLevelBindings, bytecodeTransformerConsumer, removeFinalForProxyableMethods);
             if (!finalMethods.isEmpty()) {
                 errors.add(new DeploymentException(String.format(
                         "Bean %s has a bound interceptor and must not declare final methods:\n\t- %s", getBeanClass(),

independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanProcessor.java

@@ -15,6 +15,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Consumer;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import org.jboss.jandex.AnnotationInstance;
@@ -29,7 +30,7 @@
  * <ol>
  * <li>{@link #registerCustomContexts()}</li>
  * <li>{@link #registerBeans()}</li>
- * <li>{@link #initialize()}</li>
+ * <li>{@link #initialize(Consumer)}</li>
  * <li>{@link #validate()}</li>
  * <li>{@link #processValidationErrors(io.quarkus.arc.processor.BeanDeploymentValidator.ValidationContext)}</li>
  * <li>{@link #generateResources(ReflectionRegistration)}</li>
@@ -73,7 +74,8 @@ private BeanProcessor(String name, IndexView index, Collection<BeanDefiningAnnot
             List<BeanDeploymentValidator> beanDeploymentValidators, Predicate<DotName> applicationClassPredicate,
             boolean unusedBeansRemovalEnabled,
             List<Predicate<BeanInfo>> unusedExclusions, Map<DotName, Collection<AnnotationInstance>> additionalStereotypes,
-            List<InterceptorBindingRegistrar> interceptorBindingRegistrars) {
+            List<InterceptorBindingRegistrar> interceptorBindingRegistrars,
+            boolean removeFinalForProxyableMethods) {
         this.reflectionRegistration = reflectionRegistration;
         this.applicationClassPredicate = applicationClassPredicate;
         this.name = name;
@@ -91,7 +93,7 @@ private BeanProcessor(String name, IndexView index, Collection<BeanDefiningAnnot
                 initAndSort(annotationTransformers, buildContext),
                 initAndSort(injectionPointsTransformers, buildContext), resourceAnnotations, buildContext,
                 unusedBeansRemovalEnabled, unusedExclusions,
-                additionalStereotypes, interceptorBindingRegistrars);
+                additionalStereotypes, interceptorBindingRegistrars, removeFinalForProxyableMethods);
     }
 
     public ContextRegistrar.RegistrationContext registerCustomContexts() {
@@ -102,8 +104,8 @@ public BeanRegistrar.RegistrationContext registerBeans() {
         return beanDeployment.registerBeans(beanRegistrars);
     }
 
-    public void initialize() {
-        beanDeployment.init();
+    public void initialize(Consumer<BytecodeTransformer> bytecodeTransformerConsumer) {
+        beanDeployment.init(bytecodeTransformerConsumer);
     }
 
     public BeanDeploymentValidator.ValidationContext validate() {
@@ -201,7 +203,12 @@ public BeanDeployment getBeanDeployment() {
     public BeanDeployment process() throws IOException {
         registerCustomContexts();
         registerBeans();
-        initialize();
+        initialize(new Consumer<BytecodeTransformer>() {
+            @Override
+            public void accept(BytecodeTransformer transformer) {
+
+            }
+        });
         ValidationContext validationContext = validate();
         processValidationErrors(validationContext);
         generateResources(null);
@@ -242,6 +249,8 @@ public boolean test(DotName dotName) {
             }
         };
 
+        private boolean removeFinalForProxyableMethods;
+
         public Builder setName(String name) {
             this.name = name;
             return this;
@@ -353,12 +362,17 @@ public Builder addRemovalExclusion(Predicate<BeanInfo> exclusion) {
             return this;
         }
 
+        public Builder setRemoveFinalFromProxyableMethods(boolean removeFinalForProxyableMethods) {
+            this.removeFinalForProxyableMethods = removeFinalForProxyableMethods;
+            return this;
+        }
+
         public BeanProcessor build() {
             return new BeanProcessor(name, index, additionalBeanDefiningAnnotations, output, sharedAnnotationLiterals,
                     reflectionRegistration, annotationTransformers, injectionPointTransformers, resourceAnnotations,
                     beanRegistrars, contextRegistrars, beanDeploymentValidators,
                     applicationClassPredicate, removeUnusedBeans, removalExclusions, additionalStereotypes,
-                    additionalInterceptorBindingRegistrars);
+                    additionalInterceptorBindingRegistrars, removeFinalForProxyableMethods);
         }
 
     }

independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BytecodeTransformer.java

@@ -0,0 +1,24 @@
+package io.quarkus.arc.processor;
+
+import java.util.function.BiFunction;
+import org.objectweb.asm.ClassVisitor;
+
+public class BytecodeTransformer {
+
+    final String classToTransform;
+    final BiFunction<String, ClassVisitor, ClassVisitor> visitorFunction;
+
+    public BytecodeTransformer(String classToTransform,
+            BiFunction<String, ClassVisitor, ClassVisitor> visitorFunction) {
+        this.classToTransform = classToTransform;
+        this.visitorFunction = visitorFunction;
+    }
+
+    public String getClassToTransform() {
+        return classToTransform;
+    }
+
+    public BiFunction<String, ClassVisitor, ClassVisitor> getVisitorFunction() {
+        return visitorFunction;
+    }
+}