This is an automated email from the ASF dual-hosted git repository. sergehuber pushed a commit to branch UNOMI-1031-authorizedips-fail-closed in repository https://gitbox.apache.org/repos/asf/unomi.git
commit bfd086749600d1514199316603e8e692214c6688 Author: Serge Huber <[email protected]> AuthorDate: Mon Nov 2 10:44:47 2026 +0100 UNOMI-1031: Fail closed on an empty authorized-IP set for restricted event types A tenant with no authorizedIPs, the default for a new tenant, accepted restricted event types (sessionCreated, sessionReassigned, profileUpdated, ruleFired, updateProperties and the tenant's own restrictedEventTypes) from any public caller. They are now refused until the tenant lists the IPs allowed to send them, as 3.0 refused them without a configured third-party server. Tenant private key and administrator callers are not IP-checked on REST or GraphQL: they can already change any profile through the private API. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../apache/unomi/api/services/EventService.java | 3 +- .../graphql/commands/ProcessEventsCommand.java | 17 ++++-- .../graphql/commands/ProcessEventsCommandTest.java | 20 +++++-- .../org/apache/unomi/itests/ContextServletIT.java | 61 ++++++++++++++++++++++ .../asciidoc/migrations/migrate-3.0-to-4.0.adoc | 4 ++ manual/src/main/asciidoc/multitenancy.adoc | 6 +++ .../rest/service/impl/RestServiceUtilsImpl.java | 7 ++- .../services/impl/events/EventServiceImpl.java | 5 ++ .../services/impl/events/EventServiceImplTest.java | 24 +++++++++ 9 files changed, 136 insertions(+), 11 deletions(-) diff --git a/api/src/main/java/org/apache/unomi/api/services/EventService.java b/api/src/main/java/org/apache/unomi/api/services/EventService.java index d6634bce2..ca22a2fce 100644 --- a/api/src/main/java/org/apache/unomi/api/services/EventService.java +++ b/api/src/main/java/org/apache/unomi/api/services/EventService.java @@ -63,7 +63,8 @@ public interface EventService { /** * Checks whether the tenant may send the given event. - * Restricted event types must be explicitly allowed per tenant. + * Restricted event types must be explicitly allowed per tenant: they are accepted only from a + * source IP in the tenant's authorized IPs, so a tenant with no authorized IP refuses them. * * @param event event to test * @param tenantId tenant identifier diff --git a/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/commands/ProcessEventsCommand.java b/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/commands/ProcessEventsCommand.java index 5d4147f58..c2b8eca69 100644 --- a/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/commands/ProcessEventsCommand.java +++ b/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/commands/ProcessEventsCommand.java @@ -21,6 +21,7 @@ import graphql.schema.GraphQLInputObjectField; import graphql.schema.GraphQLInputObjectType; import org.apache.unomi.api.Event; import org.apache.unomi.api.ExecutionContext; +import org.apache.unomi.api.security.SecurityService; import org.apache.unomi.api.services.EventService; import org.apache.unomi.api.services.ExecutionContextManager; import org.apache.unomi.api.services.ProfileService; @@ -166,7 +167,8 @@ public class ProcessEventsCommand extends BaseCommand<Integer> { final EventService eventService = serviceManager.getService(EventService.class); if (!isEventAllowedForCurrentTenant(event, eventService, - serviceManager.getService(ExecutionContextManager.class))) { + serviceManager.getService(ExecutionContextManager.class), + serviceManager.getService(SecurityService.class))) { LOGGER.debug("Event type {} is not authorized for this tenant, skipping it", event.getEventType()); return; } @@ -180,14 +182,19 @@ public class ProcessEventsCommand extends BaseCommand<Integer> { } /** - * Same restricted-event-type check as REST {@code EventService#send} callers. GraphQL does not - * currently thread a client IP through this command, so the source IP is {@code null}: a - * restricted type is refused when the tenant has an authorized-IP list, and unrestricted types + * Same restricted-event-type check as REST {@code EventService#send} callers. A tenant or system + * administrator can already change any profile through the private API, so it is not gated. + * GraphQL does not thread a client IP through this command, so for any other caller (public API + * key) the source IP is {@code null}: a restricted type is always refused, and unrestricted types * still pass. */ static boolean isEventAllowedForCurrentTenant(final Event event, final EventService eventService, - final ExecutionContextManager executionContextManager) { + final ExecutionContextManager executionContextManager, + final SecurityService securityService) { + if (securityService.hasSystemAccess()) { + return true; + } final ExecutionContext executionContext = executionContextManager != null ? executionContextManager.getCurrentContext() : null; final String tenantId = executionContext != null ? executionContext.getTenantId() : null; diff --git a/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/commands/ProcessEventsCommandTest.java b/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/commands/ProcessEventsCommandTest.java index a387637c9..8f12da647 100644 --- a/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/commands/ProcessEventsCommandTest.java +++ b/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/commands/ProcessEventsCommandTest.java @@ -19,6 +19,7 @@ package org.apache.unomi.graphql.commands; import org.apache.unomi.api.Event; import org.apache.unomi.api.ExecutionContext; import org.apache.unomi.api.Profile; +import org.apache.unomi.api.security.SecurityService; import org.apache.unomi.api.services.EventService; import org.apache.unomi.api.services.ExecutionContextManager; import org.junit.jupiter.api.Test; @@ -30,6 +31,8 @@ import java.util.Date; 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.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -44,6 +47,8 @@ class ProcessEventsCommandTest { private ExecutionContextManager executionContextManager; @Mock private ExecutionContext executionContext; + @Mock + private SecurityService securityService; @Test void allowsEventWhenTenantGatePasses() { @@ -52,7 +57,7 @@ class ProcessEventsCommandTest { when(executionContext.getTenantId()).thenReturn(TENANT_ID); when(eventService.isEventAllowedForTenant(event, TENANT_ID, null)).thenReturn(true); - assertTrue(ProcessEventsCommand.isEventAllowedForCurrentTenant(event, eventService, executionContextManager)); + assertTrue(ProcessEventsCommand.isEventAllowedForCurrentTenant(event, eventService, executionContextManager, securityService)); verify(eventService).isEventAllowedForTenant(event, TENANT_ID, null); } @@ -63,7 +68,7 @@ class ProcessEventsCommandTest { when(executionContext.getTenantId()).thenReturn(TENANT_ID); when(eventService.isEventAllowedForTenant(event, TENANT_ID, null)).thenReturn(false); - assertFalse(ProcessEventsCommand.isEventAllowedForCurrentTenant(event, eventService, executionContextManager)); + assertFalse(ProcessEventsCommand.isEventAllowedForCurrentTenant(event, eventService, executionContextManager, securityService)); verify(eventService).isEventAllowedForTenant(event, TENANT_ID, null); } @@ -72,10 +77,19 @@ class ProcessEventsCommandTest { Event event = viewEvent(); when(eventService.isEventAllowedForTenant(event, null, null)).thenReturn(false); - assertFalse(ProcessEventsCommand.isEventAllowedForCurrentTenant(event, eventService, null)); + assertFalse(ProcessEventsCommand.isEventAllowedForCurrentTenant(event, eventService, null, securityService)); verify(eventService).isEventAllowedForTenant(event, null, null); } + @Test + void trustedCallerSkipsTenantGate() { + Event event = new Event("updateProperties", null, new Profile(), null, null, null, new Date()); + when(securityService.hasSystemAccess()).thenReturn(true); + + assertTrue(ProcessEventsCommand.isEventAllowedForCurrentTenant(event, eventService, executionContextManager, securityService)); + verify(eventService, never()).isEventAllowedForTenant(any(), any(), any()); + } + private static Event viewEvent() { return new Event("view", null, new Profile(), null, null, null, new Date()); } diff --git a/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java b/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java index 850b04929..bbe3ae91d 100644 --- a/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java @@ -489,6 +489,67 @@ public class ContextServletIT extends BaseIT { DEFAULT_TRYING_TIMEOUT, DEFAULT_SHOULDBETRUE_TRIES); } + @Test + public void testPublicHttp_protectedEventNeedsAuthorizedIP() throws Exception { + String sessionId = "protected-ip-session-" + System.currentTimeMillis(); + ContextRequest establishReq = new ContextRequest(); + establishReq.setSessionId(sessionId); + HttpPost establish = new HttpPost(getFullUrl(CONTEXT_URL)); + addPublicTenantAuth(establish); + establish.setEntity(new StringEntity(getObjectMapper().writeValueAsString(establishReq), ContentType.APPLICATION_JSON)); + RequestResponse established = executeContextJSONRequest(establish, sessionId); + String profileId = established.getContextResponse().getProfileId(); + keepTrying("Profile not found", () -> profileService.load(profileId), Objects::nonNull, + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + // The test tenant authorizes no IP: a protected event from a public caller is refused, + // even on its own profile. + sendOwnProfileUpdate(sessionId, established.getCookieHeaderValue(), profileId, "REFUSED"); + shouldBeTrueUntilEnd("Protected event must be refused while the tenant authorizes no IP", + () -> profileService.load(profileId), + p -> p.getProperty("firstName") == null, + DEFAULT_TRYING_TIMEOUT, DEFAULT_SHOULDBETRUE_TRIES); + + Tenant settings = tenantService.getTenant(testTenant.getItemId()); + settings.setAuthorizedIPs(new HashSet<>(Arrays.asList("127.0.0.1", "::1"))); + tenantService.updateTenantSettings(settings); + try { + keepTrying("Protected event must be accepted from an authorized IP", () -> { + try { + sendOwnProfileUpdate(sessionId, established.getCookieHeaderValue(), profileId, "ACCEPTED"); + } catch (Exception e) { + throw new RuntimeException(e); + } + return profileService.load(profileId); + }, p -> "ACCEPTED".equals(p.getProperty("firstName")), DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + } finally { + // The tenant is shared by the whole suite (PerSuite): put the default back. + Tenant restore = tenantService.getTenant(testTenant.getItemId()); + restore.setAuthorizedIPs(new HashSet<>()); + tenantService.updateTenantSettings(restore); + } + } + + private void sendOwnProfileUpdate(String sessionId, String cookie, String profileId, String firstName) throws Exception { + Event updateEvent = new Event(); + updateEvent.setEventType("updateProperties"); + updateEvent.setScope(TEST_SCOPE); + Map<String, Object> props = new HashMap<>(); + props.put("targetId", profileId); + props.put("targetType", "profile"); + props.put("update", Collections.singletonMap("properties.firstName", firstName)); + updateEvent.setProperties(props); + + ContextRequest request = new ContextRequest(); + request.setSessionId(sessionId); + request.setEvents(Collections.singletonList(updateEvent)); + HttpPost post = new HttpPost(getFullUrl(CONTEXT_URL)); + addPublicTenantAuth(post); + post.addHeader("Cookie", cookie); + post.setEntity(new StringEntity(getObjectMapper().writeValueAsString(request), ContentType.APPLICATION_JSON)); + executeContextJSONRequest(post, sessionId); + } + @Test public void testPrivateKeyHttp_updateProperties_canUpdateAnotherProfile() throws Exception { String otherId = "trusted-update-other-" + System.currentTimeMillis(); diff --git a/manual/src/main/asciidoc/migrations/migrate-3.0-to-4.0.adoc b/manual/src/main/asciidoc/migrations/migrate-3.0-to-4.0.adoc index 3c2a6a090..1cde93606 100644 --- a/manual/src/main/asciidoc/migrations/migrate-3.0-to-4.0.adoc +++ b/manual/src/main/asciidoc/migrations/migrate-3.0-to-4.0.adoc @@ -470,6 +470,10 @@ In addition to tenant API keys, Unomi 4.0 hardens several contracts that can bre |Known defaults such as `karaf` / `karaf` were common in samples |Set `UNOMI_ROOT_PASSWORD` and `UNOMI_HEALTHCHECK_PASSWORD` before start (no shipped known defaults). +|Protected events from public callers +|Accepted only from a configured third-party server (`X-Unomi-Peer` key and IP) +|Accepted only from an IP in the tenant's `authorizedIPs`; an empty list refuses them. Tenant private key and administrator callers are not IP-checked. See <<_multitenancy,Multi-tenancy>>. + |Groovy Actions / Router import-export REST |Often callable with tenant private key |Requires **system administrator** (`ADMINISTRATOR`), not tenant administrator. diff --git a/manual/src/main/asciidoc/multitenancy.adoc b/manual/src/main/asciidoc/multitenancy.adoc index 9acd5c9aa..edc664744 100644 --- a/manual/src/main/asciidoc/multitenancy.adoc +++ b/manual/src/main/asciidoc/multitenancy.adoc @@ -73,6 +73,12 @@ curl -X POST http://localhost:8181/cxs/tenants \ `publicCorsAllowedOrigins` is optional: it lists the websites that may use the visitor cookie with this tenant's key (see <<_public_tracker_tenant_origins,Websites listed by each tenant>>). An invalid entry is refused with `400`. +Protected event types (`sessionCreated`, `sessionReassigned`, `profileUpdated`, `ruleFired`, `updateProperties`, plus +any extra types listed in the tenant's `restrictedEventTypes`) are accepted from a public API key caller only when the +caller IP is in the tenant's `authorizedIPs` (addresses or CIDR ranges). An empty `authorizedIPs` list, the default, +refuses them from every public caller; update the tenant to set it. Callers using the tenant private key or +administrator credentials are not IP-checked. + The create response includes the tenant with **masked** API key metadata only (plaintext secrets are not returned and cannot be recovered from storage): [source,json] diff --git a/rest/src/main/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImpl.java b/rest/src/main/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImpl.java index cc01aea51..e81c1f24e 100644 --- a/rest/src/main/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImpl.java +++ b/rest/src/main/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImpl.java @@ -476,8 +476,11 @@ public class RestServiceUtilsImpl implements RestServiceUtils { continue; } } else { - // Normal V3 event authorization - if (!eventService.isEventAllowedForTenant(event, tenantId, eventsRequestContext.getRequest().getRemoteAddr())) { + // Normal V3 event authorization. A tenant or system administrator can already + // change any profile through the private endpoints, so the IP gate on + // protected events only applies to public callers. + if (!securityService.hasSystemAccess() + && !eventService.isEventAllowedForTenant(event, tenantId, eventsRequestContext.getRequest().getRemoteAddr())) { LOGGER.debug("Tenant is not authorized to send event {} from IP {}", event.getEventType(), eventsRequestContext.getRequest().getRemoteAddr()); //Don't count the event that failed eventsRequestContext.setProcessedItems(eventsRequestContext.getProcessedItems() - 1); diff --git a/services/src/main/java/org/apache/unomi/services/impl/events/EventServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/events/EventServiceImpl.java index 493cc03ca..470539d4e 100644 --- a/services/src/main/java/org/apache/unomi/services/impl/events/EventServiceImpl.java +++ b/services/src/main/java/org/apache/unomi/services/impl/events/EventServiceImpl.java @@ -183,6 +183,11 @@ public class EventServiceImpl implements EventService { private boolean checkIPAuthorization(Tenant tenant, String sourceIP) { Set<String> authorizedIPs = tenant.getAuthorizedIPs(); + // Fail closed like the 3.0 third-party check: a tenant that authorized no IP accepts + // protected events from nobody. IPValidationUtils treats an empty set as "no restriction". + if (authorizedIPs == null || authorizedIPs.isEmpty()) { + return false; + } return IPValidationUtils.isIpAuthorized(sourceIP, authorizedIPs); } diff --git a/services/src/test/java/org/apache/unomi/services/impl/events/EventServiceImplTest.java b/services/src/test/java/org/apache/unomi/services/impl/events/EventServiceImplTest.java index a5092fd22..54c944e41 100644 --- a/services/src/test/java/org/apache/unomi/services/impl/events/EventServiceImplTest.java +++ b/services/src/test/java/org/apache/unomi/services/impl/events/EventServiceImplTest.java @@ -565,6 +565,30 @@ public class EventServiceImplTest { assertTrue(result); } + @Test + public void testIsEventAllowedForTenant_NoAuthorizedIPsRefusesRestrictedEvents() { + eventService.setRestrictedEventTypeIds(new HashSet<>(Arrays.asList("globalRestricted"))); + + Tenant tenant = new Tenant(); + tenant.setItemId(TENANT_1); + tenant.setRestrictedEventTypes(new HashSet<>(Arrays.asList("tenantRestricted"))); + tenantService.saveTenant(tenant); + + for (String eventType : Arrays.asList("globalRestricted", "tenantRestricted")) { + Event event = createTestEvent(); + event.setEventType(eventType); + assertFalse(eventService.isEventAllowedForTenant(event, TENANT_1, "127.0.0.1"), + eventType + " must be refused when the tenant authorized no IP"); + assertFalse(eventService.isEventAllowedForTenant(event, TENANT_1, null), + eventType + " must be refused without a source IP"); + } + + Event unrestricted = createTestEvent(); + unrestricted.setEventType("unrestricted"); + assertTrue(eventService.isEventAllowedForTenant(unrestricted, TENANT_1, "127.0.0.1"), + "Unrestricted event should still be allowed"); + } + @Test public void testEventAllowed() { String tenantId = "test_tenant";
