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 2010/05/05 17:50:26 UTC

svn commit: r941358 - in /jackrabbit/trunk/jackrabbit-core/src: main/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLEditor.java test/java/org/apache/jackrabbit/core/security/authorization/principalbased/WriteTest.java

Author: angela
Date: Wed May  5 15:50:26 2010
New Revision: 941358

URL: http://svn.apache.org/viewvc?rev=941358&view=rev
Log:
JCR-2621 -  principalbased ACL editing fails if principalName differs from the authorizableID

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLEditor.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/principalbased/WriteTest.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLEditor.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLEditor.java?rev=941358&r1=941357&r2=941358&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLEditor.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLEditor.java Wed May  5 15:50:26 2010
@@ -20,9 +20,10 @@ import javax.jcr.security.AccessControlE
 import javax.jcr.security.AccessControlException;
 import javax.jcr.security.Privilege;
 import javax.jcr.security.AccessControlPolicy;
-import org.apache.jackrabbit.api.security.principal.PrincipalManager;
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlPolicy;
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlEntry;
+import org.apache.jackrabbit.api.security.user.Authorizable;
+import org.apache.jackrabbit.api.security.user.UserManager;
 import org.apache.jackrabbit.core.NodeImpl;
 import org.apache.jackrabbit.core.ProtectedItemModifier;
 import org.apache.jackrabbit.core.SessionImpl;
@@ -49,7 +50,7 @@ import javax.jcr.NodeIterator;
 import java.security.Principal;
 
 /**
- * <code>CombinedEditor</code>...
+ * <code>ACLEditor</code>...
  */
 public class ACLEditor extends ProtectedItemModifier implements AccessControlEditor, AccessControlConstants {
 
@@ -377,13 +378,44 @@ public class ACLEditor extends Protected
         return princPath.toString();
     }
 
-    private Principal getPrincipal(String pathToACNode) throws RepositoryException {
-        String name = getPrincipalName(pathToACNode);
-        PrincipalManager pMgr = session.getPrincipalManager();
-        return pMgr.getPrincipal(name);
+    /**
+     * Returns the principal for the given path or null.
+     *
+     * @param pathToACNode
+     * @return
+     * @throws RepositoryException
+     */
+    private Principal getPrincipal(final String pathToACNode) throws RepositoryException {
+        final String id = getPathName(pathToACNode);
+        UserManager uMgr = session.getUserManager();
+        Authorizable authorizable = uMgr.getAuthorizable(id);
+        if (authorizable == null) {
+            // human readable id in node name is different from the hashed id
+            // use workaround to retrieve the principal
+            if (pathToACNode.startsWith(acRootPath)) {
+                final String principalPath = pathToACNode.substring(acRootPath.length());
+                if (principalPath.indexOf('/', 1) > 0) {
+                    // safe to build an item based principal
+                    authorizable = uMgr.getAuthorizable(new ItemBasedPrincipal() {
+                        public String getPath() throws RepositoryException {
+                            return principalPath;
+                        }
+                        public String getName() {
+                            return Text.getName(principalPath);
+                        }
+                    });
+                } else {
+                    // principal name was just appended to the acRootPath prefix
+                    // see getPathToAcNode above -> try to retrieve principal by name.
+                    return session.getPrincipalManager().getPrincipal(Text.getName(principalPath));
+                }
+            } // else: path doesn't start with acRootPath -> return null.
+        }
+
+        return (authorizable == null) ? null : authorizable.getPrincipal();
     }
 
-    private static String getPrincipalName(String pathToACNode) {
+    private static String getPathName(String pathToACNode) {
         return Text.unescapeIllegalJcrChars(Text.getName(pathToACNode));
     }
 
@@ -413,7 +445,7 @@ public class ACLEditor extends Protected
         Principal principal = getPrincipal(acNode.getPath());
         if (principal == null) {
             // use fall back in order to be able to get/remove the policy
-            String principalName = getPrincipalName(acNode.getPath());
+            String principalName = getPathName(acNode.getPath());
             log.warn("Principal with name " + principalName + " unknown to PrincipalManager.");
             principal = new PrincipalImpl(principalName);
         }

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/principalbased/WriteTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/principalbased/WriteTest.java?rev=941358&r1=941357&r2=941358&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/principalbased/WriteTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/principalbased/WriteTest.java Wed May  5 15:50:26 2010
@@ -16,16 +16,19 @@
  */
 package org.apache.jackrabbit.core.security.authorization.principalbased;
 
+import org.apache.jackrabbit.api.JackrabbitSession;
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlList;
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlManager;
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlPolicy;
 import org.apache.jackrabbit.api.security.principal.ItemBasedPrincipal;
+import org.apache.jackrabbit.api.security.principal.PrincipalManager;
 import org.apache.jackrabbit.api.security.user.UserManager;
 import org.apache.jackrabbit.api.security.user.User;
 import org.apache.jackrabbit.core.SessionImpl;
 import org.apache.jackrabbit.core.security.authorization.AbstractWriteTest;
 import org.apache.jackrabbit.core.security.authorization.PrivilegeRegistry;
 import org.apache.jackrabbit.core.security.TestPrincipal;
+import org.apache.jackrabbit.core.security.principal.PrincipalImpl;
 import org.apache.jackrabbit.test.NotExecutableException;
 import org.apache.jackrabbit.util.Text;
 
@@ -34,7 +37,9 @@ import javax.jcr.Node;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
 import javax.jcr.Value;
+import javax.jcr.security.AccessControlException;
 import javax.jcr.security.AccessControlManager;
+import javax.jcr.security.AccessControlPolicy;
 import javax.jcr.security.Privilege;
 import java.security.Principal;
 import java.util.Map;
@@ -44,19 +49,21 @@ import java.util.Map;
  */
 public class WriteTest extends AbstractWriteTest {
 
+    @Override
     protected boolean isExecutable() {
         return EvaluationUtil.isExecutable((SessionImpl) superuser, 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) throws RepositoryException, NotExecutableException {
         return EvaluationUtil.getRestrictions(s, path);
     }
 
-
     public void testAutocreatedProperties() throws RepositoryException, NotExecutableException {
         givePrivileges(path, testUser.getPrincipal(), privilegesFromName(PrivilegeRegistry.REP_WRITE), getRestrictions(superuser, path));
 
@@ -154,5 +161,93 @@ public class WriteTest extends AbstractW
         }
 
     }
+
+    /**
+     * Test for bug JCR-2621
+     * 
+     * @throws Exception
+     */
+    public void testPrincipalNameDiffersFromID() throws Exception {
+        UserManager uMgr = getUserManager(superuser);
+        User u = null;
+        try {
+            // create a user with different uid vs principal name
+            u = uMgr.createUser("t@foo.org", "t", new PrincipalImpl("t"), null);
+            if (!uMgr.isAutoSave()) {
+                superuser.save();
+            }
+
+            Principal principal = u.getPrincipal();
+            JackrabbitAccessControlList acl = getPolicy(acMgr, path, principal);
+            acl.addEntry(principal, privilegesFromName(Privilege.JCR_READ), true, getRestrictions(superuser, path));
+            acMgr.setPolicy(acl.getPath(), acl);
+
+            AccessControlPolicy[] plcs = acMgr.getPolicies(acl.getPath());
+            assertEquals(1, plcs.length);
+            acl = (JackrabbitAccessControlList) plcs[0];
+            acl.addEntry(principal, privilegesFromName(Privilege.JCR_WRITE), true, getRestrictions(superuser, path));
+            acMgr.setPolicy(acl.getPath(), acl);
+
+        } finally {
+            if (u != null) {
+                u.remove();
+            }
+        }
+    }
+
+    /**
+     * Test for bug JCR-2621
+     *
+     * @throws Exception
+     */
+    public void testNotItemBasedPrincipal() throws Exception {
+        try {
+            Principal everyone = ((JackrabbitSession) superuser).getPrincipalManager().getEveryone();
+            JackrabbitAccessControlList acl = getPolicy(acMgr, path, everyone);
+            acl.addEntry(everyone, privilegesFromName(Privilege.JCR_READ), true, getRestrictions(superuser, path));
+            acMgr.setPolicy(acl.getPath(), acl);
+
+            AccessControlPolicy[] plcs = acMgr.getPolicies(acl.getPath());
+            assertEquals(1, plcs.length);
+            acl = (JackrabbitAccessControlList) plcs[0];
+            acl.addEntry(everyone, privilegesFromName(Privilege.JCR_WRITE), true, getRestrictions(superuser, path));
+            acMgr.setPolicy(acl.getPath(), acl);
+        } finally {
+            // revert all kind of transient modifications
+            superuser.refresh(false);
+        }
+    }
+
+    public void testInvalidPrincipal() throws Exception {
+        PrincipalManager pMgr = ((JackrabbitSession) superuser).getPrincipalManager();
+        String unknown = "unkown";
+        while (pMgr.hasPrincipal(unknown)) {
+            unknown = unknown + "_";
+        }
+        Principal principal = new PrincipalImpl(unknown);
+        
+        if (acMgr instanceof JackrabbitAccessControlManager) {
+            // first try applicable policies
+            try {
+                AccessControlPolicy[] policies = ((JackrabbitAccessControlManager) acMgr).getApplicablePolicies(principal);
+                assertNotNull(policies);
+                assertEquals(0, policies.length);
+            } catch (AccessControlException e) {
+                // success
+            }
+            
+            // second existing policies
+            try {
+                AccessControlPolicy[] policies = ((JackrabbitAccessControlManager) acMgr).getPolicies(principal);
+                assertNotNull(policies);
+                assertEquals(0, policies.length);
+            } catch (AccessControlException e) {
+                // success
+            }
+        } else {
+            throw new NotExecutableException();
+        }
+    }
+    
     // TODO: add specific tests with other restrictions
 }