You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by an...@apache.org on 2017/03/14 15:06:55 UTC

svn commit: r1786911 - /jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/

Author: angela
Date: Tue Mar 14 15:06:55 2017
New Revision: 1786911

URL: http://svn.apache.org/viewvc?rev=1786911&view=rev
Log:
OAK-5882 : Improve coverage for oak.security code in oak-core (wip)

Added:
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AbstractGroupPrincipalTest.java   (with props)
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/GroupImplTest.java   (with props)
Modified:
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AuthorizableImplTest.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/EveryoneGroupTest.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/MembershipBaseTest.java   (props changed)
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/SystemUserImplTest.java

Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AbstractGroupPrincipalTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AbstractGroupPrincipalTest.java?rev=1786911&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AbstractGroupPrincipalTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AbstractGroupPrincipalTest.java Tue Mar 14 15:06:55 2017
@@ -0,0 +1,226 @@
+/*
+ * 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.oak.security.user;
+
+import java.security.Principal;
+import java.util.Iterator;
+import java.util.List;
+import javax.annotation.Nonnull;
+import javax.jcr.RepositoryException;
+
+import com.google.common.collect.ImmutableList;
+import org.apache.jackrabbit.api.security.user.Authorizable;
+import org.apache.jackrabbit.api.security.user.Group;
+import org.apache.jackrabbit.api.security.user.UserManager;
+import org.apache.jackrabbit.oak.AbstractSecurityTest;
+import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
+import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.when;
+
+public class AbstractGroupPrincipalTest extends AbstractSecurityTest {
+
+    private Group testGroup;
+
+    private AbstractGroupPrincipal agp;
+    private AbstractGroupPrincipal everyoneAgp;
+    private AbstractGroupPrincipal throwing;
+
+
+    @Override
+    public void before() throws Exception {
+        super.before();
+
+        testGroup = getUserManager(root).createGroup("AbstractGroupPrincipalTest");
+        root.commit();
+
+        agp = new AGP();
+        everyoneAgp = new AGP();
+        ((AGP) everyoneAgp).isEveryone = true;
+
+        throwing = new ThrowingAGP();
+    }
+
+    @Override
+    public void after() throws Exception {
+        try {
+            if (testGroup != null) {
+                testGroup.remove();
+                root.commit();
+            }
+        } finally {
+            super.after();
+        }
+    }
+
+    @Test
+    public void testIsMemberOf() throws Exception {
+        final Principal p = getTestUser().getPrincipal();
+        assertTrue(agp.isMember(p));
+        assertTrue(agp.isMember(new PrincipalImpl(p.getName())));
+        assertTrue(agp.isMember(new Principal() {
+            @Override
+            public String getName() {
+                return p.getName();
+            }
+        }));
+
+    }
+
+    @Test
+    public void testIsMemberMissingAuthorizable() throws RepositoryException {
+        List<Principal> principals = ImmutableList.of(
+                new PrincipalImpl("name"),
+                new Principal() {
+                    @Override
+                    public String getName() {
+                        return "name";
+                    }
+                }
+        );
+
+        for (Principal p : principals) {
+            assertFalse(agp.isMember(p));
+        }
+    }
+
+    @Test
+    public void testIsMemberOfEveryone() throws Exception {
+        final Principal p = getTestUser().getPrincipal();
+        assertTrue(everyoneAgp.isMember(p));
+        assertTrue(everyoneAgp.isMember(new PrincipalImpl(p.getName())));
+        assertTrue(everyoneAgp.isMember(new Principal() {
+            @Override
+            public String getName() {
+                return p.getName();
+            }
+        }));
+
+    }
+
+    @Test
+    public void testIsMemberOfEveryoneMissingAuthorizable() throws RepositoryException {
+        List<Principal> principals = ImmutableList.of(
+                new PrincipalImpl("name"),
+                new Principal() {
+                    @Override
+                    public String getName() {
+                        return "name";
+                    }
+                }
+        );
+
+        for (Principal p : principals) {
+            assertTrue(everyoneAgp.isMember(p));
+        }
+    }
+
+    @Test
+    public void testIsMemberOfInternalError() throws Exception {
+        final Principal p = getTestUser().getPrincipal();
+        assertFalse(throwing.isMember(p));
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void testMembersInternalError() throws Exception {
+        throwing.members();
+    }
+
+    @Test
+    public void testEveryoneIsMemberOfEveryone() throws RepositoryException {
+        AbstractGroupPrincipal member = Mockito.mock(AbstractGroupPrincipal.class);
+        when(member.getName()).thenReturn(EveryonePrincipal.NAME);
+
+        assertFalse(everyoneAgp.isMember(member));
+    }
+
+    @Test(expected = UnsupportedOperationException.class)
+    public void testAddMember() {
+        agp.addMember(new PrincipalImpl("name"));
+    }
+
+    @Test(expected = UnsupportedOperationException.class)
+    public void testRemoveMember() {
+        agp.removeMember(new PrincipalImpl("name"));
+    }
+
+    private class AGP extends AbstractGroupPrincipal {
+
+        private Authorizable member;
+        private boolean isEveryone;
+
+        AGP() throws Exception {
+            super(testGroup.getPrincipal().getName(), root.getTree(testGroup.getPath()), getNamePathMapper());
+            member = getTestUser();
+        }
+
+        @Override
+        UserManager getUserManager() {
+            return AbstractGroupPrincipalTest.this.getUserManager(root);
+        }
+
+        @Override
+        boolean isEveryone() throws RepositoryException {
+            return isEveryone;
+        }
+
+        @Override
+        boolean isMember(@Nonnull Authorizable authorizable) throws RepositoryException {
+            return member.getID().equals(authorizable.getID());
+        }
+
+        @Nonnull
+        @Override
+        Iterator<Authorizable> getMembers() throws RepositoryException {
+            return ImmutableList.of(member).iterator();
+        }
+    }
+
+    private class ThrowingAGP extends AGP {
+
+        ThrowingAGP() throws Exception {
+            super();
+        }
+
+        @Override
+        UserManager getUserManager() {
+            UserManager userManager = Mockito.mock(UserManager.class);
+            Mockito.doThrow(RepositoryException.class);
+            return userManager;
+        }
+
+        @Override
+        boolean isEveryone() throws RepositoryException {
+            throw new RepositoryException();
+        }
+
+        @Override
+        boolean isMember(@Nonnull Authorizable authorizable) throws RepositoryException {
+            throw new RepositoryException();
+        }
+
+        @Nonnull
+        @Override
+        Iterator<Authorizable> getMembers() throws RepositoryException {
+            throw new RepositoryException();
+        }
+    }
+}
\ No newline at end of file

Propchange: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AbstractGroupPrincipalTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AuthorizableImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AuthorizableImplTest.java?rev=1786911&r1=1786910&r2=1786911&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AuthorizableImplTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AuthorizableImplTest.java Tue Mar 14 15:06:55 2017
@@ -70,7 +70,7 @@ public class AuthorizableImplTest extend
         Authorizable user = userMgr.getAuthorizable(testUser.getID());
         Authorizable group = userMgr.getAuthorizable(testGroup.getID());
 
-        Map<Authorizable, Authorizable> equalAuthorizables = new HashMap<Authorizable, Authorizable>();
+        Map<Authorizable, Authorizable> equalAuthorizables = new HashMap();
         equalAuthorizables.put(testUser, testUser);
         equalAuthorizables.put(testGroup, testGroup);
         equalAuthorizables.put(user, user);
@@ -93,7 +93,7 @@ public class AuthorizableImplTest extend
         Authorizable user = otherUserManager.getAuthorizable(testUser.getID());
         Authorizable group = otherUserManager.getAuthorizable(testGroup.getID());
 
-        Map<Authorizable, Authorizable> notEqual = new HashMap<Authorizable, Authorizable>();
+        Map<Authorizable, Authorizable> notEqual = new HashMap();
         notEqual.put(testUser, testGroup);
         notEqual.put(user, group);
         notEqual.put(testUser, user);

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/EveryoneGroupTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/EveryoneGroupTest.java?rev=1786911&r1=1786910&r2=1786911&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/EveryoneGroupTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/EveryoneGroupTest.java Tue Mar 14 15:06:55 2017
@@ -21,6 +21,7 @@ import java.util.Iterator;
 import java.util.Set;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
 import org.apache.jackrabbit.api.security.user.Authorizable;
 import org.apache.jackrabbit.api.security.user.Group;
 import org.apache.jackrabbit.api.security.user.UserManager;
@@ -133,6 +134,11 @@ public class EveryoneGroupTest extends A
     }
 
     @Test
+    public void testAddMembers() throws Exception {
+        assertEquals(Sets.newHashSet(getTestUser().getID()), everyoneGroup.addMembers(getTestUser().getID()));
+    }
+
+    @Test
     public void testRemoveEveryoneFromMembers() throws Exception {
         assertFalse(everyoneGroup.removeMember(everyoneGroup));
     }
@@ -145,6 +151,11 @@ public class EveryoneGroupTest extends A
     }
 
     @Test
+    public void testRemoveMembers() throws Exception {
+        assertEquals(Sets.newHashSet(getTestUser().getID()), everyoneGroup.removeMembers(getTestUser().getID()));
+    }
+
+    @Test
     public void testEveryoneMemberOf() throws Exception {
         Iterator<Group> groups = everyoneGroup.memberOf();
         assertFalse(groups.hasNext());

Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/GroupImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/GroupImplTest.java?rev=1786911&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/GroupImplTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/GroupImplTest.java Tue Mar 14 15:06:55 2017
@@ -0,0 +1,124 @@
+/*
+ * 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.oak.security.user;
+
+import java.security.Principal;
+import java.util.Iterator;
+import java.util.UUID;
+
+import com.google.common.collect.Iterators;
+import org.apache.jackrabbit.api.security.user.Authorizable;
+import org.apache.jackrabbit.api.security.user.Group;
+import org.apache.jackrabbit.oak.AbstractSecurityTest;
+import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+
+public class GroupImplTest extends AbstractSecurityTest {
+
+    private final String groupId = "gr" + UUID.randomUUID();
+
+    private UserManagerImpl uMgr;
+    private GroupImpl group;
+
+    @Override
+    public void before() throws Exception {
+        super.before();
+
+        uMgr = new UserManagerImpl(root, getNamePathMapper(), getSecurityProvider());
+        Group g = uMgr.createGroup(groupId);
+
+        group = new GroupImpl(groupId, root.getTree(g.getPath()), uMgr);
+    }
+
+    @Override
+    public void after() throws Exception {
+        try {
+            root.refresh();
+        } finally {
+            super.after();
+        }
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testCheckValidTree() throws Exception {
+        new GroupImpl(getTestUser().getID(), root.getTree(getTestUser().getPath()), uMgr);
+    }
+
+    @Test
+    public void testAddMemberInvalidAuthorizable() throws Exception {
+        assertFalse(group.addMember(Mockito.mock(Authorizable.class)));
+    }
+
+    @Test
+    public void testAddMemberEveryone() throws Exception {
+        Group everyoneGroup = uMgr.createGroup(EveryonePrincipal.getInstance());
+        assertFalse(group.addMember(everyoneGroup));
+    }
+
+    @Test
+    public void testAddMemberItself() throws Exception {
+        assertFalse(group.addMember(group));
+    }
+
+    @Test
+    public void testRemoveMemberInvalidAuthorizable() throws Exception {
+        assertFalse(group.removeMember(Mockito.mock(Authorizable.class)));
+    }
+
+    @Test
+    public void testRemoveNotMember() throws Exception {
+        assertFalse(group.removeMember(getTestUser()));
+    }
+
+    @Test
+    public void testIsMemberInvalidAuthorizable() throws Exception {
+        assertFalse(group.isMember(Mockito.mock(Authorizable.class)));
+    }
+
+    @Test
+    public void testGroupPrincipal() throws Exception {
+        Principal groupPrincipal = group.getPrincipal();
+        assertTrue(groupPrincipal instanceof AbstractGroupPrincipal);
+
+        AbstractGroupPrincipal agp = (AbstractGroupPrincipal) groupPrincipal;
+        assertSame(uMgr, agp.getUserManager());
+        assertEquals(group.isEveryone(), agp.isEveryone());
+    }
+
+    @Test
+    public void testGroupPrincipalIsMember() throws Exception {
+        group.addMember(getTestUser());
+
+        AbstractGroupPrincipal groupPrincipal = (AbstractGroupPrincipal) group.getPrincipal();
+        assertTrue(groupPrincipal.isMember(getTestUser()));
+    }
+
+    @Test
+    public void testGroupPrincipalMembers() throws Exception {
+        group.addMember(getTestUser());
+
+        AbstractGroupPrincipal groupPrincipal = (AbstractGroupPrincipal) group.getPrincipal();
+        Iterator<Authorizable> members = groupPrincipal.getMembers();
+        assertTrue(Iterators.elementsEqual(group.getMembers(), members));
+    }
+}
\ No newline at end of file

Propchange: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/GroupImplTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/MembershipBaseTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/SystemUserImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/SystemUserImplTest.java?rev=1786911&r1=1786910&r2=1786911&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/SystemUserImplTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/SystemUserImplTest.java Tue Mar 14 15:06:55 2017
@@ -23,6 +23,7 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.jcr.Credentials;
 import javax.jcr.SimpleCredentials;
+import javax.jcr.UnsupportedRepositoryOperationException;
 import javax.jcr.nodetype.ConstraintViolationException;
 import javax.security.auth.login.LoginException;
 
@@ -66,7 +67,6 @@ public class SystemUserImplTest extends
 
     private UserManager userMgr;
     private String uid;
-    private User user;
 
     @Override
     @Before
@@ -80,6 +80,8 @@ public class SystemUserImplTest extends
     @Override
     public void after() throws Exception {
         try {
+            root.refresh();
+            User user = userMgr.getAuthorizable(uid, User.class);
             if (user != null) {
                 user.remove();
                 root.commit();
@@ -112,22 +114,30 @@ public class SystemUserImplTest extends
 
     @Test
     public void testCreateSystemUser() throws Exception {
-        user = createUser(null);
+        assertTrue(createUser(null) instanceof SystemUserImpl);
+    }
 
-        assertTrue(user instanceof SystemUserImpl);
+    @Test
+    public void testIsSystemUser() throws Exception {
+        assertTrue(createUser(null).isSystemUser());
     }
 
     @Test
     public void testSystemUserTree() throws Exception {
-        user = createUser(null);
-        Tree t = root.getTree(user.getPath());
+        Tree t = root.getTree(createUser(null).getPath());
         assertFalse(t.hasProperty(UserConstants.REP_PASSWORD));
         assertEquals(UserConstants.NT_REP_SYSTEM_USER, TreeUtil.getPrimaryTypeName(t));
     }
 
+    @Test(expected = IllegalArgumentException.class)
+    public void testCheckValidTree() throws Exception {
+        User testUser = getTestUser();
+        new SystemUserImpl(testUser.getID(), root.getTree(testUser.getPath()), (UserManagerImpl) userMgr);
+    }
+
     @Test
     public void testGetCredentials() throws Exception {
-        user = createUser(null);
+        User user = createUser(null);
 
         Credentials creds = user.getCredentials();
         assertTrue(creds instanceof UserIdCredentials);
@@ -138,35 +148,36 @@ public class SystemUserImplTest extends
 
     @Test
     public void testHasNoPassword() throws Exception {
-        user = createUser(null);
+        User user = createUser(null);
 
         Tree userTree = root.getTree(user.getPath());
         assertFalse(userTree.hasProperty(UserConstants.REP_PASSWORD));
     }
 
+    @Test(expected = UnsupportedRepositoryOperationException.class)
+    public void testChangePassword() throws Exception {
+        User user = createUser(null);
+        user.changePassword("pw");
+    }
+
+    @Test(expected = UnsupportedRepositoryOperationException.class)
+    public void testChangePassword2() throws Exception {
+        User user = createUser(null);
+        user.changePassword("pw", "newPw");
+    }
 
     /**
      * @since OAK 1.0 In contrast to Jackrabbit core the intermediate path may
      * not be an absolute path in OAK.
      */
-    @Test
+    @Test(expected = ConstraintViolationException.class)
     public void testCreateUserWithAbsolutePath() throws Exception {
-        try {
-            user = createUser("/any/path/to/the/new/user");
-            fail("ConstraintViolationException expected");
-        } catch (ConstraintViolationException e) {
-            // success
-        }
+        createUser("/any/path/to/the/new/user");
     }
 
-    @Test
+    @Test(expected = ConstraintViolationException.class)
     public void testCreateUserWithAbsolutePath2() throws Exception {
-        try {
-            user = createUser(UserConstants.DEFAULT_USER_PATH + "/any/path/to/the/new/user");
-            fail("ConstraintViolationException expected");
-        } catch (ConstraintViolationException e) {
-            // success
-        }
+        createUser(UserConstants.DEFAULT_USER_PATH + "/any/path/to/the/new/user");
     }
 
     @Test
@@ -174,23 +185,17 @@ public class SystemUserImplTest extends
         String userRoot = UserConstants.DEFAULT_USER_PATH + '/' + UserConstants.DEFAULT_SYSTEM_RELATIVE_PATH;
         String path = userRoot + "/any/path/to/the/new/user";
 
-        user = createUser(path);
-        assertTrue(user.getPath().startsWith(path));
+        assertTrue(createUser(path).getPath().startsWith(path));
     }
 
-    @Test
+    @Test(expected = ConstraintViolationException.class)
     public void testCreateUserWithRelativePath() throws Exception {
-        try {
-            user = createUser("any/path");
-            fail("ConstraintViolationException expected");
-        }  catch (ConstraintViolationException e) {
-            // success
-        }
+        createUser("any/path");
     }
 
     @Test
     public void testCreateUserWithRelativePath2() throws Exception {
-        user = createUser(UserConstants.DEFAULT_SYSTEM_RELATIVE_PATH + "/any/path");
+        User user = createUser(UserConstants.DEFAULT_SYSTEM_RELATIVE_PATH + "/any/path");
 
         assertNotNull(user.getID());
         assertTrue(user.getPath().contains("any/path"));
@@ -225,7 +230,7 @@ public class SystemUserImplTest extends
 
     @Test
     public void testLoginAsSystemUser() throws Exception {
-        user = createUser(null);
+        createUser(null);
         try {
             login(new SimpleCredentials(uid, new char[0])).close();
             fail();
@@ -236,7 +241,7 @@ public class SystemUserImplTest extends
 
     @Test
     public void testLoginAsSystemUser2() throws Exception {
-        user = createUser(null);
+        User user = createUser(null);
         try {
             login(user.getCredentials()).close();
             fail();
@@ -247,7 +252,7 @@ public class SystemUserImplTest extends
 
     @Test
     public void testImpersonateSystemUser() throws Exception {
-        user = createUser(null);
+        createUser(null);
         ContentSession cs = login(new ImpersonationCredentials(new SimpleCredentials(uid, new char[0]), adminSession.getAuthInfo()));
         cs.close();
     }
@@ -255,7 +260,7 @@ public class SystemUserImplTest extends
 
     @Test
     public void testImpersonateDisabledSystemUser() throws Exception {
-        user = createUser(null);
+        User user = createUser(null);
         user.disable("disabled");
         root.commit();
         try {
@@ -269,14 +274,13 @@ public class SystemUserImplTest extends
 
     @Test
     public void testGetPrincipal() throws Exception {
-        user = createUser(null);
+        User user = createUser(null);
         assertTrue(user.getPrincipal() instanceof SystemUserPrincipal);
     }
 
     @Test
     public void testAddToGroup() throws Exception {
-        user = createUser(null);
-
+        User user = createUser(null);
 
         Group g = null;
         try {
@@ -309,7 +313,7 @@ public class SystemUserImplTest extends
      */
     @Test
     public void testOnCreateOmitted() throws Exception {
-        user = createUser(null);
+        User user = createUser(null);
 
         Tree t = root.getTree(user.getPath());
         assertFalse(t.hasChild(AccessControlConstants.REP_POLICY));