Skip to content

Commit

Permalink
[SYNCOPE-1837] Prevent unwanted resets on SCIM PUT (#895)
Browse files Browse the repository at this point in the history
* [SYNCOPE-1837] Prevent unwanted resets on SCIM PUT
  • Loading branch information
SamuelGaro authored Nov 6, 2024
1 parent 24f61e0 commit e31948d
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -187,8 +188,10 @@ public Response replace(final String id, final SCIMGroup group) {
Set<String> beforeMembers = members(id);

// update group, don't change members
ProvisioningResult<GroupTO> 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<GroupTO> result = groupLogic.update(req, false);

// assign new members
Set<String> afterMembers = new HashSet<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,13 @@ public Response replace(final String id, final SCIMUser user) {

UserTO before = userLogic.read(id);

ProvisioningResult<UserTO> 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<UserTO> result = userLogic.update(req, false);

if (before.isSuspended() == user.isActive()) {
StatusR statusR = new StatusR.Builder(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -704,13 +709,23 @@ 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);
assertEquals(Response.Status.OK.getStatusCode(), response.getStatus());

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
Expand Down Expand Up @@ -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));

Expand All @@ -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));

Expand Down

0 comments on commit e31948d

Please sign in to comment.