Skip to content

Commit

Permalink
Smallrye Config property expression expansion for @RolesAllowed values
Browse files Browse the repository at this point in the history
  • Loading branch information
michalvavrik committed Dec 18, 2022
1 parent d573d17 commit c0a3361
Showing 17 changed files with 589 additions and 27 deletions.
Original file line number Diff line number Diff line change
@@ -263,13 +263,23 @@ public class SubjectExposingResource {
String name = user != null ? user.getName() : "anonymous";
return name;
}
@GET
@Path("config-property-expressions")
@RolesAllowed("${admin}") <6>
public String getSubjectSecuredConfigExpression(@Context SecurityContext sec) {
Principal user = sec.getUserPrincipal();
String name = user != null ? user.getName() : "anonymous";
return name;
}
}
----
<1> This `/subject/secured` endpoint requires an authenticated user that has been granted the role "Tester" through the use of the `@RolesAllowed("Tester")` annotation.
<2> The endpoint obtains the user principal from the JAX-RS `SecurityContext`. This will be non-null for a secured endpoint.
<3> The `/subject/unsecured` endpoint allows for unauthenticated access by specifying the `@PermitAll` annotation.
<4> This call to obtain the user principal will return null if the caller is unauthenticated, non-null if the caller is authenticated.
<5> The `/subject/denied` endpoint disallows any access regardless of whether the call is authenticated by specifying the `@DenyAll` annotation.
<6> The `@RolesAllowed` annotation value supports <<config-reference#property-expressions,Property Expressions>> including default values and nested Property Expressions.

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.

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
@@ -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")
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
@@ -14,12 +14,15 @@
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.function.BiConsumer;
import java.util.function.Function;
import java.util.function.Predicate;

@@ -473,6 +476,7 @@ void transformSecurityAnnotations(BuildProducer<AnnotationsTransformerBuildItem>
@BuildStep
@Record(ExecutionTime.STATIC_INIT)
void gatherSecurityChecks(BuildProducer<SyntheticBeanBuildItem> syntheticBeans,
BuildProducer<ConfigExpRolesAllowedSecurityCheckBuildItem> configExpSecurityCheckProducer,
BeanArchiveIndexBuildItem beanArchiveBuildItem,
BuildProducer<ApplicationClassPredicateBuildItem> classPredicate,
List<AdditionalSecuredMethodsBuildItem> additionalSecuredMethods,
@@ -489,8 +493,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);
for (AdditionalSecurityCheckBuildItem additionalSecurityCheck : additionalSecurityChecks) {
securityChecks.put(additionalSecurityCheck.getMethodInfo(),
additionalSecurityCheck.getSecurityCheck());
@@ -520,22 +524,37 @@ void gatherSecurityChecks(BuildProducer<SyntheticBeanBuildItem> syntheticBeans,
}).done());
}

private Map<MethodInfo, SecurityCheck> gatherSecurityAnnotations(
IndexView index,
@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) {

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 +567,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 +587,34 @@ 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 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);
return recorder.rolesAllowedSupplier(allowedRoles, configExpressionPositions);
}
return recorder.rolesAllowed(allowedRoles);
}
}));
}

if (hasRolesAllowedCheckWithConfigExp.get()) {
configExpSecurityCheckProducer
.produce(new ConfigExpRolesAllowedSecurityCheckBuildItem());
}

/*
* 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 +640,39 @@ 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 String getTargetName(AnnotationTarget target) {
if (target.kind() == AnnotationTarget.Kind.METHOD) {
return target.asMethod().name();
} else {
return target.asClass().name().toString();
}
}

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 +683,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 +700,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 +714,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 +725,6 @@ private Map<MethodInfo, SecurityCheck> gatherSecurityAnnotations(
}

}

return result;
}

@BuildStep
Loading

0 comments on commit c0a3361

Please sign in to comment.