[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/b70bbb37 Tree: http://git-wip-us.apache.org/repos/asf/syncope/tree/b70bbb37 Diff: http://git-wip-us.apache.org/repos/asf/syncope/diff/b70bbb37 Branch: refs/heads/2_1_X Commit: b70bbb37e0e0d2e1c9aa3f7162ab3748708b16b0 Parents: 4649a2b 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 12:55:12 2018 +0200 ---------------------------------------------------------------------- .../core/persistence/api/dao/GroupDAO.java | 1 + .../java/job/SetUMembershipsJob.java | 67 +++++++++++++----- .../pushpull/LDAPMembershipPullActions.java | 74 ++++++++++++-------- .../apache/syncope/fit/core/PullTaskITCase.java | 45 +++++++++--- 4 files changed, 132 insertions(+), 55 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/syncope/blob/b70bbb37/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/GroupDAO.java ---------------------------------------------------------------------- diff --git a/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/GroupDAO.java b/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/GroupDAO.java index ce2c183..4db06dc 100644 --- a/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/GroupDAO.java +++ b/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/GroupDAO.java @@ -57,6 +57,7 @@ public interface GroupDAO extends AnyDAO<Group> { int countUDynMembers(Group group); + @Override Collection<String> findAllResourceKeys(final String key); void clearADynMembers(Group group); http://git-wip-us.apache.org/repos/asf/syncope/blob/b70bbb37/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 b62ff33..993c999 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,6 +18,8 @@ */ 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.syncope.common.lib.patch.MembershipPatch; @@ -40,7 +42,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; @@ -48,31 +52,62 @@ public class SetUMembershipsJob extends AbstractInterruptableJob { @Override public void execute(final JobExecutionContext context) throws JobExecutionException { try { - AuthContextUtils.execWithAuthContext( - context.getMergedJobDataMap().getString(JobManager.DOMAIN_KEY), () -> { + AuthContextUtils.execWithAuthContext(context.getMergedJobDataMap().getString(JobManager.DOMAIN_KEY), () -> { + + @SuppressWarnings("unchecked") + 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>> memberships = - (Map<String, Set<String>>) context.getMergedJobDataMap().get(MEMBERSHIPS_KEY); + 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<>(); - memberships.forEach((user, groups) -> { + membershipsAfter.forEach((user, groups) -> { UserPatch userPatch = new UserPatch(); userPatch.setKey(user); + patches.add(userPatch); groups.forEach(group -> { - userPatch.getMemberships().add( - new MembershipPatch.Builder(). - operation(PatchOperation.ADD_REPLACE). - group(group). - build()); + Set<String> before = membershipsBefore.get(user); + if (before == null || !before.contains(group)) { + userPatch.getMemberships().add( + new MembershipPatch.Builder(). + operation(PatchOperation.ADD_REPLACE). + group(group). + build()); + } }); + }); + + membershipsBefore.forEach((user, groups) -> { + UserPatch userPatch = patches.stream(). + filter(patch -> user.equals(patch.getKey())).findFirst(). + orElseGet(() -> { + UserPatch patch = new UserPatch(); + patch.setKey(user); + patches.add(patch); + return patch; + }); + + groups.forEach(group -> { + Set<String> after = membershipsAfter.get(user); + if (after == null || !after.contains(group)) { + userPatch.getMemberships().add( + new MembershipPatch.Builder(). + operation(PatchOperation.DELETE). + group(group). + build()); + } + }); + }); - if (!userPatch.isEmpty()) { - LOG.debug("About to update User {}", userPatch.getKey()); - userProvisioningManager.update(userPatch, true); - } + patches.stream().filter(patch -> !patch.isEmpty()).forEach(patch -> { + LOG.debug("About to update User {}", patch); + userProvisioningManager.update(patch, true); }); return null; http://git-wip-us.apache.org/repos/asf/syncope/blob/b70bbb37/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 b7e8338..cdb5e1e 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 @@ -25,6 +25,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +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; @@ -33,7 +34,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.apache.syncope.core.persistence.api.entity.resource.Provision; import org.identityconnectors.framework.common.objects.Attribute; import org.identityconnectors.framework.common.objects.ConnectorObject; @@ -45,6 +45,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; 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 @@ -60,15 +61,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. @@ -118,29 +118,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 { - getMembAttrValues(delta, profile.getConnector()).forEach(membValue -> { - Set<String> memb = memberships.get(membValue.toString()); + if (!(entity instanceof GroupTO)) { + super.beforeUpdate(profile, delta, entity, anyPatch); + } + + groupDAO.findUMemberships(groupDAO.find(entity.getKey())).forEach(uMembership -> { + 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 @@ -156,33 +166,35 @@ public class LDAPMembershipPullActions extends SchedulingPullActions { Optional<? extends Provision> provision = profile.getTask().getResource().getProvision(anyTypeDAO.findUser()). filter(p -> p.getMapping() != null); - if (provision.isPresent()) { - populateMemberships(profile, delta, (GroupTO) entity); - } else { + if (!provision.isPresent()) { super.after(profile, delta, entity, result); } - } - - @Override - public void afterAll(final ProvisioningProfile<?, ?> profile) throws JobExecutionException { - Map<String, Set<String>> resolvedMemberships = new HashMap<>(); - memberships.forEach((name, memb) -> { + getMembAttrValues(delta, profile.getConnector()).forEach(membValue -> { Optional<String> userKey = pullUtils.match( anyTypeDAO.findUser(), - name, + membValue.toString(), profile.getTask().getResource(), profile.getConnector(), false); if (userKey.isPresent()) { - resolvedMemberships.put(userKey.get(), memb); + Set<String> memb = membershipsAfter.get(userKey.get()); + if (memb == null) { + memb = new HashSet<>(); + membershipsAfter.put(userKey.get(), memb); + } + memb.add(entity.getKey()); } else { - LOG.warn("Could not find matching user for {}", name); + LOG.warn("Could not find matching user for {}", membValue); } }); + } + @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/b70bbb37/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 c5c90e51..2dc671b 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 @@ -406,19 +406,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").get().getValues().get(0)); - AttrTO userDn = connObject.getAttr(Name.NAME).get(); + ConnObjectTO userConnObject = resourceService.readConnObject( + RESOURCE_NAME_LDAP, AnyTypeKind.USER.name(), matchingUsers.getResult().get(0).getKey()); + assertNotNull(userConnObject); + assertEquals("odd", userConnObject.getAttr("title").get().getValues().get(0)); + AttrTO userDn = userConnObject.getAttr(Name.NAME).get(); updateLdapRemoteObject(RESOURCE_LDAP_ADMIN_DN, RESOURCE_LDAP_ADMIN_PWD, userDn.getValues().get(0), Collections.singletonMap("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; @@ -440,13 +439,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").orElse(null)); + + // 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).get(); + updateLdapRemoteObject(RESOURCE_LDAP_ADMIN_DN, RESOURCE_LDAP_ADMIN_PWD, + groupDn.getValues().get(0), Collections.singletonMap("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
