Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RESTEasy Reactive - prevent repeating of standard security checks #26567

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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";
michalvavrik marked this conversation as resolved.
Show resolved Hide resolved

@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