You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by an...@apache.org on 2009/11/03 10:26:39 UTC

svn commit: r832359 - in /jackrabbit/trunk/jackrabbit-core/src: main/java/org/apache/jackrabbit/core/security/authentication/ main/java/org/apache/jackrabbit/core/security/principal/ main/java/org/apache/jackrabbit/core/security/user/ test/java/org/apa...

Author: angela
Date: Tue Nov  3 09:26:38 2009
New Revision: 832359

URL: http://svn.apache.org/viewvc?rev=832359&view=rev
Log:
JCR-2313 - Improvements to user management (2) [work in progress] 

- cryptedsimplecreds should compare userID case insensitive
- user generics, override annotations
- fix/improve javadoc
- tests

Added:
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authentication/CryptedSimpleCredentialsTest.java
Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authentication/CryptedSimpleCredentials.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProvider.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalIteratorAdapter.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalManagerImpl.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/principal/PrincipalManagerTest.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authentication/TestAll.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/AuthorizableImplTest.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/UserImporterTest.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authentication/CryptedSimpleCredentials.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authentication/CryptedSimpleCredentials.java?rev=832359&r1=832358&r2=832359&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authentication/CryptedSimpleCredentials.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authentication/CryptedSimpleCredentials.java Tue Nov  3 09:26:38 2009
@@ -113,9 +113,8 @@
     }
 
     /**
-     * Compair this instance with an instance of SimpleCredentials.
-     * If one the other Credentials' Password is plain-text treies to encode
-     * it with the current Digest.
+     * Compares this instance with the given <code>SimpleCredentials</code> and
+     * returns <code>true</code> if both match.
      *
      * @param credentials
      * @return true if {@link SimpleCredentials#getUserID() UserID} and
@@ -126,7 +125,7 @@
     public boolean matches(SimpleCredentials credentials)
             throws NoSuchAlgorithmException, UnsupportedEncodingException {
 
-        if (getUserID().matches(credentials.getUserID())) {
+        if (getUserID().equalsIgnoreCase(credentials.getUserID())) {
             String toMatch = new String(credentials.getPassword());
             String algr = getAlgorithm(toMatch);
 
@@ -148,7 +147,7 @@
     private static String crypt(String pwd, String algorithm)
             throws NoSuchAlgorithmException, UnsupportedEncodingException {
 
-        StringBuffer password = new StringBuffer();
+        StringBuilder password = new StringBuilder();
         password.append("{").append(algorithm).append("}");
         password.append(Text.digest(algorithm, pwd.getBytes("UTF-8")));
         return password.toString();

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProvider.java?rev=832359&r1=832358&r2=832359&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProvider.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProvider.java Tue Nov  3 09:26:38 2009
@@ -120,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)) {
@@ -206,6 +207,7 @@
     /**
      * @see PrincipalProvider#close()
      */
+    @Override
     public synchronized void close() {
         super.close();
         membershipCache.clear();
@@ -354,6 +356,7 @@
         /**
          * @see org.apache.jackrabbit.core.security.principal.AbstractPrincipalIterator#seekNext()
          */
+        @Override
         protected Principal seekNext() {
             while (authorizableItr.hasNext()) {
                 try {

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalIteratorAdapter.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalIteratorAdapter.java?rev=832359&r1=832358&r2=832359&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalIteratorAdapter.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalIteratorAdapter.java Tue Nov  3 09:26: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/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalManagerImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalManagerImpl.java?rev=832359&r1=832358&r2=832359&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalManagerImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/PrincipalManagerImpl.java Tue Nov  3 09:26: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/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=832359&r1=832358&r2=832359&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 Tue Nov  3 09:26:38 2009
@@ -27,8 +27,8 @@
 import org.apache.jackrabbit.core.ProtectedItemModifier;
 import org.apache.jackrabbit.core.SessionImpl;
 import org.apache.jackrabbit.core.SessionListener;
-import org.apache.jackrabbit.core.id.NodeId;
 import org.apache.jackrabbit.core.security.principal.PrincipalImpl;
+import org.apache.jackrabbit.core.id.NodeId;
 import org.apache.jackrabbit.spi.Name;
 import org.apache.jackrabbit.util.Text;
 import org.slf4j.Logger;
@@ -386,16 +386,6 @@
     }
 
     /**
-     * Creates a new Node on the repository with the specified
-     * <code>userName</code>.<br>
-     * The User will be created relative to path of the User who represents the
-     * Session this UserManager has been created for.<br>
-     * If the {@link javax.jcr.Credentials Credentials} are of type
-     * {@link javax.jcr.SimpleCredentials SimpleCredentials} they will be
-     * crypted.
-     *
-     * @param userID
-     * @param password
      * @see UserManager#createUser(String,String)
      */
     public User createUser(String userID, String password) throws RepositoryException {
@@ -403,14 +393,7 @@
     }
 
     /**
-     *
-     * @param userID
-     * @param password
-     * @param principal
-     * @param intermediatePath Is always ignored.
-     * @return
-     * @throws AuthorizableExistsException
-     * @throws RepositoryException
+     * @see UserManager#createUser(String, String, java.security.Principal, String)
      */
     public User createUser(String userID, String password,
                            Principal principal, String intermediatePath)
@@ -464,7 +447,8 @@
      * principal name != ID) the principal name is expanded by a suffix;
      * otherwise the resulting group ID equals the principal name.
      *
-     * @param principal
+     * @param principal A principal that doesn't yet represent an existing user
+     * or group.
      * @param intermediatePath Is always ignored.
      * @return A new group.
      * @throws AuthorizableExistsException
@@ -516,6 +500,14 @@
     }
 
     //--------------------------------------------------------------------------
+    /**
+     *
+     * @param node The new user/group node.
+     * @param principal A valid non-null principal.
+     * @throws AuthorizableExistsException If there is already another user/group
+     * with the same principal name.
+     * @throws RepositoryException If another error occurs.
+     */
     void setPrincipal(NodeImpl node, Principal principal) throws AuthorizableExistsException, RepositoryException {
         if (!isValidPrincipal(principal)) {
             throw new IllegalArgumentException("Cannot create Authorizable: Principal may not be null and must have a valid name.");
@@ -575,9 +567,9 @@
      * - isn't placed underneith the configured user/group tree.
      * </pre>
      *
-     * @param n
+     * @param n A user/group node.
      * @return An authorizable or <code>null</code>.
-     * @throws RepositoryException
+     * @throws RepositoryException If an error occurs.
      */
     Authorizable getAuthorizable(NodeImpl n) throws RepositoryException {
         Authorizable authorz = null;
@@ -603,7 +595,7 @@
      *
      * @param principalName to be used as hint for the groupid.
      * @return a group id.
-     * @throws RepositoryException
+     * @throws RepositoryException If an error occurs.
      */
     private String getGroupId(String principalName) throws RepositoryException {
         String groupID = principalName;
@@ -650,7 +642,7 @@
     }
 
     /**
-     * @param userID
+     * @param userID A userID.
      * @return true if the given userID belongs to the administrator user.
      */
     boolean isAdminId(String userID) {
@@ -660,9 +652,10 @@
     /**
      * Build the User object from the given user node.
      *
-     * @param userNode
-     * @return
-     * @throws RepositoryException
+     * @param userNode The new user node.
+     * @return An instance of <code>User</code>.
+     * @throws RepositoryException If the node isn't a child of the configured
+     * usersPath-node or if another error occurs.
      */
     User createUser(NodeImpl userNode) throws RepositoryException {
         if (userNode == null || !userNode.isNodeType(NT_REP_USER)) {
@@ -690,9 +683,10 @@
     /**
      * Build the Group object from the given group node.
      *
-     * @param groupNode
-     * @return
-     * @throws RepositoryException
+     * @param groupNode The new group node.
+     * @return An instance of <code>Group</code>.
+     * @throws RepositoryException If the node isn't a child of the configured
+     * groupsPath-node or if another error occurs.
      */
     Group createGroup(NodeImpl groupNode) throws RepositoryException {
         if (groupNode == null || !groupNode.isNodeType(NT_REP_GROUP)) {

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/principal/PrincipalManagerTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/principal/PrincipalManagerTest.java?rev=832359&r1=832358&r2=832359&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/principal/PrincipalManagerTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/principal/PrincipalManagerTest.java Tue Nov  3 09:26:38 2009
@@ -27,7 +27,6 @@
 import java.security.acl.Group;
 import java.util.Enumeration;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.Set;
 
 /**
@@ -145,9 +144,9 @@
         while (it.hasNext()) {
             Principal p = it.nextPrincipal();
             if (isGroup(p) && !p.equals(principalMgr.getEveryone())) {
-                Enumeration en = ((java.security.acl.Group) p).members();
+                Enumeration<? extends Principal> en = ((java.security.acl.Group) p).members();
                 while (en.hasMoreElements()) {
-                    Principal memb = (Principal) en.nextElement();
+                    Principal memb = en.nextElement();
                     assertTrue(principalMgr.hasPrincipal(memb.getName()));
                 }
             }
@@ -198,9 +197,9 @@
 
             assertTrue(isGroup(p));
 
-            Enumeration members = ((java.security.acl.Group) p).members();
+            Enumeration<? extends Principal> members = ((java.security.acl.Group) p).members();
             while (members.hasMoreElements()) {
-                Principal memb = (Principal) members.nextElement();
+                Principal memb = members.nextElement();
 
                 Principal group = null;
                 PrincipalIterator mship = principalMgr.getGroupMembership(memb);
@@ -221,7 +220,7 @@
             if (pcpl.equals(everyone)) {
                 continue;
             }
-            Iterator it = principalMgr.findPrincipals(pcpl.getName());
+            PrincipalIterator it = principalMgr.findPrincipals(pcpl.getName());
             // search must find at least a single principal
             assertTrue("findPrincipals does not find principal with filter " + pcpl.getName(), it.hasNext());
         }
@@ -236,12 +235,12 @@
             }
 
             if (isGroup(pcpl)) {
-                Iterator it = principalMgr.findPrincipals(pcpl.getName(),
+                PrincipalIterator it = principalMgr.findPrincipals(pcpl.getName(),
                         PrincipalManager.SEARCH_TYPE_GROUP);
                 // search must find at least a single matching group principal
                 assertTrue("findPrincipals does not find principal with filter " + pcpl.getName(), it.hasNext());
             } else {
-                Iterator it = principalMgr.findPrincipals(pcpl.getName(),
+                PrincipalIterator it = principalMgr.findPrincipals(pcpl.getName(),
                         PrincipalManager.SEARCH_TYPE_NOT_GROUP);
                 // search must find at least a single matching non-group principal
                 assertTrue("findPrincipals does not find principal with filter " + pcpl.getName(), it.hasNext());

Added: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authentication/CryptedSimpleCredentialsTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authentication/CryptedSimpleCredentialsTest.java?rev=832359&view=auto
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authentication/CryptedSimpleCredentialsTest.java (added)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authentication/CryptedSimpleCredentialsTest.java Tue Nov  3 09:26:38 2009
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.core.security.authentication;
+
+import org.apache.jackrabbit.api.JackrabbitSession;
+import org.apache.jackrabbit.api.security.user.User;
+import org.apache.jackrabbit.api.security.user.UserManager;
+import org.apache.jackrabbit.test.AbstractJCRTest;
+import org.apache.jackrabbit.test.NotExecutableException;
+
+import javax.jcr.Credentials;
+import javax.jcr.SimpleCredentials;
+
+/**
+ * <code>CryptedSimpleCredentialsTest</code>...
+ */
+public class CryptedSimpleCredentialsTest extends AbstractJCRTest {
+
+    private String userID;
+    private CryptedSimpleCredentials creds;
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+
+        userID = superuser.getUserID();
+        if (superuser instanceof JackrabbitSession) {
+            UserManager umg = ((JackrabbitSession) superuser).getUserManager();
+            User u = (User) umg.getAuthorizable(userID);
+            Credentials crd = u.getCredentials();
+            if (crd instanceof SimpleCredentials) {
+                creds = new CryptedSimpleCredentials((SimpleCredentials) crd);
+            } else {
+                throw new NotExecutableException();
+            }
+        } else {
+            throw new NotExecutableException();
+        }
+    }
+
+    public void testUserIDMatchesCaseInsensitive() throws Exception {
+        String uid = userID.toUpperCase();
+        assertTrue(creds.matches(new SimpleCredentials(uid, creds.getPassword().toCharArray())));
+
+        uid = userID.toLowerCase();
+        assertTrue(creds.matches(new SimpleCredentials(uid, creds.getPassword().toCharArray())));
+    }
+
+    public void testGetUserID() {
+        assertEquals(userID, creds.getUserID());
+    }
+}
\ No newline at end of file

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authentication/TestAll.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authentication/TestAll.java?rev=832359&r1=832358&r2=832359&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authentication/TestAll.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authentication/TestAll.java Tue Nov  3 09:26:38 2009
@@ -16,22 +16,19 @@
  */
 package org.apache.jackrabbit.core.security.authentication;
 
-import junit.framework.TestCase;
 import junit.framework.Test;
+import junit.framework.TestCase;
 import junit.framework.TestSuite;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /** <code>TestAll</code>... */
 public class TestAll extends TestCase {
 
-    private static Logger log = LoggerFactory.getLogger(TestAll.class);
-
     public static Test suite() {
         TestSuite suite = new TestSuite("core.security.authentication tests");
 
         suite.addTestSuite(GuestLoginTest.class);
         suite.addTestSuite(NullLoginTest.class);
+        suite.addTestSuite(CryptedSimpleCredentialsTest.class);
 
         return suite;
     }

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=832359&r1=832358&r2=832359&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 Tue Nov  3 09:26:38 2009
@@ -134,7 +134,9 @@
         NodeImpl n = user.getNode();
 
         checkProtected(n.getProperty(UserConstants.P_PASSWORD));
-        checkProtected(n.getProperty(UserConstants.P_PRINCIPAL_NAME));
+        if (n.hasProperty(UserConstants.P_PRINCIPAL_NAME)) {
+            checkProtected(n.getProperty(UserConstants.P_PRINCIPAL_NAME));
+        }
         if (n.hasProperty(UserConstants.P_IMPERSONATORS)) {
            checkProtected(n.getProperty(UserConstants.P_IMPERSONATORS));
         }
@@ -144,7 +146,9 @@
         GroupImpl gr = (GroupImpl) getTestGroup(superuser);
         NodeImpl n = gr.getNode();
 
-        checkProtected(n.getProperty(UserConstants.P_PRINCIPAL_NAME));
+        if (n.hasProperty(UserConstants.P_PRINCIPAL_NAME)) {
+            checkProtected(n.getProperty(UserConstants.P_PRINCIPAL_NAME));
+        }
         if (n.hasProperty(UserConstants.P_MEMBERS)) {
             checkProtected(n.getProperty(UserConstants.P_MEMBERS));
         }
@@ -187,12 +191,41 @@
         }
     }
 
-    public void testRemoveSpecialPropertiesDirectly() throws RepositoryException, NotExecutableException {
+    public void testRemoveSpecialUserPropertiesDirectly() throws RepositoryException, NotExecutableException {
+        AuthorizableImpl g = (AuthorizableImpl) getTestUser(superuser);
+        NodeImpl n = g.getNode();
+        try {
+            n.getProperty(UserConstants.P_PASSWORD).remove();
+            fail("Attempt to remove protected property rep:password should fail.");
+        } catch (ConstraintViolationException e) {
+            // ok.
+        }
+        try {
+            if (n.hasProperty(UserConstants.P_PRINCIPAL_NAME)) {
+                n.getProperty(UserConstants.P_PRINCIPAL_NAME).remove();
+                fail("Attempt to remove protected property rep:principalName should fail.");
+            }
+        } catch (ConstraintViolationException e) {
+            // ok.
+        }
+    }
+
+    public void testRemoveSpecialGroupPropertiesDirectly() throws RepositoryException, NotExecutableException {
         AuthorizableImpl g = (AuthorizableImpl) getTestGroup(superuser);
         NodeImpl n = g.getNode();
         try {
-            n.getProperty(UserConstants.P_PRINCIPAL_NAME).remove();
-            fail("Attempt to remove protected property rep:principalName should fail.");
+            if (n.hasProperty(UserConstants.P_PRINCIPAL_NAME)) {
+                n.getProperty(UserConstants.P_PRINCIPAL_NAME).remove();
+                fail("Attempt to remove protected property rep:principalName should fail.");
+            }
+        } catch (ConstraintViolationException e) {
+            // ok.
+        }
+        try {
+            if (n.hasProperty(UserConstants.P_MEMBERS)) {
+                n.getProperty(UserConstants.P_MEMBERS).remove();
+                fail("Attempt to remove protected property rep:members should fail.");
+            }
         } catch (ConstraintViolationException e) {
             // ok.
         }

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/UserImporterTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/UserImporterTest.java?rev=832359&r1=832358&r2=832359&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/UserImporterTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/UserImporterTest.java Tue Nov  3 09:26:38 2009
@@ -403,8 +403,9 @@
         /*
          importing a group with a multi-valued rep:principalName property
          - nonProtected node rep:Group must be created.
-         - protected property rep:principalName must be ignored
-         - saving changes must fail with ConstraintViolationEx.
+         - property rep:principalName must be created regularly without being protected
+         - saving changes must fail with ConstraintViolationEx. as the protected
+           mandatory property rep:principalName is missing
          */
         NodeImpl target = (NodeImpl) sImpl.getNode(umgr.getGroupsPath());
         try {
@@ -437,6 +438,52 @@
         }
     }
 
+    public void testMultiValuedPassword() throws RepositoryException, IOException, SAXException, NotExecutableException {
+        String xml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
+                "<sv:node sv:name=\"t\" xmlns:mix=\"http://www.jcp.org/jcr/mix/1.0\" xmlns:nt=\"http://www.jcp.org/jcr/nt/1.0\" xmlns:fn_old=\"http://www.w3.org/2004/10/xpath-functions\" xmlns:fn=\"http://www.w3.org/2005/xpath-functions\" xmlns:xs=\"http://www.w3.org/2001/XMLSchema\" xmlns:sv=\"http://www.jcp.org/jcr/sv/1.0\" xmlns:rep=\"internal\" xmlns:jcr=\"http://www.jcp.org/jcr/1.0\">" +
+                "   <sv:property sv:name=\"jcr:primaryType\" sv:type=\"Name\"><sv:value>rep:User</sv:value></sv:property>" +
+                "   <sv:property sv:name=\"jcr:uuid\" sv:type=\"String\"><sv:value>e358efa4-89f5-3062-b10d-d7316b65649e</sv:value></sv:property>" +
+                "   <sv:property sv:name=\"rep:password\" sv:type=\"String\"><sv:value>{sha1}8efd86fb78a56a5145ed7739dcb00c78581c5375</sv:value><sv:value>{sha1}8efd86fb78a56a5145ed7739dcb00c78581c5375</sv:value></sv:property>" +
+                "   <sv:property sv:name=\"rep:principalName\" sv:type=\"String\"><sv:value>t</sv:value></sv:property>" +
+                "</sv:node>";
+        /*
+         importing a user with a multi-valued rep:password property
+         - nonProtected node rep:User must be created.
+         - property rep:password must be created regularly without being protected
+         - saving changes must fail with ConstraintViolationEx. as the protected
+           mandatory property rep:password is missing
+         */
+        NodeImpl target = (NodeImpl) sImpl.getNode(umgr.getUsersPath());
+        try {
+            doImport(target, xml);
+
+            assertTrue(target.isModified());
+            assertTrue(sImpl.hasPendingChanges());
+
+            Authorizable newUser = umgr.getAuthorizable("t");
+            assertNotNull(newUser);
+
+            assertTrue(target.hasNode("t"));
+            assertTrue(target.hasProperty("t/rep:password"));
+            assertFalse(target.getProperty("t/rep:password").getDefinition().isProtected());
+
+            // saving changes of the import -> must fail as mandatory prop is missing
+            try {
+                sImpl.save();
+                fail("Import must be incomplete. Saving changes must fail.");
+            } catch (ConstraintViolationException e) {
+                // success
+            }
+
+        } finally {
+            sImpl.refresh(false);
+            if (target.hasNode("t")) {
+                target.getNode("t").remove();
+                sImpl.save();
+            }
+        }
+    }
+
     public void testIncompleteUser() throws RepositoryException, IOException, SAXException, NotExecutableException {
         List<String> incompleteXml = new ArrayList();
         incompleteXml.add("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +