From ae5f2c746777463f66ef5e6a880acd5c7c43b62c Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Mon, 29 Jun 2026 06:53:12 +0200 Subject: [PATCH 1/2] =?UTF-8?q?UNOMI-884:=20Platform=20polish=20audit=20?= =?UTF-8?q?=E2=80=94=20GraphQL=20invalid-tenant=20fallback=20(Phase=202=20?= =?UTF-8?q?PR=203)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit §2 audit of mega-PR 3 (L+M+N+O+P) found master already ahead via #760, #762, #773, #768, #781 on auth, validation, toString, rangeQuery, and past-event wiring. Port the remaining GraphQL security fix: fall back to system context when X-Unomi-Tenant-Id is invalid after JAAS login. --- .../graphql/servlet/auth/GraphQLServletSecurityValidator.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 a295e6ebf..e33a77fa7 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 @@ -198,7 +198,8 @@ private boolean isAuthenticatedUser(HttpServletRequest req) { if (tenant != null) { executionContextManager.setCurrentContext(executionContextManager.createContext(tenantId)); } else { - LOG.warn("Invalid tenant ID provided in header: {} — leaving context derived from JAAS login", tenantId); + LOG.warn("Invalid tenant ID provided in header: {}", tenantId); + executionContextManager.setCurrentContext(ExecutionContext.systemContext()); } } else { executionContextManager.setCurrentContext(ExecutionContext.systemContext()); From 291a236cbe71c0bf10f3b1ac7fbcc07fa2c9751d Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Mon, 29 Jun 2026 07:41:02 +0200 Subject: [PATCH 2/2] UNOMI-884: Add unit tests and rationale comment for GraphQL tenant fallback Closes the test-coverage gap from the prior commit's invalid-tenant fallback fix: adds JUnit/Mockito test deps to cdp-graphql-api-impl and covers the JAAS-authenticated branches of GraphQLServletSecurityValidator (invalid tenant header, valid tenant header, missing Authorization header). Also documents why the fallback explicitly sets systemContext() instead of leaving the thread-local context untouched. --- graphql/cxs-impl/pom.xml | 16 ++ .../auth/GraphQLServletSecurityValidator.java | 3 + .../GraphQLServletSecurityValidatorTest.java | 199 ++++++++++++++++++ 3 files changed, 218 insertions(+) create mode 100644 graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/servlet/auth/GraphQLServletSecurityValidatorTest.java diff --git a/graphql/cxs-impl/pom.xml b/graphql/cxs-impl/pom.xml index 03ea7b058..828d7f4bc 100644 --- a/graphql/cxs-impl/pom.xml +++ b/graphql/cxs-impl/pom.xml @@ -147,6 +147,22 @@ websocket-server provided + + + org.junit.jupiter + junit-jupiter + test + + + org.mockito + mockito-junit-jupiter + test + + + org.mockito + mockito-core + test + 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 e33a77fa7..441449e42 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 @@ -199,6 +199,9 @@ private boolean isAuthenticatedUser(HttpServletRequest req) { 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 { 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 new file mode 100644 index 000000000..e64acd008 --- /dev/null +++ b/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/servlet/auth/GraphQLServletSecurityValidatorTest.java @@ -0,0 +1,199 @@ +/* + * 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.unomi.graphql.servlet.auth; + +import org.apache.unomi.api.ExecutionContext; +import org.apache.unomi.api.security.SecurityService; +import org.apache.unomi.api.services.ExecutionContextManager; +import org.apache.unomi.api.tenants.ApiKey; +import org.apache.unomi.api.tenants.Tenant; +import org.apache.unomi.api.tenants.TenantService; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import javax.security.auth.Subject; +import javax.security.auth.callback.Callback; +import javax.security.auth.callback.CallbackHandler; +import javax.security.auth.callback.NameCallback; +import javax.security.auth.callback.PasswordCallback; +import javax.security.auth.callback.UnsupportedCallbackException; +import javax.security.auth.login.AppConfigurationEntry; +import javax.security.auth.login.Configuration; +import javax.security.auth.login.LoginException; +import javax.security.auth.spi.LoginModule; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.util.Base64; +import java.util.HashMap; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.refEq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Covers the JAAS-authenticated tenant resolution branch of {@link GraphQLServletSecurityValidator}, + * in particular the fallback to {@link ExecutionContext#systemContext()} when the + * {@code X-Unomi-Tenant-Id} header does not resolve to a known tenant (UNOMI-884). + */ +@ExtendWith(MockitoExtension.class) +class GraphQLServletSecurityValidatorTest { + + private static final String TENANT_HEADER = "X-Unomi-Tenant-Id"; + private static final String BASIC_AUTH = "Basic " + Base64.getEncoder().encodeToString("user:pass".getBytes()); + + private Configuration previousConfiguration; + + @Mock + private TenantService tenantService; + @Mock + private SecurityService securityService; + @Mock + private ExecutionContextManager executionContextManager; + @Mock + private HttpServletRequest request; + @Mock + private HttpServletResponse response; + + private GraphQLServletSecurityValidator validator; + + @BeforeEach + void setUp() { + previousConfiguration = Configuration.getConfiguration(); + Configuration.setConfiguration(new AlwaysSucceedingKarafConfiguration()); + validator = new GraphQLServletSecurityValidator(tenantService, securityService, executionContextManager); + } + + @AfterEach + void tearDown() { + Configuration.setConfiguration(previousConfiguration); + } + + @Test + void validate_withInvalidTenantHeader_fallsBackToSystemContext() throws IOException { + 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); + when(tenantService.getTenant("not-a-real-tenant")).thenReturn(null); + + 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_withValidTenantHeader_createsTenantContext() throws IOException { + Tenant tenant = new Tenant(); + tenant.setItemId("known-tenant"); + + 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); + ExecutionContext tenantContext = new ExecutionContext("known-tenant", null, null); + when(executionContextManager.createContext("known-tenant")).thenReturn(tenantContext); + + boolean authenticated = validator.validate(null, null, request, response); + + assertTrue(authenticated); + verify(executionContextManager).createContext("known-tenant"); + verify(executionContextManager).setCurrentContext(tenantContext); + verify(executionContextManager, never()).setCurrentContext(refEq(ExecutionContext.systemContext())); + } + + @Test + void validate_withoutAuthorizationHeader_isRejected() throws IOException { + when(request.getHeader("Authorization")).thenReturn(null); + + boolean authenticated = validator.validate(null, null, request, response); + + assertFalse(authenticated); + verify(response).sendError(HttpServletResponse.SC_UNAUTHORIZED); + } + + /** + * Minimal JAAS configuration that makes {@code new LoginContext("karaf", ...)} succeed + * without requiring a real Karaf realm, so the post-login branches under test can run + * as a plain unit test. + */ + private static class AlwaysSucceedingKarafConfiguration extends Configuration { + @Override + public AppConfigurationEntry[] getAppConfigurationEntry(String name) { + if (!"karaf".equals(name)) { + return null; + } + return new AppConfigurationEntry[]{ + new AppConfigurationEntry( + AlwaysSucceedingLoginModule.class.getName(), + AppConfigurationEntry.LoginModuleControlFlag.REQUIRED, + new HashMap<>()) + }; + } + } + + public static class AlwaysSucceedingLoginModule implements LoginModule { + private Subject subject; + private CallbackHandler callbackHandler; + + @Override + public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) { + this.subject = subject; + this.callbackHandler = callbackHandler; + } + + @Override + public boolean login() throws LoginException { + try { + NameCallback nameCallback = new NameCallback("name"); + PasswordCallback passwordCallback = new PasswordCallback("password", false); + callbackHandler.handle(new Callback[]{nameCallback, passwordCallback}); + } catch (IOException | UnsupportedCallbackException e) { + throw new LoginException(e.getMessage()); + } + return true; + } + + @Override + public boolean commit() { + return true; + } + + @Override + public boolean abort() { + return true; + } + + @Override + public boolean logout() { + subject.getPrincipals().clear(); + return true; + } + } +}