You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by an...@apache.org on 2011/06/22 18:16:26 UTC
svn commit: r1138531 - in /jackrabbit/branches/2.2: ./
jackrabbit-core/src/main/java/org/apache/jackrabbit/core/
jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/
jackrabbit-core/src/test/java/org/apache/jackrabbit/core/s...
Author: angela
Date: Wed Jun 22 16:16:26 2011
New Revision: 1138531
URL: http://svn.apache.org/viewvc?rev=1138531&view=rev
Log:
2.2: Merging Revision 1138511 (JCR-2999), 1097363 (JCR-2951)
Modified:
jackrabbit/branches/2.2/ (props changed)
jackrabbit/branches/2.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java
jackrabbit/branches/2.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemSaveOperation.java
jackrabbit/branches/2.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
jackrabbit/branches/2.2/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractWriteTest.java
jackrabbit/branches/2.2/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java
Propchange: jackrabbit/branches/2.2/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Jun 22 16:16:26 2011
@@ -2,4 +2,4 @@
/jackrabbit/sandbox/JCR-1456:774917-886178
/jackrabbit/sandbox/JCR-2170:812417-816332
/jackrabbit/sandbox/tripod-JCR-2209:795441-795863
-/jackrabbit/trunk:1038201,1038203,1038205,1038657,1039064,1039347,1039408,1039422-1039423,1039888,1039946,1040033,1040090,1040459,1040601,1040606,1040661,1040958,1041379,1041439,1041761,1042643,1042647,1042978-1042982,1043084-1043086,1043088,1043343,1043357-1043358,1043430,1043554,1043616,1043618,1043637,1043656,1043893,1043897,1044239,1044312,1044451,1044613,1049473,1049491,1049514,1049518,1049520,1049859,1049870,1049874,1049878,1049880,1049883,1049889,1049891,1049894-1049895,1049899-1049901,1049909-1049911,1049915-1049916,1049919,1049923,1049925,1049931,1049936,1049939,1050212,1050298,1050346,1050551,1055068,1055070-1055071,1055116-1055117,1055127,1055134,1055164,1055498,1060431,1060434,1060753,1063756,1064670,1065599,1065622,1066059,1066071,1069831,1071562,1071573,1071680,1074140,1079314,1079317,1080186,1080540,1087304,1088991,1089032,1089053,1089436,1092106,1092117,1092683,1097513-1097514,1098963-1098964,1099033,1099172,1100286,1104027,1128175,1130192,1130228,1132993,113
6353,1136360
+/jackrabbit/trunk:1038201,1038203,1038205,1038657,1039064,1039347,1039408,1039422-1039423,1039888,1039946,1040033,1040090,1040459,1040601,1040606,1040661,1040958,1041379,1041439,1041761,1042643,1042647,1042978-1042982,1043084-1043086,1043088,1043343,1043357-1043358,1043430,1043554,1043616,1043618,1043637,1043656,1043893,1043897,1044239,1044312,1044451,1044613,1049473,1049491,1049514,1049518,1049520,1049859,1049870,1049874,1049878,1049880,1049883,1049889,1049891,1049894-1049895,1049899-1049901,1049909-1049911,1049915-1049916,1049919,1049923,1049925,1049931,1049936,1049939,1050212,1050298,1050346,1050551,1055068,1055070-1055071,1055116-1055117,1055127,1055134,1055164,1055498,1060431,1060434,1060753,1063756,1064670,1065599,1065622,1066059,1066071,1069831,1071562,1071573,1071680,1074140,1079314,1079317,1080186,1080540,1087304,1088991,1089032,1089053,1089436,1092106,1092117,1092683,1097363,1097513-1097514,1098963-1098964,1099033,1099172,1100286,1104027,1128175,1130192,1130228,113
2993,1136353,1136360,1138511
Modified: jackrabbit/branches/2.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java?rev=1138531&r1=1138530&r2=1138531&view=diff
==============================================================================
--- jackrabbit/branches/2.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java (original)
+++ jackrabbit/branches/2.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java Wed Jun 22 16:16:26 2011
@@ -304,15 +304,15 @@ public class ItemManager implements Item
* @param path Path of the item to retrieve or <code>null</code>. In
* the latter case the test for access permission is executed using the
* itemId.
+ * @param permissionCheck
* @return The item identified by the given <code>itemId</code>.
* @throws ItemNotFoundException
* @throws AccessDeniedException
* @throws RepositoryException
*/
- private ItemImpl getItem(ItemId itemId, Path path) throws ItemNotFoundException, AccessDeniedException, RepositoryException {
+ private ItemImpl getItem(ItemId itemId, Path path, boolean permissionCheck) throws ItemNotFoundException, AccessDeniedException, RepositoryException {
sanityCheck();
- boolean permissionCheck = true;
ItemData data = getItemData(itemId, path, permissionCheck);
return createItemInstance(data);
}
@@ -540,7 +540,7 @@ public class ItemManager implements Item
throw new PathNotFoundException(safeGetJCRPath(path));
}
try {
- ItemImpl item = getItem(id, path);
+ ItemImpl item = getItem(id, path, true);
// Test, if this item is a shareable node.
if (item.isNode() && ((NodeImpl) item).isShareable()) {
return getNode(path);
@@ -570,7 +570,7 @@ public class ItemManager implements Item
}
try {
if (parentId == null) {
- return (NodeImpl) getItem(id, path);
+ return (NodeImpl) getItem(id, path, true);
}
// if the node is shareable, it now returns the node with the right
// parent
@@ -594,12 +594,12 @@ public class ItemManager implements Item
throw new PathNotFoundException(safeGetJCRPath(path));
}
try {
- return (PropertyImpl) getItem(id, path);
+ return (PropertyImpl) getItem(id, path, true);
} catch (ItemNotFoundException infe) {
throw new PathNotFoundException(safeGetJCRPath(path));
}
}
-
+
/**
* @param id
* @return
@@ -607,7 +607,17 @@ public class ItemManager implements Item
*/
public synchronized ItemImpl getItem(ItemId id)
throws ItemNotFoundException, AccessDeniedException, RepositoryException {
- return getItem(id, null);
+ return getItem(id, null, true);
+ }
+
+ /**
+ * @param id
+ * @return
+ * @throws RepositoryException
+ */
+ synchronized ItemImpl getItem(ItemId id, boolean permissionCheck)
+ throws ItemNotFoundException, AccessDeniedException, RepositoryException {
+ return getItem(id, null, permissionCheck);
}
/**
@@ -622,12 +632,29 @@ public class ItemManager implements Item
*/
public synchronized NodeImpl getNode(NodeId id, NodeId parentId)
throws ItemNotFoundException, AccessDeniedException, RepositoryException {
+ return getNode(id, parentId, true);
+ }
+
+ /**
+ * Returns a node with a given id and parent id. If the indicated node is
+ * shareable, there might be multiple nodes associated with the same id,
+ * but there'is only one node with the given parent id.
+ *
+ * @param id node id
+ * @param parentId parent node id
+ * @param permissionCheck Flag indicating if read permission must be check
+ * upon retrieving the node.
+ * @return node
+ * @throws RepositoryException if an error occurs
+ */
+ synchronized NodeImpl getNode(NodeId id, NodeId parentId, boolean permissionCheck)
+ throws ItemNotFoundException, AccessDeniedException, RepositoryException {
if (parentId == null) {
return (NodeImpl) getItem(id);
}
AbstractNodeData data = retrieveItem(id, parentId);
if (data == null) {
- data = (AbstractNodeData) getItemData(id);
+ data = (AbstractNodeData) getItemData(id, null, permissionCheck);
}
if (!data.getParentId().equals(parentId)) {
// verify that parent actually appears in the shared set
Modified: jackrabbit/branches/2.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemSaveOperation.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemSaveOperation.java?rev=1138531&r1=1138530&r2=1138531&view=diff
==============================================================================
--- jackrabbit/branches/2.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemSaveOperation.java (original)
+++ jackrabbit/branches/2.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemSaveOperation.java Wed Jun 22 16:16:26 2011
@@ -776,7 +776,7 @@ class ItemSaveOperation implements Sessi
nodeState);
if (nt.includesNodeType(NameConstants.MIX_VERSIONABLE)) {
if (!nodeState.hasPropertyName(NameConstants.JCR_VERSIONHISTORY)) {
- NodeImpl node = (NodeImpl) itemMgr.getItem(itemState.getId());
+ NodeImpl node = (NodeImpl) itemMgr.getItem(itemState.getId(), false);
InternalVersionManager vMgr = session.getInternalVersionManager();
/**
* check if there's already a version history for that
@@ -812,7 +812,7 @@ class ItemSaveOperation implements Sessi
vMgr.getVersionHistory(session, nodeState, null);
// create isCheckedOutProperty if not already exists
- NodeImpl node = (NodeImpl) itemMgr.getItem(itemState.getId());
+ NodeImpl node = (NodeImpl) itemMgr.getItem(itemState.getId(), false);
if (!nodeState.hasPropertyName(NameConstants.JCR_ISCHECKEDOUT)) {
node.internalSetProperty(
NameConstants.JCR_ISCHECKEDOUT,
@@ -833,7 +833,7 @@ class ItemSaveOperation implements Sessi
throws RepositoryException {
for (ItemState state : states) {
// persist state of transient item
- itemMgr.getItem(state.getId()).makePersistent();
+ itemMgr.getItem(state.getId(), false).makePersistent();
}
}
@@ -860,7 +860,7 @@ class ItemSaveOperation implements Sessi
itemState.setStatus(ItemState.STATUS_NEW);
} else {
try {
- item = itemMgr.getItem(id);
+ item = itemMgr.getItem(id, false);
} catch (ItemNotFoundException infe) {
// itemState probably represents a 'new' item and the
// ItemImpl instance wrapping it has already been gc'ed;
Modified: jackrabbit/branches/2.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java?rev=1138531&r1=1138530&r2=1138531&view=diff
==============================================================================
--- jackrabbit/branches/2.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java (original)
+++ jackrabbit/branches/2.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java Wed Jun 22 16:16:26 2011
@@ -681,7 +681,14 @@ public class NodeImpl extends ItemImpl i
NodeId childId = entry.getId();
//NodeImpl childNode = (NodeImpl) itemMgr.getItem(childId);
try {
- NodeImpl childNode = itemMgr.getNode(childId, getNodeId());
+ /* omit the read-permission check upon retrieving the
+ child item as this is an internal call to remove the
+ subtree which may contain (protected) child items which
+ are not visible to the caller of the removal. the actual
+ validation of the remove permission however is only
+ executed during Item.save().
+ */
+ NodeImpl childNode = itemMgr.getNode(childId, getNodeId(), false);
childNode.onRemove(thisState.getNodeId());
// remove the child node entry
} catch (ItemNotFoundException e) {
@@ -714,7 +721,14 @@ public class NodeImpl extends ItemImpl i
thisState.removePropertyName(propName);
// remove property
PropertyId propId = new PropertyId(thisState.getNodeId(), propName);
- itemMgr.getItem(propId).setRemoved();
+ /* omit the read-permission check upon retrieving the
+ child item as this is an internal call to remove the
+ subtree which may contain (protected) child items which
+ are not visible to the caller of the removal. the actual
+ validation of the remove permission however is only
+ executed during Item.save().
+ */
+ itemMgr.getItem(propId, false).setRemoved();
}
// finally remove this node
Modified: jackrabbit/branches/2.2/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractWriteTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.2/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractWriteTest.java?rev=1138531&r1=1138530&r2=1138531&view=diff
==============================================================================
--- jackrabbit/branches/2.2/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractWriteTest.java (original)
+++ jackrabbit/branches/2.2/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractWriteTest.java Wed Jun 22 16:16:26 2011
@@ -18,8 +18,7 @@ package org.apache.jackrabbit.core.secur
import org.apache.jackrabbit.api.security.JackrabbitAccessControlList;
import org.apache.jackrabbit.api.security.user.Group;
-import org.apache.jackrabbit.api.security.user.UserManager;
-import org.apache.jackrabbit.core.security.TestPrincipal;
+import org.apache.jackrabbit.core.UserTransactionImpl;
import org.apache.jackrabbit.test.JUnitTest;
import org.apache.jackrabbit.test.NotExecutableException;
import org.apache.jackrabbit.test.api.observation.EventResult;
@@ -40,13 +39,12 @@ import javax.jcr.observation.Observation
import javax.jcr.security.AccessControlManager;
import javax.jcr.security.AccessControlPolicy;
import javax.jcr.security.Privilege;
-import java.security.Principal;
+import javax.transaction.UserTransaction;
import java.util.HashMap;
import java.util.List;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Map;
-import java.util.UUID;
/**
* <code>AbstractEvaluationTest</code>...
@@ -1010,7 +1008,7 @@ public abstract class AbstractWriteTest
n.orderBefore(Text.getName(childNPath), Text.getName(childNPath2));
testSession.save();
}
-
+
/**
* Test case for JCR-2420
*
@@ -1239,6 +1237,51 @@ public abstract class AbstractWriteTest
assertFalse(testSession.hasPermission(childNPath2, Session.ACTION_SET_PROPERTY));
}
+ /**
+ * Tests if it is possible to create/read nodes with a non-admin session
+ * within a transaction.
+ *
+ * @throws Exception
+ * @see <a href="https://issues.apache.org/jira/browse/JCR-2999">JCR-2999</a>
+ */
+ public void testTransaction() throws Exception {
+
+ // make sure testUser has all privileges
+ Privilege[] privileges = privilegesFromName(Privilege.JCR_ALL);
+ givePrivileges(path, privileges, getRestrictions(superuser, path));
+
+ // create new node and lock it
+ Session s = getTestSession();
+ UserTransaction utx = new UserTransactionImpl(s);
+ utx.begin();
+
+ // add node and save it
+ Node n = s.getNode(childNPath);
+ if (n.hasNode(nodeName1)) {
+ Node c = n.getNode(nodeName1);
+ c.remove();
+ s.save();
+ }
+
+ // create node and save
+ Node n2 = n.addNode(nodeName1);
+ s.save(); // -> node is NEW -> no failure
+
+ // create child node
+ Node n3 = n2.addNode(nodeName2);
+ s.save(); // -> used to fail because n2 was saved (EXISTING) but not committed.
+
+ n3.remove();
+ n2.remove();
+
+ // recreate n2 // -> another area where ItemManager#getItem is called in the ItemSaveOperation
+ n2 = n.addNode(nodeName1);
+ s.save();
+
+ // commit
+ utx.commit();
+ }
+
private static Node findPolicyNode(Node start) throws RepositoryException {
Node policyNode = null;
if (start.isNodeType("rep:Policy")) {
Modified: jackrabbit/branches/2.2/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.2/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java?rev=1138531&r1=1138530&r2=1138531&view=diff
==============================================================================
--- jackrabbit/branches/2.2/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java (original)
+++ jackrabbit/branches/2.2/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java Wed Jun 22 16:16:26 2011
@@ -422,4 +422,86 @@ public class WriteTest extends AbstractW
n.addNode("someChild");
n.save();
}
+
+ public void testRemoveNodeWithPolicy() throws Exception {
+ Privilege[] privileges = privilegesFromNames(new String[] {Privilege.JCR_READ, Privilege.JCR_WRITE});
+
+ /* allow READ/WRITE privilege for testUser at 'path' */
+ givePrivileges(path, testUser.getPrincipal(), privileges, getRestrictions(superuser, path));
+ /* allow READ/WRITE privilege for testUser at 'childPath' */
+ givePrivileges(childNPath, testUser.getPrincipal(), privileges, getRestrictions(superuser, path));
+
+ Session testSession = getTestSession();
+
+ assertTrue(testSession.nodeExists(childNPath));
+ assertTrue(testSession.hasPermission(childNPath, Session.ACTION_REMOVE));
+
+ Node n = testSession.getNode(childNPath);
+
+ // removing the child node must succeed as both remove-node and
+ // remove-child-nodes are granted to testsession.
+ // the policy node underneath childNPath should silently be removed
+ // as the editing session has no knowledge about it's existence.
+ n.remove();
+ testSession.save();
+ }
+
+ public void testRemoveNodeWithInvisibleChild() throws Exception {
+ Privilege[] privileges = privilegesFromNames(new String[] {Privilege.JCR_READ, Privilege.JCR_WRITE});
+
+ Node invisible = superuser.getNode(childNPath).addNode(nodeName3);
+ superuser.save();
+
+ /* allow READ/WRITE privilege for testUser at 'path' */
+ givePrivileges(path, testUser.getPrincipal(), privileges, getRestrictions(superuser, path));
+ /* deny READ privilege at invisible node. (removal is still granted) */
+ withdrawPrivileges(invisible.getPath(), testUser.getPrincipal(),
+ privilegesFromNames(new String[] {Privilege.JCR_READ}),
+ getRestrictions(superuser, path));
+
+ Session testSession = getTestSession();
+
+ assertTrue(testSession.nodeExists(childNPath));
+ assertTrue(testSession.hasPermission(childNPath, Session.ACTION_REMOVE));
+
+ Node n = testSession.getNode(childNPath);
+
+ // removing the child node must succeed as both remove-node and
+ // remove-child-nodes are granted to testsession.
+ // the policy node underneath childNPath should silently be removed
+ // as the editing session has no knowledge about it's existence.
+ n.remove();
+ testSession.save();
+ }
+
+ public void testRemoveNodeWithInvisibleNonRemovableChild() throws Exception {
+ Privilege[] privileges = privilegesFromNames(new String[] {Privilege.JCR_READ, Privilege.JCR_WRITE});
+
+ Node invisible = superuser.getNode(childNPath).addNode(nodeName3);
+ superuser.save();
+
+ /* allow READ/WRITE privilege for testUser at 'path' */
+ givePrivileges(path, testUser.getPrincipal(), privileges, getRestrictions(superuser, path));
+ /* deny READ privilege at invisible node. (removal is still granted) */
+ withdrawPrivileges(invisible.getPath(), testUser.getPrincipal(),
+ privileges,
+ getRestrictions(superuser, path));
+
+ Session testSession = getTestSession();
+
+ assertTrue(testSession.nodeExists(childNPath));
+ assertTrue(testSession.hasPermission(childNPath, Session.ACTION_REMOVE));
+
+ Node n = testSession.getNode(childNPath);
+
+ // removing the child node must fail as a hidden child node cannot
+ // be removed.
+ try {
+ n.remove();
+ testSession.save();
+ fail();
+ } catch (AccessDeniedException e) {
+ // success
+ }
+ }
}
\ No newline at end of file