syncope-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ilgro...@apache.org
Subject [3/3] syncope git commit: [SYNCOPE-1222] Better resource management in case of membership removal
Date Wed, 11 Oct 2017 10:59:16 GMT
[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ò <ilgrosso@apache.org>
Authored: Wed Oct 11 12:59:04 2017 +0200
Committer: Francesco Chicchiriccò <ilgrosso@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());


Mime
View raw message