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 {
