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/14 00:05:57 UTC
[sling-org-apache-sling-feature-cpconverter] branch
issues/SLING-9953 created (now 1c575f8)
This is an automated email from the ASF dual-hosted git repository.
pauls pushed a change to branch issues/SLING-9953
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git.
at 1c575f8 SLING-9953: ACEs on/below user nodes are ignored upon conversion
This branch includes the following new commits:
new 1c575f8 SLING-9953: ACEs on/below user nodes are ignored upon conversion
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-9953:
ACEs on/below user nodes are ignored upon conversion
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
pauls pushed a commit to branch issues/SLING-9953
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git
commit 1c575f830b0ecb66d55ff949f610a5c5ef641514
Author: Karl Pauls <ka...@gmail.com>
AuthorDate: Thu Jan 14 01:05:39 2021 +0100
SLING-9953: ACEs on/below user nodes are ignored upon conversion
---
.../accesscontrol/DefaultAclManager.java | 109 ++++++++++++---------
.../features/DefaultFeaturesManager.java | 2 +-
.../cpconverter/accesscontrol/AclManagerTest.java | 42 --------
.../handlers/RepPolicyEntryHandlerTest.java | 73 +++++++-------
4 files changed, 97 insertions(+), 129 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 900923b..580473d 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
@@ -35,15 +35,15 @@ import java.util.Optional;
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.LinkedHashMap;
import java.util.Map;
import java.util.Set;
import java.util.Collection;
-import java.util.Map.Entry;
import java.util.Objects;
+import java.util.Map.Entry;
import java.util.function.Predicate;
import java.util.stream.Collectors;
@@ -53,6 +53,9 @@ public final class DefaultAclManager implements AclManager {
private static final String DEFAULT_TYPE = "sling:Folder";
+ // FIXME: SLING-9969. avoid hardcoding
+ private static final RepoPath USER_GROUP_ROOT = new RepoPath("/home");
+
private final Set<RepoPath> preProvidedSystemPaths = new HashSet<>();
private final Set<RepoPath> preProvidedPaths = new HashSet<>();
@@ -77,17 +80,6 @@ public final class DefaultAclManager implements AclManager {
return false;
}
- private void addPath(@NotNull RepoPath path, @NotNull Set<RepoPath> paths) {
- if (preProvidedPaths.add(path)) {
- paths.add(path);
- }
-
- RepoPath parent = path.getParent();
- if (parent != null && parent.getSegmentCount() > 0) {
- addPath(parent, paths);
- }
- }
-
public void addRepoinitExtension(@NotNull List<VaultPackageAssembler> packageAssemblers, @NotNull FeaturesManager featureManager) {
try (Formatter formatter = new Formatter()) {
@@ -139,18 +131,34 @@ public final class DefaultAclManager implements AclManager {
@NotNull List<AccessControlEntry> authorizations,
@NotNull List<VaultPackageAssembler> packageAssemblers,
@NotNull Formatter formatter) {
- // clean the unneeded ACLs, see SLING-8561
- Iterator<AccessControlEntry> authorizationsIterator = authorizations.iterator();
- while (authorizationsIterator.hasNext()) {
- AccessControlEntry acl = authorizationsIterator.next();
+ if (authorizations.isEmpty()) {
+ return;
+ }
- if (acl.getRepositoryPath().startsWith(systemUser.getIntermediatePath())) {
- authorizationsIterator.remove();
+ Map<AccessControlEntry, String> entries = new LinkedHashMap<>();
+ authorizations.forEach(entry -> {
+ String path = getRepoInitPath(entry.getRepositoryPath(), systemUser);
+ if (path != null) {
+ entries.put(entry, path);
}
- }
- // finally add ACLs
+ });
+ if (!entries.isEmpty()) {
+ formatter.format("set ACL for %s%n", systemUser.getId());
+ entries.forEach((entry, path) -> {
+ formatter.format("%s %s on %s",
+ entry.getOperation(),
+ entry.getPrivileges(),
+ path);
+
+ if (!entry.getRestrictions().isEmpty()) {
+ formatter.format(" restriction(%s)",
+ String.join(",", entry.getRestrictions()));
+ }
- addAclStatement(formatter, systemUser, authorizations);
+ formatter.format("%n");
+ });
+ formatter.format("end%n");
+ }
}
private @NotNull Optional<SystemUser> getSystemUser(@NotNull String id) {
@@ -215,44 +223,47 @@ public final class DefaultAclManager implements AclManager {
return type ? new RepoPath(current).toString() : null;
}
- private static void addAclStatement(@NotNull Formatter formatter, @NotNull SystemUser systemUser, @NotNull List<AccessControlEntry> authorizations) {
- if (authorizations.isEmpty()) {
- return;
+ @Nullable
+ private String getRepoInitPath(@NotNull RepoPath path, @NotNull SystemUser systemUser) {
+ if (path.isRepositoryPath()) {
+ return ":repository";
+ } else if (isHomePath(path, systemUser.getPath())) {
+ return getHomePath(path, systemUser);
+ } else if (isUserGroupPath(path)) {
+ SystemUser otherSystemUser = getOtherSystemUser(path);
+ if (otherSystemUser != null) {
+ return getHomePath(path, otherSystemUser);
+ } else {
+ return path.toString();
+ }
+ } else {
+ return path.toString();
}
-
- writeAccessControl(authorizations, "set ACL for %s%n", systemUser, formatter);
}
- 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 boolean isHomePath(@NotNull RepoPath path, @NotNull RepoPath systemUserPath) {
+ return path.startsWith(systemUserPath);
}
- private static void writeEntries(@NotNull List<AccessControlEntry> accessControlEntries, @NotNull SystemUser systemUser, @NotNull Formatter formatter) {
- for (AccessControlEntry entry : accessControlEntries) {
- formatter.format("%s %s on %s",
- entry.getOperation(),
- entry.getPrivileges(),
- getRepoInitPath(entry.getRepositoryPath(), systemUser));
+ private static boolean isUserGroupPath(@NotNull RepoPath path) {
+ return path.startsWith(USER_GROUP_ROOT);
+ }
- if (!entry.getRestrictions().isEmpty()) {
- formatter.format(" restriction(%s)",
- String.join(",", entry.getRestrictions()));
+ @Nullable
+ private SystemUser getOtherSystemUser(@NotNull RepoPath path) {
+ for (SystemUser su : systemUsers) {
+ if (path.startsWith(su.getPath())) {
+ return su;
}
-
- formatter.format("%n");
}
+ return null;
}
@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 String getHomePath(@NotNull RepoPath path, @NotNull SystemUser systemUser) {
+ RepoPath systemUserPath = systemUser.getPath();
+ String subpath = (path.equals(systemUserPath) ? "" : path.toString().substring(systemUserPath.toString().length()));
+ return "home("+systemUser.getId()+")"+subpath;
}
private static void registerPrivileges(@NotNull PrivilegeDefinitions definitions, @NotNull Formatter formatter) {
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 86fa637..ab4ea25 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
@@ -88,7 +88,7 @@ public class DefaultFeaturesManager implements FeaturesManager {
private Feature targetFeature = null;
DefaultFeaturesManager() {
- this(null);
+ this(new File(""));
}
public DefaultFeaturesManager(@NotNull File tempDir) {
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 98c1863..f5a510d 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
@@ -68,48 +68,6 @@ public class AclManagerTest {
}
@Test
- public void makeSureAclsAreCreatedOnlyoutsideSytemUsersPaths() throws Exception {
- aclManager.addSystemUser(new SystemUser("acs-commons-package-replication-status-event-service", new RepoPath("/home/users/system/foo"), new RepoPath("/home/users/system")));
-
- aclManager.addAcl("acs-commons-package-replication-status-event-service", newAcl(true, "jcr:read,rep:write,rep:indexDefinitionManagement", "/asd/not/system/user/path"));
- 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(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()));
- when(fm.getTargetFeature()).thenReturn(feature);
-
- aclManager.addRepoinitExtension(Arrays.asList(assembler), fm);
-
-
- Extension repoinitExtension = feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT);
- assertNotNull(repoinitExtension);
-
- // 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 /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" +
- // "end\n" +
- "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();
- String actual = repoinitExtension.getText();
- assertEquals(expected, actual);
-
- RepoInitParser repoInitParser = new RepoInitParserService();
- List<Operation> operations = repoInitParser.parse(new StringReader(actual));
- assertFalse(operations.isEmpty());
- }
-
- @Test
public void testReset() throws RepoInitParsingException {
// We assume this user will not be in the result because of the reset in the next line
aclManager.addSystemUser(new SystemUser("acs-commons-ensure-oak-index-service", new RepoPath("/home/users/system/foo"), new RepoPath("/home/users/system")));
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 a9bf8eb..267a6b0 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
@@ -96,31 +96,32 @@ public final class RepPolicyEntryHandlerTest {
// commented ACLs are due SLING-8561
String expected =
"create service user acs-commons-ensure-oak-index-service with path /home/users/system" + System.lineSeparator() +
- // "create path (sling:Folder) /asd\n" +
- // "create path (sling:Folder) /asd/public\n" +
- // "set ACL for acs-commons-ensure-oak-index-service\n" +
- // "allow jcr:read,rep:write,rep:indexDefinitionManagement on /asd/public restriction(rep:glob,*/oak:index/*)\n" +
- // "end\n" +
"create service user acs-commons-dispatcher-flush-service with path /home/users/system" + System.lineSeparator() +
- // "set ACL for acs-commons-dispatcher-flush-service\n" +
- // "allow jcr:read,crx:replicate,jcr:removeNode on /asd/public\n" +
- // "end\n" +
"create service user acs-commons-package-replication-status-event-service with path /home/users/system" + System.lineSeparator() +
- // "set ACL for acs-commons-package-replication-status-event-service\n" +
- // "allow jcr:read,rep:write,jcr:readAccessControl,jcr:modifyAccessControl on /asd/public\n" +
- // "end\n" +
"create service user acs-commons-ensure-service-user-service with path /home/users/system" + System.lineSeparator() +
- // "set ACL for acs-commons-ensure-service-user-service\n" +
- // "allow jcr:read,rep:write,jcr:readAccessControl,jcr:modifyAccessControl on /asd/public\n" +
- // "end\n" +
"create service user acs-commons-automatic-package-replicator-service with path /home/users/system" + System.lineSeparator() +
- // "set ACL for acs-commons-automatic-package-replicator-service\n" +
- // "allow jcr:read on /asd/public\n" +
- // "end\n" +
- "create service user acs-commons-on-deploy-scripts-service with path /home/users/system" + System.lineSeparator();
- // "set ACL for acs-commons-on-deploy-scripts-service\n" +
- // "allow jcr:read on /asd/public\n" +
- // "end\n";
+ "create service user acs-commons-on-deploy-scripts-service with path /home/users/system" + System.lineSeparator() +
+ "set ACL for acs-commons-automatic-package-replicator-service" + System.lineSeparator() +
+ "allow jcr:read on home(acs-commons-automatic-package-replicator-service)" + System.lineSeparator() +
+ "end" + 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(acs-commons-package-replication-status-event-service)" + System.lineSeparator() +
+ "deny jcr:write on home(acs-commons-package-replication-status-event-service)" + System.lineSeparator() +
+ "end" + System.lineSeparator() +
+ "set ACL for acs-commons-dispatcher-flush-service" + System.lineSeparator() +
+ "allow jcr:read,crx:replicate,jcr:removeNode on home(acs-commons-dispatcher-flush-service)" + System.lineSeparator() +
+ "deny jcr:write on home(acs-commons-dispatcher-flush-service)" + System.lineSeparator() +
+ "end" + System.lineSeparator() +
+ "set ACL for acs-commons-ensure-oak-index-service" + System.lineSeparator() +
+ "allow jcr:read,rep:write,rep:indexDefinitionManagement on home(acs-commons-ensure-oak-index-service) restriction(rep:glob,*/oak:index/*)" + System.lineSeparator() +
+ "end" + System.lineSeparator() +
+ "set ACL for acs-commons-on-deploy-scripts-service" + System.lineSeparator() +
+ "allow jcr:read on home(acs-commons-on-deploy-scripts-service)" + System.lineSeparator() +
+ "end" + System.lineSeparator() +
+ "set ACL for acs-commons-ensure-service-user-service" + System.lineSeparator() +
+ "allow jcr:read,rep:write,jcr:readAccessControl,jcr:modifyAccessControl on home(acs-commons-ensure-service-user-service)" + System.lineSeparator() +
+ "end" + System.lineSeparator();
+
String actual = repoinitExtension.getText();
assertEquals(expected, actual);
@@ -140,26 +141,24 @@ public final class RepPolicyEntryHandlerTest {
assertNotNull(repoinitExtension);
assertEquals(ExtensionType.TEXT, repoinitExtension.getType());
- // commented ACLs are due SLING-8561
String expected =
"create service user acs-commons-package-replication-status-event-service with path /home/users/system" + System.lineSeparator() +
- // "create path (sling:Folder) /asd\n" +
- // "create path (sling:Folder) /asd/public\n" +
- // "set ACL for acs-commons-package-replication-status-event-service\n" +
- // "allow jcr:read,rep:write,jcr:readAccessControl,jcr:modifyAccessControl on /asd/public\n" +
- // "end\n" +
"create service user acs-commons-ensure-service-user-service with path /home/users/system" + System.lineSeparator() +
- // "set ACL for acs-commons-ensure-service-user-service\n" +
- // "allow jcr:read,rep:write,jcr:readAccessControl,jcr:modifyAccessControl on /asd/public\n" +
- // "end\n" +
"create service user acs-commons-automatic-package-replicator-service with path /home/users/system" + System.lineSeparator() +
- // "set ACL for acs-commons-automatic-package-replicator-service\n" +
- // "allow jcr:read on /asd/public\n" +
- // "end\n" +
- "create service user acs-commons-on-deploy-scripts-service with path /home/users/system" + System.lineSeparator();
- //"set ACL for acs-commons-on-deploy-scripts-service\n" +
- //"allow jcr:read on /asd/public\n" +
- //"end\n";
+ "create service user acs-commons-on-deploy-scripts-service with path /home/users/system" + System.lineSeparator() +
+ "set ACL for acs-commons-automatic-package-replicator-service" + System.lineSeparator() +
+ "allow jcr:read on home(acs-commons-automatic-package-replicator-service)" + System.lineSeparator() +
+ "end" + 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(acs-commons-package-replication-status-event-service)" + System.lineSeparator() +
+ "deny jcr:write on home(acs-commons-package-replication-status-event-service)" + System.lineSeparator() +
+ "end" + System.lineSeparator() +
+ "set ACL for acs-commons-on-deploy-scripts-service" + System.lineSeparator() +
+ "allow jcr:read on home(acs-commons-on-deploy-scripts-service)" + System.lineSeparator() +
+ "end" + System.lineSeparator() +
+ "set ACL for acs-commons-ensure-service-user-service" + System.lineSeparator() +
+ "allow jcr:read,rep:write,jcr:readAccessControl,jcr:modifyAccessControl on home(acs-commons-ensure-service-user-service)" + System.lineSeparator() +
+ "end" + System.lineSeparator();
String actual = repoinitExtension.getText();
assertEquals(expected, actual);