Skip to content

Commit

Permalink
Refactor default JAX-RS sec in RESTEasy Classic
Browse files Browse the repository at this point in the history
  • Loading branch information
@michalvavrik
michalvavrik committed Feb 7, 2024
1 parent 44b8b12 commit 46b39c8
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 100 deletions.
105 changes: 8 additions & 97 deletions ...sy/deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyBuiltinsProcessor.java
View file
Original file line number Diff line number Diff line change
@@ -1,22 +1,12 @@
package io.quarkus.resteasy.deployment;

import static io.quarkus.deployment.annotations.ExecutionTime.STATIC_INIT;
import static io.quarkus.resteasy.deployment.RestPathAnnotationProcessor.getAllClassInterfaces;
import static io.quarkus.resteasy.deployment.RestPathAnnotationProcessor.isRestEndpointMethod;
import static io.quarkus.security.spi.SecurityTransformerUtils.hasSecurityAnnotation;

import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Objects;
import java.util.function.Predicate;
import java.util.stream.Collectors;

import org.jboss.jandex.ClassInfo;
import org.jboss.jandex.DotName;
import org.jboss.jandex.MethodInfo;
import org.jboss.logging.Logger;

import io.quarkus.arc.deployment.AdditionalBeanBuildItem;
import io.quarkus.deployment.Capabilities;
import io.quarkus.deployment.Capability;
Expand All @@ -25,7 +15,6 @@
import io.quarkus.deployment.annotations.BuildStep;
import io.quarkus.deployment.annotations.Record;
import io.quarkus.deployment.builditem.ApplicationArchivesBuildItem;
import io.quarkus.deployment.builditem.CombinedIndexBuildItem;
import io.quarkus.resteasy.common.spi.ResteasyJaxrsProviderBuildItem;
import io.quarkus.resteasy.runtime.AuthenticationCompletionExceptionMapper;
import io.quarkus.resteasy.runtime.AuthenticationFailedExceptionMapper;
Expand All @@ -43,8 +32,7 @@
import io.quarkus.resteasy.runtime.vertx.JsonArrayWriter;
import io.quarkus.resteasy.runtime.vertx.JsonObjectReader;
import io.quarkus.resteasy.runtime.vertx.JsonObjectWriter;
import io.quarkus.resteasy.server.common.deployment.ResteasyDeploymentBuildItem;
import io.quarkus.security.spi.AdditionalSecuredMethodsBuildItem;
import io.quarkus.security.spi.DefaultSecurityCheckBuildItem;
import io.quarkus.vertx.http.deployment.HttpRootPathBuildItem;
import io.quarkus.vertx.http.deployment.devmode.NotFoundPageDisplayableEndpointBuildItem;
import io.quarkus.vertx.http.deployment.devmode.RouteDescriptionBuildItem;
Expand All @@ -54,92 +42,15 @@
public class ResteasyBuiltinsProcessor {

protected static final String META_INF_RESOURCES = "META-INF/resources";
private static final Logger LOG = Logger.getLogger(ResteasyBuiltinsProcessor.class);

@BuildStep
void setUpDenyAllJaxRs(CombinedIndexBuildItem index,
JaxRsSecurityConfig config,
ResteasyDeploymentBuildItem resteasyDeployment,
BuildProducer<AdditionalSecuredMethodsBuildItem> additionalSecuredClasses) {
if (resteasyDeployment != null && (config.denyJaxRs || config.defaultRolesAllowed.isPresent())) {
final List<MethodInfo> methods = new ArrayList<>();

// add endpoints
List<String> resourceClasses = resteasyDeployment.getDeployment().getScannedResourceClasses();
for (String className : resourceClasses) {
ClassInfo classInfo = index.getIndex().getClassByName(DotName.createSimple(className));
if (classInfo == null)
throw new IllegalStateException("Unable to find class info for " + className);
// add unannotated class endpoints as well as parent class unannotated endpoints
addAllUnannotatedEndpoints(index, classInfo, methods);

// interface endpoints implemented on resources are already in, now we need to resolve default interface
// methods as there, CDI interceptors won't work, therefore neither will our additional secured methods
Collection<ClassInfo> interfaces = getAllClassInterfaces(index, List.of(classInfo), new ArrayList<>());
if (!interfaces.isEmpty()) {
final List<MethodInfo> interfaceEndpoints = new ArrayList<>();
for (ClassInfo anInterface : interfaces) {
addUnannotatedEndpoints(index, anInterface, interfaceEndpoints);
}
// look for implementors as implementors on resource classes are secured by CDI interceptors
if (!interfaceEndpoints.isEmpty()) {
interfaceBlock: for (MethodInfo interfaceEndpoint : interfaceEndpoints) {
if (interfaceEndpoint.isDefault()) {
for (MethodInfo endpoint : methods) {
boolean nameParamsMatch = endpoint.name().equals(interfaceEndpoint.name())
&& (interfaceEndpoint.parameterTypes().equals(endpoint.parameterTypes()));
if (nameParamsMatch) {
// whether matched method is declared on class that implements interface endpoint
Predicate<DotName> isEndpointInterface = interfaceEndpoint.declaringClass()
.name()::equals;
if (endpoint.declaringClass().interfaceNames().stream().anyMatch(isEndpointInterface)) {
continue interfaceBlock;
}
}
}
String configProperty = config.denyJaxRs ? "quarkus.security.jaxrs.deny-unannotated-endpoints"
: "quarkus.security.jaxrs.default-roles-allowed";
// this is logging only as I'm a bit worried about false positives and breaking things
// for what is very much edge case
LOG.warn("Default interface method '" + interfaceEndpoint
+ "' cannot be secured with the '" + configProperty
+ "' configuration property. Please implement this method for CDI "
+ "interceptor binding to work");
}
}
}
}
}

if (!methods.isEmpty()) {
if (config.denyJaxRs) {
additionalSecuredClasses.produce(new AdditionalSecuredMethodsBuildItem(methods));
} else {
additionalSecuredClasses
.produce(new AdditionalSecuredMethodsBuildItem(methods, config.defaultRolesAllowed));
}
}
}
}

private static void addAllUnannotatedEndpoints(CombinedIndexBuildItem index, ClassInfo classInfo,
List<MethodInfo> methods) {
if (classInfo == null) {
return;
}
addUnannotatedEndpoints(index, classInfo, methods);
if (classInfo.superClassType() != null && !classInfo.superClassType().name().equals(DotName.OBJECT_NAME)) {
addAllUnannotatedEndpoints(index, index.getIndex().getClassByName(classInfo.superClassType().name()), methods);
}
}

private static void addUnannotatedEndpoints(CombinedIndexBuildItem index, ClassInfo classInfo, List<MethodInfo> methods) {
if (!hasSecurityAnnotation(classInfo)) {
for (MethodInfo methodInfo : classInfo.methods()) {
if (isRestEndpointMethod(index, methodInfo) && !hasSecurityAnnotation(methodInfo)) {
methods.add(methodInfo);
}
}
void setUpDenyAllJaxRs(JaxRsSecurityConfig securityConfig,
BuildProducer<DefaultSecurityCheckBuildItem> defaultSecurityCheckProducer) {
if (securityConfig.denyJaxRs) {
defaultSecurityCheckProducer.produce(DefaultSecurityCheckBuildItem.denyAll());
} else if (securityConfig.defaultRolesAllowed.isPresent()) {
defaultSecurityCheckProducer
.produce(DefaultSecurityCheckBuildItem.rolesAllowed(securityConfig.defaultRolesAllowed.get()));
}
}

Expand Down
6 changes: 6 additions & 0 deletions ...assic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/EagerSecurityFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public void accept(RoutingContext routingContext) {

}
};
static final String SKIP_DEFAULT_CHECK = "io.quarkus.resteasy.runtime.EagerSecurityFilter#SKIP_DEFAULT_CHECK";
private final Map<MethodDescription, Consumer<RoutingContext>> cache = new HashMap<>();
private final EagerSecurityInterceptorStorage interceptorStorage;
private final SecurityEventHelper<AuthorizationSuccessEvent, AuthorizationFailureEvent> eventHelper;
Expand Down Expand Up @@ -86,6 +87,11 @@ public void filter(ContainerRequestContext requestContext) throws IOException {

private void applySecurityChecks(MethodDescription description) {
SecurityCheck check = securityCheckStorage.getSecurityCheck(description);
if (check == null && securityCheckStorage.getDefaultSecurityCheck() != null
&& routingContext.get(EagerSecurityFilter.class.getName()) == null
&& routingContext.get(SKIP_DEFAULT_CHECK) == null) {
check = securityCheckStorage.getDefaultSecurityCheck();
}
if (check != null) {
if (check.isPermitAll()) {
fireEventOnAuthZSuccess(check, null);
Expand Down
14 changes: 11 additions & 3 deletions ...y/runtime/src/main/java/io/quarkus/resteasy/runtime/StandardSecurityCheckInterceptor.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.quarkus.resteasy.runtime;

import static io.quarkus.resteasy.runtime.EagerSecurityFilter.SKIP_DEFAULT_CHECK;
import static io.quarkus.security.spi.runtime.SecurityHandlerConstants.EXECUTED;
import static io.quarkus.security.spi.runtime.SecurityHandlerConstants.SECURITY_HANDLER;

Expand All @@ -18,6 +19,7 @@
import io.quarkus.security.PermissionsAllowed;
import io.quarkus.security.spi.runtime.AuthorizationController;
import io.quarkus.vertx.http.runtime.CurrentVertxRequest;
import io.vertx.ext.web.RoutingContext;

/**
* Security checks for RBAC annotations on endpoints are done by the {@link EagerSecurityFilter}, this interceptor
Expand All @@ -36,9 +38,15 @@ public abstract class StandardSecurityCheckInterceptor {
public Object intercept(InvocationContext ic) throws Exception {
// RoutingContext can be null if RESTEasy is used together with other stacks that do not rely on it (e.g. gRPC)
// and this is not invoked from RESTEasy route handler
if (controller.isAuthorizationEnabled() && currentVertxRequest.getCurrent() != null) {
Method method = currentVertxRequest.getCurrent().get(EagerSecurityFilter.class.getName());
if (method != null && method.equals(ic.getMethod())) {
RoutingContext routingContext = currentVertxRequest.getCurrent();
if (controller.isAuthorizationEnabled() && routingContext != null) {
Method method = routingContext.get(EagerSecurityFilter.class.getName());
if (method == null) {
// if this interceptor is run on resource method it means this is parent method for subresource
// otherwise it would already be secured, therefore security check is applied and default JAX-RS
// security needs to be skipped (for default security is only applied on unsecured requests)
routingContext.put(SKIP_DEFAULT_CHECK, true);
} else if (method.equals(ic.getMethod())) {
ic.getContextData().put(SECURITY_HANDLER, EXECUTED);
}
}
Expand Down

0 comments on commit 46b39c8

Please sign in to comment.