You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by an...@apache.org on 2021/01/20 13:15:43 UTC

[sling-org-apache-sling-feature-cpconverter] 01/01: SLING-9975 : RepPolicyEntryHandler ignores restrictions defined in jackrabbit2 format SLING-9957 : Hardcoded list of restriction names in RepPolicyEntryHandler.RepPolicyParser

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

angela pushed a commit to branch SLING-9957_SLING-9975
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git

commit 6afb821b7449e73bbb6031044a581c7eac74d091
Author: angela <an...@adobe.com>
AuthorDate: Wed Jan 20 14:15:24 2021 +0100

    SLING-9975 : RepPolicyEntryHandler ignores restrictions defined in jackrabbit2 format
    SLING-9957 : Hardcoded list of restriction names in RepPolicyEntryHandler.RepPolicyParser
---
 .../cpconverter/handlers/AbstractPolicyParser.java | 22 ++++++----
 .../handlers/RepPolicyEntryHandler.java            |  8 ++--
 .../handlers/RepPrincipalPolicyEntryHandler.java   | 16 +++++---
 .../handlers/RepPolicyEntryHandlerTest.java        | 48 +++++++++++++++-------
 .../RepPrincipalPolicyEntryHandlerTest.java        |  2 +-
 .../feature/cpconverter/handlers/TestUtils.java    |  1 +
 .../jr2restrictions/.content.xml}                  | 14 +------
 .../jr2restrictions/_rep_policy.xml}               | 17 ++++----
 .../services/random2/_rep_principalPolicy.xml      |  5 ++-
 9 files changed, 77 insertions(+), 56 deletions(-)

diff --git a/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractPolicyParser.java b/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractPolicyParser.java
index 31cd85e..109a3bb 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractPolicyParser.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractPolicyParser.java
@@ -34,9 +34,7 @@ abstract class AbstractPolicyParser extends AbstractJcrNodeParser<Boolean> {
 
     static final String REP_RESTRICTIONS = "rep:Restrictions";
     static final String REP_PRINCIPAL_NAME = "rep:principalName";
-
-    private static final String REP_PRIVILEGES = "rep:privileges";
-    private static final String[] RESTRICTIONS = new String[] { "rep:glob", "rep:ntNames", "rep:prefixes", "rep:itemNames" };
+    static final String REP_PRIVILEGES = "rep:privileges";
 
     private static final Pattern typeIndicatorPattern = Pattern.compile("\\{[^\\}]+\\}\\[(.+)\\]");
 
@@ -69,16 +67,22 @@ abstract class AbstractPolicyParser extends AbstractJcrNodeParser<Boolean> {
         return expression;
     }
 
-    static void addRestrictions(@NotNull AccessControlEntry ace, @NotNull Attributes attributes) {
-        for (String restriction : RESTRICTIONS) {
-            String v = extractValue(attributes.getValue(restriction));
-
-            if (v != null && !v.isEmpty()) {
-                ace.addRestriction(restriction + ',' + v);
+    void addRestrictions(@NotNull AccessControlEntry ace, @NotNull Attributes attributes) {
+        for (int i = 0; i < attributes.getLength(); i++) {
+            String name = attributes.getQName(i);
+            if (isRestriction(name)) {
+                String v = extractValue(attributes.getValue(name));
+                if (v != null && !v.isEmpty()) {
+                    ace.addRestriction(name + ',' + v);
+                }
             }
         }
     }
 
+    boolean isRestriction(@NotNull String attributeName) {
+        return !(REP_PRINCIPAL_NAME.equals(attributeName) || REP_PRIVILEGES.equals(attributeName) || attributeName.startsWith("jcr:"));
+    }
+
     AccessControlEntry createEntry(boolean isAllow, @NotNull Attributes attributes) {
         return new AccessControlEntry(isAllow, Objects.requireNonNull(extractValue(attributes.getValue(REP_PRIVILEGES))), repositoryPath);
     }
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandler.java b/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandler.java
index fb5cea6..c87b65e 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandler.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandler.java
@@ -72,11 +72,13 @@ public class RepPolicyEntryHandler extends AbstractPolicyEntryHandler {
                 String primaryType = attributes.getValue(JCR_PRIMARYTYPE);
                 if (REP_GRANT_ACE.equals(primaryType) || REP_DENY_ACE.equals(primaryType)) {
                     String principalName = attributes.getValue(REP_PRINCIPAL_NAME);
-                    AccessControlEntry acl = createEntry(operations.get(primaryType), attributes);
+                    AccessControlEntry ace = createEntry(operations.get(primaryType), attributes);
+                    // handle restrictions added in jr2 format (i.e. not located below rep:restrictions node)
+                    addRestrictions(ace, attributes);
 
-                    processCurrentAcl = aclManager.addAcl(principalName, acl);
+                    processCurrentAcl = aclManager.addAcl(principalName, ace);
                     if (processCurrentAcl) {
-                        acls.add(acl);
+                        acls.add(ace);
                     } else {
                         hasRejectedNodes = true;
                     }
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPrincipalPolicyEntryHandler.java b/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPrincipalPolicyEntryHandler.java
index 1872c56..73fe351 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPrincipalPolicyEntryHandler.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPrincipalPolicyEntryHandler.java
@@ -45,10 +45,6 @@ public final class RepPrincipalPolicyEntryHandler extends AbstractPolicyEntryHan
 
         private static final String REP_RESTRICTIONS = "rep:Restrictions";
 
-        private static final String REP_PRINCIPAL_NAME = "rep:principalName";
-
-        private static final String REP_PRIVILEGES = "rep:privileges";
-
         private static final String REP_PRINCIPAL_POLICY = "rep:PrincipalPolicy";
 
         private static final String REP_PRINCIPAL_ENTRY = "rep:PrincipalEntry";
@@ -84,7 +80,8 @@ public final class RepPrincipalPolicyEntryHandler extends AbstractPolicyEntryHan
                     RepoPath effectivePath = new RepoPath(attributes.getValue(REP_EFFECTIVE_PATH));
 
                     AccessControlEntry ace = new AccessControlEntry(true, privileges, effectivePath, true);
-
+                    // NOTE: nt-definition doesn't allow for jr2-stype restrictions defined right below the entry.
+                    // instead always requires rep:restrictions child node
                     processCurrentAcl = aclManager.addAcl(principalName, ace);
                     if (processCurrentAcl) {
                         aces.add(ace);
@@ -115,5 +112,14 @@ public final class RepPrincipalPolicyEntryHandler extends AbstractPolicyEntryHan
                 handler.endElement(uri, localName, qName);
             }
         }
+
+        @Override
+        boolean isRestriction(@NotNull String attributeName) {
+            if ((REP_EFFECTIVE_PATH.equals(attributeName))) {
+                return false;
+            } else {
+                return super.isRestriction(attributeName);
+            }
+        }
     }
 }
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 421f627..9523b51 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
@@ -32,7 +32,9 @@ import org.junit.Before;
 import org.junit.Test;
 
 import java.io.ByteArrayOutputStream;
+import java.io.InputStream;
 import java.io.StringReader;
+import java.security.acl.Acl;
 import java.util.List;
 
 import static org.junit.Assert.assertEquals;
@@ -43,15 +45,18 @@ import static org.junit.Assert.assertTrue;
 public final class RepPolicyEntryHandlerTest {
 
     private RepPolicyEntryHandler handler;
+    private AclManager aclManager;
 
     @Before
     public void setUp() {
         handler = new RepPolicyEntryHandler();
+        aclManager = new DefaultAclManager();
     }
 
     @After
     public void tearDown() {
         handler = null;
+        aclManager = null;
     }
 
     @Test
@@ -198,18 +203,16 @@ public final class RepPolicyEntryHandlerTest {
 
     @Test
     public void parseEmptyAcl() throws Exception {
-        Extension extension = TestUtils.createRepoInitExtension(handler, new DefaultAclManager(), "/jcr_root/home/users/system/asd/_rep_policy.xml", getClass().getResourceAsStream("/jcr_root/home/users/system/asd/_rep_policy.xml".substring(1)), new ByteArrayOutputStream());
+        Extension extension = TestUtils.createRepoInitExtension(handler, aclManager, "/jcr_root/home/users/system/asd/_rep_policy.xml", getClass().getResourceAsStream("/jcr_root/home/users/system/asd/_rep_policy.xml".substring(1)), new ByteArrayOutputStream());
         assertNull(extension);
     }
 
     @Test
     public void policyAtAuthorizableFolder() throws Exception {
         SystemUser s1 = new SystemUser("service1", new RepoPath("/home/users/system/services/random1"), new RepoPath("/home/users/system/services"));
-
-        AclManager aclManager = new DefaultAclManager();
         aclManager.addSystemUser(s1);
 
-        ParseResult result = parseAndSetRepoInit("/jcr_root/home/groups/g/_rep_policy.xml", aclManager);
+        ParseResult result = parseAndSetRepoinit("/jcr_root/home/groups/g/_rep_policy.xml", aclManager);
         Extension repoinitExtension = result.getRepoinitExtension();
 
         String expected =
@@ -225,12 +228,10 @@ public final class RepPolicyEntryHandlerTest {
     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"));
-
-        AclManager aclManager = new DefaultAclManager();
         aclManager.addSystemUser(s1);
         aclManager.addGroup(gr);
 
-        parseAndSetRepoInit("/jcr_root/home/groups/g/HjDnfdMCjekaF4jhhUvO/_rep_policy.xml", aclManager);
+        parseAndSetRepoinit("/jcr_root/home/groups/g/HjDnfdMCjekaF4jhhUvO/_rep_policy.xml", aclManager);
     }
 
     @Test(expected = IllegalStateException.class)
@@ -238,11 +239,10 @@ public final class RepPolicyEntryHandlerTest {
         SystemUser s1 = new SystemUser("service1", new RepoPath("/home/users/system/services/random1"), new RepoPath("/home/users/system/services"));
         Group gr = new Group("testgroup3", new RepoPath("/home/groups/g/ouStmkrzT9wCEhtMD9sT"), new RepoPath("/home/groups/g"));
 
-        AclManager aclManager = new DefaultAclManager();
         aclManager.addSystemUser(s1);
         aclManager.addGroup(gr);
 
-        parseAndSetRepoInit("/jcr_root/home/groups/g/ouStmkrzT9wCEhtMD9sT/profile/_rep_policy.xml", aclManager);
+        parseAndSetRepoinit("/jcr_root/home/groups/g/ouStmkrzT9wCEhtMD9sT/profile/_rep_policy.xml", aclManager);
     }
 
     @Test(expected = IllegalStateException.class)
@@ -250,11 +250,32 @@ public final class RepPolicyEntryHandlerTest {
         SystemUser s1 = new SystemUser("service1", new RepoPath("/home/users/system/services/random1"), new RepoPath("/home/users/system/services"));
         User user = new User("author", new RepoPath("/home/users/a/author"), new RepoPath("/home/users/a"));
 
-        AclManager aclManager = new DefaultAclManager();
         aclManager.addSystemUser(s1);
         aclManager.addUser(user);
 
-        parseAndSetRepoInit("/jcr_root/home/users/a/author/_rep_policy.xml", aclManager);
+        parseAndSetRepoinit("/jcr_root/home/users/a/author/_rep_policy.xml", aclManager);
+    }
+
+    @Test
+    public void parseJackrabbit2Restrictions() throws Exception {
+        RepoPath repoPath = new RepoPath("/home/users/system/services/random1");
+        SystemUser systemUser = new SystemUser("service1", repoPath, new RepoPath("/home/users/system/services"));
+        aclManager.addSystemUser(systemUser);
+
+        String path = "/jcr_root/asd/jr2restrictions/_rep_policy.xml";
+        Extension repoinitExtension = parseAndSetRepoinit(path, aclManager).getRepoinitExtension();
+        String expected =
+                "create service user service1 with path /home/users/system/services" + System.lineSeparator() +
+                "set ACL for service1\n" +
+                "allow jcr:read on /asd/jr2restrictions restriction(rep:glob,*/subtree/*,sling:customRestriction,sling:value1,sling:value2)\n" +
+                "end\n";
+
+        String actual = repoinitExtension.getText();
+        assertEquals(expected, actual);
+
+        RepoInitParser repoInitParser = new RepoInitParserService();
+        List<Operation> operations = repoInitParser.parse(new StringReader(actual));
+        assertFalse(operations.isEmpty());
     }
 
     private ParseResult parseAndSetRepoinit(String...systemUsersNames) throws Exception {
@@ -271,15 +292,14 @@ public final class RepPolicyEntryHandlerTest {
 
     private ParseResult parseAndSetRepoinit(@NotNull SystemUser...systemUsers) throws Exception {
         String path = "/jcr_root/home/users/system/asd/_rep_policy.xml";
-        AclManager aclManager = new DefaultAclManager();
         for (SystemUser systemUser : systemUsers) {
             aclManager.addSystemUser(systemUser);
         }
-        return parseAndSetRepoInit(path, aclManager);
+        return parseAndSetRepoinit(path, aclManager);
     }
 
     @NotNull
-    private ParseResult parseAndSetRepoInit(@NotNull String path, @NotNull AclManager aclManager) throws Exception {
+    private ParseResult parseAndSetRepoinit(@NotNull String path, @NotNull AclManager aclManager) throws Exception {
         ByteArrayOutputStream baos = new ByteArrayOutputStream();
         return new ParseResult(TestUtils.createRepoInitExtension(handler, aclManager, path, getClass().getResourceAsStream(path.substring(1)), baos), new String(baos.toByteArray()));
     }
diff --git a/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPrincipalPolicyEntryHandlerTest.java b/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPrincipalPolicyEntryHandlerTest.java
index fa4e04e..e9d2c14 100644
--- a/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPrincipalPolicyEntryHandlerTest.java
+++ b/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPrincipalPolicyEntryHandlerTest.java
@@ -87,7 +87,7 @@ public final class RepPrincipalPolicyEntryHandlerTest {
         String expected =
                 "create service user service2 with path /home/users/system/services" + System.lineSeparator() +
                         "set principal ACL for service2\n" +
-                        "allow jcr:read on /asd/public restriction(rep:ntNames,nt:folder,sling:Folder)\n" +
+                        "allow jcr:read on /asd/public restriction(rep:ntNames,nt:folder,sling:Folder,sling:customRestriction,customRestrictionValue)\n" +
                         "end\n";
 
         String actual = repoinitExtension.getText();
diff --git a/src/test/java/org/apache/sling/feature/cpconverter/handlers/TestUtils.java b/src/test/java/org/apache/sling/feature/cpconverter/handlers/TestUtils.java
index a950260..26a7b1b 100644
--- a/src/test/java/org/apache/sling/feature/cpconverter/handlers/TestUtils.java
+++ b/src/test/java/org/apache/sling/feature/cpconverter/handlers/TestUtils.java
@@ -49,6 +49,7 @@ class TestUtils {
     static Extension createRepoInitExtension(@NotNull EntryHandler handler, @NotNull AclManager aclManager, @NotNull String path, @NotNull InputStream is) throws Exception {
         return createRepoInitExtension(handler, aclManager, path, is, null);
     }
+
     static Extension createRepoInitExtension(@NotNull EntryHandler handler, @NotNull AclManager aclManager, @NotNull String path, @NotNull InputStream is, @Nullable OutputStream out) throws Exception {
         Archive archive = mock(Archive.class);
         Archive.Entry entry = mock(Archive.Entry.class);
diff --git a/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/system/services/random2/_rep_principalPolicy.xml b/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/asd/jr2restrictions/.content.xml
similarity index 62%
copy from src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/system/services/random2/_rep_principalPolicy.xml
copy to src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/asd/jr2restrictions/.content.xml
index 38f5335..0589bcb 100644
--- a/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/system/services/random2/_rep_principalPolicy.xml
+++ b/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/asd/jr2restrictions/.content.xml
@@ -15,15 +15,5 @@
  License for the specific language governing permissions and limitations under
  the License.
 -->
-<jcr:root xmlns:jcr="http://www.jcp.org/jcr/1.0" xmlns:rep="internal"
-          jcr:primaryType="rep:PrincipalPolicy"
-          rep:principalName="service2">
-    <entry0
-            jcr:primaryType="rep:PrincipalEntry"
-            rep:privileges="{Name}[jcr:read]"
-            rep:effectivePath="/asd/public">
-        <rep:restrictions
-                jcr:primaryType="rep:Restrictions"
-                rep:ntNames="{Name}[nt:folder,sling:Folder]"/>
-    </entry0>
-</jcr:root>
+<jcr:root xmlns:sling="http://sling.apache.org/jcr/sling/1.0" xmlns:jcr="http://www.jcp.org/jcr/1.0"
+    jcr:primaryType="sling:Folder"/>
diff --git a/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/system/services/random2/_rep_principalPolicy.xml b/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/asd/jr2restrictions/_rep_policy.xml
similarity index 72%
copy from src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/system/services/random2/_rep_principalPolicy.xml
copy to src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/asd/jr2restrictions/_rep_policy.xml
index 38f5335..852cb01 100644
--- a/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/system/services/random2/_rep_principalPolicy.xml
+++ b/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/asd/jr2restrictions/_rep_policy.xml
@@ -15,15 +15,12 @@
  License for the specific language governing permissions and limitations under
  the License.
 -->
-<jcr:root xmlns:jcr="http://www.jcp.org/jcr/1.0" xmlns:rep="internal"
-          jcr:primaryType="rep:PrincipalPolicy"
-          rep:principalName="service2">
-    <entry0
-            jcr:primaryType="rep:PrincipalEntry"
+<jcr:root xmlns:jcr="http://www.jcp.org/jcr/1.0" xmlns:rep="internal" xmlns:sling="http://sling.apache.org/jcr/sling/1.0"
+          jcr:primaryType="rep:ACL">
+    <allow0
+            jcr:primaryType="rep:GrantACE"
+            rep:principalName="service1"
             rep:privileges="{Name}[jcr:read]"
-            rep:effectivePath="/asd/public">
-        <rep:restrictions
-                jcr:primaryType="rep:Restrictions"
-                rep:ntNames="{Name}[nt:folder,sling:Folder]"/>
-    </entry0>
+            rep:glob="{Name}[*/subtree/*]"
+            sling:customRestriction="{Name}[sling:value1,sling:value2]"/>
 </jcr:root>
diff --git a/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/system/services/random2/_rep_principalPolicy.xml b/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/system/services/random2/_rep_principalPolicy.xml
index 38f5335..d2da38d 100644
--- a/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/system/services/random2/_rep_principalPolicy.xml
+++ b/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/system/services/random2/_rep_principalPolicy.xml
@@ -15,7 +15,7 @@
  License for the specific language governing permissions and limitations under
  the License.
 -->
-<jcr:root xmlns:jcr="http://www.jcp.org/jcr/1.0" xmlns:rep="internal"
+<jcr:root xmlns:jcr="http://www.jcp.org/jcr/1.0" xmlns:rep="internal" xmlns:sling="http://sling.apache.org/jcr/sling/1.0"
           jcr:primaryType="rep:PrincipalPolicy"
           rep:principalName="service2">
     <entry0
@@ -24,6 +24,7 @@
             rep:effectivePath="/asd/public">
         <rep:restrictions
                 jcr:primaryType="rep:Restrictions"
-                rep:ntNames="{Name}[nt:folder,sling:Folder]"/>
+                rep:ntNames="{Name}[nt:folder,sling:Folder]"
+                sling:customRestriction="customRestrictionValue"/>
     </entry0>
 </jcr:root>