This is an automated email from the ASF dual-hosted git repository. kenhuuu pushed a commit to branch 3.7-dev in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit eae093bbfdbaf599fff0e44cb7250bf360dba706 Author: Ken Hu <[email protected]> AuthorDate: Mon Aug 31 16:38:05 2026 -0700 Isolate authenticated users in authorization handlers CTR Assisted-by: Codex:gpt-5.6-sol --- CHANGELOG.asciidoc | 1 + .../tinkerpop/gremlin/server/authz/Authorizer.java | 5 + .../handler/HttpBasicAuthorizationHandler.java | 10 +- .../handler/WebSocketAuthorizationHandler.java | 9 +- .../handler/HttpBasicAuthorizationHandlerTest.java | 144 ++++++++++++++++++++ .../handler/WebSocketAuthorizationHandlerTest.java | 147 +++++++++++++++++++++ 6 files changed, 305 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 81fe3fa1f0..7c639880af 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -56,6 +56,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima * Restricted `JavaTranslator` reconstruction of `TraversalStrategy` proxies to registered strategies that have not been denied with `denyStrategy()`. * Restricted GraphBinary `TraversalStrategy` deserialization to registered strategies that have not been denied with `denyStrategy()`. * Restricted GraphBinary, GraphSON and Gryo `Class` deserialization to classes registered with `ClassRegistry.register()`, `registerStrategy()` or `registerStrategies()`. +* Fixed request isolation in HTTP and WebSocket authorization handlers. * Fixed `gremlin-python` GraphBinary serialization of `BigInteger`/`BigDecimal` negative boundary values (e.g. `-129`) that raised `OverflowError`. * Fixed `gremlin-go` GraphBinary serialization of zero `BigInteger`/`BigDecimal` values, which were encoded with zero length and rejected by Java servers. diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/authz/Authorizer.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/authz/Authorizer.java index 593dab14ca..1f033f2ed9 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/authz/Authorizer.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/authz/Authorizer.java @@ -28,6 +28,11 @@ import java.util.Map; /** * Provides the interface for authorizing a user per request. + * <p> + * Gremlin Server creates one {@code Authorizer} instance and may call its {@code authorize()} methods concurrently + * for requests on different channels. Implementations are expected to be thread-safe and must not retain + * request-specific state in shared mutable fields. The {@link #setup(Map)} method is called once and completes before + * request processing begins. * * @author Marc de Lignie */ diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpBasicAuthorizationHandler.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpBasicAuthorizationHandler.java index 40c6fb3604..736b11dc4c 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpBasicAuthorizationHandler.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpBasicAuthorizationHandler.java @@ -49,7 +49,6 @@ public class HttpBasicAuthorizationHandler extends ChannelInboundHandlerAdapter private static final Logger logger = LoggerFactory.getLogger(HttpBasicAuthorizationHandler.class); private static final Logger auditLogger = LoggerFactory.getLogger(GremlinServer.AUDIT_LOGGER_NAME); - private AuthenticatedUser user; private final Authorizer authorizer; public HttpBasicAuthorizationHandler(Authorizer authorizer) { @@ -69,12 +68,11 @@ public class HttpBasicAuthorizationHandler extends ChannelInboundHandlerAdapter return; } + final AuthenticatedUser channelUser = ctx.channel().attr(StateKey.AUTHENTICATED_USER).get(); + // channelUser is null when using the AllowAllAuthenticator + final AuthenticatedUser user = null == channelUser ? + AuthenticatedUser.ANONYMOUS_USER : channelUser; try { - user = ctx.channel().attr(StateKey.AUTHENTICATED_USER).get(); - if (null == user) { // This is expected when using the AllowAllAuthenticator - user = AuthenticatedUser.ANONYMOUS_USER; - } - authorizer.authorize(user, requestMessage); ctx.fireChannelRead(request); } catch (AuthorizationException ex) { // Expected: users can alternate between allowed and disallowed requests 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 4de4b0cbf9..5a2005cbe5 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 @@ -47,7 +47,6 @@ public class WebSocketAuthorizationHandler extends ChannelInboundHandlerAdapter private static final Logger logger = LoggerFactory.getLogger(WebSocketAuthorizationHandler.class); private static final Logger auditLogger = LoggerFactory.getLogger(GremlinServer.AUDIT_LOGGER_NAME); - private AuthenticatedUser user; private final Authorizer authorizer; public WebSocketAuthorizationHandler(Authorizer authorizer) { @@ -58,11 +57,11 @@ public class WebSocketAuthorizationHandler extends ChannelInboundHandlerAdapter public void channelRead(final ChannelHandlerContext ctx, final Object msg) { if (msg instanceof RequestMessage){ final RequestMessage requestMessage = (RequestMessage) msg; + final AuthenticatedUser channelUser = ctx.channel().attr(StateKey.AUTHENTICATED_USER).get(); + // channelUser is null when using the AllowAllAuthenticator + final AuthenticatedUser user = null == channelUser ? + AuthenticatedUser.ANONYMOUS_USER : channelUser; try { - user = ctx.channel().attr(StateKey.AUTHENTICATED_USER).get(); - if (null == user) { // This is expected when using the AllowAllAuthenticator - user = AuthenticatedUser.ANONYMOUS_USER; - } switch (requestMessage.getOp()) { case Tokens.OPS_BYTECODE: final Bytecode bytecode = (Bytecode) requestMessage.getArgs().get(Tokens.ARGS_GREMLIN); diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/handler/HttpBasicAuthorizationHandlerTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/handler/HttpBasicAuthorizationHandlerTest.java new file mode 100644 index 0000000000..2e0e6e6b34 --- /dev/null +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/handler/HttpBasicAuthorizationHandlerTest.java @@ -0,0 +1,144 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.tinkerpop.gremlin.server.handler; + +import io.netty.channel.embedded.EmbeddedChannel; +import io.netty.handler.codec.http.DefaultFullHttpRequest; +import io.netty.handler.codec.http.FullHttpRequest; +import io.netty.handler.codec.http.HttpMethod; +import io.netty.handler.codec.http.HttpVersion; +import io.netty.handler.codec.http.QueryStringEncoder; +import org.apache.tinkerpop.gremlin.process.traversal.Bytecode; +import org.apache.tinkerpop.gremlin.server.auth.AuthenticatedUser; +import org.apache.tinkerpop.gremlin.server.authz.AuthorizationException; +import org.apache.tinkerpop.gremlin.server.authz.Authorizer; +import org.apache.tinkerpop.gremlin.util.Tokens; +import org.apache.tinkerpop.gremlin.util.message.RequestMessage; +import org.junit.Test; + +import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +public class HttpBasicAuthorizationHandlerTest { + + @Test + public void shouldHandleRejectedRequestWithTheUserFromItsChannel() throws Exception { + final CountDownLatch firstRequestInAuthorizer = new CountDownLatch(1); + final CountDownLatch continueFirstRequest = new CountDownLatch(1); + final BlockingAuthorizer authorizer = + new BlockingAuthorizer(firstRequestInAuthorizer, continueFirstRequest); + final HttpBasicAuthorizationHandler handler = new HttpBasicAuthorizationHandler(authorizer); + final EmbeddedChannel firstChannel = new EmbeddedChannel(handler); + final EmbeddedChannel secondChannel = new EmbeddedChannel(handler); + final RecordingUser firstUser = new RecordingUser("first"); + final RecordingUser secondUser = new RecordingUser("second"); + firstChannel.attr(StateKey.AUTHENTICATED_USER).set(firstUser); + secondChannel.attr(StateKey.AUTHENTICATED_USER).set(secondUser); + + final FullHttpRequest firstRequest = createRequest("first"); + final FullHttpRequest secondRequest = createRequest("second"); + final ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + final Future<Boolean> firstResult = executor.submit(() -> firstChannel.writeInbound(firstRequest)); + assertTrue(firstRequestInAuthorizer.await(5, TimeUnit.SECONDS)); + + secondChannel.writeInbound(secondRequest); + continueFirstRequest.countDown(); + firstResult.get(5, TimeUnit.SECONDS); + + assertEquals(1, firstUser.getNameCalls()); + assertEquals(0, secondUser.getNameCalls()); + } finally { + continueFirstRequest.countDown(); + executor.shutdownNow(); + executor.awaitTermination(5, TimeUnit.SECONDS); + firstChannel.finishAndReleaseAll(); + secondChannel.finishAndReleaseAll(); + } + } + + private static FullHttpRequest createRequest(final String script) { + final QueryStringEncoder encoder = new QueryStringEncoder("/"); + encoder.addParam(Tokens.ARGS_GREMLIN, script); + return new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, encoder.toString()); + } + + private static class BlockingAuthorizer implements Authorizer { + private final CountDownLatch firstRequestInAuthorizer; + private final CountDownLatch continueFirstRequest; + + private BlockingAuthorizer(final CountDownLatch firstRequestInAuthorizer, + final CountDownLatch continueFirstRequest) { + this.firstRequestInAuthorizer = firstRequestInAuthorizer; + this.continueFirstRequest = continueFirstRequest; + } + + @Override + public void setup(final Map<String, Object> config) { + } + + @Override + public Bytecode authorize(final AuthenticatedUser user, final Bytecode bytecode, + final Map<String, String> aliases) throws AuthorizationException { + return bytecode; + } + + @Override + public void authorize(final AuthenticatedUser user, final RequestMessage msg) throws AuthorizationException { + if (!"first".equals(msg.getArg(Tokens.ARGS_GREMLIN))) + return; + + firstRequestInAuthorizer.countDown(); + try { + if (!continueFirstRequest.await(5, TimeUnit.SECONDS)) + throw new AuthorizationException("Timed out waiting for the second request"); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + throw new AuthorizationException("Interrupted while waiting for the second request", ex); + } + throw new AuthorizationException("Request rejected"); + } + } + + private static class RecordingUser extends AuthenticatedUser { + private final AtomicInteger nameCalls = new AtomicInteger(); + + private RecordingUser(final String name) { + super(name); + } + + @Override + public String getName() { + nameCalls.incrementAndGet(); + return super.getName(); + } + + private int getNameCalls() { + return nameCalls.get(); + } + } +} diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/handler/WebSocketAuthorizationHandlerTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/handler/WebSocketAuthorizationHandlerTest.java new file mode 100644 index 0000000000..488087ceea --- /dev/null +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/handler/WebSocketAuthorizationHandlerTest.java @@ -0,0 +1,147 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.tinkerpop.gremlin.server.handler; + +import io.netty.channel.embedded.EmbeddedChannel; +import org.apache.tinkerpop.gremlin.process.traversal.Bytecode; +import org.apache.tinkerpop.gremlin.server.auth.AuthenticatedUser; +import org.apache.tinkerpop.gremlin.server.authz.AuthorizationException; +import org.apache.tinkerpop.gremlin.server.authz.Authorizer; +import org.apache.tinkerpop.gremlin.util.Tokens; +import org.apache.tinkerpop.gremlin.util.message.RequestMessage; +import org.junit.Test; + +import java.lang.reflect.Field; +import java.util.Collections; +import java.util.HashMap; +import java.util.IdentityHashMap; +import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +public class WebSocketAuthorizationHandlerTest { + + @Test + public void shouldAuthorizeEachRequestWithTheUserFromItsChannel() throws Exception { + final CountDownLatch firstRequestReadUser = new CountDownLatch(1); + final CountDownLatch continueFirstRequest = new CountDownLatch(1); + final Bytecode firstBytecode = new Bytecode(); + final Bytecode secondBytecode = new Bytecode(); + final Map<String, String> aliases = Collections.singletonMap("g", "g"); + final BlockingRequestArguments firstArgs = + new BlockingRequestArguments(firstRequestReadUser, continueFirstRequest); + firstArgs.put(Tokens.ARGS_GREMLIN, firstBytecode); + firstArgs.put(Tokens.ARGS_ALIASES, aliases); + final RequestMessage firstRequest = createRequest(firstArgs); + final RequestMessage secondRequest = RequestMessage.build(Tokens.OPS_BYTECODE) + .addArg(Tokens.ARGS_GREMLIN, secondBytecode) + .addArg(Tokens.ARGS_ALIASES, aliases).create(); + + final RecordingAuthorizer authorizer = new RecordingAuthorizer(); + final WebSocketAuthorizationHandler handler = new WebSocketAuthorizationHandler(authorizer); + final EmbeddedChannel firstChannel = new EmbeddedChannel(handler); + final EmbeddedChannel secondChannel = new EmbeddedChannel(handler); + final AuthenticatedUser firstUser = new AuthenticatedUser("first"); + final AuthenticatedUser secondUser = new AuthenticatedUser("second"); + firstChannel.attr(StateKey.AUTHENTICATED_USER).set(firstUser); + secondChannel.attr(StateKey.AUTHENTICATED_USER).set(secondUser); + + final ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + final Future<Boolean> firstResult = executor.submit(() -> firstChannel.writeInbound(firstRequest)); + assertTrue(firstRequestReadUser.await(5, TimeUnit.SECONDS)); + + secondChannel.writeInbound(secondRequest); + continueFirstRequest.countDown(); + firstResult.get(5, TimeUnit.SECONDS); + + assertEquals(firstUser, authorizer.usersByBytecode.get(firstBytecode)); + assertEquals(secondUser, authorizer.usersByBytecode.get(secondBytecode)); + } finally { + continueFirstRequest.countDown(); + executor.shutdownNow(); + executor.awaitTermination(5, TimeUnit.SECONDS); + firstChannel.finishAndReleaseAll(); + secondChannel.finishAndReleaseAll(); + } + } + + private static RequestMessage createRequest(final Map<String, Object> args) throws ReflectiveOperationException { + final RequestMessage.Builder builder = RequestMessage.build(Tokens.OPS_BYTECODE); + final Field argsField = RequestMessage.Builder.class.getDeclaredField("args"); + argsField.setAccessible(true); + argsField.set(builder, args); + return builder.create(); + } + + private static class RecordingAuthorizer implements Authorizer { + private final Map<Bytecode, AuthenticatedUser> usersByBytecode = + Collections.synchronizedMap(new IdentityHashMap<>()); + + @Override + public void setup(final Map<String, Object> config) { + } + + @Override + public Bytecode authorize(final AuthenticatedUser user, final Bytecode bytecode, + final Map<String, String> aliases) throws AuthorizationException { + usersByBytecode.put(bytecode, user); + return bytecode; + } + + @Override + public void authorize(final AuthenticatedUser user, final RequestMessage msg) throws AuthorizationException { + } + } + + private static class BlockingRequestArguments extends HashMap<String, Object> { + private final CountDownLatch requestReadUser; + private final CountDownLatch continueRequest; + private final AtomicBoolean blocked = new AtomicBoolean(); + + private BlockingRequestArguments(final CountDownLatch requestReadUser, + final CountDownLatch continueRequest) { + this.requestReadUser = requestReadUser; + this.continueRequest = continueRequest; + } + + @Override + public Object get(final Object key) { + if (Tokens.ARGS_GREMLIN.equals(key) && blocked.compareAndSet(false, true)) { + requestReadUser.countDown(); + try { + if (!continueRequest.await(5, TimeUnit.SECONDS)) + throw new IllegalStateException("Timed out waiting for the second request"); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + throw new IllegalStateException("Interrupted while waiting for the second request", ex); + } + } + + return super.get(key); + } + } +}
