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
  • Loading branch information
michalvavrik committed Oct 27, 2023
1 parent d110270 commit cc4f5ac
Show file tree
Hide file tree
Showing 3 changed files with 158 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;
Expand Down Expand Up @@ -42,7 +46,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;
Expand Down Expand Up @@ -72,6 +82,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;
Expand Down Expand Up @@ -101,6 +113,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;

Expand Down Expand Up @@ -578,7 +591,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())));
Expand Down Expand Up @@ -810,6 +823,66 @@ 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()
.filter(observer -> observer.asObserver().getObservedType().name().equals(STARTUP_EVENT_NAME))
.map(ObserverInfo::asObserver)
.forEach(observer -> {
if (observer.getObserverMethod() == null) {
// synthetic observer created for @Startup
ClassInfo ci = beanArchiveIndexBuildItem.getIndex().getClassByName(observer.getBeanClass());
if (ci != null) {
findFirstStandardSecurityAnnotation(annotationStore.getAnnotations(ci))
.map(AnnotationInstance::name)
.filter(name -> !name.equals(PERMIT_ALL))
.ifPresent(securityAnnotation -> {
var errorMsg = String.format(
"Class '%s' cannot observe '%s' as the class is secured with the '%s' annotation",
ci, STARTUP_EVENT_NAME, securityAnnotation);
validationErrorProducer
.produce(new ValidationErrorBuildItem(new ConfigurationException(errorMsg)));
});
}
} else {
MethodInfo mi = observer.getObserverMethod();
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++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
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.assertTrue(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 {

EagerAppBean() {
Assertions.fail("Illegal state - validation should detect secured observed method");
}
}

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

0 comments on commit cc4f5ac

Please sign in to comment.