You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by jo...@apache.org on 2025/02/19 10:58:08 UTC
(sling-org-apache-sling-jcr-repoinit) branch master updated: SLING-12642 performance improvements for privilege and principal handling (#58)
This is an automated email from the ASF dual-hosted git repository.
joerghoh 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 19d29cd SLING-12642 performance improvements for privilege and principal handling (#58)
19d29cd is described below
commit 19d29cd7f6d76c94836a1ceee48be560f3a02cb1
Author: Jörg Hoh <jo...@users.noreply.github.com>
AuthorDate: Wed Feb 19 11:58:03 2025 +0100
SLING-12642 performance improvements for privilege and principal handling (#58)
* Use privilegeCollections whenever possible, as they have a lower overhead than resolving all privileges individually
* cache the principal resolution
Co-authored-by: angela <an...@adobe.com>
---
.../apache/sling/jcr/repoinit/impl/AclUtil.java | 213 +++++++++------------
.../apache/sling/jcr/repoinit/impl/AclVisitor.java | 23 ++-
.../sling/jcr/repoinit/impl/SessionContext.java | 89 +++++++++
.../sling/jcr/repoinit/PrincipalBasedAclTest.java | 3 +-
.../sling/jcr/repoinit/impl/AclUtilTest.java | 61 +++---
.../jcr/repoinit/impl/ManyServiceUsersTest.java | 2 +-
.../apache/sling/jcr/repoinit/impl/TestUtil.java | 4 +-
7 files changed, 227 insertions(+), 168 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 e8a9e0b..215032a 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
@@ -27,7 +27,6 @@ import javax.jcr.security.AccessControlEntry;
import javax.jcr.security.AccessControlException;
import javax.jcr.security.AccessControlManager;
import javax.jcr.security.AccessControlPolicy;
-import javax.jcr.security.Privilege;
import java.security.Principal;
import java.text.MessageFormat;
@@ -48,6 +47,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.authorization.PrivilegeCollection;
import org.apache.jackrabbit.api.security.principal.ItemBasedPrincipal;
import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils;
@@ -132,13 +132,13 @@ public class AclUtil {
}
public static void setAcl(
- Session session, List<String> principals, List<String> paths, List<String> privileges, boolean isAllow)
+ SessionContext context, List<String> principals, List<String> paths, List<String> privileges, boolean isAllow)
throws RepositoryException {
- setAcl(session, principals, paths, privileges, isAllow, Collections.emptyList(), Collections.emptyList());
+ setAcl(context, principals, paths, privileges, isAllow, Collections.emptyList(), Collections.emptyList());
}
public static void setAcl(
- Session session,
+ SessionContext context,
List<String> principals,
List<String> paths,
List<String> privileges,
@@ -147,16 +147,16 @@ public class AclUtil {
List<String> options)
throws RepositoryException {
- for (String jcrPath : getJcrPaths(session, paths)) {
- if (jcrPath != null && !session.nodeExists(jcrPath)) {
+ for (String jcrPath : getJcrPaths(context.getSession(), paths)) {
+ if (jcrPath != null && !context.getSession().nodeExists(jcrPath)) {
throw new PathNotFoundException("Cannot set ACL on non-existent path " + jcrPath);
}
- setAcl(session, principals, jcrPath, privileges, isAllow, restrictionClauses, options);
+ setAcl(context, principals, jcrPath, privileges, isAllow, restrictionClauses, options);
}
}
private static void setAcl(
- Session session,
+ SessionContext context,
List<String> principals,
String jcrPath,
List<String> privileges,
@@ -165,15 +165,14 @@ public class AclUtil {
List<String> options)
throws RepositoryException {
- AccessControlManager acMgr = session.getAccessControlManager();
-
- final String[] privArray = privileges.toArray(new String[privileges.size()]);
- final Privilege[] jcrPriv = AccessControlUtils.privilegesFromNames(acMgr, privArray);
+ AccessControlManager acMgr = context.getAccessControlManager();
+ final String[] privArray = privileges.toArray(new String[0]);
+
JackrabbitAccessControlList acl = getAccessControlList(acMgr, jcrPath, true);
checkState(acl != null, "No JackrabbitAccessControlList available for path {0}", jcrPath);
- LocalRestrictions localRestrictions = createLocalRestrictions(restrictionClauses, acl, session);
+ LocalRestrictions localRestrictions = createLocalRestrictions(restrictionClauses, acl, context.getSession());
AccessControlEntry[] existingAces = acl.getAccessControlEntries();
@@ -182,9 +181,9 @@ public class AclUtil {
.map(o -> o.contains(AclVisitor.OPTION_IGNORE_MISSING_PRINCIPAL))
.orElse(false);
for (String name : principals) {
- final Principal principal = getPrincipal(session, name, ignoreMissingPrincipal);
+ final Principal principal = getPrincipal(context, name, ignoreMissingPrincipal);
LocalAccessControlEntry newAce =
- new LocalAccessControlEntry(principal, jcrPriv, isAllow, localRestrictions);
+ new LocalAccessControlEntry(context, principal, privArray, isAllow, localRestrictions);
if (contains(existingAces, newAce)) {
LOG.info(
"Not adding {} to path {} since an equivalent access control entry already exists",
@@ -194,7 +193,7 @@ public class AclUtil {
}
acl.addEntry(
newAce.principal,
- newAce.privileges,
+ newAce.getPrivilegeCollection().getPrivileges(),
newAce.isAllow,
newAce.restrictions.getRestrictions(),
newAce.restrictions.getMVRestrictions());
@@ -206,13 +205,13 @@ public class AclUtil {
}
@NotNull
- private static Principal getPrincipal(Session session, String name, boolean ignoreMissingPrincipal)
+ private static Principal getPrincipal(SessionContext context, String name, boolean ignoreMissingPrincipal)
throws RepositoryException {
- Principal principal = AccessControlUtils.getPrincipal(session, name);
+ Principal principal = context.getPrincipal(name);
if (principal == null) {
// backwards compatibility: fallback to original code treating principal name as authorizable ID (see
// SLING-8604)
- final Authorizable authorizable = UserUtil.getAuthorizable(session, name);
+ final Authorizable authorizable = UserUtil.getAuthorizable(context.getSession(), name);
if (!ignoreMissingPrincipal) {
checkState(authorizable != null, "Authorizable not found: {0}", name);
principal = authorizable.getPrincipal();
@@ -232,13 +231,13 @@ public class AclUtil {
/**
* Remove resource-based access control setup for the principal with the given name.
*
- * @param session
+ * @param context
* @param principalName
* @throws RepositoryException
*/
- public static void removePolicy(@NotNull Session session, @NotNull final String principalName)
+ public static void removePolicy(@NotNull SessionContext context, @NotNull final String principalName)
throws RepositoryException {
- Principal principal = AccessControlUtils.getPrincipal(session, principalName);
+ Principal principal = context.getPrincipal(principalName);
if (principal == null) {
LOG.info("Principal {} does not exist.", principalName);
// using PrincipalImpl will prevent 'removePolicy' from failing with AccessControlException
@@ -246,7 +245,7 @@ public class AclUtil {
principal = new PrincipalImpl(principalName);
}
- JackrabbitAccessControlManager acMgr = getJACM(session);
+ JackrabbitAccessControlManager acMgr = context.getAccessControlManager();
for (JackrabbitAccessControlPolicy policy : acMgr.getPolicies(principal)) {
// make sure not to remove the principal-based access control list but instead only drop
// resource-based access control content for the given principal
@@ -259,15 +258,15 @@ public class AclUtil {
/**
* Remove resource-based access control setup defined for the specified paths.
*
- * @param session
+ * @param context
* @param paths
* @throws RepositoryException
*/
- public static void removePolicies(@NotNull Session session, @NotNull List<String> paths)
+ public static void removePolicies(@NotNull SessionContext context, @NotNull List<String> paths)
throws RepositoryException {
- AccessControlManager acMgr = session.getAccessControlManager();
- for (String jcrPath : getJcrPaths(session, paths)) {
- if (!isValidPath(session, jcrPath)) {
+ AccessControlManager acMgr = context.getAccessControlManager();
+ for (String jcrPath : getJcrPaths(context.session, paths)) {
+ if (!isValidPath(context.getSession(), jcrPath)) {
LOG.info("Cannot remove ACL; no node at {} ", jcrPath);
continue;
}
@@ -282,12 +281,12 @@ public class AclUtil {
}
public static void removeEntries(
- @NotNull Session session, @NotNull List<String> principals, @NotNull List<String> paths)
+ @NotNull SessionContext context, @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 (!isValidPath(session, jcrPath)) {
+ AccessControlManager acMgr = context.getAccessControlManager();
+ for (String jcrPath : getJcrPaths(context.getSession(), paths)) {
+ if (!isValidPath(context.getSession(), jcrPath)) {
LOG.info("Cannot remove access control entries on non-existent path {}", jcrPath);
} else {
JackrabbitAccessControlList acl = getAccessControlList(acMgr, jcrPath, false);
@@ -313,7 +312,7 @@ public class AclUtil {
}
public static void removeEntries(
- @NotNull Session session,
+ @NotNull SessionContext context,
@NotNull List<String> principals,
@NotNull List<String> paths,
List<String> privileges,
@@ -321,26 +320,23 @@ public class AclUtil {
List<RestrictionClause> restrictionClauses)
throws RepositoryException {
Set<String> principalNames = new HashSet<>(principals);
- AccessControlManager acMgr = session.getAccessControlManager();
- for (String jcrPath : getJcrPaths(session, paths)) {
- if (!isValidPath(session, jcrPath)) {
+ AccessControlManager acMgr = context.getAccessControlManager();
+ for (String jcrPath : getJcrPaths(context.getSession(), paths)) {
+ if (!isValidPath(context.getSession(), jcrPath)) {
LOG.info("Cannot remove access control entries on non-existent path {}", jcrPath);
} else {
JackrabbitAccessControlList acl = getAccessControlList(acMgr, jcrPath, false);
if (acl != null) {
boolean modified = false;
- LocalRestrictions restr = createLocalRestrictions(restrictionClauses, acl, session);
- Privilege[] privs =
- AccessControlUtils.privilegesFromNames(acMgr, privileges.toArray(new String[0]));
-
+ LocalRestrictions restr = createLocalRestrictions(restrictionClauses, acl, context.getSession());
for (AccessControlEntry ace : acl.getAccessControlEntries()) {
Principal principal = ace.getPrincipal();
if (!principalNames.contains(principal.getName())) {
continue;
}
LocalAccessControlEntry entry =
- new LocalAccessControlEntry(ace.getPrincipal(), privs, isAllow, restr);
+ new LocalAccessControlEntry(context, ace.getPrincipal(), privileges.toArray(new String[0]), isAllow, restr);
if (entry.isEqual(ace)) {
acl.removeAccessControlEntry(ace);
modified = true;
@@ -368,17 +364,11 @@ public class AclUtil {
}
public static void setPrincipalAcl(
- Session session, String principalName, Collection<AclLine> lines, boolean isStrict)
+ SessionContext context, String principalName, Collection<AclLine> lines, boolean isStrict)
throws RepositoryException {
- final JackrabbitAccessControlManager acMgr = getJACM(session);
- Principal principal = AccessControlUtils.getPrincipal(session, principalName);
- if (principal == null) {
- // due to transient nature of the repo-init the principal lookup may not succeed if completed through query
- // -> save transient changes and retry principal lookup
- session.save();
- principal = AccessControlUtils.getPrincipal(session, principalName);
- checkState(principal != null, PRINCIPAL_NOT_FOUND_PATTERN, principalName);
- }
+ final JackrabbitAccessControlManager acMgr = context.getAccessControlManager();
+ Principal principal = context.getPrincipalWithSave(principalName);
+ checkState(principal != null, PRINCIPAL_NOT_FOUND_PATTERN, principalName);
final PrincipalAccessControlList acl = getPrincipalAccessControlList(acMgr, principal, true);
if (acl == null && isStrict) {
@@ -393,7 +383,7 @@ public class AclUtil {
boolean modified = false;
for (AclLine line : lines) {
AclLine.Action action = line.getAction();
- List<String> jcrPaths = getJcrPaths(session, line.getProperty(PROP_PATHS));
+ List<String> jcrPaths = getJcrPaths(context.getSession(), line.getProperty(PROP_PATHS));
if (action == AclLine.Action.DENY) {
throw new AccessControlException("PrincipalAccessControlList doesn't support 'deny' entries.");
} else if (action == AclLine.Action.REMOVE) {
@@ -404,8 +394,7 @@ public class AclUtil {
modified = true;
}
} else if (action == AclLine.Action.ALLOW) {
- final Privilege[] privileges = AccessControlUtils.privilegesFromNames(
- acMgr, line.getProperty(PROP_PRIVILEGES).toArray(new String[0]));
+ final String[] privilegeNames = 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
@@ -413,7 +402,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(
- session, effectivePath, principal, privileges, true, line.getRestrictions())) {
+ context, effectivePath, principal, privilegeNames, true, line.getRestrictions())) {
LOG.warn(
"No equivalent path-based entry exists for principal {} and effective path {} ",
principal.getName(),
@@ -422,10 +411,10 @@ public class AclUtil {
}
} else {
final LocalRestrictions restrictions =
- createLocalRestrictions(line.getRestrictions(), acl, session);
+ createLocalRestrictions(line.getRestrictions(), acl, context.getSession());
final boolean added = acl.addEntry(
effectivePath,
- privileges,
+ context.privilegeCollectionFromNames(privilegeNames).getPrivileges(),
restrictions.getRestrictions(),
restrictions.getMVRestrictions());
if (!added) {
@@ -447,31 +436,24 @@ public class AclUtil {
}
}
- public static void removePrincipalEntries(Session session, String principalName, Collection<AclLine> lines)
+ public static void removePrincipalEntries(@NotNull SessionContext context, String principalName, Collection<AclLine> lines)
throws RepositoryException {
- final JackrabbitAccessControlManager acMgr = getJACM(session);
- Principal principal = AccessControlUtils.getPrincipal(session, principalName);
- if (principal == null) {
- // due to transient nature of the repo-init the principal lookup may not succeed if completed through query
- // -> save transient changes and retry principal lookup
- session.save();
- principal = AccessControlUtils.getPrincipal(session, principalName);
- checkState(principal != null, PRINCIPAL_NOT_FOUND_PATTERN, principalName);
- }
+ final JackrabbitAccessControlManager acMgr = context.getAccessControlManager();
+ Principal principal = context.getPrincipalWithSave(principalName);
+ checkState(principal != null, PRINCIPAL_NOT_FOUND_PATTERN, principalName);
final PrincipalAccessControlList acl = getPrincipalAccessControlList(acMgr, principal, true);
boolean modified = false;
for (AclLine line : lines) {
- List<String> jcrPaths = getJcrPaths(session, line.getProperty(PROP_PATHS));
- LocalRestrictions restr = createLocalRestrictions(line.getRestrictions(), acl, session);
+ List<String> jcrPaths = getJcrPaths(context.getSession(), line.getProperty(PROP_PATHS));
+ LocalRestrictions restr = createLocalRestrictions(line.getRestrictions(), acl, context.getSession());
List<String> privNames = line.getProperty(PROP_PRIVILEGES);
- Privilege[] privs = AccessControlUtils.privilegesFromNames(acMgr, privNames.toArray(new String[0]));
Predicate<PrincipalAccessControlList.Entry> predicate = entry -> {
if (!jcrPaths.contains(entry.getEffectivePath())) {
return false;
}
- LocalAccessControlEntry lace = new LocalAccessControlEntry(
- entry.getPrincipal(), privs, line.getAction() == AclLine.Action.ALLOW, restr);
+ LocalAccessControlEntry lace = new LocalAccessControlEntry(context,
+ entry.getPrincipal(), privNames.toArray(new String[0]), line.getAction() == AclLine.Action.ALLOW, restr);
return lace.isEqual(entry);
};
if (removePrincipalEntries(acl, principalName, predicate)) {
@@ -494,19 +476,19 @@ public class AclUtil {
/**
* Remove principal-based access control setup for the principal with the given name.
*
- * @param session
+ * @param context
* @param principalName
* @throws RepositoryException
*/
- public static void removePrincipalPolicy(@NotNull Session session, @NotNull String principalName)
+ public static void removePrincipalPolicy(@NotNull SessionContext context, @NotNull String principalName)
throws RepositoryException {
- Principal principal = AccessControlUtils.getPrincipal(session, principalName);
+ Principal principal = context.getPrincipal(principalName);
if (principal == null) {
LOG.info("Cannot remove principal-based ACL. Principal {} does not exist.", principalName);
return;
}
- JackrabbitAccessControlManager acMgr = getJACM(session);
+ JackrabbitAccessControlManager acMgr = context.getAccessControlManager();
PrincipalAccessControlList acl = getPrincipalAccessControlList(acMgr, principal, false);
if (acl == null) {
LOG.info("Cannot remove principal-based ACL for principal {}. No such policy exists.", principalName);
@@ -522,7 +504,8 @@ public class AclUtil {
/**
*
* @param acMgr the access control manager
- * @param principal the principal
+ * @param path
+ * @param includeApplicable
* @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
*/
@@ -622,25 +605,25 @@ public class AclUtil {
}
private static boolean containsEquivalentEntry(
- Session session,
+ SessionContext context,
String absPath,
Principal principal,
- Privilege[] privileges,
+ String[] privilegeNames,
boolean isAllow,
List<RestrictionClause> restrictionList)
throws RepositoryException {
- if (absPath != null && !session.nodeExists(absPath)) {
+ if (absPath != null && !context.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 : context.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), context.getSession());
+ LocalAccessControlEntry newEntry = new LocalAccessControlEntry(context, principal, privilegeNames, isAllow, lr);
if (contains(((JackrabbitAccessControlList) policy).getAccessControlEntries(), newEntry)) {
LOG.info(
"Equivalent path-based entry exists for principal {} and effective path {} ",
@@ -674,7 +657,7 @@ public class AclUtil {
+ entry.isAllow() + ", restrictionNames: " + entry.getRestrictionNames() + "]";
}
- private static void checkState(boolean expression, String msgPattern, Object... args) {
+ static void checkState(boolean expression, String msgPattern, Object... args) {
if (!expression) {
if (args == null) {
throw new IllegalStateException(msgPattern);
@@ -708,25 +691,33 @@ public class AclUtil {
static class LocalAccessControlEntry {
private final Principal principal;
- private final Privilege[] privileges;
+ private final String[] privilegeNames;
private final boolean isAllow;
private final LocalRestrictions restrictions;
+ private final SessionContext context;
+
+ private PrivilegeCollection privilegeCollection;
- LocalAccessControlEntry(Principal principal, Privilege[] privileges, boolean isAllow) {
- this(principal, privileges, isAllow, null);
+ LocalAccessControlEntry(SessionContext context, Principal principal, String[] privilegeNames, boolean isAllow) {
+ this(context, principal, privilegeNames, isAllow, null);
}
LocalAccessControlEntry(
- Principal principal, Privilege[] privileges, boolean isAllow, LocalRestrictions restrictions) {
+ SessionContext context,
+ Principal principal,
+ String[] privilegeNames,
+ boolean isAllow,
+ LocalRestrictions restrictions) {
+ this.context = context;
this.principal = principal;
- this.privileges = privileges;
+ this.privilegeNames = privilegeNames;
this.isAllow = isAllow;
this.restrictions = restrictions != null ? restrictions : new LocalRestrictions();
}
public boolean isContainedIn(JackrabbitAccessControlEntry other) throws RepositoryException {
return other.getPrincipal().equals(principal)
- && contains(other.getPrivileges(), privileges)
+ && contains(other.getPrivilegeCollection(), privilegeNames)
&& other.isAllow() == isAllow
&& sameRestrictions(other);
}
@@ -737,31 +728,14 @@ public class AclUtil {
}
try {
JackrabbitAccessControlEntry otherAce = (JackrabbitAccessControlEntry) other;
- return other.getPrincipal().equals(principal)
- && equalPrivileges(other.getPrivileges(), privileges)
+ return otherAce.getPrincipal().equals(principal)
+ && otherAce.getPrivilegeCollection().equals(getPrivilegeCollection())
&& otherAce.isAllow() == isAllow
&& sameRestrictions(otherAce);
} catch (RepositoryException e) {
throw new IllegalStateException("Cannot verify equivalence of access control entries", e);
}
}
-
- private Set<Privilege> expandPrivileges(Privilege[] privileges) {
- Set<Privilege> expandedSet = new HashSet<>();
-
- if (privileges != null) {
- for (Privilege privilege : privileges) {
- if (privilege.isAggregate()) {
- expandedSet.addAll(Arrays.asList(privilege.getAggregatePrivileges()));
- } else {
- expandedSet.add(privilege);
- }
- }
- }
-
- return expandedSet;
- }
-
/**
* compares if restrictions present in jackrabbit access control entry
* is same as specified restrictions in repo init
@@ -790,25 +764,22 @@ public class AclUtil {
return false;
}
- private boolean contains(Privilege[] first, Privilege[] second) {
+ private static boolean contains(PrivilegeCollection collection, String[] privilegeNames) throws RepositoryException {
// we need to ensure that the privilege order is not taken into account, so we use sets
- Set<Privilege> set1 = expandPrivileges(first);
-
- Set<Privilege> set2 = expandPrivileges(second);
-
- return set1.containsAll(set2);
+ return collection.includes(privilegeNames);
}
-
- private boolean equalPrivileges(Privilege[] first, Privilege[] second) {
- Set<Privilege> set1 = expandPrivileges(first);
- Set<Privilege> set2 = expandPrivileges(second);
- return set1.equals(set2);
+
+ private @NotNull PrivilegeCollection getPrivilegeCollection() throws RepositoryException {
+ if (privilegeCollection == null) {
+ privilegeCollection = context.privilegeCollectionFromNames(privilegeNames);
+ }
+ return privilegeCollection;
}
@Override
public String toString() {
return "[" + getClass().getSimpleName() + "# principal " + principal + ", privileges: "
- + Arrays.toString(privileges) + ", isAllow : " + isAllow + "]";
+ + Arrays.toString(privilegeNames) + ", isAllow : " + isAllow + "]";
}
}
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 f96f696..a1d1acb 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
@@ -55,6 +55,8 @@ class AclVisitor extends DoNothingVisitor {
*/
public static final String OPTION_IGNORE_MISSING_PRINCIPAL = "ignoreMissingPrincipal";
+ private final SessionContext context;
+
private enum Instruction {
SET,
REMOVE
@@ -66,8 +68,9 @@ class AclVisitor extends DoNothingVisitor {
* @param s must have sufficient rights to create users
* and set ACLs.
*/
- public AclVisitor(Session s) {
+ public AclVisitor(Session s) {
super(s);
+ context = new SessionContext(s);
}
private void handleAclLine(
@@ -77,17 +80,17 @@ class AclVisitor extends DoNothingVisitor {
if (action == AclLine.Action.REMOVE) {
report("remove not supported. use 'remove acl' instead.");
} else if (action == AclLine.Action.REMOVE_ALL) {
- AclUtil.removeEntries(session, principals, paths);
+ AclUtil.removeEntries(context, principals, paths);
} else {
final boolean isAllow = action == AclLine.Action.ALLOW;
final String actionName = isAllow ? "allow" : "deny";
final List<String> privileges = line.getProperty(PROP_PRIVILEGES);
if (instruction == Instruction.SET) {
log.info("Adding ACL '{}' entry '{}' for {} on {}", actionName, privileges, principals, paths);
- AclUtil.setAcl(session, principals, paths, privileges, isAllow, line.getRestrictions(), options);
+ AclUtil.setAcl(context, principals, paths, privileges, isAllow, line.getRestrictions(), options);
} else if (instruction == Instruction.REMOVE) {
log.info("Removing ACL '{}' entry '{}' for {} on {}", actionName, privileges, principals, paths);
- AclUtil.removeEntries(session, principals, paths, privileges, isAllow, line.getRestrictions());
+ AclUtil.removeEntries(context, principals, paths, privileges, isAllow, line.getRestrictions());
}
}
}
@@ -130,7 +133,7 @@ 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(), false);
+ AclUtil.setPrincipalAcl(context, principalName, s.getLines(), false);
} catch (Exception e) {
report(e, "Failed to set principal-based ACL (" + e.getMessage() + ")");
}
@@ -142,7 +145,7 @@ class AclVisitor extends DoNothingVisitor {
for (String principalName : s.getPrincipals()) {
try {
log.info("Enforcing principal-based access control entry for {}", principalName);
- AclUtil.setPrincipalAcl(session, principalName, s.getLines(), true);
+ AclUtil.setPrincipalAcl(context, principalName, s.getLines(), true);
} catch (Exception e) {
report(e, "Failed to set principal-based ACL (" + e.getMessage() + ")");
}
@@ -178,7 +181,7 @@ class AclVisitor extends DoNothingVisitor {
for (String principalName : s.getPrincipals()) {
try {
log.info("Removing principal-based access control entries for {}", principalName);
- AclUtil.removePrincipalEntries(session, principalName, s.getLines());
+ AclUtil.removePrincipalEntries(context, principalName, s.getLines());
} catch (Exception e) {
report(e, "Failed to remove principal-based access control entries (" + e.getMessage() + ")");
}
@@ -190,7 +193,7 @@ class AclVisitor extends DoNothingVisitor {
for (String principalName : s.getPrincipals()) {
try {
log.info("Removing access control policy for {}", principalName);
- AclUtil.removePolicy(session, principalName);
+ AclUtil.removePolicy(context, principalName);
} catch (RepositoryException e) {
report(e, "Failed to remove ACL (" + e.getMessage() + ")");
}
@@ -200,7 +203,7 @@ class AclVisitor extends DoNothingVisitor {
@Override
public void visitDeleteAclPaths(DeleteAclPaths s) {
try {
- AclUtil.removePolicies(session, s.getPaths());
+ AclUtil.removePolicies(context, s.getPaths());
} catch (RepositoryException e) {
report(e, "Failed to remove ACL (" + e.getMessage() + ")");
}
@@ -211,7 +214,7 @@ class AclVisitor extends DoNothingVisitor {
for (String principalName : s.getPrincipals()) {
try {
log.info("Removing principal-based access control policy for {}", principalName);
- AclUtil.removePrincipalPolicy(session, principalName);
+ AclUtil.removePrincipalPolicy(context, principalName);
} catch (RepositoryException e) {
report(e, "Failed to remove principal-based ACL (" + e.getMessage() + ")");
}
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/SessionContext.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/SessionContext.java
new file mode 100644
index 0000000..805beda
--- /dev/null
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/SessionContext.java
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.jcr.repoinit.impl;
+
+import org.apache.jackrabbit.api.JackrabbitSession;
+import org.apache.jackrabbit.api.security.JackrabbitAccessControlManager;
+import org.apache.jackrabbit.api.security.authorization.PrivilegeCollection;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import java.security.Principal;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * A simple wrapper around a session, which can cache the principal lookup and resolves privilege names to
+ * {@code PrivilegeCollection} objects.
+ */
+public class SessionContext {
+
+ JackrabbitSession session;
+ JackrabbitAccessControlManager acMgr;
+ Map<String,Principal> nameToPrincipal = new HashMap<>();
+
+ public SessionContext(@NotNull Session session) {
+ AclUtil.checkState(session instanceof JackrabbitSession,"A Jackrabbit Session is required");
+ this.session = (JackrabbitSession) session;
+ try {
+ AclUtil.checkState(session.getAccessControlManager() instanceof JackrabbitAccessControlManager,
+ "A Jackrabbit AccessControlManager is required");
+ this.acMgr = (JackrabbitAccessControlManager) session.getAccessControlManager();
+ } catch (RepositoryException e) {
+ throw new IllegalStateException("Cannot retrieve the AccessControlManager");
+ }
+ }
+
+ public JackrabbitSession getSession() {
+ return session;
+ }
+
+ public JackrabbitAccessControlManager getAccessControlManager() {
+ return acMgr;
+ }
+
+ public @NotNull PrivilegeCollection privilegeCollectionFromNames(@NotNull String... privilegeNames) throws RepositoryException {
+ return acMgr.privilegeCollectionFromNames(privilegeNames);
+ }
+
+ public @Nullable Principal getPrincipal (@NotNull String principalName) throws RepositoryException {
+ if (nameToPrincipal.containsKey(principalName)) {
+ return nameToPrincipal.get(principalName);
+ }
+ Principal p = session.getPrincipalManager().getPrincipal(principalName);
+ // Do not cache null principals
+ if (p != null) {
+ nameToPrincipal.put(principalName, p);
+ }
+ return p;
+ }
+
+ public @Nullable Principal getPrincipalWithSave(@NotNull String principalName) throws RepositoryException {
+ Principal principal = getPrincipal(principalName);
+ if (principal == null) {
+ // due to transient nature of the repo-init the principal lookup may not succeed if completed through query
+ // -> save transient changes and retry principal lookup
+ session.save();
+ principal = getPrincipal(principalName);
+ }
+ return principal;
+ }
+}
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 282a3a8..694f32e 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/PrincipalBasedAclTest.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/PrincipalBasedAclTest.java
@@ -57,6 +57,7 @@ import org.apache.jackrabbit.oak.spi.security.principal.SystemUserPrincipal;
import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
import org.apache.sling.jcr.repoinit.impl.AclUtil;
+import org.apache.sling.jcr.repoinit.impl.SessionContext;
import org.apache.sling.jcr.repoinit.impl.RepoInitException;
import org.apache.sling.jcr.repoinit.impl.TestUtil;
import org.apache.sling.repoinit.parser.RepoInitParsingException;
@@ -804,7 +805,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), false);
+ AclUtil.setPrincipalAcl(new SessionContext(U.adminSession), U.username, Collections.singletonList(line), false);
PrincipalAccessControlList acl = getAcl(principal, U.adminSession);
assertNotNull(acl);
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 ff04c18..4efc51b 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
@@ -18,24 +18,8 @@
*/
package org.apache.sling.jcr.repoinit.impl;
-import javax.jcr.AccessDeniedException;
-import javax.jcr.PathNotFoundException;
-import javax.jcr.PropertyType;
-import javax.jcr.RepositoryException;
-import javax.jcr.Session;
-import javax.jcr.UnsupportedRepositoryOperationException;
-import javax.jcr.Value;
-import javax.jcr.ValueFactory;
-import javax.jcr.security.Privilege;
-
-import java.security.Principal;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.List;
-
import org.apache.jackrabbit.api.JackrabbitSession;
import org.apache.jackrabbit.api.security.JackrabbitAccessControlList;
-import org.apache.jackrabbit.api.security.user.AuthorizableExistsException;
import org.apache.jackrabbit.api.security.user.Group;
import org.apache.jackrabbit.api.security.user.UserManager;
import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils;
@@ -53,6 +37,18 @@ import org.junit.Test;
import org.junit.function.ThrowingRunnable;
import org.junit.jupiter.api.Assertions;
+import javax.jcr.PathNotFoundException;
+import javax.jcr.PropertyType;
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import javax.jcr.Value;
+import javax.jcr.ValueFactory;
+import javax.jcr.security.Privilege;
+import java.security.Principal;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
@@ -243,7 +239,7 @@ public class AclUtilTest {
assertNull(((JackrabbitSession) session).getUserManager().getAuthorizable(EveryonePrincipal.getInstance()));
AclUtil.setAcl(
- session,
+ toPCSessionWrapper(session),
Collections.singletonList(EveryonePrincipal.NAME),
Collections.singletonList(PathUtils.ROOT_PATH),
Collections.singletonList(Privilege.JCR_READ),
@@ -273,7 +269,7 @@ public class AclUtilTest {
U.adminSession.save();
AclUtil.setAcl(
- U.adminSession,
+ toPCSessionWrapper(U.adminSession),
Collections.singletonList(principal.getName()),
Collections.singletonList(PathUtils.ROOT_PATH),
Collections.singletonList(Privilege.JCR_READ),
@@ -307,7 +303,7 @@ public class AclUtilTest {
U.adminSession.save();
AclUtil.setAcl(
- U.adminSession,
+ toPCSessionWrapper(U.adminSession),
Collections.singletonList(U.username),
Collections.singletonList(PathUtils.ROOT_PATH),
Collections.singletonList(Privilege.JCR_READ),
@@ -342,7 +338,7 @@ public class AclUtilTest {
List<String> paths = Collections.singletonList(":home:" + U.username + "#");
AclUtil.setAcl(
- U.adminSession,
+ toPCSessionWrapper(U.adminSession),
Collections.singletonList(U.username),
paths,
Collections.singletonList(Privilege.JCR_READ),
@@ -374,7 +370,7 @@ public class AclUtilTest {
List<String> paths = Collections.singletonList(":home:" + U.username + "," + gr.getID() + "#");
AclUtil.setAcl(
- U.adminSession,
+ toPCSessionWrapper(U.adminSession),
Collections.singletonList(U.username),
paths,
Collections.singletonList(Privilege.JCR_READ),
@@ -399,7 +395,7 @@ public class AclUtilTest {
List<String> paths = Arrays.asList(":home:" + U.username + "#", ":repository", PathUtils.ROOT_PATH);
AclUtil.setAcl(
- U.adminSession,
+ toPCSessionWrapper(U.adminSession),
Collections.singletonList(U.username),
paths,
Collections.singletonList(Privilege.JCR_ALL),
@@ -450,7 +446,7 @@ public class AclUtilTest {
List<String> paths = Collections.singletonList(":home:" + U.username + "," + gr.getID() + "#/profiles/private");
AclUtil.setAcl(
- U.adminSession,
+ toPCSessionWrapper(U.adminSession),
Collections.singletonList(U.username),
paths,
Collections.singletonList(Privilege.JCR_READ),
@@ -490,8 +486,7 @@ public class AclUtilTest {
* @param groupId the id of the group to create
*/
protected void assertSetAclWithHomePath(String groupId)
- throws AccessDeniedException, UnsupportedRepositoryOperationException, RepositoryException,
- AuthorizableExistsException {
+ throws RepositoryException{
UserManager uMgr = ((JackrabbitSession) U.adminSession).getUserManager();
Group gr = uMgr.createGroup(groupId);
@@ -504,7 +499,7 @@ public class AclUtilTest {
List<String> paths = Collections.singletonList(":home:" + gr.getID() + "," + U.username + "#");
AclUtil.setAcl(
- U.adminSession,
+ toPCSessionWrapper(U.adminSession),
Collections.singletonList(U.username),
paths,
Collections.singletonList(Privilege.JCR_READ),
@@ -537,7 +532,7 @@ public class AclUtilTest {
public void testSetAclWithHomePathMissingTrailingHash() throws Exception {
List<String> paths = Collections.singletonList(":home:" + U.username);
AclUtil.setAcl(
- U.adminSession,
+ toPCSessionWrapper(U.adminSession),
Collections.singletonList(U.username),
paths,
Collections.singletonList(Privilege.JCR_READ),
@@ -548,7 +543,7 @@ public class AclUtilTest {
public void testSetAclWithHomePathUnknownUser() throws Exception {
List<String> paths = Collections.singletonList(":home:alice#");
AclUtil.setAcl(
- U.adminSession,
+ toPCSessionWrapper(U.adminSession),
Collections.singletonList(U.username),
paths,
Collections.singletonList(Privilege.JCR_READ),
@@ -559,7 +554,7 @@ public class AclUtilTest {
public void repeatedSetAclCallIsNoOp() throws Throwable {
final Session session = U.adminSession;
final ThrowingRunnable setAcls = () -> AclUtil.setAcl(
- session,
+ toPCSessionWrapper(session),
Collections.singletonList(U.username),
Arrays.asList(":home:" + U.username + "#", ":repository", PathUtils.ROOT_PATH),
Collections.singletonList(Privilege.JCR_ALL),
@@ -578,7 +573,7 @@ public class AclUtilTest {
public void nullRestrictionClauseAndNullOptionsAreHandled() {
final Session session = U.adminSession;
Assertions.assertDoesNotThrow(() -> AclUtil.setAcl(
- session,
+ toPCSessionWrapper(session),
Collections.singletonList(U.username),
Collections.singletonList(PathUtils.ROOT_PATH),
Collections.singletonList(Privilege.JCR_READ),
@@ -607,7 +602,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), privilegeNames, isAllow);
if (contained) {
assertTrue(
@@ -623,7 +618,7 @@ public class AclUtilTest {
return AccessControlUtils.getPrincipal(U.adminSession, principalName);
}
- private Privilege[] privileges(String... privilegeNames) throws RepositoryException {
- return AccessControlUtils.privilegesFromNames(U.adminSession, privilegeNames);
+ private static SessionContext toPCSessionWrapper (Session session) {
+ return new SessionContext(session);
}
}
diff --git a/src/test/java/org/apache/sling/jcr/repoinit/impl/ManyServiceUsersTest.java b/src/test/java/org/apache/sling/jcr/repoinit/impl/ManyServiceUsersTest.java
index 552689c..2f7d7e3 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/impl/ManyServiceUsersTest.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/impl/ManyServiceUsersTest.java
@@ -85,7 +85,7 @@ public class ManyServiceUsersTest {
try {
AclUtil.setAcl(
- otherSession,
+ new SessionContext(otherSession),
Arrays.asList(username),
Arrays.asList(path),
Arrays.asList("jcr:read"),
diff --git a/src/test/java/org/apache/sling/jcr/repoinit/impl/TestUtil.java b/src/test/java/org/apache/sling/jcr/repoinit/impl/TestUtil.java
index f5b0904..b057fae 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/impl/TestUtil.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/impl/TestUtil.java
@@ -76,13 +76,13 @@ public class TestUtil {
username = "user_" + id;
}
- public List<Operation> parse(String... inputLines) throws RepoInitParsingException {
+ public static List<Operation> parse(String... inputLines) throws RepoInitParsingException {
try (final StringReader r = new StringReader(String.join("\n", inputLines))) {
return new RepoInitParserService().parse(r);
}
}
- private void assertPathContains(Authorizable u, String pathShouldContain) throws RepositoryException {
+ private static void assertPathContains(Authorizable u, String pathShouldContain) throws RepositoryException {
if (pathShouldContain != null) {
final String path = u.getPath();
assertTrue("Expecting path " + path + " to contain " + pathShouldContain, path.contains(pathShouldContain));