This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch fix/graphql-websocket-auth
in repository https://gitbox.apache.org/repos/asf/unomi.git
The following commit(s) were added to refs/heads/fix/graphql-websocket-auth by
this push:
new d571e49fe Close a GraphQL WebSocket that does not authenticate within
its deadline
d571e49fe is described below
commit d571e49fea58a6883124182440cf1ea8357c274a
Author: Serge Huber <[email protected]>
AuthorDate: Fri Sep 4 10:02:12 2026 +0200
Close a GraphQL WebSocket that does not authenticate within its deadline
The unauthenticated-socket deadline was implemented as a Jetty idle timeout,
which is reset by any received frame, including ping/pong control frames
that
never reach the message handler, so it only held for a silent client.
Schedule
an explicit close at the deadline instead, cancelled when the socket
authenticates or closes. A single-thread scheduler owned by the factory runs
it and is stopped with the factory on undeploy.
Authentication and expiry are made mutually exclusive under a lock, so a
connection_init that arrives after the deadline cannot resurrect an expired
socket. The upgrade Javadoc now describes both the header-authenticated and
the connection_init-authenticated paths.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
---
.../unomi/graphql/servlet/GraphQLServlet.java | 7 +-
.../servlet/websocket/SubscriptionWebSocket.java | 78 ++++++++++++++++-
.../websocket/SubscriptionWebSocketFactory.java | 24 +++++-
.../websocket/SubscriptionWebSocketTest.java | 97 ++++++++++++++++++++--
4 files changed, 193 insertions(+), 13 deletions(-)
diff --git
a/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/GraphQLServlet.java
b/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/GraphQLServlet.java
index 97b40661a..3443caca8 100644
---
a/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/GraphQLServlet.java
+++
b/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/GraphQLServlet.java
@@ -146,9 +146,12 @@ public class GraphQLServlet extends WebSocketServlet {
}
/**
- * Authenticated WebSocket upgrade path. Order matters for security:
+ * WebSocket upgrade path. Order matters for security:
* <ol>
- * <li>Authenticate BEFORE {@code acceptWebSocket} (creator reads the
thread-local subject).</li>
+ * <li>Refuse foreign-origin handshakes, then authenticate any
credential the handshake carries
+ * BEFORE {@code acceptWebSocket} (creator reads the thread-local
subject). A handshake that
+ * carries no credential is upgraded unauthenticated - a browser
cannot set headers on it - and
+ * must authenticate through {@code connection_init} before the
socket will do anything.</li>
* <li>Call {@code acceptWebSocket} directly — do not call {@code
WebSocketServlet.service()},
* which can fall through to {@code doGet}/{@code doPost} when
accept fails and the response
* is not committed.</li>
diff --git
a/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocket.java
b/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocket.java
index d18ff2241..71299eb19 100644
---
a/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocket.java
+++
b/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocket.java
@@ -36,6 +36,10 @@ import java.io.IOException;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
public class SubscriptionWebSocket extends WebSocketAdapter {
private static final Logger LOGGER =
LoggerFactory.getLogger(SubscriptionWebSocket.class);
@@ -57,6 +61,16 @@ public class SubscriptionWebSocket extends WebSocketAdapter {
/** No operation is executed on this socket until this is true. */
private volatile boolean authenticated;
+ /** Runs the unauthenticated-socket deadline; owned and stopped by the
factory. */
+ private final ScheduledExecutorService deadlineScheduler;
+
+ /** Makes "authenticate" and "deadline expired" mutually exclusive
outcomes. */
+ private final Object authenticationLock = new Object();
+
+ private volatile ScheduledFuture<?> deadlineTask;
+
+ private boolean deadlineExpired;
+
private final SecurityService securityService;
private final ExecutionContextManager executionContextManager;
@@ -67,8 +81,10 @@ public class SubscriptionWebSocket extends WebSocketAdapter {
Subject subject, ExecutionContext
executionContext,
SecurityService securityService,
ExecutionContextManager
executionContextManager,
- GraphQLServletSecurityValidator validator) {
+ GraphQLServletSecurityValidator validator,
+ ScheduledExecutorService deadlineScheduler) {
this.graphQL = graphQL;
+ this.deadlineScheduler = Objects.requireNonNull(deadlineScheduler,
"deadlineScheduler");
this.serviceManager = serviceManager;
this.subject = subject;
this.executionContext = executionContext;
@@ -85,18 +101,49 @@ public class SubscriptionWebSocket extends
WebSocketAdapter {
LOGGER.info("Opening web socket");
super.onWebSocketConnect(sess);
if (!authenticated) {
- // Bound how long an unauthenticated socket may sit open, so
sockets that never authenticate
- // cannot accumulate. Jetty closes the session when this idle
window elapses.
+ // Bound how long an unauthenticated socket may sit open. The idle
timeout alone is not a
+ // deadline: Jetty resets it on any frame, including ping/pong
control frames that never reach
+ // onWebSocketText, so a client could hold an unauthenticated
socket open just by pinging.
+ // The scheduled task closes the socket at the deadline whatever
the client sends.
sess.setIdleTimeout(AUTHENTICATION_DEADLINE_MS);
+ deadlineTask =
deadlineScheduler.schedule(this::expireAuthenticationDeadline,
+ AUTHENTICATION_DEADLINE_MS, TimeUnit.MILLISECONDS);
}
}
@Override
public void onWebSocketClose(int statusCode, String reason) {
LOGGER.info("Closing web socket");
+ cancelAuthenticationDeadline();
super.onWebSocketClose(statusCode, reason);
}
+ /**
+ * Runs on the deadline thread when an unauthenticated socket reaches its
deadline. Closes it unless
+ * connection_init already won; the lock makes the two outcomes mutually
exclusive, so a socket is
+ * never authenticated and closed for expiry at the same time.
+ */
+ private void expireAuthenticationDeadline() {
+ synchronized (authenticationLock) {
+ if (authenticated) {
+ return;
+ }
+ deadlineExpired = true;
+ }
+ LOGGER.warn("Closing GraphQL WebSocket that did not authenticate
within {} ms", AUTHENTICATION_DEADLINE_MS);
+ final Session session = getSession();
+ if (session != null && session.isOpen()) {
+ session.close(CLOSE_POLICY_VIOLATION, "Authentication deadline
expired");
+ }
+ }
+
+ private void cancelAuthenticationDeadline() {
+ final ScheduledFuture<?> task = deadlineTask;
+ if (task != null) {
+ task.cancel(false);
+ }
+ }
+
@Override
public void onWebSocketText(String textMessage) {
// Deliberately not logging the message: connection_init carries the
client's credentials.
@@ -177,6 +224,9 @@ public class SubscriptionWebSocket extends WebSocketAdapter
{
if (authenticated) {
return true;
}
+ if (isDeadlineExpired()) {
+ return refuseAfterDeadline(message);
+ }
final String credential = basicCredentialFrom(message.getPayload());
if (credential == null || validator == null ||
!validator.authenticateBasicCredential(credential)) {
@@ -205,7 +255,14 @@ public class SubscriptionWebSocket extends
WebSocketAdapter {
return false;
}
- this.authenticated = true;
+ synchronized (authenticationLock) {
+ // The deadline may have fired while the credential was being
checked.
+ if (deadlineExpired) {
+ return refuseAfterDeadline(message);
+ }
+ this.authenticated = true;
+ }
+ cancelAuthenticationDeadline();
final Session session = getSession();
if (session != null) {
// Authenticated: drop the short unauthenticated deadline.
@@ -214,6 +271,19 @@ public class SubscriptionWebSocket extends
WebSocketAdapter {
return true;
}
+ private boolean isDeadlineExpired() {
+ synchronized (authenticationLock) {
+ return deadlineExpired;
+ }
+ }
+
+ /** The deadline task is already closing this socket; a late
connection_init must not resurrect it. */
+ private boolean refuseAfterDeadline(GraphQLMessage message) {
+ LOGGER.warn("Refusing GraphQL WebSocket connection_init that arrived
after the authentication deadline");
+ closeConnection(message, CLOSE_POLICY_VIOLATION, "Not authenticated");
+ return false;
+ }
+
/**
* Reads a {@code Basic} credential from a connection_init payload. The
same credential format as the
* HTTP path, so both routes are verified identically.
diff --git
a/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocketFactory.java
b/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocketFactory.java
index bc042f471..0d0c949da 100644
---
a/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocketFactory.java
+++
b/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocketFactory.java
@@ -28,6 +28,8 @@ import
org.eclipse.jetty.websocket.servlet.ServletUpgradeRequest;
import org.eclipse.jetty.websocket.servlet.ServletUpgradeResponse;
import javax.security.auth.Subject;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
public class SubscriptionWebSocketFactory extends WebSocketServerFactory {
@@ -41,6 +43,12 @@ public class SubscriptionWebSocketFactory extends
WebSocketServerFactory {
private final GraphQLServletSecurityValidator validator;
+ /**
+ * Closes sockets that do not authenticate within their deadline. One
daemon thread for all sockets;
+ * stopped with the factory, which {@code WebSocketServlet.destroy()}
stops on undeploy.
+ */
+ private final ScheduledExecutorService authenticationDeadlineScheduler;
+
public SubscriptionWebSocketFactory(GraphQL graphQL, ServiceManager
serviceManager,
SecurityService securityService,
ExecutionContextManager
executionContextManager,
@@ -50,6 +58,11 @@ public class SubscriptionWebSocketFactory extends
WebSocketServerFactory {
this.securityService = securityService;
this.executionContextManager = executionContextManager;
this.validator = validator;
+ this.authenticationDeadlineScheduler =
Executors.newSingleThreadScheduledExecutor(runnable -> {
+ Thread thread = new Thread(runnable,
"graphql-ws-authentication-deadline");
+ thread.setDaemon(true);
+ return thread;
+ });
}
@Override
@@ -60,6 +73,15 @@ public class SubscriptionWebSocketFactory extends
WebSocketServerFactory {
Subject subject = securityService.getCurrentSubject();
ExecutionContext executionContext = subject != null ?
executionContextManager.getCurrentContext() : null;
return new SubscriptionWebSocket(graphQL, serviceManager, subject,
executionContext,
- securityService, executionContextManager, validator);
+ securityService, executionContextManager, validator,
authenticationDeadlineScheduler);
+ }
+
+ @Override
+ protected void doStop() throws Exception {
+ try {
+ super.doStop();
+ } finally {
+ authenticationDeadlineScheduler.shutdownNow();
+ }
}
}
diff --git
a/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocketTest.java
b/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocketTest.java
index bcc7f715d..c874f2a7a 100644
---
a/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocketTest.java
+++
b/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocketTest.java
@@ -41,6 +41,14 @@ import org.reactivestreams.Publisher;
import javax.security.auth.Subject;
import java.util.Collections;
import java.util.Map;
+import static org.mockito.Mockito.lenient;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.atLeastOnce;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -84,14 +92,17 @@ class SubscriptionWebSocketTest {
@Mock
private GraphQLServletSecurityValidator validator;
+ @Mock
+ private ScheduledExecutorService deadlineScheduler;
private SubscriptionWebSocket socket;
@BeforeEach
void setUp() {
socket = new SubscriptionWebSocket(graphQL, serviceManager, subject,
executionContext,
- securityService, executionContextManager, validator);
- when(session.getRemote()).thenReturn(remote);
+ securityService, executionContextManager, validator,
deadlineScheduler);
+ // Not every test sends a frame; keep the shared stub from tripping
strict-stubs checks.
+ lenient().when(session.getRemote()).thenReturn(remote);
socket.onWebSocketConnect(session);
}
@@ -99,7 +110,7 @@ class SubscriptionWebSocketTest {
@Test
void unauthenticated_startIsRefusedAndSocketClosed() {
SubscriptionWebSocket unauth = new SubscriptionWebSocket(graphQL,
serviceManager, null, null,
- securityService, executionContextManager, validator);
+ securityService, executionContextManager, validator,
deadlineScheduler);
unauth.onWebSocketConnect(session);
unauth.onWebSocketText(startMessage("\"variables\":null"));
@@ -112,7 +123,7 @@ class SubscriptionWebSocketTest {
@Test
void unauthenticated_connectionInitWithoutCredential_isRefused() {
SubscriptionWebSocket unauth = new SubscriptionWebSocket(graphQL,
serviceManager, null, null,
- securityService, executionContextManager, validator);
+ securityService, executionContextManager, validator,
deadlineScheduler);
unauth.onWebSocketConnect(session);
unauth.onWebSocketText("{\"type\":\"connection_init\",\"id\":\"1\"}");
@@ -125,7 +136,7 @@ class SubscriptionWebSocketTest {
void unauthenticated_connectionInitWithBadCredential_isRefused() {
when(validator.authenticateBasicCredential(anyString())).thenReturn(false);
SubscriptionWebSocket unauth = new SubscriptionWebSocket(graphQL,
serviceManager, null, null,
- securityService, executionContextManager, validator);
+ securityService, executionContextManager, validator,
deadlineScheduler);
unauth.onWebSocketConnect(session);
unauth.onWebSocketText("{\"type\":\"connection_init\",\"id\":\"1\","
@@ -141,7 +152,7 @@ class SubscriptionWebSocketTest {
when(securityService.getCurrentSubject()).thenReturn(subject);
when(executionContextManager.getCurrentContext()).thenReturn(executionContext);
SubscriptionWebSocket unauth = new SubscriptionWebSocket(graphQL,
serviceManager, null, null,
- securityService, executionContextManager, validator);
+ securityService, executionContextManager, validator,
deadlineScheduler);
unauth.onWebSocketConnect(session);
unauth.onWebSocketText("{\"type\":\"connection_init\",\"id\":\"1\","
@@ -232,4 +243,78 @@ class SubscriptionWebSocketTest {
payload.append("}}");
return payload.toString();
}
+
+ /** The deadline is a scheduled close, not just an idle timeout: pinging
cannot keep an unauthenticated socket open. */
+ @Test
+ void unauthenticated_socketSchedulesHardAuthenticationDeadline() {
+ SubscriptionWebSocket unauth = new SubscriptionWebSocket(graphQL,
serviceManager, null, null,
+ securityService, executionContextManager, validator,
deadlineScheduler);
+
+ unauth.onWebSocketConnect(session);
+
+ verify(session).setIdleTimeout(10_000L);
+ verify(deadlineScheduler).schedule(any(Runnable.class), eq(10_000L),
eq(TimeUnit.MILLISECONDS));
+ }
+
+ @Test
+ void authenticatedAtUpgrade_schedulesNoDeadline() {
+ // setUp's socket authenticated on the upgrade.
+ verify(deadlineScheduler, never()).schedule(any(Runnable.class),
anyLong(), any(TimeUnit.class));
+ verify(session, never()).setIdleTimeout(anyLong());
+ }
+
+ @Test
+ void deadlineFiring_closesUnauthenticatedSocketWithPolicyViolation() {
+ when(session.isOpen()).thenReturn(true);
+ SubscriptionWebSocket unauth = new SubscriptionWebSocket(graphQL,
serviceManager, null, null,
+ securityService, executionContextManager, validator,
deadlineScheduler);
+ unauth.onWebSocketConnect(session);
+ ArgumentCaptor<Runnable> deadline =
ArgumentCaptor.forClass(Runnable.class);
+ verify(deadlineScheduler).schedule(deadline.capture(), eq(10_000L),
eq(TimeUnit.MILLISECONDS));
+
+ deadline.getValue().run();
+
+ verify(session).close(eq(1008), anyString());
+ }
+
+ /** Authenticating cancels the deadline, and a deadline that fires late
must not close an authenticated socket. */
+ @Test
+ void deadlineFiring_afterAuthentication_doesNotCloseSocket() {
+
when(validator.authenticateBasicCredential(anyString())).thenReturn(true);
+ when(securityService.getCurrentSubject()).thenReturn(subject);
+
when(executionContextManager.getCurrentContext()).thenReturn(executionContext);
+ ScheduledFuture<?> task = mock(ScheduledFuture.class);
+ doReturn(task).when(deadlineScheduler).schedule(any(Runnable.class),
eq(10_000L), eq(TimeUnit.MILLISECONDS));
+ SubscriptionWebSocket unauth = new SubscriptionWebSocket(graphQL,
serviceManager, null, null,
+ securityService, executionContextManager, validator,
deadlineScheduler);
+ unauth.onWebSocketConnect(session);
+ ArgumentCaptor<Runnable> deadline =
ArgumentCaptor.forClass(Runnable.class);
+ verify(deadlineScheduler).schedule(deadline.capture(), eq(10_000L),
eq(TimeUnit.MILLISECONDS));
+
+ unauth.onWebSocketText("{\"type\":\"connection_init\",\"id\":\"1\","
+ + "\"payload\":{\"Authorization\":\"Basic dXNlcjpwYXNz\"}}");
+ deadline.getValue().run();
+
+ verify(task).cancel(false);
+ verify(session, never()).close(anyInt(), anyString());
+ }
+
+ /** Once the deadline has fired, a late connection_init cannot resurrect
the socket. */
+ @Test
+ void connectionInit_afterDeadlineExpired_isRefused() {
+ when(session.isOpen()).thenReturn(true);
+ SubscriptionWebSocket unauth = new SubscriptionWebSocket(graphQL,
serviceManager, null, null,
+ securityService, executionContextManager, validator,
deadlineScheduler);
+ unauth.onWebSocketConnect(session);
+ ArgumentCaptor<Runnable> deadline =
ArgumentCaptor.forClass(Runnable.class);
+ verify(deadlineScheduler).schedule(deadline.capture(), eq(10_000L),
eq(TimeUnit.MILLISECONDS));
+ deadline.getValue().run();
+
+ unauth.onWebSocketText("{\"type\":\"connection_init\",\"id\":\"1\","
+ + "\"payload\":{\"Authorization\":\"Basic dXNlcjpwYXNz\"}}");
+
+ verify(validator, never()).authenticateBasicCredential(anyString());
+ verify(session, atLeastOnce()).close(eq(1008), anyString());
+ verifyNoInteractions(graphQL);
+ }
}
