Merge pull request apache#2622

TINKERPOP-3081 Fix traversal argument propagation under authentication
Bit-Quill · Jul 4, 2024 · e4d2e72 · e4d2e72
2 parents e26f7c0 + 9bf2566
commit e4d2e72
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 5 deletions.
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
@@ -23,6 +23,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 [[release-3-6-8]]
 === TinkerPop 3.6.8 (NOT OFFICIALLY RELEASED YET)
 
+* Fixed a bug in GremlinServer not properly propagating arguments when authentication is enabled.
 * Fixed bug in Java driver where connection pool was not removing dead connections under certain error conditions.
 * Raised handshake exceptions for Java driver for `NoHostAvailableException` situations.
 

diff --git a/.../main/java/org/apache/tinkerpop/gremlin/server/handler/WebSocketAuthorizationHandler.java b/.../main/java/org/apache/tinkerpop/gremlin/server/handler/WebSocketAuthorizationHandler.java
@@ -68,11 +68,9 @@ public void channelRead(final ChannelHandlerContext ctx, final Object msg) {
                         final Bytecode bytecode = (Bytecode) requestMessage.getArgs().get(Tokens.ARGS_GREMLIN);
                         final Map<String, String> aliases = (Map<String, String>) requestMessage.getArgs().get(Tokens.ARGS_ALIASES);
                         final Bytecode restrictedBytecode = authorizer.authorize(user, bytecode, aliases);
-                        final RequestMessage restrictedMsg = RequestMessage.build(Tokens.OPS_BYTECODE).
-                                overrideRequestId(requestMessage.getRequestId()).
-                                processor("traversal").
-                                addArg(Tokens.ARGS_GREMLIN, restrictedBytecode).
-                                addArg(Tokens.ARGS_ALIASES, aliases).create();
+                        final RequestMessage restrictedMsg = RequestMessage.from(requestMessage)
+                                .addArg(Tokens.ARGS_GREMLIN, restrictedBytecode)
+                                .addArg(Tokens.ARGS_ALIASES, aliases).create();
                         ctx.fireChannelRead(restrictedMsg);
                         break;
                     case Tokens.OPS_EVAL:

diff --git a/...er/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java b/...er/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java
@@ -31,6 +31,7 @@
 import org.apache.tinkerpop.gremlin.driver.remote.DriverRemoteConnection;
 import org.apache.tinkerpop.gremlin.process.traversal.AnonymousTraversalSource;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
 import org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.AbstractWarningVerificationStrategy;
 import org.apache.tinkerpop.gremlin.server.auth.AllowAllAuthenticator;
 import org.apache.tinkerpop.gremlin.server.auth.SimpleAuthenticator;
@@ -41,14 +42,18 @@
 import org.apache.tinkerpop.gremlin.util.function.Lambda;
 import org.apache.tinkerpop.shaded.jackson.databind.JsonNode;
 import org.apache.tinkerpop.shaded.jackson.databind.ObjectMapper;
+import org.codehaus.groovy.tools.shell.CommandException;
 import org.junit.AfterClass;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
+import java.time.Instant;
 import java.util.Base64;
 import java.util.HashMap;
 import java.util.Objects;
+import java.util.concurrent.CompletionException;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.core.Is.is;
@@ -62,6 +67,7 @@
  * @author Marc de Lignie
  */
 public class GremlinServerAuthzIntegrateTest extends AbstractGremlinServerIntegrationTest {
+    private static final Long DEFAULT_EVALUATION_TIMEOUT = 2000L;
     private static LogCaptor logCaptor;
 
     private final ObjectMapper mapper = new ObjectMapper();
@@ -107,6 +113,7 @@ public Settings overrideSettings(final Settings settings) {
         settings.authentication = authSettings;
         settings.authorization = authzSettings;
         settings.enableAuditLog = true;
+        settings.evaluationTimeout = DEFAULT_EVALUATION_TIMEOUT;
 
         final String nameOfTest = name.getMethodName();
         switch (nameOfTest) {
@@ -387,4 +394,23 @@ public void shouldAuthorizeWithAllowAllAuthenticatorAndHttpTransport() throws Ex
             assertEquals(6, node.get("result").get("data").get(GraphSONTokens.VALUEPROP).get(0).get(GraphSONTokens.VALUEPROP).intValue());
         }
     }
+
+    @Test
+    public void shouldRespectTimeoutWithAuth() {
+        final Cluster cluster = TestClientFactory.build().credentials("stephen", "password").create();
+        final GraphTraversalSource g = AnonymousTraversalSource.traversal().withRemote(
+                DriverRemoteConnection.using(cluster, "gmodern"));
+        final Instant instant = Instant.now();
+        try {
+            g.with("evaluationTimeout", DEFAULT_EVALUATION_TIMEOUT / 20).
+                    V().
+                    repeat(__.both()).
+                    until(__.count().is(0)).
+                    toList();
+        } catch (final CompletionException e) {
+            Assert.assertTrue(Instant.now().toEpochMilli() - instant.toEpochMilli() < DEFAULT_EVALUATION_TIMEOUT / 2);
+        } finally {
+            cluster.close();
+        }
+    }
 }