Repository: syncope Updated Branches: refs/heads/2_0_X 5ed622cdc -> 5cfe9666b refs/heads/master cea47da1a -> df35f9ca9
[SYNCOPE-1222] Better resource management in case of membership removal Project: http://git-wip-us.apache.org/repos/asf/syncope/repo Commit: http://git-wip-us.apache.org/repos/asf/syncope/commit/df35f9ca Tree: http://git-wip-us.apache.org/repos/asf/syncope/tree/df35f9ca Diff: http://git-wip-us.apache.org/repos/asf/syncope/diff/df35f9ca Branch: refs/heads/master Commit: df35f9ca9009a3a51f43d9d8643c73b80162cfde Parents: cea47da Author: Francesco Chicchiriccò <[email protected]> Authored: Wed Oct 11 14:04:27 2017 +0200 Committer: Francesco Chicchiriccò <[email protected]> Committed: Wed Oct 11 14:04:27 2017 +0200 ---------------------------------------------------------------------- .../java/data/AnyObjectDataBinderImpl.java | 41 +++++++++++++----- .../java/data/UserDataBinderImpl.java | 41 +++++++++++++----- .../syncope/fit/core/UserIssuesITCase.java | 45 ++++++++++---------- 3 files changed, 84 insertions(+), 43 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/syncope/blob/df35f9ca/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AnyObjectDataBinderImpl.java ---------------------------------------------------------------------- diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AnyObjectDataBinderImpl.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AnyObjectDataBinderImpl.java index 1bdfa36..c677063 100644 --- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AnyObjectDataBinderImpl.java +++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AnyObjectDataBinderImpl.java @@ -20,6 +20,7 @@ package org.apache.syncope.core.provisioning.java.data; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -262,9 +263,6 @@ public class AnyObjectDataBinderImpl extends AbstractAnyDataBinder implements An // attributes and resources propByRes.merge(fill(anyObject, anyObjectPatch, anyUtils, scce)); - Set<String> toBeDeprovisioned = new HashSet<>(); - Set<String> toBeProvisioned = new HashSet<>(); - // relationships anyObjectPatch.getRelationships().stream(). filter(patch -> patch.getRelationshipTO() != null).forEachOrdered((patch) -> { @@ -277,9 +275,6 @@ public class AnyObjectDataBinderImpl extends AbstractAnyDataBinder implements An if (relationship.isPresent()) { anyObject.getRelationships().remove(relationship.get()); relationship.get().setLeftEnd(null); - - toBeDeprovisioned.addAll( - anyObjectDAO.findAllResourceKeys(relationship.get().getRightEnd().getKey())); } if (patch.getOperation() == PatchOperation.ADD_REPLACE) { @@ -303,8 +298,6 @@ public class AnyObjectDataBinderImpl extends AbstractAnyDataBinder implements An newRelationship.setLeftEnd(anyObject); anyObject.add(newRelationship); - - toBeProvisioned.addAll(anyObjectDAO.findAllResourceKeys(otherEnd.getKey())); } else { LOG.error("{} cannot be assigned to {}", otherEnd, anyObject); @@ -318,7 +311,25 @@ public class AnyObjectDataBinderImpl extends AbstractAnyDataBinder implements An } }); + // prepare for membership-related resource management Collection<ExternalResource> resources = anyObjectDAO.findAllResources(anyObject); + + Map<String, Set<String>> reasons = new HashMap<>(); + anyObject.getResources().forEach(resource -> { + reasons.put(resource.getKey(), new HashSet<>(Collections.singleton(anyObject.getKey()))); + }); + anyObjectDAO.findAllGroupKeys(anyObject).forEach(group -> { + groupDAO.findAllResourceKeys(group).forEach(resource -> { + if (!reasons.containsKey(resource)) { + reasons.put(resource, new HashSet<>()); + } + reasons.get(resource).add(group); + }); + }); + + Set<String> toBeDeprovisioned = new HashSet<>(); + Set<String> toBeProvisioned = new HashSet<>(); + SyncopeClientException invalidValues = SyncopeClientException.build(ClientExceptionType.InvalidValues); // memberships @@ -334,7 +345,12 @@ public class AnyObjectDataBinderImpl extends AbstractAnyDataBinder implements An }); if (membPatch.getOperation() == PatchOperation.DELETE) { - toBeDeprovisioned.addAll(groupDAO.findAllResourceKeys(membership.get().getRightEnd().getKey())); + groupDAO.findAllResourceKeys(membership.get().getRightEnd().getKey()).stream(). + filter(resource -> reasons.containsKey(resource)). + forEach(resource -> { + reasons.get(resource).remove(membership.get().getRightEnd().getKey()); + toBeProvisioned.add(resource); + }); } } if (membPatch.getOperation() == PatchOperation.ADD_REPLACE) { @@ -389,10 +405,15 @@ public class AnyObjectDataBinderImpl extends AbstractAnyDataBinder implements An } }); + // finalize resource management + reasons.entrySet().stream(). + filter(entry -> entry.getValue().isEmpty()). + forEach(entry -> toBeDeprovisioned.add(entry.getKey())); + propByRes.addAll(ResourceOperation.DELETE, toBeDeprovisioned); propByRes.addAll(ResourceOperation.UPDATE, toBeProvisioned); - // In case of new memberships all current resources need to be updated in order to propagate new group + // in case of new memberships all current resources need to be updated in order to propagate new group // attribute values. if (!toBeDeprovisioned.isEmpty() || !toBeProvisioned.isEmpty()) { currentResources.removeAll(toBeDeprovisioned); http://git-wip-us.apache.org/repos/asf/syncope/blob/df35f9ca/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/UserDataBinderImpl.java ---------------------------------------------------------------------- diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/UserDataBinderImpl.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/UserDataBinderImpl.java index 6badbef..770e7da 100644 --- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/UserDataBinderImpl.java +++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/UserDataBinderImpl.java @@ -21,6 +21,7 @@ package org.apache.syncope.core.provisioning.java.data; import java.util.Collection; import java.util.Collections; import java.util.Date; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -365,9 +366,6 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat // attributes and resources propByRes.merge(fill(user, userPatch, anyUtils, scce)); - Set<String> toBeDeprovisioned = new HashSet<>(); - Set<String> toBeProvisioned = new HashSet<>(); - // relationships userPatch.getRelationships().stream(). filter(patch -> patch.getRelationshipTO() != null).forEachOrdered((patch) -> { @@ -380,9 +378,6 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat if (relationship.isPresent()) { user.getRelationships().remove(relationship.get()); relationship.get().setLeftEnd(null); - - toBeDeprovisioned.addAll( - anyObjectDAO.findAllResourceKeys(relationship.get().getRightEnd().getKey())); } if (patch.getOperation() == PatchOperation.ADD_REPLACE) { @@ -396,8 +391,6 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat newRelationship.setLeftEnd(user); user.add(newRelationship); - - toBeProvisioned.addAll(anyObjectDAO.findAllResourceKeys(otherEnd.getKey())); } else { LOG.error("{} cannot be assigned to {}", otherEnd, user); @@ -410,7 +403,25 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat } }); + // prepare for membership-related resource management Collection<ExternalResource> resources = userDAO.findAllResources(user); + + Map<String, Set<String>> reasons = new HashMap<>(); + user.getResources().forEach(resource -> { + reasons.put(resource.getKey(), new HashSet<>(Collections.singleton(user.getKey()))); + }); + userDAO.findAllGroupKeys(user).forEach(group -> { + groupDAO.findAllResourceKeys(group).forEach(resource -> { + if (!reasons.containsKey(resource)) { + reasons.put(resource, new HashSet<>()); + } + reasons.get(resource).add(group); + }); + }); + + Set<String> toBeDeprovisioned = new HashSet<>(); + Set<String> toBeProvisioned = new HashSet<>(); + SyncopeClientException invalidValues = SyncopeClientException.build(ClientExceptionType.InvalidValues); // memberships @@ -427,7 +438,12 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat }); if (membPatch.getOperation() == PatchOperation.DELETE) { - toBeDeprovisioned.addAll(groupDAO.findAllResourceKeys(membership.get().getRightEnd().getKey())); + groupDAO.findAllResourceKeys(membership.get().getRightEnd().getKey()).stream(). + filter(resource -> reasons.containsKey(resource)). + forEach(resource -> { + reasons.get(resource).remove(membership.get().getRightEnd().getKey()); + toBeProvisioned.add(resource); + }); } } if (membPatch.getOperation() == PatchOperation.ADD_REPLACE) { @@ -494,10 +510,15 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat } }); + // finalize resource management + reasons.entrySet().stream(). + filter(entry -> entry.getValue().isEmpty()). + forEach(entry -> toBeDeprovisioned.add(entry.getKey())); + propByRes.addAll(ResourceOperation.DELETE, toBeDeprovisioned); propByRes.addAll(ResourceOperation.UPDATE, toBeProvisioned); - // In case of new memberships all current resources need to be updated in order to propagate new group + // in case of new memberships all current resources need to be updated in order to propagate new group // attribute values. if (!toBeDeprovisioned.isEmpty() || !toBeProvisioned.isEmpty()) { currentResources.removeAll(toBeDeprovisioned); http://git-wip-us.apache.org/repos/asf/syncope/blob/df35f9ca/fit/core-reference/src/test/java/org/apache/syncope/fit/core/UserIssuesITCase.java ---------------------------------------------------------------------- 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 2c28906..2a1e944 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 @@ -197,7 +197,7 @@ public class UserIssuesITCase extends AbstractITCase { } @Test - public final void issue280() { + public void issue280() { UserTO userTO = UserITCase.getUniqueSampleTO("[email protected]"); userTO.getResources().clear(); userTO.getMemberships().clear(); @@ -367,7 +367,7 @@ public class UserIssuesITCase extends AbstractITCase { @Test() public void issueSYNCOPE51() { AttrTO defaultCA = configurationService.get("password.cipher.algorithm"); - final String originalCAValue = defaultCA.getValues().get(0); + String originalCAValue = defaultCA.getValues().get(0); defaultCA.getValues().set(0, "MD5"); configurationService.set(defaultCA); @@ -466,23 +466,23 @@ public class UserIssuesITCase extends AbstractITCase { assertTrue(userTO.getResources().contains(RESOURCE_NAME_TESTDB)); assertTrue(userTO.getResources().contains(RESOURCE_NAME_TESTDB2)); - final String pwdOnSyncope = userTO.getPassword(); + String pwdOnSyncope = userTO.getPassword(); ConnObjectTO userOnDb = resourceService.readConnObject( RESOURCE_NAME_TESTDB, AnyTypeKind.USER.name(), userTO.getKey()); - final AttrTO pwdOnTestDbAttr = userOnDb.getAttr(OperationalAttributes.PASSWORD_NAME).get(); + AttrTO pwdOnTestDbAttr = userOnDb.getAttr(OperationalAttributes.PASSWORD_NAME).get(); assertNotNull(pwdOnTestDbAttr); assertNotNull(pwdOnTestDbAttr.getValues()); assertFalse(pwdOnTestDbAttr.getValues().isEmpty()); - final String pwdOnTestDb = pwdOnTestDbAttr.getValues().iterator().next(); + String pwdOnTestDb = pwdOnTestDbAttr.getValues().iterator().next(); ConnObjectTO userOnDb2 = resourceService.readConnObject( RESOURCE_NAME_TESTDB2, AnyTypeKind.USER.name(), userTO.getKey()); - final AttrTO pwdOnTestDb2Attr = userOnDb2.getAttr(OperationalAttributes.PASSWORD_NAME).get(); + AttrTO pwdOnTestDb2Attr = userOnDb2.getAttr(OperationalAttributes.PASSWORD_NAME).get(); assertNotNull(pwdOnTestDb2Attr); assertNotNull(pwdOnTestDb2Attr.getValues()); assertFalse(pwdOnTestDb2Attr.getValues().isEmpty()); - final String pwdOnTestDb2 = pwdOnTestDb2Attr.getValues().iterator().next(); + String pwdOnTestDb2 = pwdOnTestDb2Attr.getValues().iterator().next(); // 2. request to change password only on testdb (no Syncope, no testdb2) UserPatch userPatch = new UserPatch(); @@ -503,7 +503,7 @@ public class UserIssuesITCase extends AbstractITCase { // 3c. verify that password *has* changed on testdb userOnDb = resourceService.readConnObject(RESOURCE_NAME_TESTDB, AnyTypeKind.USER.name(), userTO.getKey()); - final AttrTO pwdOnTestDbAttrAfter = userOnDb.getAttr(OperationalAttributes.PASSWORD_NAME).get(); + AttrTO pwdOnTestDbAttrAfter = userOnDb.getAttr(OperationalAttributes.PASSWORD_NAME).get(); assertNotNull(pwdOnTestDbAttrAfter); assertNotNull(pwdOnTestDbAttrAfter.getValues()); assertFalse(pwdOnTestDbAttrAfter.getValues().isEmpty()); @@ -511,7 +511,7 @@ public class UserIssuesITCase extends AbstractITCase { // 3d. verify that password hasn't changed on testdb2 userOnDb2 = resourceService.readConnObject(RESOURCE_NAME_TESTDB2, AnyTypeKind.USER.name(), userTO.getKey()); - final AttrTO pwdOnTestDb2AttrAfter = userOnDb2.getAttr(OperationalAttributes.PASSWORD_NAME).get(); + AttrTO pwdOnTestDb2AttrAfter = userOnDb2.getAttr(OperationalAttributes.PASSWORD_NAME).get(); assertNotNull(pwdOnTestDb2AttrAfter); assertNotNull(pwdOnTestDb2AttrAfter.getValues()); assertFalse(pwdOnTestDb2AttrAfter.getValues().isEmpty()); @@ -522,7 +522,7 @@ public class UserIssuesITCase extends AbstractITCase { public void issueSYNCOPE136AES() { // 1. read configured cipher algorithm in order to be able to restore it at the end of test AttrTO pwdCipherAlgo = configurationService.get("password.cipher.algorithm"); - final String origpwdCipherAlgo = pwdCipherAlgo.getValues().get(0); + String origpwdCipherAlgo = pwdCipherAlgo.getValues().get(0); // 2. set AES password cipher algorithm pwdCipherAlgo.getValues().set(0, "AES"); @@ -641,6 +641,7 @@ public class UserIssuesITCase extends AbstractITCase { userTO = createUser(userTO).getEntity(); assertTrue(userTO.getResources().contains(RESOURCE_NAME_LDAP)); + assertNotNull(resourceService.readConnObject(RESOURCE_NAME_LDAP, AnyTypeKind.USER.name(), userTO.getKey())); // 3. read group on resource, check that user DN is included in uniqueMember ConnObjectTO connObj = resourceService.readConnObject( @@ -664,7 +665,12 @@ public class UserIssuesITCase extends AbstractITCase { assertFalse(connObj.getAttr("uniqueMember").get().getValues(). contains("uid=" + userTO.getUsername() + ",ou=people,o=isp")); - // 6. restore original resource-ldap group mapping + // 6. user has still the LDAP resource assigned - SYNCOPE-1222 + userTO = userService.read(userTO.getKey()); + assertTrue(userTO.getResources().contains(RESOURCE_NAME_LDAP)); + assertNotNull(resourceService.readConnObject(RESOURCE_NAME_LDAP, AnyTypeKind.USER.name(), userTO.getKey())); + + // 7. restore original resource-ldap group mapping ldap.getProvision(AnyTypeKind.GROUP.name()).get().getMapping().getItems().stream(). filter(item -> ("uniqueMember".equals(item.getExtAttrName()))). forEachOrdered(item -> { @@ -781,16 +787,9 @@ public class UserIssuesITCase extends AbstractITCase { operation(PatchOperation.ADD_REPLACE).value(RESOURCE_NAME_TESTDB).build()); ProvisioningResult<UserTO> result = updateUser(userPatch); - List<PropagationStatus> propagationStatuses = result.getPropagationStatuses(); - PropagationStatus ws1PropagationStatus = null; - if (propagationStatuses != null) { - for (PropagationStatus propStatus : propagationStatuses) { - if (RESOURCE_NAME_WS1.equals(propStatus.getResource())) { - ws1PropagationStatus = propStatus; - break; - } - } - } + PropagationStatus ws1PropagationStatus = result.getPropagationStatuses().stream(). + filter(propStatus -> RESOURCE_NAME_WS1.equals(propStatus.getResource())). + findFirst().orElse(null); assertNotNull(ws1PropagationStatus); assertEquals(RESOURCE_NAME_WS1, ws1PropagationStatus.getResource()); assertNotNull(ws1PropagationStatus.getFailureReason()); @@ -1008,7 +1007,7 @@ public class UserIssuesITCase extends AbstractITCase { assertEquals(1, user.getResources().size()); // 4. Check that the DB resource has the correct password - final JdbcTemplate jdbcTemplate = new JdbcTemplate(testDataSource); + JdbcTemplate jdbcTemplate = new JdbcTemplate(testDataSource); String value = jdbcTemplate.queryForObject( "SELECT PASSWORD FROM test WHERE ID=?", String.class, user.getUsername()); assertEquals(Encryptor.getInstance().encode("security123", CipherAlgorithm.SHA1), value.toUpperCase()); @@ -1396,7 +1395,7 @@ public class UserIssuesITCase extends AbstractITCase { // 3. verify that dynamic membership is set and that resource is consequently assigned user = created.getEntity(); - final String groupKey = group.getKey(); + String groupKey = group.getKey(); assertTrue(user.getDynMemberships().stream().anyMatch(m -> m.getGroupKey().equals(groupKey))); assertTrue(user.getResources().contains(RESOURCE_NAME_TESTDB));
