Skip to content

Commit

Permalink
Add tests that assert passwords are cleared
Browse files Browse the repository at this point in the history
  • Loading branch information
@henriknyman
henriknyman committed Oct 2, 2018
1 parent e72c45b commit 61005af
Show file tree
Hide file tree
Showing 4 changed files with 310 additions and 1 deletion.
53 changes: 53 additions & 0 deletions community/bolt/src/test/java/org/neo4j/bolt/security/auth/BasicAuthenticationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
*/
package org.neo4j.bolt.security.auth;

import org.hamcrest.BaseMatcher;
import org.hamcrest.Description;
import org.hamcrest.TypeSafeMatcher;
import org.junit.Before;
Expand Down Expand Up @@ -127,6 +128,26 @@ public void shouldBeAbleToUpdateCredentials() throws Exception
authentication.authenticate( map( "scheme", "basic", "principal", "mike", "credentials", UTF8.encode( "secret" ) ) );
}

@Test
public void shouldClearCredentialsAfterUse() throws Exception
{
// When
byte[] oldPassword = UTF8.encode( "secret2" );
byte[] newPassword1 = UTF8.encode( "secret" );
byte[] newPassword2 = UTF8.encode( "secret" );

authentication.authenticate(
map( "scheme", "basic", "principal", "mike", "credentials", oldPassword,
"new_credentials", newPassword1 ) );

authentication.authenticate( map( "scheme", "basic", "principal", "mike", "credentials", newPassword2 ) );

// Then
assertThat( oldPassword, isCleared() );
assertThat( newPassword1, isCleared() );
assertThat( newPassword2, isCleared() );
}

@Test
public void shouldBeAbleToUpdateExpiredCredentials() throws Exception
{
Expand Down Expand Up @@ -242,4 +263,36 @@ protected void describeMismatchSafely( Status.HasStatus item, Description mismat
.appendValue( item.status() );
}
}

static CredentialsClearedMatcher isCleared()
{
return new CredentialsClearedMatcher();
}

static class CredentialsClearedMatcher extends BaseMatcher<byte[]>
{
@Override
public boolean matches( Object o )
{
if ( o instanceof byte[] )
{
byte[] bytes = (byte[]) o;
for ( int i = 0; i < bytes.length; i++ )
{
if ( bytes[i] != (byte) 0 )
{
return false;
}
}
return true;
}
return false;
}

@Override
public void describeTo( Description description )
{
description.appendText( "Byte array should contain only zeroes" );
}
}
}
131 changes: 131 additions & 0 deletions community/security/src/test/java/org/neo4j/server/security/auth/BasicAuthManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import org.junit.Test;

import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Map;

import org.neo4j.internal.kernel.api.security.AuthenticationResult;
import org.neo4j.internal.kernel.api.security.LoginContext;
Expand All @@ -44,6 +46,7 @@
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.neo4j.helpers.collection.MapUtil.map;
Expand Down Expand Up @@ -208,6 +211,127 @@ public void shouldSetPassword() throws Throwable
assertThat( users.getUserByName( "jake" ), equalTo( user ) );
}

@Test
public void shouldClearPasswordOnLogin() throws Throwable
{
// Given
when( authStrategy.authenticate( any(), any() ) ).thenReturn( AuthenticationResult.SUCCESS );

manager.start();
manager.newUser( "jake", password( "abc123" ), true );
byte[] password = password( "abc123" );
Map<String,Object> authToken = AuthToken.newBasicAuthToken( "jake", password );

// When
manager.login( authToken );

// Then
assertThat( password, equalTo( clearedPasswordWithSameLenghtAs( "abc123" ) ) );
assertThat( authToken.get( AuthToken.CREDENTIALS ), equalTo( clearedPasswordWithSameLenghtAs( "abc123" ) ) );
}

@Test
public void shouldClearPasswordOnInvalidAuthToken() throws Throwable
{
// Given
manager.start();
byte[] password = password( "abc123" );
Map<String,Object> authToken = AuthToken.newBasicAuthToken( "jake", password );
authToken.put( AuthToken.SCHEME_KEY, null ); // Null is not a valid scheme

// When
try
{
manager.login( authToken );
fail( "exception expected" );
}
catch ( InvalidAuthTokenException e )
{
// expected
}
assertThat( password, equalTo( clearedPasswordWithSameLenghtAs( "abc123" ) ) );
assertThat( authToken.get( AuthToken.CREDENTIALS ), equalTo( clearedPasswordWithSameLenghtAs( "abc123" ) ) );
}

@Test
public void shouldClearPasswordOnNewUser() throws Throwable
{
// Given
manager.start();
byte[] password = password( "abc123" );

// When
manager.newUser( "jake", password, true );

// Then
assertThat( password, equalTo( clearedPasswordWithSameLenghtAs( "abc123" ) ) );
User user = manager.getUser( "jake" );
assertTrue( user.credentials().matchesPassword( "abc123" ) );
}

@Test
public void shouldClearPasswordOnNewUserAlreadyExists() throws Throwable
{
// Given
manager.start();
manager.newUser( "jake", password( "abc123" ), true );
byte[] password = password( "abc123" );

// When
try
{
manager.newUser( "jake", password, true );
fail( "exception expected" );
}
catch ( InvalidArgumentsException e )
{
// expected
}

// Then
assertThat( password, equalTo( clearedPasswordWithSameLenghtAs( "abc123" ) ) );
}

@Test
public void shouldClearPasswordOnSetUserPassword() throws Throwable
{
// Given
manager.start();
manager.newUser( "jake", password( "old" ), false );
byte[] newPassword = password( "abc123" );

// When
manager.setUserPassword( "jake", newPassword, false );

// Then
assertThat( newPassword, equalTo( clearedPasswordWithSameLenghtAs( "abc123" ) ) );
User user = manager.getUser( "jake" );
assertTrue( user.credentials().matchesPassword( "abc123" ) );
}

@Test
public void shouldClearPasswordOnSetUserPasswordWithInvalidPassword() throws Throwable
{
// Given
manager.start();
manager.newUser( "jake", password( "abc123" ), false );
byte[] newPassword = password( "abc123" );

// When
try
{
manager.setUserPassword( "jake", newPassword, false );
fail( "exception expected" );
}
catch ( InvalidArgumentsException e )
{
// expected
}

// Then
assertThat( newPassword, equalTo( clearedPasswordWithSameLenghtAs( "abc123" ) ) );
}

@Test
public void shouldReturnNullWhenSettingPasswordForUnknownUser() throws Throwable
{
Expand Down Expand Up @@ -274,4 +398,11 @@ public static byte[] password( String passwordString )
{
return passwordString.getBytes( StandardCharsets.UTF_8 );
}

public static byte[] clearedPasswordWithSameLenghtAs( String passwordString )
{
byte[] password = passwordString.getBytes( StandardCharsets.UTF_8 );
Arrays.fill( password, (byte) 0 );
return password;
}
}
3 changes: 2 additions & 1 deletion community/server/src/main/java/org/neo4j/server/rest/dbms/UserService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.neo4j.server.rest.repr.InputFormat;
import org.neo4j.server.rest.repr.OutputFormat;
import org.neo4j.server.rest.transactional.error.Neo4jError;
import org.neo4j.string.UTF8;

import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
import static org.neo4j.server.rest.dbms.AuthorizedRequestWrapper.getLoginContextFromUserPrincipal;
Expand Down Expand Up @@ -133,7 +134,7 @@ public Response setPassword( @PathParam( "username" ) String username, @Context
else
{
UserManager userManager = userManagerSupplier.getUserManager( loginContext.subject(), false );
userManager.setUserPassword( username, newPassword, false );
userManager.setUserPassword( username, UTF8.encode( newPassword ), false );
}
}
catch ( IOException e )
Expand Down
124 changes: 124 additions & 0 deletions ...ty/src/test/java/org/neo4j/server/security/enterprise/auth/MultiRealmAuthManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

import java.util.Collections;
import java.util.function.ToIntFunction;
import java.util.Map;

import org.neo4j.commandline.admin.security.SetDefaultAdminCommand;
import org.neo4j.graphdb.factory.GraphDatabaseSettings;
Expand Down Expand Up @@ -66,11 +67,13 @@
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.neo4j.helpers.Strings.escape;
import static org.neo4j.helpers.collection.MapUtil.map;
import static org.neo4j.logging.AssertableLogProvider.inLog;
import static org.neo4j.server.security.auth.BasicAuthManagerTest.clearedPasswordWithSameLenghtAs;
import static org.neo4j.server.security.auth.BasicAuthManagerTest.password;
import static org.neo4j.server.security.auth.SecurityTestUtils.authToken;
import static org.neo4j.test.assertion.Assert.assertException;
Expand Down Expand Up @@ -689,6 +692,127 @@ public void shouldHaveNoPermissionsAfterLogout() throws Throwable
assertFalse( securityContext.mode().allowsSchemaWrites() );
}

@Test
public void shouldClearPasswordOnLogin() throws Throwable
{
// Given
when( authStrategy.authenticate( any(), any() ) ).thenReturn( AuthenticationResult.SUCCESS );

manager.start();
userManager.newUser( "jake", password( "abc123" ), true );
byte[] password = password( "abc123" );
Map<String,Object> authToken = AuthToken.newBasicAuthToken( "jake", password );

// When
manager.login( authToken );

// Then
assertThat( password, equalTo( clearedPasswordWithSameLenghtAs( "abc123" ) ) );
assertThat( authToken.get( AuthToken.CREDENTIALS ), equalTo( clearedPasswordWithSameLenghtAs( "abc123" ) ) );
}

@Test
public void shouldClearPasswordOnInvalidAuthToken() throws Throwable
{
// Given
manager.start();
byte[] password = password( "abc123" );
Map<String,Object> authToken = AuthToken.newBasicAuthToken( "jake", password );
authToken.put( AuthToken.SCHEME_KEY, null ); // Null is not a valid scheme

// When
try
{
manager.login( authToken );
fail( "exception expected" );
}
catch ( InvalidAuthTokenException e )
{
// expected
}
assertThat( password, equalTo( clearedPasswordWithSameLenghtAs( "abc123" ) ) );
assertThat( authToken.get( AuthToken.CREDENTIALS ), equalTo( clearedPasswordWithSameLenghtAs( "abc123" ) ) );
}

@Test
public void shouldClearPasswordOnNewUser() throws Throwable
{
// Given
manager.start();
byte[] password = password( "abc123" );

// When
userManager.newUser( "jake", password, true );

// Then
assertThat( password, equalTo( clearedPasswordWithSameLenghtAs( "abc123" ) ) );
User user = userManager.getUser( "jake" );
assertTrue( user.credentials().matchesPassword( "abc123" ) );
}

@Test
public void shouldClearPasswordOnNewUserAlreadyExists() throws Throwable
{
// Given
manager.start();
userManager.newUser( "jake", password( "abc123" ), true );
byte[] password = password( "abc123" );

// When
try
{
userManager.newUser( "jake", password, true );
fail( "exception expected" );
}
catch ( InvalidArgumentsException e )
{
// expected
}

// Then
assertThat( password, equalTo( clearedPasswordWithSameLenghtAs( "abc123" ) ) );
}

@Test
public void shouldClearPasswordOnSetUserPassword() throws Throwable
{
// Given
manager.start();
userManager.newUser( "jake", password( "old" ), false );
byte[] newPassword = password( "abc123" );

// When
userManager.setUserPassword( "jake", newPassword, false );

// Then
assertThat( newPassword, equalTo( clearedPasswordWithSameLenghtAs( "abc123" ) ) );
User user = userManager.getUser( "jake" );
assertTrue( user.credentials().matchesPassword( "abc123" ) );
}

@Test
public void shouldClearPasswordOnSetUserPasswordWithInvalidPassword() throws Throwable
{
// Given
manager.start();
userManager.newUser( "jake", password( "abc123" ), false );
byte[] newPassword = password( "abc123" );

// When
try
{
userManager.setUserPassword( "jake", newPassword, false );
fail( "exception expected" );
}
catch ( InvalidArgumentsException e )
{
// expected
}

// Then
assertThat( newPassword, equalTo( clearedPasswordWithSameLenghtAs( "abc123" ) ) );
}

private AssertableLogProvider.LogMatcher info( String message )
{
return inLog( this.getClass() ).info( message );
Expand Down

0 comments on commit 61005af

Please sign in to comment.