jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ang...@apache.org
Subject 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...
Date Wed, 22 Jun 2011 16:16:26 GMT
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



Mime
View raw message