From c4360ffff48a62b7b11d09476af6b1536a6f9d41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Vav=C5=99=C3=ADk?= Date: Tue, 20 Dec 2022 16:56:53 +0100 Subject: [PATCH] Smallrye Config property expression expansion for @RolesAllowed values closes: #25245 --- ...horization-of-web-endpoints-reference.adoc | 69 +++++++++ ...LazyAuthRolesAllowedConfigExpTestCase.java | 44 ++++++ .../test/security/RolesAllowedResource.java | 7 + ...ExpRolesAllowedSecurityCheckBuildItem.java | 14 ++ .../deployment/SecurityProcessor.java | 137 +++++++++++++++--- .../ConfigExpressionDetectionTest.java | 102 +++++++++++++ .../RolesAllowedExpressionFailureTest.java | 57 ++++++++ .../RolesAllowedExpressionTest.java | 127 ++++++++++++++++ ...rkusSecurityRolesAllowedConfigBuilder.java | 47 ++++++ .../runtime/SecurityCheckRecorder.java | 57 ++++++++ .../check/SupplierRolesAllowedCheck.java | 6 + .../reactive/elytron/RootResource.java | 7 + .../src/main/resources/application.properties | 1 + .../reactive/elytron/RootResourceTest.java | 11 ++ .../quarkus/it/rest/RBACSecuredResource.java | 7 + .../src/main/resources/application.properties | 5 +- .../io/quarkus/it/main/RBACAccessTest.java | 9 ++ 17 files changed, 682 insertions(+), 25 deletions(-) create mode 100644 extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/LazyAuthRolesAllowedConfigExpTestCase.java create mode 100644 extensions/security/deployment/src/main/java/io/quarkus/security/deployment/ConfigExpRolesAllowedSecurityCheckBuildItem.java create mode 100644 extensions/security/deployment/src/test/java/io/quarkus/security/test/rolesallowed/ConfigExpressionDetectionTest.java create mode 100644 extensions/security/deployment/src/test/java/io/quarkus/security/test/rolesallowed/RolesAllowedExpressionFailureTest.java create mode 100644 extensions/security/deployment/src/test/java/io/quarkus/security/test/rolesallowed/RolesAllowedExpressionTest.java create mode 100644 extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusSecurityRolesAllowedConfigBuilder.java diff --git a/docs/src/main/asciidoc/security-authorization-of-web-endpoints-reference.adoc b/docs/src/main/asciidoc/security-authorization-of-web-endpoints-reference.adoc index ededae5a66872..e39525d881292 100644 --- a/docs/src/main/asciidoc/security-authorization-of-web-endpoints-reference.adoc +++ b/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 <> 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] diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/LazyAuthRolesAllowedConfigExpTestCase.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/LazyAuthRolesAllowedConfigExpTestCase.java new file mode 100644 index 0000000000000..32c204a3d3055 --- /dev/null +++ b/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")); + } + +} diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/RolesAllowedResource.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/RolesAllowedResource.java index 3ea7a09536802..ede7b1a80968b 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/RolesAllowedResource.java +++ b/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") diff --git a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/ConfigExpRolesAllowedSecurityCheckBuildItem.java b/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/ConfigExpRolesAllowedSecurityCheckBuildItem.java new file mode 100644 index 0000000000000..fe77ebcd035f6 --- /dev/null +++ b/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() { + } +} diff --git a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java b/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java index d690825fbd986..1b8d98a392ae0 100644 --- a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java +++ b/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 @BuildStep @Record(ExecutionTime.STATIC_INIT) void gatherSecurityChecks(BuildProducer syntheticBeans, + BuildProducer configExpSecurityCheckProducer, BeanArchiveIndexBuildItem beanArchiveBuildItem, BuildProducer classPredicate, + BuildProducer configBuilderProducer, List additionalSecuredMethods, SecurityCheckRecorder recorder, List additionalSecurityChecks, SecurityBuildTimeConfig config) { @@ -489,8 +497,8 @@ void gatherSecurityChecks(BuildProducer syntheticBeans, } IndexView index = beanArchiveBuildItem.getIndex(); - Map securityChecks = gatherSecurityAnnotations( - index, additionalSecured.values(), config.denyUnannotated, recorder); + Map 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 syntheticBeans, }).done()); } - private Map gatherSecurityAnnotations( - IndexView index, - Collection additionalSecuredMethods, boolean denyUnannotated, SecurityCheckRecorder recorder) { + @BuildStep + @Record(ExecutionTime.RUNTIME_INIT) + public void resolveConfigExpressionRoles(Optional 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 gatherSecurityAnnotations(IndexView index, + BuildProducer configExpSecurityCheckProducer, + Collection additionalSecuredMethods, boolean denyUnannotated, SecurityCheckRecorder recorder, + BuildProducer configBuilderProducer) { Map methodToInstanceCollector = new HashMap<>(); Map classAnnotations = new HashMap<>(); - Map result = new HashMap<>(gatherSecurityAnnotations( + Map 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 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 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 gatherSecurityAnnotations( } } + // create roles allowed security checks + // we create only one security check for each role set + Map, SecurityCheck> cache = new HashMap<>(); + final AtomicInteger keyIndex = new AtomicInteger(0); + final AtomicBoolean hasRolesAllowedCheckWithConfigExp = new AtomicBoolean(false); + for (Map.Entry entry : methodToRoles.entrySet()) { + final MethodInfo methodInfo = entry.getKey(); + final String[] allowedRoles = entry.getValue(); + result.put(methodInfo, + cache.computeIfAbsent(getSetForKey(allowedRoles), new Function, SecurityCheck>() { + @Override + public SecurityCheck apply(Set 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 gatherSecurityAnnotations( return result; } + public static int[] configExpressionPositions(String[] allowedRoles) { + final Set 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 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) { && !"".equals(methodInfo.name()); } - private Map gatherSecurityAnnotations( + private void gatherSecurityAnnotations( IndexView index, DotName dotName, Map alreadyCheckedMethods, Map classLevelAnnotations, - Function securityCheckInstanceCreator) { - - Map result = new HashMap<>(); + BiConsumer putResult) { Collection instances = index.getAnnotations(dotName); // make sure we process annotations on methods first @@ -622,7 +713,7 @@ private Map 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 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 gatherSecurityAnnotations( } } - - return result; } @BuildStep diff --git a/extensions/security/deployment/src/test/java/io/quarkus/security/test/rolesallowed/ConfigExpressionDetectionTest.java b/extensions/security/deployment/src/test/java/io/quarkus/security/test/rolesallowed/ConfigExpressionDetectionTest.java new file mode 100644 index 0000000000000..945e0b93fc5b6 --- /dev/null +++ b/extensions/security/deployment/src/test/java/io/quarkus/security/test/rolesallowed/ConfigExpressionDetectionTest.java @@ -0,0 +1,102 @@ +package io.quarkus.security.test.rolesallowed; + +import java.io.IOException; +import java.io.InputStream; +import java.util.HashMap; +import java.util.Map; + +import javax.annotation.security.RolesAllowed; + +import org.jboss.jandex.DotName; +import org.jboss.jandex.Indexer; +import org.jboss.jandex.MethodInfo; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import io.quarkus.security.deployment.DotNames; +import io.quarkus.security.deployment.SecurityProcessor; + +public class ConfigExpressionDetectionTest { + + private static final Map VALID_VALUES; + + static { + // point here is to verify expected values gathered from @RolesAllowed annotation are detected correctly + var indexer = new Indexer(); + for (Class aClass : new Class[] { ConfigExpressionDetectionTest.class, ValidValues.class }) { + try (InputStream stream = ConfigExpressionDetectionTest.class.getClassLoader() + .getResourceAsStream(aClass.getName().replace('.', '/') + ".class")) { + assert stream != null; + indexer.index(stream); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + var index = indexer.complete(); + VALID_VALUES = new HashMap<>(); + for (MethodInfo methodInfo : index.getClassByName(DotName.createSimple(ValidValues.class.getName())).methods()) { + var annotation = methodInfo.annotation(DotNames.ROLES_ALLOWED); + if (annotation != null) { + VALID_VALUES.put(methodInfo.name(), annotation.value().asStringArray()); + } + } + } + + @Test + void testConfigExpIsDetected() { + VALID_VALUES.forEach((methodName, rolesAllowed) -> { + final int[] expressionPositions; + switch (methodName) { + case "secured1": + expressionPositions = new ValidValues().secured1(); + break; + case "secured2": + expressionPositions = new ValidValues().secured2(); + break; + case "secured3": + expressionPositions = new ValidValues().secured3(); + break; + case "secured4": + expressionPositions = new ValidValues().secured4(); + break; + case "secured5": + expressionPositions = new ValidValues().secured5(); + break; + default: + throw new IllegalStateException(); + } + Assertions.assertArrayEquals(SecurityProcessor.configExpressionPositions(rolesAllowed), + expressionPositions); + }); + } + + public static final class ValidValues { + + @RolesAllowed({ "first-role", "${second-role}", "third-role" }) + public int[] secured1() { + return new int[] { 1 }; + } + + @RolesAllowed({ "${first-role}", "second-role", "${third-role}" }) + public int[] secured2() { + return new int[] { 0, 2 }; + } + + @RolesAllowed({ "${first-role}", "${second-role}", "${third-role: defaultValue}" }) + public int[] secured3() { + return new int[] { 0, 1, 2 }; + } + + @RolesAllowed({ "first-role", "second-role", "third-role" }) + public int[] secured4() { + return new int[] {}; + } + + @RolesAllowed("${first-role}, ${second-role: defaultValue}, ${third-role}") + public int[] secured5() { + // we expect 1 value as this is considered 1 role + return new int[] { 0 }; + } + } + +} diff --git a/extensions/security/deployment/src/test/java/io/quarkus/security/test/rolesallowed/RolesAllowedExpressionFailureTest.java b/extensions/security/deployment/src/test/java/io/quarkus/security/test/rolesallowed/RolesAllowedExpressionFailureTest.java new file mode 100644 index 0000000000000..2a7d3b83a7357 --- /dev/null +++ b/extensions/security/deployment/src/test/java/io/quarkus/security/test/rolesallowed/RolesAllowedExpressionFailureTest.java @@ -0,0 +1,57 @@ +package io.quarkus.security.test.rolesallowed; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.NoSuchElementException; + +import javax.annotation.security.RolesAllowed; +import javax.inject.Singleton; + +import org.junit.jupiter.api.Assertions; +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.security.test.utils.SecurityTestUtils; +import io.quarkus.test.QuarkusUnitTest; + +public class RolesAllowedExpressionFailureTest { + + @RegisterExtension + static final QuarkusUnitTest config = new QuarkusUnitTest() + .withApplicationRoot((jar) -> jar + .addClasses(RolesAllowedBean.class, IdentityMock.class, + AuthData.class, SecurityTestUtils.class)) + .assertException(t -> { + Throwable e = t; + NoSuchElementException te = null; + while (e != null) { + if (e instanceof NoSuchElementException) { + te = (NoSuchElementException) e; + break; + } + e = e.getCause(); + } + assertNotNull(te); + // assert + assertTrue(te.getMessage().contains("Could not expand value admin"), te.getMessage()); + }); + + @Test + public void test() { + Assertions.fail(); + } + + @Singleton + public static class RolesAllowedBean { + + @RolesAllowed("${admin}") + public final String admin() { + return "accessibleForAdminOnly"; + } + + } + +} diff --git a/extensions/security/deployment/src/test/java/io/quarkus/security/test/rolesallowed/RolesAllowedExpressionTest.java b/extensions/security/deployment/src/test/java/io/quarkus/security/test/rolesallowed/RolesAllowedExpressionTest.java new file mode 100644 index 0000000000000..055c5f5cb79f3 --- /dev/null +++ b/extensions/security/deployment/src/test/java/io/quarkus/security/test/rolesallowed/RolesAllowedExpressionTest.java @@ -0,0 +1,127 @@ +package io.quarkus.security.test.rolesallowed; + +import static io.quarkus.security.test.utils.IdentityMock.ADMIN; +import static io.quarkus.security.test.utils.IdentityMock.USER; +import static io.quarkus.security.test.utils.SecurityTestUtils.assertFailureFor; +import static io.quarkus.security.test.utils.SecurityTestUtils.assertSuccess; + +import java.util.Collections; +import java.util.Set; + +import javax.annotation.security.RolesAllowed; +import javax.inject.Inject; +import javax.inject.Singleton; + +import org.jboss.shrinkwrap.api.asset.StringAsset; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.security.ForbiddenException; +import io.quarkus.security.test.utils.AuthData; +import io.quarkus.security.test.utils.IdentityMock; +import io.quarkus.security.test.utils.SecurityTestUtils; +import io.quarkus.test.QuarkusUnitTest; + +public class RolesAllowedExpressionTest { + + private static final String APP_PROPS = "" + + "sudo=admin\n" + + "sec-group.user-part-one=user\n" + + "sec-group.user-part-two=e\n" + + "su=ad\n" + + "do=min\n" + + "spaces.s=s\n" + + "spaces.p=p\n" + + "spaces.a=a\n" + + "spaces.c=c\n" + + "multiple-roles-grp.1st=multiple-roles.1st\n" + + "multiple-roles-grp.3rd=multiple-roles.3rd\n" + + "test-profile-admin=batman\n" + + "%test.test-profile-admin=admin\n" + + "missing-profile-profile-admin=superman\n" + + "%missing-profile.missing-profile-profile-admin=admin\n"; + + @RegisterExtension + static final QuarkusUnitTest config = new QuarkusUnitTest() + .withApplicationRoot((jar) -> jar + .addClasses(RolesAllowedBean.class, IdentityMock.class, + AuthData.class, SecurityTestUtils.class) + .addAsResource(new StringAsset(APP_PROPS), "application.properties")); + + @Inject + RolesAllowedBean bean; + + @Test + public void shouldRestrictAccessToSpecificRole() { + // resolve role from 'sudo' config property + assertSuccess(() -> bean.admin(), "accessibleForAdminOnly", ADMIN); + // use default value 'user' as config property 'kudos' is missing + assertSuccess(() -> bean.user(), "accessibleForUserOnly", USER); + // composed expression + assertSuccess(() -> bean.user2(), "accessibleForUserOnly2", USER); + // multiple expressions + assertSuccess(() -> bean.admin2(), "accessibleForAdminOnly2", ADMIN); + // spaces + assertSuccess(() -> bean.spaces(), "accessibleForSpacesOnly", + new AuthData(Collections.singleton("s p a c e s"), false, "spaces")); + // secured method allow multiple roles + assertSuccess(() -> bean.multipleRoles(), "accessibleForMultipleRoles", + new AuthData(Set.of("multiple-roles.1st"), false, "multiple-roles")); + assertSuccess(() -> bean.multipleRoles(), "accessibleForMultipleRoles", + new AuthData(Set.of("multiple-roles.2nd"), false, "multiple-roles")); + assertSuccess(() -> bean.multipleRoles(), "accessibleForMultipleRoles", + new AuthData(Set.of("multiple-roles.3rd"), false, "multiple-roles")); + assertSuccess(() -> bean.multipleRoles(), "accessibleForMultipleRoles", + new AuthData(Set.of("multiple-roles.4th"), false, "multiple-roles")); + // test profile + assertSuccess(() -> bean.testProfile(), "accessibleForTestProfileAdmin", ADMIN); + assertFailureFor(() -> bean.missingTestProfile(), ForbiddenException.class, ADMIN); + } + + @Singleton + public static class RolesAllowedBean { + + @RolesAllowed("${sudo}") + public final String admin() { + return "accessibleForAdminOnly"; + } + + @RolesAllowed("${su}${do}") + public final String admin2() { + return "accessibleForAdminOnly2"; + } + + @RolesAllowed("${kudos:user}") + public final String user() { + return "accessibleForUserOnly"; + } + + @RolesAllowed("${sec-group.user-part-on${sec-group.user-part-two}}") + public final String user2() { + return "accessibleForUserOnly2"; + } + + @RolesAllowed("${spaces.s} ${spaces.p} ${spaces.a} ${spaces.c} e s") + public final String spaces() { + return "accessibleForSpacesOnly"; + } + + @RolesAllowed({ "${multiple-roles-grp.1st}", "${multiple-roles-grp.2nd:multiple-roles.2nd}", + "${multiple-roles-grp.3rd}", "multiple-roles.4th" }) + public final String multipleRoles() { + return "accessibleForMultipleRoles"; + } + + @RolesAllowed("${test-profile-admin}") + public final String testProfile() { + return "accessibleForTestProfileAdmin"; + } + + @RolesAllowed("${missing-profile-profile-admin}") + public final void missingTestProfile() { + // should throw exception + } + + } + +} diff --git a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusSecurityRolesAllowedConfigBuilder.java b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusSecurityRolesAllowedConfigBuilder.java new file mode 100644 index 0000000000000..d79aab1fa6da0 --- /dev/null +++ b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusSecurityRolesAllowedConfigBuilder.java @@ -0,0 +1,47 @@ +package io.quarkus.security.runtime; + +import java.util.HashMap; +import java.util.Map; +import java.util.Set; + +import org.eclipse.microprofile.config.spi.ConfigSource; + +import io.quarkus.runtime.configuration.ConfigBuilder; +import io.smallrye.config.SmallRyeConfigBuilder; + +public final class QuarkusSecurityRolesAllowedConfigBuilder implements ConfigBuilder { + + private static final String ROLES_ALLOWED_CONFIG_SOURCE = "QuarkusSecurityRolesAllowedConfigSource"; + private static final Map properties = new HashMap<>(); + private final ConfigSource configSource = new ConfigSource() { + @Override + public Set getPropertyNames() { + return properties.keySet(); + } + + @Override + public String getValue(String key) { + return properties.get(key); + } + + @Override + public String getName() { + return ROLES_ALLOWED_CONFIG_SOURCE; + } + }; + + @Override + public SmallRyeConfigBuilder configBuilder(SmallRyeConfigBuilder builder) { + builder.getSources().add(configSource); + return builder; + } + + static void addProperty(int key, String value) { + // this method should be called during static init + properties.put(transformToKey(key), value); + } + + static String transformToKey(int i) { + return ROLES_ALLOWED_CONFIG_SOURCE + ".property-" + i; + } +} diff --git a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityCheckRecorder.java b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityCheckRecorder.java index 4d3b4e0d23e77..0e26042dc6dbf 100644 --- a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityCheckRecorder.java +++ b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityCheckRecorder.java @@ -1,5 +1,15 @@ package io.quarkus.security.runtime; +import static io.quarkus.security.runtime.QuarkusSecurityRolesAllowedConfigBuilder.transformToKey; + +import java.util.Arrays; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Supplier; + +import org.eclipse.microprofile.config.Config; +import org.eclipse.microprofile.config.spi.ConfigProviderResolver; + import io.quarkus.runtime.RuntimeValue; import io.quarkus.runtime.annotations.Recorder; import io.quarkus.security.runtime.interceptor.SecurityCheckStorageBuilder; @@ -7,13 +17,16 @@ import io.quarkus.security.runtime.interceptor.check.DenyAllCheck; import io.quarkus.security.runtime.interceptor.check.PermitAllCheck; import io.quarkus.security.runtime.interceptor.check.RolesAllowedCheck; +import io.quarkus.security.runtime.interceptor.check.SupplierRolesAllowedCheck; import io.quarkus.security.spi.runtime.SecurityCheck; import io.quarkus.security.spi.runtime.SecurityCheckStorage; +import io.smallrye.config.Expressions; @Recorder public class SecurityCheckRecorder { private static volatile SecurityCheckStorage storage; + private static final Set configExpRolesAllowedChecks = ConcurrentHashMap.newKeySet(); public static SecurityCheckStorage getStorage() { return storage; @@ -31,6 +44,42 @@ public SecurityCheck rolesAllowed(String... roles) { return RolesAllowedCheck.of(roles); } + public SecurityCheck rolesAllowedSupplier(String[] allowedRoles, int[] configExpIndexes, int[] configKeys) { + + // here we add generated keys and values with the property expressions to the config source, + // the config source will be registered with the Config system, + // and we get all features available from Config + for (int i = 0; i < configExpIndexes.length; i++) { + QuarkusSecurityRolesAllowedConfigBuilder.addProperty(configKeys[i], allowedRoles[configExpIndexes[i]]); + } + + final var check = new SupplierRolesAllowedCheck( + resolveRolesAllowedConfigExp(allowedRoles, configExpIndexes, configKeys)); + configExpRolesAllowedChecks.add(check); + return check; + } + + private static Supplier resolveRolesAllowedConfigExp(String[] allowedRoles, int[] configExpIndexes, + int[] configKeys) { + + final String[] roles = Arrays.copyOf(allowedRoles, allowedRoles.length); + return new Supplier() { + @Override + public String[] get() { + final var config = ConfigProviderResolver.instance().getConfig(Thread.currentThread().getContextClassLoader()); + if (config.getOptionalValue(Config.PROPERTY_EXPRESSIONS_ENABLED, Boolean.class).orElse(Boolean.TRUE) + && Expressions.isEnabled()) { + // property expressions are enabled + for (int i = 0; i < configExpIndexes.length; i++) { + // resolve configuration expressions specified as value of the @RolesAllowed annotation + roles[configExpIndexes[i]] = config.getValue(transformToKey(configKeys[i]), String.class); + } + } + return roles; + } + }; + } + public SecurityCheck authenticated() { return AuthenticatedCheck.INSTANCE; } @@ -50,4 +99,12 @@ public void create(RuntimeValue builder) { storage = builder.getValue().create(); } + public void resolveRolesAllowedConfigExpRoles() { + if (!configExpRolesAllowedChecks.isEmpty()) { + for (SupplierRolesAllowedCheck configExpRolesAllowedCheck : configExpRolesAllowedChecks) { + configExpRolesAllowedCheck.resolveAllowedRoles(); + } + configExpRolesAllowedChecks.clear(); + } + } } diff --git a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/interceptor/check/SupplierRolesAllowedCheck.java b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/interceptor/check/SupplierRolesAllowedCheck.java index 5ec82dc3cd65f..f7550a570b911 100644 --- a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/interceptor/check/SupplierRolesAllowedCheck.java +++ b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/interceptor/check/SupplierRolesAllowedCheck.java @@ -37,4 +37,10 @@ private void doApply(SecurityIdentity identity) { } RolesAllowedCheck.doApply(identity, allowedRoles); } + + public void resolveAllowedRoles() { + if (allowedRoles == null) { + allowedRoles = allowedRolesSupplier.get(); + } + } } diff --git a/integration-tests/elytron-resteasy-reactive/src/main/java/io/quarkus/it/resteasy/reactive/elytron/RootResource.java b/integration-tests/elytron-resteasy-reactive/src/main/java/io/quarkus/it/resteasy/reactive/elytron/RootResource.java index a52c6bf2ee811..5cf108735b179 100644 --- a/integration-tests/elytron-resteasy-reactive/src/main/java/io/quarkus/it/resteasy/reactive/elytron/RootResource.java +++ b/integration-tests/elytron-resteasy-reactive/src/main/java/io/quarkus/it/resteasy/reactive/elytron/RootResource.java @@ -57,6 +57,13 @@ public String user(@Context SecurityContext sec) { return sec.getUserPrincipal().getName(); } + @GET + @Path("/employee") + @RolesAllowed("${employees-config-property}") + public String employee(@Context SecurityContext sec) { + return sec.getUserPrincipal().getName(); + } + @GET @Path("/attributes") @Authenticated diff --git a/integration-tests/elytron-resteasy-reactive/src/main/resources/application.properties b/integration-tests/elytron-resteasy-reactive/src/main/resources/application.properties index 84c4d19bd9f24..578e5f42da902 100644 --- a/integration-tests/elytron-resteasy-reactive/src/main/resources/application.properties +++ b/integration-tests/elytron-resteasy-reactive/src/main/resources/application.properties @@ -7,3 +7,4 @@ quarkus.security.users.embedded.users.poul=poul quarkus.security.users.embedded.roles.poul=interns quarkus.security.users.embedded.plain-text=true quarkus.http.auth.basic=true +employees-config-property=employees \ No newline at end of file diff --git a/integration-tests/elytron-resteasy-reactive/src/test/java/io/quarkus/it/resteasy/reactive/elytron/RootResourceTest.java b/integration-tests/elytron-resteasy-reactive/src/test/java/io/quarkus/it/resteasy/reactive/elytron/RootResourceTest.java index 59b81b5cfe7c3..207cc0b9e255e 100644 --- a/integration-tests/elytron-resteasy-reactive/src/test/java/io/quarkus/it/resteasy/reactive/elytron/RootResourceTest.java +++ b/integration-tests/elytron-resteasy-reactive/src/test/java/io/quarkus/it/resteasy/reactive/elytron/RootResourceTest.java @@ -38,4 +38,15 @@ void testGet() { .body(is("get success")); } + @Test + void testRolesAllowedConfigExpression() { + given() + .auth().preemptive().basic("john", Users.password("john")) + .when() + .get("/employee") + .then() + .statusCode(200) + .body(is("john")); + } + } diff --git a/integration-tests/main/src/main/java/io/quarkus/it/rest/RBACSecuredResource.java b/integration-tests/main/src/main/java/io/quarkus/it/rest/RBACSecuredResource.java index 3582318588e5d..033c9cbf79319 100644 --- a/integration-tests/main/src/main/java/io/quarkus/it/rest/RBACSecuredResource.java +++ b/integration-tests/main/src/main/java/io/quarkus/it/rest/RBACSecuredResource.java @@ -29,6 +29,13 @@ public String forTesterOnly() { return "forTesterOnly"; } + @GET + @RolesAllowed("${tester-config-exp}") + @Path("forTesterOnlyConfigExp") + public String forTesterOnlyConfigExp() { + return "forTesterOnlyConfigExp"; + } + @GET @RolesAllowed("tester") @Path("forTesterOnlyWithMethodParamAnnotations") diff --git a/integration-tests/main/src/main/resources/application.properties b/integration-tests/main/src/main/resources/application.properties index 3c85f6fb16ebb..1cc81a74d60c0 100644 --- a/integration-tests/main/src/main/resources/application.properties +++ b/integration-tests/main/src/main/resources/application.properties @@ -58,4 +58,7 @@ quarkus.native.additional-build-args =-H:ReflectionConfigurationFiles=reflection quarkus.class-loading.removed-resources."io.quarkus\:quarkus-integration-test-shared-library"=io/quarkus/it/shared/RemovedResource.class quarkus.class-loading.removed-resources."io.quarkus\:quarkus-integration-test-main"=io/quarkus/it/rest/RemovedJaxRsApplication.class # Enable callbacks for integration tests -quarkus.test.enable-callbacks-for-integration-tests=true \ No newline at end of file +quarkus.test.enable-callbacks-for-integration-tests=true + +# @RolesAllowed value is configuration expression +tester-config-exp=tester \ No newline at end of file diff --git a/integration-tests/main/src/test/java/io/quarkus/it/main/RBACAccessTest.java b/integration-tests/main/src/test/java/io/quarkus/it/main/RBACAccessTest.java index 1453f5473daf9..68d2fa027fc3b 100644 --- a/integration-tests/main/src/test/java/io/quarkus/it/main/RBACAccessTest.java +++ b/integration-tests/main/src/test/java/io/quarkus/it/main/RBACAccessTest.java @@ -25,6 +25,15 @@ public void shouldRestrictAccessToSpecificRole() { Optional.of("forTesterOnly")); } + @Test + public void shouldRestrictAccessToSpecificRoleConfigExp() { + String path = "/rbac-secured/forTesterOnlyConfigExp"; + assertForAnonymous(path, 401, Optional.empty()); + assertStatusAndContent(RestAssured.given().auth().preemptive().basic("stuart", "test"), path, 403, Optional.empty()); + assertStatusAndContent(RestAssured.given().auth().preemptive().basic("scott", "jb0ss"), path, 200, + Optional.of("forTesterOnlyConfigExp")); + } + @Test public void shouldRestrictAccessToSpecificRoleAndMethodParameterAnnotationsShouldntAffectAnything() { String path = "/rbac-secured/forTesterOnlyWithMethodParamAnnotations";