From e31948d6256878738f6bffc053ff2865658adc58 Mon Sep 17 00:00:00 2001 From: Samuel Garofalo <72073457+SamuelGaro@users.noreply.github.com> Date: Wed, 6 Nov 2024 14:20:00 +0100 Subject: [PATCH] [SYNCOPE-1837] Prevent unwanted resets on SCIM PUT (#895) * [SYNCOPE-1837] Prevent unwanted resets on SCIM PUT --- .../cxf/service/SCIMGroupServiceImpl.java | 7 +++-- .../cxf/service/SCIMUserServiceImpl.java | 9 ++++-- .../apache/syncope/fit/core/SCIMITCase.java | 28 +++++++++++++++++++ 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/ext/scimv2/scim-rest-cxf/src/main/java/org/apache/syncope/ext/scimv2/cxf/service/SCIMGroupServiceImpl.java b/ext/scimv2/scim-rest-cxf/src/main/java/org/apache/syncope/ext/scimv2/cxf/service/SCIMGroupServiceImpl.java index 76dd496d64..d6818b8786 100644 --- a/ext/scimv2/scim-rest-cxf/src/main/java/org/apache/syncope/ext/scimv2/cxf/service/SCIMGroupServiceImpl.java +++ b/ext/scimv2/scim-rest-cxf/src/main/java/org/apache/syncope/ext/scimv2/cxf/service/SCIMGroupServiceImpl.java @@ -31,6 +31,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.syncope.common.lib.AnyOperations; import org.apache.syncope.common.lib.SyncopeConstants; +import org.apache.syncope.common.lib.request.GroupUR; import org.apache.syncope.common.lib.request.MembershipUR; import org.apache.syncope.common.lib.request.UserUR; import org.apache.syncope.common.lib.to.GroupTO; @@ -187,8 +188,10 @@ public Response replace(final String id, final SCIMGroup group) { Set beforeMembers = members(id); // update group, don't change members - ProvisioningResult result = groupLogic.update( - AnyOperations.diff(binder.toGroupTO(group, true), groupLogic.read(id), false), false); + GroupUR req = AnyOperations.diff(binder.toGroupTO(group, true), groupLogic.read(id), false); + req.getResources().clear(); + req.getAuxClasses().clear(); + ProvisioningResult result = groupLogic.update(req, false); // assign new members Set afterMembers = new HashSet<>(); diff --git a/ext/scimv2/scim-rest-cxf/src/main/java/org/apache/syncope/ext/scimv2/cxf/service/SCIMUserServiceImpl.java b/ext/scimv2/scim-rest-cxf/src/main/java/org/apache/syncope/ext/scimv2/cxf/service/SCIMUserServiceImpl.java index 7d35a1543e..69d51b4164 100644 --- a/ext/scimv2/scim-rest-cxf/src/main/java/org/apache/syncope/ext/scimv2/cxf/service/SCIMUserServiceImpl.java +++ b/ext/scimv2/scim-rest-cxf/src/main/java/org/apache/syncope/ext/scimv2/cxf/service/SCIMUserServiceImpl.java @@ -122,8 +122,13 @@ public Response replace(final String id, final SCIMUser user) { UserTO before = userLogic.read(id); - ProvisioningResult result = userLogic.update( - AnyOperations.diff(binder.toUserTO(user, true), before, false), false); + UserUR req = AnyOperations.diff(binder.toUserTO(user, true), before, false); + req.getResources().clear(); + req.getAuxClasses().clear(); + req.getRelationships().clear(); + req.getRoles().clear(); + req.getLinkedAccounts().clear(); + ProvisioningResult result = userLogic.update(req, false); if (before.isSuspended() == user.isActive()) { StatusR statusR = new StatusR.Builder( diff --git a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/SCIMITCase.java b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/SCIMITCase.java index b35f2f18b9..cd4e5a6600 100644 --- a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/SCIMITCase.java +++ b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/SCIMITCase.java @@ -45,6 +45,9 @@ import javax.ws.rs.core.Response; import org.apache.commons.lang3.StringUtils; import org.apache.cxf.jaxrs.client.WebClient; +import org.apache.syncope.common.lib.request.GroupUR; +import org.apache.syncope.common.lib.request.StringPatchItem; +import org.apache.syncope.common.lib.request.UserUR; import org.apache.syncope.common.lib.scim.SCIMComplexConf; import org.apache.syncope.common.lib.scim.SCIMConf; import org.apache.syncope.common.lib.scim.SCIMEnterpriseUserConf; @@ -54,8 +57,10 @@ import org.apache.syncope.common.lib.scim.SCIMUserConf; import org.apache.syncope.common.lib.scim.SCIMUserNameConf; import org.apache.syncope.common.lib.scim.types.EmailCanonicalType; +import org.apache.syncope.common.lib.to.GroupTO; import org.apache.syncope.common.lib.to.ProvisioningResult; import org.apache.syncope.common.lib.to.UserTO; +import org.apache.syncope.common.lib.types.PatchOperation; import org.apache.syncope.ext.scimv2.api.SCIMConstants; import org.apache.syncope.ext.scimv2.api.data.Group; import org.apache.syncope.ext.scimv2.api.data.ListResponse; @@ -704,6 +709,12 @@ public void replaceUser() { user = response.readEntity(SCIMUser.class); assertNotNull(user.getId()); + UserTO userTO = USER_SERVICE.read(user.getId()); + assertNotNull(userTO); + USER_SERVICE.update(new UserUR.Builder(userTO.getKey()).resource( + new StringPatchItem.Builder().value(RESOURCE_NAME_LDAP).operation(PatchOperation.ADD_REPLACE).build()) + .build()); + user.getName().setFormatted("new" + user.getUserName()); response = webClient().path("Users").path(user.getId()).put(user); @@ -711,6 +722,10 @@ public void replaceUser() { user = response.readEntity(SCIMUser.class); assertTrue(user.getName().getFormatted().startsWith("new")); + + userTO = USER_SERVICE.read(user.getId()); + assertNotNull(userTO); + assertTrue(userTO.getResources().contains(RESOURCE_NAME_LDAP)); } @Test @@ -860,6 +875,15 @@ public void replaceGroup() { assertEquals(1, group.getMembers().size()); assertEquals("b3cbc78d-32e6-4bd4-92e0-bbe07566a2ee", group.getMembers().get(0).getValue()); + GroupTO groupTO = GROUP_SERVICE.read(group.getId()); + assertNotNull(groupTO); + GROUP_SERVICE.update(new GroupUR.Builder(groupTO.getKey()).resource( + new StringPatchItem.Builder().value(RESOURCE_NAME_LDAP).operation(PatchOperation.ADD_REPLACE).build()) + .build()); + groupTO = GROUP_SERVICE.read(group.getId()); + assertNotNull(groupTO); + assertTrue(groupTO.getResources().contains(RESOURCE_NAME_LDAP)); + group.setDisplayName("other" + group.getId()); group.getMembers().add(new Member("c9b2dec2-00a7-4855-97c0-d854842b4b24", null, null)); @@ -870,6 +894,10 @@ public void replaceGroup() { assertTrue(group.getDisplayName().startsWith("other")); assertEquals(2, group.getMembers().size()); + groupTO = GROUP_SERVICE.read(group.getId()); + assertNotNull(groupTO); + assertTrue(groupTO.getResources().contains(RESOURCE_NAME_LDAP)); + group.getMembers().clear(); group.getMembers().add(new Member("c9b2dec2-00a7-4855-97c0-d854842b4b24", null, null));