Use body handler to allow request buffering

Fixes quarkusio#5959 This change allows the body handler to be used for Undertow and RESTEasy standalone. When the request is fully buffered it can be consumed multiple times, which allows keycloak to also process it.
pedroigor · Dec 6, 2019 · afd2dd7 · afd2dd7
1 parent fabf2d7
commit afd2dd7
Show file tree

Hide file tree

Showing 17 changed files with 226 additions and 62 deletions.
diff --git a/bom/runtime/pom.xml b/bom/runtime/pom.xml
@@ -24,7 +24,7 @@
         <opentracing-concurrent.version>0.2.0</opentracing-concurrent.version>
         <opentracing-jdbc.version>0.0.12</opentracing-jdbc.version>
         <jaeger.version>0.34.0</jaeger.version>
-        <quarkus-http.version>3.0.0.Final</quarkus-http.version>
+        <quarkus-http.version>3.0.1.Final</quarkus-http.version>
         <jboss-servlet-api_4.0_spec.version>1.0.0.Final</jboss-servlet-api_4.0_spec.version>
         <microprofile-config-api.version>1.3</microprofile-config-api.version>
         <microprofile-context-propagation.version>1.0.1</microprofile-context-propagation.version>

diff --git a/...ent/src/main/java/io/quarkus/keycloak/pep/deployment/KeycloakPolicyEnforcerBuildStep.java b/...ent/src/main/java/io/quarkus/keycloak/pep/deployment/KeycloakPolicyEnforcerBuildStep.java
@@ -1,5 +1,7 @@
 package io.quarkus.keycloak.pep.deployment;
 
+import java.util.Map;
+
 import io.quarkus.arc.deployment.AdditionalBeanBuildItem;
 import io.quarkus.arc.deployment.BeanContainerBuildItem;
 import io.quarkus.deployment.annotations.BuildStep;
@@ -13,6 +15,7 @@
 import io.quarkus.oidc.OIDCException;
 import io.quarkus.oidc.runtime.OidcBuildTimeConfig;
 import io.quarkus.oidc.runtime.OidcConfig;
+import io.quarkus.vertx.http.deployment.RequireBodyHandlerBuildItem;
 
 public class KeycloakPolicyEnforcerBuildStep {
 
@@ -21,6 +24,34 @@ FeatureBuildItem featureBuildItem() {
         return new FeatureBuildItem(FeatureBuildItem.KEYCLOAK_AUTHORIZATION);
     }
 
+    @BuildStep
+    RequireBodyHandlerBuildItem requireBody(KeycloakPolicyEnforcerConfig config) {
+        if (config.policyEnforcer.enable) {
+            if (isBodyClaimInformationPointDefined(config.policyEnforcer.claimInformationPoint.simpleConfig)) {
+                return new RequireBodyHandlerBuildItem();
+            }
+            for (KeycloakPolicyEnforcerConfig.KeycloakConfigPolicyEnforcer.PathConfig path : config.policyEnforcer.paths
+                    .values()) {
+                if (isBodyClaimInformationPointDefined(path.claimInformationPoint.simpleConfig)) {
+                    return new RequireBodyHandlerBuildItem();
+                }
+            }
+        }
+        return null;
+    }
+
+    private boolean isBodyClaimInformationPointDefined(Map<String, Map<String, String>> claims) {
+        for (Map.Entry<String, Map<String, String>> entry : claims.entrySet()) {
+            Map<String, String> value = entry.getValue();
+
+            if (value.get(entry.getKey()).contains("request.body")) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     @BuildStep
     public AdditionalBeanBuildItem beans(KeycloakPolicyEnforcerConfig config) {
         if (config.policyEnforcer.enable) {

diff --git a/...n/runtime/src/main/java/io/quarkus/keycloak/pep/runtime/KeycloakPolicyEnforcerConfig.java b/...n/runtime/src/main/java/io/quarkus/keycloak/pep/runtime/KeycloakPolicyEnforcerConfig.java
@@ -41,13 +41,13 @@ public static class KeycloakConfigPolicyEnforcer {
          * Specifies how policies are enforced.
          */
         @ConfigItem(defaultValue = "ENFORCING")
-        String enforcementMode;
+        public String enforcementMode;
 
         /**
          * Specifies the paths to protect.
          */
         @ConfigItem
-        Map<String, PathConfig> paths;
+        public Map<String, PathConfig> paths;
 
         /**
          * Defines how the policy enforcer should track associations between paths in your application and resources defined in
@@ -56,7 +56,7 @@ public static class KeycloakConfigPolicyEnforcer {
          * protected resources
          */
         @ConfigItem
-        PathCacheConfig pathCache;
+        public PathCacheConfig pathCache;
 
         /**
          * Specifies how the adapter should fetch the server for resources associated with paths in your application. If true,
@@ -65,22 +65,22 @@ public static class KeycloakConfigPolicyEnforcer {
          * enforcer is going to fetch resources on-demand accordingly with the path being requested
          */
         @ConfigItem(defaultValue = "true")
-        boolean lazyLoadPaths;
+        public boolean lazyLoadPaths;
 
         /**
          * Defines a set of one or more claims that must be resolved and pushed to the Keycloak server in order to make these
          * claims available to policies
          */
         @ConfigItem
-        ClaimInformationPointConfig claimInformationPoint;
+        public ClaimInformationPointConfig claimInformationPoint;
 
         /**
          * Specifies how scopes should be mapped to HTTP methods. If set to true, the policy enforcer will use the HTTP method
          * from
          * the current request to check whether or not access should be granted
          */
         @ConfigItem
-        boolean httpMethodAsScope;
+        public boolean httpMethodAsScope;
 
         @ConfigGroup
         public static class PathConfig {
@@ -89,36 +89,36 @@ public static class PathConfig {
              * The name of a resource on the server that is to be associated with a given path
              */
             @ConfigItem
-            Optional<String> name;
+            public Optional<String> name;
 
             /**
              * A URI relative to the application’s context path that should be protected by the policy enforcer
              */
             @ConfigItem
-            Optional<String> path;
+            public Optional<String> path;
 
             /**
              * The HTTP methods (for example, GET, POST, PATCH) to protect and how they are associated with the scopes for a
              * given
              * resource in the server
              */
             @ConfigItem
-            Map<String, MethodConfig> methods;
+            public Map<String, MethodConfig> methods;
 
             /**
              * Specifies how policies are enforced
              */
             @DefaultConverter
             @ConfigItem(defaultValue = "ENFORCING")
-            PolicyEnforcerConfig.EnforcementMode enforcementMode;
+            public PolicyEnforcerConfig.EnforcementMode enforcementMode;
 
             /**
              * Defines a set of one or more claims that must be resolved and pushed to the Keycloak server in order to make
              * these
              * claims available to policies
              */
             @ConfigItem
-            ClaimInformationPointConfig claimInformationPoint;
+            public ClaimInformationPointConfig claimInformationPoint;
         }
 
         @ConfigGroup
@@ -128,20 +128,20 @@ public static class MethodConfig {
              * The name of the HTTP method
              */
             @ConfigItem
-            String method;
+            public String method;
 
             /**
              * An array of strings with the scopes associated with the method
              */
             @ConfigItem
-            List<String> scopes;
+            public List<String> scopes;
 
             /**
              * A string referencing the enforcement mode for the scopes associated with a method
              */
             @DefaultConverter
             @ConfigItem(defaultValue = "ALL")
-            PolicyEnforcerConfig.ScopeEnforcementMode scopesEnforcementMode;
+            public PolicyEnforcerConfig.ScopeEnforcementMode scopesEnforcementMode;
         }
 
         @ConfigGroup
@@ -151,13 +151,13 @@ public static class PathCacheConfig {
              * Defines the time in milliseconds when the entry should be expired
              */
             @ConfigItem(defaultValue = "1000")
-            int maxEntries = 1000;
+            public int maxEntries = 1000;
 
             /**
              * Defines the limit of entries that should be kept in the cache
              */
             @ConfigItem(defaultValue = "30000")
-            long lifespan = 30000;
+            public long lifespan = 30000;
         }
 
         @ConfigGroup
@@ -167,13 +167,13 @@ public static class ClaimInformationPointConfig {
              *
              */
             @ConfigItem(name = ConfigItem.PARENT)
-            Map<String, Map<String, Map<String, String>>> complexConfig;
+            public Map<String, Map<String, Map<String, String>>> complexConfig;
 
             /**
              *
              */
             @ConfigItem(name = ConfigItem.PARENT)
-            Map<String, Map<String, String>> simpleConfig;
+            public Map<String, Map<String, String>> simpleConfig;
         }
     }
 }
diff --git a/...-authorization/runtime/src/main/java/io/quarkus/keycloak/pep/runtime/VertxHttpFacade.java b/...-authorization/runtime/src/main/java/io/quarkus/keycloak/pep/runtime/VertxHttpFacade.java
@@ -1,6 +1,5 @@
 package io.quarkus.keycloak.pep.runtime;
 
-import java.io.BufferedInputStream;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.InputStream;
@@ -23,6 +22,7 @@
 import io.quarkus.oidc.AccessTokenCredential;
 import io.quarkus.security.credential.TokenCredential;
 import io.quarkus.security.identity.SecurityIdentity;
+import io.quarkus.vertx.http.runtime.VertxInputStream;
 import io.quarkus.vertx.http.runtime.security.QuarkusHttpUser;
 import io.vertx.core.buffer.Buffer;
 import io.vertx.core.http.HttpServerRequest;
@@ -122,7 +122,17 @@ public InputStream getInputStream() {
 
             @Override
             public InputStream getInputStream(boolean buffered) {
-                return new BufferedInputStream(new ByteArrayInputStream(routingContext.getBody().getBytes()));
+                try {
+                    if (routingContext.getBody() != null) {
+                        return new ByteArrayInputStream(routingContext.getBody().getBytes());
+                    }
+                    if (routingContext.request().isEnded()) {
+                        return new ByteArrayInputStream(new byte[0]);
+                    }
+                    return new VertxInputStream(request);
+                } catch (Exception e) {
+                    throw new RuntimeException(e);
+                }
             }
 
             @Override

diff --git a/...asy/runtime/src/main/java/io/quarkus/resteasy/runtime/standalone/VertxRequestHandler.java b/...asy/runtime/src/main/java/io/quarkus/resteasy/runtime/standalone/VertxRequestHandler.java
@@ -1,5 +1,6 @@
 package io.quarkus.resteasy.runtime.standalone;
 
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 
@@ -19,6 +20,7 @@
 import io.quarkus.arc.runtime.BeanContainer;
 import io.quarkus.security.identity.CurrentIdentityAssociation;
 import io.quarkus.vertx.http.runtime.CurrentVertxRequest;
+import io.quarkus.vertx.http.runtime.VertxInputStream;
 import io.quarkus.vertx.http.runtime.security.QuarkusHttpUser;
 import io.vertx.core.Context;
 import io.vertx.core.Handler;
@@ -61,9 +63,13 @@ public VertxRequestHandler(Vertx vertx,
     public void handle(RoutingContext request) {
         // have to create input stream here.  Cannot execute in another thread
         // otherwise request handlers may not get set up before request ends
-        VertxInputStream is;
+        InputStream is;
         try {
-            is = new VertxInputStream(request.request());
+            if (request.getBody() != null) {
+                is = new ByteArrayInputStream(request.getBody().getBytes());
+            } else {
+                is = new VertxInputStream(request.request());
+            }
         } catch (IOException e) {
             request.fail(e);
             return;

diff --git a/...ndertow/runtime/src/main/java/io/quarkus/undertow/runtime/UndertowDeploymentRecorder.java b/...ndertow/runtime/src/main/java/io/quarkus/undertow/runtime/UndertowDeploymentRecorder.java
@@ -353,7 +353,8 @@ public void run() {
         return new Handler<RoutingContext>() {
             @Override
             public void handle(RoutingContext event) {
-                VertxHttpExchange exchange = new VertxHttpExchange(event.request(), allocator, executorService, event);
+                VertxHttpExchange exchange = new VertxHttpExchange(event.request(), allocator, executorService, event,
+                        event.getBody());
                 Optional<MemorySize> maxBodySize = httpConfiguration.limits.maxBodySize;
                 if (maxBodySize.isPresent()) {
                     exchange.setMaxEntitySize(maxBodySize.get().asLongValue());

diff --git a/...-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/BodyHandlerBuildItem.java b/...-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/BodyHandlerBuildItem.java
@@ -0,0 +1,17 @@
+package io.quarkus.vertx.http.deployment;
+
+import io.quarkus.builder.item.SimpleBuildItem;
+import io.vertx.core.Handler;
+import io.vertx.ext.web.RoutingContext;
+
+public final class BodyHandlerBuildItem extends SimpleBuildItem {
+    private final Handler<RoutingContext> handler;
+
+    public BodyHandlerBuildItem(Handler<RoutingContext> handler) {
+        this.handler = handler;
+    }
+
+    public Handler<RoutingContext> getHandler() {
+        return handler;
+    }
+}
diff --git a/...eployment/src/main/java/io/quarkus/vertx/http/deployment/RequireBodyHandlerBuildItem.java b/...eployment/src/main/java/io/quarkus/vertx/http/deployment/RequireBodyHandlerBuildItem.java
@@ -0,0 +1,10 @@
+package io.quarkus.vertx.http.deployment;
+
+import io.quarkus.builder.item.MultiBuildItem;
+
+/**
+ * This is a marker that indicates that the body handler should be installed
+ * on all routes, as an extension requires the request to be fully buffered.
+ */
+public final class RequireBodyHandlerBuildItem extends MultiBuildItem {
+}
diff --git a/...tx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/VertxHttpProcessor.java b/...tx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/VertxHttpProcessor.java
@@ -35,8 +35,10 @@
 import io.quarkus.vertx.http.runtime.VertxHttpRecorder;
 import io.quarkus.vertx.http.runtime.cors.CORSRecorder;
 import io.quarkus.vertx.http.runtime.filters.Filter;
+import io.vertx.core.Handler;
 import io.vertx.core.impl.VertxImpl;
 import io.vertx.ext.web.Router;
+import io.vertx.ext.web.RoutingContext;
 
 class VertxHttpProcessor {
 
@@ -97,6 +99,12 @@ VertxWebRouterBuildItem initializeRouter(VertxHttpRecorder recorder,
         return new VertxWebRouterBuildItem(router);
     }
 
+    @BuildStep
+    @Record(ExecutionTime.RUNTIME_INIT)
+    BodyHandlerBuildItem bodyHandler(VertxHttpRecorder recorder, HttpConfiguration httpConfiguration) {
+        return new BodyHandlerBuildItem(recorder.createBodyHandler(httpConfiguration));
+    }
+
     @BuildStep
     @Record(ExecutionTime.RUNTIME_INIT)
     ServiceStartBuildItem finalizeRouter(
@@ -106,7 +114,9 @@ ServiceStartBuildItem finalizeRouter(
             List<DefaultRouteBuildItem> defaultRoutes, List<FilterBuildItem> filters,
             VertxWebRouterBuildItem router, EventLoopCountBuildItem eventLoopCount,
             HttpBuildTimeConfig httpBuildTimeConfig, HttpConfiguration httpConfiguration,
-            BuildProducer<ReflectiveClassBuildItem> reflectiveClass, List<WebsocketSubProtocolsBuildItem> websocketSubProtocols)
+            BuildProducer<ReflectiveClassBuildItem> reflectiveClass, List<WebsocketSubProtocolsBuildItem> websocketSubProtocols,
+            List<RequireBodyHandlerBuildItem> requireBodyHandlerBuildItems,
+            BodyHandlerBuildItem bodyHandlerBuildItem)
             throws BuildException, IOException {
         Optional<DefaultRouteBuildItem> defaultRoute;
         if (defaultRoutes == null || defaultRoutes.isEmpty()) {
@@ -124,9 +134,14 @@ ServiceStartBuildItem finalizeRouter(
                 .filter(f -> f.getHandler() != null)
                 .map(FilterBuildItem::toFilter).collect(Collectors.toList());
 
+        //if the body handler is required then we know it is installed for all routes, so we don't need to register it here
+        Handler<RoutingContext> bodyHandler = !requireBodyHandlerBuildItems.isEmpty() ? bodyHandlerBuildItem.getHandler()
+                : null;
+
         recorder.finalizeRouter(beanContainer.getValue(),
                 defaultRoute.map(DefaultRouteBuildItem::getRoute).orElse(null),
-                listOfFilters, vertx.getVertx(), router.getRouter(), httpBuildTimeConfig.rootPath, launchMode.getLaunchMode());
+                listOfFilters, vertx.getVertx(), router.getRouter(), httpBuildTimeConfig.rootPath, launchMode.getLaunchMode(),
+                !requireBodyHandlerBuildItems.isEmpty(), bodyHandler);
 
         boolean startVirtual = requireVirtual.isPresent() || httpBuildTimeConfig.virtual;
         if (startVirtual) {