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 18, 2022
1 parent d573d17 commit 604755f
Show file tree
Hide file tree
Showing 17 changed files with 581 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
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,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;

Expand Down Expand Up @@ -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,
Expand All @@ -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());
Expand Down Expand Up @@ -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
Expand All @@ -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() + "#"
Expand All @@ -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
Expand All @@ -593,6 +640,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 +675,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 +692,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 +706,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 +717,6 @@ private Map<MethodInfo, SecurityCheck> gatherSecurityAnnotations(
}

}

return result;
}

@BuildStep
Expand Down
Loading

0 comments on commit 604755f

Please sign in to comment.