jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ang...@apache.org
Subject svn commit: r1506470 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/security/authorization/ main/java/org/apache/jackrabbit/oak/security/privilege/ test/java/org/apache/jackrabbit/oak/security/authorization/ test/java/org/...
Date Wed, 24 Jul 2013 09:45:12 GMT
Author: angela
Date: Wed Jul 24 09:45:12 2013
New Revision: 1506470

URL: http://svn.apache.org/r1506470
Log:
OAK-527: permissions

- move detection of duplicate ACEs to the ac-validator (resolving TODO in permissionhooktest)

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlValidator.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlValidatorProvider.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeBitsProvider.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/AccessControlValidatorTest.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractPermissionHookTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlValidator.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlValidator.java?rev=1506470&r1=1506469&r2=1506470&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlValidator.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlValidator.java
Wed Jul 24 09:45:12 2013
@@ -20,18 +20,25 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Map;
+import java.util.Set;
 import javax.jcr.security.AccessControlException;
 import javax.jcr.security.Privilege;
 
+import com.google.common.base.Objects;
+import com.google.common.collect.Sets;
 import org.apache.jackrabbit.JcrConstants;
 import org.apache.jackrabbit.oak.api.CommitFailedException;
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.core.AbstractTree;
 import org.apache.jackrabbit.oak.plugins.nodetype.ReadOnlyNodeTypeManager;
+import org.apache.jackrabbit.oak.security.privilege.PrivilegeBits;
+import org.apache.jackrabbit.oak.security.privilege.PrivilegeBitsProvider;
 import org.apache.jackrabbit.oak.spi.commit.DefaultValidator;
 import org.apache.jackrabbit.oak.spi.commit.Validator;
 import org.apache.jackrabbit.oak.spi.security.authorization.AccessControlConstants;
+import org.apache.jackrabbit.oak.spi.security.authorization.restriction.Restriction;
 import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionProvider;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
 import org.apache.jackrabbit.oak.util.TreeUtil;
@@ -49,15 +56,19 @@ class AccessControlValidator extends Def
     private final Tree parentBefore;
     private final Tree parentAfter;
 
+    private final PrivilegeBitsProvider privilegeBitsProvider;
     private final Map<String, Privilege> privileges;
     private final RestrictionProvider restrictionProvider;
     private final ReadOnlyNodeTypeManager ntMgr;
 
     AccessControlValidator(Tree parentBefore, Tree parentAfter,
                            Map<String, Privilege> privileges,
-                           RestrictionProvider restrictionProvider, ReadOnlyNodeTypeManager
ntMgr) {
+                           PrivilegeBitsProvider privilegeBitsProvider,
+                           RestrictionProvider restrictionProvider,
+                           ReadOnlyNodeTypeManager ntMgr) {
         this.parentBefore = parentBefore;
         this.parentAfter = parentAfter;
+        this.privilegeBitsProvider = privilegeBitsProvider;
         this.privileges = privileges;
         this.restrictionProvider = restrictionProvider;
         this.ntMgr = ntMgr;
@@ -94,7 +105,7 @@ class AccessControlValidator extends Def
         Tree treeAfter = checkNotNull(parentAfter.getChild(name));
 
         checkValidTree(parentAfter, treeAfter, after);
-        return new AccessControlValidator(null, treeAfter, privileges, restrictionProvider,
ntMgr);
+        return new AccessControlValidator(null, treeAfter, privileges, privilegeBitsProvider,
restrictionProvider, ntMgr);
     }
 
     @Override
@@ -103,7 +114,7 @@ class AccessControlValidator extends Def
         Tree treeAfter = checkNotNull(parentAfter.getChild(name));
 
         checkValidTree(parentAfter, treeAfter, after);
-        return new AccessControlValidator(treeBefore, treeAfter, privileges, restrictionProvider,
ntMgr);
+        return new AccessControlValidator(treeBefore, treeAfter, privileges, privilegeBitsProvider,
restrictionProvider, ntMgr);
     }
 
     @Override
@@ -156,6 +167,15 @@ class AccessControlValidator extends Def
         if (!policyNode.hasProperty(AbstractTree.OAK_CHILD_ORDER)) {
             throw accessViolation(4, "Invalid policy node: Order of children is not stable.");
         }
+
+        Set<Entry> aceSet = Sets.newHashSet();
+        for (Tree child : policyTree.getChildren()) {
+            if (isAccessControlEntry(child)) {
+                if (!aceSet.add(new Entry(parent.getPath(), child))) {
+                    throw accessViolation(13, "Duplicate ACE found in policy");
+                }
+            }
+        }
     }
 
     private void checkValidAccessControlledNode(Tree accessControlledTree, String requiredMixin)
throws CommitFailedException {
@@ -240,4 +260,36 @@ class AccessControlValidator extends Def
     private static CommitFailedException accessViolation(int code, String message) {
         return new CommitFailedException(ACCESS_CONTROL, code, message);
     }
+
+    private class Entry {
+
+        private final String principalName;
+        private final PrivilegeBits privilegeBits;
+        private final Set<Restriction> restrictions;
+
+        private Entry(String path, Tree aceTree) {
+            principalName = aceTree.getProperty(REP_PRINCIPAL_NAME).getValue(Type.STRING);
+            privilegeBits = privilegeBitsProvider.getBits(aceTree.getProperty(REP_PRIVILEGES).getValue(Type.NAMES));
+            restrictions = restrictionProvider.readRestrictions(path, aceTree);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hashCode(principalName, privilegeBits, restrictions);
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (o == this) {
+                return true;
+            }
+            if (o instanceof Entry) {
+                Entry other = (Entry) o;
+                return Objects.equal(principalName, other.principalName)
+                        && privilegeBits.equals(other.privilegeBits)
+                        && restrictions.equals(other.restrictions);
+            }
+            return false;
+        }
+    }
 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlValidatorProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlValidatorProvider.java?rev=1506470&r1=1506469&r2=1506470&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlValidatorProvider.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlValidatorProvider.java
Wed Jul 24 09:45:12 2013
@@ -29,6 +29,7 @@ import org.apache.jackrabbit.oak.core.Im
 import org.apache.jackrabbit.oak.core.ImmutableTree;
 import org.apache.jackrabbit.oak.namepath.NamePathMapper;
 import org.apache.jackrabbit.oak.plugins.nodetype.ReadOnlyNodeTypeManager;
+import org.apache.jackrabbit.oak.security.privilege.PrivilegeBitsProvider;
 import org.apache.jackrabbit.oak.spi.commit.Validator;
 import org.apache.jackrabbit.oak.spi.commit.ValidatorProvider;
 import org.apache.jackrabbit.oak.spi.security.SecurityProvider;
@@ -64,14 +65,15 @@ class AccessControlValidatorProvider ext
 
         RestrictionProvider restrictionProvider = getConfig(AccessControlConfiguration.class).getRestrictionProvider();
 
-        Map<String, Privilege> privileges = getPrivileges(before);
+        Root root = new ImmutableRoot(before);
+        Map<String, Privilege> privileges = getPrivileges(root);
+        PrivilegeBitsProvider privilegeBitsProvider = new PrivilegeBitsProvider(root);
         ReadOnlyNodeTypeManager ntMgr = ReadOnlyNodeTypeManager.getInstance(before);
 
-        return new AccessControlValidator(rootBefore, rootAfter, privileges, restrictionProvider,
ntMgr);
+        return new AccessControlValidator(rootBefore, rootAfter, privileges, privilegeBitsProvider,
restrictionProvider, ntMgr);
     }
 
-    private Map<String, Privilege> getPrivileges(NodeState beforeRoot) {
-        Root root = new ImmutableRoot(beforeRoot);
+    private Map<String, Privilege> getPrivileges(Root root) {
         PrivilegeManager pMgr = getConfig(PrivilegeConfiguration.class).getPrivilegeManager(root,
NamePathMapper.DEFAULT);
         ImmutableMap.Builder privileges = ImmutableMap.builder();
         try {

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeBitsProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeBitsProvider.java?rev=1506470&r1=1506469&r2=1506470&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeBitsProvider.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeBitsProvider.java
Wed Jul 24 09:45:12 2013
@@ -16,14 +16,11 @@
  */
 package org.apache.jackrabbit.oak.security.privilege;
 
-import static com.google.common.base.Preconditions.checkNotNull;
-
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
-
 import javax.annotation.Nonnull;
 
 import com.google.common.collect.ImmutableSet;
@@ -31,6 +28,8 @@ import org.apache.jackrabbit.oak.api.Roo
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 /**
  * Reads and writes privilege definitions from and to the repository content
  * without applying any validation.
@@ -100,6 +99,30 @@ public final class PrivilegeBitsProvider
     }
 
     /**
+     * @param privilegeNames
+     * @return
+     */
+    @Nonnull
+    public PrivilegeBits getBits(@Nonnull Iterable<String> privilegeNames) {
+        if (!privilegeNames.iterator().hasNext()) {
+            return PrivilegeBits.EMPTY;
+        }
+
+        Tree privilegesTree = getPrivilegesTree();
+        if (!privilegesTree.exists()) {
+            return PrivilegeBits.EMPTY;
+        }
+        PrivilegeBits bits = PrivilegeBits.getInstance();
+        for (String privilegeName : privilegeNames) {
+            Tree defTree = privilegesTree.getChild(checkNotNull(privilegeName));
+            if (defTree.exists()) {
+                bits.add(PrivilegeBits.getInstance(defTree));
+            }
+        }
+        return bits.unmodifiable();
+    }
+
+    /**
      * Resolve the given privilege bits to a set of privilege names.
      *
      * @param privilegeBits An instance of privilege bits.

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/AccessControlValidatorTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/AccessControlValidatorTest.java?rev=1506470&r1=1506469&r2=1506470&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/AccessControlValidatorTest.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/AccessControlValidatorTest.java
Wed Jul 24 09:45:12 2013
@@ -17,10 +17,11 @@
 package org.apache.jackrabbit.oak.security.authorization;
 
 import java.security.Principal;
-
 import javax.jcr.AccessDeniedException;
+import javax.jcr.security.AccessControlManager;
 
 import org.apache.jackrabbit.JcrConstants;
+import org.apache.jackrabbit.api.security.JackrabbitAccessControlList;
 import org.apache.jackrabbit.api.security.authorization.PrivilegeManager;
 import org.apache.jackrabbit.oak.api.CommitFailedException;
 import org.apache.jackrabbit.oak.api.Tree;
@@ -329,4 +330,25 @@ public class AccessControlValidatorTest 
             assertTrue(e.isAccessControlViolation());
         }
     }
+
+    @Test
+    public void testDuplicateAce() throws Exception {
+        AccessControlManager acMgr = getAccessControlManager(root);
+        JackrabbitAccessControlList acl = org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils.getAccessControlList(acMgr,
testPath);
+        acl.addAccessControlEntry(testPrincipal, privilegesFromNames(PrivilegeConstants.JCR_ADD_CHILD_NODES));
+        acMgr.setPolicy(testPath, acl);
+
+        // add duplicate ac-entry on OAK-API
+        NodeUtil policy = new NodeUtil(root.getTree(testPath + "/rep:policy"));
+        NodeUtil ace = policy.addChild("duplicateAce", NT_REP_GRANT_ACE);
+        ace.setString(REP_PRINCIPAL_NAME, testPrincipal.getName());
+        ace.setStrings(AccessControlConstants.REP_PRIVILEGES, PrivilegeConstants.JCR_ADD_CHILD_NODES);
+
+        try {
+            root.commit();
+            fail("Creating duplicate ACE must be detected");
+        } catch (CommitFailedException e) {
+            assertTrue(e.isAccessControlViolation());
+        }
+    }
 }
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractPermissionHookTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractPermissionHookTest.java?rev=1506470&r1=1506469&r2=1506470&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractPermissionHookTest.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractPermissionHookTest.java
Wed Jul 24 09:45:12 2013
@@ -109,12 +109,12 @@ public abstract class AbstractPermission
 
     protected Tree getEntry(String principalName, String accessControlledPath, long index)
throws Exception {
         Tree principalRoot = getPrincipalRoot(principalName);
-		return traverse(principalRoot, accessControlledPath, index);
+        return traverse(principalRoot, accessControlledPath, index);
     }
-	
-	protected Tree traverse(Tree parent, String accessControlledPath, long index) throws Exception
{
-		for (Tree entry : parent.getChildren()) {
-		    String path = entry.getProperty(REP_ACCESS_CONTROLLED_PATH).getValue(Type.STRING);
+
+    protected Tree traverse(Tree parent, String accessControlledPath, long index) throws
Exception {
+        for (Tree entry : parent.getChildren()) {
+            String path = entry.getProperty(REP_ACCESS_CONTROLLED_PATH).getValue(Type.STRING);
             long entryIndex = entry.getProperty(REP_INDEX).getValue(Type.LONG);
             if (accessControlledPath.equals(path)) {
                 if (index == entryIndex) {
@@ -123,62 +123,30 @@ public abstract class AbstractPermission
                     return traverse(entry, accessControlledPath, index);
                 }
             } else if (Text.isDescendant(path, accessControlledPath)) {
-				return traverse(entry, accessControlledPath, index);
-			}
+                return traverse(entry, accessControlledPath, index);
+            }
         }
-	    throw new RepositoryException("no such entry");
-	}
-	
-	protected long cntEntries(Tree parent) {
+        throw new RepositoryException("no such entry");
+    }
+
+    protected long cntEntries(Tree parent) {
         long cnt = parent.getChildrenCount();
-		for (Tree child : parent.getChildren()) {
-			cnt += cntEntries(child);
-		}
-		return cnt;	
-	}
+        for (Tree child : parent.getChildren()) {
+            cnt += cntEntries(child);
+        }
+        return cnt;
+    }
 
     protected void createPrincipals() throws Exception {
         if (principals.isEmpty()) {
             for (int i = 0; i < 10; i++) {
-                Group gr = getUserManager(root).createGroup("testGroup"+i);
+                Group gr = getUserManager(root).createGroup("testGroup" + i);
                 principals.add(gr.getPrincipal());
             }
             root.commit();
         }
     }
 
-    @Ignore() // FIXME: refuse out duplicate entries in ac-validation hook.
-    @Test
-    public void testDuplicateAce() throws Exception {
-        // add duplicate policy on OAK-API
-        NodeUtil policy = new NodeUtil(root.getTree(testPath + "/rep:policy"));
-        NodeUtil ace = policy.addChild("duplicateAce", NT_REP_GRANT_ACE);
-        ace.setString(REP_PRINCIPAL_NAME, testPrincipalName);
-        ace.setStrings(AccessControlConstants.REP_PRIVILEGES, JCR_ADD_CHILD_NODES);
-        root.commit();
-
-        Tree principalRoot = getPrincipalRoot(testPrincipalName);
-        assertEquals(2, cntEntries(principalRoot));
-
-        assertEquals(1, principalRoot.getChildrenCount());
-		Tree entry1 = principalRoot.getChildren().iterator().next();
-		assertEquals(bitsProvider.getBits(JCR_ADD_CHILD_NODES), PrivilegeBits.getInstance(entry1.getProperty(REP_PRIVILEGE_BITS)));
-		assertEquals(testPath, entry1.getProperty(REP_ACCESS_CONTROLLED_PATH).getValue(Type.STRING));
-        assertEquals(0, entry1.getProperty(REP_INDEX).getValue(Type.LONG).intValue());
-		
-	    assertEquals(1, entry1.getChildrenCount());
-		Tree entry2 = entry1.getChildren().iterator().next();
-		assertEquals(bitsProvider.getBits(JCR_ADD_CHILD_NODES), PrivilegeBits.getInstance(entry2.getProperty(REP_PRIVILEGE_BITS)));
-		assertEquals(testPath, entry2.getProperty(REP_ACCESS_CONTROLLED_PATH).getValue(Type.STRING));
-        assertEquals(2, entry2.getProperty(REP_INDEX).getValue(Type.LONG).intValue());
-
-        // remove duplicate policy entry again
-        root.getTree(testPath + "/rep:policy/duplicateAce").remove();
-        root.commit();
-
-        assertEquals(1, cntEntries(getPrincipalRoot(testPrincipalName)));
-    }
-
     @Test
     public void testModifyRestrictions() throws Exception {
         Tree testAce = root.getTree(testPath + "/rep:policy").getChildren().iterator().next();
@@ -340,7 +308,7 @@ public abstract class AbstractPermission
         entry = getEntry(principals.get(2).getName(), testPath, 3);
         assertEquals(3, entry.getProperty(REP_INDEX).getValue(Type.LONG).longValue());
 
-        for (String pName : new String[] {testPrincipalName, principals.get(0).getName()})
{
+        for (String pName : new String[]{testPrincipalName, principals.get(0).getName()})
{
             try {
                 getEntry(pName, testPath, 0);
                 fail();
@@ -362,7 +330,7 @@ public abstract class AbstractPermission
         acMgr.setPolicy(childPath, acl);
         root.commit();
 
-        assertTrue(root.getTree(childPath+"/rep:policy").exists());
+        assertTrue(root.getTree(childPath + "/rep:policy").exists());
 
         Tree principalRoot = getPrincipalRoot(EveryonePrincipal.NAME);
         assertEquals(2, cntEntries(principalRoot));
@@ -371,7 +339,7 @@ public abstract class AbstractPermission
         Root testRoot = testSession.getLatestRoot();
 
         assertTrue(testRoot.getTree(childPath).exists());
-        assertFalse(testRoot.getTree(childPath+"/rep:policy").exists());
+        assertFalse(testRoot.getTree(childPath + "/rep:policy").exists());
 
         testRoot.getTree(childPath).remove();
         testRoot.commit();
@@ -379,7 +347,7 @@ public abstract class AbstractPermission
 
         root.refresh();
         assertFalse(root.getTree(testPath).hasChild("childNode"));
-        assertFalse(root.getTree(childPath+"/rep:policy").exists());
+        assertFalse(root.getTree(childPath + "/rep:policy").exists());
         // aces must be removed in the permission store even if the editing
         // session wasn't able to access them.
         principalRoot = getPrincipalRoot(EveryonePrincipal.NAME);



Mime
View raw message