diff --git a/community/security/src/main/java/org/neo4j/server/security/auth/FileUserRepository.java b/community/security/src/main/java/org/neo4j/server/security/auth/FileUserRepository.java index 1b59892102991..f45bf2306e97b 100644 --- a/community/security/src/main/java/org/neo4j/server/security/auth/FileUserRepository.java +++ b/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" ); } } diff --git a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/FileUserRealm.java b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/FileUserRealm.java index 31602c73364c6..dba4bff8b64b9 100644 --- a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/FileUserRealm.java +++ b/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; } diff --git a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthProceduresTest.java b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthProceduresTest.java index 34b1c59ff7ace..5af26ff157bdd 100644 --- a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthProceduresTest.java +++ b/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" ) ); + } } /* diff --git a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/ShiroAuthManagerTest.java b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/ShiroAuthManagerTest.java index a6b9a38cdf414..86bed7d6a7c77 100644 --- a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/ShiroAuthManagerTest.java +++ b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/ShiroAuthManagerTest.java @@ -173,7 +173,9 @@ 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 @@ -181,10 +183,11 @@ public void shouldDeleteUser() throws Throwable // 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 ); @@ -192,7 +195,15 @@ public void shouldDeleteUnknownUser() throws Throwable 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" ) );