Skip to content

Commit

Permalink
Smallrye Config property expression expansion for @RolesAllowed values
Browse files Browse the repository at this point in the history
closes: #25245
  • Loading branch information
michalvavrik committed Dec 20, 2022
1 parent 1079861 commit c4360ff
Show file tree
Hide file tree
Showing 17 changed files with 682 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Original file line number Diff line number Diff line change
@@ -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"));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
@@ -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() {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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());
Expand Down Expand Up @@ -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
Expand All @@ -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() + "#"
Expand All @@ -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
Expand All @@ -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);
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -647,8 +738,6 @@ private Map<MethodInfo, SecurityCheck> gatherSecurityAnnotations(
}

}

return result;
}

@BuildStep
Expand Down
Loading

0 comments on commit c4360ff

Please sign in to comment.