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 2022/04/04 13:37:55 UTC
[sling-org-apache-sling-jcr-repoinit] branch SLING-11245_11246_11249 created (now 010f809)
This is an automated email from the ASF dual-hosted git repository.
angela pushed a change to branch SLING-11245_11246_11249
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git
at 010f809 SLING-11245 : AclUtil.removePolicies(Session, List<String> paths) will fail if no policy exists
This branch includes the following new commits:
new 010f809 SLING-11245 : AclUtil.removePolicies(Session, List<String> paths) will fail if no policy exists
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-jcr-repoinit] 01/01: SLING-11245 : AclUtil.removePolicies(Session, List paths) will fail if no policy exists
Posted by an...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
angela pushed a commit to branch SLING-11245_11246_11249
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git
commit 010f809b5644b6b9c00fa61e496e9f0ee92c87ba
Author: angela <an...@adobe.com>
AuthorDate: Mon Apr 4 15:37:43 2022 +0200
SLING-11245 : AclUtil.removePolicies(Session, List<String> paths) will fail if no policy exists
SLING-11246 : Avoid repeated calls to Session.getAccessControlManager in AclUtil
SLING-11249 : Missing test for removing entries at non-existing path
---
.../apache/sling/jcr/repoinit/impl/AclUtil.java | 60 ++++++++++++++++------
.../apache/sling/jcr/repoinit/DeleteAclTest.java | 18 ++++++-
.../org/apache/sling/jcr/repoinit/RemoveTest.java | 23 +++++++++
3 files changed, 84 insertions(+), 17 deletions(-)
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
index 407c8a7..cc8dd9f 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
@@ -129,10 +129,12 @@ public class AclUtil {
private static void setAcl(Session session, List<String> principals, String jcrPath, List<String> privileges, boolean isAllow, List<RestrictionClause> restrictionClauses)
throws RepositoryException {
- final String [] privArray = privileges.toArray(new String[privileges.size()]);
- final Privilege[] jcrPriv = AccessControlUtils.privilegesFromNames(session, privArray);
+ AccessControlManager acMgr = session.getAccessControlManager();
- JackrabbitAccessControlList acl = AccessControlUtils.getAccessControlList(session, jcrPath);
+ final String [] privArray = privileges.toArray(new String[privileges.size()]);
+ final Privilege[] jcrPriv = AccessControlUtils.privilegesFromNames(acMgr, privArray);
+
+ JackrabbitAccessControlList acl = getAccessControlList(acMgr, jcrPath, true);
checkState(acl != null, "No JackrabbitAccessControlList available for path " + jcrPath);
LocalRestrictions localRestrictions = createLocalRestrictions(restrictionClauses, acl, session);
@@ -159,7 +161,7 @@ public class AclUtil {
changed = true;
}
if ( changed ) {
- session.getAccessControlManager().setPolicy(jcrPath, acl);
+ acMgr.setPolicy(jcrPath, acl);
}
}
@@ -202,24 +204,30 @@ public class AclUtil {
* @throws RepositoryException
*/
public static void removePolicies(@NotNull Session session, @NotNull List<String> paths) throws RepositoryException {
+ AccessControlManager acMgr = session.getAccessControlManager();
for (String jcrPath : getJcrPaths(session, paths)) {
- LOG.info("Removing access control policy on {}", jcrPath);
- JackrabbitAccessControlList acl = AccessControlUtils.getAccessControlList(session, jcrPath);
+ if (!isValidPath(session, jcrPath)) {
+ LOG.info("Cannot remove ACL; no node at {} ", jcrPath);
+ continue;
+ }
+ LOG.info("Removing access control policy at {}", jcrPath);
+ JackrabbitAccessControlList acl = getAccessControlList(acMgr, jcrPath, false);
if (acl == null) {
LOG.info("No ACL to remove at path {}", jcrPath);
} else {
- session.getAccessControlManager().removePolicy(jcrPath, acl);
+ acMgr.removePolicy(jcrPath, acl);
}
}
}
public static void removeEntries(@NotNull Session session, @NotNull List<String> principals, @NotNull List<String> paths) throws RepositoryException {
Set<String> principalNames = new HashSet<>(principals);
+ AccessControlManager acMgr = session.getAccessControlManager();
for (String jcrPath : getJcrPaths(session, paths)) {
- if (jcrPath != null && !session.nodeExists(jcrPath)) {
+ if (!isValidPath(session, jcrPath)) {
LOG.info("Cannot remove access control entries on non-existent path {}", jcrPath);
} else {
- JackrabbitAccessControlList acl = AccessControlUtils.getAccessControlList(session, jcrPath);
+ JackrabbitAccessControlList acl = getAccessControlList(acMgr, jcrPath, false);
if (acl != null) {
boolean modified = false;
for (AccessControlEntry ace : acl.getAccessControlEntries()) {
@@ -229,7 +237,7 @@ public class AclUtil {
}
}
if (modified) {
- session.getAccessControlManager().setPolicy(jcrPath, acl);
+ acMgr.setPolicy(jcrPath, acl);
}
} else {
LOG.info("Cannot remove access control entries for principal(s) {}. No ACL at {}", principalNames, jcrPath);
@@ -240,16 +248,17 @@ public class AclUtil {
public static void removeEntries(@NotNull Session session, @NotNull List<String> principals, @NotNull List<String> paths, List<String> privileges, boolean isAllow, List<RestrictionClause> restrictionClauses) throws RepositoryException {
Set<String> principalNames = new HashSet<>(principals);
+ AccessControlManager acMgr = session.getAccessControlManager();
for (String jcrPath : getJcrPaths(session, paths)) {
- if (jcrPath != null && !session.nodeExists(jcrPath)) {
+ if (!isValidPath(session, jcrPath)) {
LOG.info("Cannot remove access control entries on non-existent path {}", jcrPath);
} else {
- JackrabbitAccessControlList acl = AccessControlUtils.getAccessControlList(session, jcrPath);
+ JackrabbitAccessControlList acl = getAccessControlList(acMgr, jcrPath, false);
if (acl != null) {
boolean modified = false;
LocalRestrictions restr = createLocalRestrictions(restrictionClauses, acl, session);
- Privilege[] privs = AccessControlUtils.privilegesFromNames(session, privileges.toArray(new String[0]));
+ Privilege[] privs = AccessControlUtils.privilegesFromNames(acMgr, privileges.toArray(new String[0]));
for (AccessControlEntry ace : acl.getAccessControlEntries()) {
Principal principal = ace.getPrincipal();
@@ -263,7 +272,7 @@ public class AclUtil {
}
}
if (modified) {
- session.getAccessControlManager().setPolicy(jcrPath, acl);
+ acMgr.setPolicy(jcrPath, acl);
} else {
LOG.info("No matching access control entry found to remove for principals {} at {}. Expected entry with isAllow={}, privileges={}, restrictions={}", principalNames, jcrPath, isAllow, privileges, restrictionClauses);
}
@@ -299,7 +308,7 @@ public class AclUtil {
modified = true;
}
} else if (action == AclLine.Action.ALLOW) {
- final Privilege[] privileges = AccessControlUtils.privilegesFromNames(session, line.getProperty(PROP_PRIVILEGES).toArray(new String[0]));
+ final Privilege[] privileges = AccessControlUtils.privilegesFromNames(acMgr, line.getProperty(PROP_PRIVILEGES).toArray(new String[0]));
for (String effectivePath : jcrPaths) {
if (acl == null) {
// no PrincipalAccessControlList available: don't fail if an equivalent path-based entry with the same definition exists
@@ -345,7 +354,7 @@ public class AclUtil {
List<String> jcrPaths = getJcrPaths(session, line.getProperty(PROP_PATHS));
LocalRestrictions restr = createLocalRestrictions(line.getRestrictions(), acl, session);
List<String> privNames = line.getProperty(PROP_PRIVILEGES);
- Privilege[] privs = AccessControlUtils.privilegesFromNames(session, privNames.toArray(new String[0]));
+ Privilege[] privs = AccessControlUtils.privilegesFromNames(acMgr, privNames.toArray(new String[0]));
Predicate<PrincipalAccessControlList.Entry> predicate = entry -> {
if (!jcrPaths.contains(entry.getEffectivePath())) {
return false;
@@ -386,6 +395,25 @@ public class AclUtil {
acMgr.removePolicy(acl.getPath(), acl);
}
}
+
+ private static boolean isValidPath(@NotNull Session session, @Nullable String jcrPath) throws RepositoryException {
+ return jcrPath == null || session.nodeExists(jcrPath);
+ }
+
+ @Nullable
+ private static JackrabbitAccessControlList getAccessControlList(@NotNull AccessControlManager acMgr,
+ @Nullable String path, boolean includeApplicable) throws RepositoryException {
+ if (includeApplicable) {
+ return AccessControlUtils.getAccessControlList(acMgr, path);
+ } else {
+ for (AccessControlPolicy policy : acMgr.getPolicies(path)) {
+ if (policy instanceof JackrabbitAccessControlList) {
+ return (JackrabbitAccessControlList) policy;
+ }
+ }
+ return null;
+ }
+ }
@Nullable
private static PrincipalAccessControlList getPrincipalAccessControlList(@NotNull JackrabbitAccessControlManager acMgr,
diff --git a/src/test/java/org/apache/sling/jcr/repoinit/DeleteAclTest.java b/src/test/java/org/apache/sling/jcr/repoinit/DeleteAclTest.java
index 306e63f..ed19535 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/DeleteAclTest.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/DeleteAclTest.java
@@ -50,7 +50,9 @@ import java.util.Collection;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
@RunWith(Parameterized.class)
public class DeleteAclTest {
@@ -94,6 +96,7 @@ public class DeleteAclTest {
U.parseAndExecute(""
+ "create path (nt:unstructured) /content\n"
+ "create path (nt:unstructured) /var\n"
+ + "create path (nt:unstructured) /no/policy\n"
+ "set ACL for " + U.username + "\n"
+ "allow jcr:read on /content, /var, home("+userA+")\n"
+ "allow jcr:namespaceManagement on :repository\n"
@@ -131,9 +134,22 @@ public class DeleteAclTest {
assertArrayEquals(new AccessControlPolicy[0], acMgr.getPolicies((String) null));
}
- @Test(expected = RuntimeException.class)
+ @Test
public void testDeleteAclNonExistingPath() throws Exception {
+ assertFalse(adminSession.nodeExists("/nonExisting"));
U.parseAndExecute("delete ACL on /nonExisting\n");
+ assertFalse(adminSession.nodeExists("/nonExisting"));
+ }
+
+ @Test
+ public void testDeleteNonExistingAcl() throws Exception {
+ assertTrue(adminSession.nodeExists("/no/policy"));
+ assertArrayEquals(new AccessControlPolicy[0], acMgr.getPolicies("/no/policy"));
+
+ U.parseAndExecute("delete ACL on /no/policy\n");
+
+ assertTrue(adminSession.nodeExists("/no/policy"));
+ assertArrayEquals(new AccessControlPolicy[0], acMgr.getPolicies("/no/policy"));
}
@Test
diff --git a/src/test/java/org/apache/sling/jcr/repoinit/RemoveTest.java b/src/test/java/org/apache/sling/jcr/repoinit/RemoveTest.java
index aa13145..cdce414 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/RemoveTest.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/RemoveTest.java
@@ -41,6 +41,7 @@ import javax.jcr.security.AccessControlPolicy;
import java.util.UUID;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.fail;
public class RemoveTest {
@@ -170,6 +171,28 @@ public class RemoveTest {
"end";
U.parseAndExecute(setup);
}
+
+ @Test
+ public void testRemoveByNonExistingPath() throws Exception {
+ String path = "/non/existing";
+ String setup = "remove ACE for " + U.username + "\n"
+ + "deny jcr:read on "+path+"\n"
+ + "end";
+ U.parseAndExecute(setup);
+ assertFalse(U.adminSession.nodeExists(path));
+
+ setup = "remove ACE on "+path+"\n"
+ + "deny jcr:read for " + U.username + "\n"
+ + "end";
+ U.parseAndExecute(setup);
+ assertFalse(U.adminSession.nodeExists(path));
+
+ setup = "set ACL on "+path+"\n" +
+ "remove * for "+U.username+"\n" +
+ "end";
+ U.parseAndExecute(setup);
+ assertFalse(U.adminSession.nodeExists(path));
+ }
@Test
public void testRemoveByPath() throws RepoInitParsingException, RepositoryException {