From bbf083f3ad18bf67ab458bb12aadf57f4cfdfd64 Mon Sep 17 00:00:00 2001 From: Andrea Patricelli Date: Mon, 20 Jan 2025 15:42:43 +0100 Subject: [PATCH 1/3] [SYNCOPE-1853] Avoid unwanted user/any object propagation on group delete --- .../java/data/GroupDataBinderImpl.java | 14 +++-- .../syncope/fit/core/UserIssuesITCase.java | 57 +++++++++++++++++++ 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java index ef72c7baaf8..55ac59716dc 100644 --- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java +++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java @@ -48,13 +48,13 @@ import org.apache.syncope.core.persistence.api.dao.RelationshipTypeDAO; import org.apache.syncope.core.persistence.api.dao.UserDAO; import org.apache.syncope.core.persistence.api.dao.search.SearchCond; -import org.apache.syncope.core.persistence.api.entity.Any; import org.apache.syncope.core.persistence.api.entity.AnyType; import org.apache.syncope.core.persistence.api.entity.AnyTypeClass; import org.apache.syncope.core.persistence.api.entity.AnyUtilsFactory; import org.apache.syncope.core.persistence.api.entity.DerSchema; import org.apache.syncope.core.persistence.api.entity.DynGroupMembership; import org.apache.syncope.core.persistence.api.entity.EntityFactory; +import org.apache.syncope.core.persistence.api.entity.GroupableRelatable; import org.apache.syncope.core.persistence.api.entity.Realm; import org.apache.syncope.core.persistence.api.entity.VirSchema; import org.apache.syncope.core.persistence.api.entity.anyobject.ADynGroupMembership; @@ -460,12 +460,18 @@ public GroupTO getGroupTO(final String key) { return getGroupTO(groupDAO.authFind(key), true); } - protected static void populateTransitiveResources( - final Group group, final Any any, final Map> result) { + protected void populateTransitiveResources( + final Group group, + final GroupableRelatable any, + final Map> result) { PropagationByResource propByRes = new PropagationByResource<>(); group.getResources().forEach(resource -> { - if (!any.getResources().contains(resource)) { + // exclude from propagation those objects that have that resource assigned by some other membership(s) + if (!any.getResources().contains(resource) && any.getMemberships().stream() + .filter(otherGrpMemb -> !otherGrpMemb.getRightEnd().equals(group)) + .noneMatch(otherGrpMemb -> otherGrpMemb.getRightEnd().getResources().stream() + .anyMatch(r -> resource.getKey().equals(r.getKey())))) { propByRes.add(ResourceOperation.DELETE, resource.getKey()); } diff --git a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/UserIssuesITCase.java b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/UserIssuesITCase.java index ba6c17976e5..09a198d7016 100644 --- a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/UserIssuesITCase.java +++ b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/UserIssuesITCase.java @@ -36,6 +36,8 @@ import java.util.Optional; import java.util.Set; import java.util.UUID; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; import javax.naming.NamingException; import javax.ws.rs.HttpMethod; import javax.ws.rs.core.GenericType; @@ -71,6 +73,7 @@ import org.apache.syncope.common.lib.to.MembershipTO; import org.apache.syncope.common.lib.to.PlainSchemaTO; import org.apache.syncope.common.lib.to.PropagationStatus; +import org.apache.syncope.common.lib.to.PropagationTaskTO; import org.apache.syncope.common.lib.to.ProvisioningResult; import org.apache.syncope.common.lib.to.PushTaskTO; import org.apache.syncope.common.lib.to.RealmTO; @@ -93,6 +96,7 @@ import org.apache.syncope.common.lib.types.PolicyType; import org.apache.syncope.common.lib.types.ResourceAssociationAction; import org.apache.syncope.common.lib.types.ResourceDeassociationAction; +import org.apache.syncope.common.lib.types.ResourceOperation; import org.apache.syncope.common.lib.types.SchemaType; import org.apache.syncope.common.lib.types.StatusRType; import org.apache.syncope.common.lib.types.TaskType; @@ -100,6 +104,7 @@ import org.apache.syncope.common.rest.api.RESTHeaders; import org.apache.syncope.common.rest.api.beans.RealmQuery; import org.apache.syncope.common.rest.api.beans.ReconQuery; +import org.apache.syncope.common.rest.api.beans.TaskQuery; import org.apache.syncope.common.rest.api.service.UserService; import org.apache.syncope.core.provisioning.api.serialization.POJOHelper; import org.apache.syncope.core.provisioning.java.propagation.DBPasswordPropagationActions; @@ -107,6 +112,7 @@ import org.apache.syncope.core.provisioning.java.propagation.LDAPPasswordPropagationActions; import org.apache.syncope.core.spring.security.Encryptor; import org.apache.syncope.fit.AbstractITCase; +import org.awaitility.Awaitility; import org.identityconnectors.framework.common.objects.Name; import org.identityconnectors.framework.common.objects.OperationalAttributes; import org.junit.jupiter.api.BeforeAll; @@ -1820,4 +1826,55 @@ void issueSYNCOPE1818() { jdbcTemplate.update("DELETE FROM TESTPULL WHERE USERNAME = 'rossini'"); } } + + @Test + void issueSYNCOPE1853() { + UserTO bellini = USER_SERVICE.read("bellini"); + UserTO vivaldi = USER_SERVICE.read("vivaldi"); + GroupTO cGroupForPropagation = createGroup( + new GroupCR.Builder(SyncopeConstants.ROOT_REALM, "cGroupForPropagation").resource(RESOURCE_NAME_CSV) + .build()).getEntity(); + GroupTO dGroupForPropagation = createGroup( + new GroupCR.Builder(SyncopeConstants.ROOT_REALM, "dGroupForPropagation").resource(RESOURCE_NAME_CSV) + .build()).getEntity(); + // 1. assign both groups cGroupForPropagation and dGroupForPropagation with resource-csv to bellini + updateUser(new UserUR.Builder(bellini.getKey()).memberships( + new MembershipUR.Builder(cGroupForPropagation.getKey()).build(), + new MembershipUR.Builder(dGroupForPropagation.getKey()).build()).build()); + // 2. assign cGroupForPropagation also to vivaldi + updateUser(new UserUR.Builder(vivaldi.getKey()).membership( + new MembershipUR.Builder(dGroupForPropagation.getKey()).build()).build()); + // 3. propagation tasks cleanup + TASK_SERVICE.search( + new TaskQuery.Builder(TaskType.PROPAGATION).anyTypeKind(AnyTypeKind.USER) + .resource(RESOURCE_NAME_CSV) + .entityKey(bellini.getKey()).build()) + .getResult().forEach(pt -> TASK_SERVICE.delete(TaskType.PROPAGATION, pt.getKey())); + TASK_SERVICE.search( + new TaskQuery.Builder(TaskType.PROPAGATION).anyTypeKind(AnyTypeKind.USER) + .resource(RESOURCE_NAME_CSV) + .entityKey(vivaldi.getKey()).build()) + .getResult().forEach(pt -> TASK_SERVICE.delete(TaskType.PROPAGATION, pt.getKey())); + // 4. delete group cGroupForPropagation: no deprovision should be fired on bellini, since there is already + // bGroupForPropagation, deprovision instead must be fired for vivaldi + GROUP_SERVICE.delete(cGroupForPropagation.getKey()); + Awaitility.await() + .during(5, TimeUnit.SECONDS) + .atMost(10, TimeUnit.SECONDS) + .until(() -> TASK_SERVICE.search( + new TaskQuery.Builder(TaskType.PROPAGATION).anyTypeKind(AnyTypeKind.USER) + .resource(RESOURCE_NAME_CSV) + .entityKey(bellini.getKey()).build()).getResult().stream() + .map(PropagationTaskTO.class::cast).collect(Collectors.toList()).stream() + .noneMatch(pt -> ResourceOperation.DELETE == pt.getOperation())); + GROUP_SERVICE.delete(dGroupForPropagation.getKey()); + Awaitility.await() + .atMost(10, TimeUnit.SECONDS) + .until(() -> TASK_SERVICE.search( + new TaskQuery.Builder(TaskType.PROPAGATION).anyTypeKind(AnyTypeKind.USER) + .resource(RESOURCE_NAME_CSV) + .entityKey(vivaldi.getKey()).build()).getResult().stream() + .map(PropagationTaskTO.class::cast).collect(Collectors.toList()).stream() + .anyMatch(pt -> ResourceOperation.DELETE == pt.getOperation())); + } } From 1775cbea7a0c86c0f1c02d67ecc0329580a0f9e2 Mon Sep 17 00:00:00 2001 From: Andrea Patricelli Date: Mon, 20 Jan 2025 15:45:03 +0100 Subject: [PATCH 2/3] better style for test --- .../syncope/fit/core/UserIssuesITCase.java | 81 ++++++++++--------- 1 file changed, 42 insertions(+), 39 deletions(-) diff --git a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/UserIssuesITCase.java b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/UserIssuesITCase.java index 09a198d7016..95f65f0d1d3 100644 --- a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/UserIssuesITCase.java +++ b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/UserIssuesITCase.java @@ -1831,50 +1831,53 @@ void issueSYNCOPE1818() { void issueSYNCOPE1853() { UserTO bellini = USER_SERVICE.read("bellini"); UserTO vivaldi = USER_SERVICE.read("vivaldi"); - GroupTO cGroupForPropagation = createGroup( - new GroupCR.Builder(SyncopeConstants.ROOT_REALM, "cGroupForPropagation").resource(RESOURCE_NAME_CSV) - .build()).getEntity(); - GroupTO dGroupForPropagation = createGroup( - new GroupCR.Builder(SyncopeConstants.ROOT_REALM, "dGroupForPropagation").resource(RESOURCE_NAME_CSV) - .build()).getEntity(); - // 1. assign both groups cGroupForPropagation and dGroupForPropagation with resource-csv to bellini - updateUser(new UserUR.Builder(bellini.getKey()).memberships( - new MembershipUR.Builder(cGroupForPropagation.getKey()).build(), - new MembershipUR.Builder(dGroupForPropagation.getKey()).build()).build()); - // 2. assign cGroupForPropagation also to vivaldi - updateUser(new UserUR.Builder(vivaldi.getKey()).membership( - new MembershipUR.Builder(dGroupForPropagation.getKey()).build()).build()); + GroupTO cGroupForPropagation = createGroup( + new GroupCR.Builder(SyncopeConstants.ROOT_REALM, "cGroupForPropagation") + .resource(RESOURCE_NAME_CSV) + .build()).getEntity(); + GroupTO dGroupForPropagation = createGroup( + new GroupCR.Builder(SyncopeConstants.ROOT_REALM, "dGroupForPropagation") + .resource(RESOURCE_NAME_CSV) + .build()).getEntity(); + // 1. assign both groups cGroupForPropagation and dGroupForPropagation with resource-csv to bellini + updateUser(new UserUR.Builder(bellini.getKey()).memberships( + new MembershipUR.Builder(cGroupForPropagation.getKey()).build(), + new MembershipUR.Builder(dGroupForPropagation.getKey()).build()).build()); + // 2. assign cGroupForPropagation also to vivaldi + updateUser(new UserUR.Builder(vivaldi.getKey()).membership( + new MembershipUR.Builder(dGroupForPropagation.getKey()).build()).build()); // 3. propagation tasks cleanup TASK_SERVICE.search( - new TaskQuery.Builder(TaskType.PROPAGATION).anyTypeKind(AnyTypeKind.USER) + new TaskQuery.Builder(TaskType.PROPAGATION) + .anyTypeKind(AnyTypeKind.USER) .resource(RESOURCE_NAME_CSV) - .entityKey(bellini.getKey()).build()) - .getResult().forEach(pt -> TASK_SERVICE.delete(TaskType.PROPAGATION, pt.getKey())); + .entityKey(bellini.getKey()) + .build()).getResult() + .forEach(pt -> TASK_SERVICE.delete(TaskType.PROPAGATION, pt.getKey())); TASK_SERVICE.search( - new TaskQuery.Builder(TaskType.PROPAGATION).anyTypeKind(AnyTypeKind.USER) + new TaskQuery.Builder(TaskType.PROPAGATION) + .anyTypeKind(AnyTypeKind.USER) + .resource(RESOURCE_NAME_CSV) + .entityKey(vivaldi.getKey()) + .build()).getResult() + .forEach(pt -> TASK_SERVICE.delete(TaskType.PROPAGATION, pt.getKey())); + // 4. delete group cGroupForPropagation: no deprovision should be fired on bellini, since there is already + // bGroupForPropagation, deprovision instead must be fired for vivaldi + GROUP_SERVICE.delete(cGroupForPropagation.getKey()); + Awaitility.await().during(5, TimeUnit.SECONDS).atMost(10, TimeUnit.SECONDS).until(() -> TASK_SERVICE.search( + new TaskQuery.Builder(TaskType.PROPAGATION) + .anyTypeKind(AnyTypeKind.USER) + .resource(RESOURCE_NAME_CSV) + .entityKey(bellini.getKey()).build()) + .getResult().stream().map(PropagationTaskTO.class::cast) + .collect(Collectors.toList()).stream().noneMatch(pt -> ResourceOperation.DELETE == pt.getOperation())); + GROUP_SERVICE.delete(dGroupForPropagation.getKey()); + Awaitility.await().atMost(10, TimeUnit.SECONDS).until(() -> TASK_SERVICE.search( + new TaskQuery.Builder(TaskType.PROPAGATION) + .anyTypeKind(AnyTypeKind.USER) .resource(RESOURCE_NAME_CSV) .entityKey(vivaldi.getKey()).build()) - .getResult().forEach(pt -> TASK_SERVICE.delete(TaskType.PROPAGATION, pt.getKey())); - // 4. delete group cGroupForPropagation: no deprovision should be fired on bellini, since there is already - // bGroupForPropagation, deprovision instead must be fired for vivaldi - GROUP_SERVICE.delete(cGroupForPropagation.getKey()); - Awaitility.await() - .during(5, TimeUnit.SECONDS) - .atMost(10, TimeUnit.SECONDS) - .until(() -> TASK_SERVICE.search( - new TaskQuery.Builder(TaskType.PROPAGATION).anyTypeKind(AnyTypeKind.USER) - .resource(RESOURCE_NAME_CSV) - .entityKey(bellini.getKey()).build()).getResult().stream() - .map(PropagationTaskTO.class::cast).collect(Collectors.toList()).stream() - .noneMatch(pt -> ResourceOperation.DELETE == pt.getOperation())); - GROUP_SERVICE.delete(dGroupForPropagation.getKey()); - Awaitility.await() - .atMost(10, TimeUnit.SECONDS) - .until(() -> TASK_SERVICE.search( - new TaskQuery.Builder(TaskType.PROPAGATION).anyTypeKind(AnyTypeKind.USER) - .resource(RESOURCE_NAME_CSV) - .entityKey(vivaldi.getKey()).build()).getResult().stream() - .map(PropagationTaskTO.class::cast).collect(Collectors.toList()).stream() - .anyMatch(pt -> ResourceOperation.DELETE == pt.getOperation())); + .getResult().stream().map(PropagationTaskTO.class::cast) + .collect(Collectors.toList()).stream().anyMatch(pt -> ResourceOperation.DELETE == pt.getOperation())); } } From d9f9e7b529dfb2f54b4e51f0e4f33109b475b76f Mon Sep 17 00:00:00 2001 From: Andrea Patricelli Date: Mon, 20 Jan 2025 17:29:59 +0100 Subject: [PATCH 3/3] restored static and test improvements --- .../java/data/GroupDataBinderImpl.java | 2 +- .../syncope/fit/core/UserIssuesITCase.java | 32 +++++++++---------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java index 55ac59716dc..51c59822112 100644 --- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java +++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java @@ -460,7 +460,7 @@ public GroupTO getGroupTO(final String key) { return getGroupTO(groupDAO.authFind(key), true); } - protected void populateTransitiveResources( + protected static void populateTransitiveResources( final Group group, final GroupableRelatable any, final Map> result) { diff --git a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/UserIssuesITCase.java b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/UserIssuesITCase.java index 95f65f0d1d3..5b4b367f6cf 100644 --- a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/UserIssuesITCase.java +++ b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/UserIssuesITCase.java @@ -18,6 +18,7 @@ */ package org.apache.syncope.fit.core; +import static org.awaitility.Awaitility.await; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; @@ -112,7 +113,6 @@ import org.apache.syncope.core.provisioning.java.propagation.LDAPPasswordPropagationActions; import org.apache.syncope.core.spring.security.Encryptor; import org.apache.syncope.fit.AbstractITCase; -import org.awaitility.Awaitility; import org.identityconnectors.framework.common.objects.Name; import org.identityconnectors.framework.common.objects.OperationalAttributes; import org.junit.jupiter.api.BeforeAll; @@ -1829,54 +1829,52 @@ void issueSYNCOPE1818() { @Test void issueSYNCOPE1853() { - UserTO bellini = USER_SERVICE.read("bellini"); - UserTO vivaldi = USER_SERVICE.read("vivaldi"); GroupTO cGroupForPropagation = createGroup( new GroupCR.Builder(SyncopeConstants.ROOT_REALM, "cGroupForPropagation") - .resource(RESOURCE_NAME_CSV) + .resource(RESOURCE_NAME_LDAP) .build()).getEntity(); GroupTO dGroupForPropagation = createGroup( new GroupCR.Builder(SyncopeConstants.ROOT_REALM, "dGroupForPropagation") - .resource(RESOURCE_NAME_CSV) + .resource(RESOURCE_NAME_LDAP) .build()).getEntity(); // 1. assign both groups cGroupForPropagation and dGroupForPropagation with resource-csv to bellini - updateUser(new UserUR.Builder(bellini.getKey()).memberships( + updateUser(new UserUR.Builder("c9b2dec2-00a7-4855-97c0-d854842b4b24").memberships( new MembershipUR.Builder(cGroupForPropagation.getKey()).build(), new MembershipUR.Builder(dGroupForPropagation.getKey()).build()).build()); // 2. assign cGroupForPropagation also to vivaldi - updateUser(new UserUR.Builder(vivaldi.getKey()).membership( + updateUser(new UserUR.Builder("b3cbc78d-32e6-4bd4-92e0-bbe07566a2ee").membership( new MembershipUR.Builder(dGroupForPropagation.getKey()).build()).build()); // 3. propagation tasks cleanup TASK_SERVICE.search( new TaskQuery.Builder(TaskType.PROPAGATION) .anyTypeKind(AnyTypeKind.USER) - .resource(RESOURCE_NAME_CSV) - .entityKey(bellini.getKey()) + .resource(RESOURCE_NAME_LDAP) + .entityKey("c9b2dec2-00a7-4855-97c0-d854842b4b24") .build()).getResult() .forEach(pt -> TASK_SERVICE.delete(TaskType.PROPAGATION, pt.getKey())); TASK_SERVICE.search( new TaskQuery.Builder(TaskType.PROPAGATION) .anyTypeKind(AnyTypeKind.USER) - .resource(RESOURCE_NAME_CSV) - .entityKey(vivaldi.getKey()) + .resource(RESOURCE_NAME_LDAP) + .entityKey("b3cbc78d-32e6-4bd4-92e0-bbe07566a2ee") .build()).getResult() .forEach(pt -> TASK_SERVICE.delete(TaskType.PROPAGATION, pt.getKey())); // 4. delete group cGroupForPropagation: no deprovision should be fired on bellini, since there is already // bGroupForPropagation, deprovision instead must be fired for vivaldi GROUP_SERVICE.delete(cGroupForPropagation.getKey()); - Awaitility.await().during(5, TimeUnit.SECONDS).atMost(10, TimeUnit.SECONDS).until(() -> TASK_SERVICE.search( + await().during(5, TimeUnit.SECONDS).atMost(10, TimeUnit.SECONDS).until(() -> TASK_SERVICE.search( new TaskQuery.Builder(TaskType.PROPAGATION) .anyTypeKind(AnyTypeKind.USER) - .resource(RESOURCE_NAME_CSV) - .entityKey(bellini.getKey()).build()) + .resource(RESOURCE_NAME_LDAP) + .entityKey("c9b2dec2-00a7-4855-97c0-d854842b4b24").build()) .getResult().stream().map(PropagationTaskTO.class::cast) .collect(Collectors.toList()).stream().noneMatch(pt -> ResourceOperation.DELETE == pt.getOperation())); GROUP_SERVICE.delete(dGroupForPropagation.getKey()); - Awaitility.await().atMost(10, TimeUnit.SECONDS).until(() -> TASK_SERVICE.search( + await().atMost(10, TimeUnit.SECONDS).until(() -> TASK_SERVICE.search( new TaskQuery.Builder(TaskType.PROPAGATION) .anyTypeKind(AnyTypeKind.USER) - .resource(RESOURCE_NAME_CSV) - .entityKey(vivaldi.getKey()).build()) + .resource(RESOURCE_NAME_LDAP) + .entityKey("b3cbc78d-32e6-4bd4-92e0-bbe07566a2ee").build()) .getResult().stream().map(PropagationTaskTO.class::cast) .collect(Collectors.toList()).stream().anyMatch(pt -> ResourceOperation.DELETE == pt.getOperation())); }