You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by ju...@apache.org on 2010/08/26 12:48:45 UTC

svn commit: r989597 [2/2] - in /jackrabbit/branches/2.1: ./ jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ jackrabbit-core/src/main/java/org/apache/jackrabbit/core/observation/ jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/l...

Modified: jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLProvider.java?rev=989597&r1=989596&r2=989597&view=diff
==============================================================================
--- jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLProvider.java (original)
+++ jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLProvider.java Thu Aug 26 10:48:44 2010
@@ -16,49 +16,46 @@
  */
 package org.apache.jackrabbit.core.security.authorization.principalbased;
 
-import java.security.Principal;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
-import javax.jcr.Item;
-import javax.jcr.ItemNotFoundException;
-import javax.jcr.PropertyType;
-import javax.jcr.RepositoryException;
-import javax.jcr.Session;
-import javax.jcr.Value;
-import javax.jcr.ValueFactory;
-import javax.jcr.observation.Event;
-import javax.jcr.observation.EventIterator;
-import javax.jcr.security.AccessControlEntry;
-import javax.jcr.security.AccessControlException;
-import javax.jcr.security.AccessControlManager;
-import javax.jcr.security.AccessControlPolicy;
-import javax.jcr.security.Privilege;
-
+import org.apache.commons.collections.map.LRUMap;
 import org.apache.jackrabbit.api.security.principal.PrincipalManager;
-import org.apache.jackrabbit.core.ItemImpl;
 import org.apache.jackrabbit.core.NodeImpl;
 import org.apache.jackrabbit.core.SessionImpl;
-import org.apache.jackrabbit.core.observation.SynchronousEventListener;
+import org.apache.jackrabbit.core.id.ItemId;
 import org.apache.jackrabbit.core.security.SecurityConstants;
 import org.apache.jackrabbit.core.security.authorization.AbstractAccessControlProvider;
 import org.apache.jackrabbit.core.security.authorization.AbstractCompiledPermissions;
 import org.apache.jackrabbit.core.security.authorization.AccessControlConstants;
 import org.apache.jackrabbit.core.security.authorization.AccessControlEditor;
+import org.apache.jackrabbit.core.security.authorization.AccessControlEntryImpl;
+import org.apache.jackrabbit.core.security.authorization.AccessControlListener;
+import org.apache.jackrabbit.core.security.authorization.AccessControlModifications;
 import org.apache.jackrabbit.core.security.authorization.CompiledPermissions;
 import org.apache.jackrabbit.core.security.authorization.Permission;
 import org.apache.jackrabbit.core.security.authorization.PrivilegeRegistry;
 import org.apache.jackrabbit.spi.Path;
-import org.apache.jackrabbit.spi.commons.name.PathFactoryImpl;
 import org.apache.jackrabbit.util.Text;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.jcr.ItemNotFoundException;
+import javax.jcr.PropertyType;
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import javax.jcr.Value;
+import javax.jcr.ValueFactory;
+import javax.jcr.security.AccessControlEntry;
+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.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
 /**
  * <code>CombinedProvider</code>...
  */
@@ -69,31 +66,11 @@ public class ACLProvider extends Abstrac
     // TODO: add means to show effective-policy to a user.
     private static final AccessControlPolicy effectivePolicy = EffectivePrincipalBasedPolicy.getInstance();
 
+    private NodeImpl acRoot;    
     private ACLEditor editor;
 
-    private NodeImpl acRoot;
-
-    //-------------------------------------------------< AccessControlUtils >---
-    /**
-     * @see org.apache.jackrabbit.core.security.authorization.AccessControlUtils#isAcItem(Path)
-     */
-    public boolean isAcItem(Path absPath) throws RepositoryException {
-        Path.Element[] elems = absPath.getElements();
-        for (Path.Element elem : elems) {
-            if (N_POLICY.equals(elem.getName())) {
-                return true;
-            }
-        }
-        return false;
-    }
-
-    /**
-     * @see org.apache.jackrabbit.core.security.authorization.AccessControlUtils#isAcItem(ItemImpl)
-     */
-    public boolean isAcItem(ItemImpl item) throws RepositoryException {
-        NodeImpl n = ((item.isNode()) ? (NodeImpl) item : (NodeImpl) item.getParent());
-        return n.isNodeType(NT_REP_ACL) || n.isNodeType(NT_REP_ACE);
-    }
+    private EntriesCache entriesCache;
+    private int readBits;
 
     //----------------------------------------------< AccessControlProvider >---
     /**
@@ -114,6 +91,9 @@ public class ACLProvider extends Abstrac
         }
 
         editor = new ACLEditor(session, resolver.getQPath(acRoot.getPath()));
+        entriesCache = new EntriesCache(session, editor, acRoot.getPath());
+        readBits = PrivilegeRegistry.getBits(new Privilege[] {session.getAccessControlManager().privilegeFromName(Privilege.JCR_READ)});
+
         // TODO: replace by configurable default policy (see JCR-2331)
         if (!configuration.containsKey(PARAM_OMIT_DEFAULT_PERMISSIONS)) {
             try {
@@ -169,6 +149,12 @@ public class ACLProvider extends Abstrac
             log.debug("... policy for principal  '"+principal.getName()+"'  already present.");
         }
     }
+    
+    @Override
+    public void close() {
+        super.close();        
+        entriesCache.close();
+    }
 
     /**
      * @see org.apache.jackrabbit.core.security.authorization.AccessControlProvider#getEffectivePolicies(Path)
@@ -181,7 +167,7 @@ public class ACLProvider extends Abstrac
            entries, returning the principal-based policy at 'absPath' (which for
            most nodes in the repository isn't available anyway) doesn't
            provide the desired information.
-           As tmp. solution some default policy is returned indicating.
+           As tmp. solution some default policy is returned instead.
            TODO: add proper evaluation and return a set of ACLs that take effect on the node at abs path
         */
         return new AccessControlPolicy[] {effectivePolicy};
@@ -215,7 +201,7 @@ public class ACLProvider extends Abstrac
         } else if (isReadOnly(principals)) {
             return getReadOnlyPermissions();
         } else {
-            return new ACLProvider.CompiledPermissionImpl(principals);
+            return new CompiledPermissionImpl(principals);
         }
     }
 
@@ -227,8 +213,8 @@ public class ACLProvider extends Abstrac
         if (isAdminOrSystem(principals)) {
             return true;
         } else {
-            CompiledPermissions cp = new CompiledPermissionImpl(principals, false);
-            return cp.grants(PathFactoryImpl.getInstance().getRootPath(), Permission.READ);
+            CompiledPermissionImpl cp = new CompiledPermissionImpl(principals, false);
+            return cp.canRead(((NodeImpl) session.getRootNode()).getPrimaryPath());
         }
     }
 
@@ -237,12 +223,17 @@ public class ACLProvider extends Abstrac
      *
      */
     private class CompiledPermissionImpl extends AbstractCompiledPermissions
-            implements SynchronousEventListener {
+            implements AccessControlListener {
 
         private final Set<Principal> principals;
         private final Set<String> acPaths;
         private List<AccessControlEntry> entries;
 
+        private boolean canReadAll;
+
+        private final Map<ItemId, Boolean> readCache = new LRUMap(2000);
+        private final Object monitor = new Object();
+
         /**
          * @param principals the underlying principals
          * @throws RepositoryException if an error occurs
@@ -260,16 +251,43 @@ public class ACLProvider extends Abstrac
 
             this.principals = principals;
             acPaths = new HashSet<String>(principals.size());
-            entries = reload();
+            reload();
 
-            // TODO: describe
             if (listenToEvents) {
-                int events = Event.PROPERTY_CHANGED | Event.PROPERTY_ADDED |
-                        Event.PROPERTY_REMOVED | Event.NODE_ADDED | Event.NODE_REMOVED;
-                String[] ntNames = new String[] {
-                        session.getJCRName(NT_REP_ACE)
-                };
-                observationMgr.addEventListener(this, events, acRoot.getPath(), true, null, ntNames, false);
+                /*
+                 Make sure this AclPermission recalculates the permissions if
+                 any ACL concerning it is modified.
+                 */
+                 entriesCache.addListener(this);
+            }
+        }
+               
+        /**
+         * @throws RepositoryException if an error occurs
+         */
+        private void reload() throws RepositoryException {
+            // reload the paths
+            acPaths.clear();
+            for (Principal p : principals) {
+                acPaths.add(editor.getPathToAcNode(p));
+            }
+
+            // and retrieve the entries from the entry-collector.
+            entries = entriesCache.getEntries(principals);
+            
+            // in addition: trivial check if read access is deny somewhere
+            // as as shortcut in #canRead(Path)
+            canReadAll = canRead(session.getQPath("/"));            
+            if (canReadAll) {
+                for (AccessControlEntry entry : entries) {
+                    AccessControlEntryImpl ace = (AccessControlEntryImpl) entry;
+                    if (!ace.isAllow() && ((ace.getPrivilegeBits() & readBits) == readBits)) {
+                        // found an ace that defines read deny for a sub tree
+                        // -> canReadAll is false.
+                        canReadAll = false;
+                        break;
+                    }
+                }
             }
         }
 
@@ -287,32 +305,22 @@ public class ACLProvider extends Abstrac
             String jcrPath = session.getJCRPath(absPath);
 
             // retrieve principal-based permissions and privileges
-            Result result;
-            if (session.itemExists(jcrPath)) {
-                Item item = session.getItem(jcrPath);
-                result = getResult(item, item.getPath(), isAcItem);
-            } else {
-                result = getResult(null, jcrPath, isAcItem);
-            }
-            return result;
+            return buildResult(jcrPath, isAcItem);
         }
 
 
         /**
          * Loop over all entries and evaluate allows/denies for those matching
          * the given jcrPath.
-         *
-         * @param target Existing target item for which the permissions will be
-         * evaluated or <code>null</code>.
+         * 
          * @param targetPath Path used for the evaluation; pointing to an
          * existing or non-existing item.
          * @param isAcItem the item
          * @return the result
          * @throws RepositoryException if an error occurs
          */
-        private Result getResult(Item target,
-                                 String targetPath,
-                                 boolean isAcItem) throws RepositoryException {
+        private Result buildResult(String targetPath,
+                                   boolean isAcItem) throws RepositoryException {
             int allows = Permission.NONE;
             int denies = Permission.NONE;
             int allowPrivileges = PrivilegeRegistry.NO_PRIVILEGE;
@@ -337,7 +345,7 @@ public class ACLProvider extends Abstrac
                     }
                 }
 
-                boolean matches = (target != null) ? entr.matches(target) : entr.matches(targetPath);
+                boolean matches = entr.matches(targetPath);
                 if (matches) {
                     if (entr.isAllow()) {
                         allowPrivileges |= Permission.diff(privs, denyPrivileges);
@@ -359,84 +367,63 @@ public class ACLProvider extends Abstrac
          */
         @Override
         public void close() {
-            try {
-                observationMgr.removeEventListener(this);
-            } catch (RepositoryException e) {
-                log.debug("Unable to unregister listener: ", e.getMessage());
-            }
+            entriesCache.removeListener(this);
             super.close();
         }
 
-        //--------------------------------------------------< EventListener >---
         /**
-         * @see javax.jcr.observation.EventListener#onEvent(EventIterator)
+         * @see CompiledPermissions#canRead(Path, ItemId)
          */
-        public synchronized void onEvent(EventIterator events) {
+        public boolean canRead(Path path, ItemId itemId) throws RepositoryException {
+            boolean canRead;
+            if (path == null) {
+                // only itemId: try to avoid expensive resolution from itemID to path
+                synchronized (monitor) {
+                    if (readCache.containsKey(itemId)) {
+                        // id has been evaluated before -> shortcut
+                        canRead = readCache.get(itemId);
+                    } else {
+                        canRead = canRead(session.getHierarchyManager().getPath(itemId));
+                        readCache.put(itemId, canRead);
+                        return canRead;
+                    }
+                }
+            } else {
+                // path param present:
+                canRead = canRead(path);
+            }
+            return canRead;
+        }
+
+        private boolean canRead(Path path) throws RepositoryException {
+            // first try if reading non-ac-items was always granted -> no eval
+            // otherwise evaluate the permissions.
+            return (canReadAll && !isAcItem(path)) || grants(path, Permission.READ);
+        }
+
+        //------------------------------------------< AccessControlListener >---
+        /**
+         * @see AccessControlListener#acModified(org.apache.jackrabbit.core.security.authorization.AccessControlModifications)
+         */
+        public void acModified(AccessControlModifications modifications) {
             try {
                 boolean reload = false;
-                while (events.hasNext() && !reload) {
-                    Event ev = events.nextEvent();
-                    String path = ev.getPath();
-                    // only invalidate cache if any of the events affects the
-                    // nodes defining permissions for the principals.
-                    switch (ev.getType()) {
-                        case Event.NODE_ADDED:
-                        case Event.NODE_REMOVED:
-                        case Event.NODE_MOVED:
-                            reload = acPaths.contains(Text.getRelativeParent(path, 2));
-                            break;
-                        case Event.PROPERTY_ADDED:
-                        case Event.PROPERTY_CHANGED:
-                        case Event.PROPERTY_REMOVED:
-                            reload = acPaths.contains(Text.getRelativeParent(path, 3));
-                            break;
-
-                        default:
-                            // illegal event-type: should never occur. ignore
-                            break;
-                    }
+                Iterator keys = modifications.getNodeIdentifiers().iterator();
+                while (keys.hasNext() && !reload) {
+                    String path = keys.next().toString();
+                    reload = acPaths.contains(path);
                 }
                 // eventually reload the ACL and clear the cache
                 if (reload) {
                     clearCache();
-                    // reload the acl
-                    entries = reload();
+                    // reload the ac-path list and the list of aces
+                    reload();
                 }
             } catch (RepositoryException e) {
                 // should never get here
                 log.warn("Internal error: ", e.getMessage());
             }
         }
-
-        /**
-         *
-         * @return the aces
-         * @throws RepositoryException if an error occurs
-         */
-        private List<AccessControlEntry> reload() throws RepositoryException {
-            // reload the paths
-            acPaths.clear();
-
-            // acNodes must be ordered in the same order as the principals
-            // in order to obtain proper acl-evaluation in case the given
-            // principal-set is ordered.
-            List<AccessControlEntry> allACEs = new ArrayList<AccessControlEntry>();
-            // build acl-hierarchy assuming that principal-order determines the
-            // acl-inheritance.
-            for (Principal p : principals) {
-                ACLTemplate acl = editor.getACL(p);
-                if (acl == null || acl.isEmpty()) {
-                    acPaths.add(editor.getPathToAcNode(p));
-                } else {
-                    // retrieve the ACEs from the node
-                    AccessControlEntry[] aces = acl.getAccessControlEntries();
-                    allACEs.addAll(Arrays.asList(aces));
-                    acPaths.add(acl.getPath());
-                }
-            }
-
-            return allACEs;
-        }
     }
 
     //--------------------------------------------------------------------------

Modified: jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLTemplate.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLTemplate.java?rev=989597&r1=989596&r2=989597&view=diff
==============================================================================
--- jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLTemplate.java (original)
+++ jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLTemplate.java Thu Aug 26 10:48:44 2010
@@ -155,6 +155,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 {
@@ -171,6 +172,7 @@ class ACLTemplate extends AbstractACLTem
     /**
      * @see org.apache.jackrabbit.core.security.authorization.AbstractACLTemplate#getEntries() 
      */
+    @Override
     protected List<? extends AccessControlEntry> getEntries() {
         return entries;
     }
@@ -197,20 +199,6 @@ class ACLTemplate extends AbstractACLTem
     }
 
     /**
-     * @see JackrabbitAccessControlList#isEmpty()
-     */
-    public boolean isEmpty() {
-        return entries.isEmpty();
-    }
-
-    /**
-     * @see org.apache.jackrabbit.api.security.JackrabbitAccessControlList#size()
-     */
-    public int size() {
-        return entries.size();
-    }
-
-    /**
      * Known restrictions are:
      * <pre>
      *   rep:nodePath  (mandatory) value-type: PATH
@@ -241,14 +229,6 @@ class ACLTemplate extends AbstractACLTem
 
     //--------------------------------------------------< AccessControlList >---
     /**
-     * @see javax.jcr.security.AccessControlList#getAccessControlEntries()
-     */
-    public AccessControlEntry[] getAccessControlEntries()
-            throws RepositoryException {
-        return entries.toArray(new AccessControlEntry[entries.size()]);
-    }
-
-    /**
      * @see javax.jcr.security.AccessControlList#removeAccessControlEntry(AccessControlEntry)
      */
     public void removeAccessControlEntry(AccessControlEntry ace)
@@ -269,6 +249,7 @@ class ACLTemplate extends AbstractACLTem
      * @return always zero
      * @see Object#hashCode()
      */
+    @Override
     public int hashCode() {
         return 0;
     }
@@ -280,6 +261,7 @@ class ACLTemplate extends AbstractACLTem
      * @return true if the path and the entries are equal; false otherwise.
      * @see Object#equals(Object)
      */
+    @Override
     public boolean equals(Object obj) {
         if (obj == this) {
             return true;
@@ -305,7 +287,7 @@ class ACLTemplate extends AbstractACLTem
         private final String nodePath;
 
         /**
-         * Globbing pattern
+         * Globing pattern
          */
         private final GlobPattern pattern;
 

Modified: jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/GlobPattern.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/GlobPattern.java?rev=989597&r1=989596&r2=989597&view=diff
==============================================================================
--- jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/GlobPattern.java (original)
+++ jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/GlobPattern.java Thu Aug 26 10:48:44 2010
@@ -36,9 +36,19 @@ class GlobPattern {
     public static final String WILDCARD_ALL = "*";
 
     private final String pattern;
+    private final char[] patternChars;
+
+    private final boolean matchesAll;
+    private final boolean containsWildcard;
 
     private GlobPattern(String pattern)  {
         this.pattern = pattern;
+
+        matchesAll = WILDCARD_ALL.equals(pattern);
+        containsWildcard = pattern.indexOf(ALL) > -1;
+
+        patternChars = pattern.toCharArray();
+
     }
 
     static GlobPattern create(String pattern) {
@@ -50,17 +60,17 @@ class GlobPattern {
 
     boolean matches(String toMatch) {
         // shortcut
-        if (WILDCARD_ALL.equals(pattern)) {
+        if (matchesAll) {
             return true;
         }
         if (toMatch == null) {
             return false;
         }
 
-        if (containsWildCard()) {
-            return matches(pattern, toMatch);
+        if (containsWildcard) {
+            return matches(patternChars, toMatch.toCharArray());
         } else {
-            return pattern.equals(toMatch) || Text.isDescendant(pattern, toMatch);
+            return Text.isDescendantOrEqual(pattern, toMatch);
         }
     }
 
@@ -74,21 +84,13 @@ class GlobPattern {
         }
     }
 
-    private boolean containsWildCard() {
+    private static boolean matches(char[] patternChars, char[] toMatch) {
         // TODO: add proper impl
-        return pattern.indexOf(ALL) > -1;
-    }
-
-    private static boolean matches(String pattern, String toMatch) {
-        // TODO: add proper impl
-        char[] c1 = pattern.toCharArray();
-        char[] c2 = toMatch.toCharArray();
-
-        for (int i = 0; i < c1.length; i++) {
-            if (c1[i] == ALL) {
+        for (int i = 0; i < patternChars.length; i++) {
+            if (patternChars[i] == ALL) {
                 return true;
             }
-            if (i >= c2.length || c1[i] != c2[i]) {
+            if (i >= toMatch.length || patternChars[i] != toMatch[i]) {
                 return false;
             }
         }
@@ -101,6 +103,7 @@ class GlobPattern {
     /**
      * @see Object#hashCode()
      */
+    @Override
     public int hashCode() {
         return pattern.hashCode();
     }
@@ -108,6 +111,7 @@ class GlobPattern {
     /**
      * @see Object#toString()
      */
+    @Override
     public String toString() {
         return pattern;
     }
@@ -115,6 +119,7 @@ class GlobPattern {
     /**
      * @see Object#equals(Object)
      */
+    @Override
     public boolean equals(Object obj) {
         if (obj == this) {
             return true;

Modified: jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleAccessManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleAccessManager.java?rev=989597&r1=989596&r2=989597&view=diff
==============================================================================
--- jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleAccessManager.java (original)
+++ jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleAccessManager.java Thu Aug 26 10:48:44 2010
@@ -167,7 +167,7 @@ public class SimpleAccessManager extends
         return internalIsGranted(parentPath, permissions);
     }
 
-    public boolean canRead(Path itemPath) throws RepositoryException {
+    public boolean canRead(Path itemPath, ItemId itemId) throws RepositoryException {
         return true;
     }
 

Modified: jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleLoginModule.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleLoginModule.java?rev=989597&r1=989596&r2=989597&view=diff
==============================================================================
--- jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleLoginModule.java (original)
+++ jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleLoginModule.java Thu Aug 26 10:48:44 2010
@@ -41,6 +41,7 @@ public class SimpleLoginModule extends A
     /**
      * @see AbstractLoginModule#doInit(javax.security.auth.callback.CallbackHandler, javax.jcr.Session, java.util.Map)
      */
+    @Override
     protected void doInit(CallbackHandler callbackHandler, Session session, Map options) throws LoginException {
         // nothing to do
         log.debug("init: SimpleLoginModule. Done.");
@@ -49,6 +50,7 @@ public class SimpleLoginModule extends A
     /**
      * @see AbstractLoginModule#impersonate(java.security.Principal, javax.jcr.Credentials)
      */
+    @Override
     protected boolean impersonate(Principal principal, Credentials credentials) throws RepositoryException, LoginException {
         if (principal instanceof Group) {
             return false;
@@ -60,6 +62,7 @@ public class SimpleLoginModule extends A
     /**
      * @see AbstractLoginModule#getAuthentication(java.security.Principal, javax.jcr.Credentials)
      */
+    @Override
     protected Authentication getAuthentication(Principal principal, Credentials creds) throws RepositoryException {
         if (principal instanceof Group) {
             return null;
@@ -86,6 +89,7 @@ public class SimpleLoginModule extends A
      *
      * @see AbstractLoginModule#getPrincipal(Credentials)
      */
+    @Override
     protected Principal getPrincipal(Credentials credentials) {
         String userId = getUserID(credentials);
         Principal principal = principalProvider.getPrincipal(userId);

Modified: jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserAccessControlProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserAccessControlProvider.java?rev=989597&r1=989596&r2=989597&view=diff
==============================================================================
--- jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserAccessControlProvider.java (original)
+++ jackrabbit/branches/2.1/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserAccessControlProvider.java Thu Aug 26 10:48:44 2010
@@ -24,6 +24,7 @@ import org.apache.jackrabbit.api.securit
 import org.apache.jackrabbit.core.ItemImpl;
 import org.apache.jackrabbit.core.NodeImpl;
 import org.apache.jackrabbit.core.SessionImpl;
+import org.apache.jackrabbit.core.id.ItemId;
 import org.apache.jackrabbit.core.observation.SynchronousEventListener;
 import org.apache.jackrabbit.core.security.authorization.AbstractAccessControlProvider;
 import org.apache.jackrabbit.core.security.authorization.AbstractCompiledPermissions;
@@ -462,6 +463,13 @@ public class UserAccessControlProvider e
             return session.nodeExists(userNodePath);
         }
 
+        /**
+         * @see CompiledPermissions#canRead(Path, ItemId)
+         */
+        public boolean canRead(Path path, ItemId itemId) throws RepositoryException {
+            return canReadAll();
+        }
+
         //--------------------------------------------------< EventListener >---
         /**
          * Event listener is only interested in changes of group-membership

Modified: jackrabbit/branches/2.1/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/AccessManagerTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.1/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/AccessManagerTest.java?rev=989597&r1=989596&r2=989597&view=diff
==============================================================================
--- jackrabbit/branches/2.1/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/AccessManagerTest.java (original)
+++ jackrabbit/branches/2.1/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/AccessManagerTest.java Thu Aug 26 10:48:44 2010
@@ -21,6 +21,8 @@ import org.apache.jackrabbit.core.ItemIm
 import org.apache.jackrabbit.core.id.NodeId;
 import org.apache.jackrabbit.core.id.PropertyId;
 import org.apache.jackrabbit.core.SessionImpl;
+import org.apache.jackrabbit.core.NodeImpl;
+import org.apache.jackrabbit.core.PropertyImpl;
 import org.apache.jackrabbit.core.security.authorization.Permission;
 import org.apache.jackrabbit.spi.Path;
 import org.apache.jackrabbit.spi.commons.name.NameConstants;
@@ -280,4 +282,51 @@ public class AccessManagerTest extends A
             s.logout();
         }
     }
+
+    public void testCanReadPathId() throws Exception {
+        Session s = getHelper().getReadOnlySession();
+        try {
+            AccessManager acMgr = getAccessManager(s);
+
+            ItemId id = ((NodeImpl) testRootNode).getId();
+            Path path = ((NodeImpl) testRootNode).getPrimaryPath();
+            assertTrue(acMgr.canRead(null, id));
+            assertTrue(acMgr.canRead(path, null));
+            assertTrue(acMgr.canRead(path, id));
+
+            id = ((PropertyImpl) testRootNode.getProperty(jcrPrimaryType)).getId();
+            path = ((PropertyImpl) testRootNode.getProperty(jcrPrimaryType)).getPrimaryPath();
+            assertTrue(acMgr.canRead(null, id));
+            assertTrue(acMgr.canRead(path, null));
+            assertTrue(acMgr.canRead(path, id));
+
+        } finally {
+            s.logout();
+        }
+
+    }
+
+    public void testCanReadNewId() throws Exception {
+        Session s = getHelper().getReadOnlySession();
+        try {
+            NodeImpl n = (NodeImpl) testRootNode.addNode(nodeName1);
+            PropertyImpl p = (PropertyImpl) n.setProperty(propertyName1, "somevalue");
+            try {
+                AccessManager acMgr = getAccessManager(s);
+                acMgr.canRead(null, n.getId());
+                fail("expected repositoryexception");
+            } catch (RepositoryException e) {
+                // success
+            }
+            try {
+                AccessManager acMgr = getAccessManager(s);
+                acMgr.canRead(null, p.getId());
+                fail("expected repositoryexception");
+            } catch (RepositoryException e) {
+                // success
+            }
+        } finally {
+            s.logout();
+        }
+    }
 }
\ No newline at end of file

Modified: jackrabbit/branches/2.1/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/EntryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.1/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/EntryTest.java?rev=989597&r1=989596&r2=989597&view=diff
==============================================================================
--- jackrabbit/branches/2.1/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/EntryTest.java (original)
+++ jackrabbit/branches/2.1/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/EntryTest.java Thu Aug 26 10:48:44 2010
@@ -17,12 +17,16 @@
 package org.apache.jackrabbit.core.security.authorization.acl;
 
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlEntry;
+import org.apache.jackrabbit.core.NodeImpl;
 import org.apache.jackrabbit.core.SessionImpl;
+import org.apache.jackrabbit.core.id.NodeId;
 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.AccessControlPolicy;
+import javax.jcr.security.AccessControlPolicyIterator;
 import javax.jcr.security.Privilege;
 import java.security.Principal;
 
@@ -38,6 +42,7 @@ public class EntryTest extends AbstractE
         super.setUp();
 
         SessionImpl s = (SessionImpl) superuser;
+
         acl = new ACLTemplate(testPath, s.getPrincipalManager(), new PrivilegeRegistry(s), s.getValueFactory());
     }
 
@@ -50,7 +55,34 @@ public class EntryTest extends AbstractE
     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"));
+        // false since acl has been created from path only -> no id
+        assertFalse(entry.isLocal(((NodeImpl) testRootNode).getNodeId()));
+        // false since internal id is null -> will never match.
+        assertFalse(entry.isLocal(new NodeId()));
+    }
+
+    public void testIsLocal2()  throws NotExecutableException, RepositoryException {
+        String path = testRootNode.getPath();
+        AccessControlPolicy[] acls = acMgr.getPolicies(path);
+        if (acls.length == 0) {
+            AccessControlPolicyIterator it = acMgr.getApplicablePolicies(path);
+            if (!it.hasNext()) {
+                throw new NotExecutableException();
+            }
+            acMgr.setPolicy(path, it.nextAccessControlPolicy());
+            acls = acMgr.getPolicies(path);
+        }
+
+        assertTrue(acls[0] instanceof ACLTemplate);
+
+        ACLTemplate acl = (ACLTemplate) acls[0];
+        assertEquals(path, acl.getPath());       
+
+        ACLTemplate.Entry entry = acl.createEntry(testPrincipal, new Privilege[] {acMgr.privilegeFromName(Privilege.JCR_READ)}, true);
+
+        // node is must be present + must match to testrootnodes id.
+        assertTrue(entry.isLocal(((NodeImpl) testRootNode).getNodeId()));
+        // but not to a random id.
+        assertFalse(entry.isLocal(new NodeId()));
     }
 }
\ No newline at end of file

Modified: jackrabbit/branches/2.1/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/TestAll.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.1/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/TestAll.java?rev=989597&r1=989596&r2=989597&view=diff
==============================================================================
--- jackrabbit/branches/2.1/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/TestAll.java (original)
+++ jackrabbit/branches/2.1/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/TestAll.java Thu Aug 26 10:48:44 2010
@@ -39,6 +39,7 @@ public class TestAll extends TestCase {
 
         suite.addTestSuite(ACLTemplateTest.class);
         suite.addTestSuite(EntryTest.class);
+        suite.addTestSuite(EntryCollectorTest.class);
 
         suite.addTestSuite(ReadTest.class);
         suite.addTestSuite(WriteTest.class);

Modified: jackrabbit/branches/2.1/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/combined/WriteTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.1/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/combined/WriteTest.java?rev=989597&r1=989596&r2=989597&view=diff
==============================================================================
--- jackrabbit/branches/2.1/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/combined/WriteTest.java (original)
+++ jackrabbit/branches/2.1/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/combined/WriteTest.java Thu Aug 26 10:48:44 2010
@@ -20,6 +20,8 @@ import org.apache.jackrabbit.api.securit
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlManager;
 import org.apache.jackrabbit.api.security.user.Group;
 import org.apache.jackrabbit.core.SessionImpl;
+import org.apache.jackrabbit.core.NodeImpl;
+import org.apache.jackrabbit.core.id.NodeId;
 import org.apache.jackrabbit.core.security.authorization.PrivilegeRegistry;
 import org.apache.jackrabbit.test.NotExecutableException;
 import org.slf4j.Logger;
@@ -56,6 +58,21 @@ public class WriteTest extends org.apach
         }
     }
 
+    protected boolean isExecutable() {
+        try {
+            AccessControlPolicy[] rootPolicies = acMgr.getPolicies("/");
+            if (rootPolicies.length > 0) {
+                return true;
+            }
+            if (acMgr.getApplicablePolicies("/").hasNext()) {
+                return true;
+            }
+        } catch (RepositoryException e) {
+            // ignore
+        }
+        return false;
+    }
+
     private JackrabbitAccessControlList getPrincipalBasedPolicy(AccessControlManager acM, String path, Principal principal) throws RepositoryException, AccessDeniedException, NotExecutableException {
         if (acM instanceof JackrabbitAccessControlManager) {
             AccessControlPolicy[] tmpls = ((JackrabbitAccessControlManager) acM).getApplicablePolicies(principal);
@@ -174,4 +191,57 @@ public class WriteTest extends org.apach
         assertFalse(testSession.hasPermission(childNPath+"/anyproperty", javax.jcr.Session.ACTION_SET_PROPERTY));
         assertFalse(testAcMgr.hasPrivileges(childNPath, modPropPrivs));
     }
+
+    public void testCanReadOnCombinedPolicies() throws RepositoryException, NotExecutableException {
+        Group testGroup = getTestGroup();
+        Session testSession = getTestSession();
+        /*
+          precondition:
+          testuser must have READ-only permission on test-node and below
+        */
+        checkReadOnly(path);
+
+        Privilege[] readPrivs = privilegesFromName(Privilege.JCR_READ);
+        // nodebased: remove READ privilege for 'testUser' at 'path'
+        withdrawPrivileges(path, readPrivs, getRestrictions(superuser, path));
+        // principalbased: add READ privilege for 'testGroup'
+        givePrivileges(path, testGroup.getPrincipal(), readPrivs, getPrincipalBasedRestrictions(path), false);
+        /*
+         expected result:
+         - nodebased wins over principalbased -> READ is denied
+         */
+        NodeId nodeId = ((NodeImpl) superuser.getNode(path)).getNodeId();
+        assertFalse(((SessionImpl) testSession).getAccessManager().canRead(null, nodeId));
+
+        /* allow again on child N -> should be allowed */
+        givePrivileges(childNPath, testGroup.getPrincipal(), readPrivs, getPrincipalBasedRestrictions(path), false);
+
+        NodeId childId = ((NodeImpl) superuser.getNode(childNPath)).getNodeId();
+        assertTrue(((SessionImpl) testSession).getAccessManager().canRead(null, childId));
+
+        // remove the nodebased policy
+        JackrabbitAccessControlList policy = getPolicy(acMgr, path, testUser.getPrincipal());
+        acMgr.removePolicy(policy.getPath(), policy);
+        superuser.save();
+
+        /*
+         expected result:
+         - READ privilege is present again.
+         */
+        assertTrue(((SessionImpl) testSession).getAccessManager().canRead(null, nodeId));
+
+        // nodebased: remove READ privilege for 'testUser' at 'path'
+        givePrivileges(path, readPrivs, getRestrictions(superuser, path));
+        // principalbased: add READ privilege for 'testGroup'
+        withdrawPrivileges(path, testGroup.getPrincipal(), readPrivs, getPrincipalBasedRestrictions(path), false);
+        /*
+         expected result:
+         - nodebased wins over principalbased -> READ is allowed
+         */
+        assertTrue(((SessionImpl) testSession).getAccessManager().canRead(null, nodeId));
+
+        /* allow again on child N -> should be allowed */
+        withdrawPrivileges(childNPath, testGroup.getPrincipal(), readPrivs, getPrincipalBasedRestrictions(path), false);
+        assertFalse(((SessionImpl) testSession).getAccessManager().canRead(null, childId));
+    }
 }
\ No newline at end of file