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:04 UTC
[sling-org-apache-sling-feature-cpconverter] branch SLING-10386
created (now 30d23cc)
This is an automated email from the ASF dual-hosted git repository.
angela pushed a change to branch SLING-10386
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git.
at 30d23cc SLING-10386 : Converter needs to issue create path statements for repoinit path without node type
This branch includes the following new commits:
new 30d23cc SLING-10386 : Converter needs to issue create path statements for repoinit path without node type
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[sling-org-apache-sling-feature-cpconverter] 01/01: SLING-10386 :
Converter needs to issue create path statements for repoinit path without
node type
Posted by an...@apache.org.
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");