Skip to content

Commit

Permalink
[SYNCOPE-1837] avoid that Resources, Relationships, Roles, Linked acc…
Browse files Browse the repository at this point in the history 
…ount and AuxClasses are deleted after SCIM PUT method invocation
  • Loading branch information
@SamuelGaro
SamuelGaro committed Nov 6, 2024
1 parent 24f61e0 commit 57977e4
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 15 deletions.
34 changes: 21 additions & 13 deletions common/idrepo/lib/src/main/java/org/apache/syncope/common/lib/AnyOperations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,11 @@ private static <T, K extends AbstractReplacePatchItem<T>> K replacePatchItem(
}

private static void diff(
final AnyTO updated, final AnyTO original, final AnyUR result, final boolean incremental) {
final AnyTO updated,
final AnyTO original,
final AnyUR result,
final boolean incremental,
final boolean preserve) {

// check same key
if (updated.getKey() == null && original.getKey() != null
Expand All @@ -93,7 +97,7 @@ private static void diff(
// 2. auxilairy classes
result.getAuxClasses().clear();

if (!incremental) {
if (!incremental && !preserve) {
original.getAuxClasses().stream().filter(auxClass -> !updated.getAuxClasses().contains(auxClass)).
forEach(auxClass -> result.getAuxClasses().add(new StringPatchItem.Builder().
operation(PatchOperation.DELETE).value(auxClass).build()));
Expand Down Expand Up @@ -143,7 +147,7 @@ private static void diff(
// 5. resources
result.getResources().clear();

if (!incremental) {
if (!incremental && !preserve) {
original.getResources().stream().filter(resource -> !updated.getResources().contains(resource)).
forEach(resource -> result.getResources().add(new StringPatchItem.Builder().
operation(PatchOperation.DELETE).value(resource).build()));
Expand All @@ -167,7 +171,7 @@ public static AnyObjectUR diff(

AnyObjectUR result = new AnyObjectUR();

diff(updated, original, result, incremental);
diff(updated, original, result, incremental, incremental);

// 1. name
result.setName(replacePatchItem(updated.getName(), original.getName(), new StringReplacePatchItem()));
Expand Down Expand Up @@ -231,12 +235,14 @@ private static void diff(
* @param updated updated UserTO
* @param original original UserTO
* @param incremental perform incremental diff (without removing existing info)
* @param preserve doesn't remove existing info for auxClasses, resources, roles, relationships and linked accounts
* @return {@link UserUR} containing differences
*/
public static UserUR diff(final UserTO updated, final UserTO original, final boolean incremental) {
public static UserUR diff(
final UserTO updated, final UserTO original, final boolean incremental, final boolean preserve) {
UserUR result = new UserUR();

diff(updated, original, result, incremental);
diff(updated, original, result, incremental, preserve);

// 1. password
if (updated.getPassword() != null
Expand Down Expand Up @@ -268,7 +274,7 @@ public static UserUR diff(final UserTO updated, final UserTO original, final boo
updated.isMustChangePassword(), original.isMustChangePassword(), new BooleanReplacePatchItem()));

// 4. roles
if (!incremental) {
if (!incremental && !preserve) {
original.getRoles().stream().filter(role -> !updated.getRoles().contains(role)).
forEach(toRemove -> result.getRoles().add(new StringPatchItem.Builder().
operation(PatchOperation.DELETE).value(toRemove).build()));
Expand All @@ -289,7 +295,7 @@ public static UserUR diff(final UserTO updated, final UserTO original, final boo
forEach(entry -> result.getRelationships().add(new RelationshipUR.Builder(entry.getValue()).
operation(PatchOperation.ADD_REPLACE).build()));

if (!incremental) {
if (!incremental && !preserve) {
originalRels.keySet().stream().filter(relationship -> !updatedRels.containsKey(relationship)).
forEach(key -> result.getRelationships().add(new RelationshipUR.Builder(originalRels.get(key)).
operation(PatchOperation.DELETE).build()));
Expand Down Expand Up @@ -327,7 +333,7 @@ public static UserUR diff(final UserTO updated, final UserTO original, final boo
linkedAccountTO(entry.getValue()).build());
});

if (!incremental) {
if (!incremental && !preserve) {
originalAccounts.keySet().stream().filter(account -> !updatedAccounts.containsKey(account)).
forEach(key -> {
result.getLinkedAccounts().add(new LinkedAccountUR.Builder().
Expand All @@ -345,12 +351,14 @@ public static UserUR diff(final UserTO updated, final UserTO original, final boo
* @param updated updated GroupTO
* @param original original GroupTO
* @param incremental perform incremental diff (without removing existing info)
* @param preserve doesn't remove existing info for auxClasses and resources
* @return {@link GroupUR} containing differences
*/
public static GroupUR diff(final GroupTO updated, final GroupTO original, final boolean incremental) {
public static GroupUR diff(
final GroupTO updated, final GroupTO original, final boolean incremental, final boolean preserve) {
GroupUR result = new GroupUR();

diff(updated, original, result, incremental);
diff(updated, original, result, incremental, preserve);

// 1. name
result.setName(replacePatchItem(updated.getName(), original.getName(), new StringReplacePatchItem()));
Expand All @@ -376,9 +384,9 @@ public static <TO extends AnyTO, P extends AnyUR> P diff(
final TO updated, final TO original, final boolean incremental) {

if (updated instanceof UserTO && original instanceof UserTO) {
return (P) diff((UserTO) updated, (UserTO) original, incremental);
return (P) diff((UserTO) updated, (UserTO) original, incremental, incremental);
} else if (updated instanceof GroupTO && original instanceof GroupTO) {
return (P) diff((GroupTO) updated, (GroupTO) original, incremental);
return (P) diff((GroupTO) updated, (GroupTO) original, incremental, incremental);
} else if (updated instanceof AnyObjectTO && original instanceof AnyObjectTO) {
return (P) diff((AnyObjectTO) updated, (AnyObjectTO) original, incremental);
}
Expand Down
2 changes: 1 addition & 1 deletion ...est-cxf/src/main/java/org/apache/syncope/ext/scimv2/cxf/service/SCIMGroupServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ public Response replace(final String id, final SCIMGroup group) {

// update group, don't change members
ProvisioningResult<GroupTO> result = groupLogic.update(
AnyOperations.diff(binder.toGroupTO(group, true), groupLogic.read(id), false), false);
AnyOperations.diff(binder.toGroupTO(group, true), groupLogic.read(id), false, true), false);

// assign new members
Set<String> afterMembers = new HashSet<>();
Expand Down
2 changes: 1 addition & 1 deletion ...rest-cxf/src/main/java/org/apache/syncope/ext/scimv2/cxf/service/SCIMUserServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ 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);
AnyOperations.diff(binder.toUserTO(user, true), before, false, true), false);

if (before.isSuspended() == user.isActive()) {
StatusR statusR = new StatusR.Builder(
Expand Down
28 changes: 28 additions & 0 deletions fit/core-reference/src/test/java/org/apache/syncope/fit/core/SCIMITCase.java
View file
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 57977e4

Please sign in to comment.