jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ang...@apache.org
Subject svn commit: r921394 - in /jackrabbit/trunk/jackrabbit-core/src: main/java/org/apache/jackrabbit/core/security/authorization/acl/ test/java/org/apache/jackrabbit/core/security/authorization/ test/java/org/apache/jackrabbit/core/security/authorization/ac...
Date Wed, 10 Mar 2010 15:18:17 GMT
Author: angela
Date: Wed Mar 10 15:18:17 2010
New Revision: 921394

URL: http://svn.apache.org/viewvc?rev=921394&view=rev
Log:
JCR-2420: Node removal fails with AccessDeniedException

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLProvider.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplate.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractEntryTest.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractWriteTest.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/EntryTest.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/principalbased/EntryTest.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLProvider.java?rev=921394&r1=921393&r2=921394&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLProvider.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLProvider.java
Wed Mar 10 15:18:17 2010
@@ -19,19 +19,15 @@ package org.apache.jackrabbit.core.secur
 import java.security.Principal;
 import java.security.acl.Group;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.Iterator;
 
 import javax.jcr.ItemNotFoundException;
-import javax.jcr.NodeIterator;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
-import javax.jcr.Value;
 import javax.jcr.observation.Event;
 import javax.jcr.observation.EventIterator;
 import javax.jcr.security.AccessControlEntry;
@@ -402,14 +398,6 @@ public class ACLProvider extends Abstrac
             // retrieve all ACEs at path or at the direct ancestor of path that
             // apply for the principal names.
             Iterator<AccessControlEntry> entries = retrieveResultEntries(getNode(node),
principalNames);
-            // build a list of ACEs that are defined locally at the node
-            List<AccessControlEntry> localACEs;
-            if (existingNode && isAccessControlled(node)) {
-                NodeImpl aclNode = node.getNode(N_POLICY);
-                localACEs = Arrays.asList(systemEditor.getACL(aclNode).getAccessControlEntries());
-            } else {
-                localACEs = Collections.emptyList();
-            }
             /*
              Calculate privileges and permissions:
              Since the ACEs only define privileges on a node and do not allow
@@ -426,12 +414,14 @@ public class ACLProvider extends Abstrac
 
             while (entries.hasNext()) {
                 ACLTemplate.Entry ace = (ACLTemplate.Entry) entries.next();
-                // Determine if the ACE is defined on the node at absPath (locally):
-                // Except for READ-privileges the permissions must be determined
-                // from privileges defined for the parent. Consequently aces
-                // defined locally must be treated different than inherited entries.
+                /*
+                 Determine if the ACE is defined on the node at absPath (locally):
+                 Except for READ-privileges the permissions must be determined
+                 from privileges defined for the parent. Consequently aces
+                 defined locally must be treated different than inherited entries.
+                 */
                 int entryBits = ace.getPrivilegeBits();
-                boolean isLocal = localACEs.contains(ace);
+                boolean isLocal = existingNode && ace.isLocal(jcrPath);
                 if (!isLocal) {
                     if (ace.isAllow()) {
                         parentAllows |= Permission.diff(entryBits, parentDenies);
@@ -456,6 +446,7 @@ public class ACLProvider extends Abstrac
         /**
          * @see CompiledPermissions#close()
          */
+        @Override
         public void close() {
             try {
                 observationMgr.removeEventListener(this);
@@ -574,43 +565,21 @@ public class ACLProvider extends Abstrac
          * @throws RepositoryException if an error occurs
          */
         private void collectEntriesFromAcl(NodeImpl aclNode) throws RepositoryException {
-            SessionImpl sImpl = (SessionImpl) aclNode.getSession();
-            PrincipalManager principalMgr = sImpl.getPrincipalManager();
-            AccessControlManager acMgr = sImpl.getAccessControlManager();
-
             // first collect aces present on the given aclNode.
             List<AccessControlEntry> gaces = new ArrayList<AccessControlEntry>();
             List<AccessControlEntry> uaces = new ArrayList<AccessControlEntry>();
 
-            NodeIterator itr = aclNode.getNodes();
-            while (itr.hasNext()) {
-                NodeImpl aceNode = (NodeImpl) itr.nextNode();
-                String principalName = aceNode.getProperty(AccessControlConstants.P_PRINCIPAL_NAME).getString();
+            ACLTemplate tmpl = (ACLTemplate) systemEditor.getACL(aclNode);
+            for (AccessControlEntry ace : tmpl.getAccessControlEntries()) {
+                Principal principal = ace.getPrincipal();
                 // only process aceNode if 'principalName' is contained in the given set
-                if (principalNames.contains(principalName)) {
-                    Principal princ = principalMgr.getPrincipal(principalName);
-                    if (princ == null) {
-                        log.warn("Principal with name " + principalName + " unknown to PrincipalManager
-> Ignored from AC evaluation.");
-                        continue;
-                    }
-
-                    Value[] privValues = aceNode.getProperty(AccessControlConstants.P_PRIVILEGES).getValues();
-                    Privilege[] privs = new Privilege[privValues.length];
-                    for (int i = 0; i < privValues.length; i++) {
-                        privs[i] = acMgr.privilegeFromName(privValues[i].getString());
-                    }
-                    // create a new ACEImpl (omitting validation check)
-                    AccessControlEntry ace = new ACLTemplate.Entry(
-                            princ,
-                            privs,
-                            aceNode.isNodeType(AccessControlConstants.NT_REP_GRANT_ACE),
-                            sImpl.getValueFactory());
+                if (principalNames.contains(principal.getName())) {
                     // add it to the proper list (e.g. separated by principals)
                     /**
                      * NOTE: access control entries must be collected in reverse
                      * order in order to assert proper evaluation.
                      */
-                    if (princ instanceof Group) {
+                    if (principal instanceof Group) {
                         gaces.add(0, ace);
                     } else {
                         uaces.add(0, ace);

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplate.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplate.java?rev=921394&r1=921393&r2=921394&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplate.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplate.java
Wed Mar 10 15:18:17 2010
@@ -127,11 +127,10 @@ class ACLTemplate extends AbstractACLTem
                     privs[i] = acMgr.privilegeFromName(privValues[i].getString());
                 }
                 // create a new ACEImpl (omitting validation check)
-                Entry ace = new Entry(
+                Entry ace = createEntry(
                         princ,
                         privs,
-                        aceNode.isNodeType(AccessControlConstants.NT_REP_GRANT_ACE),
-                        valueFactory);
+                        aceNode.isNodeType(AccessControlConstants.NT_REP_GRANT_ACE));
                 // add the entry
                 internalAdd(ace);
             } catch (RepositoryException e) {
@@ -140,6 +139,18 @@ class ACLTemplate extends AbstractACLTem
         }
     }
 
+    /**
+     * Create a new entry omitting any validation checks.
+     * 
+     * @param principal
+     * @param privileges
+     * @param isAllow
+     * @return
+     */
+    Entry createEntry(Principal principal, Privilege[] privileges, boolean isAllow) throws
AccessControlException {
+        return new Entry(principal, privileges, isAllow);
+    }
+
     private List<Entry> internalGetEntries(Principal principal) {
         String principalName = principal.getName();
         List entriesPerPrincipal = new ArrayList(2);
@@ -185,7 +196,7 @@ class ACLTemplate extends AbstractACLTem
                     int mergedBits = e.getPrivilegeBits() | entry.getPrivilegeBits();
                     Privilege[] mergedPrivs = privilegeRegistry.getPrivileges(mergedBits);
                     // omit validation check.
-                    entry = new Entry(entry.getPrincipal(), mergedPrivs, entry.isAllow(),
valueFactory);
+                    entry = createEntry(entry.getPrincipal(), mergedPrivs, entry.isAllow());
                 } else {
                     complementEntry = e;
                 }
@@ -209,9 +220,9 @@ class ACLTemplate extends AbstractACLTem
                     // replace the existing entry having the privileges adjusted
                     int index = entries.indexOf(complementEntry);
                     entries.remove(complementEntry);
-                    Entry tmpl = new Entry(entry.getPrincipal(),
+                    Entry tmpl = createEntry(entry.getPrincipal(),
                             privilegeRegistry.getPrivileges(resultPrivs),
-                            !entry.isAllow(), valueFactory);
+                            !entry.isAllow());
                     entries.add(index, tmpl);
                 } /* else: does not need to be modified.*/
             }
@@ -231,6 +242,7 @@ class ACLTemplate extends AbstractACLTem
     /**
      * @see AbstractACLTemplate#checkValidEntry(java.security.Principal, javax.jcr.security.Privilege[],
boolean, java.util.Map) 
      */
+    @Override
     protected void checkValidEntry(Principal principal, Privilege[] privileges,
                                  boolean isAllow, Map<String, Value> restrictions)
             throws AccessControlException {
@@ -248,6 +260,7 @@ class ACLTemplate extends AbstractACLTem
     /**
      * @see org.apache.jackrabbit.core.security.authorization.AbstractACLTemplate#getEntries()
      */
+    @Override
     protected List<? extends AccessControlEntry> getEntries() {
         return entries;
     }
@@ -317,7 +330,7 @@ class ACLTemplate extends AbstractACLTem
                             boolean isAllow, Map<String, Value> restrictions)
             throws AccessControlException, RepositoryException {
         checkValidEntry(principal, privileges, isAllow, restrictions);
-        Entry ace = new Entry(principal, privileges, isAllow, valueFactory);
+        Entry ace = createEntry(principal, privileges, isAllow);
         return internalAdd(ace);
     }
 
@@ -356,11 +369,20 @@ class ACLTemplate extends AbstractACLTem
     /**
      *
      */
-    static class Entry extends AccessControlEntryImpl {
+    class Entry extends AccessControlEntryImpl {
 
-        Entry(Principal principal, Privilege[] privileges, boolean allow, ValueFactory valueFactory)
+        private Entry(Principal principal, Privilege[] privileges, boolean allow)
                 throws AccessControlException {
             super(principal, privileges, allow, Collections.<String, Value>emptyMap(),
valueFactory);
         }
+
+        /**
+         * @param nodePath
+         * @return <code>true</code> if this entry is defined on the node
+         * at <code>nodePath</code>
+         */
+        boolean isLocal(String nodePath) {
+            return path != null && path.equals(nodePath);
+        }
     }
 }

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractEntryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractEntryTest.java?rev=921394&r1=921393&r2=921394&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractEntryTest.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractEntryTest.java
Wed Mar 10 15:18:17 2010
@@ -37,6 +37,7 @@ public abstract class AbstractEntryTest 
 
     protected Principal testPrincipal;
 
+    @Override
     protected void setUp() throws Exception {
         super.setUp();
         testPrincipal = new Principal() {

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractWriteTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractWriteTest.java?rev=921394&r1=921393&r2=921394&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractWriteTest.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractWriteTest.java
Wed Mar 10 15:18:17 2010
@@ -63,6 +63,7 @@ public abstract class AbstractWriteTest 
     // TODO: test AC for moved node
     // TODO: test AC for moved AC-controlled node
 
+    @Override
     protected void setUp() throws Exception {
         super.setUp();
 
@@ -85,6 +86,7 @@ public abstract class AbstractWriteTest 
         siblingPath = n2.getPath();
     }
 
+    @Override
     protected void tearDown() throws Exception {
         try {
             if (testGroup != null && testUser != null) {
@@ -1035,6 +1037,33 @@ public abstract class AbstractWriteTest 
         n.orderBefore(Text.getName(childNPath), Text.getName(childNPath2));
         testSession.save();
     }
+    
+    /**
+     * Test case for JCR-2420
+     *
+     * @throws Exception
+     * @see <a href="https://issues.apache.org/jira/browse/JCR-2420">JCR-2420</a>
+     */
+    public void testRemovalJCR242() throws Exception {
+        Privilege[] allPriv = privilegesFromNames(new String[] {Privilege.JCR_ALL});
+
+        /* grant ALL privilege for testUser at 'path' */
+        givePrivileges(path, testUser.getPrincipal(), allPriv, getRestrictions(superuser,
path));
+        /* grant ALL privilege for testUser at 'childNPath' */
+        givePrivileges(childNPath, testUser.getPrincipal(), allPriv, getRestrictions(superuser,
childNPath));
+
+        Session testSession = getTestSession();
+        AccessControlManager acMgr = testSession.getAccessControlManager();
+
+        assertTrue(acMgr.hasPrivileges(path, allPriv));
+        assertTrue(acMgr.hasPrivileges(childNPath, allPriv));
+
+        assertTrue(testSession.hasPermission(childNPath, Session.ACTION_REMOVE));
+
+        Node child = testSession.getNode(childNPath);
+        child.remove();
+        testSession.save();
+    }
 
     private static Node findPolicyNode(Node start) throws RepositoryException {
         Node policyNode = null;

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/EntryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/EntryTest.java?rev=921394&r1=921393&r2=921394&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/EntryTest.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/EntryTest.java
Wed Mar 10 15:18:17 2010
@@ -17,7 +17,10 @@
 package org.apache.jackrabbit.core.security.authorization.acl;
 
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlEntry;
+import org.apache.jackrabbit.core.SessionImpl;
 import org.apache.jackrabbit.core.security.authorization.AbstractEntryTest;
+import org.apache.jackrabbit.core.security.authorization.PrivilegeRegistry;
+import org.apache.jackrabbit.test.NotExecutableException;
 
 import javax.jcr.RepositoryException;
 import javax.jcr.security.Privilege;
@@ -28,8 +31,26 @@ import java.security.Principal;
  */
 public class EntryTest extends AbstractEntryTest {
 
+    private ACLTemplate acl;
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+
+        SessionImpl s = (SessionImpl) superuser;
+        acl = new ACLTemplate(testPath, s.getPrincipalManager(), new PrivilegeRegistry(s),
s.getValueFactory());
+    }
+
+    @Override
     protected JackrabbitAccessControlEntry createEntry(Principal principal, Privilege[] privileges,
boolean isAllow)
             throws RepositoryException {
-        return new ACLTemplate.Entry(principal, privileges, isAllow, superuser.getValueFactory());
+        return acl.createEntry(principal, privileges, isAllow);
+    }
+
+    public void testIsLocal() throws NotExecutableException, RepositoryException {
+        ACLTemplate.Entry entry = (ACLTemplate.Entry) createEntry(new String[] {Privilege.JCR_READ},
true);
+
+        assertTrue(entry.isLocal(testPath));
+        assertFalse(entry.isLocal(testPath + "/foo"));
     }
 }
\ No newline at end of file

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java?rev=921394&r1=921393&r2=921394&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java
Wed Mar 10 15:18:17 2010
@@ -47,14 +47,17 @@ import java.util.UUID;
  */
 public class WriteTest extends AbstractWriteTest {
 
+    @Override
     protected boolean isExecutable() {
         return EvaluationUtil.isExecutable(acMgr);
     }
 
+    @Override
     protected JackrabbitAccessControlList getPolicy(AccessControlManager acM, String path,
Principal principal) throws RepositoryException, AccessDeniedException, NotExecutableException
{
         return EvaluationUtil.getPolicy(acM, path, principal);
     }
 
+    @Override
     protected Map<String, Value> getRestrictions(Session s, String path) {
         return Collections.emptyMap();
     }

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/principalbased/EntryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/principalbased/EntryTest.java?rev=921394&r1=921393&r2=921394&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/principalbased/EntryTest.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/principalbased/EntryTest.java
Wed Mar 10 15:18:17 2010
@@ -47,6 +47,7 @@ public class EntryTest extends AbstractE
     private String nodePath;
     private String glob;
 
+    @Override
     protected void setUp() throws Exception {
         super.setUp();
 
@@ -64,6 +65,7 @@ public class EntryTest extends AbstractE
         acl = new ACLTemplate(testPrincipal, testPath, (SessionImpl) superuser, superuser.getValueFactory());
     }
 
+    @Override
     protected JackrabbitAccessControlEntry createEntry(Principal principal, Privilege[] privileges,
boolean isAllow)
             throws RepositoryException {
         return (JackrabbitAccessControlEntry) acl.createEntry(principal, privileges, isAllow,
restrictions);



Mime
View raw message