Merge pull request #29935 from michalvavrik/feature/roles-allowed-con…

…fig-property-expression-lang

Support Smallrye Config property expression expansion for @RolesAllowed value

docs/src/main/asciidoc/security-authorization-of-web-endpoints-reference.adoc

@@ -273,6 +273,75 @@ public class SubjectExposingResource {
 
 CAUTION: Please refer to the xref:security-built-in-authentication-support-concept.adoc#proactive-authentication[Proactive Authentication] section of the Built-In Authentication Support guide if you plan to use standard security annotations on IO thread.
 
+The `@RolesAllowed` annotation value supports <<config-reference#property-expressions,Property Expressions>> including default values and nested Property Expressions.
+Configuration properties used with the annotation are resolved at runtime.
+
+[source,properties]
+----
+admin=Administrator
+tester.group=Software
+tester.role=Tester
+%prod.secured=User
+%dev.secured=**
+----
+
+[source,java]
+----
+import java.security.Principal;
+
+import javax.annotation.security.DenyAll;
+import javax.annotation.security.PermitAll;
+import javax.annotation.security.RolesAllowed;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.core.Context;
+import javax.ws.rs.core.SecurityContext;
+
+@Path("subject")
+public class SubjectExposingResource {
+
+    @GET
+    @Path("admin")
+    @RolesAllowed("${admin}") <1>
+    public String getSubjectSecuredAdmin(@Context SecurityContext sec) {
+        Principal user = sec.getUserPrincipal();
+        String name = user != null ? user.getName() : "anonymous";
+        return name;
+    }
+
+    @GET
+    @Path("software-tester")
+    @RolesAllowed("${tester.group}-${tester.role}") <2>
+    public String getSubjectSoftwareTester(@Context SecurityContext sec) {
+        Principal user = sec.getUserPrincipal();
+        String name = user != null ? user.getName() : "anonymous";
+        return name;
+    }
+
+    @GET
+    @Path("user")
+    @RolesAllowed("${customer:User}") <3>
+    public String getSubjectUser(@Context SecurityContext sec) {
+        Principal user = sec.getUserPrincipal();
+        String name = user != null ? user.getName() : "anonymous";
+        return name;
+    }
+
+    @GET
+    @Path("secured")
+    @RolesAllowed("${secured}") <4>
+    public String getSubjectSecured(@Context SecurityContext sec) {
+        Principal user = sec.getUserPrincipal();
+        String name = user != null ? user.getName() : "anonymous";
+        return name;
+    }
+}
+----
+<1> The `@RolesAllowed` annotation value is set to the value of the `admin`.
+<2> This `/subject/software-tester` endpoint requires an authenticated user that has been granted the role "Software-Tester". It is possible to use multiple expressions in the role definition.
+<3> This `/subject/user` endpoint requires an authenticated user that has been granted the role "User" through the use of the `@RolesAllowed("${customer:User}")` annotation, as we did not set the configuration property `customer`.
+<4> This `/subject/secured` endpoint requires an authenticated user that has been granted the role `User` in production, but allows any authenticated user in development mode.
+
 == References
 
 * xref:security-overview-concept.adoc[Quarkus Security overview]

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

@@ -0,0 +1,44 @@
+package io.quarkus.resteasy.reactive.server.test.security;
+
+import org.hamcrest.Matchers;
+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.security.test.utils.TestIdentityController;
+import io.quarkus.security.test.utils.TestIdentityProvider;
+import io.quarkus.test.QuarkusUnitTest;
+import io.restassured.RestAssured;
+
+public class LazyAuthRolesAllowedConfigExpTestCase {
+
+    @RegisterExtension
+    static QuarkusUnitTest runner = new QuarkusUnitTest()
+            .withApplicationRoot((jar) -> jar
+                    .addClasses(RolesAllowedResource.class, UserResource.class,
+                            TestIdentityProvider.class,
+                            TestIdentityController.class,
+                            SecurityOverrideFilter.class)
+                    .addAsResource(new StringAsset("quarkus.http.auth.proactive=false\n" +
+                            "admin-config-property=admin\n"), "application.properties"));
+
+    @BeforeAll
+    public static void setupUsers() {
+        TestIdentityController.resetRoles()
+                .add("admin", "admin", "admin")
+                .add("user", "user", "user");
+    }
+
+    @Test
+    public void testRolesAllowedConfigExp() {
+        RestAssured.given()
+                .header("user", "admin")
+                .header("role", "admin")
+                .get("/roles/admin-config-exp")
+                .then()
+                .statusCode(200)
+                .body(Matchers.equalTo("admin-config-exp"));
+    }
+
+}

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

@@ -32,6 +32,13 @@ public String admin() {
         return "admin";
     }
 
+    @Path("/admin-config-exp")
+    @RolesAllowed("${admin-config-property:missing}")
+    @GET
+    public String adminConfigExp() {
+        return "admin-config-exp";
+    }
+
     @NonBlocking
     @Path("/admin/security-identity")
     @RolesAllowed("admin")

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

@@ -0,0 +1,14 @@
+package io.quarkus.security.deployment;
+
+import io.quarkus.builder.item.SimpleBuildItem;
+import io.quarkus.security.runtime.interceptor.check.SupplierRolesAllowedCheck;
+
+/**
+ * Marker build item that is used to indicate that there are {@link SupplierRolesAllowedCheck}s whose roles
+ * contains config expressions that should be resolved at runtime.
+ */
+public final class ConfigExpRolesAllowedSecurityCheckBuildItem extends SimpleBuildItem {
+
+    ConfigExpRolesAllowedSecurityCheckBuildItem() {
+    }
+}

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

@@ -14,12 +14,16 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.BiConsumer;
 import java.util.function.Function;
 import java.util.function.Predicate;
 
@@ -49,6 +53,7 @@
 import io.quarkus.deployment.builditem.GeneratedNativeImageClassBuildItem;
 import io.quarkus.deployment.builditem.LaunchModeBuildItem;
 import io.quarkus.deployment.builditem.NativeImageFeatureBuildItem;
+import io.quarkus.deployment.builditem.RunTimeConfigBuilderBuildItem;
 import io.quarkus.deployment.builditem.nativeimage.JPMSExportBuildItem;
 import io.quarkus.deployment.builditem.nativeimage.NativeImageSecurityProviderBuildItem;
 import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
@@ -64,6 +69,7 @@
 import io.quarkus.runtime.LaunchMode;
 import io.quarkus.runtime.RuntimeValue;
 import io.quarkus.security.runtime.IdentityProviderManagerCreator;
+import io.quarkus.security.runtime.QuarkusSecurityRolesAllowedConfigBuilder;
 import io.quarkus.security.runtime.SecurityBuildTimeConfig;
 import io.quarkus.security.runtime.SecurityCheckRecorder;
 import io.quarkus.security.runtime.SecurityIdentityAssociation;
@@ -473,8 +479,10 @@ void transformSecurityAnnotations(BuildProducer<AnnotationsTransformerBuildItem>
     @BuildStep
     @Record(ExecutionTime.STATIC_INIT)
     void gatherSecurityChecks(BuildProducer<SyntheticBeanBuildItem> syntheticBeans,
+            BuildProducer<ConfigExpRolesAllowedSecurityCheckBuildItem> configExpSecurityCheckProducer,
             BeanArchiveIndexBuildItem beanArchiveBuildItem,
             BuildProducer<ApplicationClassPredicateBuildItem> classPredicate,
+            BuildProducer<RunTimeConfigBuilderBuildItem> configBuilderProducer,
             List<AdditionalSecuredMethodsBuildItem> additionalSecuredMethods,
             SecurityCheckRecorder recorder,
             List<AdditionalSecurityCheckBuildItem> additionalSecurityChecks, SecurityBuildTimeConfig config) {
@@ -489,8 +497,8 @@ void gatherSecurityChecks(BuildProducer<SyntheticBeanBuildItem> syntheticBeans,
         }
 
         IndexView index = beanArchiveBuildItem.getIndex();
-        Map<MethodInfo, SecurityCheck> securityChecks = gatherSecurityAnnotations(
-                index, additionalSecured.values(), config.denyUnannotated, recorder);
+        Map<MethodInfo, SecurityCheck> securityChecks = gatherSecurityAnnotations(index, configExpSecurityCheckProducer,
+                additionalSecured.values(), config.denyUnannotated, recorder, configBuilderProducer);
         for (AdditionalSecurityCheckBuildItem additionalSecurityCheck : additionalSecurityChecks) {
             securityChecks.put(additionalSecurityCheck.getMethodInfo(),
                     additionalSecurityCheck.getSecurityCheck());
@@ -520,22 +528,38 @@ void gatherSecurityChecks(BuildProducer<SyntheticBeanBuildItem> syntheticBeans,
                         }).done());
     }
 
-    private Map<MethodInfo, SecurityCheck> gatherSecurityAnnotations(
-            IndexView index,
-            Collection<AdditionalSecured> additionalSecuredMethods, boolean denyUnannotated, SecurityCheckRecorder recorder) {
+    @BuildStep
+    @Record(ExecutionTime.RUNTIME_INIT)
+    public void resolveConfigExpressionRoles(Optional<ConfigExpRolesAllowedSecurityCheckBuildItem> configExpRolesChecks,
+            SecurityCheckRecorder recorder) {
+        if (configExpRolesChecks.isPresent()) {
+            // we created supplier security check for each role set with at least one config expression
+            // now we need to resolve config expression so that if there are any failures they happen when app starts
+            // rather than first time request is checked (which would be more likely to affect end user)
+            recorder.resolveRolesAllowedConfigExpRoles();
+        }
+    }
+
+    private Map<MethodInfo, SecurityCheck> gatherSecurityAnnotations(IndexView index,
+            BuildProducer<ConfigExpRolesAllowedSecurityCheckBuildItem> configExpSecurityCheckProducer,
+            Collection<AdditionalSecured> additionalSecuredMethods, boolean denyUnannotated, SecurityCheckRecorder recorder,
+            BuildProducer<RunTimeConfigBuilderBuildItem> configBuilderProducer) {
 
         Map<MethodInfo, AnnotationInstance> methodToInstanceCollector = new HashMap<>();
         Map<ClassInfo, AnnotationInstance> classAnnotations = new HashMap<>();
-        Map<MethodInfo, SecurityCheck> result = new HashMap<>(gatherSecurityAnnotations(
+        Map<MethodInfo, SecurityCheck> result = new HashMap<>();
+        gatherSecurityAnnotations(index, DotNames.PERMIT_ALL, methodToInstanceCollector, classAnnotations,
+                ((m, i) -> result.put(m, recorder.permitAll())));
+        gatherSecurityAnnotations(index, DotNames.AUTHENTICATED, methodToInstanceCollector, classAnnotations,
+                ((m, i) -> result.put(m, recorder.authenticated())));
+        gatherSecurityAnnotations(index, DENY_ALL, methodToInstanceCollector, classAnnotations,
+                ((m, i) -> result.put(m, recorder.denyAll())));
+
+        // here we just collect all methods annotated with @RolesAllowed
+        Map<MethodInfo, String[]> methodToRoles = new HashMap<>();
+        gatherSecurityAnnotations(
                 index, ROLES_ALLOWED, methodToInstanceCollector, classAnnotations,
-                (instance -> recorder.rolesAllowed(instance.value().asStringArray()))));
-        result.putAll(gatherSecurityAnnotations(index, DotNames.PERMIT_ALL, methodToInstanceCollector, classAnnotations,
-                (instance -> recorder.permitAll())));
-        result.putAll(gatherSecurityAnnotations(index, DotNames.AUTHENTICATED, methodToInstanceCollector, classAnnotations,
-                (instance -> recorder.authenticated())));
-
-        result.putAll(gatherSecurityAnnotations(index, DENY_ALL, methodToInstanceCollector, classAnnotations,
-                (instance -> recorder.denyAll())));
+                ((methodInfo, instance) -> methodToRoles.put(methodInfo, instance.value().asStringArray())));
 
         /*
          * Handle additional secured methods by adding the denyAll/rolesAllowed check to all public non-static methods
@@ -548,8 +572,8 @@ private Map<MethodInfo, SecurityCheck> gatherSecurityAnnotations(
             AnnotationInstance alreadyExistingInstance = methodToInstanceCollector.get(additionalSecuredMethod.methodInfo);
             if (additionalSecuredMethod.rolesAllowed.isPresent()) {
                 if (alreadyExistingInstance == null) {
-                    result.put(additionalSecuredMethod.methodInfo, recorder
-                            .rolesAllowed(additionalSecuredMethod.rolesAllowed.get().toArray(String[]::new)));
+                    methodToRoles.put(additionalSecuredMethod.methodInfo,
+                            additionalSecuredMethod.rolesAllowed.get().toArray(String[]::new));
                 } else if (alreadyHasAnnotation(alreadyExistingInstance, ROLES_ALLOWED)) {
                     // we should not try to add second @RolesAllowed
                     throw new IllegalStateException("Method " + additionalSecuredMethod.methodInfo.declaringClass() + "#"
@@ -568,6 +592,50 @@ private Map<MethodInfo, SecurityCheck> gatherSecurityAnnotations(
             }
         }
 
+        // create roles allowed security checks
+        // we create only one security check for each role set
+        Map<Set<String>, SecurityCheck> cache = new HashMap<>();
+        final AtomicInteger keyIndex = new AtomicInteger(0);
+        final AtomicBoolean hasRolesAllowedCheckWithConfigExp = new AtomicBoolean(false);
+        for (Map.Entry<MethodInfo, String[]> entry : methodToRoles.entrySet()) {
+            final MethodInfo methodInfo = entry.getKey();
+            final String[] allowedRoles = entry.getValue();
+            result.put(methodInfo,
+                    cache.computeIfAbsent(getSetForKey(allowedRoles), new Function<Set<String>, SecurityCheck>() {
+                        @Override
+                        public SecurityCheck apply(Set<String> allowedRolesSet) {
+                            final int[] configExpressionPositions = configExpressionPositions(allowedRoles);
+                            if (configExpressionPositions.length > 0) {
+                                // we need to use supplier check as security checks are created during static init
+                                // while config expressions are resolved during runtime
+                                hasRolesAllowedCheckWithConfigExp.set(true);
+
+                                // we don't create security check for each method, therefore we need artificial keys
+                                // we can safely use numbers as RolesAllowed config source prefix all keys
+                                final int[] configKeys = new int[configExpressionPositions.length];
+                                for (int i = 0; i < configExpressionPositions.length; i++) {
+                                    // now we just collect artificial keys, but
+                                    // before we add the property to the Config system, we prefix it, e.g.
+                                    // @RolesAllowed("${admin}") -> QuarkusSecurityRolesAllowedConfigSource.property-0=${admin}
+                                    configKeys[i] = keyIndex.getAndIncrement();
+                                }
+                                return recorder.rolesAllowedSupplier(allowedRoles, configExpressionPositions, configKeys);
+                            }
+                            return recorder.rolesAllowed(allowedRoles);
+                        }
+                    }));
+        }
+
+        if (hasRolesAllowedCheckWithConfigExp.get()) {
+            // make sure config expressions are resolved when app starts
+            configExpSecurityCheckProducer
+                    .produce(new ConfigExpRolesAllowedSecurityCheckBuildItem());
+
+            // register config source with the Config system
+            configBuilderProducer
+                    .produce(new RunTimeConfigBuilderBuildItem(QuarkusSecurityRolesAllowedConfigBuilder.class.getName()));
+        }
+
         /*
          * If we need to add the denyAll security check to all unannotated methods, we simply go through all secured methods,
          * collect the declaring classes, then go through all methods of the classes and add the necessary check
@@ -593,6 +661,31 @@ private Map<MethodInfo, SecurityCheck> gatherSecurityAnnotations(
         return result;
     }
 
+    public static int[] configExpressionPositions(String[] allowedRoles) {
+        final Set<Integer> expPositions = new HashSet<>();
+        for (int i = 0; i < allowedRoles.length; i++) {
+            final int exprStart = allowedRoles[i].indexOf("${");
+            if (exprStart >= 0 && allowedRoles[i].indexOf('}', exprStart + 2) > 0) {
+                expPositions.add(i);
+            }
+        }
+
+        if (expPositions.isEmpty()) {
+            return new int[0];
+        }
+        return expPositions.stream().mapToInt(Integer::intValue).toArray();
+    }
+
+    private static Set<String> getSetForKey(String[] allowedRoles) {
+        if (allowedRoles.length == 0) { // shouldn't happen, but let's be on the safe side
+            return Collections.emptySet();
+        } else if (allowedRoles.length == 1) {
+            return Collections.singleton(allowedRoles[0]);
+        }
+        // use a set in order to avoid caring about the order of elements
+        return new HashSet<>(Arrays.asList(allowedRoles));
+    }
+
     private boolean alreadyHasAnnotation(AnnotationInstance alreadyExistingInstance, DotName annotationName) {
         return alreadyExistingInstance.target().kind() == AnnotationTarget.Kind.METHOD
                 && alreadyExistingInstance.name().equals(annotationName);
@@ -603,13 +696,11 @@ private boolean isPublicNonStaticNonConstructor(MethodInfo methodInfo) {
                 && !"<init>".equals(methodInfo.name());
     }
 
-    private Map<MethodInfo, SecurityCheck> gatherSecurityAnnotations(
+    private void gatherSecurityAnnotations(
             IndexView index, DotName dotName,
             Map<MethodInfo, AnnotationInstance> alreadyCheckedMethods,
             Map<ClassInfo, AnnotationInstance> classLevelAnnotations,
-            Function<AnnotationInstance, SecurityCheck> securityCheckInstanceCreator) {
-
-        Map<MethodInfo, SecurityCheck> result = new HashMap<>();
+            BiConsumer<MethodInfo, AnnotationInstance> putResult) {
 
         Collection<AnnotationInstance> instances = index.getAnnotations(dotName);
         // make sure we process annotations on methods first
@@ -622,7 +713,7 @@ private Map<MethodInfo, SecurityCheck> gatherSecurityAnnotations(
                             + " is annotated with multiple security annotations");
                 }
                 alreadyCheckedMethods.put(methodInfo, instance);
-                result.put(methodInfo, securityCheckInstanceCreator.apply(instance));
+                putResult.accept(methodInfo, instance);
             }
         }
         // now add the class annotations to methods if they haven't already been annotated
@@ -636,7 +727,7 @@ private Map<MethodInfo, SecurityCheck> gatherSecurityAnnotations(
                     for (MethodInfo methodInfo : methods) {
                         AnnotationInstance alreadyExistingInstance = alreadyCheckedMethods.get(methodInfo);
                         if ((alreadyExistingInstance == null)) {
-                            result.put(methodInfo, securityCheckInstanceCreator.apply(instance));
+                            putResult.accept(methodInfo, instance);
                         }
                     }
                 } else {
@@ -647,8 +738,6 @@ private Map<MethodInfo, SecurityCheck> gatherSecurityAnnotations(
             }
 
         }
-
-        return result;
     }
 
     @BuildStep