Fix slow Arc req ctx on duplicated Vertx ctx & enable sec events

quarkusio · Jan 15, 2024 · 81a872d · 81a872d
1 parent e073a61
commit 81a872d
Show file tree

Hide file tree

Showing 8 changed files with 93 additions and 92 deletions.
diff --git a/docs/src/main/asciidoc/security-customization.adoc b/docs/src/main/asciidoc/security-customization.adoc
@@ -691,8 +691,6 @@ Depending on the application, that can be a lot of the `AuthenticationSuccessEve
 For that reason, asynchronous processing can have positive effect on performance.
 <2> Common code for all supported security event types is possible because they all implement the `io.quarkus.security.spi.runtime.SecurityEvent` interface.
 
-IMPORTANT: The gRPC extension currently does not support security events.
-
 == References
 
 * xref:security-overview.adoc[Quarkus Security overview]

diff --git a/extensions/grpc/deployment/src/main/java/io/quarkus/grpc/deployment/GrpcServerProcessor.java b/extensions/grpc/deployment/src/main/java/io/quarkus/grpc/deployment/GrpcServerProcessor.java
@@ -45,15 +45,13 @@
 import io.quarkus.arc.deployment.BeanContainerBuildItem;
 import io.quarkus.arc.deployment.CustomScopeAnnotationsBuildItem;
 import io.quarkus.arc.deployment.RecorderBeanInitializedBuildItem;
-import io.quarkus.arc.deployment.SynthesisFinishedBuildItem;
 import io.quarkus.arc.deployment.SyntheticBeanBuildItem;
 import io.quarkus.arc.deployment.SyntheticBeansRuntimeInitBuildItem;
 import io.quarkus.arc.deployment.UnremovableBeanBuildItem;
 import io.quarkus.arc.deployment.ValidationPhaseBuildItem;
 import io.quarkus.arc.processor.AnnotationsTransformer;
 import io.quarkus.arc.processor.BeanInfo;
 import io.quarkus.arc.processor.BuiltinScope;
-import io.quarkus.arc.processor.ObserverInfo;
 import io.quarkus.deployment.ApplicationArchive;
 import io.quarkus.deployment.Capabilities;
 import io.quarkus.deployment.Capability;
@@ -69,7 +67,6 @@
 import io.quarkus.deployment.builditem.ExtensionSslNativeSupportBuildItem;
 import io.quarkus.deployment.builditem.FeatureBuildItem;
 import io.quarkus.deployment.builditem.LaunchModeBuildItem;
-import io.quarkus.deployment.builditem.RuntimeConfigSetupCompleteBuildItem;
 import io.quarkus.deployment.builditem.ServiceStartBuildItem;
 import io.quarkus.deployment.builditem.ShutdownContextBuildItem;
 import io.quarkus.deployment.builditem.nativeimage.NativeImageResourceBuildItem;
@@ -93,7 +90,6 @@
 import io.quarkus.kubernetes.spi.KubernetesPortBuildItem;
 import io.quarkus.netty.deployment.MinNettyAllocatorMaxOrderBuildItem;
 import io.quarkus.runtime.LaunchMode;
-import io.quarkus.security.spi.runtime.SecurityEvent;
 import io.quarkus.smallrye.health.deployment.spi.HealthBuildItem;
 import io.quarkus.vertx.deployment.VertxBuildItem;
 import io.quarkus.vertx.http.deployment.VertxWebRouterBuildItem;
@@ -797,36 +793,4 @@ void initGrpcSecurityInterceptor(List<BindableServiceBuildItem> bindables, Capab
         }
     }
 
-    @Record(RUNTIME_INIT)
-    @Consume(RuntimeConfigSetupCompleteBuildItem.class)
-    @BuildStep
-    void validateSecurityEventsNotObserved(SynthesisFinishedBuildItem synthesisFinished,
-            Capabilities capabilities,
-            GrpcSecurityRecorder recorder,
-            BeanArchiveIndexBuildItem indexBuildItem) {
-        if (!capabilities.isPresent(Capability.SECURITY)) {
-            return;
-        }
-
-        // collect all SecurityEvent classes
-        Set<DotName> knownSecurityEventClasses = new HashSet<>();
-        knownSecurityEventClasses.add(DotName.createSimple(SecurityEvent.class));
-        indexBuildItem
-                .getIndex()
-                .getAllKnownImplementors(SecurityEvent.class)
-                .stream()
-                .map(ClassInfo::name)
-                .forEach(knownSecurityEventClasses::add);
-
-        // find at least one CDI observer and validate security events are disabled
-        knownClasses: for (DotName knownSecurityEventClass : knownSecurityEventClasses) {
-            for (ObserverInfo observer : synthesisFinished.getObservers()) {
-                if (observer.getObservedType().name().equals(knownSecurityEventClass)) {
-                    recorder.validateSecurityEventsDisabled(knownSecurityEventClass.toString());
-                    break knownClasses;
-                }
-            }
-        }
-    }
-
 }
diff --git a/extensions/grpc/deployment/src/test/java/io/quarkus/grpc/auth/GrpcAuthTestBase.java b/extensions/grpc/deployment/src/test/java/io/quarkus/grpc/auth/GrpcAuthTestBase.java
@@ -2,8 +2,14 @@
 
 import static com.example.security.Security.ThreadInfo.newBuilder;
 import static io.quarkus.grpc.auth.BlockingHttpSecurityPolicy.BLOCK_REQUEST;
+import static io.quarkus.security.spi.runtime.AuthorizationSuccessEvent.AUTHORIZATION_CONTEXT;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.awaitility.Awaitility.await;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
@@ -12,11 +18,13 @@
 import java.util.concurrent.atomic.AtomicReference;
 
 import jakarta.annotation.security.RolesAllowed;
+import jakarta.inject.Inject;
 
 import org.jboss.shrinkwrap.api.ShrinkWrap;
 import org.jboss.shrinkwrap.api.asset.StringAsset;
 import org.jboss.shrinkwrap.api.spec.JavaArchive;
 import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import com.example.security.SecuredService;
@@ -26,6 +34,11 @@
 import io.quarkus.grpc.GrpcClient;
 import io.quarkus.grpc.GrpcClientUtils;
 import io.quarkus.grpc.GrpcService;
+import io.quarkus.security.UnauthorizedException;
+import io.quarkus.security.runtime.interceptor.check.RolesAllowedCheck;
+import io.quarkus.security.spi.runtime.AuthenticationSuccessEvent;
+import io.quarkus.security.spi.runtime.AuthorizationFailureEvent;
+import io.quarkus.security.spi.runtime.AuthorizationSuccessEvent;
 import io.quarkus.test.QuarkusUnitTest;
 import io.smallrye.common.annotation.Blocking;
 import io.smallrye.mutiny.Multi;
@@ -54,7 +67,7 @@ protected static QuarkusUnitTest createQuarkusUnitTest(String extraProperty, boo
                         props += extraProperty;
                     }
                     var jar = ShrinkWrap.create(JavaArchive.class)
-                            .addClasses(Service.class, BlockingHttpSecurityPolicy.class)
+                            .addClasses(Service.class, BlockingHttpSecurityPolicy.class, SecurityEventObserver.class)
                             .addPackage(SecuredService.class.getPackage())
                             .add(new StringAsset(props), "application.properties");
                     return useGrpcAuthMechanism ? jar.addClass(BasicGrpcSecurityMechanism.class) : jar;
@@ -67,6 +80,14 @@ protected static QuarkusUnitTest createQuarkusUnitTest(String extraProperty, boo
     @GrpcClient
     SecuredService securityClient;
 
+    @Inject
+    SecurityEventObserver securityEventObserver;
+
+    @BeforeEach
+    void clearEvents() {
+        securityEventObserver.getStorage().clear();
+    }
+
     @Test
     void shouldSecureUniEndpoint() {
         Metadata headers = new Metadata();
@@ -83,6 +104,7 @@ void shouldSecureUniEndpoint() {
 
         await().atMost(10, TimeUnit.SECONDS)
                 .until(() -> resultCount.get() == 1);
+        assertSecurityEventsFired("john");
     }
 
     @Test
@@ -101,6 +123,7 @@ void shouldSecureBlockingUniEndpoint() {
 
         await().atMost(10, TimeUnit.SECONDS)
                 .until(() -> resultCount.get() == 1);
+        assertSecurityEventsFired("john");
     }
 
     @Test
@@ -117,6 +140,7 @@ void shouldSecureMultiEndpoint() {
                 .until(() -> results.size() == 5);
 
         assertThat(results.stream().filter(e -> !e)).isEmpty();
+        assertSecurityEventsFired("paul");
     }
 
     @Test
@@ -133,6 +157,7 @@ void shouldSecureBlockingMultiEndpoint() {
                 .until(() -> results.size() == 5);
 
         assertThat(results.stream().filter(e -> e)).isEmpty();
+        assertSecurityEventsFired("paul");
     }
 
     @Test
@@ -167,6 +192,16 @@ void shouldFailWithInvalidInsufficientRole() {
 
         await().atMost(10, TimeUnit.SECONDS)
                 .until(() -> error.get() != null);
+
+        // we don't check exact count as HTTP Security policies are not supported when gRPC is running on separate server
+        assertFalse(securityEventObserver.getStorage().isEmpty());
+        // fails RolesAllowed check as the anonymous identity has no roles
+        AuthorizationFailureEvent event = (AuthorizationFailureEvent) securityEventObserver
+                .getStorage().get(securityEventObserver.getStorage().size() - 1);
+        assertNotNull(event.getSecurityIdentity());
+        assertTrue(event.getSecurityIdentity().isAnonymous());
+        assertInstanceOf(UnauthorizedException.class, event.getAuthorizationFailure());
+        assertEquals(RolesAllowedCheck.class.getName(), event.getAuthorizationContext());
     }
 
     @Test
@@ -186,6 +221,7 @@ void shouldSecureUniEndpointWithBlockingHttpSecurityPolicy() {
 
         await().atMost(10, TimeUnit.SECONDS)
                 .until(() -> resultCount.get() == 1);
+        assertSecurityEventsFired("john");
     }
 
     @Test
@@ -205,6 +241,7 @@ void shouldSecureBlockingUniEndpointWithBlockingHttpSecurityPolicy() {
 
         await().atMost(10, TimeUnit.SECONDS)
                 .until(() -> resultCount.get() == 1);
+        assertSecurityEventsFired("john");
     }
 
     @Test
@@ -224,6 +261,7 @@ void shouldSecureMultiEndpointWithBlockingHttpSecurityPolicy() {
                 .until(() -> results.size() == 5);
 
         assertThat(results.stream().filter(e -> !e)).isEmpty();
+        assertSecurityEventsFired("paul");
     }
 
     @Test
@@ -241,6 +279,19 @@ void shouldSecureBlockingMultiEndpointWithBlockingHttpSecurityPolicy() {
                 .until(() -> results.size() == 5);
 
         assertThat(results.stream().filter(e -> e)).isEmpty();
+        assertSecurityEventsFired("paul");
+    }
+
+    private void assertSecurityEventsFired(String username) {
+        // expect at least authentication success and RolesAllowed security check success
+        // we don't check exact count as HTTP Security policies are not supported when gRPC is running on separate server
+        assertTrue(securityEventObserver.getStorage().size() >= 2);
+        assertTrue(securityEventObserver.getStorage().stream().anyMatch(e -> e instanceof AuthenticationSuccessEvent));
+        AuthorizationSuccessEvent event = (AuthorizationSuccessEvent) securityEventObserver.getStorage()
+                .get(securityEventObserver.getStorage().size() - 1);
+        assertNotNull(event.getSecurityIdentity());
+        assertEquals(username, event.getSecurityIdentity().getPrincipal().getName());
+        assertEquals(RolesAllowedCheck.class.getName(), event.getEventProperties().get(AUTHORIZATION_CONTEXT));
     }
 
     private static void addBlockingHeaders(Metadata headers) {

diff --git a/extensions/grpc/deployment/src/test/java/io/quarkus/grpc/auth/SecurityEventObserver.java b/extensions/grpc/deployment/src/test/java/io/quarkus/grpc/auth/SecurityEventObserver.java
@@ -3,10 +3,12 @@
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
 
+import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.enterprise.event.Observes;
 
 import io.quarkus.security.spi.runtime.SecurityEvent;
 
+@ApplicationScoped
 public class SecurityEventObserver {
 
     private final List<SecurityEvent> storage = new CopyOnWriteArrayList<>();

diff --git a/...pc/deployment/src/test/java/io/quarkus/grpc/auth/SecurityEventsValidationFailureTest.java b/...pc/deployment/src/test/java/io/quarkus/grpc/auth/SecurityEventsValidationFailureTest.java
diff --git a/extensions/grpc/runtime/src/main/java/io/quarkus/grpc/auth/GrpcSecurityInterceptor.java b/extensions/grpc/runtime/src/main/java/io/quarkus/grpc/auth/GrpcSecurityInterceptor.java
@@ -1,5 +1,7 @@
 package io.quarkus.grpc.auth;
 
+import static io.quarkus.security.spi.runtime.SecurityEventHelper.AUTHENTICATION_FAILURE;
+import static io.quarkus.security.spi.runtime.SecurityEventHelper.AUTHENTICATION_SUCCESS;
 import static io.quarkus.vertx.core.runtime.context.VertxContextSafetyToggle.isExplicitlyMarkedAsUnsafe;
 import static io.quarkus.vertx.http.runtime.security.QuarkusHttpUser.DEFERRED_IDENTITY_KEY;
 import static io.smallrye.common.vertx.VertxContext.isDuplicatedContext;
@@ -10,8 +12,11 @@
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.Executor;
+import java.util.function.Consumer;
 
+import jakarta.enterprise.event.Event;
 import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.inject.spi.BeanManager;
 import jakarta.enterprise.inject.spi.Prioritized;
 import jakarta.inject.Inject;
 import jakarta.inject.Singleton;
@@ -29,6 +34,9 @@
 import io.quarkus.security.identity.IdentityProviderManager;
 import io.quarkus.security.identity.SecurityIdentity;
 import io.quarkus.security.identity.request.AuthenticationRequest;
+import io.quarkus.security.spi.runtime.AuthenticationFailureEvent;
+import io.quarkus.security.spi.runtime.AuthenticationSuccessEvent;
+import io.quarkus.security.spi.runtime.SecurityEventHelper;
 import io.quarkus.vertx.http.runtime.security.QuarkusHttpUser;
 import io.smallrye.mutiny.Uni;
 import io.vertx.core.Context;
@@ -55,14 +63,20 @@ public final class GrpcSecurityInterceptor implements ServerInterceptor, Priorit
     private final Map<String, List<String>> serviceToBlockingMethods = new HashMap<>();
     private boolean hasBlockingMethods = false;
     private final boolean notUsingSeparateGrpcServer;
+    private final SecurityEventHelper<AuthenticationSuccessEvent, AuthenticationFailureEvent> securityEventHelper;
 
     @Inject
     public GrpcSecurityInterceptor(
             CurrentIdentityAssociation identityAssociation,
             IdentityProviderManager identityProviderManager,
             Instance<GrpcSecurityMechanism> securityMechanisms,
             Instance<AuthExceptionHandlerProvider> exceptionHandlers,
-            @ConfigProperty(name = "quarkus.grpc.server.use-separate-server") boolean usingSeparateGrpcServer) {
+            @ConfigProperty(name = "quarkus.grpc.server.use-separate-server") boolean usingSeparateGrpcServer,
+            @ConfigProperty(name = "quarkus.security.events.enabled") boolean securityEventsEnabled,
+            BeanManager beanManager, Event<AuthenticationFailureEvent> authFailureEvent,
+            Event<AuthenticationSuccessEvent> authSuccessEvent) {
+        this.securityEventHelper = new SecurityEventHelper<>(authSuccessEvent, authFailureEvent, AUTHENTICATION_SUCCESS,
+                AUTHENTICATION_FAILURE, beanManager, securityEventsEnabled);
         this.identityAssociation = identityAssociation;
         this.identityProviderManager = identityProviderManager;
         this.notUsingSeparateGrpcServer = !usingSeparateGrpcServer;
@@ -131,6 +145,23 @@ public void handle(Void event) {
                                             }
                                         }
                                     });
+                            if (securityEventHelper.fireEventOnSuccess()) {
+                                auth = auth.invoke(new Consumer<SecurityIdentity>() {
+                                    @Override
+                                    public void accept(SecurityIdentity securityIdentity) {
+                                        securityEventHelper
+                                                .fireSuccessEvent(new AuthenticationSuccessEvent(securityIdentity, null));
+                                    }
+                                });
+                            }
+                            if (securityEventHelper.fireEventOnFailure()) {
+                                auth = auth.onFailure().invoke(new Consumer<Throwable>() {
+                                    @Override
+                                    public void accept(Throwable throwable) {
+                                        securityEventHelper.fireFailureEvent(new AuthenticationFailureEvent(throwable, null));
+                                    }
+                                });
+                            }
                             identityAssociation.setIdentity(auth);
                             error = null;
                             identityAssociationNotSet = false;
@@ -143,8 +174,11 @@ public void handle(Void event) {
                 }
             }
             if (error != null) { // if parsing for all security mechanisms failed, let's propagate the last exception
-                identityAssociation.setIdentity(Uni.createFrom()
-                        .failure(new AuthenticationFailedException("Failed to parse authentication data", error)));
+                var authFailedEx = new AuthenticationFailedException("Failed to parse authentication data", error);
+                if (securityEventHelper.fireEventOnFailure()) {
+                    securityEventHelper.fireFailureEvent(new AuthenticationFailureEvent(authFailedEx, null));
+                }
+                identityAssociation.setIdentity(Uni.createFrom().failure(authFailedEx));
             }
         }
         if (identityAssociationNotSet && notUsingSeparateGrpcServer) {