Improve Security events by assuring we add identity and RoutingContext whenever available and help to locate security check with secured method property

extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/AbstractSecurityEventTest.java

@@ -8,6 +8,7 @@
 
 import java.time.Duration;
 import java.util.List;
+import java.util.Objects;
 import java.util.concurrent.CopyOnWriteArrayList;
 
 import jakarta.enterprise.event.Observes;
@@ -22,6 +23,7 @@
 
 import io.quarkus.security.AuthenticationFailedException;
 import io.quarkus.security.identity.SecurityIdentity;
+import io.quarkus.security.runtime.interceptor.check.RolesAllowedCheck;
 import io.quarkus.security.spi.runtime.AuthenticationFailureEvent;
 import io.quarkus.security.spi.runtime.AuthenticationSuccessEvent;
 import io.quarkus.security.spi.runtime.AuthorizationFailureEvent;
@@ -38,7 +40,7 @@ public abstract class AbstractSecurityEventTest {
     protected static final Class<?>[] TEST_CLASSES = {
             RolesAllowedResource.class, TestIdentityProvider.class, TestIdentityController.class,
             UnsecuredResource.class, UnsecuredSubResource.class, EventObserver.class, UnsecuredResourceInterface.class,
-            UnsecuredParentResource.class
+            UnsecuredParentResource.class, RolesAllowedService.class, RolesAllowedServiceResource.class
     };
 
     @Inject
@@ -95,6 +97,99 @@ public void testRolesAllowed() {
         assertAsyncAuthZFailureObserved(2);
     }
 
+    @Test
+    public void testNestedRolesAllowed() {
+        // there are 2 different checks in place: user & admin on resource, admin on service
+        RestAssured.given().auth().preemptive().basic("admin", "admin").get("/roles-service/hello").then().statusCode(200)
+                .body(is(RolesAllowedService.SERVICE_HELLO));
+        assertSyncObserved(3);
+        AuthenticationSuccessEvent successEvent = (AuthenticationSuccessEvent) observer.syncEvents.get(0);
+        SecurityIdentity identity = successEvent.getSecurityIdentity();
+        assertNotNull(identity);
+        assertEquals("admin", identity.getPrincipal().getName());
+        RoutingContext routingContext = (RoutingContext) successEvent.getEventProperties().get(RoutingContext.class.getName());
+        assertNotNull(routingContext);
+        assertTrue(routingContext.request().path().endsWith("/roles-service/hello"));
+        // authorization success on endpoint
+        AuthorizationSuccessEvent authZSuccessEvent = (AuthorizationSuccessEvent) observer.syncEvents.get(1);
+        assertEquals(identity, authZSuccessEvent.getSecurityIdentity());
+        identity = authZSuccessEvent.getSecurityIdentity();
+        assertEquals(routingContext, authZSuccessEvent.getEventProperties().get(RoutingContext.class.getName()));
+        String securedMethod = (String) authZSuccessEvent.getEventProperties()
+                .get(AuthorizationSuccessEvent.SECURED_METHOD_KEY);
+        assertEquals("io.quarkus.resteasy.test.security.RolesAllowedServiceResource#getServiceHello",
+                securedMethod);
+        // authorization success on service level performed by CDI interceptor
+        authZSuccessEvent = (AuthorizationSuccessEvent) observer.syncEvents.get(2);
+        securedMethod = (String) authZSuccessEvent.getEventProperties().get(AuthorizationSuccessEvent.SECURED_METHOD_KEY);
+        assertEquals("io.quarkus.resteasy.test.security.RolesAllowedService#hello", securedMethod);
+        assertEquals(identity, authZSuccessEvent.getSecurityIdentity());
+        assertNotNull(authZSuccessEvent.getEventProperties().get(RoutingContext.class.getName()));
+        assertAsyncAuthZFailureObserved(0);
+        RestAssured.given().auth().preemptive().basic("user", "user").get("/roles-service/hello").then().statusCode(403);
+        assertSyncObserved(6);
+        // "roles-service" Jakarta REST resource requires 'admin' or 'user' role, therefore check succeeds
+        successEvent = (AuthenticationSuccessEvent) observer.syncEvents.get(3);
+        identity = successEvent.getSecurityIdentity();
+        assertNotNull(identity);
+        assertEquals("user", identity.getPrincipal().getName());
+        routingContext = (RoutingContext) successEvent.getEventProperties().get(RoutingContext.class.getName());
+        assertNotNull(routingContext);
+        assertTrue(routingContext.request().path().endsWith("/roles-service/hello"));
+        authZSuccessEvent = (AuthorizationSuccessEvent) observer.syncEvents.get(4);
+        assertEquals(identity, authZSuccessEvent.getSecurityIdentity());
+        assertEquals(routingContext, authZSuccessEvent.getEventProperties().get(RoutingContext.class.getName()));
+        // RolesService requires 'admin' role, therefore user fails
+        assertAsyncAuthZFailureObserved(1);
+        AuthorizationFailureEvent authZFailureEvent = observer.asyncAuthZFailureEvents.get(0);
+        securedMethod = (String) authZFailureEvent.getEventProperties().get(AuthorizationFailureEvent.SECURED_METHOD_KEY);
+        assertEquals("io.quarkus.resteasy.test.security.RolesAllowedService#hello", securedMethod);
+        SecurityIdentity userIdentity = authZFailureEvent.getSecurityIdentity();
+        assertNotNull(userIdentity);
+        assertTrue(userIdentity.hasRole("user"));
+        assertEquals("user", userIdentity.getPrincipal().getName());
+        assertNotNull(authZFailureEvent.getEventProperties().get(RoutingContext.class.getName()));
+        assertEquals(RolesAllowedCheck.class.getName(), authZFailureEvent.getAuthorizationContext());
+    }
+
+    @Test
+    public void testNestedPermitAll() {
+        // @PermitAll is on CDI bean but resource is not secured
+        RestAssured.given().auth().preemptive().basic("admin", "admin").get("/roles-service/bye").then().statusCode(200)
+                .body(is(RolesAllowedService.SERVICE_BYE));
+        final int expectedEventsCount;
+        if (isProactiveAuth()) {
+            // auth + @PermitAll
+            expectedEventsCount = 2;
+        } else {
+            // @PermitAll
+            expectedEventsCount = 1;
+        }
+        assertSyncObserved(expectedEventsCount, true);
+
+        if (expectedEventsCount == 2) {
+            AuthenticationSuccessEvent successEvent = (AuthenticationSuccessEvent) observer.syncEvents.get(0);
+            SecurityIdentity identity = successEvent.getSecurityIdentity();
+            assertNotNull(identity);
+            assertEquals("admin", identity.getPrincipal().getName());
+            RoutingContext routingContext = (RoutingContext) successEvent.getEventProperties()
+                    .get(RoutingContext.class.getName());
+            assertNotNull(routingContext);
+            assertTrue(routingContext.request().path().endsWith("/roles-service/bye"));
+        }
+        // authorization success on service level performed by CDI interceptor
+        var authZSuccessEvent = (AuthorizationSuccessEvent) observer.syncEvents.get(expectedEventsCount - 1);
+        String securedMethod = (String) authZSuccessEvent.getEventProperties()
+                .get(AuthorizationSuccessEvent.SECURED_METHOD_KEY);
+        assertEquals("io.quarkus.resteasy.test.security.RolesAllowedService#bye", securedMethod);
+        assertNotNull(authZSuccessEvent.getEventProperties().get(RoutingContext.class.getName()));
+        if (isProactiveAuth()) {
+            assertNotNull(authZSuccessEvent.getSecurityIdentity());
+            assertEquals("admin", authZSuccessEvent.getSecurityIdentity().getPrincipal().getName());
+        }
+        assertAsyncAuthZFailureObserved(0);
+    }
+
     @Test
     public void testAuthenticated() {
         RestAssured.given().auth().preemptive().basic("admin", "admin").get("/unsecured/authenticated").then().statusCode(200)
@@ -115,6 +210,8 @@ public void testAuthenticated() {
         SecurityIdentity anonymousIdentity = authZFailure.getSecurityIdentity();
         assertNotNull(anonymousIdentity);
         assertTrue(anonymousIdentity.isAnonymous());
+        String securedMethod = (String) authZFailure.getEventProperties().get(AuthorizationFailureEvent.SECURED_METHOD_KEY);
+        assertEquals("io.quarkus.resteasy.test.security.UnsecuredResource#authenticated", securedMethod);
     }
 
     @Test
@@ -152,8 +249,7 @@ public void testPermitAll() {
             assertNotNull(routingContext);
             assertTrue(routingContext.request().path().endsWith("/unsecured/permitAll"));
             AuthorizationSuccessEvent authZSuccessEvent = (AuthorizationSuccessEvent) observer.syncEvents.get(1);
-            // SecurityIdentity is not required for the permit all check
-            assertNull(authZSuccessEvent.getSecurityIdentity());
+            assertNotNull(authZSuccessEvent.getSecurityIdentity());
         } else {
             assertSyncObserved(1, true);
             AuthorizationSuccessEvent authZSuccessEvent = (AuthorizationSuccessEvent) observer.syncEvents.get(0);
@@ -186,6 +282,9 @@ private void assertAsyncAuthZFailureObserved(int count) {
                 .untilAsserted(() -> assertEquals(count, observer.asyncAuthZFailureEvents.size()));
         if (count > 0) {
             assertTrue(observer.asyncAuthZFailureEvents.stream().allMatch(e -> e.getSecurityIdentity() != null));
+            assertTrue(observer.asyncAuthZFailureEvents.stream()
+                    .map(e -> e.getEventProperties().get(RoutingContext.class.getName()))
+                    .allMatch(Objects::nonNull));
         }
     }
 

extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/RolesAllowedService.java

@@ -0,0 +1,23 @@
+package io.quarkus.resteasy.test.security;
+
+import jakarta.annotation.security.PermitAll;
+import jakarta.annotation.security.RolesAllowed;
+import jakarta.enterprise.context.ApplicationScoped;
+
+@ApplicationScoped
+public class RolesAllowedService {
+
+    public static final String SERVICE_HELLO = "Hello from Service!";
+    public static final String SERVICE_BYE = "Bye from Service!";
+
+    @RolesAllowed("admin")
+    public String hello() {
+        return SERVICE_HELLO;
+    }
+
+    @PermitAll
+    public String bye() {
+        return SERVICE_BYE;
+    }
+
+}

extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/RolesAllowedServiceResource.java

@@ -0,0 +1,27 @@
+package io.quarkus.resteasy.test.security;
+
+import jakarta.annotation.security.RolesAllowed;
+import jakarta.inject.Inject;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+
+@Path("/roles-service")
+public class RolesAllowedServiceResource {
+
+    @Inject
+    RolesAllowedService rolesAllowedService;
+
+    @Path("/hello")
+    @RolesAllowed({ "user", "admin" })
+    @GET
+    public String getServiceHello() {
+        return rolesAllowedService.hello();
+    }
+
+    @Path("/bye")
+    @GET
+    public String getServiceBye() {
+        return rolesAllowedService.bye();
+    }
+
+}

extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/EagerSecurityFilter.java

@@ -22,7 +22,9 @@
 import io.quarkus.security.spi.runtime.MethodDescription;
 import io.quarkus.security.spi.runtime.SecurityCheck;
 import io.quarkus.security.spi.runtime.SecurityCheckStorage;
+import io.quarkus.vertx.http.runtime.CurrentVertxRequest;
 import io.quarkus.vertx.http.runtime.security.EagerSecurityInterceptorStorage;
+import io.quarkus.vertx.http.runtime.security.QuarkusHttpUser;
 import io.vertx.ext.web.RoutingContext;
 
 @Priority(Priorities.AUTHENTICATION)
@@ -35,7 +37,7 @@ public class EagerSecurityFilter implements ContainerRequestFilter {
     ResourceInfo resourceInfo;
 
     @Inject
-    RoutingContext routingContext;
+    CurrentVertxRequest currentVertxRequest;
 
     @Inject
     SecurityCheckStorage securityCheckStorage;
@@ -71,19 +73,27 @@ public void filter(ContainerRequestContext requestContext) throws IOException {
     private void applySecurityChecks(MethodDescription description) {
         SecurityCheck check = securityCheckStorage.getSecurityCheck(description);
         if (check == null && securityCheckStorage.getDefaultSecurityCheck() != null
-                && routingContext.get(EagerSecurityFilter.class.getName()) == null
-                && routingContext.get(SKIP_DEFAULT_CHECK) == null) {
+                && routingContext().get(EagerSecurityFilter.class.getName()) == null
+                && routingContext().get(SKIP_DEFAULT_CHECK) == null) {
             check = securityCheckStorage.getDefaultSecurityCheck();
         }
         if (check != null) {
             if (check.isPermitAll()) {
-                fireEventOnAuthZSuccess(check, null);
+                // add the identity only if authentication has already finished
+                final SecurityIdentity identity;
+                if (routingContext().user() instanceof QuarkusHttpUser user) {
+                    identity = user.getSecurityIdentity();
+                } else {
+                    identity = null;
+                }
+
+                fireEventOnAuthZSuccess(check, identity, description);
             } else {
                 if (check.requiresMethodArguments()) {
                     if (identityAssociation.getIdentity().isAnonymous()) {
                         var exception = new UnauthorizedException();
                         if (jaxRsPermissionChecker.getEventHelper().fireEventOnFailure()) {
-                            fireEventOnAuthZFailure(exception, check);
+                            fireEventOnAuthZFailure(exception, check, description);
                         }
                         throw exception;
                     }
@@ -94,36 +104,43 @@ private void applySecurityChecks(MethodDescription description) {
                     try {
                         check.apply(identityAssociation.getIdentity(), description, null);
                     } catch (Exception e) {
-                        fireEventOnAuthZFailure(e, check);
+                        fireEventOnAuthZFailure(e, check, description);
                         throw e;
                     }
                 } else {
                     check.apply(identityAssociation.getIdentity(), description, null);
                 }
-                fireEventOnAuthZSuccess(check, identityAssociation.getIdentity());
+                fireEventOnAuthZSuccess(check, identityAssociation.getIdentity(), description);
             }
             // prevent repeated security checks
-            routingContext.put(EagerSecurityFilter.class.getName(), resourceInfo.getResourceMethod());
+            routingContext().put(EagerSecurityFilter.class.getName(), resourceInfo.getResourceMethod());
         }
     }
 
-    private void fireEventOnAuthZFailure(Exception exception, SecurityCheck check) {
+    private void fireEventOnAuthZFailure(Exception exception, SecurityCheck check, MethodDescription description) {
         jaxRsPermissionChecker.getEventHelper().fireFailureEvent(new AuthorizationFailureEvent(
                 identityAssociation.getIdentity(), exception, check.getClass().getName(),
-                Map.of(RoutingContext.class.getName(), routingContext)));
+                Map.of(RoutingContext.class.getName(), routingContext()), description));
     }
 
-    private void fireEventOnAuthZSuccess(SecurityCheck check, SecurityIdentity securityIdentity) {
+    private void fireEventOnAuthZSuccess(SecurityCheck check, SecurityIdentity securityIdentity,
+            MethodDescription description) {
         if (jaxRsPermissionChecker.getEventHelper().fireEventOnSuccess()) {
             jaxRsPermissionChecker.getEventHelper().fireSuccessEvent(new AuthorizationSuccessEvent(securityIdentity,
-                    check.getClass().getName(), Map.of(RoutingContext.class.getName(), routingContext)));
+                    check.getClass().getName(), Map.of(RoutingContext.class.getName(), routingContext()), description));
         }
     }
 
+    private RoutingContext routingContext() {
+        // use actual RoutingContext (not the bean) to async events are invoked with new CDI request context
+        // where the RoutingContext is not available
+        return currentVertxRequest.getCurrent();
+    }
+
     private void applyEagerSecurityInterceptors(MethodDescription description) {
         var interceptor = interceptorStorage.getInterceptor(description);
         if (interceptor != null) {
-            interceptor.accept(routingContext);
+            interceptor.accept(routingContext());
         }
     }
 }