From 551edd2178e1c1bd2adab4f5c130731bb0bad240 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Vav=C5=99=C3=ADk?= Date: Fri, 1 Jul 2022 00:04:00 +0200 Subject: [PATCH] Resolve Sec. Identity in RESTEasy Reactive when Proactive Auth disabled f ix #23547 for RESTEasy Reactive cases (e.g. the issue reproducer) --- .../deployment/ResteasyReactiveProcessor.java | 6 ++- .../LazyAuthRolesAllowedJaxRsTestCase.java | 3 ++ .../RolesAllowedBlockingResource.java | 15 ++++++ .../test/security/RolesAllowedResource.java | 16 ++++++ .../security/EagerSecurityHandler.java | 50 +++++++++++++++++-- .../startup/RuntimeResourceDeployment.java | 3 +- .../spi/ResteasyReactiveResourceInfo.java | 7 ++- 7 files changed, 93 insertions(+), 7 deletions(-) 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 58b0ecf4b2df1..57e2f1a5dd99a 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 @@ -1025,7 +1025,8 @@ public void securityExceptionMappers(BuildProducer exc } @BuildStep - MethodScannerBuildItem integrateEagerSecurity(Capabilities capabilities, CombinedIndexBuildItem indexBuildItem) { + MethodScannerBuildItem integrateEagerSecurity(Capabilities capabilities, CombinedIndexBuildItem indexBuildItem, + HttpBuildTimeConfig httpBuildTimeConfig) { if (!capabilities.isPresent(Capability.SECURITY)) { return null; } @@ -1036,7 +1037,8 @@ public List scan(MethodInfo method, ClassInfo actualEndp Map methodContext) { return Objects.requireNonNullElse( consumeStandardSecurityAnnotations(method, actualEndpointClass, index, - (c) -> Collections.singletonList(new EagerSecurityHandler.Customizer())), + (c) -> Collections.singletonList( + EagerSecurityHandler.Customizer.newInstance(httpBuildTimeConfig.auth.proactive))), Collections.emptyList()); } }); diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/LazyAuthRolesAllowedJaxRsTestCase.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/LazyAuthRolesAllowedJaxRsTestCase.java index 4b7baebdb2438..264e69c5969b2 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/LazyAuthRolesAllowedJaxRsTestCase.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/LazyAuthRolesAllowedJaxRsTestCase.java @@ -4,6 +4,7 @@ import java.util.Arrays; +import org.hamcrest.Matchers; import org.jboss.shrinkwrap.api.asset.StringAsset; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -41,6 +42,8 @@ public void testRolesAllowed() { RestAssured.given().auth().basic("user", "user").get(path).then().statusCode(200); RestAssured.given().auth().basic("admin", "admin").get(path + "/admin").then().statusCode(200); RestAssured.given().auth().basic("user", "user").get(path + "/admin").then().statusCode(403); + RestAssured.given().auth().basic("admin", "admin").get(path + "/admin/security-identity").then().statusCode(200) + .body(Matchers.is("admin")); }); } diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/RolesAllowedBlockingResource.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/RolesAllowedBlockingResource.java index 6d44a2d7e0faf..d3fa6f2e7aa3d 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/RolesAllowedBlockingResource.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/RolesAllowedBlockingResource.java @@ -2,10 +2,13 @@ import javax.annotation.security.PermitAll; import javax.annotation.security.RolesAllowed; +import javax.inject.Inject; import javax.ws.rs.GET; import javax.ws.rs.Path; +import io.quarkus.security.identity.CurrentIdentityAssociation; import io.smallrye.common.annotation.Blocking; +import io.smallrye.common.annotation.NonBlocking; /** * @author Michal Szynkiewicz, michal.l.szynkiewicz@gmail.com @@ -14,6 +17,10 @@ @PermitAll @Blocking public class RolesAllowedBlockingResource { + + @Inject + CurrentIdentityAssociation currentIdentityAssociation; + @GET @RolesAllowed({ "user", "admin" }) public String defaultSecurity() { @@ -27,4 +34,12 @@ public String admin() { return "admin"; } + @NonBlocking + @Path("/admin/security-identity") + @RolesAllowed("admin") + @GET + public String getSecurityIdentity() { + return currentIdentityAssociation.getIdentity().getPrincipal().getName(); + } + } diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/RolesAllowedResource.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/RolesAllowedResource.java index 842e04d7d0ec8..3ea7a09536802 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/RolesAllowedResource.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/RolesAllowedResource.java @@ -2,15 +2,23 @@ import javax.annotation.security.PermitAll; import javax.annotation.security.RolesAllowed; +import javax.inject.Inject; import javax.ws.rs.GET; import javax.ws.rs.Path; +import io.quarkus.security.identity.CurrentIdentityAssociation; +import io.smallrye.common.annotation.NonBlocking; + /** * @author Michal Szynkiewicz, michal.l.szynkiewicz@gmail.com */ @Path("/roles") @PermitAll public class RolesAllowedResource { + + @Inject + CurrentIdentityAssociation currentIdentityAssociation; + @GET @RolesAllowed({ "user", "admin" }) public String defaultSecurity() { @@ -24,4 +32,12 @@ public String admin() { return "admin"; } + @NonBlocking + @Path("/admin/security-identity") + @RolesAllowed("admin") + @GET + public String getSecurityIdentity() { + return currentIdentityAssociation.getIdentity().getPrincipal().getName(); + } + } 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 21078343d3390..ad0d2f5bd44c5 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 @@ -20,6 +20,7 @@ import io.quarkus.security.spi.runtime.MethodDescription; import io.quarkus.security.spi.runtime.SecurityCheck; import io.quarkus.security.spi.runtime.SecurityCheckStorage; +import io.smallrye.mutiny.Uni; import io.smallrye.mutiny.subscription.UniSubscriber; import io.smallrye.mutiny.subscription.UniSubscription; @@ -37,10 +38,15 @@ public void apply(SecurityIdentity identity, MethodDescription method, Object[] } }; + private final boolean isProactiveAuthDisabled; private volatile InjectableInstance currentIdentityAssociation; private volatile SecurityCheck check; private volatile AuthorizationController authorizationController; + public EagerSecurityHandler(boolean isProactiveAuthDisabled) { + this.isProactiveAuthDisabled = isProactiveAuthDisabled; + } + @Override public void handle(ResteasyReactiveRequestContext requestContext) throws Exception { if (this.check == NULL_SENTINEL) { @@ -66,11 +72,25 @@ public void handle(ResteasyReactiveRequestContext requestContext) throws Excepti if (!authorizationController.isAuthorizationEnabled()) { return; } + requestContext.requireCDIRequestScope(); SecurityCheck theCheck = check; if (!theCheck.isPermitAll()) { requestContext.suspend(); - getCurrentIdentityAssociation().get().getDeferredIdentity().map(new Function() { + Uni 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 + if (isProactiveAuthDisabled && lazyMethod.isNonBlocking) { + deferredIdentity = deferredIdentity.call(securityIdentity -> { + if (securityIdentity != null) { + getCurrentIdentityAssociation().get().setIdentity(securityIdentity); + } + return Uni.createFrom().item(securityIdentity); + }); + } + + deferredIdentity.map(new Function() { @Override public Object apply(SecurityIdentity securityIdentity) { theCheck.apply(securityIdentity, methodDescription, @@ -105,14 +125,38 @@ private InjectableInstance getCurrentIdentityAssocia return identityAssociation; } - public static class Customizer implements HandlerChainCustomizer { + public static abstract class Customizer implements HandlerChainCustomizer { + + public static Customizer newInstance(boolean isProactiveAuthEnabled) { + return isProactiveAuthEnabled ? new ProactiveAuthEnabledCustomizer() : new ProactiveAuthDisabledCustomizer(); + } + + protected abstract boolean isProactiveAuthDisabled(); + @Override public List handlers(Phase phase, ResourceClass resourceClass, ServerResourceMethod serverResourceMethod) { if (phase == Phase.AFTER_MATCH) { - return Collections.singletonList(new EagerSecurityHandler()); + return Collections.singletonList(new EagerSecurityHandler(isProactiveAuthDisabled())); } return Collections.emptyList(); } + + public static class ProactiveAuthEnabledCustomizer extends Customizer { + + @Override + protected boolean isProactiveAuthDisabled() { + return false; + } + } + + public static class ProactiveAuthDisabledCustomizer extends Customizer { + + @Override + protected boolean isProactiveAuthDisabled() { + return true; + } + } + } } diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/startup/RuntimeResourceDeployment.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/startup/RuntimeResourceDeployment.java index 92cd5fe43a8bd..74a2f370e7e2c 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/startup/RuntimeResourceDeployment.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/startup/RuntimeResourceDeployment.java @@ -176,7 +176,8 @@ public RuntimeResource buildResourceMethod(ResourceClass clazz, } ResteasyReactiveResourceInfo lazyMethod = new ResteasyReactiveResourceInfo(method.getName(), resourceClass, - parameterDeclaredUnresolvedTypes, classAnnotationNames, method.getMethodAnnotationNames()); + parameterDeclaredUnresolvedTypes, classAnnotationNames, method.getMethodAnnotationNames(), + !defaultBlocking && !method.isBlocking()); RuntimeInterceptorDeployment.MethodInterceptorContext interceptorDeployment = runtimeInterceptorDeployment .forMethod(method, lazyMethod); diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/spi/ResteasyReactiveResourceInfo.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/spi/ResteasyReactiveResourceInfo.java index 932bb1ffde13c..4d9da9f5e0f15 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/spi/ResteasyReactiveResourceInfo.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/spi/ResteasyReactiveResourceInfo.java @@ -20,6 +20,10 @@ public class ResteasyReactiveResourceInfo implements ResourceInfo { private final Class[] parameterTypes; private final Set classAnnotationNames; private final Set methodAnnotationNames; + /** + * If it's non-blocking method within the runtime that won't always default to blocking + */ + public final boolean isNonBlocking; private volatile Annotation[] classAnnotations; private volatile Method method; @@ -28,12 +32,13 @@ public class ResteasyReactiveResourceInfo implements ResourceInfo { private volatile String methodId; public ResteasyReactiveResourceInfo(String name, Class declaringClass, Class[] parameterTypes, - Set classAnnotationNames, Set methodAnnotationNames) { + Set classAnnotationNames, Set methodAnnotationNames, boolean isNonBlocking) { this.name = name; this.declaringClass = declaringClass; this.parameterTypes = parameterTypes; this.classAnnotationNames = classAnnotationNames; this.methodAnnotationNames = methodAnnotationNames; + this.isNonBlocking = isNonBlocking; } public String getName() {