This is an automated email from the ASF dual-hosted git repository.
sergehuber pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/unomi.git
The following commit(s) were added to refs/heads/master by this push:
new 503a0d690 Check roles and tenant authority after a realm login (#875)
503a0d690 is described below
commit 503a0d690f47d858ad135aff2dda612ef9068c63
Author: Serge Huber <[email protected]>
AuthorDate: Thu Sep 24 12:59:37 2026 +0200
Check roles and tenant authority after a realm login (#875)
Co-authored-by: Claude Opus 4.8 <[email protected]>
---
.../auth/GraphQLServletSecurityValidator.java | 79 ++++++++++-----
.../servlet/websocket/SubscriptionWebSocket.java | 8 +-
.../auth/GraphQLServletSecurityValidatorTest.java | 107 ++++++++++++++++++++-
.../websocket/SubscriptionWebSocketTest.java | 4 +-
4 files changed, 166 insertions(+), 32 deletions(-)
diff --git
a/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/auth/GraphQLServletSecurityValidator.java
b/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/auth/GraphQLServletSecurityValidator.java
index d6ef91881..97624d7f1 100644
---
a/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/auth/GraphQLServletSecurityValidator.java
+++
b/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/auth/GraphQLServletSecurityValidator.java
@@ -21,6 +21,7 @@ import graphql.language.*;
import graphql.parser.Parser;
import org.apache.unomi.api.ExecutionContext;
import org.apache.unomi.api.security.SecurityService;
+import org.apache.unomi.api.security.UnomiRoles;
import org.apache.unomi.api.services.ExecutionContextManager;
import org.apache.unomi.api.tenants.ApiKey;
import org.apache.unomi.api.tenants.Tenant;
@@ -86,6 +87,8 @@ public class GraphQLServletSecurityValidator {
if (isAuthenticatedUser(req)) {
return true;
}
+ // A 401 carries a challenge whether the header was missing or its
credential was refused.
+ res.addHeader("WWW-Authenticate", "Basic realm=\"karaf\"");
res.sendError(HttpServletResponse.SC_UNAUTHORIZED);
return false;
}
@@ -289,7 +292,8 @@ public class GraphQLServletSecurityValidator {
* (WebSocket {@code connection_init}); when null, no tenant
header is consulted.
*/
private boolean authenticateBasic(String authHeader, HttpServletRequest
req) {
- if (authHeader == null || !authHeader.startsWith("Basic ")) {
+ // The scheme token is case-insensitive (RFC 7235).
+ if (authHeader == null || !authHeader.regionMatches(true, 0, "Basic ",
0, 6)) {
return false;
}
@@ -353,33 +357,58 @@ public class GraphQLServletSecurityValidator {
});
loginContext.login();
Subject loginSubject = loginContext.getSubject();
- boolean success = loginSubject != null;
- if (success) {
- if (req != null) {
- req.setAttribute(REMOTE_USER, username);
+ if (loginSubject == null) {
+ return false;
+ }
+
+ // Set the security context for JAAS authentication
+ securityService.setCurrentSubject(loginSubject);
+
+ // A successful realm login is not by itself an authorization to
use this API: the realm
+ // can carry accounts that hold no Unomi role at all. Require the
same administrator roles
+ // the REST admin surface requires.
+ if (!securityService.hasRole(UnomiRoles.ADMINISTRATOR)
+ &&
!securityService.hasRole(UnomiRoles.TENANT_ADMINISTRATOR)) {
+ LOG.warn("Refusing GraphQL access to '{}': the account holds
no Unomi administrator role", username);
+ securityService.clearCurrentSubject();
+ return false;
+ }
+
+ // Check for tenant ID header (only present when the credential
arrived on a request;
+ // the connection_init route carries none, so it can never select
a tenant this way)
+ String tenantId = req != null ?
req.getHeader(UNOMI_TENANT_ID_HEADER) : null;
+ if (tenantId != null && !tenantId.trim().isEmpty()) {
+ // Validate tenant exists
+ Tenant tenant = tenantService.getTenant(tenantId);
+ if (tenant == null) {
+ LOG.warn("Invalid tenant ID provided in header: {}",
tenantId);
+ securityService.clearCurrentSubject();
+ return false;
}
- // Set the security context for JAAS authentication
- securityService.setCurrentSubject(loginSubject);
-
- // Check for tenant ID header (only meaningful when the
credential arrived on a request)
- String tenantId = req != null ?
req.getHeader(UNOMI_TENANT_ID_HEADER) : null;
- if (tenantId != null && !tenantId.trim().isEmpty()) {
- // Validate tenant exists
- Tenant tenant = tenantService.getTenant(tenantId);
- if (tenant != null) {
-
executionContextManager.setCurrentContext(executionContextManager.createContext(tenantId));
- } else {
- LOG.warn("Invalid tenant ID provided in header: {}",
tenantId);
- // Same fallback as the "no tenant header" branch
below: the thread-local
- // execution context must always be set explicitly
here, otherwise a stale
- // context from a previous request on this pooled
thread could leak in.
-
executionContextManager.setCurrentContext(ExecutionContext.systemContext());
- }
- } else {
-
executionContextManager.setCurrentContext(ExecutionContext.systemContext());
+ // Naming a tenant is not the same as having authority over it.
+ if (!securityService.hasSystemAccess() &&
!securityService.hasTenantAccess(tenantId)) {
+ LOG.warn("Refusing GraphQL access to '{}': no authority
over tenant {}", username, tenantId);
+ securityService.clearCurrentSubject();
+ return false;
}
+
executionContextManager.setCurrentContext(executionContextManager.createContext(tenantId));
+ } else {
+ // No tenant header. The system context is inherited by every
tenant, so it is reserved
+ // for subjects that actually hold system access rather than
being the default.
+ if (!securityService.hasSystemAccess()) {
+ LOG.warn("Refusing GraphQL access to '{}': no tenant
specified and no system access", username);
+ securityService.clearCurrentSubject();
+ return false;
+ }
+ // The thread-local execution context must always be set
explicitly here, otherwise a
+ // stale context from a previous request on this pooled thread
could leak in.
+
executionContextManager.setCurrentContext(ExecutionContext.systemContext());
+ }
+
+ if (req != null) {
+ req.setAttribute(REMOTE_USER, username);
}
- return success;
+ return true;
} catch (LoginException e) {
LOG.debug("Login failed", e);
return false;
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 71299eb19..cf0a0c852 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
@@ -69,6 +69,9 @@ public class SubscriptionWebSocket extends WebSocketAdapter {
private volatile ScheduledFuture<?> deadlineTask;
+ /** The session's configured idle timeout, shortened while unauthenticated
and restored on authentication. */
+ private volatile long configuredIdleTimeout;
+
private boolean deadlineExpired;
private final SecurityService securityService;
@@ -101,6 +104,7 @@ public class SubscriptionWebSocket extends WebSocketAdapter
{
LOGGER.info("Opening web socket");
super.onWebSocketConnect(sess);
if (!authenticated) {
+ configuredIdleTimeout = sess.getIdleTimeout();
// 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.
@@ -265,8 +269,8 @@ public class SubscriptionWebSocket extends WebSocketAdapter
{
cancelAuthenticationDeadline();
final Session session = getSession();
if (session != null) {
- // Authenticated: drop the short unauthenticated deadline.
- session.setIdleTimeout(0);
+ // Authenticated: back to the idle timeout the session was
configured with, not "none".
+ session.setIdleTimeout(configuredIdleTimeout);
}
return true;
}
diff --git
a/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/servlet/auth/GraphQLServletSecurityValidatorTest.java
b/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/servlet/auth/GraphQLServletSecurityValidatorTest.java
index 0f842f220..a05d09193 100644
---
a/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/servlet/auth/GraphQLServletSecurityValidatorTest.java
+++
b/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/servlet/auth/GraphQLServletSecurityValidatorTest.java
@@ -19,6 +19,7 @@ package org.apache.unomi.graphql.servlet.auth;
import org.apache.unomi.api.ExecutionContext;
import org.apache.unomi.api.security.SecurityService;
+import org.apache.unomi.api.security.UnomiRoles;
import org.apache.unomi.api.services.ExecutionContextManager;
import org.apache.unomi.api.tenants.ApiKey;
import org.apache.unomi.api.tenants.Tenant;
@@ -94,8 +95,16 @@ class GraphQLServletSecurityValidatorTest {
Configuration.setConfiguration(previousConfiguration);
}
+ /** Grants the administrator role the JAAS branch now requires before it
will authorize anything. */
+ private void givenAdministratorRole() {
+
lenient().when(securityService.hasRole(UnomiRoles.ADMINISTRATOR)).thenReturn(true);
+ }
+
@Test
- void validate_withInvalidTenantHeader_fallsBackToSystemContext() throws
IOException {
+ void validate_withInvalidTenantHeader_isRejected() throws IOException {
+ // A tenant header that names no known tenant is refused; it must not
fall back to the system
+ // context, which is inherited by every tenant.
+ givenAdministratorRole();
when(request.getHeader("Authorization")).thenReturn(BASIC_AUTH);
when(request.getHeader(TENANT_HEADER)).thenReturn("not-a-real-tenant");
when(tenantService.getTenantByApiKey(any(),
eq(ApiKey.ApiKeyType.PRIVATE))).thenReturn(null);
@@ -103,9 +112,60 @@ class GraphQLServletSecurityValidatorTest {
boolean authenticated = validator.validate(null, null, request,
response);
- assertTrue(authenticated);
-
verify(executionContextManager).setCurrentContext(refEq(ExecutionContext.systemContext()));
- verify(response, never()).sendError(any(Integer.class));
+ assertFalse(authenticated);
+ verify(executionContextManager,
never()).setCurrentContext(refEq(ExecutionContext.systemContext()));
+ verify(response).sendError(HttpServletResponse.SC_UNAUTHORIZED);
+ }
+
+ @Test
+ void validate_withoutUnomiRole_isRejected() throws IOException {
+ // A realm account that carries no Unomi role (the shipped
health-check account, for one) must
+ // not obtain access, even though the realm login itself succeeds.
+ when(request.getHeader("Authorization")).thenReturn(BASIC_AUTH);
+ when(tenantService.getTenantByApiKey(any(),
eq(ApiKey.ApiKeyType.PRIVATE))).thenReturn(null);
+
+ boolean authenticated = validator.validate(null, null, request,
response);
+
+ assertFalse(authenticated);
+ verify(executionContextManager, never()).setCurrentContext(any());
+ verify(securityService).clearCurrentSubject();
+ verify(response).sendError(HttpServletResponse.SC_UNAUTHORIZED);
+ }
+
+ @Test
+ void validate_withTenantHeaderButNoAuthorityOverIt_isRejected() throws
IOException {
+ Tenant tenant = new Tenant();
+ tenant.setItemId("someone-elses-tenant");
+
+ givenAdministratorRole();
+ when(request.getHeader("Authorization")).thenReturn(BASIC_AUTH);
+
when(request.getHeader(TENANT_HEADER)).thenReturn("someone-elses-tenant");
+ when(tenantService.getTenantByApiKey(any(),
eq(ApiKey.ApiKeyType.PRIVATE))).thenReturn(null);
+
when(tenantService.getTenant("someone-elses-tenant")).thenReturn(tenant);
+ when(securityService.hasSystemAccess()).thenReturn(false);
+
when(securityService.hasTenantAccess("someone-elses-tenant")).thenReturn(false);
+
+ boolean authenticated = validator.validate(null, null, request,
response);
+
+ assertFalse(authenticated);
+ verify(executionContextManager,
never()).createContext("someone-elses-tenant");
+ verify(response).sendError(HttpServletResponse.SC_UNAUTHORIZED);
+ }
+
+ @Test
+ void validate_withoutTenantHeaderAndNoSystemAccess_isRejected() throws
IOException {
+ // The system context is not the default for an authenticated caller.
+ givenAdministratorRole();
+ when(request.getHeader("Authorization")).thenReturn(BASIC_AUTH);
+ when(request.getHeader(TENANT_HEADER)).thenReturn(null);
+ when(tenantService.getTenantByApiKey(any(),
eq(ApiKey.ApiKeyType.PRIVATE))).thenReturn(null);
+ when(securityService.hasSystemAccess()).thenReturn(false);
+
+ boolean authenticated = validator.validate(null, null, request,
response);
+
+ assertFalse(authenticated);
+ verify(executionContextManager,
never()).setCurrentContext(refEq(ExecutionContext.systemContext()));
+ verify(response).sendError(HttpServletResponse.SC_UNAUTHORIZED);
}
@Test
@@ -113,10 +173,12 @@ class GraphQLServletSecurityValidatorTest {
Tenant tenant = new Tenant();
tenant.setItemId("known-tenant");
+ givenAdministratorRole();
when(request.getHeader("Authorization")).thenReturn(BASIC_AUTH);
when(request.getHeader(TENANT_HEADER)).thenReturn("known-tenant");
when(tenantService.getTenantByApiKey(any(),
eq(ApiKey.ApiKeyType.PRIVATE))).thenReturn(null);
when(tenantService.getTenant("known-tenant")).thenReturn(tenant);
+ when(securityService.hasTenantAccess("known-tenant")).thenReturn(true);
ExecutionContext tenantContext = new ExecutionContext("known-tenant",
null, null);
when(executionContextManager.createContext("known-tenant")).thenReturn(tenantContext);
@@ -128,6 +190,21 @@ class GraphQLServletSecurityValidatorTest {
verify(executionContextManager,
never()).setCurrentContext(refEq(ExecutionContext.systemContext()));
}
+ @Test
+ void validate_withSystemAccessAndNoTenantHeader_usesSystemContext() throws
IOException {
+ givenAdministratorRole();
+ when(request.getHeader("Authorization")).thenReturn(BASIC_AUTH);
+ when(request.getHeader(TENANT_HEADER)).thenReturn(null);
+ when(tenantService.getTenantByApiKey(any(),
eq(ApiKey.ApiKeyType.PRIVATE))).thenReturn(null);
+ when(securityService.hasSystemAccess()).thenReturn(true);
+
+ boolean authenticated = validator.validate(null, null, request,
response);
+
+ assertTrue(authenticated);
+
verify(executionContextManager).setCurrentContext(refEq(ExecutionContext.systemContext()));
+ verify(response, never()).sendError(any(Integer.class));
+ }
+
@Test
void validate_withoutAuthorizationHeader_isRejected() throws IOException {
when(request.getHeader("Authorization")).thenReturn(null);
@@ -151,6 +228,8 @@ class GraphQLServletSecurityValidatorTest {
@Test
void validateWebSocketUpgrade_withBasicAuth_isAccepted() throws
IOException {
+ givenAdministratorRole();
+ when(securityService.hasSystemAccess()).thenReturn(true);
when(request.getHeader("Authorization")).thenReturn(BASIC_AUTH);
when(tenantService.getTenantByApiKey(any(),
eq(ApiKey.ApiKeyType.PRIVATE))).thenReturn(null);
@@ -161,6 +240,21 @@ class GraphQLServletSecurityValidatorTest {
verify(response, never()).sendError(any(Integer.class));
}
+ /** HTTP authentication scheme names are case-insensitive. */
+ @Test
+ void validateWebSocketUpgrade_withLowercaseScheme_isAccepted() throws
IOException {
+ givenAdministratorRole();
+ when(securityService.hasSystemAccess()).thenReturn(true);
+
when(request.getHeader("Authorization")).thenReturn(BASIC_AUTH.replaceFirst("Basic",
"basic"));
+ when(tenantService.getTenantByApiKey(any(),
eq(ApiKey.ApiKeyType.PRIVATE))).thenReturn(null);
+
+ boolean authenticated = validator.validateWebSocketUpgrade(request,
response);
+
+ assertTrue(authenticated);
+ verify(securityService).setCurrentSubject(any(Subject.class));
+ verify(response, never()).sendError(any(Integer.class));
+ }
+
@Test
void validateWebSocketUpgrade_rejectsPublicApiKeyOnly() throws IOException
{
// No Authorization header — public API key alone must not open
subscriptions.
@@ -182,6 +276,8 @@ class GraphQLServletSecurityValidatorTest {
assertFalse(authenticated);
verify(response).sendError(HttpServletResponse.SC_UNAUTHORIZED);
+ // A 401 must carry a challenge whichever branch produced it.
+ verify(response).addHeader("WWW-Authenticate", "Basic
realm=\"karaf\"");
verify(securityService, never()).setCurrentSubject(any());
}
@@ -298,6 +394,9 @@ class GraphQLServletSecurityValidatorTest {
/** Control: a non-blank credential still reaches the realm and is
accepted by it. */
@Test
void validate_withNonBlankPassword_reachesJaas() throws IOException {
+ // Role + system access are required after JAAS succeeds when no
tenant header is present.
+ givenAdministratorRole();
+ when(securityService.hasSystemAccess()).thenReturn(true);
when(request.getHeader("Authorization")).thenReturn(BASIC_AUTH);
when(tenantService.getTenantByApiKey(any(),
eq(ApiKey.ApiKeyType.PRIVATE))).thenReturn(null);
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 c874f2a7a..895d422c2 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
@@ -151,6 +151,7 @@ class SubscriptionWebSocketTest {
when(validator.authenticateBasicCredential(anyString())).thenReturn(true);
when(securityService.getCurrentSubject()).thenReturn(subject);
when(executionContextManager.getCurrentContext()).thenReturn(executionContext);
+ when(session.getIdleTimeout()).thenReturn(30_000L);
SubscriptionWebSocket unauth = new SubscriptionWebSocket(graphQL,
serviceManager, null, null,
securityService, executionContextManager, validator,
deadlineScheduler);
unauth.onWebSocketConnect(session);
@@ -161,7 +162,8 @@ class SubscriptionWebSocketTest {
// Identity captured onto the socket, and not left bound to this
shared IO thread.
verify(securityService).clearCurrentSubject();
verify(session, never()).close(anyInt(), anyString());
- verify(session).setIdleTimeout(0);
+ // Back to the session's configured idle timeout, not disabled.
+ verify(session).setIdleTimeout(30_000L);
}
@Test
