[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/5ed622cd Tree: http://git-wip-us.apache.org/repos/asf/syncope/tree/5ed622cd Diff: http://git-wip-us.apache.org/repos/asf/syncope/diff/5ed622cd Branch: refs/heads/2_0_X Commit: 5ed622cdc04803bf1b3254efbea5debaa465604c Parents: fe774bc Author: Francesco Chicchiriccò <[email protected]> Authored: Wed Oct 11 12:59:04 2017 +0200 Committer: Francesco Chicchiriccò <[email protected]> Committed: Wed Oct 11 12:59:04 2017 +0200 ---------------------------------------------------------------------- .../java/data/AnyObjectDataBinderImpl.java | 46 +++++++++++++++----- .../java/data/UserDataBinderImpl.java | 46 +++++++++++++++----- .../syncope/fit/core/UserIssuesITCase.java | 30 ++++++++----- 3 files changed, 90 insertions(+), 32 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/syncope/blob/5ed622cd/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 d77d578..6f03191 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; @@ -281,9 +282,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 for (RelationshipPatch patch : anyObjectPatch.getRelationships()) { if (patch.getRelationshipTO() != null) { @@ -296,9 +294,6 @@ public class AnyObjectDataBinderImpl extends AbstractAnyDataBinder implements An if (relationship != null) { anyObject.getRelationships().remove(relationship); relationship.setLeftEnd(null); - - toBeDeprovisioned.addAll( - anyObjectDAO.findAllResourceKeys(relationship.getRightEnd().getKey())); } if (patch.getOperation() == PatchOperation.ADD_REPLACE) { @@ -324,8 +319,6 @@ public class AnyObjectDataBinderImpl extends AbstractAnyDataBinder implements An relationship.setLeftEnd(anyObject); anyObject.add(relationship); - - toBeProvisioned.addAll(anyObjectDAO.findAllResourceKeys(otherEnd.getKey())); } else { LOG.error("{} cannot be assigned to {}", otherEnd, anyObject); @@ -340,7 +333,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<>(); + for (ExternalResource resource : anyObject.getResources()) { + reasons.put(resource.getKey(), new HashSet<>(Collections.singleton(anyObject.getKey()))); + } + for (String group : anyObjectDAO.findAllGroupKeys(anyObject)) { + for (String resource : groupDAO.findAllResourceKeys(group)) { + if (!reasons.containsKey(resource)) { + reasons.put(resource, new HashSet<String>()); + } + reasons.get(resource).add(group); + } + } + + Set<String> toBeDeprovisioned = new HashSet<>(); + Set<String> toBeProvisioned = new HashSet<>(); + SyncopeClientException invalidValues = SyncopeClientException.build(ClientExceptionType.InvalidValues); // memberships @@ -356,7 +367,15 @@ public class AnyObjectDataBinderImpl extends AbstractAnyDataBinder implements An } if (membPatch.getOperation() == PatchOperation.DELETE) { - toBeDeprovisioned.addAll(groupDAO.findAllResourceKeys(membership.getRightEnd().getKey())); + for (String resource : groupDAO.findAllResourceKeys(membership.getRightEnd().getKey())) { + if (reasons.containsKey(resource)) { + reasons.get(resource).remove(membership.getRightEnd().getKey()); + + if (reasons.get(resource).contains(anyObject.getKey())) { + toBeProvisioned.add(resource); + } + } + } } } @@ -412,10 +431,17 @@ public class AnyObjectDataBinderImpl extends AbstractAnyDataBinder implements An } } + // finalize resource management + for (Map.Entry<String, Set<String>> entry : reasons.entrySet()) { + if (entry.getValue().isEmpty()) { + 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/5ed622cd/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 3213da0..d80673c 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; @@ -379,9 +380,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 for (RelationshipPatch patch : userPatch.getRelationships()) { if (patch.getRelationshipTO() != null) { @@ -394,9 +392,6 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat if (relationship != null) { user.getRelationships().remove(relationship); relationship.setLeftEnd(null); - - toBeDeprovisioned.addAll( - anyObjectDAO.findAllResourceKeys(relationship.getRightEnd().getKey())); } if (patch.getOperation() == PatchOperation.ADD_REPLACE) { @@ -410,8 +405,6 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat relationship.setLeftEnd(user); user.add(relationship); - - toBeProvisioned.addAll(anyObjectDAO.findAllResourceKeys(otherEnd.getKey())); } else { LOG.error("{} cannot be assigned to {}", otherEnd, user); @@ -425,7 +418,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<>(); + for (ExternalResource resource : user.getResources()) { + reasons.put(resource.getKey(), new HashSet<>(Collections.singleton(user.getKey()))); + } + for (String group : userDAO.findAllGroupKeys(user)) { + for (String resource : groupDAO.findAllResourceKeys(group)) { + if (!reasons.containsKey(resource)) { + reasons.put(resource, new HashSet<String>()); + } + reasons.get(resource).add(group); + } + } + + Set<String> toBeDeprovisioned = new HashSet<>(); + Set<String> toBeProvisioned = new HashSet<>(); + SyncopeClientException invalidValues = SyncopeClientException.build(ClientExceptionType.InvalidValues); // memberships @@ -442,7 +453,15 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat } if (membPatch.getOperation() == PatchOperation.DELETE) { - toBeDeprovisioned.addAll(groupDAO.findAllResourceKeys(membership.getRightEnd().getKey())); + for (String resource : groupDAO.findAllResourceKeys(membership.getRightEnd().getKey())) { + if (reasons.containsKey(resource)) { + reasons.get(resource).remove(membership.getRightEnd().getKey()); + + if (reasons.get(resource).contains(user.getKey())) { + toBeProvisioned.add(resource); + } + } + } } } @@ -511,10 +530,17 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat } } + // finalize resource management + for (Map.Entry<String, Set<String>> entry : reasons.entrySet()) { + if (entry.getValue().isEmpty()) { + 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/5ed622cd/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 b2cf3d9..716300e 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 @@ -194,7 +194,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(); @@ -364,7 +364,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); @@ -463,23 +463,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); + AttrTO pwdOnTestDbAttr = userOnDb.getAttr(OperationalAttributes.PASSWORD_NAME); 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); + AttrTO pwdOnTestDb2Attr = userOnDb2.getAttr(OperationalAttributes.PASSWORD_NAME); 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(); @@ -500,7 +500,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); + AttrTO pwdOnTestDbAttrAfter = userOnDb.getAttr(OperationalAttributes.PASSWORD_NAME); assertNotNull(pwdOnTestDbAttrAfter); assertNotNull(pwdOnTestDbAttrAfter.getValues()); assertFalse(pwdOnTestDbAttrAfter.getValues().isEmpty()); @@ -508,7 +508,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); + AttrTO pwdOnTestDb2AttrAfter = userOnDb2.getAttr(OperationalAttributes.PASSWORD_NAME); assertNotNull(pwdOnTestDb2AttrAfter); assertNotNull(pwdOnTestDb2AttrAfter.getValues()); assertFalse(pwdOnTestDb2AttrAfter.getValues().isEmpty()); @@ -519,7 +519,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"); @@ -638,6 +638,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( @@ -661,7 +662,12 @@ public class UserIssuesITCase extends AbstractITCase { assertFalse(connObj.getAttr("uniqueMember").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 for (ItemTO item : ldap.getProvision(AnyTypeKind.GROUP.name()).getMapping().getItems()) { if ("uniqueMember".equals(item.getExtAttrName())) { item.setExtAttrName("description"); @@ -986,7 +992,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());
