[SYNCOPE-1356] Enhancing LDAPMembershipPullActions / SetUMembershipsJob with capabilites from 1_2_X
Project: http://git-wip-us.apache.org/repos/asf/syncope/repo Commit: http://git-wip-us.apache.org/repos/asf/syncope/commit/4810fb79 Tree: http://git-wip-us.apache.org/repos/asf/syncope/tree/4810fb79 Diff: http://git-wip-us.apache.org/repos/asf/syncope/diff/4810fb79 Branch: refs/heads/2_0_X Commit: 4810fb796b7d6e5035005a24e213e1c9cc495693 Parents: 16d290b Author: Francesco Chicchiriccò <ilgro...@apache.org> Authored: Thu Aug 16 12:55:12 2018 +0200 Committer: Francesco Chicchiriccò <ilgro...@apache.org> Committed: Thu Aug 16 14:15:36 2018 +0200 ---------------------------------------------------------------------- .../core/migration/MigrationPullActions.java | 5 +- .../java/job/SetUMembershipsJob.java | 73 ++++++++++++++++---- .../pushpull/LDAPMembershipPullActions.java | 73 ++++++++++++-------- .../apache/syncope/fit/core/PullTaskITCase.java | 45 +++++++++--- 4 files changed, 141 insertions(+), 55 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/syncope/blob/4810fb79/core/migration/src/main/java/org/apache/syncope/core/migration/MigrationPullActions.java ---------------------------------------------------------------------- diff --git a/core/migration/src/main/java/org/apache/syncope/core/migration/MigrationPullActions.java b/core/migration/src/main/java/org/apache/syncope/core/migration/MigrationPullActions.java index aab5f8f..b6a19e5 100644 --- a/core/migration/src/main/java/org/apache/syncope/core/migration/MigrationPullActions.java +++ b/core/migration/src/main/java/org/apache/syncope/core/migration/MigrationPullActions.java @@ -18,6 +18,7 @@ */ package org.apache.syncope.core.migration; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -131,8 +132,8 @@ public class MigrationPullActions extends SchedulingPullActions { @Override public void afterAll(final ProvisioningProfile<?, ?> profile) throws JobExecutionException { Map<String, Object> jobMap = new HashMap<>(); - jobMap.put(SetUMembershipsJob.MEMBERSHIPS_KEY, memberships); + jobMap.put(SetUMembershipsJob.MEMBERSHIPS_BEFORE_KEY, Collections.emptyMap()); + jobMap.put(SetUMembershipsJob.MEMBERSHIPS_AFTER_KEY, memberships); schedule(SetUMembershipsJob.class, jobMap); } - } http://git-wip-us.apache.org/repos/asf/syncope/blob/4810fb79/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/job/SetUMembershipsJob.java ---------------------------------------------------------------------- diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/job/SetUMembershipsJob.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/job/SetUMembershipsJob.java index 8fe4b67..228a61f 100644 --- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/job/SetUMembershipsJob.java +++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/job/SetUMembershipsJob.java @@ -18,8 +18,12 @@ */ package org.apache.syncope.core.provisioning.java.job; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.Set; +import org.apache.commons.collections4.IterableUtils; +import org.apache.commons.collections4.Predicate; import org.apache.syncope.common.lib.patch.MembershipPatch; import org.apache.syncope.common.lib.patch.UserPatch; import org.apache.syncope.common.lib.types.PatchOperation; @@ -40,7 +44,9 @@ public class SetUMembershipsJob extends AbstractInterruptableJob { private static final Logger LOG = LoggerFactory.getLogger(SetUMembershipsJob.class); - public static final String MEMBERSHIPS_KEY = "memberships"; + public static final String MEMBERSHIPS_BEFORE_KEY = "membershipsBefore"; + + public static final String MEMBERSHIPS_AFTER_KEY = "membershipsAfter"; @Autowired private UserProvisioningManager userProvisioningManager; @@ -54,26 +60,64 @@ public class SetUMembershipsJob extends AbstractInterruptableJob { @Override public Void exec() { @SuppressWarnings("unchecked") - Map<String, Set<String>> memberships = - (Map<String, Set<String>>) context.getMergedJobDataMap().get(MEMBERSHIPS_KEY); + Map<String, Set<String>> membershipsBefore = + (Map<String, Set<String>>) context.getMergedJobDataMap().get(MEMBERSHIPS_BEFORE_KEY); + LOG.debug("Memberships before pull (User -> Groups) {}", membershipsBefore); + + @SuppressWarnings("unchecked") + Map<String, Set<String>> membershipsAfter = + (Map<String, Set<String>>) context.getMergedJobDataMap().get(MEMBERSHIPS_AFTER_KEY); + LOG.debug("Memberships after pull (User -> Groups) {}", membershipsAfter); - LOG.debug("About to set memberships (User -> Groups) {}", memberships); + List<UserPatch> patches = new ArrayList<>(); - for (Map.Entry<String, Set<String>> membership : memberships.entrySet()) { + for (Map.Entry<String, Set<String>> membership : membershipsAfter.entrySet()) { UserPatch userPatch = new UserPatch(); userPatch.setKey(membership.getKey()); + patches.add(userPatch); - for (String groupKey : membership.getValue()) { - userPatch.getMemberships().add( - new MembershipPatch.Builder(). - operation(PatchOperation.ADD_REPLACE). - group(groupKey). - build()); + for (String group : membership.getValue()) { + Set<String> before = membershipsBefore.get(membership.getKey()); + if (before == null || !before.contains(group)) { + userPatch.getMemberships().add( + new MembershipPatch.Builder(). + operation(PatchOperation.ADD_REPLACE). + group(group). + build()); + } } + } + + for (final Map.Entry<String, Set<String>> membership : membershipsBefore.entrySet()) { + UserPatch userPatch = IterableUtils.find(patches, new Predicate<UserPatch>() { - if (!userPatch.isEmpty()) { - LOG.debug("About to update User {}", userPatch.getKey()); - userProvisioningManager.update(userPatch, true); + @Override + public boolean evaluate(final UserPatch patch) { + return membership.getKey().equals(patch.getKey()); + } + }); + if (userPatch == null) { + userPatch = new UserPatch(); + userPatch.setKey(membership.getKey()); + patches.add(userPatch); + } + + for (String group : membership.getValue()) { + Set<String> after = membershipsAfter.get(membership.getKey()); + if (after == null || !after.contains(group)) { + userPatch.getMemberships().add( + new MembershipPatch.Builder(). + operation(PatchOperation.DELETE). + group(group). + build()); + } + } + } + + for (UserPatch patch : patches) { + if (!patch.isEmpty()) { + LOG.debug("About to update User {}", patch); + userProvisioningManager.update(patch, true); } } @@ -85,5 +129,4 @@ public class SetUMembershipsJob extends AbstractInterruptableJob { throw new JobExecutionException("While executing memberships", e); } } - } http://git-wip-us.apache.org/repos/asf/syncope/blob/4810fb79/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActions.java ---------------------------------------------------------------------- diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActions.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActions.java index 02ac42c..fd262b5 100644 --- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActions.java +++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActions.java @@ -26,6 +26,7 @@ import java.util.Map; import java.util.Set; import org.apache.commons.collections4.IterableUtils; import org.apache.commons.collections4.Predicate; +import org.apache.syncope.common.lib.patch.AnyPatch; import org.apache.syncope.common.lib.to.EntityTO; import org.apache.syncope.common.lib.to.GroupTO; import org.apache.syncope.common.lib.types.ConnConfProperty; @@ -34,7 +35,6 @@ import org.apache.syncope.core.provisioning.api.Connector; import org.apache.syncope.core.provisioning.api.pushpull.ProvisioningProfile; import org.apache.syncope.core.provisioning.api.pushpull.ProvisioningReport; import org.apache.syncope.core.persistence.api.dao.AnyTypeDAO; -import org.apache.syncope.core.persistence.api.dao.UserDAO; import org.identityconnectors.framework.common.objects.Attribute; import org.identityconnectors.framework.common.objects.ConnectorObject; import org.identityconnectors.framework.common.objects.ObjectClass; @@ -45,7 +45,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.apache.syncope.core.persistence.api.entity.task.PullTask; +import org.apache.syncope.core.persistence.api.entity.user.UMembership; import org.apache.syncope.core.provisioning.java.job.SetUMembershipsJob; +import org.springframework.transaction.annotation.Transactional; /** * Simple action for pulling LDAP groups memberships to Syncope group memberships, when the same resource is @@ -61,15 +63,14 @@ public class LDAPMembershipPullActions extends SchedulingPullActions { protected AnyTypeDAO anyTypeDAO; @Autowired - protected UserDAO userDAO; - - @Autowired protected GroupDAO groupDAO; @Autowired private PullUtils pullUtils; - protected final Map<String, Set<String>> memberships = new HashMap<>(); + protected final Map<String, Set<String>> membershipsBefore = new HashMap<>(); + + protected final Map<String, Set<String>> membershipsAfter = new HashMap<>(); /** * Allows easy subclassing for the ConnId AD connector bundle. @@ -127,30 +128,39 @@ public class LDAPMembershipPullActions extends SchedulingPullActions { } /** - * Pull Syncope memberships with the situation read on the external resource's group. + * Keep track of members of the group being updated before actual update takes place. + * This is not needed on + * <ul> + * <li>{@link #beforeProvision} because the pulling group does not exist yet on Syncope</li> + * <li>{@link #beforeDelete} because group delete cascades as membership removal for all users involved</li> + * </ul> * - * @param profile pull profile - * @param delta representing the pullong group - * @param groupTO group after modification performed by the handler - * @throws JobExecutionException if anything goes wrong + * {@inheritDoc} */ - protected void populateMemberships( - final ProvisioningProfile<?, ?> profile, final SyncDelta delta, final GroupTO groupTO) - throws JobExecutionException { + @Transactional(readOnly = true) + @Override + public <P extends AnyPatch> void beforeUpdate( + final ProvisioningProfile<?, ?> profile, + final SyncDelta delta, + final EntityTO entity, + final P anyPatch) throws JobExecutionException { - Connector connector = profile.getConnector(); - for (Object membValue : getMembAttrValues(delta, connector)) { - Set<String> memb = memberships.get(membValue.toString()); + if (!(entity instanceof GroupTO)) { + super.beforeUpdate(profile, delta, entity, anyPatch); + } + + for (UMembership uMembership : groupDAO.findUMemberships(groupDAO.find(entity.getKey()))) { + Set<String> memb = membershipsBefore.get(uMembership.getLeftEnd().getKey()); if (memb == null) { memb = new HashSet<>(); - memberships.put(membValue.toString(), memb); + membershipsBefore.put(uMembership.getLeftEnd().getKey(), memb); } - memb.add(groupTO.getKey()); + memb.add(entity.getKey()); } } /** - * Pull membership at group pull time (because PullJob first pulls users then groups). + * Keep track of members of the group being updated after actual update took place. * {@inheritDoc} */ @Override @@ -169,29 +179,32 @@ public class LDAPMembershipPullActions extends SchedulingPullActions { || profile.getTask().getResource().getProvision(anyTypeDAO.findUser()).getMapping() == null) { super.after(profile, delta, entity, result); - } else { - populateMemberships(profile, delta, (GroupTO) entity); } - } - @Override - public void afterAll(final ProvisioningProfile<?, ?> profile) throws JobExecutionException { - Map<String, Set<String>> resolvedMemberships = new HashMap<>(); - for (Map.Entry<String, Set<String>> entry : this.memberships.entrySet()) { + for (Object membValue : getMembAttrValues(delta, profile.getConnector())) { String userKey = pullUtils.match( anyTypeDAO.findUser(), - entry.getKey(), + membValue.toString(), profile.getTask().getResource(), profile.getConnector()); if (userKey == null) { - LOG.warn("Could not find matching user for {}", entry.getKey()); + LOG.warn("Could not find matching user for {}", membValue); } else { - resolvedMemberships.put(userKey, entry.getValue()); + Set<String> memb = membershipsAfter.get(userKey); + if (memb == null) { + memb = new HashSet<>(); + membershipsAfter.put(userKey, memb); + } + memb.add(entity.getKey()); } } + } + @Override + public void afterAll(final ProvisioningProfile<?, ?> profile) throws JobExecutionException { Map<String, Object> jobMap = new HashMap<>(); - jobMap.put(SetUMembershipsJob.MEMBERSHIPS_KEY, resolvedMemberships); + jobMap.put(SetUMembershipsJob.MEMBERSHIPS_BEFORE_KEY, membershipsBefore); + jobMap.put(SetUMembershipsJob.MEMBERSHIPS_AFTER_KEY, membershipsAfter); schedule(SetUMembershipsJob.class, jobMap); } } http://git-wip-us.apache.org/repos/asf/syncope/blob/4810fb79/fit/core-reference/src/test/java/org/apache/syncope/fit/core/PullTaskITCase.java ---------------------------------------------------------------------- diff --git a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/PullTaskITCase.java b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/PullTaskITCase.java index 140e056..5be82dc 100644 --- a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/PullTaskITCase.java +++ b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/PullTaskITCase.java @@ -393,19 +393,18 @@ public class PullTaskITCase extends AbstractTaskITCase { assertEquals(matchingUsers.getResult().iterator().next().getKey(), groupTO.getUserOwner()); assertNull(groupTO.getGroupOwner()); // SYNCOPE-1343, set value title to null on LDAP - ConnObjectTO connObject = - resourceService.readConnObject(RESOURCE_NAME_LDAP, AnyTypeKind.USER.name(), - matchingUsers.getResult().get(0).getKey()); - assertNotNull(connObject); - assertEquals("odd", connObject.getAttr("title").getValues().get(0)); - AttrTO userDn = connObject.getAttr(Name.NAME); + ConnObjectTO userConnObject = resourceService.readConnObject( + RESOURCE_NAME_LDAP, AnyTypeKind.USER.name(), matchingUsers.getResult().get(0).getKey()); + assertNotNull(userConnObject); + assertEquals("odd", userConnObject.getAttr("title").getValues().get(0)); + AttrTO userDn = userConnObject.getAttr(Name.NAME); updateLdapRemoteObject(RESOURCE_LDAP_ADMIN_DN, RESOURCE_LDAP_ADMIN_PWD, userDn.getValues().get(0), Pair.of("title", (String) null)); // SYNCOPE-317 execProvisioningTask(taskService, TaskType.PULL, "1e419ca4-ea81-4493-a14f-28b90113686d", 50, false); - // 4. verify that LDAP group membership is propagated as Syncope membership + // 4. verify that LDAP group membership is pulled as Syncope membership int i = 0; int maxit = 50; PagedResult<UserTO> members; @@ -427,13 +426,43 @@ public class PullTaskITCase extends AbstractTaskITCase { } assertEquals(1, members.getResult().size()); - // SYNCOPE-1343, verify that the title attribte has been reset + // SYNCOPE-1343, verify that the title attribute has been reset matchingUsers = userService.search( new AnyQuery.Builder().realm(SyncopeConstants.ROOT_REALM). fiql(SyncopeClient.getUserSearchConditionBuilder().is("username").equalTo("pullFromLDAP"). query()). build()); assertNull(matchingUsers.getResult().get(0).getPlainAttr("title")); + + // SYNCOPE-1356 remove group membership from LDAP, pull and check in Syncope + ConnObjectTO groupConnObject = resourceService.readConnObject( + RESOURCE_NAME_LDAP, AnyTypeKind.GROUP.name(), matchingGroups.getResult().get(0).getKey()); + assertNotNull(groupConnObject); + AttrTO groupDn = groupConnObject.getAttr(Name.NAME); + updateLdapRemoteObject(RESOURCE_LDAP_ADMIN_DN, RESOURCE_LDAP_ADMIN_PWD, + groupDn.getValues().get(0), Pair.of("uniquemember", "uid=admin,ou=system")); + + execProvisioningTask(taskService, TaskType.PULL, "1e419ca4-ea81-4493-a14f-28b90113686d", 50, false); + + i = 0; + maxit = 50; + do { + try { + Thread.sleep(1000); + } catch (InterruptedException e) { + } + + members = userService.search(new AnyQuery.Builder().realm(SyncopeConstants.ROOT_REALM). + fiql(SyncopeClient.getUserSearchConditionBuilder().inGroups(groupTO.getKey()).query()). + build()); + assertNotNull(members); + + i++; + } while (!members.getResult().isEmpty() && i < maxit); + if (i == maxit) { + fail("Timeout while checking for memberships of " + groupTO.getName()); + } + assertEquals(0, members.getResult().size()); } @Test
