diff --git a/community/security/src/main/java/org/neo4j/server/security/auth/BasicAuthManager.java b/community/security/src/main/java/org/neo4j/server/security/auth/BasicAuthManager.java index f679c931d6437..93245f95cc8d5 100644 --- a/community/security/src/main/java/org/neo4j/server/security/auth/BasicAuthManager.java +++ b/community/security/src/main/java/org/neo4j/server/security/auth/BasicAuthManager.java @@ -27,7 +27,6 @@ import org.neo4j.kernel.api.security.AuthSubject; import org.neo4j.kernel.api.security.AuthenticationResult; import org.neo4j.kernel.api.security.exception.IllegalCredentialsException; -import org.neo4j.kernel.lifecycle.Lifecycle; import org.neo4j.server.security.auth.exception.ConcurrentModificationException; /** @@ -39,7 +38,7 @@ * so the given UserRepository should not be added to another LifeSupport. *

*/ -public class BasicAuthManager implements Lifecycle, AuthManager, UserManager +public class BasicAuthManager implements AuthManager, UserManager { private final AuthenticationStrategy authStrategy; private final UserRepository users; @@ -145,7 +144,8 @@ public User getUser( String username ) return users.findByName( username ); } - public void setPassword( AuthSubject authSubject, String username, String password ) throws IOException + public void setPassword( AuthSubject authSubject, String username, String password ) throws IOException, + IllegalCredentialsException { if ( !(authSubject instanceof BasicAuthSubject) ) { @@ -157,25 +157,23 @@ public void setPassword( AuthSubject authSubject, String username, String passwo { throw new AuthorizationViolationException( "Invalid attempt to change the password for user " + username ); } - if ( setUserPassword( username, password ) == null ) - { - throw new IllegalArgumentException( "User " + username + " does not exist" ); - } + setUserPassword( username, password ); } @Override - public User setUserPassword( String username, String password ) throws IOException + public void setUserPassword( String username, String password ) throws IOException, + IllegalCredentialsException { assertAuthEnabled(); User existingUser = users.findByName( username ); if ( existingUser == null ) { - return null; + throw new IllegalCredentialsException( "User " + username + " does not exist" ); } if ( existingUser.credentials().matchesPassword( password ) ) { - return existingUser; + return; } try @@ -185,11 +183,10 @@ public User setUserPassword( String username, String password ) throws IOExcepti .withRequiredPasswordChange( false ) .build(); users.update( existingUser, updatedUser ); - return updatedUser; } catch ( ConcurrentModificationException e ) { // try again - return setUserPassword( username, password ); + setUserPassword( username, password ); } } diff --git a/community/security/src/main/java/org/neo4j/server/security/auth/UserManager.java b/community/security/src/main/java/org/neo4j/server/security/auth/UserManager.java index 28c1980ffdde7..62616c57346f4 100644 --- a/community/security/src/main/java/org/neo4j/server/security/auth/UserManager.java +++ b/community/security/src/main/java/org/neo4j/server/security/auth/UserManager.java @@ -32,5 +32,5 @@ User newUser( String username, String initialPassword, boolean requirePasswordCh User getUser( String username ); - User setUserPassword( String username, String password ) throws IOException; + void setUserPassword( String username, String password ) throws IOException, IllegalCredentialsException; } diff --git a/community/security/src/test/java/org/neo4j/server/security/auth/BasicAuthManagerTest.java b/community/security/src/test/java/org/neo4j/server/security/auth/BasicAuthManagerTest.java index b5a42efc63e58..13983dfd2ed6a 100644 --- a/community/security/src/test/java/org/neo4j/server/security/auth/BasicAuthManagerTest.java +++ b/community/security/src/test/java/org/neo4j/server/security/auth/BasicAuthManagerTest.java @@ -22,6 +22,7 @@ import org.junit.Test; import org.neo4j.kernel.api.security.AuthenticationResult; +import org.neo4j.kernel.api.security.exception.IllegalCredentialsException; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertNotNull; @@ -194,9 +195,10 @@ public void shouldSetPassword() throws Throwable manager.start(); // When - User user = manager.setUserPassword( "jake", "hello, world!" ); + manager.setUserPassword( "jake", "hello, world!" ); // Then + User user = manager.getUser( "jake" ); assertTrue( user.credentials().matchesPassword( "hello, world!" ) ); assertThat( users.findByName( "jake" ), equalTo( user ) ); } @@ -210,10 +212,15 @@ public void shouldReturnNullWhenSettingPasswordForUnknownUser() throws Throwable manager.start(); // When - User user = manager.setUserPassword( "unknown", "hello, world!" ); - - // Then - assertNull( user ); + try + { + manager.setUserPassword( "unknown", "hello, world!" ); + fail( "exception expected" ); + } + catch ( IllegalCredentialsException e ) + { + // expected + } } @Test diff --git a/community/server/src/main/java/org/neo4j/server/rest/dbms/UserService.java b/community/server/src/main/java/org/neo4j/server/rest/dbms/UserService.java index 41dcd78a7956a..e29bafc856dff 100644 --- a/community/server/src/main/java/org/neo4j/server/rest/dbms/UserService.java +++ b/community/server/src/main/java/org/neo4j/server/rest/dbms/UserService.java @@ -32,6 +32,7 @@ import org.neo4j.kernel.api.security.AuthManager; import org.neo4j.kernel.api.exceptions.Status; +import org.neo4j.kernel.api.security.exception.IllegalCredentialsException; import org.neo4j.server.rest.repr.AuthorizationRepresentation; import org.neo4j.server.rest.repr.BadInputException; import org.neo4j.server.rest.repr.ExceptionRepresentation; @@ -134,15 +135,17 @@ public Response setPassword( @PathParam("username") String username, @Context Ht try { - if ( userManager.setUserPassword( username, newPassword ) == null ) - { - return output.notFound(); - } + userManager.setUserPassword( username, newPassword ); } catch ( IOException e ) { return output.serverErrorWithoutLegacyStacktrace( e ); } + catch ( IllegalCredentialsException e ) + { + return output.response( UNPROCESSABLE, new ExceptionRepresentation( + new Neo4jError( e.status(), e.getMessage() ) ) ); + } return output.ok(); } diff --git a/community/server/src/test/java/org/neo4j/server/rest/dbms/UserServiceTest.java b/community/server/src/test/java/org/neo4j/server/rest/dbms/UserServiceTest.java index 6b01ec3964c47..ce05346de33ae 100644 --- a/community/server/src/test/java/org/neo4j/server/rest/dbms/UserServiceTest.java +++ b/community/server/src/test/java/org/neo4j/server/rest/dbms/UserServiceTest.java @@ -150,7 +150,6 @@ public void shouldChangePasswordAndReturnSuccess() throws Exception BasicAuthManager authManager = mock( BasicAuthManager.class ); when( authManager.getUser( "neo4j" ) ).thenReturn( NEO4J_USER ); - when( authManager.setUserPassword( "neo4j", "test" ) ).thenReturn( NEO4J_USER ); OutputFormat outputFormat = new EntityOutputFormat( new JsonFormat(), new URI( "http://www.example.com" ), null ); UserService userService = new UserService( authManager, new JsonFormat(), outputFormat );