Disallow deletion of non-existent user and introduced noneSubject in …

…tests

community/security/src/main/java/org/neo4j/server/security/auth/FileUserRepository.java

@@ -77,11 +77,11 @@ public void start() throws Throwable
     }
 
     @Override
-    public void create( User user ) throws IllegalCredentialsException, IOException
+    public void create( User user ) throws IllegalArgumentException, IOException
     {
         if ( !isValidName( user.name() ) )
         {
-            throw new IllegalCredentialsException( "'" + user.name() + "' is not a valid user name." );
+            throw new IllegalArgumentException( "'" + user.name() + "' is not a valid user name." );
         }
 
         synchronized (this)
@@ -91,7 +91,7 @@ public void create( User user ) throws IllegalCredentialsException, IOException
             {
                 if ( other.name().equals( user.name() ) )
                 {
-                    throw new IllegalCredentialsException( "The specified user already exists" );
+                    throw new IllegalArgumentException( "The specified user already exists" );
                 }
             }
 

enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/FileUserRealm.java

@@ -262,6 +262,10 @@ boolean deleteUser( String username ) throws IOException
                 removeUserFromAllRoles( username );
                 result = true;
             }
+            else
+            {
+                throw new IllegalArgumentException( "The user '" + username + "' does not exist" );
+            }
         }
         return result;
     }

enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthProceduresTest.java

@@ -56,6 +56,7 @@ public class AuthProceduresTest
     private AuthSubject schemaSubject;
     private AuthSubject writeSubject;
     private AuthSubject readSubject;
+    private AuthSubject noneSubject;
 
     private GraphDatabaseAPI db;
     private ShiroAuthManager manager;
@@ -67,6 +68,7 @@ public void setUp() throws Exception
         registerProcedures( db );
         manager = new EnterpriseAuthManager( new InMemoryUserRepository(), new InMemoryRoleRepository(),
                 new BasicPasswordPolicy(), systemUTC(), true );
+        manager.newUser( "noneSubject", "abc", false );
         manager.newUser( "adminSubject", "abc", false );
         manager.newUser( "schemaSubject", "abc", false );
         manager.newUser( "readWriteSubject", "abc", false );
@@ -75,6 +77,7 @@ public void setUp() throws Exception
         manager.newRole( PredefinedRolesBuilder.ARCHITECT, "schemaSubject" );
         manager.newRole( PredefinedRolesBuilder.PUBLISHER, "readWriteSubject" );
         manager.newRole( PredefinedRolesBuilder.READER, "readSubject" );
+        noneSubject = manager.login( "noneSubject", "abc" );
         readSubject = manager.login( "readSubject", "123" );
         writeSubject = manager.login( "readWriteSubject", "abc" );
         schemaSubject = manager.login( "schemaSubject", "abc" );
@@ -125,6 +128,7 @@ public void shouldNotCreateExistingUser() throws Exception
     @Test
     public void shouldNotAllowNonAdminCreateUser() throws Exception
     {
+        testFailCreateUser( noneSubject );
         testFailCreateUser( readSubject );
         testFailCreateUser( writeSubject );
         testFailCreateUser( schemaSubject );
@@ -315,6 +319,7 @@ public void shouldAllowAddingAndRemovingUserFromMultipleRoles() throws Exception
     @Test
     public void shouldNotAllowNonAdminAddingUserToRole() throws Exception
     {
+        testFailAddUserToRoleAction( noneSubject );
         testFailAddUserToRoleAction( readSubject );
         testFailAddUserToRoleAction( writeSubject );
         testFailAddUserToRoleAction( schemaSubject );
@@ -323,6 +328,7 @@ public void shouldNotAllowNonAdminAddingUserToRole() throws Exception
     @Test
     public void shouldNotAllowNonAdminRemovingUserFromRole() throws Exception
     {
+        testFailRemoveUserFromRoleAction( noneSubject );
         testFailRemoveUserFromRoleAction( readSubject );
         testFailRemoveUserFromRoleAction( writeSubject );
         testFailRemoveUserFromRoleAction( schemaSubject );
@@ -422,6 +428,7 @@ public void shouldDeleteUser() throws Exception
     @Test
     public void shouldNotAllowNonAdminDeleteUser() throws Exception
     {
+        testFailDeleteUser( noneSubject );
         testFailDeleteUser( readSubject );
         testFailDeleteUser( writeSubject );
         testFailDeleteUser( schemaSubject );
@@ -434,8 +441,16 @@ public void shouldAllowDeletingUserMultipleTimes() throws Exception
         assertNotNull( "User Craig should exist", manager.getUser( "Craig" ) );
         testCallEmpty( db, adminSubject, "CALL dbms.deleteUser('Craig')", null );
         assertNull( "User Craig should not exist", manager.getUser( "Craig" ) );
-        testCallEmpty( db, adminSubject, "CALL dbms.deleteUser('Craig')", null );
-        assertNull( "User Craig should not exist", manager.getUser( "Craig" ) );
+        try
+        {
+            testCallEmpty( db, adminSubject, "CALL dbms.deleteUser('Craig')", null );
+            fail( "Expected exception to be thrown" );
+        }
+        catch ( QueryExecutionException e )
+        {
+            assertTrue( "Exception should contain 'The user 'Craig' does not exist''",
+                    e.getMessage().contains( "The user 'Craig' does not exist" ) );
+        }
     }
 
     /*

enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/ShiroAuthManagerTest.java

@@ -173,26 +173,37 @@ public void shouldDeleteUser() throws Throwable
 
         // Given
         final User user = new User( "jake", Credential.forPassword( "abc123" ), true );
+        final User user2 = new User( "craig", Credential.forPassword( "321cba" ), true );
         users.create( user );
+        users.create( user2 );
         manager.start();
 
         // When
         manager.deleteUser( "jake" );
 
         // Then
         assertNull( users.findByName( "jake" ) );
+        assertNotNull( users.findByName( "craig" ) );
     }
 
     @Test
-    public void shouldDeleteUnknownUser() throws Throwable
+    public void shouldFailDeletingUnknownUser() throws Throwable
     {
         // Given
         final User user = new User( "jake", Credential.forPassword( "abc123" ), true );
         users.create( user );
         manager.start();
 
         // When
-        manager.deleteUser( "unknown" );
+        try
+        {
+            manager.deleteUser( "unknown" );
+            fail("Should throw exception on deleting unknown user");
+        }
+        catch ( IllegalArgumentException e )
+        {
+            e.getMessage().equals( "User 'unknown' does not exist" );
+        }
 
         // Then
         assertNotNull( users.findByName( "jake" ) );