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/07/04 14:07:08 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 937c5fd SLING-8561 - ACLs on service users should be ignored because of randomised node names
937c5fd is described below
commit 937c5fd01152e2fa27b65da06502b7a2cf6fe0dd
Author: Simo Tripodi <st...@adobe.com>
AuthorDate: Thu Jul 4 16:07:00 2019 +0200
SLING-8561 - ACLs on service users should be ignored because of
randomised node names
---
.../feature/cpconverter/acl/DefaultAclManager.java | 17 +++-
.../feature/cpconverter/acl/AclManagerTest.java | 11 ++-
.../handlers/RepPolicyEntryHandlerTest.java | 108 ++++++++++++++-------
3 files changed, 92 insertions(+), 44 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 79d8556..4db3d54 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
@@ -22,6 +22,7 @@ import java.nio.file.Path;
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;
@@ -106,12 +107,24 @@ public final class DefaultAclManager implements AclManager {
// system users
for (SystemUser systemUser : systemUsers) {
- List<Acl> authorizations = acls.remove(systemUser.getId());
-
// make sure all users are created first
formatter.format("create service user %s with path %s%n", systemUser.getId(), systemUser.getPath());
+ // 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);
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 b70baed..37afda1 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
@@ -55,7 +55,7 @@ public class AclManagerTest {
}
@Test
- public void makeSureAclsAreCreatedOnlyForKnownUsers() throws Exception {
+ public void makeSureAclsAreCreatedOnlyoutsideSytemUsersPaths() throws Exception {
aclManager.addSystemUser(new SystemUser("acs-commons-ensure-oak-index-service", Paths.get("/asd/public")));
// emulate a second iteration of conversion
@@ -81,10 +81,11 @@ 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 /asd/public\n" +
"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,crx:replicate,jcr:removeNode on /asd/public\n" +
- "end\n" +
+ "create path (sling:Folder) /asd/public\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" +
"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 97022a8..7bd8fc6 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
@@ -28,6 +28,7 @@ import static org.mockito.Mockito.when;
import java.io.File;
import java.io.StringReader;
+import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.List;
@@ -92,32 +93,33 @@ public final class RepPolicyEntryHandlerTest {
assertNotNull(repoinitExtension);
assertEquals(ExtensionType.TEXT, repoinitExtension.getType());
+ // commented ACLs are due SLING-8561
String expected = "create service user acs-commons-ensure-oak-index-service with path /asd/public\n" +
- "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 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 /asd/public\n" +
- "set ACL for acs-commons-dispatcher-flush-service\n" +
- "allow jcr:read,crx:replicate,jcr:removeNode on /asd/public\n" +
- "end\n" +
+ // "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 /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" +
+ // "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 /asd/public\n" +
- "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" +
+ // "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 /asd/public\n" +
- "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 /asd/public\n" +
- "set ACL for acs-commons-on-deploy-scripts-service\n" +
- "allow jcr:read on /asd/public\n" +
- "end\n";
+ // "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 /asd/public\n";
+ // "set ACL for acs-commons-on-deploy-scripts-service\n" +
+ // "allow jcr:read on /asd/public\n" +
+ // "end\n";
String actual = repoinitExtension.getText();
assertEquals(expected, actual);
@@ -135,23 +137,44 @@ 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 /asd/public\n" +
+ // "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 /asd/public\n" +
+ // "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 /asd/public\n" +
+ // "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 /asd/public\n";
+ //"set ACL for acs-commons-on-deploy-scripts-service\n" +
+ //"allow jcr:read on /asd/public\n" +
+ //"end\n";
+ 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 systemUserAclSetNotForUserPath() throws Exception {
+ Extension repoinitExtension = parseAndSetRepoinit(new SystemUser("acs-commons-package-replication-status-event-service", Paths.get("/this/is/a/completely/different/path")));
+ assertNotNull(repoinitExtension);
+ assertEquals(ExtensionType.TEXT, repoinitExtension.getType());
+
+ String expected = "create service user acs-commons-package-replication-status-event-service with path /this/is/a/completely/different/path\n" +
"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 /asd/public\n" +
- "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 /asd/public\n" +
- "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 /asd/public\n" +
- "set ACL for acs-commons-on-deploy-scripts-service\n" +
- "allow jcr:read on /asd/public\n" +
"end\n";
String actual = repoinitExtension.getText();
assertEquals(expected, actual);
@@ -163,11 +186,22 @@ public final class RepPolicyEntryHandlerTest {
@Test
public void parseEmptyAcl() throws Exception {
- Extension repoinitExtension = parseAndSetRepoinit();
+ Extension repoinitExtension = parseAndSetRepoinit(new String[] {});
assertNull(repoinitExtension);
}
- private Extension parseAndSetRepoinit(String...systemUsers) throws Exception {
+ private Extension parseAndSetRepoinit(String...systemUsersNames) throws Exception {
+ Path alwaysTheSamePath = Paths.get("/asd/public");
+
+ SystemUser[] systemUsers = new SystemUser[systemUsersNames.length];
+ for (int i = 0; i < systemUsersNames.length; i++) {
+ systemUsers[i] = new SystemUser(systemUsersNames[i], alwaysTheSamePath);
+ }
+
+ return parseAndSetRepoinit(systemUsers);
+ }
+
+ private Extension parseAndSetRepoinit(SystemUser...systemUsers) throws Exception {
String path = "/jcr_root/asd/public/_rep_policy.xml";
Archive archive = mock(Archive.class);
Entry entry = mock(Entry.class);
@@ -185,8 +219,8 @@ public final class RepPolicyEntryHandlerTest {
handler.handle(path, archive, entry, converter);
if (systemUsers != null) {
- for (String systemUser : systemUsers) {
- converter.getAclManager().addSystemUser(new SystemUser(systemUser, Paths.get("/asd/public")));
+ for (SystemUser systemUser : systemUsers) {
+ converter.getAclManager().addSystemUser(systemUser);
}
}