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/05/18 12:46:05 UTC
[sling-org-apache-sling-feature-cpconverter] 01/01: SLING-10386 :
Converter needs to issue create path statements for repoinit path without
node type
This is an automated email from the ASF dual-hosted git repository.
angela pushed a commit to branch SLING-10386
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git
commit 30d23ccb23cb98b079b0bb40c46d8985f35d490f
Author: angela <an...@adobe.com>
AuthorDate: Tue May 18 14:45:41 2021 +0200
SLING-10386 : Converter needs to issue create path statements for repoinit path without node type
---
.../accesscontrol/DefaultAclManager.java | 40 ++++++++++++++++++----
.../cpconverter/accesscontrol/AclManagerTest.java | 16 ++++++---
.../accesscontrol/EnforcePrincipalBasedTest.java | 2 ++
.../handlers/RepPolicyEntryHandlerTest.java | 1 +
4 files changed, 48 insertions(+), 11 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 19bc4db..e071f3b 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
@@ -20,9 +20,9 @@ import org.apache.jackrabbit.spi.Name;
import org.apache.jackrabbit.spi.PrivilegeDefinition;
import org.apache.jackrabbit.spi.commons.conversion.DefaultNamePathResolver;
import org.apache.jackrabbit.spi.commons.conversion.NameResolver;
+import org.apache.jackrabbit.util.Text;
import org.apache.jackrabbit.vault.fs.spi.PrivilegeDefinitions;
import org.apache.jackrabbit.vault.util.PlatformNameFormat;
-import org.apache.jackrabbit.vault.util.Text;
import org.apache.sling.feature.cpconverter.features.FeaturesManager;
import org.apache.sling.feature.cpconverter.repoinit.NoOpVisitor;
import org.apache.sling.feature.cpconverter.repoinit.OperationProcessor;
@@ -92,6 +92,8 @@ public class DefaultAclManager implements AclManager, EnforceInfo {
private final List<RegisterNodetypes> nodetypeOperations = new LinkedList<>();
private volatile PrivilegeDefinitions privilegeDefinitions;
+
+ private RepoPath userRootPath;
public DefaultAclManager() {
this(null, "system");
@@ -118,6 +120,7 @@ public class DefaultAclManager implements AclManager, EnforceInfo {
public boolean addSystemUser(@NotNull SystemUser systemUser) {
if (systemUsers.add(systemUser)) {
recordSystemUserIds(systemUser.getId());
+ setUserRoot(systemUser.getPath());
return true;
} else {
return false;
@@ -307,7 +310,7 @@ public class DefaultAclManager implements AclManager, EnforceInfo {
}
}
- private List<AclLine> asAcLines(@NotNull Map<AccessControlEntry, String> entries) {
+ private static List<AclLine> asAcLines(@NotNull Map<AccessControlEntry, String> entries) {
List<AclLine> lines = new ArrayList<>();
entries.forEach((entry, path) -> lines.add(entry.asAclLine(path)));
return lines;
@@ -430,10 +433,20 @@ public class DefaultAclManager implements AclManager, EnforceInfo {
cp.addSegment(part, null);
}
}
- return (foundType) ? cp : null;
+
+ if (!foundType && isBelowUserRoot(path)) {
+ // if no type information has been detected, don't issue a 'create path' statement for nodes below the
+ // user-root
+ log.warn("Failed to extract primary type information for node at path '{}'", path);
+ return null;
+ } else {
+ // assume that primary type information is present or can be extracted from default primary type definition
+ // of the the top-level nodes (i.e. their effective node type definition).
+ return cp;
+ }
}
- private boolean addSegment(@NotNull CreatePath cp, @NotNull String part, @NotNull File currentContent) {
+ private static boolean addSegment(@NotNull CreatePath cp, @NotNull String part, @NotNull File currentContent) {
try (FileInputStream input = new FileInputStream(currentContent);
FileInputStream input2 = new FileInputStream(currentContent)) {
String primary = new PrimaryTypeParser().parse(input);
@@ -480,7 +493,7 @@ public class DefaultAclManager implements AclManager, EnforceInfo {
}
}
- private boolean isHomePath(@NotNull RepoPath path, @NotNull RepoPath systemUserPath) {
+ private static boolean isHomePath(@NotNull RepoPath path, @NotNull RepoPath systemUserPath) {
// ACE located in the subtree are not supported
return path.equals(systemUserPath);
}
@@ -491,7 +504,7 @@ public class DefaultAclManager implements AclManager, EnforceInfo {
}
@NotNull
- private String getHomePath(@NotNull AbstractUser abstractUser) {
+ private static String getHomePath(@NotNull AbstractUser abstractUser) {
// since ACEs located in the subtree of a user are not supported by the converter,
// there is no need to calculate a potential sub-path to be appended.
return "home("+abstractUser.getId()+")";
@@ -524,4 +537,19 @@ public class DefaultAclManager implements AclManager, EnforceInfo {
}).collect(Collectors.toList());
}
}
+
+ /**
+ * Record the root path for all users/groups assuming that their common ancestor is a top-level node
+ *
+ * @param userPath A user path
+ */
+ private void setUserRoot(@NotNull RepoPath userPath) {
+ if (userRootPath == null) {
+ userRootPath = new RepoPath(Text.getAbsoluteParent(userPath.toString(), 0));
+ }
+ }
+
+ private boolean isBelowUserRoot(@NotNull RepoPath path) {
+ return userRootPath != null && path.startsWith(userRootPath);
+ }
}
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 fa9cb0b..06fe4f2 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
@@ -191,6 +191,7 @@ public class AclManagerTest {
String expected = Util.normalize(
"create service user sys-usr with path system\n" +
+ "create path /content/cq:tags\n"+
"set ACL for sys-usr\n" +
" allow jcr:read on /content/cq:tags\n" +
" allow jcr:write on /content/cq:tags\n" +
@@ -205,7 +206,7 @@ public class AclManagerTest {
}
@Test(expected = IllegalStateException.class)
- public void testGroupHandlingWithGroupUsed() throws RepoInitParsingException {
+ public void testGroupHandlingWithGroupUsed() {
aclManager.addSystemUser(new SystemUser("sys-usr", new RepoPath("/home/users/system/foo"), new RepoPath("/home/users/system")));
aclManager.addGroup(new Group("test", new RepoPath("/home/groups/test"), new RepoPath("/home/groups/test")));
@@ -242,6 +243,7 @@ public class AclManagerTest {
String expected = Util.normalize(
"create service user sys-usr with path system\n" +
+ "create path /content/test\n" +
"set ACL for sys-usr\n" +
" allow jcr:read on /content/test\n" +
"end\n");
@@ -252,7 +254,7 @@ public class AclManagerTest {
}
@Test(expected = IllegalStateException.class)
- public void testGroupHandlingWithGroupMatchingSubPath() throws RepoInitParsingException {
+ public void testGroupHandlingWithGroupMatchingSubPath() {
aclManager.addSystemUser(new SystemUser("sys-usr", new RepoPath("/home/users/system/foo"), new RepoPath("/home/users/system")));
aclManager.addGroup(new Group("test", new RepoPath("/home/groups/test"), new RepoPath("/home/groups/test")));
@@ -267,7 +269,7 @@ public class AclManagerTest {
}
@Test(expected = IllegalStateException.class)
- public void testUserHandlingWithMatchingUser() throws RepoInitParsingException {
+ public void testUserHandlingWithMatchingUser() {
aclManager.addSystemUser(new SystemUser("sys-usr", new RepoPath("/home/users/system/foo"), new RepoPath("/home/users/system")));
aclManager.addUser(new User("test", new RepoPath("/home/users/test"), new RepoPath("/home/users/test")));
@@ -301,6 +303,7 @@ public class AclManagerTest {
String expected = Util.normalize(
"create service user sys-usr with path system\n" +
+ "create path /content/test\n" +
"set ACL for sys-usr\n" +
" allow jcr:read on /content/test\n" +
"end\n");
@@ -314,7 +317,7 @@ public class AclManagerTest {
aclManager.addSystemUser(new SystemUser("sys-usr", new RepoPath("/home/users/system/foo"), new RepoPath("/home/users/system")));
aclManager.addUser(new User("test", new RepoPath("/home/users/test"), new RepoPath("/home/users/test")));
- aclManager.addAcl("sys-usr", newAcl(true, "jcr:read", "/home/users/test2"));
+ aclManager.addAcl("sys-usr", newAcl(true, "jcr:read", "/home/users/notMatching"));
VaultPackageAssembler assembler = mock(VaultPackageAssembler.class);
when(assembler.getEntry(anyString())).thenReturn(new File(System.getProperty("java.io.tmpdir")));
Feature feature = new Feature(new ArtifactId("org.apache.sling", "org.apache.sling.cp2fm", "0.0.1", null, null));
@@ -327,10 +330,13 @@ public class AclManagerTest {
Extension repoinitExtension = feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT);
assertNotNull(repoinitExtension);
+ // in contrast to testUserHandlingWithMatchingUser in this test /home/users/test2 is not detected
+ // as user-home-path and thus is processed like a regular path (no 'home(uid)' repo-init statement and no exception).\
+ // however, no attempt is made to create the path without any available node type information.
String expected = Util.normalize(
"create service user sys-usr with path system\n" +
"set ACL for sys-usr\n" +
- " allow jcr:read on /home/users/test2\n" +
+ " allow jcr:read on /home/users/notMatching\n" +
"end\n");
String actual = repoinitExtension.getText();
diff --git a/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/EnforcePrincipalBasedTest.java b/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/EnforcePrincipalBasedTest.java
index 89353ae..62ffcdd 100644
--- a/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/EnforcePrincipalBasedTest.java
+++ b/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/EnforcePrincipalBasedTest.java
@@ -197,6 +197,7 @@ public class EnforcePrincipalBasedTest {
String expected = normalize(
"create service user user1 with path " +relativeIntermediatePath+ "\n" +
+ "create path /content/feature\n" +
"set ACL for user1\n" +
" allow jcr:read on /content/feature\n" +
"end\n");
@@ -239,6 +240,7 @@ public class EnforcePrincipalBasedTest {
String expected = normalize(
"create service user user1 with path " + relativeIntermediatePath + "\n" +
+ "create path /content/feature\n" +
"set ACL for user1\n" +
" allow jcr:read on /content/feature\n" +
"end\n");
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 3352fa4..6e44b9e 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
@@ -265,6 +265,7 @@ public final class RepPolicyEntryHandlerTest {
Extension repoinitExtension = parseAndSetRepoinit(path, aclManager).getRepoinitExtension();
String expected = normalize(
"create service user service1 with path system/services\n" +
+ "create path /asd/jr2restrictions\n" +
"set ACL for service1\n" +
" allow jcr:read on /asd/jr2restrictions restriction(rep:glob,*/subtree/*) restriction(sling:customRestriction,sling:value1,sling:value2)\n" +
"end\n");