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 <[email protected]> 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>
