This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch graphql-authenticated-role-tenant-checks in repository https://gitbox.apache.org/repos/asf/unomi.git
commit c62897a469a5821abd2981289e15a1d6af8a26a2 Author: Serge Huber <[email protected]> AuthorDate: Fri Aug 21 12:09:47 2026 +0200 Check roles and tenant authority after a realm login A successful karaf realm login was treated as sufficient authorization for the GraphQL API: no role was inspected, the X-Unomi-Tenant-Id header was honoured without checking the subject's authority over the named tenant, and a request without that header fell back to the system context. Require an administrator role after login, honour the tenant header only for a subject with authority over that tenant, and reserve the system context for subjects that hold system access, refusing the request otherwise instead of falling back to it. Co-Authored-By: Claude Opus 4.8 <[email protected]> --- .../auth/GraphQLServletSecurityValidator.java | 74 ++++++++++++------ .../auth/GraphQLServletSecurityValidatorTest.java | 87 +++++++++++++++++++++- 2 files changed, 133 insertions(+), 28 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..199f6394a 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; @@ -353,33 +354,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/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..fbf5feac2 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);
