Merge pull request #19598 from stuartwdouglas/resteasy-reactive-eager…

…-security Push security checks further up the handler chain
quarkusio · Aug 24, 2021 · 998b73e · 998b73e
2 parents 5d72a32 + 51c4052
commit 998b73e
Show file tree

Hide file tree

Showing 38 changed files with 409 additions and 74 deletions.
diff --git a/...c/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java b/...c/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java
@@ -72,6 +72,7 @@
 import io.quarkus.arc.runtime.BeanContainer;
 import io.quarkus.arc.runtime.ClientProxyUnwrapper;
 import io.quarkus.deployment.Capabilities;
+import io.quarkus.deployment.Capability;
 import io.quarkus.deployment.Feature;
 import io.quarkus.deployment.GeneratedClassGizmoAdaptor;
 import io.quarkus.deployment.annotations.BuildProducer;
@@ -80,6 +81,7 @@
 import io.quarkus.deployment.annotations.Record;
 import io.quarkus.deployment.builditem.ApplicationClassPredicateBuildItem;
 import io.quarkus.deployment.builditem.BytecodeTransformerBuildItem;
+import io.quarkus.deployment.builditem.CombinedIndexBuildItem;
 import io.quarkus.deployment.builditem.FeatureBuildItem;
 import io.quarkus.deployment.builditem.GeneratedClassBuildItem;
 import io.quarkus.deployment.builditem.LaunchModeBuildItem;
@@ -108,6 +110,7 @@
 import io.quarkus.resteasy.reactive.server.runtime.exceptionmappers.AuthenticationRedirectExceptionMapper;
 import io.quarkus.resteasy.reactive.server.runtime.exceptionmappers.ForbiddenExceptionMapper;
 import io.quarkus.resteasy.reactive.server.runtime.exceptionmappers.UnauthorizedExceptionMapper;
+import io.quarkus.resteasy.reactive.server.runtime.security.EagerSecurityHandler;
 import io.quarkus.resteasy.reactive.server.runtime.security.SecurityContextOverrideHandler;
 import io.quarkus.resteasy.reactive.server.spi.MethodScannerBuildItem;
 import io.quarkus.resteasy.reactive.spi.CustomExceptionMapperBuildItem;
@@ -544,6 +547,8 @@ private boolean hasAnnotation(MethodInfo method, short paramPosition, DotName an
                     .setDynamicFeatures(dynamicFeats)
                     .setSerialisers(serialisers)
                     .setApplicationPath(applicationPath)
+                    .setGlobalHandlerCustomers(
+                            new ArrayList<>(Collections.singletonList(new SecurityContextOverrideHandler.Customizer()))) //TODO: should be pluggable
                     .setResourceClasses(resourceClasses)
                     .setLocatableResourceClasses(subResourceClasses)
                     .setParamConverterProviders(paramConverterProviders);
@@ -632,12 +637,26 @@ public void securityExceptionMappers(BuildProducer<ExceptionMapperBuildItem> exc
     }
 
     @BuildStep
-    MethodScannerBuildItem integrateSecurityOverrideSupport() {
+    MethodScannerBuildItem integrateEagerSecurity(Capabilities capabilities, CombinedIndexBuildItem indexBuildItem) {
+        if (!capabilities.isPresent(Capability.SECURITY)) {
+            return null;
+        }
+        var index = indexBuildItem.getComputingIndex();
         return new MethodScannerBuildItem(new MethodScanner() {
             @Override
             public List<HandlerChainCustomizer> scan(MethodInfo method, ClassInfo actualEndpointClass,
                     Map<String, Object> methodContext) {
-                return Collections.singletonList(new SecurityContextOverrideHandler.Customizer());
+                if (SecurityTransformerUtils.hasStandardSecurityAnnotation(method)) {
+                    return Collections.singletonList(new EagerSecurityHandler.Customizer());
+                }
+                ClassInfo c = actualEndpointClass;
+                while (c.superName() != null) {
+                    if (SecurityTransformerUtils.hasStandardSecurityAnnotation(c)) {
+                        return Collections.singletonList(new EagerSecurityHandler.Customizer());
+                    }
+                    c = index.getClassByName(c.superName());
+                }
+                return Collections.emptyList();
             }
         });
     }

diff --git a/...rc/main/java/io/quarkus/resteasy/reactive/server/deployment/SecurityTransformerUtils.java b/...rc/main/java/io/quarkus/resteasy/reactive/server/deployment/SecurityTransformerUtils.java
@@ -0,0 +1,64 @@
+package io.quarkus.resteasy.reactive.server.deployment;
+
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Optional;
+import java.util.Set;
+
+import javax.annotation.security.DenyAll;
+import javax.annotation.security.PermitAll;
+import javax.annotation.security.RolesAllowed;
+
+import org.jboss.jandex.AnnotationInstance;
+import org.jboss.jandex.ClassInfo;
+import org.jboss.jandex.DotName;
+import org.jboss.jandex.MethodInfo;
+
+import io.quarkus.security.Authenticated;
+
+public class SecurityTransformerUtils {
+    public static final Set<DotName> SECURITY_BINDINGS = new HashSet<>();
+
+    static {
+        // keep the contents the same as in io.quarkus.resteasy.deployment.SecurityTransformerUtils
+        SECURITY_BINDINGS.add(DotName.createSimple(RolesAllowed.class.getName()));
+        SECURITY_BINDINGS.add(DotName.createSimple(Authenticated.class.getName()));
+        SECURITY_BINDINGS.add(DotName.createSimple(DenyAll.class.getName()));
+        SECURITY_BINDINGS.add(DotName.createSimple(PermitAll.class.getName()));
+    }
+
+    public static boolean hasStandardSecurityAnnotation(MethodInfo methodInfo) {
+        return hasStandardSecurityAnnotation(methodInfo.annotations());
+    }
+
+    public static boolean hasStandardSecurityAnnotation(ClassInfo classInfo) {
+        return hasStandardSecurityAnnotation(classInfo.classAnnotations());
+    }
+
+    private static boolean hasStandardSecurityAnnotation(Collection<AnnotationInstance> instances) {
+        for (AnnotationInstance instance : instances) {
+            if (SECURITY_BINDINGS.contains(instance.name())) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    public static Optional<AnnotationInstance> findFirstStandardSecurityAnnotation(MethodInfo methodInfo) {
+        return findFirstStandardSecurityAnnotation(methodInfo.annotations());
+    }
+
+    public static Optional<AnnotationInstance> findFirstStandardSecurityAnnotation(ClassInfo classInfo) {
+        return findFirstStandardSecurityAnnotation(classInfo.classAnnotations());
+    }
+
+    private static Optional<AnnotationInstance> findFirstStandardSecurityAnnotation(Collection<AnnotationInstance> instances) {
+        for (AnnotationInstance instance : instances) {
+            if (SECURITY_BINDINGS.contains(instance.name())) {
+                return Optional.of(instance);
+            }
+        }
+        return Optional.empty();
+    }
+
+}
diff --git a/.../io/quarkus/resteasy/reactive/server/test/security/LazyAuthRolesAllowedJaxRsTestCase.java b/.../io/quarkus/resteasy/reactive/server/test/security/LazyAuthRolesAllowedJaxRsTestCase.java
@@ -2,6 +2,8 @@
 
 import static org.hamcrest.Matchers.is;
 
+import java.util.Arrays;
+
 import org.jboss.shrinkwrap.api.ShrinkWrap;
 import org.jboss.shrinkwrap.api.asset.StringAsset;
 import org.jboss.shrinkwrap.api.spec.JavaArchive;
@@ -18,7 +20,7 @@ public class LazyAuthRolesAllowedJaxRsTestCase {
     @RegisterExtension
     static QuarkusUnitTest runner = new QuarkusUnitTest()
             .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
-                    .addClasses(RolesAllowedResource.class, UserResource.class,
+                    .addClasses(RolesAllowedResource.class, RolesAllowedBlockingResource.class, UserResource.class,
                             TestIdentityProvider.class,
                             TestIdentityController.class,
                             UnsecuredSubResource.class)
@@ -34,12 +36,14 @@ public static void setupUsers() {
 
     @Test
     public void testRolesAllowed() {
-        RestAssured.get("/roles").then().statusCode(401);
-        RestAssured.given().auth().basic("admin", "admin").get("/roles").then().statusCode(200);
-        RestAssured.given().auth().basic("admin", "wrong").get("/roles").then().statusCode(401);
-        RestAssured.given().auth().basic("user", "user").get("/roles").then().statusCode(200);
-        RestAssured.given().auth().basic("admin", "admin").get("/roles/admin").then().statusCode(200);
-        RestAssured.given().auth().basic("user", "user").get("/roles/admin").then().statusCode(403);
+        Arrays.asList("/roles", "/roles-blocking").forEach((path) -> {
+            RestAssured.get(path).then().statusCode(401);
+            RestAssured.given().auth().basic("admin", "admin").get(path).then().statusCode(200);
+            RestAssured.given().auth().basic("admin", "wrong").get(path).then().statusCode(401);
+            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);
+        });
     }
 
     @Test

diff --git a/...teasy/reactive/server/test/security/ReplaceIdentityLazyAuthRolesAllowedJaxRsTestCase.java b/...teasy/reactive/server/test/security/ReplaceIdentityLazyAuthRolesAllowedJaxRsTestCase.java
@@ -2,6 +2,8 @@
 
 import static org.hamcrest.Matchers.is;
 
+import java.util.Arrays;
+
 import org.jboss.shrinkwrap.api.ShrinkWrap;
 import org.jboss.shrinkwrap.api.asset.StringAsset;
 import org.jboss.shrinkwrap.api.spec.JavaArchive;
@@ -18,7 +20,7 @@ public class ReplaceIdentityLazyAuthRolesAllowedJaxRsTestCase {
     @RegisterExtension
     static QuarkusUnitTest runner = new QuarkusUnitTest()
             .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
-                    .addClasses(RolesAllowedResource.class, UserResource.class,
+                    .addClasses(RolesAllowedResource.class, UserResource.class, RolesAllowedBlockingResource.class,
                             TestIdentityProvider.class,
                             TestIdentityController.class,
                             SecurityOverrideFilter.class,
@@ -36,14 +38,17 @@ public static void setupUsers() {
     @Test
     public void testRolesAllowedModified() {
         //make sure that things work as normal when no modification happens
-        RestAssured.given()
-                .header("user", "admin")
-                .header("role", "admin")
-                .get("/roles").then().statusCode(200);
-        RestAssured.given()
-                .auth().basic("user", "user")
-                .header("user", "admin")
-                .header("role", "admin").get("/roles/admin").then().statusCode(200);
+
+        Arrays.asList("/roles", "/roles-blocking").forEach((path) -> {
+            RestAssured.given()
+                    .header("user", "admin")
+                    .header("role", "admin")
+                    .get(path).then().statusCode(200);
+            RestAssured.given()
+                    .auth().basic("user", "user")
+                    .header("user", "admin")
+                    .header("role", "admin").get(path + "/admin").then().statusCode(200);
+        });
     }
 
     @Test

diff --git a/.../java/io/quarkus/resteasy/reactive/server/test/security/RolesAllowedBlockingResource.java b/.../java/io/quarkus/resteasy/reactive/server/test/security/RolesAllowedBlockingResource.java
@@ -0,0 +1,30 @@
+package io.quarkus.resteasy.reactive.server.test.security;
+
+import javax.annotation.security.PermitAll;
+import javax.annotation.security.RolesAllowed;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+
+import io.smallrye.common.annotation.Blocking;
+
+/**
+ * @author Michal Szynkiewicz, [email protected]
+ */
+@Path("/roles-blocking")
+@PermitAll
+@Blocking
+public class RolesAllowedBlockingResource {
+    @GET
+    @RolesAllowed({ "user", "admin" })
+    public String defaultSecurity() {
+        return "default";
+    }
+
+    @Path("/admin")
+    @RolesAllowed("admin")
+    @GET
+    public String admin() {
+        return "admin";
+    }
+
+}
diff --git a/...est/java/io/quarkus/resteasy/reactive/server/test/security/RolesAllowedJaxRsTestCase.java b/...est/java/io/quarkus/resteasy/reactive/server/test/security/RolesAllowedJaxRsTestCase.java
@@ -2,8 +2,21 @@
 
 import static org.hamcrest.Matchers.is;
 
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Type;
+import java.util.Arrays;
+
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.MultivaluedMap;
+import javax.ws.rs.ext.MessageBodyReader;
+import javax.ws.rs.ext.Provider;
+
 import org.jboss.shrinkwrap.api.ShrinkWrap;
 import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -17,7 +30,8 @@ public class RolesAllowedJaxRsTestCase {
     @RegisterExtension
     static QuarkusUnitTest runner = new QuarkusUnitTest()
             .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
-                    .addClasses(RolesAllowedResource.class, UserResource.class,
+                    .addClasses(RolesAllowedResource.class, UserResource.class, RolesAllowedBlockingResource.class,
+                            SerializationEntity.class, SerializationRolesResource.class,
                             TestIdentityProvider.class,
                             TestIdentityController.class,
                             UnsecuredSubResource.class));
@@ -31,11 +45,13 @@ public static void setupUsers() {
 
     @Test
     public void testRolesAllowed() {
-        RestAssured.get("/roles").then().statusCode(401);
-        RestAssured.given().auth().basic("admin", "admin").get("/roles").then().statusCode(200);
-        RestAssured.given().auth().basic("user", "user").get("/roles").then().statusCode(200);
-        RestAssured.given().auth().basic("admin", "admin").get("/roles/admin").then().statusCode(200);
-        RestAssured.given().auth().basic("user", "user").get("/roles/admin").then().statusCode(403);
+        Arrays.asList("/roles", "/roles-blocking").forEach((path) -> {
+            RestAssured.get(path).then().statusCode(401);
+            RestAssured.given().auth().basic("admin", "admin").get(path).then().statusCode(200);
+            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);
+        });
     }
 
     @Test
@@ -46,4 +62,47 @@ public void testUser() {
         RestAssured.given().auth().basic("user", "user").get("/user").then().body(is(""));
         RestAssured.given().auth().preemptive().basic("user", "user").get("/user").then().body(is("user"));
     }
+
+    @Test
+    public void testSecurityRunsBeforeValidation() {
+        read = false;
+        RestAssured.given().body(new SerializationEntity()).post("/roles-validate").then().statusCode(401);
+        Assertions.assertFalse(read);
+        RestAssured.given().body(new SerializationEntity()).auth().basic("admin", "admin").post("/roles-validate").then()
+                .statusCode(200);
+        Assertions.assertTrue(read);
+        read = false;
+        RestAssured.given().body(new SerializationEntity()).auth().basic("user", "user").post("/roles-validate").then()
+                .statusCode(200);
+        Assertions.assertTrue(read);
+        read = false;
+        RestAssured.given().body(new SerializationEntity()).auth().basic("admin", "admin").post("/roles-validate/admin").then()
+                .statusCode(200);
+        Assertions.assertTrue(read);
+        read = false;
+        RestAssured.given().body(new SerializationEntity()).auth().basic("user", "user").post("/roles-validate/admin").then()
+                .statusCode(403);
+        Assertions.assertFalse(read);
+    }
+
+    static volatile boolean read = false;
+
+    @Provider
+    public static class Reader implements MessageBodyReader<SerializationEntity> {
+
+        @Override
+        public boolean isReadable(Class<?> type, Type genericType, Annotation[] annotations, MediaType mediaType) {
+            return true;
+        }
+
+        @Override
+        public SerializationEntity readFrom(Class<SerializationEntity> type, Type genericType, Annotation[] annotations,
+                MediaType mediaType, MultivaluedMap<String, String> httpHeaders, InputStream entityStream)
+                throws IOException, WebApplicationException {
+            read = true;
+            SerializationEntity entity = new SerializationEntity();
+            entity.setName("read");
+            return entity;
+        }
+    }
 }
diff --git a/...src/test/java/io/quarkus/resteasy/reactive/server/test/security/RolesAllowedResource.java b/...src/test/java/io/quarkus/resteasy/reactive/server/test/security/RolesAllowedResource.java
@@ -5,14 +5,11 @@
 import javax.ws.rs.GET;
 import javax.ws.rs.Path;
 
-import io.smallrye.common.annotation.Blocking;
-
 /**
  * @author Michal Szynkiewicz, [email protected]
  */
 @Path("/roles")
 @PermitAll
-@Blocking
 public class RolesAllowedResource {
     @GET
     @RolesAllowed({ "user", "admin" })

diff --git a/.../src/test/java/io/quarkus/resteasy/reactive/server/test/security/SerializationEntity.java b/.../src/test/java/io/quarkus/resteasy/reactive/server/test/security/SerializationEntity.java
@@ -0,0 +1,14 @@
+package io.quarkus.resteasy.reactive.server.test.security;
+
+public class SerializationEntity {
+    private String name;
+
+    public String getName() {
+        return name;
+    }
+
+    public SerializationEntity setName(String name) {
+        this.name = name;
+        return this;
+    }
+}
diff --git a/...st/java/io/quarkus/resteasy/reactive/server/test/security/SerializationRolesResource.java b/...st/java/io/quarkus/resteasy/reactive/server/test/security/SerializationRolesResource.java
@@ -0,0 +1,27 @@
+package io.quarkus.resteasy.reactive.server.test.security;
+
+import javax.annotation.security.PermitAll;
+import javax.annotation.security.RolesAllowed;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+
+import io.smallrye.common.annotation.Blocking;
+
+@Path("/roles-validate")
+@PermitAll
+@Blocking
+public class SerializationRolesResource {
+    @POST
+    @RolesAllowed({ "user", "admin" })
+    public String defaultSecurity(SerializationEntity entity) {
+        return entity.getName();
+    }
+
+    @Path("/admin")
+    @RolesAllowed("admin")
+    @POST
+    public String admin(SerializationEntity entity) {
+        return entity.getName();
+    }
+
+}