Skip to content

Commit

Permalink
RESTEasy Reactive - prevent repeating of standard security checks
Browse files Browse the repository at this point in the history
fixes: #26536
  • Loading branch information
michalvavrik committed Jul 12, 2022
1 parent 4e046f5 commit a5c9818
Show file tree
Hide file tree
Showing 13 changed files with 450 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ public ContextRegistrationPhaseBuildItem initialize(
List<ResourceAnnotationBuildItem> resourceAnnotations,
List<BeanDefiningAnnotationBuildItem> additionalBeanDefiningAnnotations,
List<SuppressConditionGeneratorBuildItem> suppressConditionGenerators,
List<MethodInterceptorBindingFilterBuildItem> methodInterceptorBindingFilterBuildItems,
Optional<TestClassPredicateBuildItem> testClassPredicate,
Capabilities capabilities,
CustomScopeAnnotationsBuildItem customScopes,
Expand Down Expand Up @@ -274,6 +275,11 @@ public void transform(TransformationContext transformationContext) {
for (InterceptorBindingRegistrarBuildItem registrar : interceptorBindingRegistrars) {
builder.addInterceptorBindingRegistrar(registrar.getInterceptorBindingRegistrar());
}
// register method interceptor binding filters
for (MethodInterceptorBindingFilterBuildItem filterBuildItem : methodInterceptorBindingFilterBuildItems) {
builder.addMethodInterceptorFilter(MethodDescriptor.of(filterBuildItem.interceptedMethod),
filterBuildItem.shouldRemoveBinding);
}
// register additional qualifiers
for (QualifierRegistrarBuildItem registrar : qualifierRegistrars) {
builder.addQualifierRegistrar(registrar.getQualifierRegistrar());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package io.quarkus.arc.deployment;

import java.util.Objects;
import java.util.function.Predicate;

import org.jboss.jandex.MethodInfo;

import io.quarkus.arc.processor.InterceptorInfo;
import io.quarkus.builder.item.MultiBuildItem;

/**
* Makes it possible to break binding between interceptors and concrete method. An interceptor won't
* intercept the {@code interceptedMethod} anymore if {@code shouldRemoveBinding} is true, but the rest of interceptors
* bindings are honoured.
*
* Only a single filter (build item) per method is supported, if there are multiple filters registered for the method,
* the last one is used.
*/
public final class MethodInterceptorBindingFilterBuildItem extends MultiBuildItem {

/**
* If evaluated as true, {@code interceptedMethod} won't be intercepted by the interceptor
*/
final Predicate<InterceptorInfo> shouldRemoveBinding;

/**
* Intercepted method, only non-static methods are supported.
*/
final MethodInfo interceptedMethod;

public MethodInterceptorBindingFilterBuildItem(Predicate<InterceptorInfo> shouldRemoveBinding,
MethodInfo interceptedMethod) {
this.shouldRemoveBinding = Objects.requireNonNull(shouldRemoveBinding);
this.interceptedMethod = Objects.requireNonNull(interceptedMethod);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
package io.quarkus.arc.test.interceptor;

import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.ElementType.TYPE;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.lang.annotation.Retention;
import java.lang.annotation.Target;
import java.util.List;
import java.util.function.Predicate;

import javax.annotation.Priority;
import javax.inject.Inject;
import javax.inject.Singleton;
import javax.interceptor.AroundInvoke;
import javax.interceptor.Interceptor;
import javax.interceptor.InvocationContext;

import org.jboss.jandex.DotName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.arc.deployment.BeanArchiveIndexBuildItem;
import io.quarkus.arc.deployment.InterceptorBindingRegistrarBuildItem;
import io.quarkus.arc.deployment.MethodInterceptorBindingFilterBuildItem;
import io.quarkus.arc.processor.InterceptorBindingRegistrar;
import io.quarkus.arc.processor.InterceptorInfo;
import io.quarkus.builder.BuildContext;
import io.quarkus.builder.BuildStep;
import io.quarkus.test.QuarkusUnitTest;

public class MethodInterceptorBindingFilterTest {

@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addClasses(NotAnInterceptorBinding.class, PingPongBean.class, PingInterceptor.class,
PungInterceptor.class))
.addBuildChainCustomizer(b -> {
b.addBuildStep(new BuildStep() {
@Override
public void execute(BuildContext context) {
context.produce(new InterceptorBindingRegistrarBuildItem(new InterceptorBindingRegistrar() {
@Override
public List<InterceptorBinding> getAdditionalBindings() {
return List.of(InterceptorBinding.of(NotAnInterceptorBinding.class));
}
}));
}
}).produces(InterceptorBindingRegistrarBuildItem.class).build();
})
.addBuildChainCustomizer(b -> {
b.addBuildStep(new BuildStep() {
@Override
public void execute(BuildContext context) {
var pingMethodInfo = context
.consume(BeanArchiveIndexBuildItem.class)
.getIndex()
.getClassByName(DotName.createSimple(PingPongBean.class.getName()))
.methods()
.stream()
.filter(mi -> mi.name().contains("ping"))
.findFirst()
.orElseThrow();
Predicate<InterceptorInfo> removePungInterceptorBinding = interceptorInfo -> interceptorInfo
.getTarget()
.map(t -> t.asClass().name().equals(DotName.createSimple(PungInterceptor.class.getName())))
.orElse(false);
context.produce(
new MethodInterceptorBindingFilterBuildItem(removePungInterceptorBinding, pingMethodInfo));
}
}).consumes(BeanArchiveIndexBuildItem.class).produces(MethodInterceptorBindingFilterBuildItem.class).build();
});

@Inject
PingPongBean bean;

@Test
public void testInterceptor() {
assertEquals("PING PONG", bean.ping());
}

@Singleton
static class PingPongBean {

@NotAnInterceptorBinding
public String ping() {
return "PONG";
}

}

@Priority(1)
@Interceptor
@NotAnInterceptorBinding
static class PingInterceptor {

@AroundInvoke
Object aroundInvoke(InvocationContext ctx) throws Exception {
return "PING " + ctx.proceed();
}

}

@Priority(2)
@Interceptor
@NotAnInterceptorBinding
static class PungInterceptor {

@AroundInvoke
Object aroundInvoke(InvocationContext ctx) throws Exception {
return "PUNG " + ctx.proceed();
}

}

@Target({ TYPE, METHOD })
@Retention(RUNTIME)
@interface NotAnInterceptorBinding {

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
import static io.quarkus.resteasy.reactive.common.deployment.QuarkusResteasyReactiveDotNames.HTTP_SERVER_REQUEST;
import static io.quarkus.resteasy.reactive.common.deployment.QuarkusResteasyReactiveDotNames.HTTP_SERVER_RESPONSE;
import static io.quarkus.resteasy.reactive.common.deployment.QuarkusResteasyReactiveDotNames.ROUTING_CONTEXT;
import static io.quarkus.resteasy.reactive.server.deployment.SecurityTransformerUtils.SECURITY_BINDINGS;
import static java.util.stream.Collectors.toList;
import static org.jboss.resteasy.reactive.common.processor.EndpointIndexer.findEndpoints;
import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.DATE_FORMAT;

import java.io.File;
Expand Down Expand Up @@ -65,6 +67,7 @@
import org.jboss.resteasy.reactive.common.processor.TargetJavaVersion;
import org.jboss.resteasy.reactive.common.processor.scanning.ApplicationScanningResult;
import org.jboss.resteasy.reactive.common.processor.scanning.ResourceScanningResult;
import org.jboss.resteasy.reactive.common.processor.transformation.AnnotationStore;
import org.jboss.resteasy.reactive.common.processor.transformation.AnnotationsTransformer;
import org.jboss.resteasy.reactive.common.types.AllWriteableMarker;
import org.jboss.resteasy.reactive.common.util.Encode;
Expand Down Expand Up @@ -168,6 +171,8 @@
import io.quarkus.security.AuthenticationCompletionException;
import io.quarkus.security.AuthenticationRedirectException;
import io.quarkus.security.ForbiddenException;
import io.quarkus.security.spi.AssumedStandardSecurityCheckBuildItem;
import io.quarkus.security.spi.runtime.SecurityCheckStorage;
import io.quarkus.vertx.http.deployment.RouteBuildItem;
import io.quarkus.vertx.http.runtime.HttpBuildTimeConfig;
import io.quarkus.vertx.http.runtime.VertxHttpRecorder;
Expand Down Expand Up @@ -323,7 +328,15 @@ void registerCustomExceptionMappers(BuildProducer<CustomExceptionMapperBuildItem

@BuildStep
public void unremovableBeans(Optional<ResourceScanningResultBuildItem> resourceScanningResultBuildItem,
List<AssumedStandardSecurityCheckBuildItem> assumedStandardSecurityCheckBuildItems,
BuildProducer<UnremovableBeanBuildItem> unremovableBeans) {

if (!assumedStandardSecurityCheckBuildItems.isEmpty()) {
// Necessary as the Quarkus Security doesn't need the SecurityCheckStorage bean in cases
// when only security checks are for endpoints and these are handled exclusively by EagerSecurityHandler
unremovableBeans.produce(UnremovableBeanBuildItem.beanTypes(SecurityCheckStorage.class));
}

if (!resourceScanningResultBuildItem.isPresent()) {
return;
}
Expand Down Expand Up @@ -1038,11 +1051,8 @@ MethodScannerBuildItem integrateEagerSecurity(Capabilities capabilities, Combine
return null;
}

final boolean denyJaxRs = ConfigProvider.getConfig()
.getOptionalValue("quarkus.security.jaxrs.deny-unannotated-endpoints", Boolean.class).orElse(false);
final boolean hasDefaultJaxRsRolesAllowed = ConfigProvider.getConfig()
.getOptionalValues("quarkus.security.jaxrs.default-roles-allowed", String.class).map(l -> !l.isEmpty())
.orElse(false);
final boolean denyJaxRs = getDenyJaxRs();
final boolean hasDefaultJaxRsRolesAllowed = getHasDefaultJaxRsRolesAllowed();
var index = indexBuildItem.getComputingIndex();
return new MethodScannerBuildItem(new MethodScanner() {
@Override
Expand All @@ -1061,6 +1071,85 @@ public List<HandlerChainCustomizer> scan(MethodInfo method, ClassInfo actualEndp
});
}

private boolean getHasDefaultJaxRsRolesAllowed() {
return ConfigProvider.getConfig()
.getOptionalValues("quarkus.security.jaxrs.default-roles-allowed", String.class)
.map(l -> !l.isEmpty())
.orElse(false);
}

private boolean getDenyJaxRs() {
return ConfigProvider.getConfig()
.getOptionalValue("quarkus.security.jaxrs.deny-unannotated-endpoints", Boolean.class)
.orElse(false);
}

/**
* Prevent repeating {@link io.quarkus.security.spi.runtime.SecurityCheck}s in the Quarkus Security on methods
* checked by {@link EagerSecurityHandler}.
*/
@BuildStep
void preventRepeatingSecurityChecks(Capabilities capabilities,
Optional<ResourceScanningResultBuildItem> resourceScanningResultBuildItem,
List<AnnotationsTransformerBuildItem> annotationTransformerBuildItems,
BuildProducer<AssumedStandardSecurityCheckBuildItem> assumedSecurityAnnotationCheckBuildItemBuildProducer,
ApplicationResultBuildItem applicationResultBuildItem, BeanArchiveIndexBuildItem beanArchiveIndexBuildItem) {

if (!capabilities.isPresent(Capability.SECURITY) || resourceScanningResultBuildItem.isEmpty()) {
return;
}

final boolean denyJaxRs = getDenyJaxRs();
final boolean hasDefaultJaxRsRolesAllowed = getHasDefaultJaxRsRolesAllowed();
final IndexView index = beanArchiveIndexBuildItem.getIndex();
final AnnotationStore annotationStore;
if (!annotationTransformerBuildItems.isEmpty()) {
List<AnnotationsTransformer> annotationsTransformers = new ArrayList<>(annotationTransformerBuildItems.size());
for (AnnotationsTransformerBuildItem bi : annotationTransformerBuildItems) {
annotationsTransformers.add(bi.getAnnotationsTransformer());
}
annotationStore = new AnnotationStore(annotationsTransformers);
} else {
annotationStore = new AnnotationStore(null);
}

// collect all endpoints
final Set<MethodInfo> endpoints = findEndpoints(resourceScanningResultBuildItem.get().getResult(), annotationStore,
index, applicationResultBuildItem.getResult());

// tell the Quarkus Security we're going to handle standard security checks for the endpoints ourselves
assumeEndpointSecurityCheck: for (MethodInfo endpoint : endpoints) {

if (hasDefaultJaxRsRolesAllowed || denyJaxRs) {
assumedSecurityAnnotationCheckBuildItemBuildProducer.produce(
new AssumedStandardSecurityCheckBuildItem(endpoint));
continue;
}

// find security annotation declared on the method
for (AnnotationInstance annotation : endpoint.annotations()) {
if (SECURITY_BINDINGS.contains(annotation.name())) {
assumedSecurityAnnotationCheckBuildItemBuildProducer.produce(
new AssumedStandardSecurityCheckBuildItem(endpoint));
continue assumeEndpointSecurityCheck;
}
}

// find security annotation declared on the class
ClassInfo c = endpoint.declaringClass();
while (c.superName() != null) {
for (AnnotationInstance annotation : c.classAnnotations()) {
if (SECURITY_BINDINGS.contains(annotation.name())) {
assumedSecurityAnnotationCheckBuildItemBuildProducer.produce(
new AssumedStandardSecurityCheckBuildItem(endpoint));
continue assumeEndpointSecurityCheck;
}
}
c = index.getClassByName(c.superName());
}
}
}

/**
* This results in adding {@link AllWriteableMarker} to user provided {@link MessageBodyWriter} classes
* that handle every class
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@
import java.lang.reflect.Type;
import java.util.Arrays;

import javax.annotation.Priority;
import javax.annotation.security.RolesAllowed;
import javax.interceptor.AroundInvoke;
import javax.interceptor.Interceptor;
import javax.interceptor.InvocationContext;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.MultivaluedMap;
Expand All @@ -19,6 +24,7 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.security.runtime.interceptor.SecurityHandler;
import io.quarkus.security.test.utils.TestIdentityController;
import io.quarkus.security.test.utils.TestIdentityProvider;
import io.quarkus.test.QuarkusUnitTest;
Expand All @@ -32,7 +38,7 @@ public class RolesAllowedJaxRsTestCase {
SerializationEntity.class, SerializationRolesResource.class,
TestIdentityProvider.class,
TestIdentityController.class,
UnsecuredSubResource.class));
UnsecuredSubResource.class, AssureNoDuplicateSecurityChecksInterceptor.class));

@BeforeAll
public static void setupUsers() {
Expand Down Expand Up @@ -103,4 +109,16 @@ public SerializationEntity readFrom(Class<SerializationEntity> type, Type generi
return entity;
}
}

@Interceptor
@RolesAllowed("")
@Priority(Interceptor.Priority.PLATFORM_AFTER)
public static class AssureNoDuplicateSecurityChecksInterceptor {

@AroundInvoke
public Object intercept(InvocationContext ic) throws Exception {
Assertions.assertNull(ic.getContextData().get(SecurityHandler.class.getName()));
return ic.proceed();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public void handle(ResteasyReactiveRequestContext requestContext) throws Excepti
Uni<SecurityIdentity> deferredIdentity = getCurrentIdentityAssociation().get().getDeferredIdentity();

// if proactive auth is disabled, then accessing SecurityIdentity is a blocking operation for synchronous methods
// setting identity here will enable SecurityInterceptors registered in Quarkus Security Deployment to run checks
// setting identity here will allow users to access SecurityIdentity in a synchronous manner
if (isProactiveAuthDisabled && lazyMethod.isNonBlocking) {
deferredIdentity = deferredIdentity.call(securityIdentity -> {
if (securityIdentity != null) {
Expand Down
Loading

0 comments on commit a5c9818

Please sign in to comment.