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/02/12 15:46:54 UTC
svn commit: r1853449 - 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/
test/java/org/apache/jackrabbit/oak/securi...
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();