From 2da0921312b09ecf8ad70953c00ace00cec049ce Mon Sep 17 00:00:00 2001 From: lyndon Date: Wed, 29 May 2024 13:27:41 -0700 Subject: [PATCH 1/8] TINKERPOP-3081 Fix traversal argument propagation under authenticated traversal settings. --- .../handler/WebSocketAuthorizationHandler.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WebSocketAuthorizationHandler.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WebSocketAuthorizationHandler.java index 98a24119c76..74f4da9005b 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WebSocketAuthorizationHandler.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WebSocketAuthorizationHandler.java @@ -68,11 +68,19 @@ public void channelRead(final ChannelHandlerContext ctx, final Object msg) { final Bytecode bytecode = (Bytecode) requestMessage.getArgs().get(Tokens.ARGS_GREMLIN); final Map aliases = (Map) requestMessage.getArgs().get(Tokens.ARGS_ALIASES); final Bytecode restrictedBytecode = authorizer.authorize(user, bytecode, aliases); - final RequestMessage restrictedMsg = RequestMessage.build(Tokens.OPS_BYTECODE). + final RequestMessage.Builder restrictedMsgBuilder = RequestMessage.build(Tokens.OPS_BYTECODE). overrideRequestId(requestMessage.getRequestId()). processor("traversal"). addArg(Tokens.ARGS_GREMLIN, restrictedBytecode). - addArg(Tokens.ARGS_ALIASES, aliases).create(); + addArg(Tokens.ARGS_ALIASES, aliases); + + // Apply all other arguments except the bytecode and aliases. + for (Map.Entry entry : requestMessage.getArgs().entrySet()) { + if (!Tokens.ARGS_GREMLIN.equals(entry.getKey()) && !Tokens.ARGS_ALIASES.equals(entry.getKey())) { + restrictedMsgBuilder.addArg(entry.getKey(), entry.getValue()); + } + } + final RequestMessage restrictedMsg = restrictedMsgBuilder.create(); ctx.fireChannelRead(restrictedMsg); break; case Tokens.OPS_EVAL: From 0d72e23e2e12f48129a7389b9c9d7908eef370f0 Mon Sep 17 00:00:00 2001 From: lyndon Date: Wed, 29 May 2024 13:42:29 -0700 Subject: [PATCH 2/8] TINKERPOP-3081 Fix traversal argument propagation under authenticated traversal settings. --- CHANGELOG.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 1ee86f030cd..a43da34b610 100644 --- 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. [[release-3-6-7]] === TinkerPop 3.6.7 (April 8, 2024) From ba594fdeed50271f134e445c388231851d678ece Mon Sep 17 00:00:00 2001 From: lyndon Date: Mon, 10 Jun 2024 12:34:07 -0700 Subject: [PATCH 3/8] TINKERPOP-3081 Updating to use RequestMessage pattern. --- .../handler/WebSocketAuthorizationHandler.java | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WebSocketAuthorizationHandler.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WebSocketAuthorizationHandler.java index 74f4da9005b..575fd39247d 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WebSocketAuthorizationHandler.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WebSocketAuthorizationHandler.java @@ -68,19 +68,9 @@ public void channelRead(final ChannelHandlerContext ctx, final Object msg) { final Bytecode bytecode = (Bytecode) requestMessage.getArgs().get(Tokens.ARGS_GREMLIN); final Map aliases = (Map) requestMessage.getArgs().get(Tokens.ARGS_ALIASES); final Bytecode restrictedBytecode = authorizer.authorize(user, bytecode, aliases); - final RequestMessage.Builder restrictedMsgBuilder = RequestMessage.build(Tokens.OPS_BYTECODE). - overrideRequestId(requestMessage.getRequestId()). - processor("traversal"). - addArg(Tokens.ARGS_GREMLIN, restrictedBytecode). - addArg(Tokens.ARGS_ALIASES, aliases); - - // Apply all other arguments except the bytecode and aliases. - for (Map.Entry entry : requestMessage.getArgs().entrySet()) { - if (!Tokens.ARGS_GREMLIN.equals(entry.getKey()) && !Tokens.ARGS_ALIASES.equals(entry.getKey())) { - restrictedMsgBuilder.addArg(entry.getKey(), entry.getValue()); - } - } - final RequestMessage restrictedMsg = restrictedMsgBuilder.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: From db9b1104ef98d0af00420bac757f70c9319835d5 Mon Sep 17 00:00:00 2001 From: lyndon Date: Fri, 14 Jun 2024 14:23:58 -0700 Subject: [PATCH 4/8] TINKERPOP-3081 Adding test. --- .../GremlinServerAuthzIntegrateTest.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java index b3069fdee5c..b5b4b08c398 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java @@ -62,6 +62,7 @@ * @author Marc de Lignie */ public class GremlinServerAuthzIntegrateTest extends AbstractGremlinServerIntegrationTest { + private static final Long DEFAULT_EVALUATION_TIMEOUT = 1000L; private static LogCaptor logCaptor; private final ObjectMapper mapper = new ObjectMapper(); @@ -107,6 +108,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 +389,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")); + try { + g.with("evaluationTimeout", DEFAULT_EVALUATION_TIMEOUT + 1000).V().limit(1).map(t -> { + try { + Thread.sleep(DEFAULT_EVALUATION_TIMEOUT + 500); + } catch (InterruptedException ie) { + // do nothing + } + return t; + }).next(); + } finally { + cluster.close(); + } + } } From d1d2b7f87eb67d629310bfd14ddbd3ed87bd5d67 Mon Sep 17 00:00:00 2001 From: lyndon Date: Tue, 18 Jun 2024 12:51:22 -0700 Subject: [PATCH 5/8] TINKERPOP-3081 Adding test. --- .../server/GremlinServerAuthzIntegrateTest.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java index b5b4b08c398..18dd4f82a4d 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java @@ -396,14 +396,13 @@ public void shouldRespectTimeoutWithAuth() { final GraphTraversalSource g = AnonymousTraversalSource.traversal().withRemote( DriverRemoteConnection.using(cluster, "gmodern")); try { - g.with("evaluationTimeout", DEFAULT_EVALUATION_TIMEOUT + 1000).V().limit(1).map(t -> { - try { - Thread.sleep(DEFAULT_EVALUATION_TIMEOUT + 500); - } catch (InterruptedException ie) { - // do nothing - } - return t; - }).next(); + final String lambdaFunction = String.format("{ n, i -> sleep(%d); return i; }", DEFAULT_EVALUATION_TIMEOUT + 1000); + g.with("evaluationTimeout", DEFAULT_EVALUATION_TIMEOUT + 2000). + V(). + limit(1). + sack( + Lambda.biFunction(lambdaFunction, "gremlin-groovy")). + next(); } finally { cluster.close(); } From 3b7c92708bec4f8957cd9721b05363601927b96e Mon Sep 17 00:00:00 2001 From: lyndon Date: Tue, 18 Jun 2024 13:22:43 -0700 Subject: [PATCH 6/8] TINKERPOP-3081 convert to seconds --- .../gremlin/server/GremlinServerAuthzIntegrateTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java index 18dd4f82a4d..e38626efc18 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java @@ -396,7 +396,7 @@ public void shouldRespectTimeoutWithAuth() { final GraphTraversalSource g = AnonymousTraversalSource.traversal().withRemote( DriverRemoteConnection.using(cluster, "gmodern")); try { - final String lambdaFunction = String.format("{ n, i -> sleep(%d); return i; }", DEFAULT_EVALUATION_TIMEOUT + 1000); + final String lambdaFunction = String.format("{ n, i -> sleep(%d); return i; }", (DEFAULT_EVALUATION_TIMEOUT + 1000) / 1000); g.with("evaluationTimeout", DEFAULT_EVALUATION_TIMEOUT + 2000). V(). limit(1). From a00b5dd63b71b1084dc09d28570e21153c487e07 Mon Sep 17 00:00:00 2001 From: lyndon Date: Fri, 21 Jun 2024 10:36:04 -0700 Subject: [PATCH 7/8] TINKERPOP-3081 convert to seconds --- .../gremlin/server/GremlinServerAuthzIntegrateTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java index e38626efc18..18dd4f82a4d 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java @@ -396,7 +396,7 @@ public void shouldRespectTimeoutWithAuth() { final GraphTraversalSource g = AnonymousTraversalSource.traversal().withRemote( DriverRemoteConnection.using(cluster, "gmodern")); try { - final String lambdaFunction = String.format("{ n, i -> sleep(%d); return i; }", (DEFAULT_EVALUATION_TIMEOUT + 1000) / 1000); + final String lambdaFunction = String.format("{ n, i -> sleep(%d); return i; }", DEFAULT_EVALUATION_TIMEOUT + 1000); g.with("evaluationTimeout", DEFAULT_EVALUATION_TIMEOUT + 2000). V(). limit(1). From 9bf2566be38b1c09062594f2dc64159866e18bc6 Mon Sep 17 00:00:00 2001 From: lyndon Date: Fri, 21 Jun 2024 13:07:07 -0700 Subject: [PATCH 8/8] TINKERPOP-3081 Resorting to a timer. --- .../GremlinServerAuthzIntegrateTest.java | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java index 18dd4f82a4d..cb43f39afa1 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java +++ b/gremlin-server/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,7 +67,7 @@ * @author Marc de Lignie */ public class GremlinServerAuthzIntegrateTest extends AbstractGremlinServerIntegrationTest { - private static final Long DEFAULT_EVALUATION_TIMEOUT = 1000L; + private static final Long DEFAULT_EVALUATION_TIMEOUT = 2000L; private static LogCaptor logCaptor; private final ObjectMapper mapper = new ObjectMapper(); @@ -395,14 +400,15 @@ 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 { - final String lambdaFunction = String.format("{ n, i -> sleep(%d); return i; }", DEFAULT_EVALUATION_TIMEOUT + 1000); - g.with("evaluationTimeout", DEFAULT_EVALUATION_TIMEOUT + 2000). + g.with("evaluationTimeout", DEFAULT_EVALUATION_TIMEOUT / 20). V(). - limit(1). - sack( - Lambda.biFunction(lambdaFunction, "gremlin-groovy")). - next(); + 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(); }