You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@syncope.apache.org by il...@apache.org on 2017/10/11 10:59:16 UTC
[3/3] syncope git commit: [SYNCOPE-1222] Better resource management
in case of membership removal
[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ò <il...@apache.org>
Authored: Wed Oct 11 12:59:04 2017 +0200
Committer: Francesco Chicchiriccò <il...@apache.org>
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("issue280@syncope.apache.org");
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());