Skip to content

Commit

Permalink
Log authentication failures
Browse files Browse the repository at this point in the history
  • Loading branch information
@pontusmelke
pontusmelke committed Feb 21, 2016
1 parent d29fae5 commit 27ca8dd
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 38 deletions.
9 changes: 6 additions & 3 deletions community/bolt/src/main/java/org/neo4j/bolt/security/auth/BasicAuthentication.java
View file
Expand Up @@ -23,6 +23,8 @@
import java.util.Map; import java.util.Map;


import org.neo4j.kernel.api.exceptions.Status; import org.neo4j.kernel.api.exceptions.Status;
import org.neo4j.logging.Log;
import org.neo4j.logging.LogProvider;
import org.neo4j.server.security.auth.AuthManager; import org.neo4j.server.security.auth.AuthManager;


/** /**
Expand All @@ -32,11 +34,13 @@ public class BasicAuthentication implements Authentication
{ {
private final AuthManager authManager; private final AuthManager authManager;
private final static String SCHEME = "basic"; private final static String SCHEME = "basic";
private final Log log;




public BasicAuthentication( AuthManager authManager ) public BasicAuthentication( AuthManager authManager, LogProvider logProvider )
{ {
this.authManager = authManager; this.authManager = authManager;
this.log = logProvider.getLog( getClass() );
} }


@Override @Override
Expand Down Expand Up @@ -70,9 +74,8 @@ private void authenticate( String user, String password ) throws AuthenticationE
throw new AuthenticationException( Status.Security.CredentialsExpired ); throw new AuthenticationException( Status.Security.CredentialsExpired );
case TOO_MANY_ATTEMPTS: case TOO_MANY_ATTEMPTS:
throw new AuthenticationException( Status.Security.AuthenticationRateLimit ); throw new AuthenticationException( Status.Security.AuthenticationRateLimit );
case FAILURE:
throw new AuthenticationException( Status.Security.AuthenticationFailed );
default: default:
log.warn( "Failed authentication attempt for '%s'", user);
throw new AuthenticationException( Status.Security.AuthenticationFailed ); throw new AuthenticationException( Status.Security.AuthenticationFailed );
} }
} }
Expand Down
2 changes: 1 addition & 1 deletion community/bolt/src/main/java/org/neo4j/bolt/v1/runtime/internal/StandardSessions.java
View file
Expand Up @@ -99,7 +99,7 @@ private Authentication authentication( DependencyResolver dependencyResolver )


if ( config.get( GraphDatabaseSettings.auth_enabled ) ) if ( config.get( GraphDatabaseSettings.auth_enabled ) )
{ {
return new BasicAuthentication( dependencyResolver.resolveDependency( AuthManager.class ) ); return new BasicAuthentication( dependencyResolver.resolveDependency( AuthManager.class ), logging.getUserLogProvider() );
} }
else else
{ {
Expand Down
85 changes: 51 additions & 34 deletions community/bolt/src/test/java/org/neo4j/bolt/security/auth/BasicAuthenticationTest.java
View file
Expand Up @@ -27,12 +27,15 @@
import org.junit.rules.ExpectedException; import org.junit.rules.ExpectedException;


import org.neo4j.kernel.api.exceptions.Status; import org.neo4j.kernel.api.exceptions.Status;
import org.neo4j.logging.Log;
import org.neo4j.logging.LogProvider;
import org.neo4j.server.security.auth.AuthManager; import org.neo4j.server.security.auth.AuthManager;
import org.neo4j.server.security.auth.AuthenticationResult; import org.neo4j.server.security.auth.AuthenticationResult;


import static java.util.Collections.singletonList; import static java.util.Collections.singletonList;
import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.anyString;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
import static org.neo4j.helpers.collection.MapUtil.map; import static org.neo4j.helpers.collection.MapUtil.map;


Expand All @@ -46,21 +49,24 @@ public void shouldNotDoAnythingOnSuccess() throws AuthenticationException
{ {
// Given // Given
AuthManager manager = mock( AuthManager.class ); AuthManager manager = mock( AuthManager.class );
BasicAuthentication authentication = new BasicAuthentication( manager ); BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ) );
when( manager.authenticate( anyString(), anyString() ) ).thenReturn( AuthenticationResult.SUCCESS ); when( manager.authenticate( anyString(), anyString() ) ).thenReturn( AuthenticationResult.SUCCESS );


//Expect nothing //Expect nothing


// When // When
authentication.authenticate( map("scheme", "basic", "principal", "bob", "credentials", "secret") ); authentication.authenticate( map( "scheme", "basic", "principal", "bob", "credentials", "secret" ) );
} }


@Test @Test
public void shouldThrowOnFailure() throws AuthenticationException public void shouldThrowAndLogOnFailure() throws AuthenticationException
{ {
// Given // Given
AuthManager manager = mock( AuthManager.class ); AuthManager manager = mock( AuthManager.class );
BasicAuthentication authentication = new BasicAuthentication( manager ); Log log = mock( Log.class );
LogProvider logProvider = mock( LogProvider.class );
when( logProvider.getLog( BasicAuthentication.class ) ).thenReturn( log );
BasicAuthentication authentication = new BasicAuthentication( manager, logProvider );
when( manager.authenticate( anyString(), anyString() ) ).thenReturn( AuthenticationResult.FAILURE ); when( manager.authenticate( anyString(), anyString() ) ).thenReturn( AuthenticationResult.FAILURE );


// Expect // Expect
Expand All @@ -69,32 +75,36 @@ public void shouldThrowOnFailure() throws AuthenticationException
exception.expectMessage( "The client provided an incorrect username and/or password." ); exception.expectMessage( "The client provided an incorrect username and/or password." );


// When // When
authentication.authenticate( map("scheme", "basic", "principal", "bob", "credentials", "secret") ); authentication.authenticate( map( "scheme", "basic", "principal", "bob", "credentials", "secret" ) );

//Then
verify( log ).warn( "Failed authentication attempt for 'bob')" );
} }


@Test @Test
public void shouldIndicateThatCredentialsExpired() throws AuthenticationException public void shouldIndicateThatCredentialsExpired() throws AuthenticationException
{ {
// Given // Given
AuthManager manager = mock( AuthManager.class ); AuthManager manager = mock( AuthManager.class );
BasicAuthentication authentication = new BasicAuthentication( manager ); BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ) );
when( manager.authenticate( anyString(), anyString() ) ).thenReturn( AuthenticationResult.PASSWORD_CHANGE_REQUIRED ); when( manager.authenticate( anyString(), anyString() ) )
.thenReturn( AuthenticationResult.PASSWORD_CHANGE_REQUIRED );


// Expect // Expect
exception.expect( AuthenticationException.class ); exception.expect( AuthenticationException.class );
exception.expect( hasStatus( Status.Security.CredentialsExpired ) ); exception.expect( hasStatus( Status.Security.CredentialsExpired ) );
exception.expectMessage( "The credentials have expired and needs to be updated." ); exception.expectMessage( "The credentials have expired and needs to be updated." );


// When // When
authentication.authenticate( map("scheme", "basic", "principal", "bob", "credentials", "secret") ); authentication.authenticate( map( "scheme", "basic", "principal", "bob", "credentials", "secret" ) );
} }


@Test @Test
public void shouldFailWhenTooManyAttempts() throws AuthenticationException public void shouldFailWhenTooManyAttempts() throws AuthenticationException
{ {
// Given // Given
AuthManager manager = mock( AuthManager.class ); AuthManager manager = mock( AuthManager.class );
BasicAuthentication authentication = new BasicAuthentication( manager ); BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ) );
when( manager.authenticate( anyString(), anyString() ) ).thenReturn( AuthenticationResult.TOO_MANY_ATTEMPTS ); when( manager.authenticate( anyString(), anyString() ) ).thenReturn( AuthenticationResult.TOO_MANY_ATTEMPTS );


// Expect // Expect
Expand All @@ -103,45 +113,46 @@ public void shouldFailWhenTooManyAttempts() throws AuthenticationException
exception.expectMessage( "The client has provided incorrect authentication details too many times in a row." ); exception.expectMessage( "The client has provided incorrect authentication details too many times in a row." );


// When // When
authentication.authenticate( map("scheme", "basic", "principal", "bob", "credentials", "secret") ); authentication.authenticate( map( "scheme", "basic", "principal", "bob", "credentials", "secret" ) );
} }


@Test @Test
public void shouldBeAbleToUpdateCredentials() throws AuthenticationException public void shouldBeAbleToUpdateCredentials() throws AuthenticationException
{ {
// Given // Given
AuthManager manager = mock( AuthManager.class ); AuthManager manager = mock( AuthManager.class );
BasicAuthentication authentication = new BasicAuthentication( manager ); BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ) );
when( manager.authenticate( anyString(), anyString() ) ).thenReturn( AuthenticationResult.SUCCESS ); when( manager.authenticate( anyString(), anyString() ) ).thenReturn( AuthenticationResult.SUCCESS );


//Expect nothing //Expect nothing


// When // When
authentication.authenticate( map("scheme", "basic", "principal", "bob", "credentials", "secret", authentication.authenticate( map( "scheme", "basic", "principal", "bob", "credentials", "secret",
"new-credentials", "secret2") ); "new-credentials", "secret2" ) );
} }


@Test @Test
public void shouldBeAbleToUpdateExpiredCredentials() throws AuthenticationException public void shouldBeAbleToUpdateExpiredCredentials() throws AuthenticationException
{ {
// Given // Given
AuthManager manager = mock( AuthManager.class ); AuthManager manager = mock( AuthManager.class );
BasicAuthentication authentication = new BasicAuthentication( manager ); BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ) );
when( manager.authenticate( anyString(), anyString() ) ).thenReturn( AuthenticationResult.PASSWORD_CHANGE_REQUIRED ); when( manager.authenticate( anyString(), anyString() ) )
.thenReturn( AuthenticationResult.PASSWORD_CHANGE_REQUIRED );


//Expect nothing //Expect nothing


// When // When
authentication.authenticate( map("scheme", "basic", "principal", "bob", "credentials", "secret", authentication.authenticate( map( "scheme", "basic", "principal", "bob", "credentials", "secret",
"new-credentials", "secret2") ); "new-credentials", "secret2" ) );
} }


@Test @Test
public void shouldNotBeAbleToUpdateCredentialsIfOldCredentialsAreInvalid() throws AuthenticationException public void shouldNotBeAbleToUpdateCredentialsIfOldCredentialsAreInvalid() throws AuthenticationException
{ {
// Given // Given
AuthManager manager = mock( AuthManager.class ); AuthManager manager = mock( AuthManager.class );
BasicAuthentication authentication = new BasicAuthentication( manager ); BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ) );
when( manager.authenticate( anyString(), anyString() ) ).thenReturn( AuthenticationResult.FAILURE ); when( manager.authenticate( anyString(), anyString() ) ).thenReturn( AuthenticationResult.FAILURE );


// Expect // Expect
Expand All @@ -151,16 +162,16 @@ public void shouldNotBeAbleToUpdateCredentialsIfOldCredentialsAreInvalid() throw


// When // When
// When // When
authentication.authenticate( map("scheme", "basic", "principal", "bob", "credentials", "secret", authentication.authenticate( map( "scheme", "basic", "principal", "bob", "credentials", "secret",
"new-credentials", "secret2") ); "new-credentials", "secret2" ) );
} }


@Test @Test
public void shouldFailOnUnknownScheme() throws AuthenticationException public void shouldFailOnUnknownScheme() throws AuthenticationException
{ {
// Given // Given
AuthManager manager = mock( AuthManager.class ); AuthManager manager = mock( AuthManager.class );
BasicAuthentication authentication = new BasicAuthentication( manager ); BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ) );
when( manager.authenticate( anyString(), anyString() ) ).thenReturn( AuthenticationResult.SUCCESS ); when( manager.authenticate( anyString(), anyString() ) ).thenReturn( AuthenticationResult.SUCCESS );


// Expect // Expect
Expand All @@ -169,27 +180,29 @@ public void shouldFailOnUnknownScheme() throws AuthenticationException
exception.expectMessage( "Authorization token must contain: 'scheme : basic'" ); exception.expectMessage( "Authorization token must contain: 'scheme : basic'" );


// When // When
authentication.authenticate( map("scheme", "UNKNOWN", "principal", "bob", "credentials", "secret") ); authentication.authenticate( map( "scheme", "UNKNOWN", "principal", "bob", "credentials", "secret" ) );
} }


@Test @Test
public void shouldFailOnMalformedToken() throws AuthenticationException public void shouldFailOnMalformedToken() throws AuthenticationException
{ {
// Given // Given
AuthManager manager = mock( AuthManager.class ); AuthManager manager = mock( AuthManager.class );
BasicAuthentication authentication = new BasicAuthentication( manager ); BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ) );
when( manager.authenticate( anyString(), anyString() ) ).thenReturn( AuthenticationResult.SUCCESS ); when( manager.authenticate( anyString(), anyString() ) ).thenReturn( AuthenticationResult.SUCCESS );


// Expect // Expect
exception.expect( AuthenticationException.class ); exception.expect( AuthenticationException.class );
exception.expect( hasStatus( Status.Security.AuthenticationFailed ) ); exception.expect( hasStatus( Status.Security.AuthenticationFailed ) );
exception.expectMessage( "The value associated with the key `principal` must be a String but was: SingletonList" ); exception.expectMessage(
"The value associated with the key `principal` must be a String but was: SingletonList" );


// When // When
authentication.authenticate( map("scheme", "basic", "principal", singletonList( "bob" ), "credentials", "secret") ); authentication
.authenticate( map( "scheme", "basic", "principal", singletonList( "bob" ), "credentials", "secret" ) );
} }


private HasStatus hasStatus(Status status) private HasStatus hasStatus( Status status )
{ {
return new HasStatus( status ); return new HasStatus( status );
} }
Expand All @@ -198,25 +211,29 @@ static class HasStatus extends TypeSafeMatcher<Status.HasStatus>
{ {
private Status status; private Status status;


public HasStatus(Status status) { public HasStatus( Status status )
{
this.status = status; this.status = status;
} }


@Override @Override
protected boolean matchesSafely(Status.HasStatus item) { protected boolean matchesSafely( Status.HasStatus item )
{
return item.status() == status; return item.status() == status;
} }


@Override @Override
public void describeTo(Description description) { public void describeTo( Description description )
description.appendText("expects status ") {
.appendValue(status); description.appendText( "expects status " )
.appendValue( status );
} }


@Override @Override
protected void describeMismatchSafely(Status.HasStatus item, Description mismatchDescription) { protected void describeMismatchSafely( Status.HasStatus item, Description mismatchDescription )
mismatchDescription.appendText("was ") {
.appendValue(item.status()); mismatchDescription.appendText( "was " )
.appendValue( item.status() );
} }
} }


Expand Down

0 comments on commit 27ca8dd

Please sign in to comment.