You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by si...@apache.org on 2019/08/02 13:56:52 UTC
[sling-org-apache-sling-feature-cpconverter] branch master updated:
SLING-8561 - ACLs on service users should be ignored because of randomised
node names
This is an automated email from the ASF dual-hosted git repository.
simonetripodi 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 ff7dcda SLING-8561 - ACLs on service users should be ignored because of randomised node names
ff7dcda is described below
commit ff7dcda5b753feefcbf44015b8698fbe00a98eb1
Author: Simo Tripodi <st...@adobe.com>
AuthorDate: Fri Aug 2 15:54:49 2019 +0200
SLING-8561 - ACLs on service users should be ignored because of
randomised node names
applying the policy to all ACLs, not just detected system users
---
.../feature/cpconverter/acl/DefaultAclManager.java | 66 ++++++++++++----------
.../feature/cpconverter/acl/AclManagerTest.java | 11 ++--
2 files changed, 42 insertions(+), 35 deletions(-)
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/acl/DefaultAclManager.java b/src/main/java/org/apache/sling/feature/cpconverter/acl/DefaultAclManager.java
index 5a31386..43e7764 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/acl/DefaultAclManager.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/acl/DefaultAclManager.java
@@ -29,6 +29,7 @@ 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.stream.Collectors;
@@ -67,7 +68,7 @@ public final class DefaultAclManager implements AclManager {
}
public boolean addAcl(String systemUser, Acl acl) {
- if (isKnownSystemUser(systemUser)) {
+ if (getSystemUser(systemUser).isPresent()) {
acls.computeIfAbsent(systemUser, k -> new LinkedList<>()).add(acl);
return true;
}
@@ -124,41 +125,19 @@ public final class DefaultAclManager implements AclManager {
// clean the unneeded ACLs, see SLING-8561
List<Acl> authorizations = acls.remove(systemUser.getId());
- if (authorizations != null) {
- Iterator<Acl> authorizationsIterator = authorizations.iterator();
- while (authorizationsIterator.hasNext()) {
- Acl acl = authorizationsIterator.next();
-
- if (acl.getPath().startsWith(systemUser.getPath())) {
- authorizationsIterator.remove();
- }
- }
- }
-
- // create then the paths
-
- addPaths(authorizations, packageAssemblers, formatter);
-
- // finally add ACLs
- addAclStatement(formatter, systemUser.getId(), authorizations);
+ addStatements(systemUser, authorizations, packageAssemblers, formatter);
}
// all the resting ACLs can now be set
for (Entry<String, List<Acl>> currentAcls : acls.entrySet()) {
- String systemUser = currentAcls.getKey();
+ Optional<SystemUser> systemUser = getSystemUser(currentAcls.getKey());
- if (isKnownSystemUser(systemUser)) {
+ if (systemUser.isPresent()) {
List<Acl> authorizations = currentAcls.getValue();
- // make sure all paths are created first
-
- addPaths(authorizations, packageAssemblers, formatter);
-
- // finally add ACLs
-
- addAclStatement(formatter, systemUser, authorizations);
+ addStatements(systemUser.get(), authorizations, packageAssemblers, formatter);
}
}
@@ -176,13 +155,38 @@ public final class DefaultAclManager implements AclManager {
}
}
- private boolean isKnownSystemUser(String id) {
+ private void addStatements(SystemUser systemUser,
+ List<Acl> authorizations,
+ List<VaultPackageAssembler> packageAssemblers,
+ Formatter formatter) {
+ // clean the unneeded ACLs, see SLING-8561
+ if (authorizations != null) {
+ Iterator<Acl> authorizationsIterator = authorizations.iterator();
+ while (authorizationsIterator.hasNext()) {
+ Acl acl = authorizationsIterator.next();
+
+ if (acl.getPath().startsWith(systemUser.getPath())) {
+ authorizationsIterator.remove();
+ }
+ }
+ }
+
+ // make sure all paths are created first
+
+ addPaths(authorizations, packageAssemblers, formatter);
+
+ // finally add ACLs
+
+ addAclStatement(formatter, systemUser.getId(), authorizations);
+ }
+
+ private Optional<SystemUser> getSystemUser(String id) {
for (SystemUser systemUser : preProvidedSystemUsers) {
if (id.equals(systemUser.getId())) {
- return true;
+ return Optional.of(systemUser);
}
}
- return false;
+ return Optional.empty();
}
private final void addSystemUserPath(Formatter formatter, Path path) {
@@ -252,7 +256,7 @@ public final class DefaultAclManager implements AclManager {
}
private static void addAclStatement(Formatter formatter, String systemUser, List<Acl> authorizations) {
- if (areEmpty(authorizations)) {
+ if (authorizations == null || areEmpty(authorizations)) {
return;
}
diff --git a/src/test/java/org/apache/sling/feature/cpconverter/acl/AclManagerTest.java b/src/test/java/org/apache/sling/feature/cpconverter/acl/AclManagerTest.java
index a20f8be..fef2200 100644
--- a/src/test/java/org/apache/sling/feature/cpconverter/acl/AclManagerTest.java
+++ b/src/test/java/org/apache/sling/feature/cpconverter/acl/AclManagerTest.java
@@ -65,8 +65,8 @@ public class AclManagerTest {
aclManager.addSystemUser(new SystemUser("acs-commons-package-replication-status-event-service", Paths.get("/asd/public")));
- aclManager.addAcl("acs-commons-ensure-oak-index-service", newAcl("allow", "jcr:read,rep:write,rep:indexDefinitionManagement", "/asd/public"));
- aclManager.addAcl("acs-commons-package-replication-status-event-service", newAcl("allow", "jcr:read,crx:replicate,jcr:removeNode", "/asd/public"));
+ aclManager.addAcl("acs-commons-ensure-oak-index-service", newAcl("allow", "jcr:read,rep:write,rep:indexDefinitionManagement", "/asd/not/system/user/path"));
+ aclManager.addAcl("acs-commons-package-replication-status-event-service", newAcl("allow", "jcr:read,crx:replicate,jcr:removeNode", "/asd/public"));
// add an ACL for unknown user
aclManager.addAcl("acs-commons-on-deploy-scripts-service", newAcl("allow", "jcr:read,crx:replicate,jcr:removeNode", "/asd/public"));
@@ -84,13 +84,16 @@ public class AclManagerTest {
String expected = "create path (rep:AuthorizableFolder) /asd/public\n" + // SLING-8586
"create service user acs-commons-package-replication-status-event-service with path /asd/public\n" +
"create path (sling:Folder) /asd\n" +
- "create path (sling:Folder) /asd/public\n" +
+ "create path (sling:Folder) /asd/not\n" +
+ "create path (sling:Folder) /asd/not/system\n" +
+ "create path (sling:Folder) /asd/not/system/user\n" +
+ "create path (sling:Folder) /asd/not/system/user/path\n" +
// 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-ensure-oak-index-service\n" +
- "allow jcr:read,rep:write,rep:indexDefinitionManagement on /asd/public\n" +
+ "allow jcr:read,rep:write,rep:indexDefinitionManagement on /asd/not/system/user/path\n" +
"end\n";
String actual = repoinitExtension.getText();
assertEquals(expected, actual);