Skip to content

Commit

Permalink
Validate Startup observers are not secured with RBAC annotations
Browse files Browse the repository at this point in the history
michalvavrik committed Oct 27, 2023

Verified

This commit was signed with the committer’s verified signature.
amimart Arnaud Mimart
1 parent d110270 commit e9f6d6d
Showing 3 changed files with 139 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package io.quarkus.security.deployment;

import static io.quarkus.arc.processor.DotNames.NO_CLASS_INTERCEPTORS;
import static io.quarkus.gizmo.MethodDescriptor.ofMethod;
import static io.quarkus.security.deployment.DotNames.DENY_ALL;
import static io.quarkus.security.deployment.DotNames.PERMISSIONS_ALLOWED;
import static io.quarkus.security.deployment.DotNames.PERMIT_ALL;
import static io.quarkus.security.deployment.DotNames.ROLES_ALLOWED;
import static io.quarkus.security.deployment.SecurityTransformerUtils.findFirstStandardSecurityAnnotation;
import static io.quarkus.security.deployment.SecurityTransformerUtils.hasStandardSecurityAnnotation;
import static io.quarkus.security.runtime.SecurityProviderUtils.findProviderIndex;

import java.io.IOException;
@@ -20,6 +24,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -42,7 +47,13 @@
import io.quarkus.arc.deployment.AnnotationsTransformerBuildItem;
import io.quarkus.arc.deployment.BeanArchiveIndexBuildItem;
import io.quarkus.arc.deployment.InterceptorBindingRegistrarBuildItem;
import io.quarkus.arc.deployment.SynthesisFinishedBuildItem;
import io.quarkus.arc.deployment.SyntheticBeanBuildItem;
import io.quarkus.arc.deployment.ValidationPhaseBuildItem;
import io.quarkus.arc.deployment.ValidationPhaseBuildItem.ValidationErrorBuildItem;
import io.quarkus.arc.processor.AnnotationStore;
import io.quarkus.arc.processor.BuildExtension;
import io.quarkus.arc.processor.ObserverInfo;
import io.quarkus.builder.item.MultiBuildItem;
import io.quarkus.deployment.Feature;
import io.quarkus.deployment.annotations.BuildProducer;
@@ -72,6 +83,8 @@
import io.quarkus.gizmo.TryBlock;
import io.quarkus.runtime.LaunchMode;
import io.quarkus.runtime.RuntimeValue;
import io.quarkus.runtime.StartupEvent;
import io.quarkus.runtime.configuration.ConfigurationException;
import io.quarkus.security.deployment.PermissionSecurityChecks.PermissionSecurityChecksBuilder;
import io.quarkus.security.runtime.IdentityProviderManagerCreator;
import io.quarkus.security.runtime.QuarkusSecurityRolesAllowedConfigBuilder;
@@ -101,6 +114,7 @@
public class SecurityProcessor {

private static final Logger log = Logger.getLogger(SecurityProcessor.class);
private static final DotName STARTUP_EVENT_NAME = DotName.createSimple(StartupEvent.class.getName());

SecurityConfig security;

@@ -578,7 +592,7 @@ private Map<MethodInfo, SecurityCheck> gatherSecurityAnnotations(IndexView index
Map<MethodInfo, AnnotationInstance> methodToInstanceCollector = new HashMap<>();
Map<ClassInfo, AnnotationInstance> classAnnotations = new HashMap<>();
Map<MethodInfo, SecurityCheck> result = new HashMap<>();
gatherSecurityAnnotations(index, DotNames.PERMIT_ALL, methodToInstanceCollector, classAnnotations,
gatherSecurityAnnotations(index, PERMIT_ALL, methodToInstanceCollector, classAnnotations,
((m, i) -> result.put(m, recorder.permitAll())));
gatherSecurityAnnotations(index, DotNames.AUTHENTICATED, methodToInstanceCollector, classAnnotations,
((m, i) -> result.put(m, recorder.authenticated())));
@@ -810,6 +824,50 @@ AdditionalBeanBuildItem authorizationController(LaunchModeBuildItem launchMode)
return AdditionalBeanBuildItem.builder().addBeanClass(controllerClass).build();
}

@BuildStep
void validateStartUpObserversNotSecured(SynthesisFinishedBuildItem synthesisFinished,
ValidationPhaseBuildItem validationPhase,
BeanArchiveIndexBuildItem beanArchiveIndexBuildItem,
BuildProducer<ValidationErrorBuildItem> validationErrorProducer) {
AnnotationStore annotationStore = validationPhase.getContext().get(BuildExtension.Key.ANNOTATION_STORE);
synthesisFinished
.getObservers()
.stream()
.map(ObserverInfo::asObserver)
.filter(observer -> observer.getObservedType().name().equals(STARTUP_EVENT_NAME))
.map(ObserverInfo::getObserverMethod)
.filter(Objects::nonNull) // synthetic observer method created for @Startup is null and not secured
.forEach(mi -> {
if (hasStandardSecurityAnnotation(annotationStore.getAnnotations(mi))
|| hasClassLevelStandardSecurityAnnotation(mi, annotationStore)) {
var declaringClass = mi.declaringClass();
findFirstStandardSecurityAnnotation(annotationStore.getAnnotations(mi))
.or(() -> findFirstStandardSecurityAnnotation(
annotationStore.getAnnotations(declaringClass)))
.map(AnnotationInstance::name)
.filter(name -> !name.equals(PERMIT_ALL))
.ifPresent(securityAnnotation -> {
var errorMsg = String.format(
"Method '%s#%s' cannot observe '%s' as the method is secured with the '%s' annotation",
declaringClass.name(), mi.name(), STARTUP_EVENT_NAME, securityAnnotation);
validationErrorProducer
.produce(new ValidationErrorBuildItem(new ConfigurationException(errorMsg)));
});
}
});
}

private static boolean hasClassLevelStandardSecurityAnnotation(MethodInfo method, AnnotationStore annotationStore) {
return applyClassLevenInterceptor(method, annotationStore)
&& hasStandardSecurityAnnotation(annotationStore.getAnnotations(method.declaringClass()));
}

private static boolean applyClassLevenInterceptor(MethodInfo method, AnnotationStore store) {
// whether class-level business method interceptors (@AroundInvoke) are applied
return !method.isConstructor() && Modifier.isPublic(method.flags())
&& !store.hasAnnotation(method, NO_CLASS_INTERCEPTORS);
}

static MethodDescription createMethodDescription(MethodInfo additionalSecuredMethod) {
String[] paramTypes = new String[additionalSecuredMethod.parametersCount()];
for (int i = 0; i < additionalSecuredMethod.parametersCount(); i++) {
Original file line number Diff line number Diff line change
@@ -32,7 +32,7 @@ public static boolean hasStandardSecurityAnnotation(ClassInfo classInfo) {
return hasStandardSecurityAnnotation(classInfo.declaredAnnotations());
}

private static boolean hasStandardSecurityAnnotation(Collection<AnnotationInstance> instances) {
static boolean hasStandardSecurityAnnotation(Collection<AnnotationInstance> instances) {
for (AnnotationInstance instance : instances) {
if (SECURITY_ANNOTATIONS.contains(instance.name())) {
return true;
@@ -49,7 +49,7 @@ public static Optional<AnnotationInstance> findFirstStandardSecurityAnnotation(C
return findFirstStandardSecurityAnnotation(classInfo.declaredAnnotations());
}

private static Optional<AnnotationInstance> findFirstStandardSecurityAnnotation(Collection<AnnotationInstance> instances) {
static Optional<AnnotationInstance> findFirstStandardSecurityAnnotation(Collection<AnnotationInstance> instances) {
for (AnnotationInstance instance : instances) {
if (SECURITY_ANNOTATIONS.contains(instance.name())) {
return Optional.of(instance);
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package io.quarkus.security.test.cdi;

import jakarta.annotation.security.PermitAll;
import jakarta.annotation.security.RolesAllowed;
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.enterprise.event.Observes;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.runtime.Startup;
import io.quarkus.runtime.StartupEvent;
import io.quarkus.security.Authenticated;
import io.quarkus.security.PermissionsAllowed;
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 SecurityAnnotationOnObservedMethodTest {

@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addClasses(StartupMethodBean.class, IdentityMock.class, PermitAllEagerAppBean.class,
AuthData.class, SecurityTestUtils.class, EagerAppBean.class))
.assertException(throwable -> {
String errorMsg = throwable.getMessage();
Assertions.assertTrue(errorMsg.contains("StartupMethodBean#packagePrivateClassAnnotatedMethod"));
Assertions.assertTrue(errorMsg.contains("StartupMethodBean#publicInit"));
Assertions.assertFalse(errorMsg.contains("EagerAppBean"));
Assertions.assertFalse(errorMsg.contains("packagePrivateInit"));
Assertions.assertFalse(errorMsg.contains("permittedInit"));
Assertions.assertFalse(errorMsg.contains("PermitAllEagerAppBean"));
});

@Test
public void test() {
// must be here to run test
Assertions.fail();
}

@PermissionsAllowed("ignored")
public static class StartupMethodBean {

public void publicInit(@Observes StartupEvent event) {
Assertions.fail("Illegal state - validation should detect secured observed method");
}

void packagePrivateInit(@Observes StartupEvent event) {
// invoked as not intercepted by class level annotation
}

@Authenticated
void packagePrivateClassAnnotatedMethod(@Observes StartupEvent event) {
Assertions.fail("Illegal state - validation should detect secured observed method");
}

@PermitAll
public void permittedInit(@Observes StartupEvent event) {
// invoked as not secured
}

}

@RolesAllowed("ignored")
@Startup
@ApplicationScoped
public static class EagerAppBean {
}

@PermitAll
@Startup
@ApplicationScoped
public static class PermitAllEagerAppBean {
}
}

0 comments on commit e9f6d6d

Please sign in to comment.