You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by pa...@apache.org on 2021/01/19 10:19:44 UTC

[sling-org-apache-sling-feature-cpconverter] branch master updated: SLING-9953 : ACEs on/below user nodes are ignored upon conversion (treat groups the same as regular users and abort converter if ace points to them) (#55)

This is an automated email from the ASF dual-hosted git repository.

pauls pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git


The following commit(s) were added to refs/heads/master by this push:
     new 2a2bea6  SLING-9953 : ACEs on/below user nodes are ignored upon conversion (treat groups the same as regular users and abort converter if ace points to them) (#55)
2a2bea6 is described below

commit 2a2bea682da849504c56d41d3f37b587529d207f
Author: anchela <an...@apache.org>
AuthorDate: Tue Jan 19 11:19:38 2021 +0100

    SLING-9953 : ACEs on/below user nodes are ignored upon conversion (treat groups the same as regular users and abort converter if ace points to them) (#55)
    
    Co-authored-by: angela <an...@adobe.com>
---
 .../accesscontrol/DefaultAclManager.java           | 29 ++++++++--------------
 .../cpconverter/accesscontrol/AclManagerTest.java  | 14 +----------
 .../handlers/RepPolicyEntryHandlerTest.java        | 18 ++------------
 3 files changed, 14 insertions(+), 47 deletions(-)

diff --git a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java
index f79c6d5..aff08fc 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java
@@ -31,18 +31,18 @@ import org.jetbrains.annotations.Nullable;
 import javax.jcr.NamespaceException;
 import java.io.File;
 import java.io.FileInputStream;
-import java.util.Optional;
+import java.util.Collection;
 import java.util.Formatter;
 import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.LinkedHashMap;
 import java.util.Map;
-import java.util.Set;
-import java.util.Collection;
-import java.util.Objects;
 import java.util.Map.Entry;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -134,20 +134,13 @@ public class DefaultAclManager implements AclManager {
             }
         }
 
-        for (Group group : groups) {
-            if (aclStartsWith(group.getPath())) {
-                formatter.format("create group %s with path %s%n", group.getId(), group.getIntermediatePath());
-            }
-            if (aclIsBelow(group.getPath())) {
-                throw new IllegalStateException("Detected policy on subpath of group: " + group);
-            }
-        }
-
-        for (User user : users) {
-            if (aclStartsWith(user.getPath())) {
-                throw new IllegalStateException("Detected policy on user: " + user);
+        // abort the conversion if an access control entry takes effect at or below a user/group which is not
+        // created by repo-init statements generated here.
+        Stream.concat(groups.stream(), users.stream()).forEach(abstractUser -> {
+            if (aclStartsWith(abstractUser.getPath())) {
+                throw new IllegalStateException("Detected policy on user: " + abstractUser);
             }
-        }
+        });
     }
 
     @NotNull
diff --git a/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/AclManagerTest.java b/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/AclManagerTest.java
index 8e74167..bddc611 100644
--- a/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/AclManagerTest.java
+++ b/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/AclManagerTest.java
@@ -200,7 +200,7 @@ public class AclManagerTest {
         assertFalse(operations.isEmpty());
     }
 
-    @Test
+    @Test(expected = IllegalStateException.class)
     public void testGroupHandlingWithGroupUsed() {
         aclManager.addSystemUser(new SystemUser("sys-usr", new RepoPath("/home/users/system/foo"), new RepoPath("/home/users/system")));
 
@@ -216,18 +216,6 @@ public class AclManagerTest {
         aclManager.addRepoinitExtension(Collections.singletonList(assembler), fm);
 
         Extension repoinitExtension = feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT);
-        assertNotNull(repoinitExtension);
-
-        String expected =
-                "create service user sys-usr with path /home/users/system" + System.lineSeparator() +
-                        "create group test with path /home/groups/test" + System.lineSeparator() +
-                        "set ACL for sys-usr" + System.lineSeparator() +
-                        "allow jcr:read on home(test)" + System.lineSeparator() +
-                        "end" + System.lineSeparator();
-
-        String actual = repoinitExtension.getText();
-        assertEquals(expected, actual);
-
     }
 
     @Test
diff --git a/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandlerTest.java b/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandlerTest.java
index c70d39d..421f627 100644
--- a/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandlerTest.java
+++ b/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandlerTest.java
@@ -221,7 +221,7 @@ public final class RepPolicyEntryHandlerTest {
         assertTrue(result.getExcludedAcls().isEmpty());
     }
 
-    @Test
+    @Test(expected = IllegalStateException.class)
     public void policyAtGroupNode() throws Exception {
         SystemUser s1 = new SystemUser("service1", new RepoPath("/home/users/system/services/random1"), new RepoPath("/home/users/system/services"));
         Group gr = new Group("testgroup", new RepoPath("/home/groups/g/HjDnfdMCjekaF4jhhUvO"), new RepoPath("/home/groups/g"));
@@ -230,21 +230,7 @@ public final class RepPolicyEntryHandlerTest {
         aclManager.addSystemUser(s1);
         aclManager.addGroup(gr);
 
-        ParseResult result = parseAndSetRepoInit("/jcr_root/home/groups/g/HjDnfdMCjekaF4jhhUvO/_rep_policy.xml", aclManager);
-        Extension repoinitExtension = result.getRepoinitExtension();
-
-        String expected =
-                "create service user service1 with path /home/users/system/services" + System.lineSeparator() +
-                "create group testgroup with path /home/groups/g" + System.lineSeparator() +
-                "set ACL for service1" + System.lineSeparator() +
-                "allow jcr:read on home(testgroup)" + System.lineSeparator() +
-                "end" + System.lineSeparator();
-        assertEquals(expected, repoinitExtension.getText());
-
-        String expectedExclusions = "<?xml version=\"1.0\" encoding=\"UTF-8\"?><jcr:root xmlns:jcr=\"http://www.jcp.org/jcr/1.0\" xmlns:rep=\"internal\" jcr:primaryType=\"rep:ACL\">\n" +
-                "    <allow1 jcr:primaryType=\"rep:GrantACE\" rep:principalName=\"testgroup\" rep:privileges=\"{Name}[jcr:read]\"/>\n" +
-                "</jcr:root>\n";
-        assertEquals(expectedExclusions, result.getExcludedAcls());
+        parseAndSetRepoInit("/jcr_root/home/groups/g/HjDnfdMCjekaF4jhhUvO/_rep_policy.xml", aclManager);
     }
 
     @Test(expected = IllegalStateException.class)