Skip to content

Commit

Permalink
RESTEasy Reactive - Prevent repeated security checks
Browse files Browse the repository at this point in the history
  • Loading branch information
michalvavrik authored and fercomunello committed Aug 31, 2022
1 parent 18e4cd0 commit c172971
Show file tree
Hide file tree
Showing 8 changed files with 172 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@
import io.quarkus.resteasy.reactive.server.runtime.ResteasyReactiveRecorder;
import io.quarkus.resteasy.reactive.server.runtime.ResteasyReactiveRuntimeRecorder;
import io.quarkus.resteasy.reactive.server.runtime.ResteasyReactiveServerRuntimeConfig;
import io.quarkus.resteasy.reactive.server.runtime.StandardSecurityCheckInterceptor;
import io.quarkus.resteasy.reactive.server.runtime.exceptionmappers.AuthenticationCompletionExceptionMapper;
import io.quarkus.resteasy.reactive.server.runtime.exceptionmappers.AuthenticationFailedExceptionMapper;
import io.quarkus.resteasy.reactive.server.runtime.exceptionmappers.AuthenticationRedirectExceptionMapper;
Expand Down Expand Up @@ -1131,6 +1132,17 @@ public void visit(int version, int access, String name, String signature,

}

@BuildStep
void registerSecurityInterceptors(Capabilities capabilities,
BuildProducer<AdditionalBeanBuildItem> beans) {
if (capabilities.isPresent(Capability.SECURITY)) {
// Register interceptors for standard security annotations to prevent repeated security checks
beans.produce(new AdditionalBeanBuildItem(StandardSecurityCheckInterceptor.RolesAllowedInterceptor.class,
StandardSecurityCheckInterceptor.AuthenticatedInterceptor.class,
StandardSecurityCheckInterceptor.PermitAllInterceptor.class));
}
}

private <T> T consumeStandardSecurityAnnotations(MethodInfo methodInfo, ClassInfo classInfo, IndexView index,
Function<ClassInfo, T> function) {
if (SecurityTransformerUtils.hasStandardSecurityAnnotation(methodInfo)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,9 @@ public class RolesAllowedJaxRsTestCase {
static QuarkusUnitTest runner = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addClasses(RolesAllowedResource.class, UserResource.class, RolesAllowedBlockingResource.class,
SerializationEntity.class, SerializationRolesResource.class,
TestIdentityProvider.class,
TestIdentityController.class,
UnsecuredSubResource.class));
SerializationEntity.class, SerializationRolesResource.class, TestIdentityProvider.class,
TestIdentityController.class, UnsecuredSubResource.class, RolesAllowedService.class,
RolesAllowedServiceResource.class));

@BeforeAll
public static void setupUsers() {
Expand Down Expand Up @@ -61,6 +60,14 @@ public void testUser() {
RestAssured.given().auth().preemptive().basic("user", "user").get("/user").then().body(is("user"));
}

@Test
public void testNestedRolesAllowed() {
// there are 2 different checks in place: user & admin on resource, admin on service
RestAssured.given().auth().basic("admin", "admin").get("/roles-service/hello").then().statusCode(200)
.body(is(RolesAllowedService.SERVICE_HELLO));
RestAssured.given().auth().basic("user", "user").get("/roles-service/hello").then().statusCode(403);
}

@Test
public void testSecurityRunsBeforeValidation() {
read = false;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package io.quarkus.resteasy.reactive.server.test.security;

import javax.annotation.security.RolesAllowed;
import javax.enterprise.context.ApplicationScoped;

@ApplicationScoped
public class RolesAllowedService {

public static final String SERVICE_HELLO = "Hello from Service!";

@RolesAllowed("admin")
public String hello() {
return SERVICE_HELLO;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package io.quarkus.resteasy.reactive.server.test.security;

import javax.annotation.security.RolesAllowed;
import javax.inject.Inject;
import javax.ws.rs.GET;
import javax.ws.rs.Path;

@Path("/roles-service")
public class RolesAllowedServiceResource {

@Inject
RolesAllowedService rolesAllowedService;

@Path("/hello")
@RolesAllowed({ "user", "admin" })
@GET
public String getServiceHello() {
return rolesAllowedService.hello();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package io.quarkus.resteasy.reactive.server.runtime;

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

import java.lang.reflect.Method;

import javax.annotation.Priority;
import javax.annotation.security.DenyAll;
import javax.annotation.security.PermitAll;
import javax.annotation.security.RolesAllowed;
import javax.inject.Inject;
import javax.interceptor.AroundInvoke;
import javax.interceptor.Interceptor;
import javax.interceptor.InvocationContext;

import org.jboss.resteasy.reactive.server.core.CurrentRequestManager;

import io.quarkus.security.Authenticated;
import io.quarkus.security.spi.runtime.AuthorizationController;
import io.quarkus.security.spi.runtime.MethodDescription;

/**
* Security checks for RBAC annotations on endpoints are done by
* the {@link io.quarkus.resteasy.reactive.server.runtime.security.EagerSecurityHandler},
* this interceptor propagates the information to the SecurityHandler to prevent repeated checks. The {@link DenyAll}
* security check is performed just once.
*/
public abstract class StandardSecurityCheckInterceptor {

public static final String STANDARD_SECURITY_CHECK_INTERCEPTOR = StandardSecurityCheckInterceptor.class.getName();

@Inject
AuthorizationController controller;

@AroundInvoke
public Object intercept(InvocationContext ic) throws Exception {
if (controller.isAuthorizationEnabled() && CurrentRequestManager.get() != null
&& alreadyDoneByEagerSecurityHandler(
CurrentRequestManager.get().getProperty(STANDARD_SECURITY_CHECK_INTERCEPTOR), ic.getMethod())) {
ic.getContextData().put(SECURITY_HANDLER, EXECUTED);
}
return ic.proceed();
}

private boolean alreadyDoneByEagerSecurityHandler(Object methodWithFinishedChecks, Method method) {
// compare methods: EagerSecurityHandler only intercept endpoints, we still want SecurityHandler run for CDI beans
return methodWithFinishedChecks != null && MethodDescription.ofMethod(method).equals(methodWithFinishedChecks);
}

/**
* Prevent the SecurityHandler from performing {@link RolesAllowed} security checks
*/
@Interceptor
@RolesAllowed("")
@Priority(Interceptor.Priority.PLATFORM_BEFORE)
public static final class RolesAllowedInterceptor extends StandardSecurityCheckInterceptor {

}

/**
* Prevent the SecurityHandler from performing {@link javax.annotation.security.PermitAll} security checks
*/
@Interceptor
@PermitAll
@Priority(Interceptor.Priority.PLATFORM_BEFORE)
public static final class PermitAllInterceptor extends StandardSecurityCheckInterceptor {

}

/**
* Prevent the SecurityHandler from performing {@link Authenticated} security checks
*/
@Interceptor
@Authenticated
@Priority(Interceptor.Priority.PLATFORM_BEFORE)
public static final class AuthenticatedInterceptor extends StandardSecurityCheckInterceptor {

}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.quarkus.resteasy.reactive.server.runtime.security;

import static io.quarkus.resteasy.reactive.server.runtime.StandardSecurityCheckInterceptor.STANDARD_SECURITY_CHECK_INTERCEPTOR;

import java.lang.reflect.Method;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -75,7 +77,9 @@ public void handle(ResteasyReactiveRequestContext requestContext) throws Excepti

requestContext.requireCDIRequestScope();
SecurityCheck theCheck = check;
if (!theCheck.isPermitAll()) {
if (theCheck.isPermitAll()) {
preventRepeatedSecurityChecks(requestContext, methodDescription);
} else {
requestContext.suspend();
Uni<SecurityIdentity> deferredIdentity = getCurrentIdentityAssociation().get().getDeferredIdentity();

Expand All @@ -95,6 +99,7 @@ public void handle(ResteasyReactiveRequestContext requestContext) throws Excepti
public Object apply(SecurityIdentity securityIdentity) {
theCheck.apply(securityIdentity, methodDescription,
requestContext.getParameters());
preventRepeatedSecurityChecks(requestContext, methodDescription);
return null;
}
})
Expand All @@ -117,6 +122,13 @@ public void onFailure(Throwable failure) {
}
}

private void preventRepeatedSecurityChecks(ResteasyReactiveRequestContext requestContext,
MethodDescription methodDescription) {
// propagate information that security check has been performed on this method to the SecurityHandler
// via io.quarkus.resteasy.reactive.server.runtime.StandardSecurityCheckInterceptor
requestContext.setProperty(STANDARD_SECURITY_CHECK_INTERCEPTOR, methodDescription);
}

private InjectableInstance<CurrentIdentityAssociation> getCurrentIdentityAssociation() {
InjectableInstance<CurrentIdentityAssociation> identityAssociation = this.currentIdentityAssociation;
if (identityAssociation == null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package io.quarkus.security.spi.runtime;

public class SecurityHandlerConstants {

/**
* Invocation context data key used by the SecurityHandler to save a security checks state
*/
public static final String SECURITY_HANDLER = "io.quarkus.security.securityHandler";

/**
* The SecurityHandler keep a state of security checks in the Invocation context data to prevent repeated checks.
* `executed` means the check has already been done.
*/
public static final String EXECUTED = "executed";
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package io.quarkus.security.runtime.interceptor;

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

import java.util.concurrent.CompletionStage;
import java.util.function.Function;

Expand All @@ -16,9 +19,6 @@
@Singleton
public class SecurityHandler {

private static final String HANDLER_NAME = SecurityHandler.class.getName();
private static final String EXECUTED = "executed";

@Inject
SecurityConstrainer constrainer;

Expand Down Expand Up @@ -49,7 +49,7 @@ public Object handle(InvocationContext ic) throws Exception {
}

private boolean alreadyHandled(InvocationContext ic) {
return ic.getContextData().put(HANDLER_NAME, EXECUTED) != null;
return ic.getContextData().put(SECURITY_HANDLER, EXECUTED) != null;
}

private static class UniContinuation implements Function<Object, Uni<?>> {
Expand Down

0 comments on commit c172971

Please sign in to comment.