jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ju...@apache.org
Subject svn commit: r884108 [4/10] - in /jackrabbit/sandbox/JCR-1456: ./ jackrabbit-api/ jackrabbit-api/src/main/java/org/apache/jackrabbit/api/security/ jackrabbit-api/src/main/java/org/apache/jackrabbit/api/security/user/ jackrabbit-core/ jackrabbit-core/src...
Date Wed, 25 Nov 2009 14:04:50 GMT
Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLTemplate.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLTemplate.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLTemplate.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLTemplate.java Wed Nov 25 14:04:38 2009
@@ -167,7 +167,14 @@
             throw new AccessControlException("Missing mandatory restriction: " + jcrNodePathName);
         }
     }
-    
+
+    /**
+     * @see org.apache.jackrabbit.core.security.authorization.AbstractACLTemplate#getEntries() 
+     */
+    protected List<? extends AccessControlEntry> getEntries() {
+        return entries;
+    }
+
     //----------------------------------------< JackrabbitAccessControlList >---
     /**
      * @see JackrabbitAccessControlList#getRestrictionNames()

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProvider.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProvider.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProvider.java Wed Nov 25 14:04:38 2009
@@ -16,18 +16,6 @@
  */
 package org.apache.jackrabbit.core.security.principal;
 
-import java.security.Principal;
-import java.util.Iterator;
-import java.util.LinkedHashSet;
-import java.util.Map;
-import java.util.Set;
-
-import javax.jcr.RepositoryException;
-import javax.jcr.Session;
-import javax.jcr.observation.Event;
-import javax.jcr.observation.EventIterator;
-import javax.jcr.observation.EventListener;
-
 import org.apache.commons.collections.iterators.IteratorChain;
 import org.apache.commons.collections.map.LRUMap;
 import org.apache.jackrabbit.api.security.principal.PrincipalIterator;
@@ -36,12 +24,25 @@
 import org.apache.jackrabbit.api.security.user.Group;
 import org.apache.jackrabbit.api.security.user.UserManager;
 import org.apache.jackrabbit.core.SessionImpl;
+import org.apache.jackrabbit.core.security.SystemPrincipal;
 import org.apache.jackrabbit.core.security.user.UserManagerImpl;
 import org.apache.jackrabbit.spi.commons.conversion.NameResolver;
 import org.apache.jackrabbit.util.Text;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import javax.jcr.observation.Event;
+import javax.jcr.observation.EventIterator;
+import javax.jcr.observation.EventListener;
+import javax.security.auth.Subject;
+import java.security.Principal;
+import java.util.Iterator;
+import java.util.LinkedHashSet;
+import java.util.Map;
+import java.util.Set;
+
 /**
  * Provides principals for the users contained within the Repository.<p/>
  * Each {@link Authorizable} accessible via {@link UserManager}
@@ -71,7 +72,7 @@
 
     private final EveryonePrincipal everyonePrincipal;
 
-    private final String pGroupName;
+    private final String pMembers;
     private final String pPrincipalName;
 
     /**
@@ -93,25 +94,19 @@
         String[] ntNames = new String[1];
         if (securitySession instanceof SessionImpl) {
             NameResolver resolver = (SessionImpl) securitySession;
-            ntNames[0] = resolver.getJCRName(UserManagerImpl.NT_REP_USER);
-            pGroupName = resolver.getJCRName(UserManagerImpl.P_GROUPS);
+            ntNames[0] = resolver.getJCRName(UserManagerImpl.NT_REP_GROUP);
+            pMembers = resolver.getJCRName(UserManagerImpl.P_MEMBERS);
             pPrincipalName = resolver.getJCRName(UserManagerImpl.P_PRINCIPAL_NAME);
         } else {
-            ntNames[0] = "rep:User";
-            pGroupName = "rep:groups";
+            ntNames[0] = "rep:Group";
+            pMembers = "rep:members";
             pPrincipalName = "rep:principalName";
         }
 
-        // find common ancestor of all user and group nodes.
-        String userPath = userManager.getUsersPath();
         String groupPath = userManager.getGroupsPath();
-        String obsPath = userPath;
-        while (!Text.isDescendant(obsPath, groupPath)) {
-            obsPath = Text.getRelativeParent(obsPath, 1);
-        }       
         securitySession.getWorkspace().getObservationManager().addEventListener(this,
                 Event.NODE_REMOVED | Event.PROPERTY_ADDED | Event.PROPERTY_CHANGED | Event.PROPERTY_REMOVED,
-                obsPath,
+                groupPath,
                 true,
                 null,
                 ntNames,
@@ -125,6 +120,7 @@
      * This implementation uses the user and node resolver to find the
      * appropriate nodes.
      */
+    @Override
     protected Principal providePrincipal(String principalName) {
         // check for 'everyone'
         if (everyonePrincipal.getName().equals(principalName)) {
@@ -211,24 +207,19 @@
     /**
      * @see PrincipalProvider#close()
      */
+    @Override
     public synchronized void close() {
         super.close();
         membershipCache.clear();
     }
 
     /**
-     * Always returns true.
-     *
      * @see PrincipalProvider#canReadPrincipal(javax.jcr.Session,java.security.Principal)
      */
     public boolean canReadPrincipal(Session session, Principal principal) {
         checkInitialized();
-        // by default (UserAccessControlProvider) READ-privilege is granted to
-        // everybody -> omit any (expensive) checks.
-        return true;
-        /*
-        // TODO: uncomment code if it turns out that the previous assumption is problematic.
-        // check if the session is granted read to the node.
+        // check if the session can read the user/group associated with the
+        // given principal
         if (session instanceof SessionImpl) {
             SessionImpl sImpl = (SessionImpl) session;
             Subject subject = sImpl.getSubject();
@@ -237,14 +228,13 @@
                 return true;
             }
             try {
-                UserManager umgr = ((SessionImpl)session).getUserManager();
+                UserManager umgr = sImpl.getUserManager();
                 return umgr.getAuthorizable(principal) != null;
             } catch (RepositoryException e) {
                 // ignore and return false
             }
         }
         return false;
-        */
     }
 
     //------------------------------------------------------< EventListener >---
@@ -262,7 +252,7 @@
             if (type == Event.PROPERTY_ADDED || type == Event.PROPERTY_CHANGED
                     || type == Event.PROPERTY_REMOVED) {
                 try {
-                    if (pGroupName.equals(Text.getName(ev.getPath()))) {
+                    if (pMembers.equals(Text.getName(ev.getPath()))) {
                         synchronized (membershipCache) {
                             membershipCache.clear();
                         }
@@ -286,9 +276,9 @@
      * including inherited membership.
      */
     private Set<Principal> collectGroupMembership(Principal princ) {
-        Set<Principal> membership = new LinkedHashSet<Principal>();
+        final Set<Principal> membership = new LinkedHashSet<Principal>();
             try {
-                Authorizable auth = userManager.getAuthorizable(princ);
+                final Authorizable auth = userManager.getAuthorizable(princ);
                 if (auth != null) {
                     addToCache(princ);
                     Iterator<Group> itr = auth.memberOf();
@@ -315,7 +305,7 @@
     private PrincipalIterator findUserPrincipals(String simpleFilter) {
         synchronized (userManager) {
             try {
-                Iterator itr = userManager.findAuthorizables(pPrincipalName, simpleFilter, UserManager.SEARCH_TYPE_USER);
+                Iterator<Authorizable> itr = userManager.findAuthorizables(pPrincipalName, simpleFilter, UserManager.SEARCH_TYPE_USER);
                 return new PrincipalIteratorImpl(itr, false);
             } catch (RepositoryException e) {
                 log.error("Error while searching user principals.", e);
@@ -332,7 +322,7 @@
     private PrincipalIterator findGroupPrincipals(final String simpleFilter) {
         synchronized (userManager) {
             try {
-                Iterator itr = userManager.findAuthorizables(pPrincipalName, simpleFilter, UserManager.SEARCH_TYPE_GROUP);
+                Iterator<Authorizable> itr = userManager.findAuthorizables(pPrincipalName, simpleFilter, UserManager.SEARCH_TYPE_GROUP);
 
                 // everyone will not be found by the user manager -> extra test
                 boolean addEveryone = everyonePrincipal.getName().matches(".*"+simpleFilter+".*");
@@ -353,10 +343,10 @@
      */
     private class PrincipalIteratorImpl extends AbstractPrincipalIterator {
 
-        private final Iterator authorizableItr;
+        private final Iterator<Authorizable> authorizableItr;
         private boolean addEveryone;
 
-        private PrincipalIteratorImpl(Iterator authorizableItr, boolean addEveryone) {
+        private PrincipalIteratorImpl(Iterator<Authorizable> authorizableItr, boolean addEveryone) {
             this.authorizableItr = authorizableItr;
             this.addEveryone = addEveryone;
 
@@ -366,10 +356,11 @@
         /**
          * @see org.apache.jackrabbit.core.security.principal.AbstractPrincipalIterator#seekNext()
          */
+        @Override
         protected Principal seekNext() {
             while (authorizableItr.hasNext()) {
                 try {
-                    Principal p = ((Authorizable) authorizableItr.next()).getPrincipal();
+                    Principal p = authorizableItr.next().getPrincipal();
                     addToCache(p);
                     return p;
                 } catch (RepositoryException e) {

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalIteratorAdapter.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalIteratorAdapter.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalIteratorAdapter.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalIteratorAdapter.java Wed Nov 25 14:04:38 2009
@@ -42,7 +42,7 @@
      *
      * @param iterator iterator of {@link Principal}s
      */
-    public PrincipalIteratorAdapter(Iterator iterator) {
+    public PrincipalIteratorAdapter(Iterator<? extends Principal> iterator) {
         super(new RangeIteratorAdapter(iterator));
     }
 
@@ -51,7 +51,7 @@
      *
      * @param collection collection of {@link Principal} objects.
      */
-    public PrincipalIteratorAdapter(Collection collection) {
+    public PrincipalIteratorAdapter(Collection<? extends Principal> collection) {
         super(new RangeIteratorAdapter(collection));
     }
 

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalManagerImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalManagerImpl.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalManagerImpl.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalManagerImpl.java Wed Nov 25 14:04:38 2009
@@ -133,7 +133,7 @@
         }
         // additional entry for the 'everyone' group
         if (!(principal instanceof EveryonePrincipal)) {
-            Iterator it = Collections.singletonList(getEveryone()).iterator();
+            Iterator<Principal> it = Collections.singletonList(getEveryone()).iterator();
             entries.add(new CheckedIteratorEntry(it, null));
         }
         return new CheckedPrincipalIterator(entries);
@@ -230,7 +230,7 @@
         }
 
         public Enumeration<? extends Principal> members() {
-            Iterator it = Collections.list(delegatee.members()).iterator();
+            Iterator<? extends Principal> it = Collections.list(delegatee.members()).iterator();
             final PrincipalIterator members = new CheckedPrincipalIterator(it, provider);
             return new Enumeration<Principal>() {
                 public boolean hasMoreElements() {
@@ -247,10 +247,12 @@
         }
 
         //---------------------------------------------------------< Object >---
+        @Override
         public int hashCode() {
             return delegatee.hashCode();
         }
 
+        @Override
         public boolean equals(Object obj) {
             return delegatee.equals(obj);
         }
@@ -284,7 +286,7 @@
 
         private final List<CheckedIteratorEntry> entries;
 
-        private CheckedPrincipalIterator(Iterator it, PrincipalProvider provider) {
+        private CheckedPrincipalIterator(Iterator<? extends Principal> it, PrincipalProvider provider) {
             entries = new ArrayList<CheckedIteratorEntry>(1);
             entries.add(new CheckedIteratorEntry(it, provider));
             next = seekNext();
@@ -298,13 +300,14 @@
         /**
          * @see org.apache.jackrabbit.core.security.principal.AbstractPrincipalIterator#seekNext()
          */
+        @Override
         protected final Principal seekNext() {
             while (!entries.isEmpty()) {
                 // first test if current iterator has more elements
                 CheckedIteratorEntry current = entries.get(0);
-                Iterator iterator = current.iterator;
+                Iterator<? extends Principal> iterator = current.iterator;
                 while (iterator.hasNext()) {
-                    Principal chk = (Principal) iterator.next();
+                    Principal chk = iterator.next();
                     if (current.provider == null ||
                         current.provider.canReadPrincipal(session, chk)) {
                         return disguise(chk, current.provider);
@@ -324,9 +327,9 @@
     private static class CheckedIteratorEntry {
 
         private final PrincipalProvider provider;
-        private final Iterator iterator;
+        private final Iterator<? extends Principal> iterator;
 
-        private CheckedIteratorEntry(Iterator iterator, PrincipalProvider provider) {
+        private CheckedIteratorEntry(Iterator<? extends Principal> iterator, PrincipalProvider provider) {
             this.iterator = iterator;
             this.provider = provider;
         }

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalProvider.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalProvider.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalProvider.java Wed Nov 25 14:04:38 2009
@@ -110,7 +110,6 @@
     void close();
 
     /**
-     * // TODO: review again.
      * Tests if the provided session is allowed to read the given principal.
      * Since the principal providers do not restrict the access
      * on the prinicipals they provide, this method is used by the PrincipalManger
@@ -118,7 +117,7 @@
      *
      * @param session
      * @param principalToRead The principal to be accessed by the specified subject.
-     * @return <code>true</code> if the subject is allowed to read the principal;
+     * @return <code>true</code> if the session is allowed to read the principal;
      * <code>false</code> otherwise.
      */
     boolean canReadPrincipal(Session session, Principal principalToRead);

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/ProviderRegistryImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/ProviderRegistryImpl.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/ProviderRegistryImpl.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/ProviderRegistryImpl.java Wed Nov 25 14:04:38 2009
@@ -49,10 +49,11 @@
      */
     public ProviderRegistryImpl(PrincipalProvider defaultPrincipalProvider) {
         this.defaultPrincipalProvider = defaultPrincipalProvider;
-        providers.put(defaultPrincipalProvider.getClass().getName(), defaultPrincipalProvider);
+        if (defaultPrincipalProvider != null) {
+            providers.put(defaultPrincipalProvider.getClass().getName(), defaultPrincipalProvider);
+        }
     }
 
-
     //------------------------------------------< PrincipalProviderRegistry >---
     /**
      * @see PrincipalProviderRegistry#registerProvider(Properties)

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleAccessManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleAccessManager.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleAccessManager.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleAccessManager.java Wed Nov 25 14:04:38 2009
@@ -148,7 +148,7 @@
             // system has always all permissions
             return true;
         } else if (anonymous) {
-            // anonymous is always denied WRITE & REMOVE premissions
+            // anonymous is always denied WRITE & REMOVE permissions
             if ((permissions & WRITE) == WRITE
                     || (permissions & REMOVE) == REMOVE) {
                 return false;
@@ -180,7 +180,7 @@
             // system has always all permissions
             return true;
         } else if (anonymous) {
-            // anonymous is only granted READ premissions
+            // anonymous is only granted READ permissions
             return permissions == Permission.READ;
         }
 
@@ -216,7 +216,7 @@
                 return true;
             } else if (anonymous) {
                 if (privileges.length != 1 || !privileges[0].equals(privilegeRegistry.getPrivilege(Privilege.JCR_READ))) {
-                    // anonymous is only granted READ premissions
+                    // anonymous is only granted READ permissions
                     return false;
                 }
             }

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleSecurityManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleSecurityManager.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleSecurityManager.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleSecurityManager.java Wed Nov 25 14:04:38 2009
@@ -176,7 +176,8 @@
 
         SecurityManagerConfig smc = config.getSecurityManagerConfig();
         if (smc != null && smc.getWorkspaceAccessConfig() != null) {
-            workspaceAccessManager = (WorkspaceAccessManager) smc.getWorkspaceAccessConfig().newInstance();
+            workspaceAccessManager =
+                smc.getWorkspaceAccessConfig().newInstance(WorkspaceAccessManager.class);
         } else {
             // fallback -> the default simple implementation
             log.debug("No WorkspaceAccessManager configured; using default.");
@@ -216,7 +217,7 @@
             if (amc == null) {
                 accessMgr = new SimpleAccessManager();
             } else {
-                accessMgr = (AccessManager) amc.newInstance();
+                accessMgr = amc.newInstance(AccessManager.class);
             }
             accessMgr.init(amContext, acP, workspaceAccessManager);
             return accessMgr;
@@ -254,10 +255,11 @@
     }
 
     /**
-     * @see JackrabbitSecurityManager#getUserID(Subject)
+     * @see JackrabbitSecurityManager#getUserID(javax.security.auth.Subject, String)
      */
-    public String getUserID(Subject subject) throws RepositoryException {
+    public String getUserID(Subject subject, String workspaceName) throws RepositoryException {
         String uid = null;
+
         // if SimpleCredentials are present, the UserID can easily be retrieved.
         Iterator<SimpleCredentials> creds = subject.getPublicCredentials(SimpleCredentials.class).iterator();
         if (creds.hasNext()) {
@@ -288,7 +290,7 @@
      * @return an {@link AuthContext} for the given Credentials, Subject
      * @throws RepositoryException in other exceptional repository states
      */
-    public AuthContext getAuthContext(Credentials creds, Subject subject)
+    public AuthContext getAuthContext(Credentials creds, Subject subject, String workspaceName)
             throws RepositoryException {
         checkInitialized();
         return authCtxProvider.getAuthContext(creds, subject, systemSession, principalProviderRegistry, adminID, anonymID);

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleWorkspaceAccessManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleWorkspaceAccessManager.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleWorkspaceAccessManager.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/simple/SimpleWorkspaceAccessManager.java Wed Nov 25 14:04:38 2009
@@ -32,7 +32,7 @@
     /**
      * @see WorkspaceAccessManager#init(Session)
      */
-    public void init(Session securitySession) {
+    public void init(Session systemSession) {
         // nothing to do
     }
 

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java Wed Nov 25 14:04:38 2009
@@ -21,8 +21,8 @@
 import org.apache.jackrabbit.api.security.user.Group;
 import org.apache.jackrabbit.api.security.user.User;
 import org.apache.jackrabbit.core.NodeImpl;
-import org.apache.jackrabbit.core.PropertyImpl;
 import org.apache.jackrabbit.core.SessionImpl;
+import org.apache.jackrabbit.core.PropertyImpl;
 import org.apache.jackrabbit.core.nodetype.NodeTypeImpl;
 import org.apache.jackrabbit.core.security.principal.PrincipalImpl;
 import org.apache.jackrabbit.spi.Name;
@@ -30,18 +30,23 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.jcr.ItemNotFoundException;
 import javax.jcr.Property;
 import javax.jcr.PropertyIterator;
-import javax.jcr.PropertyType;
 import javax.jcr.RepositoryException;
 import javax.jcr.Value;
+import javax.jcr.Session;
+import javax.jcr.ItemNotFoundException;
+import javax.jcr.AccessDeniedException;
+import javax.jcr.ItemVisitor;
+import javax.jcr.Node;
+import javax.jcr.util.TraversingItemVisitor;
 import javax.jcr.nodetype.ConstraintViolationException;
 import javax.jcr.nodetype.PropertyDefinition;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Set;
+import java.util.HashSet;
 
 /**
  * AuthorizableImpl
@@ -52,6 +57,7 @@
 
     final UserManagerImpl userManager;
     private final NodeImpl node;
+    private int hashCode;
 
     /**
      * @param node The node this Authorizable is persisted to.
@@ -62,9 +68,6 @@
      */
     protected AuthorizableImpl(NodeImpl node, UserManagerImpl userManager)
             throws RepositoryException {
-        if (!node.isNodeType(NT_REP_AUTHORIZABLE)) {
-            throw new IllegalArgumentException("Node argument of NodeType " + NT_REP_AUTHORIZABLE + " required");
-        }
         this.node = node;
         this.userManager = userManager;
     }
@@ -84,7 +87,7 @@
      * @see Authorizable#declaredMemberOf()
      */
     public Iterator<Group> declaredMemberOf() throws RepositoryException {
-        List<Group> memberShip = new ArrayList<Group>();
+        Set<Group> memberShip = new HashSet<Group>();
         collectMembership(memberShip, false);
         return memberShip.iterator();
     }
@@ -93,7 +96,7 @@
      * @see Authorizable#memberOf()
      */
     public Iterator<Group> memberOf() throws RepositoryException {
-        List<Group> memberShip = new ArrayList<Group>();
+        Set<Group> memberShip = new HashSet<Group>();
         collectMembership(memberShip, true);
         return memberShip.iterator();
     }
@@ -151,12 +154,20 @@
     public synchronized void setProperty(String name, Value value) throws RepositoryException {
         checkProtectedProperty(name);
         try {
+            // check if the property has already been created as multi valued
+            // property before -> in this case remove in order to avoid valueformatex.
+            if (node.hasProperty(name)) {
+                Property p = node.getProperty(name);
+                if (p.isMultiple()) {
+                    p.remove();
+                }
+            }
             node.setProperty(name, value);
-            if (!userManager.batchModus) {
+            if (userManager.isAutoSave()) {
                 node.save();
             }
         } catch (RepositoryException e) {
-            log.warn("Failed to set Property " + name + " for Authorizable " + getID());
+            log.warn("Failed to set Property " + name + " for " + this, e);
             node.refresh(false);
             throw e;
         }
@@ -176,12 +187,20 @@
     public synchronized void setProperty(String name, Value[] values) throws RepositoryException {
         checkProtectedProperty(name);
         try {
+            // check if the property has already been created as single valued
+            // property before -> in this case remove in order to avoid valueformatex.
+            if (node.hasProperty(name)) {
+                Property p = node.getProperty(name);
+                if (!p.isMultiple()) {
+                    p.remove();
+                }
+            }
             node.setProperty(name, values);
-            if (!userManager.batchModus) {
+            if (userManager.isAutoSave()) {
                 node.save();
             }
         } catch (RepositoryException e) {
-            log.warn("Failed to set Property " + name + " for Authorizable " + getID());
+            log.warn("Failed to set Property " + name + " for " + this, e);
             node.refresh(false);
             throw e;
         }
@@ -200,7 +219,7 @@
                 } else {
                     p.setValue((Value) null);
                 }
-                if (!userManager.batchModus) {
+                if (userManager.isAutoSave()) {
                     node.save();
                 }
                 return true;
@@ -208,7 +227,7 @@
                 return false;
             }
         } catch (RepositoryException e) {
-            log.warn("Failed to remove Property " + name + " from Authorizable " + getID());
+            log.warn("Failed to remove Property " + name + " from " + this, e);
             node.refresh(false);
             throw e;
         }
@@ -223,7 +242,51 @@
         if (!isGroup() && ((User) this).isAdmin()) {
             throw new RepositoryException("The administrator cannot be removed.");
         }
-        userManager.removeProtectedItem(node, node.getParent());
+        Session s = getSession();
+        node.remove();
+        if (userManager.isAutoSave()) {
+            s.save();
+        }
+    }
+
+    //-------------------------------------------------------------< Object >---
+    @Override
+    public int hashCode() {
+        if (hashCode == 0) {
+            try {
+                StringBuilder sb = new StringBuilder();
+                sb.append(isGroup() ? "group:" : "user:");
+                sb.append(getSession().getWorkspace().getName());
+                sb.append(":");
+                sb.append(node.getIdentifier());
+                hashCode = sb.toString().hashCode();
+            } catch (RepositoryException e) {
+            }
+        }
+        return hashCode;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (obj instanceof AuthorizableImpl) {
+            AuthorizableImpl otherAuth = (AuthorizableImpl) obj;
+            try {
+                return isGroup() == otherAuth.isGroup() && node.isSame(otherAuth.node);
+            } catch (RepositoryException e) {
+                // should not occur -> return false in this case.
+            }
+        }
+        return false;
+    }
+
+    @Override
+    public String toString() {
+        try {
+            String typeStr = (isGroup()) ? "Group '" : "User '";
+            return new StringBuilder().append(typeStr).append(getID()).append("'").toString();
+        } catch (RepositoryException e) {
+            return super.toString();
+        }
     }
 
     //--------------------------------------------------------------------------
@@ -243,79 +306,69 @@
         return node.getProperty(P_PRINCIPAL_NAME).getString();
     }
 
-    boolean addToGroup(GroupImpl group) throws RepositoryException {
-        try {
-            Value[] values;
-            Value added = getSession().getValueFactory().createValue(group.getNode(), true);
-            NodeImpl node = getNode();
-            if (node.hasProperty(P_GROUPS)) {
-                Value[] old = node.getProperty(P_GROUPS).getValues();
-                values = new Value[old.length + 1];
-                System.arraycopy(old, 0, values, 0, old.length);
-            } else {
-                values = new Value[1];
-            }
-            values[values.length - 1] = added;
-            userManager.setProtectedProperty(node, P_GROUPS, values, PropertyType.WEAKREFERENCE);
-            return true;
-        } catch (RepositoryException e) {
-            // revert all pending changes and rethrow.
-            getSession().refresh(false);
-            throw e;
-        }
-    }
-
-    boolean removeFromGroup(GroupImpl group) throws RepositoryException {
-        NodeImpl node = getNode();
-        String message = "Authorizable " + getID() + " is not member of " + group.getID();
-        if (!node.hasProperty(P_GROUPS)) {
-            log.debug(message);
-            return false;
-        }
-
-        Value toRemove = getSession().getValueFactory().createValue(group.getNode(), true);
-        PropertyImpl property = node.getProperty(P_GROUPS);
-        List<Value> valList = new ArrayList<Value>(Arrays.asList(property.getValues()));
-        if (valList.remove(toRemove)) {
-            try {
-                if (valList.isEmpty()) {
-                    userManager.removeProtectedItem(property, node);
-                } else {
-                    Value[] values = valList.toArray(new Value[valList.size()]);
-                    userManager.setProtectedProperty(node, P_GROUPS, values, PropertyType.WEAKREFERENCE);
+    private void collectMembership(final Set<Group> groups, boolean includeIndirect) throws RepositoryException {
+        PropertyIterator refs = getMembershipReferences();
+        if (refs != null) {
+            while (refs.hasNext()) {
+                try {
+                    NodeImpl n = (NodeImpl) refs.nextProperty().getParent();
+                    if (n.isNodeType(NT_REP_GROUP)) {
+                        Group group = userManager.createGroup(n);
+                        // only retrieve indirect membership if the group is not
+                        // yet present (detected eventual circular membership).
+                        if (groups.add(group) && includeIndirect) {
+                            ((AuthorizableImpl) group).collectMembership(groups, true);
+                        }
+                    } else {
+                        // weak-ref property 'rep:members' that doesn't reside under an
+                        // group node -> doesn't represent a valid group member.
+                        log.debug("Invalid member reference to '" + this + "' -> Not included in membership set.");
+                    }
+                } catch (ItemNotFoundException e) {
+                    // group node doesn't exist  -> -> ignore exception
+                    // and skip this reference from membership list.
+                } catch (AccessDeniedException e) {
+                    // not allowed to see the group node -> ignore exception
+                    // and skip this reference from membership list.
                 }
-                return true;
-            } catch (RepositoryException e) {
-                // modification failed -> revert all pending changes.
-                node.refresh(false);
-                throw e;
             }
         } else {
-            // nothing changed
-            log.debug(message);
-            return false;
+            // workaround for failure of Node#getWeakReferences
+            // traverse the tree below groups-path and collect membership manually.
+            log.info("Traversing groups tree to collect membership.");
+            ItemVisitor visitor = new TraversingItemVisitor.Default() {
+                @Override
+                protected void entering(Property property, int level) throws RepositoryException {
+                    PropertyImpl pImpl = (PropertyImpl) property;
+                    NodeImpl n = (NodeImpl) pImpl.getParent();
+                    if (P_MEMBERS.equals(pImpl.getQName()) && n.isNodeType(NT_REP_GROUP)) {
+                        for (Value value : property.getValues()) {
+                            if (value.getString().equals(node.getIdentifier())) {
+                                Group gr = (Group) userManager.getAuthorizable(n);
+                                groups.add(gr);
+                            }
+                        }
+                    }
+                }
+            };
+            Node groupsNode = getSession().getNode(userManager.getGroupsPath());
+            visitor.visit(groupsNode);
         }
     }
 
-    private void collectMembership(List<Group> groups, boolean includedIndirect) throws RepositoryException {
-        NodeImpl node = getNode();
-        if (!node.hasProperty(P_GROUPS)) {
-            return;
-        }
-        Value[] refs = node.getProperty(P_GROUPS).getValues();
-        for (Value ref : refs) {
-            try {
-                NodeImpl groupNode = (NodeImpl) getSession().getNodeByUUID(ref.getString());
-                Group group = GroupImpl.create(groupNode, userManager);
-                if (groups.add(group) && includedIndirect) {
-                    ((AuthorizableImpl) group).collectMembership(groups, true);
-                }
-            } catch (ItemNotFoundException e) {
-                // groupNode doesn't exist any more
-                log.warn("Group node referenced by " + getID() + " doesn't exist anymore -> Ignored from membership list.");
-                // TODO: possibly clean up list of group memberships
-            }
+    /**
+     * @return the iterator returned by {@link Node#getWeakReferences(String)}
+     * or <code>null</code> if the method call fails with <code>RepositoryException</code>.
+     * See fallback scenario above.
+     */
+    private PropertyIterator getMembershipReferences() {
+        PropertyIterator refs = null;
+        try {
+            refs = node.getWeakReferences(getSession().getJCRName(P_MEMBERS));
+        } catch (RepositoryException e) {
+            log.error("Failed to retrieve membership references of " + this + ".", e);
         }
+        return refs;
     }
 
     /**
@@ -343,11 +396,8 @@
      * following that has a special meaning and must be altered using this
      * user API:
      * <ul>
-     * <ul>
      * <li>rep:principalName</li>
-     * <li>rep:userId</li>
-     * <li>rep:referees</li>
-     * <li>rep:groups</li>
+     * <li>rep:members</li>
      * <li>rep:impersonators</li>
      * <li>rep:password</li>
      * </ul>
@@ -363,7 +413,7 @@
     private boolean isProtectedProperty(String propertyName) throws RepositoryException {
         Name pName = getSession().getQName(propertyName);
         return P_PRINCIPAL_NAME.equals(pName)
-                || P_GROUPS.equals(pName)
+                || P_MEMBERS.equals(pName)
                 || P_IMPERSONATORS.equals(pName) || P_PASSWORD.equals(pName);
     }
 
@@ -378,7 +428,7 @@
      */
     private void checkProtectedProperty(String propertyName) throws ConstraintViolationException, RepositoryException {
         if (isProtectedProperty(propertyName)) {
-            throw new ConstraintViolationException("Attempt to modify protected property " + propertyName + " of an Authorizable.");
+            throw new ConstraintViolationException("Attempt to modify protected property " + propertyName + " of " + this);
         }
     }
 

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/GroupImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/GroupImpl.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/GroupImpl.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/GroupImpl.java Wed Nov 25 14:04:38 2009
@@ -19,14 +19,17 @@
 import org.apache.jackrabbit.api.security.user.Authorizable;
 import org.apache.jackrabbit.api.security.user.Group;
 import org.apache.jackrabbit.api.security.user.User;
+import org.apache.jackrabbit.api.security.user.UserManager;
 import org.apache.jackrabbit.core.NodeImpl;
-import org.apache.jackrabbit.util.Text;
+import org.apache.jackrabbit.core.PropertyImpl;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import javax.jcr.Node;
-import javax.jcr.PropertyIterator;
 import javax.jcr.RepositoryException;
+import javax.jcr.Value;
+import javax.jcr.ItemNotFoundException;
+import javax.jcr.PropertyType;
 import java.io.IOException;
 import java.io.ObjectOutputStream;
 import java.security.Principal;
@@ -36,6 +39,9 @@
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Set;
+import java.util.List;
+import java.util.ArrayList;
+import java.util.Arrays;
 
 /**
  * GroupImpl...
@@ -46,21 +52,10 @@
 
     private Principal principal;
 
-    private GroupImpl(NodeImpl node, UserManagerImpl userManager) throws RepositoryException {
+    protected GroupImpl(NodeImpl node, UserManagerImpl userManager) throws RepositoryException {
         super(node, userManager);
     }
 
-    static Group create(NodeImpl node, UserManagerImpl userManager) throws RepositoryException {
-        if (node == null || !node.isNodeType(NT_REP_GROUP)) {
-            throw new IllegalArgumentException();
-        }
-        if (!Text.isDescendant(userManager.getGroupsPath(), node.getPath())) {
-            throw new IllegalArgumentException("Group has to be within the Group Path");
-        }
-        return new GroupImpl(node, userManager);
-    }
-
-
     //-------------------------------------------------------< Authorizable >---
     /**
      * @see Authorizable#isGroup()
@@ -84,27 +79,28 @@
      * @see Group#getDeclaredMembers()
      */
     public Iterator<Authorizable> getDeclaredMembers() throws RepositoryException {
-        return getMembers(false).iterator();
+        return getMembers(false, UserManager.SEARCH_TYPE_AUTHORIZABLE).iterator();
     }
 
     /**
      * @see Group#getMembers()
      */
     public Iterator<Authorizable> getMembers() throws RepositoryException {
-        return getMembers(true).iterator();
+        return getMembers(true, UserManager.SEARCH_TYPE_AUTHORIZABLE).iterator();
     }
 
     /**
      * @see Group#isMember(Authorizable)
      */
     public boolean isMember(Authorizable authorizable) throws RepositoryException {
-        if (authorizable == null || !(authorizable instanceof AuthorizableImpl)) {
+        if (authorizable == null || !(authorizable instanceof AuthorizableImpl)
+                || getNode().isSame(((AuthorizableImpl) authorizable).getNode())) {
             return false;
         } else {
             String thisID = getID();
             AuthorizableImpl impl = (AuthorizableImpl) authorizable;
-            for (Iterator it = impl.memberOf(); it.hasNext();) {
-                if (thisID.equals(((GroupImpl) it.next()).getID())) {
+            for (Iterator<Group> it = impl.memberOf(); it.hasNext();) {
+                if (thisID.equals(it.next().getID())) {
                     return true;
                 }
             }
@@ -116,12 +112,7 @@
      * @see Group#addMember(Authorizable)
      */
     public boolean addMember(Authorizable authorizable) throws RepositoryException {
-        if (authorizable == null || !(authorizable instanceof AuthorizableImpl)
-                || isMember(authorizable)) {
-            return false;
-        }
-        if (isCyclicMembership(authorizable)) {
-            log.warn("Attempt to create circular group membership.");
+        if (authorizable == null || !(authorizable instanceof AuthorizableImpl)) {
             return false;
         }
 
@@ -133,18 +124,71 @@
             return false;
         }
 
-        // preconditions are met -> delegate to authorizableImpl
-        return authImpl.addToGroup(this);
+        if (isCyclicMembership(authImpl)) {
+            log.warn("Attempt to create circular group membership.");
+            return false;
+        }
+
+        Value[] values;
+        Value toAdd = getSession().getValueFactory().createValue(memberNode, true);
+        NodeImpl node = getNode();
+        if (node.hasProperty(P_MEMBERS)) {
+            Value[] old = node.getProperty(P_MEMBERS).getValues();
+            for (Value v : old) {
+                if (v.equals(toAdd)) {
+                    log.debug("Authorizable " + authImpl + " is already member of " + this);
+                    return false;
+                }
+            }
+
+            values = new Value[old.length + 1];
+            System.arraycopy(old, 0, values, 0, old.length);
+        } else {
+            values = new Value[1];
+        }
+        values[values.length - 1] = toAdd;
+
+        userManager.setProtectedProperty(node, P_MEMBERS, values, PropertyType.WEAKREFERENCE);
+        return true;
     }
 
     /**
      * @see Group#removeMember(Authorizable)
      */
     public boolean removeMember(Authorizable authorizable) throws RepositoryException {
-        if (!isMember(authorizable) || !(authorizable instanceof AuthorizableImpl)) {
+        if (!(authorizable instanceof AuthorizableImpl)) {
+            return false;
+        }
+        NodeImpl node = getNode();
+        if (!node.hasProperty(P_MEMBERS)) {
+            log.debug("Group has no members -> cannot remove member " + authorizable.getID());
+            return false;
+        }
+
+        Value toRemove = getSession().getValueFactory().createValue(((AuthorizableImpl)authorizable).getNode(), true);
+
+        PropertyImpl property = node.getProperty(P_MEMBERS);
+        List<Value> valList = new ArrayList<Value>(Arrays.asList(property.getValues()));
+
+        if (valList.remove(toRemove)) {
+            try {
+                if (valList.isEmpty()) {
+                    userManager.removeProtectedItem(property, node);
+                } else {
+                    Value[] values = valList.toArray(new Value[valList.size()]);
+                    userManager.setProtectedProperty(node, P_MEMBERS, values);
+                }
+                return true;
+            } catch (RepositoryException e) {
+                // modification failed -> revert all pending changes.
+                node.refresh(false);
+                throw e;
+            }
+        } else {
+            // nothing changed
+            log.debug("Authorizable " + authorizable.getID() + " was not member of " + getID());
             return false;
         }
-        return ((AuthorizableImpl) authorizable).removeFromGroup(this);
     }
 
     //--------------------------------------------------------------------------
@@ -152,50 +196,68 @@
      *
      * @param includeIndirect If <code>true</code> all members of this group
      * will be return; otherwise only the declared members.
+     * @param type Any of {@link UserManager#SEARCH_TYPE_AUTHORIZABLE},
+     * {@link UserManager#SEARCH_TYPE_GROUP}, {@link UserManager#SEARCH_TYPE_USER}.
      * @return A collection of members of this group.
      * @throws RepositoryException If an error occurs while collecting the members.
      */
-    private Collection<Authorizable> getMembers(boolean includeIndirect) throws RepositoryException {
-        PropertyIterator itr = getNode().getWeakReferences(getSession().getJCRName(P_GROUPS));
-        Collection<Authorizable> members = new HashSet<Authorizable>((int) itr.getSize());
-        while (itr.hasNext()) {
-            NodeImpl n = (NodeImpl) itr.nextProperty().getParent();
-            if (n.isNodeType(NT_REP_GROUP)) {
-                Group group = userManager.createGroup(n);
-                // only retrieve indirect group-members if the group is not
-                // yet present (detected eventual circular membership).
-                if (members.add(group) && includeIndirect) {
-                    members.addAll(((GroupImpl) group).getMembers(true));
+    private Collection<Authorizable> getMembers(boolean includeIndirect, int type) throws RepositoryException {
+        Collection<Authorizable> members = new HashSet<Authorizable>();
+        if (getNode().hasProperty(P_MEMBERS)) {
+            Value[] vs = getNode().getProperty(P_MEMBERS).getValues();
+            for (Value v : vs) {
+                try {
+                    NodeImpl n = (NodeImpl) getSession().getNodeByIdentifier(v.getString());
+                    if (n.isNodeType(NT_REP_GROUP)) {
+                        if (type != UserManager.SEARCH_TYPE_USER) {
+                            Group group = userManager.createGroup(n);
+                            // only retrieve indirect group-members if the group is not
+                            // yet present (detected eventual circular membership).
+                            if (members.add(group) && includeIndirect) {
+                                members.addAll(((GroupImpl) group).getMembers(true, type));
+                            }
+                        } // else: groups are ignored
+                    } else if (n.isNodeType(NT_REP_USER)) {
+                        if (type != UserManager.SEARCH_TYPE_GROUP) {
+                            User user = userManager.createUser(n);
+                            members.add(user);
+                        }
+                    } else {
+                        // reference does point to an authorizable node -> not a
+                        // member of this group -> ignore
+                        log.debug("Group member entry with invalid node type " + n.getPrimaryNodeType().getName() + " -> Not included in member set.");
+                    }
+                } catch (ItemNotFoundException e) {
+                    // dangling weak reference -> clean upon next write.
+                    log.debug("Authorizable node referenced by " + getID() + " doesn't exist any more -> Ignored from member list.");
                 }
-            } else if (n.isNodeType(NT_REP_USER)) {
-                User user = userManager.createUser(n);
-                members.add(user);
-            } else {
-                // weak-ref property 'rep:groups' that doesn't reside under an
-                // authorizable node -> doesn't represent a member of this group.
-                log.debug("Undefined reference to group '" + getID() + "' -> Not included in member set.");
             }
-        }
+        } // no rep:member property
         return members;
     }
 
     /**
-     * Since {@link #isMember(Authorizable)} detects declared and inherited
-     * membership this method simply checks if the potential new member is
-     * a group that would in turn have <code>this</code> as a member.
+     * Returns <code>true</code> if the given <code>newMember</code> is a Group
+     * and contains <code>this</code> Group as declared or inherited member.
      *
      * @param newMember The new member to be tested for cyclic membership.
      * @return true if the 'newMember' is a group and 'this' is an declared or
      * inherited member of it.
      * @throws javax.jcr.RepositoryException If an error occurs.
      */
-    private boolean isCyclicMembership(Authorizable newMember) throws RepositoryException {
-        boolean cyclic = false;
+    private boolean isCyclicMembership(AuthorizableImpl newMember) throws RepositoryException {
         if (newMember.isGroup()) {
-            Group gr = (Group) newMember;
-            cyclic = gr.isMember(this);
+            GroupImpl gr = (GroupImpl) newMember;
+            for (Authorizable member : gr.getMembers(true, UserManager.SEARCH_TYPE_GROUP)) {
+                GroupImpl grMemberImpl = (GroupImpl) member;
+                if (getNode().getUUID().equals(grMemberImpl.getNode().getUUID())) {
+                    // found cyclic group membership
+                    return true;
+                }
+
+            }
         }
-        return cyclic;
+        return false;
     }
 
     //------------------------------------------------------< inner classes >---
@@ -281,9 +343,8 @@
             if (members == null) {
                 members = new HashSet<Principal>();
                 try {
-                    for (Iterator it = GroupImpl.this.getMembers(); it.hasNext();) {
-                        Authorizable authrz = (Authorizable) it.next();
-                        members.add(authrz.getPrincipal());
+                    for (Iterator<Authorizable> it = GroupImpl.this.getMembers(); it.hasNext();) {
+                        members.add(it.next().getPrincipal());
                     }
                 } catch (RepositoryException e) {
                     // should not occur.

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/ImpersonationImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/ImpersonationImpl.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/ImpersonationImpl.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/ImpersonationImpl.java Wed Nov 25 14:04:38 2009
@@ -83,14 +83,14 @@
      */
     public synchronized boolean grantImpersonation(Principal principal) throws RepositoryException {
         if (principal instanceof AdminPrincipal || principal instanceof SystemPrincipal) {
-            log.debug("Admin and System principal are already granted impersonation.");
+            log.warn("Admin and System principal are already granted impersonation.");
             return false;
         }
 
         // make sure the given principals belong to an existing authorizable
         Authorizable auth = user.userManager.getAuthorizable(principal);
         if (auth == null || auth.isGroup()) {
-            log.debug("Cannot grant impersonation to a principal that is a Group " +
+            log.warn("Cannot grant impersonation to a principal that is a Group " +
                       "or an unknown Authorizable.");
             return false;
         }
@@ -98,7 +98,7 @@
         String pName = principal.getName();
         // make sure user does not impersonate himself
         if (user.getPrincipal().getName().equals(pName)) {
-            log.debug("Cannot grant impersonation to oneself.");
+            log.warn("Cannot grant impersonation to oneself.");
             return false;
         }
 
@@ -116,7 +116,7 @@
      */
     public synchronized boolean revokeImpersonation(Principal principal) throws RepositoryException {
         if (principal instanceof AdminPrincipal || principal instanceof SystemPrincipal) {
-            log.debug("Admin and System principal are always granted impersonation.");
+            log.warn("Admin and System principal are always granted impersonation.");
             return false;
         }
 
@@ -151,7 +151,7 @@
 
         boolean allows = false;
         try {
-            Set impersonators = getImpersonatorNames();
+            Set<String> impersonators = getImpersonatorNames();
             allows = impersonators.removeAll(principalNames);
         } catch (RepositoryException e) {
             // should never get here

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/IndexNodeResolver.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/IndexNodeResolver.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/IndexNodeResolver.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/IndexNodeResolver.java Wed Nov 25 14:04:38 2009
@@ -118,7 +118,10 @@
                              boolean exact, long maxSize)
             throws RepositoryException {
         StringBuilder stmt = new StringBuilder("/jcr:root");
-        stmt.append(getSearchRoot(ntName));
+        String searchRoot = getSearchRoot(ntName);
+        if (!"/".equals(searchRoot)) {
+            stmt.append(searchRoot);
+        }
         stmt.append("//element(*,");
         stmt.append(getNamePathResolver().getJCRName(ntName));
 

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserAccessControlProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserAccessControlProvider.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserAccessControlProvider.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserAccessControlProvider.java Wed Nov 25 14:04:38 2009
@@ -33,6 +33,7 @@
 import org.apache.jackrabbit.core.security.authorization.Permission;
 import org.apache.jackrabbit.core.security.authorization.PrivilegeRegistry;
 import org.apache.jackrabbit.core.security.principal.PrincipalImpl;
+import org.apache.jackrabbit.core.security.SecurityConstants;
 import org.apache.jackrabbit.spi.Path;
 import org.apache.jackrabbit.util.Text;
 import org.slf4j.Logger;
@@ -88,11 +89,15 @@
 
     private final AccessControlPolicy policy;
 
-    private Path groupsPath;
-    private Path usersPath;
+    private String groupsPath;
+    private String usersPath;
 
-    private String userAdminGroup;
-    private String groupAdminGroup;
+    private Principal userAdminGroup;
+    private Principal groupAdminGroup;
+
+    private String userAdminGroupPath;
+    private String groupAdminGroupPath;
+    private String administratorsGroupPath;
 
     /**
      *
@@ -130,22 +135,26 @@
         super.init(systemSession, configuration);
         if (systemSession instanceof SessionImpl) {
             SessionImpl sImpl = (SessionImpl) systemSession;
-            userAdminGroup = (configuration.containsKey(USER_ADMIN_GROUP_NAME)) ? configuration.get(USER_ADMIN_GROUP_NAME).toString() : USER_ADMIN_GROUP_NAME;
-            groupAdminGroup = (configuration.containsKey(GROUP_ADMIN_GROUP_NAME)) ? configuration.get(GROUP_ADMIN_GROUP_NAME).toString() : GROUP_ADMIN_GROUP_NAME;
+            String userAdminName = (configuration.containsKey(USER_ADMIN_GROUP_NAME)) ? configuration.get(USER_ADMIN_GROUP_NAME).toString() : USER_ADMIN_GROUP_NAME;
+            String groupAdminName = (configuration.containsKey(GROUP_ADMIN_GROUP_NAME)) ? configuration.get(GROUP_ADMIN_GROUP_NAME).toString() : GROUP_ADMIN_GROUP_NAME;
 
             // make sure the groups exist (and possibly create them).
             UserManager uMgr = sImpl.getUserManager();
-            if (!initGroup(uMgr, userAdminGroup)) {
-                log.warn("Unable to initialize User admininistrator group -> no user admins.");
-                userAdminGroup = null;
+            userAdminGroup = initGroup(uMgr, userAdminName);
+            if (userAdminGroup != null && userAdminGroup instanceof ItemBasedPrincipal) {
+                userAdminGroupPath = ((ItemBasedPrincipal) userAdminGroup).getPath();
             }
-            if (!initGroup(uMgr, groupAdminGroup)) {
-                log.warn("Unable to initialize Group admininistrator group -> no group admins.");
-                groupAdminGroup = null;
+            groupAdminGroup = initGroup(uMgr, groupAdminName);
+            if (groupAdminGroup != null && groupAdminGroup instanceof ItemBasedPrincipal) {
+                groupAdminGroupPath = ((ItemBasedPrincipal) groupAdminGroup).getPath();
             }
 
-            usersPath = sImpl.getQPath(USERS_PATH);
-            groupsPath = sImpl.getQPath(GROUPS_PATH);
+            Principal administrators = initGroup(uMgr, SecurityConstants.ADMINISTRATORS_NAME);
+            if (administrators != null && administrators instanceof ItemBasedPrincipal) {
+                administratorsGroupPath = ((ItemBasedPrincipal) administrators).getPath();
+            }
+            usersPath = (uMgr instanceof UserManagerImpl) ? ((UserManagerImpl) uMgr).getUsersPath() : UserConstants.USERS_PATH;
+            groupsPath = (uMgr instanceof UserManagerImpl) ? ((UserManagerImpl) uMgr).getGroupsPath() : UserConstants.GROUPS_PATH;
         } else {
             throw new RepositoryException("SessionImpl (system session) expected.");
         }
@@ -194,7 +203,7 @@
     /**
      * @see org.apache.jackrabbit.core.security.authorization.AccessControlProvider#canAccessRoot(Set)
      */
-    public boolean canAccessRoot(Set principals) throws RepositoryException {
+    public boolean canAccessRoot(Set<Principal> principals) throws RepositoryException {
         checkInitialized();
         return true;
     }
@@ -255,35 +264,36 @@
         return PrivilegeRegistry.getBits(privs);
     }
 
-    private static boolean containsGroup(Set<Principal> principals, String groupName) {
-        for (Iterator it = principals.iterator(); it.hasNext() && groupName != null;) {
-            Principal p = (Principal) it.next();
-            if (p.getName().equals(groupName)) {
+    private static boolean containsGroup(Set<Principal> principals, Principal group) {
+        for (Iterator<Principal> it = principals.iterator(); it.hasNext() && group != null;) {
+            Principal p = it.next();
+            if (p.getName().equals(group.getName())) {
                 return true;
             }
         }
         return false;
     }
 
-    private static boolean initGroup(UserManager uMgr, String principalName) {
-        boolean success;
+    private static Principal initGroup(UserManager uMgr, String principalName) {
         Principal prnc = new PrincipalImpl(principalName);
         try {
             Authorizable auth = uMgr.getAuthorizable(prnc);
             if (auth == null) {
-                success = (uMgr.createGroup(prnc) != null);
+                auth = uMgr.createGroup(prnc);
             } else {
-                success = auth.isGroup();
-                if (!success) {
+                if (!auth.isGroup()) {
                     log.warn("Cannot create group '" + principalName + "'; User with that principal already exists.");
+                    auth = null;
                 }
             }
+            if (auth != null) {
+                return auth.getPrincipal();
+            }
         } catch (RepositoryException e) {
             // should never get here
             log.error("Error while initializing user/group administrators", e.getMessage());
-            success = false;
         }
-        return success;
+        return null;
     }
 
     //--------------------------------------------------------< inner class >---
@@ -304,7 +314,7 @@
             isGroupAdmin = containsGroup(principals, groupAdminGroup);
 
             int events = Event.PROPERTY_CHANGED | Event.PROPERTY_ADDED | Event.PROPERTY_REMOVED;
-            observationMgr.addEventListener(this, events, USERS_PATH, true, null, null, false);
+            observationMgr.addEventListener(this, events, groupsPath, true, null, null, false);
         }
 
         //------------------------------------< AbstractCompiledPermissions >---
@@ -335,102 +345,78 @@
             int privs;
             // Determine if for path, the set of privileges must be calculated:
             // Generally, privileges can only be determined for existing nodes.
-            boolean calcPrivs = session.nodeExists(resolver.getJCRPath(path.getNormalizedPath()));
+            String jcrPath = resolver.getJCRPath(path.getNormalizedPath());
+            boolean calcPrivs = session.nodeExists(jcrPath);
             if (calcPrivs) {
                 privs = getPrivilegeBits(Privilege.JCR_READ);
             } else {
                 privs = PrivilegeRegistry.NO_PRIVILEGE;
             }
 
-            Path abs2Path = (4 > path.getLength()) ? null : path.subPath(0, 4);
-            if (usersPath.equals(abs2Path)) {
+            if (Text.isDescendant(usersPath, jcrPath)) {
                 /*
                  below the user-tree
-                 - determine position of target relative
+                 - determine position of target relative to the editing user
                  - target may not be below an existing user but only below an
                    authorizable folder.
-                 - determine if the editing user is user/group-admin
-                 - special treatment for rep:groups property
+                 - determine if the editing user is user-admin
                  */
                 NodeImpl node = (NodeImpl) getExistingNode(path);
-
-                if (node.isNodeType(NT_REP_AUTHORIZABLE) || node.isNodeType(NT_REP_AUTHORIZABLE_FOLDER)) {
-                    boolean editingHimSelf = node.isSame(userNode);
-                    boolean isGroupProp = P_GROUPS.equals(path.getNameElement().getName());
-                    // only user-admin is allowed to modify users.
-                    // for group membership (rep:groups) group-admin is required
-                    // in addition.
-                    boolean memberOfRequiredGroups = isUserAdmin;
-                    if (memberOfRequiredGroups && isGroupProp) {
-                        memberOfRequiredGroups = isGroupAdmin;
+                if (node.isNodeType(NT_REP_AUTHORIZABLE_FOLDER)) {
+                    // an authorizable folder -> must be user admin in order
+                    // to have permission to write.
+                    if (isUserAdmin) {
+                        allows |= (Permission.ADD_NODE | Permission.REMOVE_NODE | Permission.SET_PROPERTY | Permission.REMOVE_PROPERTY | Permission.NODE_TYPE_MNGMT);
+                        if (calcPrivs) {
+                            // grant WRITE privilege
+                            // note: ac-read/modification is not included
+                            privs |= getPrivilegeBits(PrivilegeRegistry.REP_WRITE);
+                        }
                     }
-                    if (editingHimSelf) {
-                        /*
-                        node to be modified is same node as userNode. 3 cases to distinguish
-                        1) user is User-Admin -> R, W
-                        2) user is NOT U-admin but nodeID is its own node.
-                        3) special treatment for rep:group property which can
-                           only be modified by group-administrators
-                        */
-                        Path aPath = session.getQPath(node.getPath());
-                        if (memberOfRequiredGroups) {
-                            // principals contain 'user-admin'
-                            // -> user can modify items below the user-node except rep:group.
-                            // principals contains 'user-admin' + 'group-admin'
-                            // -> user can modify rep:group property as well.
-                            if (path.equals(aPath)) {
-                                allows |= (Permission.ADD_NODE | Permission.REMOVE_PROPERTY | Permission.SET_PROPERTY);
-                            } else {
-                                allows |= Permission.ALL;
-                            }
+                } else {
+                    // rep:User node or some other custom node below an existing user.
+                    // as the auth-folder doesn't allow other residual child nodes.
+                    boolean editingOwnUser = node.isSame(userNode);
+                    if (editingOwnUser) {
+                        // user can only read && write his own props
+                        allows |= (Permission.SET_PROPERTY | Permission.REMOVE_PROPERTY);
+                        if (calcPrivs) {
+                            privs |= getPrivilegeBits(Privilege.JCR_MODIFY_PROPERTIES);
+                        }
+                    } else if (isUserAdmin) {
+                        allows |= (Permission.ADD_NODE | Permission.REMOVE_NODE | Permission.SET_PROPERTY | Permission.REMOVE_PROPERTY | Permission.NODE_TYPE_MNGMT);
+                        if (calcPrivs) {
+                            // grant WRITE privilege
+                            // note: ac-read/modification is not included
+                            privs |= getPrivilegeBits(PrivilegeRegistry.REP_WRITE);
+                        }
+                    } // else: normal user that isn't allowed to modify another user.
+                }
+            } else if (Text.isDescendant(groupsPath, jcrPath)) {
+                /*
+                below group-tree:
+                - test if the user is group-administrator.
+                - make sure group-admin cannot modify user-admin or administrators
+                - ... and cannot remove itself.
+                */
+                if (isGroupAdmin) {
+                    if (!jcrPath.startsWith(administratorsGroupPath) &&
+                            !jcrPath.startsWith(userAdminGroupPath)) {
+                        if (jcrPath.equals(groupAdminGroupPath)) {
+                            // no remove perm on group-admin node
+                            allows |= (Permission.ADD_NODE | Permission.SET_PROPERTY | Permission.REMOVE_PROPERTY | Permission.NODE_TYPE_MNGMT);
                             if (calcPrivs) {
-                                // grant WRITE privilege
-                                // note: ac-read/modification is not included
-                                //       remove_node is not included
                                 privs |= getPrivilegeBits(PrivilegeRegistry.REP_WRITE);
-                                if (!path.equals(aPath)) {
-                                    privs |= getPrivilegeBits(Privilege.JCR_REMOVE_NODE);
-                                }
+                                privs ^= getPrivilegeBits(Privilege.JCR_REMOVE_NODE);
                             }
-                        } else if (userNode.isSame(node) && (!isGroupProp || isGroupAdmin)) {
-                            // user can only read && write his own props
-                            // except for the rep:group property.
-                            allows |= (Permission.SET_PROPERTY | Permission.REMOVE_PROPERTY);
+                        } else {
+                            // complete write
+                            allows |= (Permission.ADD_NODE | Permission.REMOVE_NODE | Permission.SET_PROPERTY | Permission.REMOVE_PROPERTY | Permission.NODE_TYPE_MNGMT);
                             if (calcPrivs) {
-                                privs |= getPrivilegeBits(Privilege.JCR_MODIFY_PROPERTIES);
-                            }
-                        } // else some other node below but not U-admin -> read-only.
-                    } else {
-                        /*
-                        authN points to some other user-node, i.e.
-                        1) nodeId points to an authorizable that isn't the editing user
-                        2) nodeId points to an auth-folder within the user-tree
-
-                        In either case user-admin group-membership is
-                        required in order to get write permission.
-                        group-admin group-membership is required in addition
-                        if rep:groups is the target item.
-                        */
-                        if (memberOfRequiredGroups) {
-                            allows = Permission.ALL;
-                            if (calcPrivs) {
-                                // grant WRITE privilege
-                                // note: ac-read/modification is not included
                                 privs |= getPrivilegeBits(PrivilegeRegistry.REP_WRITE);
                             }
                         }
                     }
-                } // outside of the user tree
-            } else if (groupsPath.equals(abs2Path)) {
-                /*
-                below group-tree:
-                - test if the user is group-administrator.
-                */
-                if (isGroupAdmin) {
-                    allows = Permission.ALL;
-                    if (calcPrivs) {
-                        privs |= getPrivilegeBits(PrivilegeRegistry.REP_WRITE);
-                    }
                 }
             } // else outside of user/group tree -> read only.
             return new Result(allows, denies, privs, PrivilegeRegistry.NO_PRIVILEGE);
@@ -480,33 +466,27 @@
                 Event ev = events.nextEvent();
                 try {
                     String evPath = ev.getPath();
-                    String repGroups = session.getJCRName(UserConstants.P_GROUPS);
-                    // TODO: add better evaluation.
-                    if (repGroups.equals(Text.getName(evPath)) &&
-                            userNodePath.equals(Text.getRelativeParent(evPath, 1))) {
+                    String repMembers = session.getJCRName(UserConstants.P_MEMBERS);
+                    if (repMembers.equals(Text.getName(evPath))) {
                         // recalculate the is...Admin flags
-                        switch (ev.getType()) {
-                            case Event.PROPERTY_REMOVED:
-                                isUserAdmin = false;
-                                isGroupAdmin = false;
-                                break;
-                            case Event.PROPERTY_ADDED:
-                            case Event.PROPERTY_CHANGED:
-                                if (session.propertyExists(evPath)) {
-                                    Value[] vs = session.getProperty(evPath).getValues();
-                                    String princName = session.getJCRName(P_PRINCIPAL_NAME);
-                                    for (Value v : vs) {
-                                        Node groupNode = session.getNodeByUUID(v.getString());
-                                        String pName = groupNode.getProperty(princName).getString();
-                                        if (userAdminGroup.equals(pName)) {
-                                            isUserAdmin = true;
-                                        } else if (groupAdminGroup.equals(pName)) {
-                                            isGroupAdmin = true;
-                                        }
-                                    }
+                        Node userNode = session.getNode(userNodePath);
+                        String nodePath = Text.getRelativeParent(evPath, 1);
+                        if (userAdminGroupPath.equals(nodePath)) {
+                            isUserAdmin = false;
+                            if (ev.getType() != Event.PROPERTY_REMOVED) {
+                                Value[] vs = session.getProperty(evPath).getValues();
+                                for (int i = 0; i < vs.length && !isUserAdmin; i++) {
+                                    isUserAdmin = userNode.getIdentifier().equals(vs[i].getString());
                                 }
-                                break;
-                                // default: other events are not relevant.
+                            }
+                        } else if (groupAdminGroupPath.equals(nodePath)) {
+                            isGroupAdmin = false;
+                            if (ev.getType() != Event.PROPERTY_REMOVED) {
+                                Value[] vs = session.getProperty(evPath).getValues();
+                                for (int i = 0; i < vs.length && !isGroupAdmin; i++) {
+                                    isGroupAdmin = userNode.getIdentifier().equals(vs[i].getString());
+                                }
+                            }
                         }
                         // invalidate the cached results
                         clearCache();

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserConstants.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserConstants.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserConstants.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserConstants.java Wed Nov 25 14:04:38 2009
@@ -55,7 +55,12 @@
     Name P_USERID = NF.create(Name.NS_REP_URI, "userId");
     Name P_PASSWORD = NF.create(Name.NS_REP_URI, "password");
 
+    /**
+     * @deprecated As of 2.0 group membership is stored with the group node.
+     * @see #P_MEMBERS
+     */
     Name P_GROUPS = NF.create(Name.NS_REP_URI, "groups");
+    Name P_MEMBERS = NF.create(Name.NS_REP_URI, "members");
 
     /**
      * Name of the user property containing the principal names of those allowed
@@ -67,5 +72,6 @@
     Name NT_REP_AUTHORIZABLE_FOLDER = NF.create(Name.NS_REP_URI, "AuthorizableFolder");
     Name NT_REP_USER = NF.create(Name.NS_REP_URI, "User");
     Name NT_REP_GROUP = NF.create(Name.NS_REP_URI, "Group");
+    Name MIX_REP_IMPERSONATABLE = NF.create(Name.NS_REP_URI, "Impersonatable");
 
 }
\ No newline at end of file



Mime
View raw message