From 8bc652f0c298a1d301f267b42b46efbf6c39a1ce Mon Sep 17 00:00:00 2001 From: Mats Rydberg Date: Fri, 2 Sep 2016 15:27:46 +0200 Subject: [PATCH] Add tests for logging creation of roles --- .../neo4j/logging/AssertableLogProvider.java | 2 +- .../enterprise/auth/AuthProcedures.java | 2 +- .../auth/AuthProceduresLoggingTest.java | 72 ++++++++++++++++--- .../AuthScenariosInteractionTestBase.java | 6 +- 4 files changed, 67 insertions(+), 15 deletions(-) diff --git a/community/logging/src/test/java/org/neo4j/logging/AssertableLogProvider.java b/community/logging/src/test/java/org/neo4j/logging/AssertableLogProvider.java index c6dfdda5e76a..a90841e04d1d 100644 --- a/community/logging/src/test/java/org/neo4j/logging/AssertableLogProvider.java +++ b/community/logging/src/test/java/org/neo4j/logging/AssertableLogProvider.java @@ -149,7 +149,7 @@ public String toString() builder.append( ',' ); } first = false; - builder.append( escapeJava( arg.toString() ) ); + builder.append( escapeJava( "" + arg ) ); } builder.append( "]" ); } else diff --git a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AuthProcedures.java b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AuthProcedures.java index 496278beb49f..9493e42154dc 100644 --- a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AuthProcedures.java +++ b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AuthProcedures.java @@ -361,7 +361,7 @@ public void createRole( @Name( "roleName" ) String roleName ) } catch ( Exception e ) { - securityLog.error( authSubject, "tried to create role `%s`. %s", roleName, e.getMessage() ); + securityLog.error( authSubject, "tried to create role `%s`: %s", roleName, e.getMessage() ); throw e; } } diff --git a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthProceduresLoggingTest.java b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthProceduresLoggingTest.java index 1070f2bce9c4..1e70a96d78f1 100644 --- a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthProceduresLoggingTest.java +++ b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthProceduresLoggingTest.java @@ -26,6 +26,7 @@ import org.junit.Before; import org.junit.Test; +import org.neo4j.function.ThrowingAction; import org.neo4j.graphdb.security.AuthorizationViolationException; import org.neo4j.kernel.api.security.AuthSubject; import org.neo4j.kernel.api.security.exception.InvalidArgumentsException; @@ -35,6 +36,7 @@ import org.neo4j.server.security.auth.AuthenticationStrategy; import org.neo4j.server.security.auth.BasicPasswordPolicy; import org.neo4j.server.security.auth.InMemoryUserRepository; +import org.neo4j.test.assertion.Assert; import static org.mockito.Mockito.mock; @@ -86,14 +88,21 @@ public void shouldLogCreatingUser() throws Throwable } @Test - public void shouldLogCreatingUserWithBadPassword() throws Throwable + public void shouldLogFailureToCreateUser() throws Throwable { + catchInvalidArguments( () -> authProcedures.createUser( null, "pw", true ) ); + catchInvalidArguments( () -> authProcedures.createUser( "", "pw", true ) ); catchInvalidArguments( () -> authProcedures.createUser( "andres", "", true ) ); catchInvalidArguments( () -> authProcedures.createUser( "mats", null, true ) ); + catchInvalidArguments( () -> authProcedures.createUser( "neo4j", "nonEmpty", true ) ); log.assertExactly( + error( "[admin]: tried to create user `%s`: %s", null, "The provided username is empty." ), + error( "[admin]: tried to create user `%s`: %s", "", "The provided username is empty." ), error( "[admin]: tried to create user `%s`: %s", "andres", "A password cannot be empty." ), - error( "[admin]: tried to create user `%s`: %s", "mats", "A password cannot be empty." ) ); + error( "[admin]: tried to create user `%s`: %s", "mats", "A password cannot be empty." ), + error( "[admin]: tried to create user `%s`: %s", "neo4j", "The specified user 'neo4j' already exists." ) + ); } @Test @@ -381,14 +390,60 @@ public void shouldLogUnauthorizedActivateUser() throws Throwable ); } - private void catchInvalidArguments( CheckedFunction f ) throws Throwable + @Test + public void shouldLogCreatingRole() throws Throwable + { + // When + authProcedures.createRole( "role" ); + + // Then + log.assertExactly( info( "[admin]: created role `%s`", "role" ) ); + } + + @Test + public void shouldLogFailureToCreateRole() throws Throwable + { + // Given + authProcedures.createRole( "role" ); + log.clear(); + + // When + catchInvalidArguments( () -> authProcedures.createRole( null ) ); + catchInvalidArguments( () -> authProcedures.createRole( "" ) ); + catchInvalidArguments( () -> authProcedures.createRole( "role" ) ); + catchInvalidArguments( () -> authProcedures.createRole( "!@#$" ) ); + + // Then + log.assertExactly( + error( "[admin]: tried to create role `%s`: %s", null, "The provided role name is empty." ), + error( "[admin]: tried to create role `%s`: %s", "", "The provided role name is empty." ), + error( "[admin]: tried to create role `%s`: %s", "role", "The specified role 'role' already exists." ), + error( "[admin]: tried to create role `%s`: %s", "!@#$", + "Role name '!@#$' contains illegal characters. Use simple ascii characters and numbers." ) + ); + } + + @Test + public void shouldLogUnauthorizedCreateRole() throws Exception + { + // Given + authProcedures.authSubject = matsSubject; + + // When + catchAuthorizationViolation( () -> authProcedures.createRole( "role" ) ); + + // Then + log.assertExactly( error("[mats]: tried to create role `%s`: %s", "role", PERMISSION_DENIED) ); + } + + private void catchInvalidArguments( ThrowingAction f ) throws Exception { - try { f.apply(); } catch (InvalidArgumentsException e) { /*ignore*/ } + Assert.assertException( f, InvalidArgumentsException.class, "" ); } - private void catchAuthorizationViolation( CheckedFunction f ) throws Throwable + private void catchAuthorizationViolation( ThrowingAction f ) throws Exception { - try { f.apply(); } catch (AuthorizationViolationException e) { /*ignore*/ } + Assert.assertException( f, AuthorizationViolationException.class, "" ); } private AssertableLogProvider.LogMatcher info( String message, String... arguments ) @@ -405,11 +460,6 @@ private AssertableLogProvider.LogMatcher error( String message, String... argume return inLog( this.getClass() ).error( message, arguments ); } - @FunctionalInterface - private interface CheckedFunction { - void apply() throws Throwable; - } - private static class TestAuthSubject extends EnterpriseAuthSubject { private final String name; diff --git a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthScenariosInteractionTestBase.java b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthScenariosInteractionTestBase.java index 892d482a2df5..3565e39c1751 100644 --- a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthScenariosInteractionTestBase.java +++ b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthScenariosInteractionTestBase.java @@ -99,7 +99,8 @@ public void shouldLogSecurityEvents() throws Exception // for REST, login doesn't happen until the subject does something neo.executeQuery( mats, "UNWIND [] AS i RETURN 1", Collections.emptyMap(), r -> {} ); assertEmpty( adminSubject, "CALL dbms.security.createUser('mats', 'neo4j', false)" ); -// assertEmpty( adminSubject, "CALL dbms.security.createRole('role1')" ); + assertEmpty( adminSubject, "CALL dbms.security.createRole('role1')" ); + assertEmpty( adminSubject, "CALL dbms.security.deleteRole('role1')" ); assertEmpty( adminSubject, "CALL dbms.security.addRoleToUser('reader', 'mats')" ); mats = neo.login( "mats", "neo4j" ); assertEmpty( mats, "MATCH (n) WHERE id(n) < 0 RETURN 1" ); @@ -107,7 +108,6 @@ public void shouldLogSecurityEvents() throws Exception assertFail( mats, "CALL dbms.security.changeUserPassword('mats', '')", "A password cannot be empty." ); assertEmpty( mats, "CALL dbms.security.changeUserPassword('mats', 'hackerPassword')" ); assertEmpty( adminSubject, "CALL dbms.security.removeRoleFromUser('reader', 'mats')" ); -// assertEmpty( adminSubject, "CALL dbms.security.deleteRole('role1')" ); assertEmpty( adminSubject, "CALL dbms.security.deleteUser('mats')" ); // flush log @@ -118,6 +118,8 @@ public void shouldLogSecurityEvents() throws Exception log.assertHasLine( "mats", "logged in" ); log.assertHasLine( "adminSubject", "created user `mats`" ); + log.assertHasLine( "adminSubject", "created role `role1`" ); + log.assertHasLine( "adminSubject", "deleted role `role1`" ); log.assertHasLine( "mats", "logged in" ); log.assertHasLine( "adminSubject", "added role `reader` to user `mats`" ); log.assertHasLine( "mats", "tried to change password for user `neo4j`: " + PERMISSION_DENIED);