This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch UNOMI-972-credentials-profile-binding-privileged-rest in repository https://gitbox.apache.org/repos/asf/unomi.git
commit 4c970681aa2ce5698f17c0dad469da57e422776d Author: Serge Huber <[email protected]> AuthorDate: Mon Aug 10 09:27:03 2026 +0200 UNOMI-972: gate cross-profile merge and systemProperties writes on a trusted caller Reported issue 6. MergeProfilesOnPropertyAction resolved its merge value from attacker-controlled event input and never checked that the event's profile was entitled to claim it, and UpdatePropertiesAction took targetId straight from the event and loaded any profile with it. Wired to a public event type - which the shipped login sample demonstrated - a credential-less caller could rebind its session to a victim's profile or write arbitrary properties onto one. Both actions now refuse a cross-profile merge or update, and any write to the reserved systemProperties area, unless the caller holds system access. The refusal is logged at WARN so an operator sees attempts. The systemProperties gate matches the bare key as well as the dotted prefix: for a flat name PropertyHelper falls through to BeanUtils.setProperty, which calls Profile#setSystemProperties(Map) and replaces the entire map - a superset of the per-key write the gate exists to block, and enough to plant the mergeIdentifier the merge action keys on. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- .../actions/MergeProfilesOnPropertyAction.java | 32 ++++ .../baseplugin/actions/UpdatePropertiesAction.java | 71 +++++++- .../resources/OSGI-INF/blueprint/blueprint.xml | 1 + .../actions/MergeProfilesOnPropertyActionTest.java | 136 +++++++++++++++ .../actions/UpdatePropertiesActionTest.java | 193 +++++++++++++++++++++ 5 files changed, 427 insertions(+), 6 deletions(-) diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/MergeProfilesOnPropertyAction.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/MergeProfilesOnPropertyAction.java index a29b39f30..ff4832cc3 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/MergeProfilesOnPropertyAction.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/MergeProfilesOnPropertyAction.java @@ -26,6 +26,7 @@ import org.apache.unomi.api.actions.Action; import org.apache.unomi.api.actions.ActionExecutor; import org.apache.unomi.api.conditions.Condition; import org.apache.unomi.api.security.SecurityService; +import org.apache.unomi.api.security.UnomiRoles; import org.apache.unomi.api.services.*; import org.apache.unomi.persistence.spi.PersistenceService; import org.slf4j.Logger; @@ -91,6 +92,13 @@ public class MergeProfilesOnPropertyAction implements ActionExecutor { // Check if the user switched to another profile if (StringUtils.isNotEmpty(currentProfileMergeValue) && !currentProfileMergeValue.equals(mergePropValue)) { + if (!isTrustedIdentityCaller()) { + LOGGER.warn("Refusing profile merge switch for untrusted caller (mergeProp={})", mergePropName); + if (tracer != null) { + tracer.endOperation(false, "Untrusted caller cannot switch merge identity"); + } + return EventService.NO_CHANGE; + } if (tracer != null) { tracer.trace("Profile switch detected", Map.of( "fromValue", currentProfileMergeValue, @@ -119,6 +127,18 @@ public class MergeProfilesOnPropertyAction implements ActionExecutor { return profileUpdated ? EventService.PROFILE_UPDATED : EventService.NO_CHANGE; } + // Merging into another existing profile rebinds the session — require a caller holding + // system access (see isTrustedIdentityCaller), not a public/unauthenticated event. + if (!isTrustedIdentityCaller()) { + LOGGER.warn("Refusing profile merge for untrusted caller (mergeProp={}, candidates={})", + mergePropName, profilesToBeMerge.size()); + if (tracer != null) { + tracer.endOperation(false, "Untrusted caller cannot merge into another profile"); + } + // Keep only the merge identifier write on the current profile when it was empty + return profileUpdated ? EventService.PROFILE_UPDATED : EventService.NO_CHANGE; + } + // add current Profile to profiles to be merged if (profilesToBeMerge.stream().noneMatch(p -> StringUtils.equals(p.getItemId(), eventProfile.getItemId()))) { profilesToBeMerge.add(eventProfile); @@ -327,6 +347,18 @@ public class MergeProfilesOnPropertyAction implements ActionExecutor { } } + /** + * Whether the caller holds system access, i.e. the administrator or tenant administrator role. + * <p> + * This is a role check, not a check of the credential that produced it: a tenant private key + * authenticates as {@link UnomiRoles#TENANT_ADMINISTRATOR} and therefore passes, while a tenant + * public API key or an unauthenticated context event does not. Identity merges rebind sessions, + * so they are restricted to callers that hold that role. + */ + private boolean isTrustedIdentityCaller() { + return securityService != null && securityService.hasSystemAccess(); + } + public void setProfileService(ProfileService profileService) { this.profileService = profileService; } diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/UpdatePropertiesAction.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/UpdatePropertiesAction.java index af9a2a091..78070d144 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/UpdatePropertiesAction.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/UpdatePropertiesAction.java @@ -24,6 +24,9 @@ import org.apache.unomi.api.Profile; import org.apache.unomi.api.PropertyType; import org.apache.unomi.api.actions.Action; import org.apache.unomi.api.actions.ActionExecutor; +import org.apache.unomi.api.security.SecurityService; +import org.apache.unomi.api.utils.LogSanitizer; +import org.apache.unomi.api.security.UnomiRoles; import org.apache.unomi.api.services.EventService; import org.apache.unomi.api.services.ProfileService; import org.apache.unomi.persistence.spi.PropertyHelper; @@ -46,11 +49,17 @@ public class UpdatePropertiesAction implements ActionExecutor { public static final String TARGET_TYPE_PROFILE = "profile"; + /** The reserved profile field that only a caller with system access may write. */ + private static final String SYSTEM_PROPERTIES_KEY = "systemProperties"; + /** Prefix of the profile properties that only a caller with system access may write. */ + private static final String SYSTEM_PROPERTIES_PREFIX = SYSTEM_PROPERTIES_KEY + "."; + private static final Logger LOGGER = LoggerFactory.getLogger(UpdatePropertiesAction.class.getName()); private ProfileService profileService; private EventService eventService; private TracerService tracerService; + private SecurityService securityService; public int execute(Action action, Event event) { RequestTracer tracer = null; @@ -64,6 +73,8 @@ public class UpdatePropertiesAction implements ActionExecutor { Profile target = event.getProfile(); String targetId = (String) event.getProperty(TARGET_ID_KEY); String targetType = (String) event.getProperty(TARGET_TYPE_KEY); + // Resolved once for the whole action: the caller's roles cannot change while it runs. + final boolean trustedCaller = isTrustedIdentityCaller(); if (tracer != null) { Map<String, Object> traceData = new HashMap<>(); @@ -74,12 +85,20 @@ public class UpdatePropertiesAction implements ActionExecutor { } if (StringUtils.isNotBlank(targetId) && event.getProfile() != null && !targetId.equals(event.getProfile().getItemId())) { + if (!trustedCaller) { + LOGGER.warn("Refusing cross-profile property update for untrusted caller (targetId={})", + LogSanitizer.forLogging(targetId)); + if (tracer != null) { + tracer.endOperation(false, "Untrusted caller cannot update another profile"); + } + return EventService.NO_CHANGE; + } target = TARGET_TYPE_PROFILE.equals(targetType) ? profileService.load(targetId) : profileService.loadPersona(targetId); if (target == null) { if (tracer != null) { tracer.endOperation(false, "No profile found with Id: " + targetId); } - LOGGER.warn("No profile found with Id : {}. Update skipped.", targetId); + LOGGER.warn("No profile found with Id : {}. Update skipped.", LogSanitizer.forLogging(targetId)); return EventService.NO_CHANGE; } } @@ -89,22 +108,26 @@ public class UpdatePropertiesAction implements ActionExecutor { Map<String, Object> propsToAdd = (HashMap<String, Object>) event.getProperties().get(PROPS_TO_ADD); if (propsToAdd != null) { - isProfileOrPersonaUpdated |= processProperties(target, propsToAdd, "setIfMissing"); + isProfileOrPersonaUpdated |= processProperties(target, propsToAdd, "setIfMissing", trustedCaller); } Map<String, Object> propsToUpdate = (HashMap<String, Object>) event.getProperties().get(PROPS_TO_UPDATE); if (propsToUpdate != null) { - isProfileOrPersonaUpdated |= processProperties(target, propsToUpdate, "alwaysSet"); + isProfileOrPersonaUpdated |= processProperties(target, propsToUpdate, "alwaysSet", trustedCaller); } Map<String, Object> propsToAddToSet = (HashMap<String, Object>) event.getProperties().get(PROPS_TO_ADD_TO_SET); if (propsToAddToSet != null) { - isProfileOrPersonaUpdated |= processProperties(target, propsToAddToSet, "addValues"); + isProfileOrPersonaUpdated |= processProperties(target, propsToAddToSet, "addValues", trustedCaller); } List<String> propsToDelete = (List<String>) event.getProperties().get(PROPS_TO_DELETE); if (propsToDelete != null) { for (String prop : propsToDelete) { + if (!trustedCaller && isSystemPropertiesWrite(prop)) { + LOGGER.warn("Refusing systemProperties delete for untrusted caller: {}", LogSanitizer.forLogging(prop)); + continue; + } isProfileOrPersonaUpdated |= PropertyHelper.setProperty(target, prop, null, "remove"); } } @@ -140,11 +163,15 @@ public class UpdatePropertiesAction implements ActionExecutor { } } - private boolean processProperties(Profile target, Map<String, Object> propsMap, String strategy) { + private boolean processProperties(Profile target, Map<String, Object> propsMap, String strategy, boolean trustedCaller) { boolean isProfileOrPersonaUpdated = false; for (String prop : propsMap.keySet()) { + if (!trustedCaller && isSystemPropertiesWrite(prop)) { + LOGGER.warn("Refusing systemProperties update for untrusted caller: {}", LogSanitizer.forLogging(prop)); + continue; + } PropertyType propType = null; - if (prop.startsWith("properties.") || prop.startsWith("systemProperties.")) { + if (prop.startsWith("properties.") || prop.startsWith(SYSTEM_PROPERTIES_PREFIX)) { propType = profileService.getPropertyType(prop.substring(prop.indexOf('.') + 1)); } else { propType = profileService.getPropertyType(prop); @@ -162,6 +189,34 @@ public class UpdatePropertiesAction implements ActionExecutor { return isProfileOrPersonaUpdated; } + + /** + * Whether a property name writes the reserved {@code systemProperties} area. + * <p> + * Matching the {@code systemProperties.} prefix alone was not enough: the bare key + * {@code systemProperties} has no dot, so it slipped through, and for a flat name + * {@link org.apache.unomi.persistence.spi.PropertyHelper#setProperty} falls through to + * {@code BeanUtils.setProperty}, which invokes {@code Profile#setSystemProperties(Map)} and + * replaces the entire map. That is strictly more than the per-key write this gate blocks — it + * is enough to plant {@code mergeIdentifier} and drive the profile-merge action. + * + * @param propertyName the event-supplied property name + * @return true when the name targets systemProperties, whether wholesale or a single entry + */ + private static boolean isSystemPropertiesWrite(String propertyName) { + return SYSTEM_PROPERTIES_KEY.equals(propertyName) || propertyName.startsWith(SYSTEM_PROPERTIES_PREFIX); + } + /** + * Whether the caller holds system access, i.e. the administrator or tenant administrator role. + * <p> + * Cross-profile updates and {@code systemProperties.*} writes are restricted to such callers. + * A tenant private key authenticates as {@link UnomiRoles#TENANT_ADMINISTRATOR} and passes; a + * tenant public API key or an unauthenticated context event does not. + */ + private boolean isTrustedIdentityCaller() { + return securityService != null && securityService.hasSystemAccess(); + } + public void setProfileService(ProfileService profileService) { this.profileService = profileService; } @@ -174,4 +229,8 @@ public class UpdatePropertiesAction implements ActionExecutor { this.tracerService = tracerService; } + public void setSecurityService(SecurityService securityService) { + this.securityService = securityService; + } + } diff --git a/plugins/baseplugin/src/main/resources/OSGI-INF/blueprint/blueprint.xml b/plugins/baseplugin/src/main/resources/OSGI-INF/blueprint/blueprint.xml index 2b10af39a..c317dc34d 100644 --- a/plugins/baseplugin/src/main/resources/OSGI-INF/blueprint/blueprint.xml +++ b/plugins/baseplugin/src/main/resources/OSGI-INF/blueprint/blueprint.xml @@ -181,6 +181,7 @@ <property name="profileService" ref="profileService"/> <property name="eventService" ref="eventService"/> <property name="tracerService" ref="tracerService"/> + <property name="securityService" ref="securityService"/> </bean> </service> diff --git a/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/actions/MergeProfilesOnPropertyActionTest.java b/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/actions/MergeProfilesOnPropertyActionTest.java new file mode 100644 index 000000000..9d67ae495 --- /dev/null +++ b/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/actions/MergeProfilesOnPropertyActionTest.java @@ -0,0 +1,136 @@ +/* + * 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.plugins.baseplugin.actions; + +import org.apache.unomi.api.Event; +import org.apache.unomi.api.PartialList; +import org.apache.unomi.api.Profile; +import org.apache.unomi.api.actions.Action; +import org.apache.unomi.api.conditions.ConditionType; +import org.apache.unomi.api.security.SecurityService; +import org.apache.unomi.api.security.UnomiRoles; +import org.apache.unomi.api.services.DefinitionsService; +import org.apache.unomi.api.services.EventService; +import org.apache.unomi.api.services.PrivacyService; +import org.apache.unomi.api.services.ProfileService; +import org.apache.unomi.persistence.spi.PersistenceService; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Date; +import java.util.HashMap; +import java.util.Map; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Regression: public/untrusted events must not rebind a session onto another profile via merge. + */ +@RunWith(MockitoJUnitRunner.class) +public class MergeProfilesOnPropertyActionTest { + + @Mock private ProfileService profileService; + @Mock private PersistenceService persistenceService; + @Mock private EventService eventService; + @Mock private DefinitionsService definitionsService; + @Mock private PrivacyService privacyService; + @Mock private SecurityService securityService; + + private MergeProfilesOnPropertyAction actionExecutor; + + @Before + public void setUp() { + actionExecutor = new MergeProfilesOnPropertyAction(); + actionExecutor.setProfileService(profileService); + actionExecutor.setPersistenceService(persistenceService); + actionExecutor.setEventService(eventService); + actionExecutor.setDefinitionsService(definitionsService); + actionExecutor.setPrivacyService(privacyService); + actionExecutor.bindSecurityService(securityService); + actionExecutor.setMaxProfilesInOneMerge("50"); + + when(definitionsService.getConditionType("profilePropertyCondition")).thenReturn(new ConditionType()); + when(securityService.hasSystemAccess()).thenReturn(false); + } + + @Test + public void untrustedCaller_cannotMergeIntoExistingVictimProfile() { + Profile attacker = new Profile("attacker"); + Profile victim = new Profile("victim"); + victim.getSystemProperties().put("mergeIdentifier", "[email protected]"); + + when(persistenceService.query(any(), anyString(), eq(Profile.class), anyInt(), anyInt())) + .thenReturn(new PartialList<>(new ArrayList<>(Collections.singletonList(victim)), 0, 1, 1, PartialList.Relation.EQUAL)); + + Event event = new Event("login", null, attacker, "systemscope", null, null, null, new Date(), true); + Action action = mergeAction("[email protected]"); + + int changes = actionExecutor.execute(action, event); + + // May write mergeIdentifier onto the attacker profile, but must not rebind to victim + assertNotEquals(EventService.PROFILE_UPDATED + EventService.SESSION_UPDATED, changes); + assertEquals("attacker", event.getProfile().getItemId()); + verify(profileService, never()).mergeProfiles(any(), any()); + } + + @Test + public void trustedTenantAdmin_canMergeIntoExistingProfile() { + when(securityService.hasSystemAccess()).thenReturn(true); + + Profile caller = new Profile("caller"); + Profile victim = new Profile("victim"); + victim.setProperty("firstVisit", new Date(0)); + caller.setProperty("firstVisit", new Date()); + + when(persistenceService.query(any(), anyString(), eq(Profile.class), anyInt(), anyInt())) + .thenReturn(new PartialList<>(new ArrayList<>(Collections.singletonList(victim)), 0, 1, 1, PartialList.Relation.EQUAL)); + when(profileService.mergeProfiles(eq(victim), any())).thenReturn(victim); + when(privacyService.isRequireAnonymousBrowsing(any(Profile.class))).thenReturn(false); + when(privacyService.isRequireAnonymousBrowsing("victim")).thenReturn(false); + + Event event = new Event("login", null, caller, "systemscope", null, null, null, new Date(), true); + Action action = mergeAction("[email protected]"); + + int changes = actionExecutor.execute(action, event); + + assertEquals(EventService.PROFILE_UPDATED + EventService.SESSION_UPDATED, changes); + assertEquals("victim", event.getProfile().getItemId()); + verify(profileService).mergeProfiles(eq(victim), any()); + } + + private static Action mergeAction(String mergeValue) { + Action action = new Action(); + Map<String, Object> params = new HashMap<>(); + params.put("mergeProfilePropertyName", "mergeIdentifier"); + params.put("mergeProfilePropertyValue", mergeValue); + action.setParameterValues(params); + return action; + } +} diff --git a/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/actions/UpdatePropertiesActionTest.java b/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/actions/UpdatePropertiesActionTest.java new file mode 100644 index 000000000..5f77b847e --- /dev/null +++ b/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/actions/UpdatePropertiesActionTest.java @@ -0,0 +1,193 @@ +/* + * 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.plugins.baseplugin.actions; + +import org.apache.unomi.api.Event; +import org.apache.unomi.api.Profile; +import org.apache.unomi.api.actions.Action; +import org.apache.unomi.api.security.SecurityService; +import org.apache.unomi.api.security.UnomiRoles; +import org.apache.unomi.api.services.EventService; +import org.apache.unomi.api.services.ProfileService; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import java.util.Date; +import java.util.HashMap; +import java.util.Map; + +import static org.junit.Assert.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Regression: untrusted events must not update another profile or systemProperties. + */ +@RunWith(MockitoJUnitRunner.class) +public class UpdatePropertiesActionTest { + + @Mock private ProfileService profileService; + @Mock private EventService eventService; + @Mock private SecurityService securityService; + + private UpdatePropertiesAction actionExecutor; + + @Before + public void setUp() { + actionExecutor = new UpdatePropertiesAction(); + actionExecutor.setProfileService(profileService); + actionExecutor.setEventService(eventService); + actionExecutor.setSecurityService(securityService); + + when(securityService.hasSystemAccess()).thenReturn(false); + } + + @Test + public void untrustedCaller_cannotUpdateAnotherProfile() { + Profile caller = new Profile("caller"); + Map<String, Object> updateMap = new HashMap<>(); + updateMap.put("properties.email", "pwned"); + Map<String, Object> eventProps = new HashMap<>(); + eventProps.put(UpdatePropertiesAction.TARGET_ID_KEY, "victim"); + eventProps.put(UpdatePropertiesAction.TARGET_TYPE_KEY, UpdatePropertiesAction.TARGET_TYPE_PROFILE); + eventProps.put(UpdatePropertiesAction.PROPS_TO_UPDATE, updateMap); + + Event event = new Event("updateProperties", null, caller, "systemscope", null, null, eventProps, new Date(), true); + Action action = new Action(); + + int changes = actionExecutor.execute(action, event); + + assertEquals(EventService.NO_CHANGE, changes); + verify(profileService, never()).load(any(String.class)); + verify(profileService, never()).save(any(Profile.class)); + } + + @Test + public void untrustedCaller_cannotWriteSystemProperties() { + Profile caller = new Profile("caller"); + Map<String, Object> updateMap = new HashMap<>(); + updateMap.put("systemProperties.mergeIdentifier", "stolen"); + Map<String, Object> eventProps = new HashMap<>(); + eventProps.put(UpdatePropertiesAction.PROPS_TO_UPDATE, updateMap); + + Event event = new Event("updateProperties", null, caller, "systemscope", null, null, eventProps, new Date(), true); + + int changes = actionExecutor.execute(new Action(), event); + + assertEquals(EventService.NO_CHANGE, changes); + assertEquals(null, caller.getSystemProperties().get("mergeIdentifier")); + } + + /** + * The gate matched on the {@code systemProperties.} prefix only, so the exact key + * {@code systemProperties} slipped past it. That is not a harmless near-miss: for a flat name + * PropertyHelper falls through to {@code BeanUtils.setProperty}, which calls + * {@code Profile#setSystemProperties(Map)} and replaces the whole map — a superset of the + * per-key write the gate exists to block, and enough to plant {@code mergeIdentifier} and drive + * the profile-merge action. + */ + @Test + public void untrustedCaller_cannotReplaceTheWholeSystemPropertiesMap() { + Profile caller = new Profile("caller"); + caller.getSystemProperties().put("mergeIdentifier", "[email protected]"); + + Map<String, Object> replacement = new HashMap<>(); + replacement.put("mergeIdentifier", "[email protected]"); + Map<String, Object> updateMap = new HashMap<>(); + updateMap.put("systemProperties", replacement); + Map<String, Object> eventProps = new HashMap<>(); + eventProps.put(UpdatePropertiesAction.PROPS_TO_UPDATE, updateMap); + + Event event = new Event("updateProperties", null, caller, "systemscope", null, null, eventProps, new Date(), true); + + int changes = actionExecutor.execute(new Action(), event); + + assertEquals(EventService.NO_CHANGE, changes); + assertEquals("the untrusted caller must not replace the systemProperties map", + "[email protected]", caller.getSystemProperties().get("mergeIdentifier")); + } + + /** + * The delete mapping is not exploitable the same way - PropertyHelper's remove strategy bails out + * for a name with no dot - so this passes even against the unfixed gate. Kept as a guard: if that + * remove path ever learns to handle flat names, this catches it rather than the next reporter. + */ + @Test + public void untrustedCaller_cannotDeleteTheWholeSystemPropertiesMap() { + Profile caller = new Profile("caller"); + caller.getSystemProperties().put("mergeIdentifier", "[email protected]"); + + Map<String, Object> eventProps = new HashMap<>(); + eventProps.put(UpdatePropertiesAction.PROPS_TO_DELETE, + java.util.Collections.singletonList("systemProperties")); + + Event event = new Event("updateProperties", null, caller, "systemscope", null, null, eventProps, new Date(), true); + + actionExecutor.execute(new Action(), event); + + assertEquals("the untrusted caller must not clear the systemProperties map", + "[email protected]", caller.getSystemProperties().get("mergeIdentifier")); + } + + /** A trusted caller is still allowed to set it, so the gate is a trust check and not a ban. */ + @Test + public void trustedAdmin_mayReplaceTheSystemPropertiesMap() { + when(securityService.hasSystemAccess()).thenReturn(true); + + Profile caller = new Profile("caller"); + Map<String, Object> replacement = new HashMap<>(); + replacement.put("mergeIdentifier", "[email protected]"); + Map<String, Object> updateMap = new HashMap<>(); + updateMap.put("systemProperties", replacement); + Map<String, Object> eventProps = new HashMap<>(); + eventProps.put(UpdatePropertiesAction.PROPS_TO_UPDATE, updateMap); + + Event event = new Event("updateProperties", null, caller, "systemscope", null, null, eventProps, new Date(), true); + + actionExecutor.execute(new Action(), event); + + assertEquals("[email protected]", caller.getSystemProperties().get("mergeIdentifier")); + } + + @Test + public void trustedAdmin_canUpdateAnotherProfile() { + when(securityService.hasSystemAccess()).thenReturn(true); + + Profile caller = new Profile("caller"); + Profile victim = new Profile("victim"); + when(profileService.load("victim")).thenReturn(victim); + + Map<String, Object> updateMap = new HashMap<>(); + updateMap.put("properties.email", "admin-set"); + Map<String, Object> eventProps = new HashMap<>(); + eventProps.put(UpdatePropertiesAction.TARGET_ID_KEY, "victim"); + eventProps.put(UpdatePropertiesAction.TARGET_TYPE_KEY, UpdatePropertiesAction.TARGET_TYPE_PROFILE); + eventProps.put(UpdatePropertiesAction.PROPS_TO_UPDATE, updateMap); + + Event event = new Event("updateProperties", null, caller, "systemscope", null, null, eventProps, new Date(), true); + + actionExecutor.execute(new Action(), event); + + verify(profileService).load("victim"); + verify(profileService).save(victim); + } +}
