This is an automated email from the ASF dual-hosted git repository. joerghoh pushed a commit to branch SLING-12642-cache-privileges in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git
commit c46758487a77ad3a7e27a16fa823736f20dc56c3 Author: Joerg Hoh <[email protected]> AuthorDate: Sun Feb 9 14:38:03 2025 +0100 also cache the expansion of aggregated privileges --- .../apache/sling/jcr/repoinit/impl/AclUtil.java | 36 ++++++++++++---------- .../impl/PrivilegeCachingSessionWrapper.java | 21 ++++++++++++- .../sling/jcr/repoinit/impl/AclUtilTest.java | 2 +- 3 files changed, 40 insertions(+), 19 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 a459c45..9d8d0af 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 @@ -184,7 +184,7 @@ public class AclUtil { for (String name : principals) { final Principal principal = getPrincipal(pcsw.getSession(), name, ignoreMissingPrincipal); LocalAccessControlEntry newAce = - new LocalAccessControlEntry(principal, jcrPriv, isAllow, localRestrictions); + new LocalAccessControlEntry(pcsw, principal, jcrPriv, isAllow, localRestrictions); if (contains(existingAces, newAce)) { LOG.info( "Not adding {} to path {} since an equivalent access control entry already exists", @@ -340,7 +340,7 @@ public class AclUtil { continue; } LocalAccessControlEntry entry = - new LocalAccessControlEntry(ace.getPrincipal(), privs, isAllow, restr); + new LocalAccessControlEntry(pcsw, ace.getPrincipal(), privs, isAllow, restr); if (entry.isEqual(ace)) { acl.removeAccessControlEntry(ace); modified = true; @@ -413,7 +413,7 @@ public class AclUtil { // or if there exists no node at the effective path (unable to evaluate path-based entries). LOG.info("No PrincipalAccessControlList available for principal {}", principal); if (!containsEquivalentEntry( - pcsw.getSession(), effectivePath, principal, privileges, true, line.getRestrictions())) { + pcsw, effectivePath, principal, privileges, true, line.getRestrictions())) { LOG.warn( "No equivalent path-based entry exists for principal {} and effective path {} ", principal.getName(), @@ -470,7 +470,7 @@ public class AclUtil { if (!jcrPaths.contains(entry.getEffectivePath())) { return false; } - LocalAccessControlEntry lace = new LocalAccessControlEntry( + LocalAccessControlEntry lace = new LocalAccessControlEntry(pcsw, entry.getPrincipal(), privs, line.getAction() == AclLine.Action.ALLOW, restr); return lace.isEqual(entry); }; @@ -622,25 +622,25 @@ public class AclUtil { } private static boolean containsEquivalentEntry( - Session session, + PrivilegeCachingSessionWrapper pcsw, String absPath, Principal principal, Privilege[] privileges, boolean isAllow, List<RestrictionClause> restrictionList) throws RepositoryException { - if (absPath != null && !session.nodeExists(absPath)) { + if (absPath != null && !pcsw.getSession().nodeExists(absPath)) { LOG.info( "Cannot determine existence of equivalent path-based entry for principal {}. No node at path {} ", principal.getName(), absPath); return true; } - for (AccessControlPolicy policy : session.getAccessControlManager().getPolicies(absPath)) { + for (AccessControlPolicy policy : pcsw.getAccessControlManager().getPolicies(absPath)) { if (policy instanceof JackrabbitAccessControlList) { LocalRestrictions lr = - createLocalRestrictions(restrictionList, ((JackrabbitAccessControlList) policy), session); - LocalAccessControlEntry newEntry = new LocalAccessControlEntry(principal, privileges, isAllow, lr); + createLocalRestrictions(restrictionList, ((JackrabbitAccessControlList) policy), pcsw.getSession()); + LocalAccessControlEntry newEntry = new LocalAccessControlEntry(pcsw,principal, privileges, isAllow, lr); if (contains(((JackrabbitAccessControlList) policy).getAccessControlEntries(), newEntry)) { LOG.info( "Equivalent path-based entry exists for principal {} and effective path {} ", @@ -711,13 +711,19 @@ public class AclUtil { private final Privilege[] privileges; private final boolean isAllow; private final LocalRestrictions restrictions; + private final PrivilegeCachingSessionWrapper pcsw; - LocalAccessControlEntry(Principal principal, Privilege[] privileges, boolean isAllow) { - this(principal, privileges, isAllow, null); + LocalAccessControlEntry(PrivilegeCachingSessionWrapper pcsw, Principal principal, Privilege[] privileges, boolean isAllow) { + this(pcsw,principal, privileges, isAllow, null); } LocalAccessControlEntry( - Principal principal, Privilege[] privileges, boolean isAllow, LocalRestrictions restrictions) { + PrivilegeCachingSessionWrapper pcsw, + Principal principal, + Privilege[] privileges, + boolean isAllow, + LocalRestrictions restrictions) { + this.pcsw = pcsw; this.principal = principal; this.privileges = privileges; this.isAllow = isAllow; @@ -751,11 +757,7 @@ public class AclUtil { if (privileges != null) { for (Privilege privilege : privileges) { - if (privilege.isAggregate()) { - expandedSet.addAll(Arrays.asList(privilege.getAggregatePrivileges())); - } else { - expandedSet.add(privilege); - } + expandedSet.addAll(pcsw.expandPrivilege(privilege)); } } diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java index 9c642a6..e38b10f 100644 --- a/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java +++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java @@ -18,19 +18,22 @@ */ package org.apache.sling.jcr.repoinit.impl; +import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import javax.jcr.RepositoryException; import javax.jcr.Session; -import javax.jcr.security.AccessControlManager; import javax.jcr.security.Privilege; import org.apache.jackrabbit.api.JackrabbitSession; import org.apache.jackrabbit.api.security.JackrabbitAccessControlManager; +import com.google.common.collect.Lists; + /** * A simple wrapper around a session, which can cache the privilege resolution */ @@ -39,6 +42,7 @@ public class PrivilegeCachingSessionWrapper { JackrabbitSession session; JackrabbitAccessControlManager acMgr; Map<String,Privilege> nameToPrivilegeMap = new HashMap<>(); + Map<Privilege,List<Privilege>> privilegeToAggreate = new HashMap<>(); public PrivilegeCachingSessionWrapper (Session session) { AclUtil.checkState(session instanceof JackrabbitSession,"A Jackrabbit Session is required"); @@ -80,4 +84,19 @@ public class PrivilegeCachingSessionWrapper { } return privileges.toArray(new Privilege[privileges.size()]); } + + /** + * If a privilege is an aggreated, return the privilges it contains, otherwise return the privilege itself + * @param priv the privilege + * @return + */ + public List<Privilege> expandPrivilege (Privilege priv) { + return privilegeToAggreate.computeIfAbsent(priv, (p) -> { + if (p.isAggregate()) { + return Arrays.asList(p.getAggregatePrivileges()); + } else { + return Lists.newArrayList(p); + } + }); + } } diff --git a/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java b/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java index 2d72021..8dc7700 100644 --- a/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java +++ b/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java @@ -607,7 +607,7 @@ public class AclUtilTest { boolean contained) throws RepositoryException { AclUtil.LocalAccessControlEntry localAce = - new AclUtil.LocalAccessControlEntry(principal(username), privileges(privilegeNames), isAllow); + new AclUtil.LocalAccessControlEntry(toPCSessionWrapper(U.adminSession),principal(username), privileges(privilegeNames), isAllow); if (contained) { assertTrue(
