Author: angela
Date: Thu Jun 27 15:24:48 2013
New Revision: 1497394
URL: http://svn.apache.org/r1497394
Log:
OAK-878 : IllegalArgumentException while adding/removing read permission to
user/group
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java?rev=1497394&r1=1497393&r2=1497394&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java
Thu Jun 27 15:24:48 2013
@@ -16,8 +16,6 @@
*/
package org.apache.jackrabbit.oak.security.authorization.permission;
-import java.util.ArrayList;
-import java.util.List;
import java.util.Set;
import javax.annotation.Nonnull;
@@ -40,6 +38,7 @@ import org.apache.jackrabbit.oak.spi.com
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.ChildNodeEntry;
import org.apache.jackrabbit.oak.spi.state.DefaultNodeStateDiff;
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
import org.apache.jackrabbit.oak.spi.state.NodeState;
@@ -103,38 +102,17 @@ public class PermissionHook implements P
private final Node parentBefore;
private final AfterNode parentAfter;
- private final List<String> processed = new ArrayList<String>();
-
private Diff(@Nonnull Node parentBefore, @Nonnull AfterNode
parentAfter) {
this.parentBefore = parentBefore;
this.parentAfter = parentAfter;
}
@Override
- public boolean propertyChanged(PropertyState before, PropertyState
after) {
- if (isACL(parentAfter) &&
TreeImpl.OAK_CHILD_ORDER.equals(before.getName())) {
- List<String> reordered = new ChildOrderDiff(before,
after).getReordered();
- for (String name : reordered) {
- NodeState beforeNode =
parentBefore.getNodeState().getChildNode(name);
- removeEntry(name, beforeNode);
- }
- for (String name : reordered) {
- NodeState afterNode =
parentAfter.getNodeState().getChildNode(name);
- addEntry(name, afterNode);
-
- log.debug("Processed reordered child node " + name);
- processed.add(name);
- }
- }
- return true;
- }
-
- @Override
public boolean childNodeAdded(String name, NodeState after) {
if (NodeStateUtils.isHidden(name)) {
// ignore hidden nodes
- } else if (isACE(name, after)) {
- addEntry(name, after);
+ } else if (isACL(name, after)) {
+ addEntries(name, after);
} else {
Node before = new BeforeNode(parentBefore.getPath(), name,
EMPTY_NODE);
AfterNode node = new AfterNode(parentAfter, name);
@@ -147,8 +125,9 @@ public class PermissionHook implements P
public boolean childNodeChanged(String name, final NodeState before,
NodeState after) {
if (NodeStateUtils.isHidden(name)) {
// ignore hidden nodes
- } else if (isACE(name, before) || isACE(name, after)) {
- updateEntry(name, before, after);
+ } else if (isACL(name, before) || isACL(name, after)) {
+ removeEntries(name, before);
+ addEntries(name, after);
} else {
BeforeNode nodeBefore = new BeforeNode(parentBefore.getPath(),
name, before);
AfterNode nodeAfter = new AfterNode(parentAfter, name);
@@ -161,8 +140,8 @@ public class PermissionHook implements P
public boolean childNodeDeleted(String name, NodeState before) {
if (NodeStateUtils.isHidden(name)) {
// ignore hidden nodes
- } else if (isACE(name, before)) {
- removeEntry(name, before);
+ } else if (isACL(name, before)) {
+ removeEntries(name, before);
} else {
BeforeNode nodeBefore = new BeforeNode(parentBefore.getPath(),
name, before);
AfterNode after = new AfterNode(parentAfter.getPath(), name,
EMPTY_NODE);
@@ -172,34 +151,34 @@ public class PermissionHook implements P
}
//--------------------------------------------------------< private
>---
- private boolean isACL(Node parent) {
- return ntMgr.isNodeType(getTree(parent.getName(),
parent.getNodeState()), NT_REP_POLICY);
+
+ private boolean isACL(String name, NodeState nodeState) {
+ return ntMgr.isNodeType(getTree(name, nodeState), NT_REP_ACL);
}
private boolean isACE(String name, NodeState nodeState) {
return ntMgr.isNodeType(getTree(name, nodeState), NT_REP_ACE);
}
- private void addEntry(String name, NodeState ace) {
- PermissionEntry entry = createPermissionEntry(name, ace,
parentAfter);
- entry.add();
- }
-
- private void removeEntry(String name, NodeState ace) {
- PermissionEntry entry = createPermissionEntry(name, ace,
parentBefore);
- entry.remove();
- }
-
- private void updateEntry(String name, NodeState before, NodeState
after) {
- if (processed.contains(name)) {
- log.debug("ACE entry already processed -> skip updateEntry.");
- return;
- }
- PermissionEntry beforeEntry = createPermissionEntry(name, before,
parentBefore);
- PermissionEntry afterEntry = createPermissionEntry(name, after,
parentAfter);
- if (!beforeEntry.equals(afterEntry)) {
- beforeEntry.remove();
- afterEntry.add();
+ private void addEntries(String aclName, NodeState acl) {
+ for (ChildNodeEntry cne : acl.getChildNodeEntries()) {
+ NodeState child = cne.getNodeState();
+ if (isACE(cne.getName(), child)) {
+ PermissionEntry entry =
createPermissionEntry(cne.getName(),
+ child, new AfterNode(parentAfter, aclName));
+ entry.add();
+ }
+ }
+ }
+
+ private void removeEntries(String aclName, NodeState acl) {
+ for (ChildNodeEntry cne : acl.getChildNodeEntries()) {
+ NodeState child = cne.getNodeState();
+ if (isACE(cne.getName(), child)) {
+ PermissionEntry entry =
createPermissionEntry(cne.getName(),
+ child, new BeforeNode(parentBefore, aclName, acl));
+ entry.remove();
+ }
}
}
@@ -244,12 +223,16 @@ public class PermissionHook implements P
this.nodeState = root;
}
-
BeforeNode(String parentPath, String name, NodeState nodeState) {
super(parentPath, name);
this.nodeState = nodeState;
}
+ BeforeNode(Node parent, String name, NodeState nodeState) {
+ super(parent.getPath(), name);
+ this.nodeState = nodeState;
+ }
+
@Override
NodeState getNodeState() {
return nodeState;
Modified:
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java?rev=1497394&r1=1497393&r2=1497394&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java
(original)
+++
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java
Thu Jun 27 15:24:48 2013
@@ -21,11 +21,15 @@ import java.util.HashSet;
import java.util.Set;
import javax.jcr.Node;
import javax.jcr.PathNotFoundException;
+import javax.jcr.RepositoryException;
import javax.jcr.Session;
+import javax.jcr.security.AccessControlEntry;
import javax.jcr.security.Privilege;
+import javax.jcr.util.TraversingItemVisitor;
import org.apache.jackrabbit.api.security.JackrabbitAccessControlManager;
import org.apache.jackrabbit.api.security.user.Group;
+import org.apache.jackrabbit.oak.security.authorization.AccessControlUtils;
import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
import org.junit.Test;
@@ -411,4 +415,43 @@ public class ReadTest extends AbstractEv
Node n2 = test.getNode("a");
assertTrue(n.isSame(n2));
}
+
+ /**
+ * @see <a href="https://issues.apache.org/jira/browse/OAK-878";>OAK-878 :
+ * IllegalArgumentException while adding/removing permission to
user/group</a>
+ */
+ @Test
+ public void testImplicitReorder() throws Exception{
+ allow(path, testUser.getPrincipal(), readPrivileges);
+ assertEntry(0, true);
+
+ allow(path, getTestGroup().getPrincipal(), readPrivileges);
+ assertEntry(0, true);
+
+ deny(path, testUser.getPrincipal(), readPrivileges);
+ assertEntry(1, false);
+
+ deny(path, getTestGroup().getPrincipal(), readPrivileges);
+ assertEntry(0, false);
+
+ allow(path, testUser.getPrincipal(), readPrivileges);
+ }
+
+ private void assertEntry(final int index, final boolean isAllow) throws
RepositoryException {
+ AccessControlEntry first =
AccessControlUtils.getAccessControlList(superuser,
path).getAccessControlEntries()[index];
+
+ assertEquals(testUser.getPrincipal(), first.getPrincipal());
+
+ Node n = superuser.getNode("/jcr:system/rep:permissionStore/default/"
+ testUser.getPrincipal().getName());
+ TraversingItemVisitor v = new TraversingItemVisitor.Default(true, -1) {
+ @Override
+ protected void entering(Node node, int level) throws
RepositoryException {
+ if (node.isNodeType("rep:Permissions") &&
path.equals(node.getProperty("rep:accessControlledPath").getString())) {
+ assertEquals(index,
node.getProperty("rep:index").getLong());
+ assertEquals(isAllow,
node.getProperty("rep:isAllow").getBoolean());
+ }
+ }
+ };
+ v.visit(n);
+ }
}
