You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by ju...@apache.org on 2009/11/25 15:04:50 UTC

svn commit: r884108 [7/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...

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/AccessManagerTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/AccessManagerTest.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/AccessManagerTest.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/AccessManagerTest.java Wed Nov 25 14:04:38 2009
@@ -100,7 +100,8 @@
         AccessManager acMgr = getAccessManager(superuser);
 
         NodeId id = (NodeId) getItemId(superuser.getItem(testRootNode.getPath()));
-        // NOTE: backwards compat. for depr. method: invalid perm-flags will be ignored
+        // NOTE: backwards compatibility.
+        // for deprecated method: invalid perm-flags will be ignored
         acMgr.checkPermission(id, AccessManager.READ - 1);
     }
 
@@ -108,7 +109,8 @@
         AccessManager acMgr = getAccessManager(superuser);
 
         NodeId id = (NodeId) getItemId(superuser.getItem(testRootNode.getPath()));
-        // NOTE: backwards compat. for depr. method: invalid perm-flags will be ignored
+        // NOTE: backwards compatibility.
+        // for deprecated method: invalid perm-flags will be ignored
         acMgr.checkPermission(id, AccessManager.READ | AccessManager.WRITE | AccessManager.REMOVE  + 1);
     }
 
@@ -186,8 +188,8 @@
         Session s = getHelper().getReadOnlySession();
         try {
             String[] wspNames = s.getWorkspace().getAccessibleWorkspaceNames();
-            for (int i = 0; i < wspNames.length; i++) {
-                assertTrue(getAccessManager(s).canAccess(wspNames[i]));
+            for (String wspName : wspNames) {
+                assertTrue(getAccessManager(s).canAccess(wspName));
             }
         } finally {
             s.logout();
@@ -214,7 +216,7 @@
     public void testCanAccessNotExistingWorkspace() throws RepositoryException, NotExecutableException {
         Session s = getHelper().getReadOnlySession();
         try {
-            List all = Arrays.asList(s.getWorkspace().getAccessibleWorkspaceNames());
+            List<String> all = Arrays.asList(s.getWorkspace().getAccessibleWorkspaceNames());
             String testName = "anyWorkspace";
             int i = 0;
             while (all.contains(testName)) {

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authentication/TestAll.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authentication/TestAll.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authentication/TestAll.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authentication/TestAll.java Wed Nov 25 14:04: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/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractACLTemplateTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractACLTemplateTest.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractACLTemplateTest.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractACLTemplateTest.java Wed Nov 25 14:04:38 2009
@@ -66,6 +66,8 @@
 
     protected abstract JackrabbitAccessControlList createEmptyTemplate(String path) throws RepositoryException;
 
+    protected abstract Principal getSecondPrincipal() throws Exception;
+
     public void testEmptyTemplate() throws RepositoryException {
         JackrabbitAccessControlList pt = createEmptyTemplate(getTestPath());
 
@@ -260,4 +262,63 @@
             // success
         }
     }
+
+    public void testReorder() throws Exception {
+        Privilege[] read = privilegesFromName(Privilege.JCR_READ);
+        Privilege[] write = privilegesFromName(Privilege.JCR_WRITE);
+
+        Principal p2 = getSecondPrincipal();
+
+        AbstractACLTemplate acl = (AbstractACLTemplate) createEmptyTemplate(getTestPath());
+        acl.addAccessControlEntry(testPrincipal, read);
+        acl.addEntry(testPrincipal, write, false);
+        acl.addAccessControlEntry(p2, write);
+
+        AccessControlEntry[] entries = acl.getAccessControlEntries();
+
+        assertEquals(3, entries.length);
+        AccessControlEntry aReadTP = entries[0];
+        AccessControlEntry dWriteTP = entries[1];
+        AccessControlEntry aWriteP2 = entries[2];
+
+        // reorder aWriteP2 to the first position
+        acl.orderBefore(aWriteP2, aReadTP);
+        assertEquals(0, acl.getEntries().indexOf(aWriteP2));
+        assertEquals(1, acl.getEntries().indexOf(aReadTP));
+        assertEquals(2, acl.getEntries().indexOf(dWriteTP));
+
+        // reorder aReadTP to the end of the list
+        acl.orderBefore(aReadTP, null);
+        assertEquals(0, acl.getEntries().indexOf(aWriteP2));
+        assertEquals(1, acl.getEntries().indexOf(dWriteTP));
+        assertEquals(2, acl.getEntries().indexOf(aReadTP));
+    }
+
+    public void testReorderInvalidElements() throws Exception {
+        Privilege[] read = privilegesFromName(Privilege.JCR_READ);
+        Privilege[] write = privilegesFromName(Privilege.JCR_WRITE);
+
+        Principal p2 = getSecondPrincipal();
+
+        AbstractACLTemplate acl = (AbstractACLTemplate) createEmptyTemplate(getTestPath());
+        acl.addAccessControlEntry(testPrincipal, read);
+        acl.addAccessControlEntry(p2, write);
+
+        AbstractACLTemplate acl2 = (AbstractACLTemplate) createEmptyTemplate(getTestPath());
+        acl2.addEntry(testPrincipal, write, false);
+
+        AccessControlEntry invalid = acl2.getEntries().get(0);
+        try {
+            acl.orderBefore(invalid, acl.getEntries().get(0));
+            fail("src entry not contained in list -> reorder should fail.");
+        } catch (AccessControlException e) {
+            // success
+        }
+        try {
+            acl.orderBefore(acl.getEntries().get(0), invalid);
+            fail("dest entry not contained in list -> reorder should fail.");
+        } catch (AccessControlException e) {
+            // success
+        }
+    }
 }
\ No newline at end of file

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractEvaluationTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractEvaluationTest.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractEvaluationTest.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractEvaluationTest.java Wed Nov 25 14:04:38 2009
@@ -20,6 +20,7 @@
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlList;
 import org.apache.jackrabbit.api.security.user.User;
 import org.apache.jackrabbit.api.security.user.UserManager;
+import org.apache.jackrabbit.api.security.user.Authorizable;
 import org.apache.jackrabbit.test.NotExecutableException;
 import org.apache.jackrabbit.test.api.security.AbstractAccessControlTest;
 import org.slf4j.Logger;
@@ -45,6 +46,7 @@
 
     private static Logger log = LoggerFactory.getLogger(AbstractEvaluationTest.class);
 
+    private String uid;
     protected User testUser;
     protected Credentials creds;
     
@@ -63,10 +65,13 @@
         try {
             UserManager uMgr = getUserManager(superuser);
             // create the testUser
-            String uid = "testUser" + UUID.randomUUID();
+            uid = "testUser" + UUID.randomUUID();
             creds = new SimpleCredentials(uid, uid.toCharArray());
 
             testUser = uMgr.createUser(uid, uid);
+            if (!uMgr.isAutoSave()) {
+                superuser.save();
+            }
         } catch (Exception e) {
             superuser.logout();
             throw e;
@@ -90,8 +95,14 @@
         if (testSession != null && testSession.isLive()) {
             testSession.logout();
         }
-        if (testUser != null) {
-            testUser.remove();
+        if (uid != null) {
+            Authorizable a = getUserManager(superuser).getAuthorizable(uid);
+            if (a != null) {
+                a.remove();
+                if (!getUserManager(superuser).isAutoSave()) {
+                    superuser.save();
+                }
+            }
         }
         super.tearDown();
     }
@@ -151,7 +162,7 @@
         acMgr.setPolicy(tmpl.getPath(), tmpl);
         superuser.save();
 
-        // remember for clean up during teardown
+        // remember for clean up during tearDown
         toClear.add(tmpl.getPath());
         return tmpl;
     }

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractVersionManagementTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractVersionManagementTest.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractVersionManagementTest.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractVersionManagementTest.java Wed Nov 25 14:04:38 2009
@@ -75,7 +75,7 @@
         modifyPrivileges(trn.getPath(), Privilege.JCR_VERSION_MANAGEMENT, true);
 
         Node n = createVersionableNode(trn);
-        Version v = n.checkin();
+        n.checkin();
         n.checkout();
     }
 

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractWriteTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractWriteTest.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractWriteTest.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractWriteTest.java Wed Nov 25 14:04:38 2009
@@ -18,7 +18,7 @@
 
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlList;
 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.security.TestPrincipal;
 import org.apache.jackrabbit.test.JUnitTest;
 import org.apache.jackrabbit.test.NotExecutableException;
@@ -86,24 +86,29 @@
     }
 
     protected void tearDown() throws Exception {
-        // make sure all ac info is removed
-        if (testGroup != null && testUser != null) {
-            testGroup.removeMember(testUser);
-            testGroup.remove();
+        try {
+            if (testGroup != null && testUser != null) {
+                testGroup.removeMember(testUser);
+                testGroup.remove();
+                if (!getUserManager(superuser).isAutoSave() && superuser.hasPendingChanges()) {
+                    superuser.save();
+                }
+            }
+        } finally {
+            super.tearDown();
         }
-        super.tearDown();
-    }
-
-    protected User getTestUser() {
-        return testUser;
     }
 
     protected Group getTestGroup() throws RepositoryException, NotExecutableException {
         if (testGroup == null) {
             // create the testGroup
             Principal principal = new TestPrincipal("testGroup" + UUID.randomUUID());
-            testGroup = getUserManager(superuser).createGroup(principal);
+            UserManager umgr = getUserManager(superuser);
+            testGroup = umgr.createGroup(principal);
             testGroup.addMember(testUser);
+            if (!umgr.isAutoSave() && superuser.hasPendingChanges()) {
+                superuser.save();
+            }
         }
         return testGroup;
     }
@@ -945,6 +950,9 @@
 
         // remove the test user
         testUser.remove();
+        if (!getUserManager(superuser).isAutoSave() && superuser.hasPendingChanges()) {
+            superuser.save();
+        }
         testUser = null;
 
         // try to retrieve the acl again
@@ -983,6 +991,51 @@
         assertTrue(acMgr.hasPrivileges(path, remainingprivs.toArray(new Privilege[remainingprivs.size()])));
     }
 
+    public void testReorder() throws RepositoryException, NotExecutableException {
+        Session testSession = getTestSession();
+        Node n = testSession.getNode(path);
+        try {
+            if (!n.getPrimaryNodeType().hasOrderableChildNodes()) {
+                throw new NotExecutableException("Reordering child nodes is not supported..");
+            }
+
+            n.orderBefore(Text.getName(childNPath), Text.getName(childNPath2));
+            testSession.save();
+            fail("test session must not be allowed to reorder nodes.");
+        } catch (AccessDeniedException e) {
+            // success.
+        }
+
+        // give 'add_child_nodes' and 'nt-management' privilege
+        // -> not sufficient privileges for a reorder
+        givePrivileges(path, privilegesFromNames(new String[] {Privilege.JCR_ADD_CHILD_NODES, Privilege.JCR_NODE_TYPE_MANAGEMENT}), getRestrictions(superuser, path));
+        try {
+            n.orderBefore(Text.getName(childNPath), Text.getName(childNPath2));
+            testSession.save();
+            fail("test session must not be allowed to reorder nodes.");
+        } catch (AccessDeniedException e) {
+            // success.
+        }
+
+        // add 'remove_child_nodes' at 'path
+        // -> not sufficient for a reorder since 'remove_node' privilege is missing
+        //    on the target
+        givePrivileges(path, privilegesFromName(Privilege.JCR_REMOVE_CHILD_NODES), getRestrictions(superuser, path));
+        try {
+            n.orderBefore(Text.getName(childNPath), Text.getName(childNPath2));
+            testSession.save();
+            fail("test session must not be allowed to reorder nodes.");
+        } catch (AccessDeniedException e) {
+            // success.
+        }
+
+        // allow 'remove_node' at childNPath
+        // -> now reorder must succeed
+        givePrivileges(childNPath, privilegesFromName(Privilege.JCR_REMOVE_NODE), getRestrictions(superuser, childNPath));
+        n.orderBefore(Text.getName(childNPath), Text.getName(childNPath2));
+        testSession.save();
+    }
+
     private static Node findPolicyNode(Node start) throws RepositoryException {
         Node policyNode = null;
         if (start.isNodeType("rep:Policy")) {

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplateTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplateTest.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplateTest.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplateTest.java Wed Nov 25 14:04:38 2009
@@ -18,11 +18,13 @@
 
 import org.apache.jackrabbit.api.JackrabbitSession;
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlList;
+import org.apache.jackrabbit.api.security.JackrabbitAccessControlEntry;
 import org.apache.jackrabbit.api.security.principal.PrincipalIterator;
 import org.apache.jackrabbit.api.security.principal.PrincipalManager;
 import org.apache.jackrabbit.core.SessionImpl;
 import org.apache.jackrabbit.core.security.authorization.AbstractACLTemplateTest;
 import org.apache.jackrabbit.core.security.authorization.PrivilegeRegistry;
+import org.apache.jackrabbit.core.security.principal.PrincipalImpl;
 import org.apache.jackrabbit.test.NotExecutableException;
 
 import javax.jcr.RepositoryException;
@@ -32,12 +34,15 @@
 import java.security.Principal;
 import java.security.acl.Group;
 import java.util.Collections;
+import java.util.Map;
 
 /**
  * <code>ACLTemplateTest</code>...
  */
 public class ACLTemplateTest extends AbstractACLTemplateTest {
 
+    private Map<String, Value> restrictions = Collections.<String, Value>emptyMap();
+
     protected String getTestPath() {
         return "/ab/c/d";
     }
@@ -49,10 +54,14 @@
         return new ACLTemplate(path, princicipalMgr, privilegeRegistry, sImpl.getValueFactory());
     }
 
+    protected Principal getSecondPrincipal() throws Exception {
+        return pMgr.getEveryone();
+    }
+
     public void testMultipleEntryEffect() throws RepositoryException, NotExecutableException {
         JackrabbitAccessControlList pt = createEmptyTemplate(getTestPath());
         Privilege[] privileges = privilegesFromName(Privilege.JCR_READ);
-        pt.addEntry(testPrincipal, privileges, true, Collections.<String, Value>emptyMap());
+        pt.addEntry(testPrincipal, privileges, true, restrictions);
 
         // new entry extends privileges.
         privileges = privilegesFromNames(new String[] {
@@ -60,7 +69,7 @@
                 Privilege.JCR_ADD_CHILD_NODES});
         assertTrue(pt.addEntry(testPrincipal,
                 privileges,
-                true, Collections.<String, Value>emptyMap()));
+                true, restrictions));
 
         // net-effect: only a single allow-entry with both privileges
         assertTrue(pt.size() == 1);
@@ -68,14 +77,14 @@
 
         // adding just ADD_CHILD_NODES -> must not remove READ privilege
         Privilege[] achPrivs = privilegesFromName(Privilege.JCR_ADD_CHILD_NODES);
-        assertFalse(pt.addEntry(testPrincipal, achPrivs, true, Collections.<String, Value>emptyMap()));
+        assertFalse(pt.addEntry(testPrincipal, achPrivs, true, restrictions));
         // net-effect: only a single allow-entry with add_child_nodes + read privilege
         assertTrue(pt.size() == 1);
         assertSamePrivileges(privileges, pt.getAccessControlEntries()[0].getPrivileges());
 
         // revoke the 'READ' privilege
         privileges = privilegesFromName(Privilege.JCR_READ);
-        assertTrue(pt.addEntry(testPrincipal, privileges, false, Collections.<String, Value>emptyMap()));
+        assertTrue(pt.addEntry(testPrincipal, privileges, false, restrictions));
         // net-effect: 2 entries one allowing ADD_CHILD_NODES, the other denying READ
         assertTrue(pt.size() == 2);
         assertSamePrivileges(privilegesFromName(Privilege.JCR_ADD_CHILD_NODES),
@@ -160,13 +169,113 @@
         JackrabbitAccessControlList pt = createEmptyTemplate(getTestPath());
         Privilege[] privileges = privilegesFromName(Privilege.JCR_READ);
 
-        pt.addEntry(testPrincipal, privileges, true, Collections.<String, Value>emptyMap());
+        pt.addEntry(testPrincipal, privileges, true, restrictions);
 
         // same entry but with revers 'isAllow' flag
-        assertTrue(pt.addEntry(testPrincipal, privileges, false, Collections.<String, Value>emptyMap()));
+        assertTrue(pt.addEntry(testPrincipal, privileges, false, restrictions));
 
         // net-effect: only a single deny-read entry
-        assertTrue(pt.size() == 1);
+        assertEquals(1, pt.size());
         assertSamePrivileges(privileges, pt.getAccessControlEntries()[0].getPrivileges());
     }
+
+    public void testUpdateEntry() throws RepositoryException, NotExecutableException {
+        JackrabbitAccessControlList pt = createEmptyTemplate(getTestPath());
+
+        Privilege[] readPriv = privilegesFromName(Privilege.JCR_READ);
+        Privilege[] writePriv = privilegesFromName(Privilege.JCR_WRITE);
+
+        Principal principal2 = pMgr.getEveryone();
+
+        pt.addEntry(testPrincipal, readPriv, true, restrictions);
+        pt.addEntry(principal2, readPriv, true, restrictions);
+        pt.addEntry(testPrincipal, writePriv, false, restrictions);
+
+        // adding an entry that should update the existing allow-entry for everyone.
+        pt.addEntry(principal2, writePriv, true, restrictions);
+
+        AccessControlEntry[] entries = pt.getAccessControlEntries();
+        assertEquals(3, entries.length);
+        JackrabbitAccessControlEntry princ2AllowEntry = (JackrabbitAccessControlEntry) entries[1];
+        assertEquals(principal2, princ2AllowEntry.getPrincipal());
+        assertTrue(princ2AllowEntry.isAllow());
+        assertSamePrivileges(new Privilege[] {readPriv[0], writePriv[0]}, princ2AllowEntry.getPrivileges());
+    }
+
+    public void testUpdateComplementaryEntry() throws RepositoryException, NotExecutableException {
+        JackrabbitAccessControlList pt = createEmptyTemplate(getTestPath());
+
+        Privilege[] readPriv = privilegesFromName(Privilege.JCR_READ);
+        Privilege[] writePriv = privilegesFromName(Privilege.JCR_WRITE);
+        Principal principal2 = pMgr.getEveryone();
+
+        pt.addEntry(testPrincipal, readPriv, true, restrictions);
+        pt.addEntry(principal2, readPriv, true, restrictions);
+        pt.addEntry(testPrincipal, writePriv, false, restrictions);
+        pt.addEntry(principal2, writePriv, true, restrictions);
+        // entry complementary to the first entry
+        // -> must remove the allow-READ entry and update the deny-WRITE entry.
+        pt.addEntry(testPrincipal, readPriv, false, restrictions);
+
+        AccessControlEntry[] entries = pt.getAccessControlEntries();
+
+        assertEquals(2, entries.length);
+
+        JackrabbitAccessControlEntry first = (JackrabbitAccessControlEntry) entries[0];
+        assertEquals(principal2, first.getPrincipal());
+
+        JackrabbitAccessControlEntry second = (JackrabbitAccessControlEntry) entries[1];
+        assertEquals(testPrincipal, second.getPrincipal());
+        assertFalse(second.isAllow());
+        assertSamePrivileges(new Privilege[] {readPriv[0], writePriv[0]}, second.getPrivileges());
+    }
+
+    public void testTwoEntriesPerPrincipal() throws RepositoryException, NotExecutableException {
+        JackrabbitAccessControlList pt = createEmptyTemplate(getTestPath());
+
+        Privilege[] readPriv = privilegesFromName(Privilege.JCR_READ);
+        Privilege[] writePriv = privilegesFromName(Privilege.JCR_WRITE);
+        Privilege[] acReadPriv = privilegesFromName(Privilege.JCR_READ_ACCESS_CONTROL);
+
+        pt.addEntry(testPrincipal, readPriv, true, restrictions);
+        pt.addEntry(testPrincipal, writePriv, true, restrictions);
+        pt.addEntry(testPrincipal, acReadPriv, true, restrictions);
+
+        pt.addEntry(testPrincipal, readPriv, false, restrictions);
+        pt.addEntry(new PrincipalImpl(testPrincipal.getName()), readPriv, false, restrictions);
+        pt.addEntry(new Principal() {
+            public String getName() {
+                return testPrincipal.getName();
+            }
+        }, readPriv, false, restrictions);
+
+        AccessControlEntry[] entries = pt.getAccessControlEntries();
+        assertEquals(2, entries.length);
+    }
+
+    /**
+     * Test if new entries get appended at the end of the list.
+     *
+     * @throws RepositoryException
+     * @throws NotExecutableException
+     */
+    public void testNewEntriesAppendedAtEnd() throws RepositoryException, NotExecutableException {
+        JackrabbitAccessControlList pt = createEmptyTemplate(getTestPath());
+
+        Privilege[] readPriv = privilegesFromName(Privilege.JCR_READ);
+        Privilege[] writePriv = privilegesFromName(Privilege.JCR_WRITE);
+
+        pt.addEntry(testPrincipal, readPriv, true, restrictions);
+        pt.addEntry(pMgr.getEveryone(), readPriv, true, restrictions);
+        pt.addEntry(testPrincipal, writePriv, false, restrictions);
+
+        AccessControlEntry[] entries = pt.getAccessControlEntries();
+
+        assertEquals(3, entries.length);
+
+        JackrabbitAccessControlEntry last = (JackrabbitAccessControlEntry) entries[2];
+        assertEquals(testPrincipal, last.getPrincipal());
+        assertEquals(false, last.isAllow());
+        assertEquals(writePriv[0], last.getPrivileges()[0]);
+    }
 }
\ No newline at end of file

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java Wed Nov 25 14:04:38 2009
@@ -17,10 +17,14 @@
 package org.apache.jackrabbit.core.security.authorization.acl;
 
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlList;
+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.authorization.AbstractWriteTest;
 import org.apache.jackrabbit.core.security.authorization.AccessControlConstants;
 import org.apache.jackrabbit.core.security.authorization.PrivilegeRegistry;
+import org.apache.jackrabbit.core.security.principal.EveryonePrincipal;
+import org.apache.jackrabbit.core.security.TestPrincipal;
 import org.apache.jackrabbit.test.NotExecutableException;
 
 import javax.jcr.AccessDeniedException;
@@ -35,6 +39,7 @@
 import java.security.Principal;
 import java.util.Collections;
 import java.util.Map;
+import java.util.UUID;
 
 /**
  * <code>EvaluationTest</code>...
@@ -109,7 +114,7 @@
         // test if testuser can modify AC-items
         // 1) add an ac-entry
         ACLTemplate acl = (ACLTemplate) policies[0];
-        acl.addAccessControlEntry(getTestUser().getPrincipal(), privilegesFromName(PrivilegeRegistry.REP_WRITE));
+        acl.addAccessControlEntry(testUser.getPrincipal(), privilegesFromName(PrivilegeRegistry.REP_WRITE));
         testAcMgr.setPolicy(path, acl);
         testSession.save();
 
@@ -214,4 +219,123 @@
         // remove_child_node privilege is missing on the direct ancestor.
         assertFalse(testSession.hasPermission(gcPath, Session.ACTION_REMOVE));
     }
+
+    public void testInheritedGroupPermissions() throws NotExecutableException, RepositoryException {
+        Group testGroup = getTestGroup();
+        AccessControlManager testAcMgr = getTestACManager();
+        /*
+         precondition:
+         testuser must have READ-only permission on test-node and below
+        */
+        checkReadOnly(path);
+
+        Privilege[] privileges = privilegesFromName(Privilege.JCR_MODIFY_PROPERTIES);
+
+        /* give MODIFY_PROPERTIES privilege for testGroup at 'path' */
+        givePrivileges(path, testGroup.getPrincipal(), privileges, getRestrictions(superuser, path));
+        /*
+         withdraw MODIFY_PROPERTIES privilege for everyone at 'childNPath'
+         */
+        withdrawPrivileges(childNPath, EveryonePrincipal.getInstance(), privileges, getRestrictions(superuser, path));
+
+        // result at 'child path' must be deny
+        assertFalse(testAcMgr.hasPrivileges(childNPath, privilegesFromName(Privilege.JCR_MODIFY_PROPERTIES)));    
+    }
+
+    public void testInheritedGroupPermissions2() throws NotExecutableException, RepositoryException {
+        Group testGroup = getTestGroup();
+        AccessControlManager testAcMgr = getTestACManager();
+        /*
+         precondition:
+         testuser must have READ-only permission on test-node and below
+        */
+        checkReadOnly(path);
+
+        Privilege[] privileges = privilegesFromName(Privilege.JCR_MODIFY_PROPERTIES);
+
+        // NOTE: same as testInheritedGroupPermissions above but using
+        // everyone on path, testgroup on childpath -> result must be the same
+
+        /* give MODIFY_PROPERTIES privilege for everyone at 'path' */
+        givePrivileges(path, EveryonePrincipal.getInstance(), privileges, getRestrictions(superuser, path));
+        /*
+         withdraw MODIFY_PROPERTIES privilege for testGroup at 'childNPath'
+         */
+        withdrawPrivileges(childNPath, testGroup.getPrincipal(), privileges, getRestrictions(superuser, path));
+
+        // result at 'child path' must be deny
+        assertFalse(testAcMgr.hasPrivileges(childNPath, privilegesFromName(Privilege.JCR_MODIFY_PROPERTIES)));
+    }
+
+        public void testMultipleGroupPermissionsOnNode() throws NotExecutableException, RepositoryException {
+        Group testGroup = getTestGroup();
+
+        /* create a second group the test user is member of */
+        Principal principal = new TestPrincipal("testGroup" + UUID.randomUUID());
+        UserManager umgr = getUserManager(superuser);
+        Group group2 = umgr.createGroup(principal);
+        try {
+            group2.addMember(testUser);
+            if (!umgr.isAutoSave() && superuser.hasPendingChanges()) {
+                superuser.save();
+            }
+
+            /* add privileges for the Group the test-user is member of */
+            Privilege[] privileges = privilegesFromName(Privilege.JCR_MODIFY_PROPERTIES);
+            givePrivileges(path, testGroup.getPrincipal(), privileges, getRestrictions(superuser, path));
+
+            withdrawPrivileges(path, group2.getPrincipal(), privileges, getRestrictions(superuser, path));
+
+            /*
+             testuser must get the permissions/privileges inherited from
+             the group it is member of.
+             the denial of group2 must succeed
+            */
+            String actions = javax.jcr.Session.ACTION_SET_PROPERTY + "," + javax.jcr.Session.ACTION_READ;
+
+            AccessControlManager testAcMgr = getTestACManager();
+
+            assertFalse(getTestSession().hasPermission(path, actions));
+            Privilege[] privs = privilegesFromName(Privilege.JCR_MODIFY_PROPERTIES);
+            assertFalse(testAcMgr.hasPrivileges(path, privs));
+        } finally {
+            group2.remove();
+        }
+    }
+
+    public void testMultipleGroupPermissionsOnNode2() throws NotExecutableException, RepositoryException {
+        Group testGroup = getTestGroup();
+
+        /* create a second group the test user is member of */
+        Principal principal = new TestPrincipal("testGroup" + UUID.randomUUID());
+        UserManager umgr = getUserManager(superuser);
+        Group group2 = umgr.createGroup(principal);
+
+        try {
+            group2.addMember(testUser);
+            if (!umgr.isAutoSave() && superuser.hasPendingChanges()) {
+                superuser.save();
+            }
+
+            /* add privileges for the Group the test-user is member of */
+            Privilege[] privileges = privilegesFromName(Privilege.JCR_MODIFY_PROPERTIES);
+            withdrawPrivileges(path, testGroup.getPrincipal(), privileges, getRestrictions(superuser, path));
+
+            givePrivileges(path, group2.getPrincipal(), privileges, getRestrictions(superuser, path));
+
+            /*
+             testuser must get the permissions/privileges inherited from
+             the group it is member of.
+             the denial of group2 must succeed
+            */
+            String actions = javax.jcr.Session.ACTION_SET_PROPERTY + "," + javax.jcr.Session.ACTION_READ;
+
+            AccessControlManager testAcMgr = getTestACManager();
+            assertTrue(getTestSession().hasPermission(path, actions));
+            Privilege[] privs = privilegesFromName(Privilege.JCR_MODIFY_PROPERTIES);
+            assertTrue(testAcMgr.hasPrivileges(path, privs));
+        } finally {
+            group2.remove();
+        }
+    }
 }
\ No newline at end of file

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/combined/WriteTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/combined/WriteTest.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/combined/WriteTest.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/combined/WriteTest.java Wed Nov 25 14:04:38 2009
@@ -49,7 +49,7 @@
 
         // simple test to check if proper provider is present:
         try {
-            getPrincipalBasedPolicy(acMgr, path, getTestUser().getPrincipal());
+            getPrincipalBasedPolicy(acMgr, path, testUser.getPrincipal());
         } catch (Exception e) {
             superuser.logout();
             throw e;
@@ -135,7 +135,7 @@
         assertFalse(testAcMgr.hasPrivileges(path, readPrivs));
 
         // remove the nodebased policy
-        JackrabbitAccessControlList policy = getPolicy(acMgr, path, getTestUser().getPrincipal());
+        JackrabbitAccessControlList policy = getPolicy(acMgr, path, testUser.getPrincipal());
         acMgr.removePolicy(policy.getPath(), policy);
         superuser.save();
 
@@ -152,7 +152,7 @@
         givePrivileges(path, wrtPrivileges, getRestrictions(superuser, path));
         // userbased: deny MODIFY_PROPERTIES privileges for 'testUser'
         Privilege[] modPropPrivs = privilegesFromName(Privilege.JCR_MODIFY_PROPERTIES);
-        withdrawPrivileges(path, getTestUser().getPrincipal(), modPropPrivs, getPrincipalBasedRestrictions(path), false);
+        withdrawPrivileges(path, testUser.getPrincipal(), modPropPrivs, getPrincipalBasedRestrictions(path), false);
         /*
          expected result:
          - MODIFY_PROPERTIES privilege still present
@@ -162,7 +162,7 @@
 
         // nodebased: deny MODIFY_PROPERTIES privileges for 'testUser'
         //            on a child node.
-        withdrawPrivileges(childNPath, getTestUser().getPrincipal(), modPropPrivs, getRestrictions(superuser, childNPath));
+        withdrawPrivileges(childNPath, testUser.getPrincipal(), modPropPrivs, getRestrictions(superuser, childNPath));
         /*
          expected result:
          - MODIFY_PROPERTIES privilege still present at 'path'

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLTemplateTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLTemplateTest.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLTemplateTest.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/principalbased/ACLTemplateTest.java Wed Nov 25 14:04:38 2009
@@ -25,6 +25,7 @@
 import javax.jcr.RepositoryException;
 import java.util.Arrays;
 import java.util.List;
+import java.security.Principal;
 
 /**
  * <code>ACLTemplateTest</code>...
@@ -42,6 +43,10 @@
         return new ACLTemplate(testPrincipal, testPath, (SessionImpl) superuser, superuser.getValueFactory());
     }
 
+    protected Principal getSecondPrincipal() throws Exception {
+        return testPrincipal;
+    }
+
     public void testGetRestrictionNames() throws RepositoryException {
         List names = Arrays.asList(createEmptyTemplate(getTestPath()).getRestrictionNames());
 

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/principalbased/WriteTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/principalbased/WriteTest.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/principalbased/WriteTest.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/principalbased/WriteTest.java Wed Nov 25 14:04:38 2009
@@ -73,10 +73,14 @@
     }
 
     public void testEditor() throws NotExecutableException, RepositoryException {
+        UserManager uMgr = getUserManager(superuser);        
         User u = null;
         try {
-            UserManager uMgr = getUserManager(superuser);
             u = uMgr.createUser("t", "t");
+            if (!uMgr.isAutoSave()) {
+                superuser.save();
+            }
+
             Principal p = u.getPrincipal();
 
             JackrabbitAccessControlManager acMgr = (JackrabbitAccessControlManager) getAccessControlManager(superuser);
@@ -103,19 +107,23 @@
             superuser.refresh(false);
             if (u != null) {
                 u.remove();
+                if (!uMgr.isAutoSave()) {
+                    superuser.save();
+                }
             }
         }
     }
 
     public void testEditor2() throws NotExecutableException, RepositoryException {
+        UserManager uMgr = getUserManager(superuser);
         User u = null;
         User u2 = null;
-
         try {
-            UserManager uMgr = getUserManager(superuser);
-
             u = uMgr.createUser("t", "t");
             u2 = uMgr.createUser("tt", "tt", new TestPrincipal("tt"), "t/tt");
+            if (!uMgr.isAutoSave()) {
+                superuser.save();
+            }
 
             Principal p = u.getPrincipal();
             Principal p2 = u2.getPrincipal();
@@ -140,7 +148,10 @@
             superuser.refresh(false);
             if (u2 != null) u2.remove();
             if (u != null) u.remove();
-     }
+            if (!uMgr.isAutoSave()) {
+                superuser.save();
+            }
+        }
 
     }
     // TODO: add specific tests with other restrictions

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProviderTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProviderTest.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProviderTest.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProviderTest.java Wed Nov 25 14:04:38 2009
@@ -64,7 +64,7 @@
         assertFalse(pit.hasNext());
     }
 
-    public void testInheritedMemberShip() throws RepositoryException {
+    public void testInheritedMemberShip() throws RepositoryException, NotExecutableException {
         Principal up = getTestPrincipal();
 
         User u = null;
@@ -74,9 +74,12 @@
             u = userMgr.createUser(up.getName(), buildPassword(up));
             gr1 = userMgr.createGroup(getTestPrincipal());
             gr2 = userMgr.createGroup(getTestPrincipal());
+            save(superuser);
+
 
             gr1.addMember(gr2);
             gr2.addMember(u);
+            save(superuser);
 
             PrincipalIterator it = principalProvider.getGroupMembership(u.getPrincipal());
             while (it.hasNext()) {
@@ -96,6 +99,7 @@
             if (gr1 != null) gr1.remove();
             if (gr2 != null) gr2.remove();
             if (u != null) u.remove();
+            save(superuser);
         }
     }
 }
\ No newline at end of file

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/AdministratorTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/AdministratorTest.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/AdministratorTest.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/AdministratorTest.java Wed Nov 25 14:04:38 2009
@@ -18,33 +18,254 @@
 
 import org.apache.jackrabbit.api.security.user.AbstractUserTest;
 import org.apache.jackrabbit.api.security.user.Authorizable;
+import org.apache.jackrabbit.api.security.user.UserManager;
+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.nodetype.NodeTypeImpl;
+import org.apache.jackrabbit.core.id.NodeId;
 import org.apache.jackrabbit.core.security.principal.AdminPrincipal;
 import org.apache.jackrabbit.test.NotExecutableException;
+import org.apache.jackrabbit.spi.commons.conversion.NameResolver;
+import org.apache.jackrabbit.spi.Name;
 
+import javax.jcr.Node;
 import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import java.util.Properties;
 
 /**
  * <code>AdministratorTest</code>...
  */
 public class AdministratorTest extends AbstractUserTest {
 
+    private String adminId;
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+
+        if (!(userMgr instanceof UserManagerImpl)) {
+            throw new NotExecutableException();
+        }
+        adminId = superuser.getUserID();
+        if (!((UserManagerImpl) userMgr).isAdminId(adminId)) {
+            throw new NotExecutableException();
+        }
+    }
+
     public void testGetPrincipal() throws RepositoryException {
-        Authorizable authr = userMgr.getAuthorizable(superuser.getUserID());
-        assertNotNull(authr);
-        assertFalse(authr.isGroup());
-        assertTrue(authr.getPrincipal() instanceof AdminPrincipal);
+        Authorizable admin = userMgr.getAuthorizable(adminId);
+        assertNotNull(admin);
+        assertFalse(admin.isGroup());
+        assertTrue(admin.getPrincipal() instanceof AdminPrincipal);
     }
 
     public void testRemoveSelf() throws RepositoryException, NotExecutableException {
-        Authorizable authr = userMgr.getAuthorizable(superuser.getUserID());
-        if (authr == null) {
+        Authorizable admin = userMgr.getAuthorizable(adminId);
+        if (admin == null) {
             throw new NotExecutableException();
         }
         try {
-            authr.remove();
+            admin.remove();
             fail("The Administrator should not be allowed to remove the own authorizable.");
         } catch (RepositoryException e) {
             // success
         }
     }
+
+    /**
+     * Test if the administrator is recreated upon login if the corresponding
+     * node gets removed.
+     *
+     * @throws RepositoryException
+     * @throws NotExecutableException
+     */
+    public void testRemoveAdminNode() throws RepositoryException, NotExecutableException {
+        Authorizable admin = userMgr.getAuthorizable(adminId);
+
+        if (admin == null || !(admin instanceof AuthorizableImpl)) {
+            throw new NotExecutableException();
+        }
+
+        // access the node corresponding to the admin user and remove it
+        NodeImpl adminNode = ((AuthorizableImpl) admin).getNode();
+        Session s = adminNode.getSession();
+        adminNode.remove();
+        // use session obtained from the node as usermgr may point to a dedicated
+        // system workspace different from the superusers workspace.
+        s.save();
+
+        // after removing the node the admin user doesn't exist any more
+        assertNull(userMgr.getAuthorizable(adminId));
+
+        // login must succeed as system user mgr recreates the admin user
+        Session s2 = getHelper().getSuperuserSession();
+        try {
+            admin = userMgr.getAuthorizable(adminId);
+            assertNotNull(admin);
+            assertNotNull(getUserManager(s2).getAuthorizable(adminId));
+        } finally {
+            s2.logout();
+        }
+    }
+
+    /**
+     * Test for collisions that would prevent from recreate the admin user.
+     *
+     * @throws RepositoryException
+     * @throws NotExecutableException
+     */
+    public void testAdminNodeCollidingWithAuthorizableFolder() throws RepositoryException, NotExecutableException {
+        Authorizable admin = userMgr.getAuthorizable(adminId);
+
+        if (admin == null || !(admin instanceof AuthorizableImpl)) {
+            throw new NotExecutableException();
+        }
+
+        // access the node corresponding to the admin user and remove it
+        NodeImpl adminNode = ((AuthorizableImpl) admin).getNode();
+        String adminPath = adminNode.getPath();
+        String adminNodeName = adminNode.getName();
+        Node parentNode = adminNode.getParent();
+
+        Session s = adminNode.getSession();
+        adminNode.remove();
+        // use session obtained from the node as usermgr may point to a dedicated
+        // system workspace different from the superusers workspace.
+        s.save();
+
+        Session s2 = null;
+        try {
+            // now create a colliding node:
+            parentNode.addNode(adminNodeName, "rep:AuthorizableFolder");
+            s.save();
+
+            // force recreation of admin user.
+            s2 = getHelper().getSuperuserSession();
+
+            admin = userMgr.getAuthorizable(adminId);
+            assertNotNull(admin);
+            assertEquals(adminNodeName, ((AuthorizableImpl) admin).getNode().getName());
+            assertFalse(adminPath.equals(((AuthorizableImpl) admin).getNode().getPath()));
+
+        } finally {
+            if (s2 == null) {
+                // something went wrong -> remove the folder again.
+                parentNode.remove();
+                s.save();
+                // force recreation of admin user.
+                s2 = getHelper().getSuperuserSession();
+            }
+            if (s2 != null) {
+                s2.logout();
+            }
+        }
+    }
+
+    /**
+     * Test if creation of the administrator user forces the removal of some
+     * other node in the repository that by change happens to have the same
+     * jcr:uuid and thus inhibits the creation of the admininstrator user.
+     */
+    public void testAdminNodeCollidingWithRandomNode() throws RepositoryException, NotExecutableException {
+        Authorizable admin = userMgr.getAuthorizable(adminId);
+
+        if (admin == null || !(admin instanceof AuthorizableImpl)) {
+            throw new NotExecutableException();
+        }
+
+        // access the node corresponding to the admin user and remove it
+        NodeImpl adminNode = ((AuthorizableImpl) admin).getNode();
+        NodeId nid = adminNode.getNodeId();
+        String adminPath = adminNode.getPath();
+
+        Session s = adminNode.getSession();
+        adminNode.remove();
+        // use session obtained from the node as usermgr may point to a dedicated
+        // system workspace different from the superusers workspace.
+        s.save();
+
+        Session s2 = null;
+        String collidingPath = null;
+        try {
+            // create a colliding node outside of the user tree
+            NameResolver nr = (SessionImpl) s;
+            NodeImpl tr = (NodeImpl) s.getRootNode();
+            Node n = tr.addNode(nr.getQName(nodeName1), nr.getQName(testNodeType), nid);
+            collidingPath = n.getPath();
+            s.save();
+
+            // force recreation of admin user.
+            s2 = getHelper().getSuperuserSession();
+
+            admin = userMgr.getAuthorizable(adminId);
+            assertNotNull(admin);
+            // the colliding node must have been removed.
+            assertFalse(s2.nodeExists(collidingPath));
+
+        } finally {
+            if (s2 != null) {
+                s2.logout();
+            }
+            if (collidingPath != null && s.nodeExists(collidingPath)) {
+                s.getNode(collidingPath).remove();
+                s.save();
+            }
+        }
+    }
+
+    /**
+     * Reconfiguration of the user-root-path will result in node collision
+     * upon initialization of the built-in repository users. Test if the
+     * UserManagerImpl in this case removes the colliding admin-user node.
+     */
+    public void testChangeUserRootPath() throws RepositoryException, NotExecutableException {
+        Authorizable admin = userMgr.getAuthorizable(adminId);
+
+        if (admin == null || !(admin instanceof AuthorizableImpl)) {
+            throw new NotExecutableException();
+        }
+
+        // access the node corresponding to the admin user and remove it
+        NodeImpl adminNode = ((AuthorizableImpl) admin).getNode();
+        NodeId nid = adminNode.getNodeId();
+        Name nName = adminNode.getQName();
+        Name ntName = ((NodeTypeImpl) adminNode.getPrimaryNodeType()).getQName();
+
+        Session s = adminNode.getSession();
+        adminNode.remove();
+        // use session obtained from the node as usermgr may point to a dedicated
+        // system workspace different from the superusers workspace.
+        s.save();
+
+        Session s2 = null;
+        String collidingPath = null;
+        try {
+            // create a colliding user node outside of the user tree
+            Properties props = new Properties();
+            props.setProperty("usersPath", "/testPath");
+            UserManager um = new UserManagerImpl((SessionImpl) s, adminId, props);
+            User collidingUser = um.createUser(adminId, adminId);
+            collidingPath = ((AuthorizableImpl) collidingUser).getNode().getPath();
+            s.save();
+
+            // force recreation of admin user.
+            s2 = getHelper().getSuperuserSession();
+
+            admin = userMgr.getAuthorizable(adminId);
+            assertNotNull(admin);
+            // the colliding node must have been removed.
+            assertFalse(s2.nodeExists(collidingPath));
+
+        } finally {
+            if (s2 != null) {
+                s2.logout();
+            }
+            if (collidingPath != null && s.nodeExists(collidingPath)) {
+                s.getNode(collidingPath).remove();
+                s.save();
+            }
+        }
+    }
 }

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/AuthorizableImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/AuthorizableImplTest.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/AuthorizableImplTest.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/AuthorizableImplTest.java Wed Nov 25 14:04:38 2009
@@ -20,34 +20,34 @@
 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.core.SessionImpl;
 import org.apache.jackrabbit.core.PropertyImpl;
+import org.apache.jackrabbit.core.SessionImpl;
 import org.apache.jackrabbit.spi.commons.conversion.NameResolver;
 import org.apache.jackrabbit.test.NotExecutableException;
 import org.apache.jackrabbit.value.StringValue;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import javax.jcr.Property;
+import javax.jcr.PropertyIterator;
 import javax.jcr.RepositoryException;
 import javax.jcr.Value;
-import javax.jcr.PropertyIterator;
+import javax.jcr.PropertyType;
 import javax.jcr.nodetype.ConstraintViolationException;
-import javax.jcr.nodetype.NodeType;
 import java.util.ArrayList;
-import java.util.Iterator;
 import java.util.List;
+import java.util.Iterator;
+import java.util.HashSet;
+import java.util.Set;
+import java.security.Principal;
 
 /**
  * <code>AuthorizableImplTest</code>...
  */
 public class AuthorizableImplTest extends AbstractUserTest {
 
-    private static Logger log = LoggerFactory.getLogger(AuthorizableImplTest.class);
-
-    private List protectedUserProps = new ArrayList();
-    private List protectedGroupProps = new ArrayList();
+    private List<String> protectedUserProps = new ArrayList();
+    private List<String> protectedGroupProps = new ArrayList();
 
     protected void setUp() throws Exception {
         super.setUp();
@@ -55,11 +55,10 @@
         if (superuser instanceof SessionImpl) {
             NameResolver resolver = (SessionImpl) superuser;
             protectedUserProps.add(resolver.getJCRName(UserConstants.P_PASSWORD));
-            protectedUserProps.add(resolver.getJCRName(UserConstants.P_GROUPS));
             protectedUserProps.add(resolver.getJCRName(UserConstants.P_IMPERSONATORS));
             protectedUserProps.add(resolver.getJCRName(UserConstants.P_PRINCIPAL_NAME));
 
-            protectedUserProps.add(resolver.getJCRName(UserConstants.P_GROUPS));
+            protectedUserProps.add(resolver.getJCRName(UserConstants.P_MEMBERS));
             protectedGroupProps.add(resolver.getJCRName(UserConstants.P_PRINCIPAL_NAME));
         } else {
             throw new NotExecutableException();
@@ -85,22 +84,22 @@
         Value v = superuser.getValueFactory().createValue("any_value");
 
         User u = getTestUser(superuser);
-        for (Iterator it = protectedUserProps.iterator(); it.hasNext();) {
-            String pName = it.next().toString();
+        for (String pName : protectedUserProps) {
             try {
                 u.setProperty(pName, v);
-                fail("changing the '" +pName+ "' property on a User should fail.");
+                save(superuser);
+                fail("changing the '" + pName + "' property on a User should fail.");
             } catch (RepositoryException e) {
                 // success
             }
         }
         
         Group g = getTestGroup(superuser);
-        for (Iterator it = protectedGroupProps.iterator(); it.hasNext();) {
-            String pName = it.next().toString();
+        for (String pName : protectedGroupProps) {
             try {
                 g.setProperty(pName, v);
-                fail("changing the '" +pName+ "' property on a Group should fail.");
+                save(superuser);
+                fail("changing the '" + pName + "' property on a Group should fail.");
             } catch (RepositoryException e) {
                 // success
             }
@@ -109,21 +108,21 @@
 
     public void testRemoveSpecialProperties() throws NotExecutableException, RepositoryException {
         User u = getTestUser(superuser);
-        for (Iterator it = protectedUserProps.iterator(); it.hasNext();) {
-            String pName = it.next().toString();
+        for (String pName : protectedUserProps) {
             try {
                 u.removeProperty(pName);
-                fail("removing the '" +pName+ "' property on a User should fail.");
+                save(superuser);
+                fail("removing the '" + pName + "' property on a User should fail.");
             } catch (RepositoryException e) {
                 // success
             }
         }
         Group g = getTestGroup(superuser);
-        for (Iterator it = protectedGroupProps.iterator(); it.hasNext();) {
-            String pName = it.next().toString();
+        for (String pName : protectedGroupProps) {
             try {
                 g.removeProperty(pName);
-                fail("removing the '" +pName+ "' property on a Group should fail.");
+                save(superuser);
+                fail("removing the '" + pName + "' property on a Group should fail.");
             } catch (RepositoryException e) {
                 // success
             }
@@ -135,9 +134,8 @@
         NodeImpl n = user.getNode();
 
         checkProtected(n.getProperty(UserConstants.P_PASSWORD));
-        checkProtected(n.getProperty(UserConstants.P_PRINCIPAL_NAME));
-        if (n.hasProperty(UserConstants.P_GROUPS)) {
-            checkProtected(n.getProperty(UserConstants.P_GROUPS));
+        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));
@@ -148,9 +146,25 @@
         GroupImpl gr = (GroupImpl) getTestGroup(superuser);
         NodeImpl n = gr.getNode();
 
-        checkProtected(n.getProperty(UserConstants.P_PRINCIPAL_NAME));
-        if (n.hasProperty(UserConstants.P_GROUPS)) {
-            checkProtected(n.getProperty(UserConstants.P_GROUPS));
+        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));
+        }
+    }
+
+    public void testMembersPropertyType() throws NotExecutableException, RepositoryException {
+        GroupImpl gr = (GroupImpl) getTestGroup(superuser);
+        NodeImpl n = gr.getNode();
+
+        if (!n.hasProperty(UserConstants.P_MEMBERS)) {
+            gr.addMember(getTestUser(superuser));
+        }
+
+        Property p = n.getProperty(UserConstants.P_MEMBERS);
+        for (Value v : p.getValues()) {
+            assertEquals(PropertyType.WEAKREFERENCE, v.getType());
         }
     }
 
@@ -177,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.
         }
@@ -194,9 +237,7 @@
 
         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())) {
+            if (p.getDefinition().isProtected()) {
                 assertFalse(user.hasProperty(p.getName()));
                 assertNull(user.getProperty(p.getName()));
             } else {
@@ -213,9 +254,7 @@
 
         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())) {
+            if (p.getDefinition().isProtected()) {
                 assertFalse(group.hasProperty(p.getName()));
                 assertNull(group.getProperty(p.getName()));
             } else {
@@ -225,4 +264,111 @@
             }
         }
     }
+
+    public void testSingleToMultiValued() throws Exception {
+        AuthorizableImpl user = (AuthorizableImpl) getTestUser(superuser);
+        UserManager uMgr = getUserManager(superuser);
+        try {
+            Value v = superuser.getValueFactory().createValue("anyValue");
+            user.setProperty("someProp", v);
+            if (!uMgr.isAutoSave()) {
+                superuser.save();
+            }
+            Value[] vs = new Value[] {v, v};
+            user.setProperty("someProp", vs);
+            if (!uMgr.isAutoSave()) {
+                superuser.save();
+            }
+        } finally {
+            if (user.removeProperty("someProp") && !uMgr.isAutoSave()) {
+                superuser.save();
+            }
+        }
+    }
+
+    public void testMultiValuedToSingle() throws Exception {
+        AuthorizableImpl user = (AuthorizableImpl) getTestUser(superuser);
+        UserManager uMgr = getUserManager(superuser);
+        try {
+            Value v = superuser.getValueFactory().createValue("anyValue");
+            Value[] vs = new Value[] {v, v};
+            user.setProperty("someProp", vs);
+            if (!uMgr.isAutoSave()) {
+                superuser.save();
+            }
+            user.setProperty("someProp", v);
+            if (!uMgr.isAutoSave()) {
+                superuser.save();
+            }
+        } finally {
+            if (user.removeProperty("someProp") && !uMgr.isAutoSave()) {
+                superuser.save();
+            }
+        }
+    }
+
+    public void testObjectMethods() throws Exception {
+        final AuthorizableImpl user = (AuthorizableImpl) getTestUser(superuser);
+        AuthorizableImpl user2 = (AuthorizableImpl) getTestUser(superuser);
+
+        assertEquals(user, user2);
+        assertEquals(user.hashCode(), user2.hashCode());
+        Set<Authorizable> s = new HashSet();
+        s.add(user);
+        assertFalse(s.add(user2));
+
+        Authorizable user3 = new Authorizable() {
+
+            public String getID() throws RepositoryException {
+                return user.getID();
+            }
+
+            public boolean isGroup() {
+                return user.isGroup();
+            }
+
+            public Principal getPrincipal() throws RepositoryException {
+                return user.getPrincipal();
+            }
+
+            public Iterator<Group> declaredMemberOf() throws RepositoryException {
+                return user.declaredMemberOf();
+            }
+
+            public Iterator<Group> memberOf() throws RepositoryException {
+                return user.memberOf();
+            }
+
+            public void remove() throws RepositoryException {
+                user.remove();
+            }
+
+            public Iterator<String> getPropertyNames() throws RepositoryException {
+                return user.getPropertyNames();
+            }
+
+            public boolean hasProperty(String name) throws RepositoryException {
+                return user.hasProperty(name);
+            }
+
+            public void setProperty(String name, Value value) throws RepositoryException {
+                user.setProperty(name, value);
+            }
+
+            public void setProperty(String name, Value[] values) throws RepositoryException {
+                user.setProperty(name, values);
+            }
+
+            public Value[] getProperty(String name) throws RepositoryException {
+                return user.getProperty(name);
+            }
+
+            public boolean removeProperty(String name) throws RepositoryException {
+                return user.removeProperty(name);
+            }
+        };
+
+        assertFalse(user.equals(user3));
+        assertTrue(s.add(user3));
+    }
 }
\ No newline at end of file

Modified: jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/GroupAdministratorTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/GroupAdministratorTest.java?rev=884108&r1=884107&r2=884108&view=diff
==============================================================================
--- jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/GroupAdministratorTest.java (original)
+++ jackrabbit/sandbox/JCR-1456/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/GroupAdministratorTest.java Wed Nov 25 14:04:38 2009
@@ -29,6 +29,7 @@
 import javax.jcr.Credentials;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
+import javax.jcr.Node;
 import java.security.Principal;
 import java.util.Iterator;
 
@@ -45,6 +46,7 @@
     private String otherUID2;
     private String grID;
 
+    private String groupsPath;
 
     private Group groupAdmin;
 
@@ -54,6 +56,7 @@
         // create a first user
         Principal p = getTestPrincipal();
         UserImpl pUser = (UserImpl) userMgr.createUser(p.getName(), buildPassword(p));
+        save(superuser);
         otherUID = pUser.getID();
 
         // create a second user and make it group-admin
@@ -61,19 +64,23 @@
         String pw = buildPassword(p);
         Credentials creds = buildCredentials(p.getName(), pw);
         User user = userMgr.createUser(p.getName(), pw);
+        save(superuser);
         uID = user.getID();
 
         // make other user a group-administrator:
         Authorizable grAdmin = userMgr.getAuthorizable(UserConstants.GROUP_ADMIN_GROUP_NAME);
         if (grAdmin == null || !grAdmin.isGroup()) {
-            throw new NotExecutableException("Cannot execute test. Group-Admin name has been changed by config.");
+            throw new NotExecutableException("Cannot execute test. No group-administrator group found.");
         }
         groupAdmin = (Group) grAdmin;
         groupAdmin.addMember(user);
+        save(superuser);
         grID = groupAdmin.getID();
 
-        // create a session for the grou-admin user.
+        // create a session for the group-admin user.
         uSession = getHelper().getRepository().login(creds);
+
+        groupsPath = (userMgr instanceof UserManagerImpl) ? ((UserManagerImpl) userMgr).getGroupsPath() : UserConstants.GROUPS_PATH;
     }
 
     protected void tearDown() throws Exception {
@@ -90,16 +97,17 @@
             if (a != null) {
                 a.remove();
             }
-
+            save(superuser);
         }
         super.tearDown();
     }
 
-    private String getYetAnotherID() throws RepositoryException {
+    private String getYetAnotherID() throws RepositoryException, NotExecutableException {
         if (otherUID2 == null) {
             // create a third user
             Principal p = getTestPrincipal();
             otherUID2 = userMgr.createUser(p.getName(), buildPassword(p)).getID();
+            save(superuser);
         }
         return otherUID2;
     }
@@ -118,10 +126,15 @@
         try {
             Principal p = getTestPrincipal();
             u = (UserImpl) umgr.createUser(p.getName(), buildPassword(p));
+            save(uSession);
             fail("Group administrator should not be allowed to create a new user.");
-            u.remove();
         } catch (AccessDeniedException e) {
             // success
+        } finally {
+            if (u != null) {
+                u.remove();
+                save(uSession);
+            }
         }
     }
 
@@ -131,21 +144,37 @@
         Authorizable himself = umgr.getAuthorizable(uID);
         try {
             himself.remove();
+            save(uSession);
             fail("A GroupAdministrator should not be allowed to remove the own authorizable.");
         } catch (AccessDeniedException e) {
             // success
         }
     }
 
+    public void testRemoveGroupAdmin() throws RepositoryException, NotExecutableException {
+        UserManager umgr = getUserManager(uSession);
+
+        Authorizable groupAdmin = umgr.getAuthorizable(grID);
+        try {
+            groupAdmin.remove();
+            save(uSession);
+            fail("A GroupAdministrator should not be allowed to remove the group admin.");
+        } catch (AccessDeniedException e) {
+            // success
+        }
+    }
+
     public void testCreateGroup() throws RepositoryException, NotExecutableException {
         UserManager umgr = getUserManager(uSession);
         Group testGroup = null;
         try {
             testGroup = umgr.createGroup(getTestPrincipal());
-            assertTrue(Text.isDescendant(UserConstants.GROUPS_PATH, ((GroupImpl)testGroup).getNode().getPath()));
+            save(uSession);
+            assertTrue(Text.isDescendant(groupsPath, ((GroupImpl)testGroup).getNode().getPath()));
         } finally {
             if (testGroup != null) {
                 testGroup.remove();
+                save(uSession);
             }
         }
     }
@@ -155,10 +184,12 @@
         Group testGroup = null;
         try {
             testGroup = umgr.createGroup(getTestPrincipal(), "/any/intermediate/path");
-            assertTrue(Text.isDescendant(UserConstants.GROUPS_PATH + "/any/intermediate/path", ((GroupImpl)testGroup).getNode().getPath()));
+            save(uSession);
+            assertTrue(Text.isDescendant(groupsPath + "/any/intermediate/path", ((GroupImpl)testGroup).getNode().getPath()));
         } finally {
             if (testGroup != null) {
                 testGroup.remove();
+                save(uSession);
             }
         }
     }
@@ -168,14 +199,13 @@
         Authorizable cU = umgr.getAuthorizable(getYetAnotherID());
         Group gr = (Group) umgr.getAuthorizable(grID);
 
-        // adding and removing the child-user as member of a group must not 
-        // succeed as long editing session is not user-admin.
+        // adding and removing the test user as member of a group must succeed.
         try {
-            assertFalse("Modifying group membership requires GroupAdmin and UserAdmin.",gr.addMember(cU));
-        } catch (AccessDeniedException e) {
-            // ok
+            assertTrue("Modifying group membership requires GroupAdmin membership.",gr.addMember(cU));
+            save(uSession);
         } finally {
             gr.removeMember(cU);
+            save(uSession);
         }
     }
 
@@ -183,75 +213,80 @@
         UserManager umgr = getUserManager(uSession);
         Authorizable cU = umgr.getAuthorizable(getYetAnotherID());
 
-        Authorizable auth = umgr.getAuthorizable(UserConstants.USER_ADMIN_GROUP_NAME);
-        if (auth == null || !auth.isGroup()) {
-            throw new NotExecutableException("Cannot execute test. User-Admin name has been changed by config.");
-        }
-        Group userAdmin = (Group)auth;
-        User self = (User) umgr.getAuthorizable(uID);
-        try {
-            assertTrue(userAdmin.addMember(self));
-
-            Group gr = (Group) umgr.getAuthorizable(groupAdmin.getID());
-            assertTrue(gr.addMember(cU));
-            assertTrue(gr.removeMember(cU));
-        } finally {
-            userAdmin.removeMember(self);
-        }
+        Group gr = (Group) umgr.getAuthorizable(groupAdmin.getID());
+        assertTrue(gr.addMember(cU));
+        save(uSession);
+        assertTrue(gr.removeMember(cU));
+        save(uSession);
     }
 
     public void testAddMembersToCreatedGroup() throws RepositoryException, NotExecutableException {
         UserManager umgr = getUserManager(uSession);
-        Authorizable auth = umgr.getAuthorizable(UserConstants.USER_ADMIN_GROUP_NAME);
-        if (auth == null || !auth.isGroup()) {
-            throw new NotExecutableException("Cannot execute test. User-Admin name has been changed by config.");
-        }
-        Group userAdmin = (Group) auth;
         Group testGroup = null;
         User self = (User) umgr.getAuthorizable(uID);
         try {
             // let groupadmin create a new group
             testGroup = umgr.createGroup(getTestPrincipal(), "/a/b/c/d");
+            save(uSession);
 
             // editing session adds itself to that group -> must succeed.
             assertTrue(testGroup.addMember(self));
-
-            // editing session adds itself to user-admin group
-            userAdmin.addMember(self);
-            assertTrue(userAdmin.isMember(self));
+            save(uSession);
 
             // add child-user to test group
             Authorizable testUser = umgr.getAuthorizable(getYetAnotherID());
             assertFalse(testGroup.isMember(testUser));
             assertTrue(testGroup.addMember(testUser));
+            save(uSession);
         } finally {
             if (testGroup != null) {
-                for (Iterator it = testGroup.getDeclaredMembers(); it.hasNext();) {
-                    testGroup.removeMember((Authorizable) it.next());
+                for (Iterator<Authorizable> it = testGroup.getDeclaredMembers(); it.hasNext();) {
+                    testGroup.removeMember(it.next());
                 }
                 testGroup.remove();
+                save(uSession);
             }
-            userAdmin.removeMember(self);            
         }
     }
 
-    public void testAddMemberToForeignGroup() throws RepositoryException, NotExecutableException {
+    public void testAddMembersUserAdmins() throws RepositoryException, NotExecutableException {
+        UserManager umgr = getUserManager(uSession);
+        Authorizable auth = umgr.getAuthorizable(UserConstants.USER_ADMIN_GROUP_NAME);
+        if (auth == null || !auth.isGroup()) {
+            throw new NotExecutableException("Cannot execute test. No User-Admin group found.");
+        }
+        Group userAdmin = (Group) auth;
+        Group testGroup = null;
+        User self = (User) umgr.getAuthorizable(uID);
         try {
-            // let superuser create child user below the user with uID.
-            UserManager umgr = getUserManager(uSession);
-            Authorizable cU = umgr.getAuthorizable(getYetAnotherID());
-            Group uadminGr = (Group) umgr.getAuthorizable(UserConstants.USER_ADMIN_GROUP_NAME);
-            if (uadminGr.isMember(cU)) {
-                throw new RepositoryException("Test user is already member -> cannot execute.");
-            }
 
-            // adding to and removing a child user from a group the group-admin
-            // is NOT member of must fail.
-            uadminGr.addMember(cU);
-            fail("A GroupAdmin should not be allowed to add a user to a group she/he is not member of.");
+            userAdmin.addMember(self);
+            save(uSession);
+
+            userAdmin.removeMember(self);
+            save(uSession);
+            fail("Group admin cannot add member to user-admins");
+        } catch (AccessDeniedException e) {
+            // success
+        }
+
+        try {
+            // let groupadmin create a new group
+            testGroup = umgr.createGroup(getTestPrincipal(), "/a/b/c/d");
+            save(uSession);
 
+            userAdmin.addMember(testGroup);
+            save(uSession);
+            userAdmin.removeMember(testGroup);
+            save(uSession);
+            fail("Group admin cannot add member to user-admins");
         } catch (AccessDeniedException e) {
-            // success.
+            // success
+        } finally {
+            if (testGroup != null) {
+                testGroup.remove();
+                save(uSession);
+            }
         }
     }
 
@@ -261,35 +296,12 @@
         Authorizable pU = umgr.getAuthorizable(otherUID);
         Group gr = (Group) umgr.getAuthorizable(groupAdmin.getID());
 
-        // adding and removing the parent-user as member of a group must not
-        // succeed: editing session isn't UserAdmin
         try {
-            assertFalse(gr.addMember(pU));
-        } catch (AccessDeniedException e) {
-            // ok
-        } finally {
-            gr.removeMember(pU);
-        }
-
-        // ... if the editing user becomes member of the user-admin group it
-        // must work.
-        Group uAdministrators = null;
-        try {
-            Authorizable userAdmin = userMgr.getAuthorizable(UserConstants.USER_ADMIN_GROUP_NAME);
-            if (userAdmin == null || !userAdmin.isGroup()) {
-                throw new NotExecutableException("Cannot execute test. User-Admin name has been changed by config.");
-            }
-            uAdministrators = (Group) userAdmin;
-            uAdministrators.addMember(userMgr.getAuthorizable(uID));
-
             assertTrue(gr.addMember(pU));
-            gr.removeMember(pU);
+            save(uSession);
         } finally {
-            // let superuser do the clean up.
-            // remove testuser from u-admin group again.
-            if (uAdministrators != null) {
-                uAdministrators.removeMember(userMgr.getAuthorizable(uID));
-            }
+            gr.removeMember(pU);
+            save(uSession);
         }
     }
 
@@ -316,16 +328,20 @@
 
     public void testAddOwnAuthorizableToForeignGroup() throws RepositoryException, NotExecutableException {
         UserManager umgr = getUserManager(uSession);
+        Authorizable self = umgr.getAuthorizable(uID);
 
-        Authorizable user = umgr.getAuthorizable(uID);
-        Group uadminGr = (Group) umgr.getAuthorizable(UserConstants.USER_ADMIN_GROUP_NAME);
-        if (uadminGr.isMember(user)) {
-            throw new RepositoryException("Test user is already member -> cannot execute.");
-        }
+        Group gr = userMgr.createGroup(getTestPrincipal());
+        save(superuser);
 
-        String msg = "GroupAdmin must be able to add its own authorizable to a group she/he is not yet member of.";
-        assertTrue(msg, uadminGr.addMember(user));
-        assertTrue(msg, uadminGr.removeMember(user));
+        try {
+            assertTrue(((Group) umgr.getAuthorizable(gr.getID())).addMember(self));
+            save(uSession);
+            assertTrue(((Group) umgr.getAuthorizable(gr.getID())).removeMember(self));
+            save(uSession);
+        } finally {
+            gr.remove();
+            save(superuser);
+        }
     }
 
     public void testRemoveMembersOfForeignGroup() throws RepositoryException, NotExecutableException {
@@ -336,24 +352,32 @@
         try {
             // let superuser create a group and a user a make user member of group
             nGr = userMgr.createGroup(getTestPrincipal());
+            save(superuser);
+
             Principal p = getTestPrincipal();
             nUs = userMgr.createUser(p.getName(), buildPassword(p));
+            save(superuser);
+
             p = getTestPrincipal();
             nUs2 = userMgr.createUser(p.getName(), buildPassword(p));
+            save(superuser);
             nGr.addMember(nUs);
             nGr.addMember(nUs2);
+            save(superuser);
 
-            Group gr = (Group) getUserManager(uSession).getAuthorizable(nGr.getID());
+            UserManager umgr = getUserManager(uSession);
+            Group gr = (Group) umgr.getAuthorizable(nGr.getID());
 
             // removing any member must fail unless the testuser is user-admin
-            Iterator it = gr.getMembers();
+            Iterator<Authorizable> it = gr.getMembers();
             if (it.hasNext()) {
-                Authorizable auth = (Authorizable) it.next();
+                Authorizable auth = it.next();
 
-                String msg = "GroupAdmin cannot remove members of other user unless he/she is user-admin.";
-                assertFalse(msg, gr.removeMember(auth));
+                String msg = "GroupAdmin must be able to modify group membership.";
+                assertTrue(msg, gr.removeMember(auth));
+                save(uSession);
             } else {
-                throw new RepositoryException("Must contain members....");
+                fail("Must contain members....");
             }
 
         } catch (AccessDeniedException e) {
@@ -367,6 +391,7 @@
             }
             if (nUs != null) nUs.remove();
             if (nUs2 != null) nUs2.remove();
+            save(superuser);
         }
     }
 
@@ -377,19 +402,23 @@
         try {
             // let superuser create a group and a user a make user member of group
             nGr = userMgr.createGroup(getTestPrincipal());
+            save(superuser);
             Principal p = getTestPrincipal();
             nUs = userMgr.createUser(p.getName(), buildPassword(p));
             nGr.addMember(nUs);
+            save(superuser);
 
-            Group gr = (Group) getUserManager(uSession).getAuthorizable(nGr.getID());
+            UserManager umgr = getUserManager(uSession);
+            Group gr = (Group) umgr.getAuthorizable(nGr.getID());
 
             // since only 1 single member -> removal rather than modification.
             // since uSession is not user-admin this must fail.
-            for (Iterator it = gr.getMembers(); it.hasNext();) {
-                Authorizable auth = (Authorizable) it.next();
+            for (Iterator<Authorizable> it = gr.getMembers(); it.hasNext();) {
+                Authorizable auth = it.next();
 
-                String msg = "GroupAdmin cannot remove members of groups unless he/she is UserAdmin.";
-                assertFalse(msg, gr.removeMember(auth));
+                String msg = "GroupAdmin must be able to remove a member of another group.";
+                assertTrue(msg, gr.removeMember(auth));
+                save(uSession);
             }
         } catch (AccessDeniedException e) {
             // fine as well.
@@ -398,6 +427,7 @@
             if (nGr != null && nUs != null) nGr.removeMember(nUs);
             if (nGr != null) nGr.remove();
             if (nUs != null) nUs.remove();
+            save(superuser);
         }
     }
 
@@ -410,6 +440,7 @@
         assertFalse(impers.allows(buildSubject(selfPrinc)));
         try {
             assertFalse(impers.grantImpersonation(selfPrinc));
+            save(uSession);
         } catch (AccessDeniedException e) {
             // ok.
         }
@@ -420,6 +451,7 @@
         assertFalse(impers.allows(buildSubject(selfPrinc)));
         try {
             assertFalse(impers.grantImpersonation(selfPrinc));
+            save(uSession);
         } catch (AccessDeniedException e) {
             // ok.
         }
@@ -432,14 +464,32 @@
         try {
             Principal p = getTestPrincipal();
             gr = umgr.createGroup(p);
+            save(uSession);
 
+            // must be visible for the user-mgr attached to another session.
             Authorizable az = userMgr.getAuthorizable(gr.getID());
             assertNotNull(az);
             assertEquals(gr.getID(), az.getID());
         } finally {
             if (gr != null) {
                 gr.remove();
+                save(uSession);
             }
         }
     }
+
+    public void testAddCustomNodeToGroupAdminNode() throws RepositoryException, NotExecutableException {
+        UserManager umgr = getUserManager(uSession);
+        Node groupAdminNode = ((AuthorizableImpl) umgr.getAuthorizable(grID)).getNode();
+        Session s = groupAdminNode.getSession();
+
+        Node n = groupAdminNode.addNode(nodeName1, ntUnstructured);
+        save(uSession);
+
+        n.setProperty(propertyName1, s.getValueFactory().createValue("anyValue"));
+        save(uSession);
+
+        n.remove();
+        save(uSession);
+    }
 }
\ No newline at end of file