You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by kw...@apache.org on 2023/01/23 18:33:12 UTC

[sling-org-apache-sling-jcr-repoinit] branch master updated: SLING-10281 support new "ensure principal ACL" statements (#14)

This is an automated email from the ASF dual-hosted git repository.

kwin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git


The following commit(s) were added to refs/heads/master by this push:
     new 63e0d13  SLING-10281 support new "ensure principal ACL" statements (#14)
63e0d13 is described below

commit 63e0d135e7220165f5a761930309c60406474718
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Mon Jan 23 19:33:06 2023 +0100

    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
---
 .../apache/sling/jcr/repoinit/impl/AclUtil.java    |  20 +-
 .../apache/sling/jcr/repoinit/impl/AclVisitor.java |  16 +-
 .../sling/jcr/repoinit/impl/DoNothingVisitor.java  |   6 +
 .../sling/jcr/repoinit/PrincipalBasedAclTest.java  | 214 ++++++++++++++++++++-
 4 files changed, 252 insertions(+), 4 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 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.JackrabbitAccessControlList;
 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 class AclUtil {
         }
     }
 
-    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 @@ public class AclUtil {
         }
 
         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 @@ public class AclUtil {
     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 javax.jcr.Session;
 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 @@ class AclVisitor extends DoNothingVisitor {
         }
     }
 
+    
     @Override
     public void visitSetAclPrincipal(SetAclPrincipals s) {
         final List<String> principals = s.getPrincipals();
@@ -127,7 +129,19 @@ class AclVisitor extends DoNothingVisitor {
         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.DeleteGroup;
 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;
@@ -116,6 +117,11 @@ class DoNothingVisitor implements OperationVisitor {
         // no-op
     }
 
+    @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 @@ public class PrincipalBasedAclTest {
 
     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 @@ public class PrincipalBasedAclTest {
         }
     }
 
+    @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();
@@ -520,6 +541,37 @@ public class PrincipalBasedAclTest {
         }
     }
 
+    @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();
@@ -550,6 +602,36 @@ public class PrincipalBasedAclTest {
         }
     }
 
+    @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();
@@ -578,6 +660,33 @@ public class PrincipalBasedAclTest {
         }
     }
 
+    @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();
@@ -606,6 +715,34 @@ public class PrincipalBasedAclTest {
         }
     }
 
+    @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();
@@ -630,6 +767,34 @@ public class PrincipalBasedAclTest {
         }
     }
 
+    @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);
@@ -641,7 +806,7 @@ public class PrincipalBasedAclTest {
         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);
@@ -783,6 +948,34 @@ public class PrincipalBasedAclTest {
         assertNull(getAcl(getPrincipal("otherSystemPrincipal"), U.adminSession));
     }
 
+    @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"
@@ -902,6 +1095,25 @@ public class PrincipalBasedAclTest {
         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(""