From d0b11f5a04617f9128df97a4af3b81e400aff410 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Vav=C5=99=C3=ADk?= <mvavrik@redhat.com> Date: Tue, 5 Jul 2022 22:29:27 +0200 Subject: [PATCH] RESTEasy Reactive - prevent repeating of standard security checks fix: #26536 --- .../deployment/ResteasyReactiveProcessor.java | 12 +++ .../StandardSecurityCheckInterceptor.java | 73 +++++++++++++++++++ .../security/EagerSecurityHandler.java | 14 +++- .../spi/runtime/SecurityHandlerConstants.java | 15 ++++ .../runtime/interceptor/SecurityHandler.java | 8 +- 5 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/StandardSecurityCheckInterceptor.java create mode 100644 extensions/security/runtime-spi/src/main/java/io/quarkus/security/spi/runtime/SecurityHandlerConstants.java diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java index 7514a6409fcac..dabe6a90114b2 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java @@ -138,6 +138,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; @@ -1044,6 +1045,17 @@ public List<HandlerChainCustomizer> scan(MethodInfo method, ClassInfo actualEndp }); } + @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)) { diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/StandardSecurityCheckInterceptor.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/StandardSecurityCheckInterceptor.java new file mode 100644 index 0000000000000..96576a0111a97 --- /dev/null +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/StandardSecurityCheckInterceptor.java @@ -0,0 +1,73 @@ +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 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; + +/** + * 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 { + + @Inject + AuthorizationController controller; + + @AroundInvoke + public Object intercept(InvocationContext ic) throws Exception { + if (controller.isAuthorizationEnabled() + && alreadyDoneByEagerSecurityHandler(CurrentRequestManager.get().getProperty(SECURITY_HANDLER))) { + ic.getContextData().put(SECURITY_HANDLER, EXECUTED); + } + return ic.proceed(); + } + + private boolean alreadyDoneByEagerSecurityHandler(Object property) { + return property != null && property.equals(EXECUTED); + } + + /** + * 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 { + + } +} diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/security/EagerSecurityHandler.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/security/EagerSecurityHandler.java index ad0d2f5bd44c5..7f39797f5a1cd 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/security/EagerSecurityHandler.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/security/EagerSecurityHandler.java @@ -1,5 +1,8 @@ package io.quarkus.resteasy.reactive.server.runtime.security; +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 java.util.Collections; import java.util.List; @@ -75,7 +78,9 @@ public void handle(ResteasyReactiveRequestContext requestContext) throws Excepti requestContext.requireCDIRequestScope(); SecurityCheck theCheck = check; - if (!theCheck.isPermitAll()) { + if (theCheck.isPermitAll()) { + preventRepeatedSecurityChecks(requestContext); + } else { requestContext.suspend(); Uni<SecurityIdentity> deferredIdentity = getCurrentIdentityAssociation().get().getDeferredIdentity(); @@ -95,6 +100,7 @@ public void handle(ResteasyReactiveRequestContext requestContext) throws Excepti public Object apply(SecurityIdentity securityIdentity) { theCheck.apply(securityIdentity, methodDescription, requestContext.getParameters()); + preventRepeatedSecurityChecks(requestContext); return null; } }) @@ -117,6 +123,12 @@ public void onFailure(Throwable failure) { } } + private void preventRepeatedSecurityChecks(ResteasyReactiveRequestContext requestContext) { + // propagate information that security check has been performed to the SecurityHandler + // via io.quarkus.resteasy.reactive.server.runtime.StandardSecurityCheckInterceptor + requestContext.setProperty(SECURITY_HANDLER, EXECUTED); + } + private InjectableInstance<CurrentIdentityAssociation> getCurrentIdentityAssociation() { InjectableInstance<CurrentIdentityAssociation> identityAssociation = this.currentIdentityAssociation; if (identityAssociation == null) { diff --git a/extensions/security/runtime-spi/src/main/java/io/quarkus/security/spi/runtime/SecurityHandlerConstants.java b/extensions/security/runtime-spi/src/main/java/io/quarkus/security/spi/runtime/SecurityHandlerConstants.java new file mode 100644 index 0000000000000..bdff2d77ab0e6 --- /dev/null +++ b/extensions/security/runtime-spi/src/main/java/io/quarkus/security/spi/runtime/SecurityHandlerConstants.java @@ -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 = "SECURITY_HANDLER"; + + /** + * 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"; +} diff --git a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/interceptor/SecurityHandler.java b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/interceptor/SecurityHandler.java index ab0153df22bc3..6c960be600c19 100644 --- a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/interceptor/SecurityHandler.java +++ b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/interceptor/SecurityHandler.java @@ -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; @@ -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; @@ -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<?>> {