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 2015/07/08 10:55:51 UTC

svn commit: r1689820 - 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: Wed Jul  8 08:55:50 2015
New Revision: 1689820

URL: http://svn.apache.org/r1689820
Log:
OAK-3082 : Redundent entries in effective policies per principal-set

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=1689820&r1=1689819&r2=1689820&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 Wed Jul  8 08:55:50 2015
@@ -22,6 +22,7 @@ import java.security.Principal;
 import java.text.ParseException;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -47,6 +48,8 @@ import javax.jcr.security.Privilege;
 import com.google.common.base.Objects;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
+import com.google.common.primitives.Ints;
 import org.apache.jackrabbit.JcrConstants;
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlList;
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlPolicy;
@@ -361,7 +364,30 @@ public class AccessControlManagerImpl ex
         Root r = getLatestRoot();
 
         Result aceResult = searchAces(principals, r);
-        List<AccessControlPolicy> effective = new ArrayList<AccessControlPolicy>();
+        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<String> paths = Sets.newHashSet();
         for (ResultRow row : aceResult.getRows()) {
             String acePath = row.getPath();
             String aclName = Text.getName(Text.getRelativeParent(acePath, 1));
@@ -373,9 +399,13 @@ public class AccessControlManagerImpl ex
             }
 
             String path = (REP_REPO_POLICY.equals(aclName)) ? null : accessControlledTree.getPath();
-            AccessControlPolicy policy = createACL(path, accessControlledTree, true);
+            if (paths.contains(path)) {
+                continue;
+            }
+            JackrabbitAccessControlList policy = createACL(path, accessControlledTree, true);
             if (policy != null) {
                 effective.add(policy);
+                paths.add(path);
             }
         }
         return effective.toArray(new AccessControlPolicy[effective.size()]);

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=1689820&r1=1689819&r2=1689820&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 Wed Jul  8 08:55:50 2015
@@ -128,7 +128,7 @@ public class AccessControlManagerImplTes
         rootNode.addChild(testName, JcrConstants.NT_UNSTRUCTURED);
         root.commit();
 
-        testPrivileges = privilegesFromNames(Privilege.JCR_ADD_CHILD_NODES, Privilege.JCR_READ);
+        testPrivileges = privilegesFromNames(PrivilegeConstants.JCR_ADD_CHILD_NODES, PrivilegeConstants.JCR_READ);
         testPrincipal = getTestPrincipal();
     }
 
@@ -1782,6 +1782,73 @@ public class AccessControlManagerImplTes
                 assertEquals(0, policies.length);
             }
         }
+    }
+
+    @Test
+    public void testNoEffectiveDuplicateEntries() throws Exception {
+        Set<Principal> principalSet = ImmutableSet.of(testPrincipal, EveryonePrincipal.getInstance());
+
+        // create first policy with multiple ACEs for the test principal set.
+        ACL policy = getApplicablePolicy(testPath);
+        policy.addEntry(testPrincipal, testPrivileges, true, getGlobRestriction("*"));
+        policy.addEntry(testPrincipal, privilegesFromNames(PrivilegeConstants.JCR_VERSION_MANAGEMENT), false);
+        policy.addEntry(EveryonePrincipal.getInstance(), privilegesFromNames(PrivilegeConstants.JCR_LIFECYCLE_MANAGEMENT), false);
+        assertEquals(3, policy.getAccessControlEntries().length);
+        acMgr.setPolicy(testPath, policy);
+        root.commit();
+
+        AccessControlPolicy[] policies = acMgr.getEffectivePolicies(principalSet);
+        assertEquals(1, policies.length);
+
+        // add another policy
+        NodeUtil child = new NodeUtil(root.getTree(testPath)).addChild("child", JcrConstants.NT_UNSTRUCTURED);
+        String childPath = child.getTree().getPath();
+        setupPolicy(childPath);
+        root.commit();
+
+        policies = acMgr.getEffectivePolicies(principalSet);
+        assertEquals(2, policies.length);
+    }
+
+    @Test
+    public void testEffectiveSorting() throws Exception {
+        Set<Principal> principalSet = ImmutableSet.of(testPrincipal, EveryonePrincipal.getInstance());
+
+        ACL nullPathPolicy = null;
+        try {
+            // 1. policy at 'testPath'
+            ACL policy = getApplicablePolicy(testPath);
+            policy.addEntry(testPrincipal, testPrivileges, true, getGlobRestriction("*"));
+            policy.addEntry(testPrincipal, privilegesFromNames(PrivilegeConstants.JCR_VERSION_MANAGEMENT), false);
+            policy.addEntry(EveryonePrincipal.getInstance(), privilegesFromNames(PrivilegeConstants.JCR_LIFECYCLE_MANAGEMENT), false);
+            acMgr.setPolicy(testPath, policy);
+
+            // 2. policy at child node
+            NodeUtil child = new NodeUtil(root.getTree(testPath)).addChild("child", JcrConstants.NT_UNSTRUCTURED);
+            String childPath = child.getTree().getPath();
+            setupPolicy(childPath);
+
+            // 3. policy for null-path
+            nullPathPolicy = getApplicablePolicy(null);
+            assertNotNull(nullPathPolicy);
+
+            nullPathPolicy.addEntry(testPrincipal, privilegesFromNames(PrivilegeConstants.REP_PRIVILEGE_MANAGEMENT), true);
+            acMgr.setPolicy(null, nullPathPolicy);
+            root.commit();
+
+            AccessControlPolicy[] effectivePolicies = acMgr.getEffectivePolicies(principalSet);
+            assertEquals(3, effectivePolicies.length);
+
+            assertNull(((JackrabbitAccessControlPolicy) effectivePolicies[0]).getPath());
+            assertEquals(testPath, ((JackrabbitAccessControlPolicy) effectivePolicies[1]).getPath());
+            assertEquals(childPath, ((JackrabbitAccessControlPolicy) effectivePolicies[2]).getPath());
+
+        } finally {
+            if (nullPathPolicy != null) {
+                acMgr.removePolicy(null, nullPathPolicy);
+                root.commit();
+            }
+        }
 
     }