You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by an...@apache.org on 2019/03/21 13:04:25 UTC
svn commit: r1855986 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/
test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/
Author: angela
Date: Thu Mar 21 13:04:25 2019
New Revision: 1855986
URL: http://svn.apache.org/viewvc?rev=1855986&view=rev
Log:
OAK-8151 : Let ACE.getPrincipal return principals obtained from PrincipalManager
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImpl.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImplTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImpl.java?rev=1855986&r1=1855985&r2=1855986&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImpl.java Thu Mar 21 13:04:25 2019
@@ -311,6 +311,7 @@ public class AccessControlManagerImpl ex
String oakPath = getOakPath(absPath);
Util.checkValidPolicy(oakPath, policy);
+ Map<String, Principal> principalMap = new HashMap<>();
if (policy instanceof PrincipalACL) {
PrincipalACL principalAcl = (PrincipalACL) policy;
for (ACE ace : principalAcl.getEntries()) {
@@ -323,7 +324,7 @@ public class AccessControlManagerImpl ex
Iterator<Tree> children = aclTree.getChildren().iterator();
while (children.hasNext()) {
Tree child = children.next();
- if (ace.equals(createACE(path, child, principalAcl.rProvider))) {
+ if (ace.equals(createACE(path, child, principalAcl.rProvider, principalMap))) {
child.remove();
}
}
@@ -469,9 +470,10 @@ public class AccessControlManagerImpl ex
}
List<ACE> entries = new ArrayList<>();
+ Map<String, Principal> principalMap = new HashMap<>();
for (Tree child : aclTree.getChildren()) {
if (Util.isACE(child, ntMgr) && predicate.apply(child)) {
- ACE ace = createACE(oakPath, child, restrictionProvider);
+ ACE ace = createACE(oakPath, child, restrictionProvider, principalMap);
entries.add(ace);
}
}
@@ -489,6 +491,7 @@ public class AccessControlManagerImpl ex
Result aceResult = searchAces(Collections.singleton(principal), root);
RestrictionProvider restrProvider = new PrincipalRestrictionProvider(restrictionProvider);
List<ACE> entries = new ArrayList<>();
+ Map<String, Principal> principalMap = new HashMap<>();
for (ResultRow row : aceResult.getRows()) {
Tree aceTree = root.getTree(row.getPath());
if (Util.isACE(aceTree, ntMgr)) {
@@ -499,7 +502,7 @@ public class AccessControlManagerImpl ex
} else {
path = Text.getRelativeParent(aclPath, 1);
}
- entries.add(createACE(path, aceTree, restrProvider));
+ entries.add(createACE(path, aceTree, restrProvider, principalMap));
}
}
if (entries.isEmpty()) {
@@ -513,11 +516,12 @@ public class AccessControlManagerImpl ex
@NotNull
private ACE createACE(@Nullable String oakPath,
@NotNull Tree aceTree,
- @NotNull RestrictionProvider restrictionProvider) throws RepositoryException {
+ @NotNull RestrictionProvider restrictionProvider,
+ @NotNull Map<String, Principal> principalMap) throws RepositoryException {
boolean isAllow = NT_REP_GRANT_ACE.equals(TreeUtil.getPrimaryTypeName(aceTree));
Set<Restriction> restrictions = restrictionProvider.readRestrictions(oakPath, aceTree);
Iterable<String> privNames = checkNotNull(TreeUtil.getStrings(aceTree, REP_PRIVILEGES));
- return new Entry(getPrincipal(aceTree), bitsProvider.getBits(privNames), isAllow, restrictions, getNamePathMapper());
+ return new Entry(getPrincipal(aceTree, principalMap), bitsProvider.getBits(privNames), isAllow, restrictions, getNamePathMapper());
}
@NotNull
@@ -554,9 +558,15 @@ public class AccessControlManagerImpl ex
}
@NotNull
- private Principal getPrincipal(@NotNull Tree aceTree) {
+ private Principal getPrincipal(@NotNull Tree aceTree, @NotNull Map<String, Principal> principalMap) {
String principalName = checkNotNull(TreeUtil.getString(aceTree, REP_PRINCIPAL_NAME));
- return new PrincipalImpl(principalName);
+ return principalMap.computeIfAbsent(principalName, pn -> {
+ Principal principal = principalManager.getPrincipal(pn);
+ if (principal == null) {
+ principal = new PrincipalImpl(pn);
+ }
+ return principal;
+ });
}
private String getNodePath(ACE principalBasedAce) throws RepositoryException {
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImplTest.java?rev=1855986&r1=1855985&r2=1855986&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImplTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImplTest.java Thu Mar 21 13:04:25 2019
@@ -27,6 +27,7 @@ import org.apache.jackrabbit.api.securit
import org.apache.jackrabbit.api.security.JackrabbitAccessControlManager;
import org.apache.jackrabbit.api.security.JackrabbitAccessControlPolicy;
import org.apache.jackrabbit.api.security.authorization.PrivilegeManager;
+import org.apache.jackrabbit.api.security.principal.GroupPrincipal;
import org.apache.jackrabbit.api.security.principal.PrincipalManager;
import org.apache.jackrabbit.oak.AbstractSecurityTest;
import org.apache.jackrabbit.oak.api.ContentSession;
@@ -95,6 +96,7 @@ import static org.junit.Assert.assertEqu
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -1153,6 +1155,24 @@ public class AccessControlManagerImplTes
}
}
+ @Test
+ public void testGetPoliciesLimitsPrincipalLookup() throws Exception {
+ ACL policy = getApplicablePolicy(testPath);
+ policy.addAccessControlEntry(EveryonePrincipal.getInstance(), privilegesFromNames(PrivilegeConstants.JCR_READ));
+ policy.addEntry(testPrincipal, privilegesFromNames(PrivilegeConstants.JCR_ADD_CHILD_NODES, PrivilegeConstants.JCR_REMOVE_CHILD_NODES), true, ImmutableMap.of(REP_GLOB, getValueFactory(root).createValue("")));
+ policy.addAccessControlEntry(testPrincipal, privilegesFromNames(PrivilegeConstants.JCR_REMOVE_NODE));
+ acMgr.setPolicy(policy.getPath(), policy);
+
+ // read policy again
+ policy = (ACL) acMgr.getPolicies(policy.getPath())[0];
+ assertEquals(3, policy.size());
+ AccessControlEntry[] entries = policy.getAccessControlEntries();
+ // reading policies attempts to lookup principals (see OAK-xxx)
+ assertTrue(entries[0].getPrincipal() instanceof GroupPrincipal);
+ // reading policies must only lookup a given principal once
+ assertSame(entries[1].getPrincipal(), entries[2].getPrincipal());
+ }
+
//---------------------------------------< getEffectivePolicies(String) >---
@Test
public void testGetEffectivePoliciesNoPoliciesSet() throws Exception {