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);
