SLING-10281 support new "ensure principal ACL" statements (#14)
make repoinit throw exceptions in case principal acls can not be applied
fail even earlier when no principal access control list is available
improve logging in case principal acl is not available
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 68e1466..5f6da64 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
@@ -21,6 +21,7 @@
import org.apache.jackrabbit.api.security.JackrabbitAccessControlManager;
import org.apache.jackrabbit.api.security.JackrabbitAccessControlPolicy;
import org.apache.jackrabbit.api.security.authorization.PrincipalAccessControlList;
+import org.apache.jackrabbit.api.security.principal.ItemBasedPrincipal;
import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils;
import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
@@ -291,7 +292,7 @@
}
}
- public static void setPrincipalAcl(Session session, String principalName, Collection<AclLine> lines) throws RepositoryException {
+ public static void setPrincipalAcl(Session session, String principalName, Collection<AclLine> lines, boolean isStrict) throws RepositoryException {
final JackrabbitAccessControlManager acMgr = getJACM(session);
Principal principal = AccessControlUtils.getPrincipal(session, principalName);
if (principal == null) {
@@ -303,6 +304,14 @@
}
final PrincipalAccessControlList acl = getPrincipalAccessControlList(acMgr, principal, true);
+ if (acl == null && isStrict) {
+ String principalDescription = principal.getName();
+ // try to get path of principal in case it is backed by a JCR user/group
+ if (principal instanceof ItemBasedPrincipal) {
+ principalDescription += " (" + ((ItemBasedPrincipal) principal).getPath() + ")";
+ }
+ throw new IllegalStateException("No PrincipalAccessControlList available for principal '" + principalDescription + "'.");
+ }
boolean modified = false;
for (AclLine line : lines) {
AclLine.Action action = line.getAction();
@@ -407,7 +416,14 @@
private static boolean isValidPath(@NotNull Session session, @Nullable String jcrPath) throws RepositoryException {
return jcrPath == null || session.nodeExists(jcrPath);
}
-
+
+ /**
+ *
+ * @param acMgr the access control manager
+ * @param principal the principal
+ * @return the first available {@link PrincipalAccessControlList} bound to the given principal or {@code null} of <a href="https://jackrabbit.apache.org/oak/docs/security/authorization/principalbased.html">principal-based authorization</a> is not enabled for the given principal
+ * @throws RepositoryException
+ */
@Nullable
private static JackrabbitAccessControlList getAccessControlList(@NotNull AccessControlManager acMgr,
@Nullable String path, boolean includeApplicable) throws RepositoryException {
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclVisitor.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclVisitor.java
index b69dd24..3aef4ec 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclVisitor.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclVisitor.java
@@ -29,6 +29,7 @@
import org.apache.sling.repoinit.parser.operations.AclLine;
import org.apache.sling.repoinit.parser.operations.DeleteAclPrincipalBased;
import org.apache.sling.repoinit.parser.operations.DeleteAclPrincipals;
+import org.apache.sling.repoinit.parser.operations.EnsureAclPrincipalBased;
import org.apache.sling.repoinit.parser.operations.DeleteAclPaths;
import org.apache.sling.repoinit.parser.operations.RemoveAcePaths;
import org.apache.sling.repoinit.parser.operations.RemoveAcePrincipalBased;
@@ -101,6 +102,7 @@
}
}
+
@Override
public void visitSetAclPrincipal(SetAclPrincipals s) {
final List<String> principals = s.getPrincipals();
@@ -127,7 +129,19 @@
for (String principalName : s.getPrincipals()) {
try {
log.info("Adding principal-based access control entry for {}", principalName);
- AclUtil.setPrincipalAcl(session, principalName, s.getLines());
+ AclUtil.setPrincipalAcl(session, principalName, s.getLines(), false);
+ } catch (Exception e) {
+ report(e, "Failed to set principal-based ACL (" + e.getMessage() + ")");
+ }
+ }
+ }
+
+ @Override
+ public void visitEnsureAclPrincipalBased(EnsureAclPrincipalBased s) {
+ for (String principalName : s.getPrincipals()) {
+ try {
+ log.info("Enforcing principal-based access control entry for {}", principalName);
+ AclUtil.setPrincipalAcl(session, principalName, s.getLines(), true);
} catch (Exception e) {
report(e, "Failed to set principal-based ACL (" + e.getMessage() + ")");
}
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/DoNothingVisitor.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/DoNothingVisitor.java
index b9a5f39..8f313ec 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/DoNothingVisitor.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/DoNothingVisitor.java
@@ -29,6 +29,7 @@
import org.apache.sling.repoinit.parser.operations.DeleteServiceUser;
import org.apache.sling.repoinit.parser.operations.DeleteUser;
import org.apache.sling.repoinit.parser.operations.DisableServiceUser;
+import org.apache.sling.repoinit.parser.operations.EnsureAclPrincipalBased;
import org.apache.sling.repoinit.parser.operations.EnsureNodes;
import org.apache.sling.repoinit.parser.operations.OperationVisitor;
import org.apache.sling.repoinit.parser.operations.RegisterNamespace;
@@ -117,6 +118,11 @@
}
@Override
+ public void visitEnsureAclPrincipalBased(EnsureAclPrincipalBased ensureAclPrincipalBased) {
+ // no-op
+ }
+
+ @Override
public void visitRemoveAcePrincipal(RemoveAcePrincipals s) {
// no-op
}
diff --git a/src/test/java/org/apache/sling/jcr/repoinit/PrincipalBasedAclTest.java b/src/test/java/org/apache/sling/jcr/repoinit/PrincipalBasedAclTest.java
index c9468c1..8083a35 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/PrincipalBasedAclTest.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/PrincipalBasedAclTest.java
@@ -79,6 +79,7 @@
private static final String PRINCIPAL_BASED_SUBTREE = "principalbased";
private static final String REMOVE_NOT_SUPPORTED_REGEX = ".*REMOVE[a-zA-Z ]+not supported.*";
+ private static final String NO_PRINCIPAL_CONTROL_LIST_AVAILABLE = ".*No PrincipalAccessControlList available.*";
@Rule
public final OsgiContext context = new OsgiContext();
@@ -496,6 +497,26 @@
}
}
+ @Test(expected = None.class)
+ public void principalAclNotAvailableStrict() throws Exception {
+ try {
+ // create service user outside of supported tree for principal-based access control
+ U.parseAndExecute("create service user otherSystemPrincipal");
+ // principal-based ac-setup must fail as service user is not located below supported path
+ String setup = "ensure principal ACL for otherSystemPrincipal \n"
+ + "allow jcr:read on " + path + "\n"
+ + "end";
+ try {
+ U.parseAndExecute(setup);
+ fail("Setting a principal ACL outside a supported path must not succeed");
+ } catch (RuntimeException e) {
+ assertRegex(NO_PRINCIPAL_CONTROL_LIST_AVAILABLE, e.getMessage());
+ }
+ } finally {
+ U.cleanupServiceUser("otherSystemPrincipal");
+ }
+ }
+
@Test
public void principalAclNotAvailableRestrictionMismatch() throws Exception {
JackrabbitAccessControlManager acMgr = (JackrabbitAccessControlManager) adminSession.getAccessControlManager();
@@ -521,6 +542,37 @@
}
@Test
+ public void principalAclNotAvailableRestrictionMismatchStrict() throws Exception {
+ JackrabbitAccessControlManager acMgr = (JackrabbitAccessControlManager) adminSession.getAccessControlManager();
+ try {
+ // create service user outside of supported tree for principal-based access control
+ U.parseAndExecute("create service user otherSystemPrincipal");
+ // setup path-based access control to establish effective permission setup
+ String setup = "set ACL for otherSystemPrincipal \n"
+ + "allow jcr:read on " + path + "\n"
+ + "end";
+ U.parseAndExecute(setup);
+
+ Principal principal = adminSession.getUserManager().getAuthorizable("otherSystemPrincipal").getPrincipal();
+ assertTrue(acMgr.hasPrivileges(path, Collections.singleton(principal), AccessControlUtils.privilegesFromNames(adminSession, Privilege.JCR_READ)));
+
+ // setting up principal-acl will not succeed (principal not located below supported path)
+ // since effective entry doesn't match the restriction -> setup must fail
+ setup = "ensure principal ACL for otherSystemPrincipal \n"
+ + "allow jcr:read on " + path + " restriction(rep:glob,*mismatch)\n"
+ + "end";
+ try {
+ U.parseAndExecute(setup);
+ fail("Setting a principal ACL outside a supported path must not succeed");
+ } catch (RuntimeException e) {
+ assertRegex(NO_PRINCIPAL_CONTROL_LIST_AVAILABLE, e.getMessage());
+ }
+ } finally {
+ U.cleanupServiceUser("otherSystemPrincipal");
+ }
+ }
+
+ @Test
public void principalAclNotAvailableEntryPresent() throws Exception {
JackrabbitAccessControlManager acMgr = (JackrabbitAccessControlManager) adminSession.getAccessControlManager();
try {
@@ -551,6 +603,36 @@
}
@Test
+ public void principalAclNotAvailableEntryPresentStrict() throws Exception {
+ JackrabbitAccessControlManager acMgr = (JackrabbitAccessControlManager) adminSession.getAccessControlManager();
+ try {
+ // create service user outside of supported tree for principal-based access control
+ U.parseAndExecute("create service user otherSystemPrincipal");
+ // setup path-based access control to establish effective permission setup
+ String setup = "set ACL for otherSystemPrincipal \n"
+ + "allow jcr:read on " + path + "\n"
+ + "end";
+ U.parseAndExecute(setup);
+
+ Principal principal = adminSession.getUserManager().getAuthorizable("otherSystemPrincipal").getPrincipal();
+ assertTrue(acMgr.hasPrivileges(path, Collections.singleton(principal), AccessControlUtils.privilegesFromNames(adminSession, Privilege.JCR_READ)));
+
+ // setting up principal-acl will not succeed (principal not located below supported path)
+ setup = "ensure principal ACL for otherSystemPrincipal \n"
+ + "allow jcr:read on " + path + "\n"
+ + "end";
+ try {
+ U.parseAndExecute(setup);
+ fail("Setting a principal ACL outside a supported path must not succeed");
+ } catch (RuntimeException e) {
+ assertRegex(NO_PRINCIPAL_CONTROL_LIST_AVAILABLE, e.getMessage());
+ }
+ } finally {
+ U.cleanupServiceUser("otherSystemPrincipal");
+ }
+ }
+
+ @Test
public void principalAclNotAvailableEntryWithRestrictionPresent() throws Exception {
JackrabbitAccessControlManager acMgr = (JackrabbitAccessControlManager) adminSession.getAccessControlManager();
try {
@@ -579,6 +661,33 @@
}
@Test
+ public void principalAclNotAvailableEntryWithRestrictionPresentStrict() throws Exception {
+ JackrabbitAccessControlManager acMgr = (JackrabbitAccessControlManager) adminSession.getAccessControlManager();
+ try {
+ // create service user outside of supported tree for principal-based access control
+ U.parseAndExecute("create service user otherSystemPrincipal");
+ // setup path-based access control to establish effective permission setup
+ String setup = "set ACL for otherSystemPrincipal \n"
+ + "allow jcr:read on " + path + " restriction(rep:glob,*abc*)\n"
+ + "end";
+ U.parseAndExecute(setup);
+
+ // setting up principal-acl will not succeed (principal not located below supported path)
+ setup = "ensure principal ACL for otherSystemPrincipal \n"
+ + "allow jcr:read on " + path + " restriction(rep:glob,*abc*)\n"
+ + "end";
+ try {
+ U.parseAndExecute(setup);
+ fail("Setting a principal ACL outside a supported path must not succeed");
+ } catch (RuntimeException e) {
+ assertRegex(NO_PRINCIPAL_CONTROL_LIST_AVAILABLE, e.getMessage());
+ }
+ } finally {
+ U.cleanupServiceUser("otherSystemPrincipal");
+ }
+ }
+
+ @Test
public void principalAclNotAvailableRepoLevelPermissions() throws Exception {
JackrabbitAccessControlManager acMgr = (JackrabbitAccessControlManager) adminSession.getAccessControlManager();
try {
@@ -607,6 +716,34 @@
}
@Test
+ public void principalAclNotAvailableRepoLevelPermissionsStrict() throws Exception {
+ JackrabbitAccessControlManager acMgr = (JackrabbitAccessControlManager) adminSession.getAccessControlManager();
+ try {
+ // create service user outside of supported tree for principal-based access control
+ U.parseAndExecute("create service user otherSystemPrincipal");
+ // setup path-based access control to establish effective permission setup
+ String setup = "set ACL for otherSystemPrincipal \n"
+ + "allow jcr:namespaceManagement on :repository\n"
+ + "end";
+ U.parseAndExecute(setup);
+
+ // setting up principal-acl will not succeed (principal not located below supported path)
+ setup = "ensure principal ACL for otherSystemPrincipal \n"
+ + "allow jcr:namespaceManagement on :repository\n"
+ + "end";
+ try {
+ U.parseAndExecute(setup);
+ fail("Setting a principal ACL outside a supported path must not succeed");
+ } catch (RuntimeException e) {
+ assertRegex(NO_PRINCIPAL_CONTROL_LIST_AVAILABLE, e.getMessage());
+ }
+
+ } finally {
+ U.cleanupServiceUser("otherSystemPrincipal");
+ }
+ }
+
+ @Test
public void principalAclNotAvailableNonExistingNode() throws Exception {
JackrabbitAccessControlManager acMgr = (JackrabbitAccessControlManager) adminSession.getAccessControlManager();
try {
@@ -631,6 +768,34 @@
}
@Test
+ public void principalAclNotAvailableNonExistingNodeStrict() throws Exception {
+ JackrabbitAccessControlManager acMgr = (JackrabbitAccessControlManager) adminSession.getAccessControlManager();
+ try {
+ // create service user outside of supported tree for principal-based access control
+ U.parseAndExecute("create service user otherSystemPrincipal");
+
+ // setting up principal-acl will not succeed (principal not located below supported path)
+ String setup = "ensure principal ACL for otherSystemPrincipal \n"
+ + "allow jcr:read on /non/existing/path\n"
+ + "end";
+ try {
+ U.parseAndExecute(setup);
+ fail("Setting a principal ACL outside a supported path must not succeed");
+ } catch (RuntimeException e) {
+ assertRegex(NO_PRINCIPAL_CONTROL_LIST_AVAILABLE, e.getMessage());
+ }
+
+ Principal principal = getPrincipal("otherSystemPrincipal");
+ for (AccessControlPolicy policy : acMgr.getPolicies(principal)) {
+ assertFalse(policy instanceof PrincipalAccessControlList);
+ }
+
+ } finally {
+ U.cleanupServiceUser("otherSystemPrincipal");
+ }
+ }
+
+ @Test
public void testHomePath() throws Exception {
Authorizable su = getServiceUser(U.username);
Principal principal = su.getPrincipal();
@@ -641,7 +806,7 @@
line.setProperty(AclLine.PROP_PRINCIPALS, Collections.singletonList(principal.getName()));
line.setProperty(AclLine.PROP_PRIVILEGES, Collections.singletonList(Privilege.JCR_READ));
line.setProperty(AclLine.PROP_PATHS, Collections.singletonList(":home:"+U.username+"#"));
- AclUtil.setPrincipalAcl(U.adminSession, U.username, Collections.singletonList(line));
+ AclUtil.setPrincipalAcl(U.adminSession, U.username, Collections.singletonList(line), false);
PrincipalAccessControlList acl = getAcl(principal, U.adminSession);
assertNotNull(acl);
@@ -784,6 +949,34 @@
}
@Test
+ public void testRemovePrincipalMismatchStrict() throws Exception {
+ String setup = "ensure principal ACL for " + U.username + "\n"
+ + "allow jcr:write on "+path+"\n"
+ + "end";
+ U.parseAndExecute(setup);
+ U.parseAndExecute("create service user otherSystemPrincipal");
+ assertPolicy(getPrincipal(U.username), U.adminSession, 1);
+
+ setup = "remove principal ACE for otherSystemPrincipal\n"
+ + "allow jcr:write on " + path + "\n"
+ + "end";
+ U.parseAndExecute(setup);
+ // ace must not have been removed
+ assertPolicy(getPrincipal(U.username), U.adminSession, 1);
+ assertNull(getAcl(getPrincipal("otherSystemPrincipal"), U.adminSession));
+
+ try {
+ setup = "ensure principal ACL for otherSystemPrincipal\n"
+ + "remove jcr:write on " + path + "\n"
+ + "end";
+ U.parseAndExecute(setup);
+ fail("Expecting REMOVE to fail");
+ } catch(RuntimeException rex) {
+ assertRegex(NO_PRINCIPAL_CONTROL_LIST_AVAILABLE, rex.getMessage());
+ }
+ }
+
+ @Test
public void testRemoveAllNoExistingPolicy() throws Exception {
String setup = "set principal ACL for " + U.username + "\n"
+ "remove * on " + path + "\n"
@@ -902,6 +1095,25 @@
U.parseAndExecute(setup);
}
+ @Test(expected = None.class)
+ public void testRemoveAllPrincipalMismatchStrict() throws Exception {
+ String setup = "ensure principal ACL for " + U.username + "\n"
+ + "allow jcr:write on "+path+"\n"
+ + "end";
+ U.parseAndExecute(setup);
+ U.parseAndExecute("create service user otherSystemPrincipal");
+
+ setup = "ensure principal ACL for otherSystemPrincipal\n"
+ + "remove * on " + path + "\n"
+ + "end";
+ try {
+ U.parseAndExecute(setup);
+ fail("Setting a principal ACL outside a supported path must not succeed");
+ } catch (RuntimeException e) {
+ assertRegex(NO_PRINCIPAL_CONTROL_LIST_AVAILABLE, e.getMessage());
+ }
+ }
+
@Test
public void testRemoveAllPrincipalDuplicated() throws Exception {
U.parseAndExecute(""
