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/13 16:59:14 UTC
[sling-org-apache-sling-feature-cpconverter] branch master updated: SLING-9962: DefaultAclManager#addPaths prone to causing ConstraintVio… (#50)
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 3c6f4ff SLING-9962: DefaultAclManager#addPaths prone to causing ConstraintVio… (#50)
3c6f4ff is described below
commit 3c6f4ff60aca3542c1b23f76e09159144ae53cfd
Author: Karl Pauls <pa...@apache.org>
AuthorDate: Wed Jan 13 17:59:08 2021 +0100
SLING-9962: DefaultAclManager#addPaths prone to causing ConstraintVio… (#50)
* SLING-9962: DefaultAclManager#addPaths prone to causing ConstraintViolationException
---
.../accesscontrol/DefaultAclManager.java | 90 ++++++++++++----------
.../{PrimaryTypeParser.java => MixinParser.java} | 15 ++--
.../accesscontrol/PrimaryTypeParser.java | 4 +
.../features/DefaultFeaturesManager.java | 2 +-
.../cpconverter/accesscontrol/AclManagerTest.java | 23 +++---
.../handlers/RepPolicyEntryHandlerTest.java | 4 -
.../cpconverter/accesscontrol/asd/not/.content.xml | 23 ++++++
7 files changed, 94 insertions(+), 67 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..900923b 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,6 +31,7 @@ import org.jetbrains.annotations.Nullable;
import javax.jcr.NamespaceException;
import java.io.File;
import java.io.FileInputStream;
+import java.util.Optional;
import java.util.Formatter;
import java.util.HashMap;
import java.util.HashSet;
@@ -39,10 +40,11 @@ import java.util.LinkedHashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
-import java.util.Map.Entry;
-import java.util.Optional;
import java.util.Set;
-import java.util.TreeSet;
+import java.util.Collection;
+import java.util.Map.Entry;
+import java.util.Objects;
+import java.util.function.Predicate;
import java.util.stream.Collectors;
public final class DefaultAclManager implements AclManager {
@@ -104,6 +106,21 @@ 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 +148,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);
@@ -164,49 +176,43 @@ public final class DefaultAclManager implements AclManager {
privilegeDefinitions = null;
}
- private void addPaths(@NotNull List<AccessControlEntry> authorizations, @NotNull List<VaultPackageAssembler> packageAssemblers, @NotNull Formatter formatter) {
- if (authorizations.isEmpty()) {
- return;
- }
-
- Set<RepoPath> paths = new TreeSet<>();
- for (AccessControlEntry authorization : authorizations) {
- RepoPath rp = authorization.getRepositoryPath();
- // exclude special paths: user/group home nodes and subtrees therein, repository-level marker path
- if (!(rp.isRepositoryPath())) {
- addPath(authorization.getRepositoryPath(), paths);
- }
- }
-
- for (RepoPath path : paths) {
- String type = computePathType(path, packageAssemblers);
-
- formatter.format("create path (%s) %s%n", type, path);
- }
- }
-
- private static @NotNull String computePathType(@NotNull RepoPath path, @NotNull List<VaultPackageAssembler> packageAssemblers) {
+ private static @Nullable String computePathWithTypes(@NotNull RepoPath path, @NotNull List<VaultPackageAssembler> packageAssemblers) {
path = new RepoPath(PlatformNameFormat.getPlatformPath(path.toString()));
- for (VaultPackageAssembler packageAssembler: packageAssemblers) {
- File currentDir = packageAssembler.getEntry(path.toString());
-
- if (currentDir.exists()) {
- File currentContent = new File(currentDir, CONTENT_XML_FILE_NAME);
- if (currentContent.exists()) {
- try (FileInputStream input = new FileInputStream(currentContent)) {
- return new PrimaryTypeParser(DEFAULT_TYPE).parse(input);
+ 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().parse(input);
+ mixin = new MixinParser().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);
+ + currentContent
+ + "' file, see nested exceptions: "
+ + e);
}
}
}
}
- return DEFAULT_TYPE;
+ return type ? new RepoPath(current).toString() : null;
}
private static void addAclStatement(@NotNull Formatter formatter, @NotNull SystemUser systemUser, @NotNull List<AccessControlEntry> authorizations) {
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/PrimaryTypeParser.java b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/MixinParser.java
similarity index 81%
copy from src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/PrimaryTypeParser.java
copy to src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/MixinParser.java
index ce600c2..5371fff 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/PrimaryTypeParser.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/MixinParser.java
@@ -16,23 +16,26 @@
*/
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 PrimaryTypeParser extends AbstractJcrNodeParser<String> {
+final class MixinParser extends AbstractJcrNodeParser<String> {
+ private String mixins;
- private String detectedPrimaryType;
-
- public PrimaryTypeParser(@NotNull String primaryType) {
+ public MixinParser() {
+ this("sling:Folder");
+ }
+ 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
@@ -43,7 +46,7 @@ final class PrimaryTypeParser extends AbstractJcrNodeParser<String> {
@Override
protected String getParsingResult() {
- return detectedPrimaryType != null ? detectedPrimaryType : getPrimaryType();
+ return mixins;
}
}
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/PrimaryTypeParser.java b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/PrimaryTypeParser.java
index ce600c2..27f769f 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/PrimaryTypeParser.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/PrimaryTypeParser.java
@@ -25,6 +25,10 @@ final class PrimaryTypeParser extends AbstractJcrNodeParser<String> {
private String detectedPrimaryType;
+ public PrimaryTypeParser() {
+ this("sling:Folder");
+ }
+
public PrimaryTypeParser(@NotNull String primaryType) {
super(primaryType);
}
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..98c1863 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(nt:unstructured mixin rep:AccessControllable,mix:created)/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(nt:unstructured mixin rep:AccessControllable,mix:created)/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..d3ffb51
--- /dev/null
+++ b/src/test/resources/org/apache/sling/feature/cpconverter/accesscontrol/asd/not/.content.xml
@@ -0,0 +1,23 @@
+<?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"
+ xmlns:nt="http://www.jcp.org/jcr/nt/1.0"
+ xmlns:mix="http://www.jcp.org/jcr/mix/1.0"
+ jcr:mixinTypes="[rep:AccessControllable,mix:created]"
+ jcr:primaryType="nt:unstructured"/>
\ No newline at end of file