Author: angela
Date: Tue Feb 12 15:46:54 2019
New Revision: 1853449
URL: http://svn.apache.org/viewvc?rev=1853449&view=rev
Log:
OAK-8044 : AccessControlManagerImpl.getEffectivePolicies returns empty ACLs
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
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeAccessControlManagerTest.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=1853449&r1=1853448&r2=1853449&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
Tue Feb 12 15:46:54 2019
@@ -39,10 +39,10 @@ import javax.jcr.security.AccessControlP
import javax.jcr.security.NamedAccessControlPolicy;
import javax.jcr.security.Privilege;
-import com.google.common.base.Function;
import com.google.common.base.Objects;
import com.google.common.base.Predicate;
import com.google.common.base.Strings;
+import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
@@ -134,7 +134,7 @@ public class AccessControlManagerImpl ex
public AccessControlPolicy[] getPolicies(@Nullable String absPath) throws
RepositoryException {
String oakPath = getOakPath(absPath);
Tree tree = getTree(oakPath, Permissions.READ_ACCESS_CONTROL, true);
- AccessControlPolicy policy = createACL(oakPath, tree, false);
+ AccessControlPolicy policy = createACL(oakPath, tree, false,
Predicates.alwaysTrue());
List<AccessControlPolicy> policies = new ArrayList<>(2);
if (policy != null) {
@@ -156,7 +156,7 @@ public class AccessControlManagerImpl ex
tree = r.getTree(tree.getPath());
List<AccessControlPolicy> effective = new ArrayList<>();
- AccessControlPolicy policy = createACL(oakPath, tree, true);
+ AccessControlPolicy policy = createACL(oakPath, tree, true,
Predicates.alwaysTrue());
if (policy != null) {
effective.add(policy);
}
@@ -164,7 +164,7 @@ public class AccessControlManagerImpl ex
String parentPath = Text.getRelativeParent(oakPath, 1);
while (!parentPath.isEmpty()) {
Tree t = r.getTree(parentPath);
- AccessControlPolicy plc = createACL(parentPath, t, true);
+ AccessControlPolicy plc = createACL(parentPath, t, true,
Predicates.alwaysTrue());
if (plc != null) {
effective.add(plc);
}
@@ -238,7 +238,7 @@ public class AccessControlManagerImpl ex
String path = getNodePath(ace);
Tree tree = getTree(path, Permissions.MODIFY_ACCESS_CONTROL, true);
- ACL acl = (ACL) createACL(path, tree, false);
+ ACL acl = (ACL) createACL(path, tree, false,
Predicates.alwaysTrue());
if (acl == null) {
acl = new NodeACL(path);
}
@@ -267,7 +267,7 @@ public class AccessControlManagerImpl ex
String path = getNodePath(ace);
Tree tree = getTree(path, Permissions.MODIFY_ACCESS_CONTROL, true);
- ACL acl = (ACL) createACL(path, tree, false);
+ ACL acl = (ACL) createACL(path, tree, false,
Predicates.alwaysTrue());
if (acl != null) {
// remove rep:nodePath restriction before removing the entry
from
// the node-based policy (see above for adding entries without
@@ -382,37 +382,17 @@ public class AccessControlManagerImpl ex
Root r = getLatestRoot();
Result aceResult = searchAces(principals, r);
- Set<JackrabbitAccessControlList> effective = Sets.newTreeSet(new
Comparator<JackrabbitAccessControlList>() {
- @Override
- public int compare(JackrabbitAccessControlList list1,
JackrabbitAccessControlList list2) {
- if (list1.equals(list2)) {
- return 0;
- } else {
- String p1 = list1.getPath();
- String p2 = list2.getPath();
-
- if (p1 == null) {
- return -1;
- } else if (p2 == null) {
- return 1;
- } else {
- int depth1 = PathUtils.getDepth(p1);
- int depth2 = PathUtils.getDepth(p2);
- return (depth1 == depth2) ? p1.compareTo(p2) :
Ints.compare(depth1, depth2);
- }
-
- }
- }
- });
+ Set<JackrabbitAccessControlList> effective = Sets.newTreeSet(new
AcListComparator());
Set<String> paths = Sets.newHashSet();
+ Predicate<Tree> predicate = new PrincipalPredicate(principals);
for (ResultRow row : aceResult.getRows()) {
String acePath = row.getPath();
String aclName = Text.getName(Text.getRelativeParent(acePath, 1));
Tree accessControlledTree =
r.getTree(Text.getRelativeParent(acePath, 2));
if (aclName.isEmpty() || !accessControlledTree.exists()) {
- log.debug("Isolated access control entry -> ignore query
result at " + acePath);
+ log.debug("Isolated access control entry -> ignore query
result at {}", acePath);
continue;
}
@@ -420,7 +400,7 @@ public class AccessControlManagerImpl ex
if (paths.contains(path)) {
continue;
}
- JackrabbitAccessControlList policy = createACL(path,
accessControlledTree, true, new AcePredicate(principals));
+ JackrabbitAccessControlList policy = createACL(path,
accessControlledTree, true, predicate);
if (policy != null) {
effective.add(policy);
paths.add(path);
@@ -435,7 +415,7 @@ public class AccessControlManagerImpl ex
try {
return Util.isValidPolicy(getOakPath(absPath),
accessControlPolicy);
} catch (RepositoryException e) {
- log.warn("Invalid absolute path: " + absPath, e.getMessage());
+ log.warn("Invalid absolute path '{}': {}", absPath,
e.getMessage());
return false;
}
}
@@ -479,44 +459,36 @@ public class AccessControlManagerImpl ex
@Nullable
private JackrabbitAccessControlList createACL(@Nullable String oakPath,
@NotNull Tree
accessControlledTree,
- boolean isEffectivePolicy)
throws RepositoryException {
- return createACL(oakPath, accessControlledTree, isEffectivePolicy,
null);
- }
-
- @Nullable
- private JackrabbitAccessControlList createACL(@Nullable String oakPath,
- @NotNull Tree
accessControlledTree,
boolean isEffectivePolicy,
- @Nullable Predicate<ACE>
predicate) throws RepositoryException {
- JackrabbitAccessControlList acl = null;
- String aclName = Util.getAclName(oakPath);
- if (accessControlledTree.exists() && Util.isAccessControlled(oakPath,
accessControlledTree, ntMgr)) {
- Tree aclTree = accessControlledTree.getChild(aclName);
- if (aclTree.exists()) {
- List<ACE> entries = new ArrayList<>();
- for (Tree child : aclTree.getChildren()) {
- if (Util.isACE(child, ntMgr)) {
- ACE ace = createACE(oakPath, child,
restrictionProvider);
- if (predicate == null || predicate.apply(ace)) {
- entries.add(ace);
- }
- }
- }
- if (isEffectivePolicy) {
- acl = new ImmutableACL(oakPath, entries,
restrictionProvider, getNamePathMapper());
- } else {
- acl = new NodeACL(oakPath, entries);
- }
+ @NotNull Predicate<Tree>
predicate) throws RepositoryException {
+ if (!accessControlledTree.exists() ||
!Util.isAccessControlled(oakPath, accessControlledTree, ntMgr)) {
+ return null;
+ }
+
+ Tree aclTree = accessControlledTree.getChild(Util.getAclName(oakPath));
+ if (!aclTree.exists()) {
+ return null;
+ }
+
+ List<ACE> entries = new ArrayList<>();
+ for (Tree child : aclTree.getChildren()) {
+ if (Util.isACE(child, ntMgr) && predicate.apply(child)) {
+ ACE ace = createACE(oakPath, child, restrictionProvider);
+ entries.add(ace);
}
}
- return acl;
+ if (!isEffectivePolicy) {
+ return new NodeACL(oakPath, entries);
+ } else {
+ return (entries.isEmpty()) ? null : new ImmutableACL(oakPath,
entries, restrictionProvider, getNamePathMapper());
+ }
}
@Nullable
private JackrabbitAccessControlList createPrincipalACL(@Nullable String
oakPath,
@NotNull Principal
principal) throws RepositoryException {
Root root = getRoot();
- Result aceResult =
searchAces(Collections.<Principal>singleton(principal), root);
+ Result aceResult = searchAces(Collections.singleton(principal), root);
RestrictionProvider restrProvider = new
PrincipalRestrictionProvider(restrictionProvider);
List<ACE> entries = new ArrayList<>();
for (ResultRow row : aceResult.getRows()) {
@@ -628,7 +600,7 @@ public class AccessControlManagerImpl ex
}
if (PermissionUtil.isAdminOrSystem(ImmutableSet.of(principal),
configParams)) {
- log.warn("Attempt to create an ACE for an administrative
principal which always has full access:" + getPath());
+ log.warn("Attempt to create an ACE for an administrative
principal which always has full access: {}", getPath());
switch (Util.getImportBehavior(getConfig())) {
case ImportBehavior.ABORT:
throw new AccessControlException("Attempt to create an
ACE for an administrative principal which always has full access.");
@@ -775,22 +747,40 @@ public class AccessControlManagerImpl ex
}
}
- private static final class AcePredicate implements Predicate<ACE> {
+ private static final class PrincipalPredicate implements Predicate<Tree> {
private final Iterable<String> principalNames;
- private AcePredicate(@NotNull Set<Principal> principals) {
- principalNames = Iterables.transform(principals, new
Function<Principal, String>() {
- @Override
- public String apply(Principal input) {
- return input.getName();
- }
- });
+ private PrincipalPredicate(@NotNull Set<Principal> principals) {
+ principalNames = Iterables.transform(principals,
Principal::getName);
+ }
+
+ @Override
+ public boolean apply(@Nullable Tree aceTree) {
+ return aceTree != null && Iterables.contains(principalNames,
TreeUtil.getString(aceTree, REP_PRINCIPAL_NAME));
}
+ }
+ private static final class AcListComparator implements
Comparator<JackrabbitAccessControlList> {
@Override
- public boolean apply(@Nullable ACE ace) {
- return ace != null && Iterables.contains(principalNames,
ace.getPrincipal().getName());
+ public int compare(JackrabbitAccessControlList list1,
JackrabbitAccessControlList list2) {
+ if (list1.equals(list2)) {
+ return 0;
+ } else {
+ String p1 = list1.getPath();
+ String p2 = list2.getPath();
+
+ if (p1 == null) {
+ return -1;
+ } else if (p2 == null) {
+ return 1;
+ } else {
+ int depth1 = PathUtils.getDepth(p1);
+ int depth2 = PathUtils.getDepth(p2);
+ return (depth1 == depth2) ? p1.compareTo(p2) :
Ints.compare(depth1, depth2);
+ }
+
+ }
}
}
}
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=1853449&r1=1853448&r2=1853449&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
Tue Feb 12 15:46:54 2019
@@ -217,6 +217,11 @@ public class AccessControlManagerImplTes
return ImmutableSet.<Principal>of(EveryonePrincipal.getInstance());
}
+ private static void assertPolicies(@Nullable AccessControlPolicy[]
policies, long expectedSize) {
+ assertNotNull(policies);
+ assertEquals(expectedSize, policies.length);
+ }
+
private ACL getApplicablePolicy(@Nullable String path) throws
RepositoryException {
AccessControlPolicyIterator itr = acMgr.getApplicablePolicies(path);
if (itr.hasNext()) {
@@ -1140,6 +1145,21 @@ public class AccessControlManagerImplTes
//---------------------------------------< getEffectivePolicies(String)
>---
@Test
+ public void testGetEffectivePoliciesNoPoliciesSet() throws Exception {
+ assertPolicies(acMgr.getEffectivePolicies(testPath), 0);
+ }
+
+ @Test
+ public void testGetEffectivePoliciesEmptyACL() throws Exception {
+ // set empty policy -> no effective ACEs
+ acMgr.setPolicy(testPath,
acMgr.getApplicablePolicies(testPath).nextAccessControlPolicy());
+ root.commit();
+
+ // resulting effective policies should be empty array
+ assertPolicies(acMgr.getEffectivePolicies(testPath), 0);
+ }
+
+ @Test
public void testGetEffectivePolicies() throws Exception {
AccessControlPolicy[] policies = acMgr.getEffectivePolicies(testPath);
assertNotNull(policies);
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeAccessControlManagerTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeAccessControlManagerTest.java?rev=1853449&r1=1853448&r2=1853449&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeAccessControlManagerTest.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeAccessControlManagerTest.java
Tue Feb 12 15:46:54 2019
@@ -20,6 +20,7 @@ import java.util.Collections;
import java.util.List;
import java.util.Set;
import javax.jcr.security.AccessControlException;
+import javax.jcr.security.AccessControlList;
import javax.jcr.security.AccessControlManager;
import javax.jcr.security.AccessControlPolicy;
import javax.jcr.security.AccessControlPolicyIterator;
@@ -36,6 +37,8 @@ import org.apache.jackrabbit.oak.api.Tre
import org.apache.jackrabbit.oak.namepath.NamePathMapper;
import org.apache.jackrabbit.oak.spi.nodetype.NodeTypeConstants;
import
org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.PolicyOwner;
+import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
+import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
import org.apache.jackrabbit.oak.util.NodeUtil;
import org.jetbrains.annotations.NotNull;
import org.junit.Test;
@@ -139,6 +142,9 @@ public class CompositeAccessControlManag
AccessControlPolicyIterator it =
acMgr.getApplicablePolicies(TEST_PATH);
while (it.hasNext()) {
AccessControlPolicy plc = it.nextAccessControlPolicy();
+ if (plc instanceof AccessControlList) {
+ ((AccessControlList)
plc).addAccessControlEntry(EveryonePrincipal.getInstance(),
privilegesFromNames(PrivilegeConstants.JCR_READ));
+ }
acMgr.setPolicy(TEST_PATH, plc);
}
root.commit();
