Mailing-List: contact commits-help@jackrabbit.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@jackrabbit.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r746301 - in /jackrabbit/trunk/jackrabbit-core/src:
 main/java/org/apache/jackrabbit/core/nodetype/
 main/java/org/apache/jackrabbit/core/security/principal/
 main/java/org/apache/jackrabbit/core/security/user/
 test/java/org/apache/jackrabbit/...
Date: Fri, 20 Feb 2009 17:40:02 -0000
To: commits@jackrabbit.apache.org
From: angela@apache.org
Message-Id: <20090220174003.5CCEE23888A2@eris.apache.org>

Author: angela
Date: Fri Feb 20 17:40:01 2009
New Revision: 746301

URL: http://svn.apache.org/viewvc?rev=746301&view=rev
Log:
JCR-1984 : UserManagement: Limit set of properties exposed by AuthorizableImpl
JCR-1993 : UserManagement: Missing assertion that Principal name isn't ""

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/nodetype/NodeTypeImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserManagerImpl.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/AbstractUserTest.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/UserManagerCreateUserTest.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/TestPrincipal.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/AuthorizableImplTest.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/nodetype/NodeTypeImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/nodetype/NodeTypeImpl.java?rev=746301&r1=746300&r2=746301&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/nodetype/NodeTypeImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/nodetype/NodeTypeImpl.java Fri Feb 20 17:40:01 2009
@@ -75,6 +75,16 @@
     }
 
     /**
+     * Checks if the effective node type includes the given <code>nodeTypeName</code>.
+     *
+     * @param nodeTypeName
+     * @return true if the effective node type includes the given <code>nodeTypeName</code>.
+     */
+    public boolean isNodeType(Name nodeTypeName) {
+        return ent.includesNodeType(nodeTypeName);
+    }
+
+    /**
      * Checks if this node type is directly or indirectly derived from the
      * specified node type.
      *
@@ -349,7 +359,7 @@
             log.warn("invalid node type name: " + nodeTypeName, e);
             return false;
         }
-        return (getQName().equals(ntName) || isDerivedFrom(ntName));
+        return isNodeType(ntName);
     }
 
     /**

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalImpl.java?rev=746301&r1=746300&r2=746301&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalImpl.java Fri Feb 20 17:40:01 2009
@@ -39,8 +39,8 @@
      * @param name the name of this principal
      */
     public PrincipalImpl(String name) {
-        if (name == null) {
-            throw new IllegalArgumentException("name can not be null");
+        if (name == null || name.length() == 0) {
+            throw new IllegalArgumentException("Principal name can neither be null nor empty String.");
         }
         this.name = name;
     }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java?rev=746301&r1=746300&r2=746301&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java Fri Feb 20 17:40:01 2009
@@ -26,6 +26,7 @@
 import org.apache.jackrabbit.core.NodeImpl;
 import org.apache.jackrabbit.core.PropertyImpl;
 import org.apache.jackrabbit.core.SessionImpl;
+import org.apache.jackrabbit.core.nodetype.NodeTypeImpl;
 import org.apache.jackrabbit.core.security.principal.ItemBasedPrincipal;
 import org.apache.jackrabbit.core.security.principal.PrincipalImpl;
 import org.apache.jackrabbit.core.security.principal.PrincipalIteratorAdapter;
@@ -39,6 +40,7 @@
 import javax.jcr.RepositoryException;
 import javax.jcr.Value;
 import javax.jcr.nodetype.ConstraintViolationException;
+import javax.jcr.nodetype.PropertyDefinition;
 import java.security.Principal;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -164,8 +166,10 @@
     public Iterator getPropertyNames() throws RepositoryException {
         List l = new ArrayList();
         for (PropertyIterator it = node.getProperties(); it.hasNext();) {
-            String propName = it.nextProperty().getName();
-            l.add(propName);
+            Property prop = it.nextProperty();
+            if (isAuthorizableProperty(prop)) {
+                l.add(prop.getName());
+            }
         }
         return l.iterator();
     }
@@ -174,7 +178,7 @@
      * @see #getProperty(String)
      */
     public boolean hasProperty(String name) throws RepositoryException {
-        return node.hasProperty(name);
+        return node.hasProperty(name) && isAuthorizableProperty(node.getProperty(name));
     }
 
     /**
@@ -184,10 +188,12 @@
     public Value[] getProperty(String name) throws RepositoryException {
         if (hasProperty(name)) {
             Property prop = node.getProperty(name);
-            if (prop.getDefinition().isMultiple()) {
-                return prop.getValues();
-            } else {
-                return new Value[] {prop.getValue()};
+            if (isAuthorizableProperty(prop)) {
+                if (prop.getDefinition().isMultiple()) {
+                    return prop.getValues();
+                } else {
+                    return new Value[] {prop.getValue()};
+                }
             }
         }
         return null;
@@ -359,6 +365,26 @@
     }
 
     /**
+     * Returns true if the given property of the authorizable node is one of the
+     * non-protected properties defined by the rep:authorizable.
+     *
+     * @param prop
+     * @return <code>true</code> if the given property is defined
+     * by the rep:authorizable node type or one of it's sub-node types;
+     * <code>false</code> otherwise.
+     * @throws RepositoryException
+     */
+    private static boolean isAuthorizableProperty(Property prop) throws RepositoryException {
+        PropertyDefinition def = prop.getDefinition();
+        if (def.isProtected()) {
+            return false;
+        } else  {
+            NodeTypeImpl declaringNt = (NodeTypeImpl) prop.getDefinition().getDeclaringNodeType();
+            return declaringNt.isNodeType(UserConstants.NT_REP_AUTHORIZABLE);
+        }
+    }
+
+    /**
      * Test if the JCR property to be modified/removed is one of the
      * following that has a special meaning and must be altered using this
      * user API:
@@ -381,14 +407,14 @@
      */
     private boolean isProtectedProperty(String propertyName) throws RepositoryException {
         Name pName = getSession().getQName(propertyName);
-         if (P_PRINCIPAL_NAME.equals(pName) || P_USERID.equals(pName)
-                 || P_REFEREES.equals(pName) || P_GROUPS.equals(pName)
-                 || P_IMPERSONATORS.equals(pName) || P_PASSWORD.equals(pName)) {
-             return true;
-         } else {
-             return false;
-         }
-     }
+        if (P_PRINCIPAL_NAME.equals(pName) || P_USERID.equals(pName)
+                || P_REFEREES.equals(pName) || P_GROUPS.equals(pName)
+                || P_IMPERSONATORS.equals(pName) || P_PASSWORD.equals(pName)) {
+            return true;
+        } else {
+            return false;
+        }
+    }
 
     /**
      * Throws ConstraintViolationException if {@link #isProtectedProperty(String)}
@@ -399,8 +425,8 @@
      */
     private void checkProtectedProperty(String propertyName) throws 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 an Authorizable.");
+        }
     }
 
     private List getRefereeValues() throws RepositoryException {

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserManagerImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserManagerImpl.java?rev=746301&r1=746300&r2=746301&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserManagerImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserManagerImpl.java Fri Feb 20 17:40:01 2009
@@ -200,8 +200,14 @@
     public User createUser(String userID, String password,
                            Principal principal, String intermediatePath)
             throws AuthorizableExistsException, RepositoryException {
-        if (userID == null || password == null || principal == null) {
-            throw new IllegalArgumentException("Not possible to create user with null parameters");
+        if (userID == null || userID.length() == 0) {
+            throw new IllegalArgumentException("Cannot create user: UserID can neither be null nor empty String.");
+        }
+        if (password == null) {
+            throw new IllegalArgumentException("Cannot create user: null password.");
+        }
+        if (!isValidPrincipal(principal)) {
+            throw new IllegalArgumentException("Cannot create user: Principal may not be null and must have a valid name.");            
         }
         if (getAuthorizable(userID) != null) {
             throw new AuthorizableExistsException("User for '" + userID + "' already exists");
@@ -258,8 +264,8 @@
      * @throws RepositoryException
      */
     public Group createGroup(Principal principal, String intermediatePath) throws AuthorizableExistsException, RepositoryException {
-        if (principal == null) {
-            throw new IllegalArgumentException("Principal might not be null.");
+        if (!isValidPrincipal(principal)) {
+            throw new IllegalArgumentException("Cannot create Group: Principal may not be null and must have a valid name.");
         }
         if (hasAuthorizableOrReferee(principal)) {
             throw new AuthorizableExistsException("Authorizable for '" + principal.getName() + "' already exists: ");
@@ -436,7 +442,7 @@
      */
     private String getCurrentUserPath() {
         // fallback: default user-path
-        String currentUserPath = USERS_PATH;;
+        String currentUserPath = USERS_PATH;
         String userId = session.getUserID();
 
         if (idPathMap.containsKey(userId)) {
@@ -455,6 +461,10 @@
         return currentUserPath;
     }
 
+    private static boolean isValidPrincipal(Principal principal) {
+        return principal != null && principal.getName() != null && principal.getName().length() > 0;
+    }
+    
     private static String getParentPath(String hint, String root) {
         StringBuffer b = new StringBuffer();
         if (hint == null || !hint.startsWith(root)) {

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/AbstractUserTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/AbstractUserTest.java?rev=746301&r1=746300&r2=746301&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/AbstractUserTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/AbstractUserTest.java Fri Feb 20 17:40:01 2009
@@ -70,7 +70,11 @@
 
     protected Principal getTestPrincipal() throws RepositoryException {
         String pn = "any_principal" + UUID.randomUUID();
-        Principal p = new TestPrincipal(pn);
+        return getTestPrincipal(pn);
+    }
+
+    protected Principal getTestPrincipal(String name) throws RepositoryException {
+        Principal p = new TestPrincipal(name);
         return p;
     }
 

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/UserManagerCreateUserTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/UserManagerCreateUserTest.java?rev=746301&r1=746300&r2=746301&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/UserManagerCreateUserTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/UserManagerCreateUserTest.java Fri Feb 20 17:40:01 2009
@@ -67,6 +67,16 @@
         assertEquals(p.getName(), user.getPrincipal().getName());
     }
 
+    public void testCreateUserWithPath2() throws RepositoryException {
+        Principal p = getTestPrincipal();
+        String uid = p.getName();
+        User user = userMgr.createUser(uid, buildPassword(uid, true), p, "any/path");
+        createdUsers.add(user);
+
+        assertNotNull(user.getID());
+        assertEquals(p.getName(), user.getPrincipal().getName());
+    }
+
     public void testCreateUserWithDifferentPrincipalName() throws RepositoryException {
         Principal p = getTestPrincipal();
         String uid = getTestPrincipal().getName();
@@ -114,7 +124,7 @@
             User user = userMgr.createUser("", "anyPW");
             createdUsers.add(user);
 
-            fail("A User cannot be built with 'null' userID");
+            fail("A User cannot be built with \"\" userID");
         } catch (Exception e) {
             // ok
         }
@@ -122,7 +132,7 @@
             User user = userMgr.createUser("", "anyPW", getTestPrincipal(), null);
             createdUsers.add(user);
 
-            fail("A User cannot be built with 'null' userID");
+            fail("A User cannot be built with \"\" userID");
         } catch (Exception e) {
             // ok
         }
@@ -159,6 +169,29 @@
         }
     }
 
+    public void testCreateUserWithEmptyPrincipal() throws RepositoryException {
+        try {
+            Principal p = getTestPrincipal("");
+            String uid = p.getName();
+            User user = userMgr.createUser(uid, buildPassword(uid, true), p, "/a/b/c");
+            createdUsers.add(user);
+
+            fail("A User cannot be built with ''-named Principal");
+        } catch (Exception e) {
+            // ok
+        }
+        try {
+            Principal p = getTestPrincipal(null);
+            String uid = p.getName();
+            User user = userMgr.createUser(uid, buildPassword(uid, true), p, "/a/b/c");
+            createdUsers.add(user);
+
+            fail("A User cannot be built with ''-named Principal");
+        } catch (Exception e) {
+            // ok
+        }
+    }
+
     public void testCreateTwiceWithSameUserID() throws RepositoryException {
         String uid = getTestPrincipal().getName();
         User user = userMgr.createUser(uid, buildPassword(uid, false));

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/TestPrincipal.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/TestPrincipal.java?rev=746301&r1=746300&r2=746301&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/TestPrincipal.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/TestPrincipal.java Fri Feb 20 17:40:01 2009
@@ -40,7 +40,7 @@
     }
 
     public int hashCode() {
-        return name.hashCode();
+        return name == null ? 0 : name.hashCode();
     }
 
     public boolean equals(Object obj) {
@@ -48,7 +48,8 @@
             return true;
         }
         if (obj instanceof Principal) {
-            return name.equals(((Principal)obj).getName());
+            String otherName = ((Principal)obj).getName();
+            return (name == null) ? otherName == null : name.equals(otherName);
         }
         return false;
     }

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/AuthorizableImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/AuthorizableImplTest.java?rev=746301&r1=746300&r2=746301&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/AuthorizableImplTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/AuthorizableImplTest.java Fri Feb 20 17:40:01 2009
@@ -22,6 +22,7 @@
 import org.apache.jackrabbit.api.security.user.User;
 import org.apache.jackrabbit.core.NodeImpl;
 import org.apache.jackrabbit.core.SessionImpl;
+import org.apache.jackrabbit.core.PropertyImpl;
 import org.apache.jackrabbit.spi.commons.conversion.NameResolver;
 import org.apache.jackrabbit.test.NotExecutableException;
 import org.apache.jackrabbit.value.StringValue;
@@ -31,7 +32,9 @@
 import javax.jcr.Property;
 import javax.jcr.RepositoryException;
 import javax.jcr.Value;
+import javax.jcr.PropertyIterator;
 import javax.jcr.nodetype.ConstraintViolationException;
+import javax.jcr.nodetype.NodeType;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
@@ -58,6 +61,7 @@
             protectedUserProps.add(resolver.getJCRName(UserConstants.P_PRINCIPAL_NAME));
             protectedUserProps.add(resolver.getJCRName(UserConstants.P_REFEREES));
 
+            protectedUserProps.add(resolver.getJCRName(UserConstants.P_GROUPS));
             protectedGroupProps.add(resolver.getJCRName(UserConstants.P_PRINCIPAL_NAME));
             protectedGroupProps.add(resolver.getJCRName(UserConstants.P_REFEREES));
         } else {
@@ -198,4 +202,42 @@
             // ok.
         }
     }
+
+    public void testUserGetProperties() throws RepositoryException, NotExecutableException {
+        AuthorizableImpl user = (AuthorizableImpl) getTestUser(superuser);
+        NodeImpl n = user.getNode();
+
+        for (PropertyIterator it = n.getProperties(); it.hasNext();) {
+            PropertyImpl p = (PropertyImpl) it.nextProperty();
+            NodeType declaringNt = p.getDefinition().getDeclaringNodeType();
+            if (!declaringNt.isNodeType("rep:Authorizable") ||
+                    protectedUserProps.contains(p.getName())) {
+                assertFalse(user.hasProperty(p.getName()));
+                assertNull(user.getProperty(p.getName()));
+            } else {
+                // authorizable defined property
+                assertTrue(user.hasProperty(p.getName()));
+                assertNotNull(user.getProperty(p.getName()));
+            }
+        }
+    }
+
+    public void testGroupGetProperties() throws RepositoryException, NotExecutableException {
+        AuthorizableImpl group = (AuthorizableImpl) getTestGroup(superuser);
+        NodeImpl n = group.getNode();
+
+        for (PropertyIterator it = n.getProperties(); it.hasNext();) {
+            PropertyImpl p = (PropertyImpl) it.nextProperty();
+            NodeType declaringNt = p.getDefinition().getDeclaringNodeType();
+            if (!declaringNt.isNodeType("rep:Authorizable") ||
+                    protectedGroupProps.contains(p.getName())) {
+                assertFalse(group.hasProperty(p.getName()));
+                assertNull(group.getProperty(p.getName()));
+            } else {
+                // authorizable defined property
+                assertTrue(group.hasProperty(p.getName()));
+                assertNotNull(group.getProperty(p.getName()));
+            }
+        }
+    }
 }
\ No newline at end of file