This is an automated email from the ASF dual-hosted git repository. angela pushed a commit to branch SLING-9956 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git
commit 622e61d0aa7cc9f64230f7aee3a45fc9fa8c9d7a Author: angela <[email protected]> AuthorDate: Wed Jan 13 10:32:15 2021 +0100 SLING-9956 : RepPolicyEntryHandler ignores ACEs on repository level --- .../accesscontrol/DefaultAclManager.java | 37 ++++++--- .../handlers/RepPolicyEntryHandler.java | 8 +- .../handlers/RepRepoPolicyEntryHandler.java | 35 +++++++++ .../sling/feature/cpconverter/shared/RepoPath.java | 8 ++ .../handlers/RepRepoPolicyEntryHandlerTest.java | 87 ++++++++++++++++++++++ .../handlers/SystemUsersEntryHandlerTest.java | 33 +------- .../feature/cpconverter/handlers/TestUtils.java | 68 +++++++++++++++++ .../handlers/jcr_root/_rep_repoPolicy.xml | 24 ++++++ 8 files changed, 256 insertions(+), 44 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 c1f8925..12bb776 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 @@ -162,7 +162,7 @@ public final class DefaultAclManager implements AclManager { // finally add ACLs - addAclStatement(formatter, systemUser.getId(), authorizations); + addAclStatement(formatter, systemUser, authorizations); } private @NotNull Optional<SystemUser> getSystemUser(@NotNull String id) { @@ -187,6 +187,7 @@ public final class DefaultAclManager implements AclManager { } } + @Override public void addPrivilegeDefinitions(@NotNull PrivilegeDefinitions privilegeDefinitions) { this.privilegeDefinitions = privilegeDefinitions; } @@ -239,28 +240,44 @@ public final class DefaultAclManager implements AclManager { return DEFAULT_TYPE; } - private static void addAclStatement(@NotNull Formatter formatter, @NotNull String systemUser, @NotNull List<AccessControlEntry> authorizations) { + private static void addAclStatement(@NotNull Formatter formatter, @NotNull SystemUser systemUser, @NotNull List<AccessControlEntry> authorizations) { if (authorizations.isEmpty()) { return; } - formatter.format("set ACL for %s%n", systemUser); + writeAccessControl(authorizations, "set ACL for %s%n", systemUser, formatter); + } - for (AccessControlEntry authorization : authorizations) { + private static void writeAccessControl(@NotNull List<AccessControlEntry> accessControlEntries, @NotNull String statement, @NotNull SystemUser systemUser, @NotNull Formatter formatter) { + formatter.format(statement, systemUser.getId()); + writeEntries(accessControlEntries, systemUser, formatter); + formatter.format("end%n"); + } + + private static void writeEntries(@NotNull List<AccessControlEntry> accessControlEntries, @NotNull SystemUser systemUser, @NotNull Formatter formatter) { + for (AccessControlEntry entry : accessControlEntries) { formatter.format("%s %s on %s", - authorization.getOperation(), - authorization.getPrivileges(), - authorization.getRepositoryPath()); + entry.getOperation(), + entry.getPrivileges(), + getRepoInitPath(entry.getRepositoryPath(), systemUser)); - if (!authorization.getRestrictions().isEmpty()) { + if (!entry.getRestrictions().isEmpty()) { formatter.format(" restriction(%s)", - String.join(",", authorization.getRestrictions())); + String.join(",", entry.getRestrictions())); } formatter.format("%n"); } + } - formatter.format("end%n"); + @NotNull + private static String getRepoInitPath(@NotNull RepoPath path, @NotNull SystemUser systemUser) { + // FIXME SLING-9953 : add special handing for path pointing to the system-user home or some other user/group home + if (path.isRepositoryPath()) { + return ":repository"; + } else { + return path.toString(); + } } private static void registerPrivileges(@NotNull PrivilegeDefinitions definitions, @NotNull Formatter formatter) { 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 1ac8e97..0cc3296 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 @@ -30,18 +30,22 @@ import java.util.Stack; import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE; -public final class RepPolicyEntryHandler extends AbstractPolicyEntryHandler { +public class RepPolicyEntryHandler extends AbstractPolicyEntryHandler { public RepPolicyEntryHandler() { super("/jcr_root(.*/)_rep_policy.xml"); } + RepPolicyEntryHandler(@NotNull String regex) { + super(regex); + } + @NotNull AbstractPolicyParser createPolicyParser(@NotNull RepoPath repositoryPath, @NotNull AclManager aclManager, @NotNull TransformerHandler handler) { return new RepPolicyParser(repositoryPath, aclManager, handler); } - private static final class RepPolicyParser extends AbstractPolicyParser { + static final class RepPolicyParser extends AbstractPolicyParser { private static final String REP_ACL = "rep:ACL"; private static final String REP_GRANT_ACE = "rep:GrantACE"; diff --git a/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepRepoPolicyEntryHandler.java b/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepRepoPolicyEntryHandler.java new file mode 100644 index 0000000..eec5ec8 --- /dev/null +++ b/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepRepoPolicyEntryHandler.java @@ -0,0 +1,35 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sling.feature.cpconverter.handlers; + +import org.apache.sling.feature.cpconverter.accesscontrol.AclManager; +import org.apache.sling.feature.cpconverter.shared.RepoPath; +import org.jetbrains.annotations.NotNull; + +import javax.xml.transform.sax.TransformerHandler; + +public class RepRepoPolicyEntryHandler extends RepPolicyEntryHandler { + + public RepRepoPolicyEntryHandler() { + super("/jcr_root(/)_rep_repoPolicy.xml"); + } + + @Override + @NotNull AbstractPolicyParser createPolicyParser(@NotNull RepoPath repositoryPath, @NotNull AclManager aclManager, @NotNull TransformerHandler handler) { + return new RepPolicyParser(new RepoPath("") , aclManager, handler); + } +} \ No newline at end of file diff --git a/src/main/java/org/apache/sling/feature/cpconverter/shared/RepoPath.java b/src/main/java/org/apache/sling/feature/cpconverter/shared/RepoPath.java index 6f4be68..a55bfb3 100644 --- a/src/main/java/org/apache/sling/feature/cpconverter/shared/RepoPath.java +++ b/src/main/java/org/apache/sling/feature/cpconverter/shared/RepoPath.java @@ -34,6 +34,7 @@ import java.util.stream.Collectors; */ public class RepoPath implements Comparable<RepoPath>{ private final List<String> path; + private final boolean isRepositoryPath; /** * Construct a Repo Path from a string. The string should separate the path @@ -44,6 +45,8 @@ public class RepoPath implements Comparable<RepoPath>{ */ public RepoPath(@NotNull String path) { path = path.trim(); + isRepositoryPath = path.isEmpty(); + if (path.startsWith("/")) path = path.substring(1); @@ -58,6 +61,7 @@ public class RepoPath implements Comparable<RepoPath>{ */ public RepoPath(@NotNull List<String> list) { this.path = new ArrayList<>(list); + this.isRepositoryPath = false; } @Override @@ -110,6 +114,10 @@ public class RepoPath implements Comparable<RepoPath>{ return l.equals(otherPath.path); } + public boolean isRepositoryPath() { + return isRepositoryPath; + } + @Override public int hashCode() { return Objects.hash(path); diff --git a/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepRepoPolicyEntryHandlerTest.java b/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepRepoPolicyEntryHandlerTest.java new file mode 100644 index 0000000..7f53d64 --- /dev/null +++ b/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepRepoPolicyEntryHandlerTest.java @@ -0,0 +1,87 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to You under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package org.apache.sling.feature.cpconverter.handlers; + +import org.apache.sling.feature.Extension; +import org.apache.sling.feature.ExtensionType; +import org.apache.sling.feature.cpconverter.accesscontrol.AclManager; +import org.apache.sling.feature.cpconverter.accesscontrol.DefaultAclManager; +import org.apache.sling.feature.cpconverter.accesscontrol.SystemUser; +import org.apache.sling.feature.cpconverter.shared.RepoPath; +import org.apache.sling.repoinit.parser.RepoInitParser; +import org.apache.sling.repoinit.parser.impl.RepoInitParserService; +import org.apache.sling.repoinit.parser.operations.Operation; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.StringReader; +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +public class RepRepoPolicyEntryHandlerTest { + + private RepPolicyEntryHandler handler; + + @Before + public void setUp() { + handler = new RepRepoPolicyEntryHandler(); + } + + @After + public void tearDown() { + handler = null; + } + + @Test + public void doesNotMatch() { + assertFalse(handler.matches("/this/is/a/path/not/pointing/to/a/valid/repoPolicy.xml")); + assertFalse(handler.matches("/this/is/a/path/not/pointing/to/a/valid/_rep_repoPolicy.xml")); + } + + @Test + public void matches() { + assertTrue(handler.matches("/jcr_root/_rep_repoPolicy.xml")); + } + + @Test + public void parsePolicy() throws Exception { + String path = "/jcr_root/_rep_repoPolicy.xml"; + AclManager aclManager = new DefaultAclManager(); + aclManager.addSystemUser(new SystemUser("repolevel-service", new RepoPath("/home/users/system/test"), new RepoPath("/home/users/system"))); + Extension repoinitExtension = TestUtils.createRepoInitExtension(handler, aclManager, path, getClass().getResourceAsStream(path.substring(1))); + + assertNotNull(repoinitExtension); + assertEquals(ExtensionType.TEXT, repoinitExtension.getType()); + assertTrue(repoinitExtension.isRequired()); + + String expectedEnd = + "set ACL for repolevel-service" + System.lineSeparator() + + "allow jcr:namespaceManagement on :repository" + System.lineSeparator() + + "end" + System.lineSeparator(); + String actual = repoinitExtension.getText(); + assertTrue(actual.endsWith(expectedEnd)); + + RepoInitParser repoInitParser = new RepoInitParserService(); + List<Operation> operations = repoInitParser.parse(new StringReader(actual)); + assertFalse(operations.isEmpty()); + } +} diff --git a/src/test/java/org/apache/sling/feature/cpconverter/handlers/SystemUsersEntryHandlerTest.java b/src/test/java/org/apache/sling/feature/cpconverter/handlers/SystemUsersEntryHandlerTest.java index 72f88bc..604a1f9 100644 --- a/src/test/java/org/apache/sling/feature/cpconverter/handlers/SystemUsersEntryHandlerTest.java +++ b/src/test/java/org/apache/sling/feature/cpconverter/handlers/SystemUsersEntryHandlerTest.java @@ -21,25 +21,13 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.Mockito.*; -import java.io.File; -import java.util.Arrays; import java.io.StringReader; import java.util.List; -import org.apache.jackrabbit.vault.fs.io.Archive; -import org.apache.jackrabbit.vault.fs.io.Archive.Entry; -import org.apache.sling.feature.ArtifactId; import org.apache.sling.feature.Extension; import org.apache.sling.feature.ExtensionType; -import org.apache.sling.feature.Feature; -import org.apache.sling.feature.cpconverter.ContentPackage2FeatureModelConverter; import org.apache.sling.feature.cpconverter.accesscontrol.DefaultAclManager; -import org.apache.sling.feature.cpconverter.features.DefaultFeaturesManager; -import org.apache.sling.feature.cpconverter.features.FeaturesManager; -import org.apache.sling.feature.cpconverter.vltpkg.VaultPackageAssembler; import org.apache.sling.repoinit.parser.RepoInitParser; import org.apache.sling.repoinit.parser.impl.RepoInitParserService; import org.apache.sling.repoinit.parser.operations.Operation; @@ -114,25 +102,6 @@ public class SystemUsersEntryHandlerTest { } private Extension parseAndSetRepoinit(String path) throws Exception { - Archive archive = mock(Archive.class); - Entry entry = mock(Entry.class); - VaultPackageAssembler packageAssembler = mock(VaultPackageAssembler.class); - - when(archive.openInputStream(entry)).thenReturn(getClass().getResourceAsStream(path.substring(1))); - - Feature feature = new Feature(new ArtifactId("org.apache.sling", "org.apache.sling.cp2fm", "0.0.1", null, null)); - FeaturesManager featuresManager = spy(DefaultFeaturesManager.class); - when(featuresManager.getTargetFeature()).thenReturn(feature); - ContentPackage2FeatureModelConverter converter = spy(ContentPackage2FeatureModelConverter.class); - when(converter.getFeaturesManager()).thenReturn(featuresManager); - when(converter.getAclManager()).thenReturn(new DefaultAclManager()); - - systemUsersEntryHandler.handle(path, archive, entry, converter); - - when(packageAssembler.getEntry(anyString())).thenReturn(new File("itdoesnotexist")); - - converter.getAclManager().addRepoinitExtension(Arrays.asList(packageAssembler), featuresManager); - return feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT); + return TestUtils.createRepoInitExtension(systemUsersEntryHandler, new DefaultAclManager(), path, getClass().getResourceAsStream(path.substring(1))); } - } 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 new file mode 100644 index 0000000..9e674e0 --- /dev/null +++ b/src/test/java/org/apache/sling/feature/cpconverter/handlers/TestUtils.java @@ -0,0 +1,68 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sling.feature.cpconverter.handlers; + +import org.apache.jackrabbit.vault.fs.io.Archive; +import org.apache.sling.feature.ArtifactId; +import org.apache.sling.feature.Extension; +import org.apache.sling.feature.Feature; +import org.apache.sling.feature.cpconverter.ContentPackage2FeatureModelConverter; +import org.apache.sling.feature.cpconverter.accesscontrol.AclManager; +import org.apache.sling.feature.cpconverter.features.DefaultFeaturesManager; +import org.apache.sling.feature.cpconverter.features.FeaturesManager; +import org.apache.sling.feature.cpconverter.vltpkg.VaultPackageAssembler; +import org.jetbrains.annotations.NotNull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.File; +import java.io.InputStream; +import java.util.Arrays; + +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; + +class TestUtils { + + private static final Logger log = LoggerFactory.getLogger(TestUtils.class); + + private TestUtils() {} + + static Extension createRepoInitExtension(@NotNull EntryHandler handler, @NotNull AclManager aclManager, @NotNull String path, @NotNull InputStream is) throws Exception { + Archive archive = mock(Archive.class); + Archive.Entry entry = mock(Archive.Entry.class); + VaultPackageAssembler packageAssembler = mock(VaultPackageAssembler.class); + + when(archive.openInputStream(entry)).thenReturn(is); + + Feature feature = new Feature(new ArtifactId("org.apache.sling", "org.apache.sling.cp2fm", "0.0.1", null, null)); + FeaturesManager featuresManager = spy(DefaultFeaturesManager.class); + when(featuresManager.getTargetFeature()).thenReturn(feature); + ContentPackage2FeatureModelConverter converter = spy(ContentPackage2FeatureModelConverter.class); + when(converter.getFeaturesManager()).thenReturn(featuresManager); + when(converter.getAclManager()).thenReturn(aclManager); + + handler.handle(path, archive, entry, converter); + + when(packageAssembler.getEntry(anyString())).thenReturn(new File("itdoesnotexist")); + + converter.getAclManager().addRepoinitExtension(Arrays.asList(packageAssembler), featuresManager); + return feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT); + } +} \ No newline at end of file diff --git a/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/_rep_repoPolicy.xml b/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/_rep_repoPolicy.xml new file mode 100644 index 0000000..91117f6 --- /dev/null +++ b/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/_rep_repoPolicy.xml @@ -0,0 +1,24 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with this + work for additional information regarding copyright ownership. The ASF + licenses this file to You under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + 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:ACL"> + <allow + jcr:primaryType="rep:GrantACE" + rep:principalName="repolevel-service" + rep:privileges="{Name}[jcr:namespaceManagement]"/> +</jcr:root> \ No newline at end of file
