Skip to content

Commit

Permalink
Change setUserPassword to void
Browse files Browse the repository at this point in the history
Throw IllegalCredentialsException instead of returning null on failure.
  • Loading branch information
henriknyman committed May 11, 2016
1 parent 921bd8c commit 8f1cda5
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 23 deletions.
Expand Up @@ -27,7 +27,6 @@
import org.neo4j.kernel.api.security.AuthSubject; import org.neo4j.kernel.api.security.AuthSubject;
import org.neo4j.kernel.api.security.AuthenticationResult; import org.neo4j.kernel.api.security.AuthenticationResult;
import org.neo4j.kernel.api.security.exception.IllegalCredentialsException; import org.neo4j.kernel.api.security.exception.IllegalCredentialsException;
import org.neo4j.kernel.lifecycle.Lifecycle;
import org.neo4j.server.security.auth.exception.ConcurrentModificationException; import org.neo4j.server.security.auth.exception.ConcurrentModificationException;


/** /**
Expand All @@ -39,7 +38,7 @@
* so the given UserRepository should not be added to another LifeSupport. * so the given UserRepository should not be added to another LifeSupport.
* </p> * </p>
*/ */
public class BasicAuthManager implements Lifecycle, AuthManager, UserManager public class BasicAuthManager implements AuthManager, UserManager
{ {
private final AuthenticationStrategy authStrategy; private final AuthenticationStrategy authStrategy;
private final UserRepository users; private final UserRepository users;
Expand Down Expand Up @@ -145,7 +144,8 @@ public User getUser( String username )
return users.findByName( 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) ) if ( !(authSubject instanceof BasicAuthSubject) )
{ {
Expand All @@ -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 ); throw new AuthorizationViolationException( "Invalid attempt to change the password for user " + username );
} }
if ( setUserPassword( username, password ) == null ) setUserPassword( username, password );
{
throw new IllegalArgumentException( "User " + username + " does not exist" );
}
} }


@Override @Override
public User setUserPassword( String username, String password ) throws IOException public void setUserPassword( String username, String password ) throws IOException,
IllegalCredentialsException
{ {
assertAuthEnabled(); assertAuthEnabled();
User existingUser = users.findByName( username ); User existingUser = users.findByName( username );
if ( existingUser == null ) if ( existingUser == null )
{ {
return null; throw new IllegalCredentialsException( "User " + username + " does not exist" );
} }


if ( existingUser.credentials().matchesPassword( password ) ) if ( existingUser.credentials().matchesPassword( password ) )
{ {
return existingUser; return;
} }


try try
Expand All @@ -185,11 +183,10 @@ public User setUserPassword( String username, String password ) throws IOExcepti
.withRequiredPasswordChange( false ) .withRequiredPasswordChange( false )
.build(); .build();
users.update( existingUser, updatedUser ); users.update( existingUser, updatedUser );
return updatedUser;
} catch ( ConcurrentModificationException e ) } catch ( ConcurrentModificationException e )
{ {
// try again // try again
return setUserPassword( username, password ); setUserPassword( username, password );
} }
} }


Expand Down
Expand Up @@ -32,5 +32,5 @@ User newUser( String username, String initialPassword, boolean requirePasswordCh


User getUser( String username ); User getUser( String username );


User setUserPassword( String username, String password ) throws IOException; void setUserPassword( String username, String password ) throws IOException, IllegalCredentialsException;
} }
Expand Up @@ -22,6 +22,7 @@
import org.junit.Test; import org.junit.Test;


import org.neo4j.kernel.api.security.AuthenticationResult; import org.neo4j.kernel.api.security.AuthenticationResult;
import org.neo4j.kernel.api.security.exception.IllegalCredentialsException;


import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;
Expand Down Expand Up @@ -194,9 +195,10 @@ public void shouldSetPassword() throws Throwable
manager.start(); manager.start();


// When // When
User user = manager.setUserPassword( "jake", "hello, world!" ); manager.setUserPassword( "jake", "hello, world!" );


// Then // Then
User user = manager.getUser( "jake" );
assertTrue( user.credentials().matchesPassword( "hello, world!" ) ); assertTrue( user.credentials().matchesPassword( "hello, world!" ) );
assertThat( users.findByName( "jake" ), equalTo( user ) ); assertThat( users.findByName( "jake" ), equalTo( user ) );
} }
Expand All @@ -210,10 +212,15 @@ public void shouldReturnNullWhenSettingPasswordForUnknownUser() throws Throwable
manager.start(); manager.start();


// When // When
User user = manager.setUserPassword( "unknown", "hello, world!" ); try

{
// Then manager.setUserPassword( "unknown", "hello, world!" );
assertNull( user ); fail( "exception expected" );
}
catch ( IllegalCredentialsException e )
{
// expected
}
} }


@Test @Test
Expand Down
Expand Up @@ -32,6 +32,7 @@


import org.neo4j.kernel.api.security.AuthManager; import org.neo4j.kernel.api.security.AuthManager;
import org.neo4j.kernel.api.exceptions.Status; 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.AuthorizationRepresentation;
import org.neo4j.server.rest.repr.BadInputException; import org.neo4j.server.rest.repr.BadInputException;
import org.neo4j.server.rest.repr.ExceptionRepresentation; import org.neo4j.server.rest.repr.ExceptionRepresentation;
Expand Down Expand Up @@ -134,15 +135,17 @@ public Response setPassword( @PathParam("username") String username, @Context Ht


try try
{ {
if ( userManager.setUserPassword( username, newPassword ) == null ) userManager.setUserPassword( username, newPassword );
{
return output.notFound();
}
} }
catch ( IOException e ) catch ( IOException e )
{ {
return output.serverErrorWithoutLegacyStacktrace( e ); return output.serverErrorWithoutLegacyStacktrace( e );
} }
catch ( IllegalCredentialsException e )
{
return output.response( UNPROCESSABLE, new ExceptionRepresentation(
new Neo4jError( e.status(), e.getMessage() ) ) );
}
return output.ok(); return output.ok();
} }


Expand Down
Expand Up @@ -150,7 +150,6 @@ public void shouldChangePasswordAndReturnSuccess() throws Exception


BasicAuthManager authManager = mock( BasicAuthManager.class ); BasicAuthManager authManager = mock( BasicAuthManager.class );
when( authManager.getUser( "neo4j" ) ).thenReturn( NEO4J_USER ); 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 ); OutputFormat outputFormat = new EntityOutputFormat( new JsonFormat(), new URI( "http://www.example.com" ), null );
UserService userService = new UserService( authManager, new JsonFormat(), outputFormat ); UserService userService = new UserService( authManager, new JsonFormat(), outputFormat );
Expand Down

0 comments on commit 8f1cda5

Please sign in to comment.