This is an automated email from the ASF dual-hosted git repository. pauls pushed a commit to branch issues/SLING-9962 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git
commit b1dd6e4aad279887fff324247df8693bec93ebe9 Author: Karl Pauls <[email protected]> AuthorDate: Wed Jan 13 16:20:04 2021 +0100 SLING-9962: DefaultAclManager#addPaths prone to causing ConstraintViolationException --- .../accesscontrol/DefaultAclManager.java | 73 +++++++++++++++++----- .../cpconverter/accesscontrol/MixinParser.java | 52 +++++++++++++++ .../features/DefaultFeaturesManager.java | 2 +- .../cpconverter/accesscontrol/AclManagerTest.java | 23 +++---- .../handlers/RepPolicyEntryHandlerTest.java | 4 -- .../cpconverter/accesscontrol/asd/not/.content.xml | 20 ++++++ 6 files changed, 139 insertions(+), 35 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 4f581a3..7bbbe35 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,9 @@ import org.jetbrains.annotations.Nullable; import javax.jcr.NamespaceException; import java.io.File; import java.io.FileInputStream; -import java.util.Formatter; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.LinkedHashSet; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; -import java.util.Optional; -import java.util.Set; -import java.util.TreeSet; +import java.util.function.Predicate; import java.util.stream.Collectors; public final class DefaultAclManager implements AclManager { @@ -104,6 +95,22 @@ public final class DefaultAclManager implements AclManager { formatter.format("create service user %s with path %s%n", systemUser.getId(), systemUser.getIntermediatePath()); } + Set<RepoPath> paths = acls.entrySet().stream() + .filter(entry -> getSystemUser(entry.getKey()).isPresent()) + .map(Entry::getValue) + .flatMap(Collection::stream) + .map(AccessControlEntry::getRepositoryPath).collect(Collectors.toSet()); + + paths.stream() + .filter(path -> !paths.stream().anyMatch(other -> !other.equals(path) && other.startsWith(path))). + filter(((Predicate<RepoPath>)RepoPath::isRepositoryPath).negate()). + map(path -> computePathWithTypes(path, packageAssemblers)) + .filter(Objects::nonNull) + . + forEach( + path -> formatter.format("create path %s%n", path) + ); + // add the acls acls.forEach((systemUserID, authorizations) -> getSystemUser(systemUserID).ifPresent(systemUser -> @@ -131,11 +138,6 @@ public final class DefaultAclManager implements AclManager { authorizationsIterator.remove(); } } - - // make sure all paths are created first - - addPaths(authorizations, packageAssemblers, formatter); - // finally add ACLs addAclStatement(formatter, systemUser, authorizations); @@ -185,6 +187,45 @@ public final class DefaultAclManager implements AclManager { } } + private static @Nullable String computePathWithTypes(@NotNull RepoPath path, @NotNull List<VaultPackageAssembler> packageAssemblers) { + path = new RepoPath(PlatformNameFormat.getPlatformPath(path.toString())); + + boolean type = false; + String current = ""; + for (String part : path.toString().substring(1).split("/")) { + current += current.isEmpty() ? part : "/" + part; + for (VaultPackageAssembler packageAssembler : packageAssemblers) { + File currentContent = packageAssembler.getEntry(current + "/" + CONTENT_XML_FILE_NAME); + if (currentContent.isFile()) { + String primary; + String mixin; + try (FileInputStream input = new FileInputStream(currentContent); + FileInputStream input2 = new FileInputStream(currentContent)) { + primary = new PrimaryTypeParser(DEFAULT_TYPE).parse(input); + mixin = new MixinParser(DEFAULT_TYPE).parse(input2); + current += "(" + primary; + if (mixin != null) { + mixin = mixin.trim(); + if (mixin.startsWith("[")) { + mixin = mixin.substring(1, mixin.length() - 1); + } + current += " mixin " + mixin; + } + current += ")"; + type = true; + } catch (Exception e) { + throw new RuntimeException("A fatal error occurred while parsing the '" + + currentContent + + "' file, see nested exceptions: " + + e); + } + } + } + } + + return type ? new RepoPath(current).toString() : null; + } + private static @NotNull String computePathType(@NotNull RepoPath path, @NotNull List<VaultPackageAssembler> packageAssemblers) { path = new RepoPath(PlatformNameFormat.getPlatformPath(path.toString())); diff --git a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/MixinParser.java b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/MixinParser.java new file mode 100644 index 0000000..8da9b2f --- /dev/null +++ b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/MixinParser.java @@ -0,0 +1,52 @@ +/* + * 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.accesscontrol; + +import org.apache.jackrabbit.JcrConstants; +import org.apache.sling.feature.cpconverter.shared.AbstractJcrNodeParser; +import org.jetbrains.annotations.NotNull; +import org.xml.sax.Attributes; +import org.xml.sax.SAXException; + +final class MixinParser extends AbstractJcrNodeParser<String> { + + private String detectedPrimaryType; + private String mixins; + + public MixinParser(@NotNull String primaryType) { + super(primaryType); + } + + @Override + protected void onJcrRootNode(String uri, String localName, String qName, Attributes attributes, String primaryType) + throws SAXException { + detectedPrimaryType = primaryType; + mixins = attributes.getValue(JcrConstants.JCR_MIXINTYPES); + } + + @Override + protected void onJcrRootElement(String uri, String localName, String qName, Attributes attributes) + throws SAXException { + // not needed + } + + @Override + protected String getParsingResult() { + return mixins; + } + +} diff --git a/src/main/java/org/apache/sling/feature/cpconverter/features/DefaultFeaturesManager.java b/src/main/java/org/apache/sling/feature/cpconverter/features/DefaultFeaturesManager.java index 202189e..86fa637 100644 --- a/src/main/java/org/apache/sling/feature/cpconverter/features/DefaultFeaturesManager.java +++ b/src/main/java/org/apache/sling/feature/cpconverter/features/DefaultFeaturesManager.java @@ -92,7 +92,7 @@ public class DefaultFeaturesManager implements FeaturesManager { } public DefaultFeaturesManager(@NotNull File tempDir) { - this(true, 20, tempDir, null, null, null); + this(true, 20, tempDir, null, null, new HashMap<>()); } public DefaultFeaturesManager(boolean mergeConfigurations, 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 77a25b2..e7072eb 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 @@ -75,7 +75,10 @@ public class AclManagerTest { aclManager.addAcl("acs-commons-package-replication-status-event-service", newAcl(true, "jcr:read,crx:replicate,jcr:removeNode", "/home/users/system")); VaultPackageAssembler assembler = mock(VaultPackageAssembler.class); - when(assembler.getEntry(anyString())).thenReturn(new File(System.getProperty("java.io.tmpdir"))); + when(assembler.getEntry(anyString())).thenReturn(tempDir.toFile()); + when(assembler.getEntry("asd/not/.content.xml")).thenReturn(new File(getClass().getResource("asd/not/.content.xml").getFile())); + + Feature feature = new Feature(new ArtifactId("org.apache.sling", "org.apache.sling.cp2fm", "0.0.1", null, null)); FeaturesManager fm = Mockito.spy(new DefaultFeaturesManager(tempDir.toFile())); @@ -90,11 +93,7 @@ public class AclManagerTest { // acs-commons-on-deploy-scripts-service will be missed String expected = "create service user acs-commons-package-replication-status-event-service with path /home/users/system" + System.lineSeparator() + - "create path (sling:Folder) /asd" + System.lineSeparator() + - "create path (sling:Folder) /asd/not" + System.lineSeparator() + - "create path (sling:Folder) /asd/not/system" + System.lineSeparator() + - "create path (sling:Folder) /asd/not/system/user" + System.lineSeparator() + - "create path (sling:Folder) /asd/not/system/user/path" + System.lineSeparator() + + "create path /asd/not(any:Type mixin rep:AccessControllable)/system/user/path" + System.lineSeparator() + // see SLING-8561 // "set ACL for acs-commons-package-replication-status-event-service\n" + // "allow jcr:read,crx:replicate,jcr:removeNode on /asd/public\n" + @@ -122,7 +121,9 @@ public class AclManagerTest { aclManager.addAcl("acs-commons-package-replication-status-event-service", newAcl(true, "jcr:read,rep:write,rep:indexDefinitionManagement", "/asd/not/system/user/path")); VaultPackageAssembler assembler = mock(VaultPackageAssembler.class); - when(assembler.getEntry(anyString())).thenReturn(new File(System.getProperty("java.io.tmpdir"))); + when(assembler.getEntry(anyString())).thenReturn(tempDir.toFile()); + when(assembler.getEntry("asd/not/.content.xml")).thenReturn(new File(getClass().getResource("asd/not/.content.xml").getFile())); + Feature feature = new Feature(new ArtifactId("org.apache.sling", "org.apache.sling.cp2fm", "0.0.1", null, null)); FeaturesManager fm = Mockito.spy(new DefaultFeaturesManager(tempDir.toFile())); @@ -137,11 +138,7 @@ public class AclManagerTest { // aacs-commons-ensure-oak-index-service will be missed String expected = "create service user acs-commons-package-replication-status-event-service with path /home/users/system" + System.lineSeparator() + - "create path (sling:Folder) /asd" + System.lineSeparator() + - "create path (sling:Folder) /asd/not" + System.lineSeparator() + - "create path (sling:Folder) /asd/not/system" + System.lineSeparator() + - "create path (sling:Folder) /asd/not/system/user" + System.lineSeparator() + - "create path (sling:Folder) /asd/not/system/user/path" + System.lineSeparator() + + "create path /asd/not(any:Type mixin rep:AccessControllable)/system/user/path" + System.lineSeparator() + "set ACL for acs-commons-package-replication-status-event-service" + System.lineSeparator() + "allow jcr:read,rep:write,rep:indexDefinitionManagement on /asd/not/system/user/path" + System.lineSeparator() + "end" + System.lineSeparator(); @@ -191,8 +188,6 @@ public class AclManagerTest { String expected = "create service user sys-usr with path /home/users/system" + System.lineSeparator() + - "create path (sling:Folder) /content" + System.lineSeparator() + - "create path (sling:Folder) /content/cq:tags" + System.lineSeparator() + "set ACL for sys-usr" + System.lineSeparator() + "allow jcr:read on /content/cq:tags" + System.lineSeparator() + "allow jcr:write on /content/cq:tags" + System.lineSeparator() + 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 cbd1b0c..a9bf8eb 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 @@ -190,10 +190,6 @@ public final class RepPolicyEntryHandlerTest { String expected = "create service user acs-commons-package-replication-status-event-service with path /this/is/a/completely/different/path" + System.lineSeparator() + - "create path (sling:Folder) /home" + System.lineSeparator() + - "create path (sling:Folder) /home/users" + System.lineSeparator() + - "create path (sling:Folder) /home/users/system" + System.lineSeparator() + - "create path (sling:Folder) /home/users/system/asd" + System.lineSeparator() + "set ACL for acs-commons-package-replication-status-event-service" + System.lineSeparator() + "allow jcr:read,rep:write,jcr:readAccessControl,jcr:modifyAccessControl on /home/users/system/asd" + System.lineSeparator() + "deny jcr:write on /home/users/system/asd" + System.lineSeparator() + diff --git a/src/test/resources/org/apache/sling/feature/cpconverter/accesscontrol/asd/not/.content.xml b/src/test/resources/org/apache/sling/feature/cpconverter/accesscontrol/asd/not/.content.xml new file mode 100644 index 0000000..c1a8a69 --- /dev/null +++ b/src/test/resources/org/apache/sling/feature/cpconverter/accesscontrol/asd/not/.content.xml @@ -0,0 +1,20 @@ +<?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:sling="http://sling.apache.org/jcr/sling/1.0"; xmlns:jcr="http://www.jcp.org/jcr/1.0"; xmlns:rep="internal" + jcr:mixinTypes="[rep:AccessControllable]" + jcr:primaryType="any:Type"/> \ No newline at end of file
