From f6b0406217e043eed0b7807c73adf84dd353a532 Mon Sep 17 00:00:00 2001 From: fickludd Date: Thu, 1 Sep 2016 15:21:06 +0200 Subject: [PATCH] Move logging from user manager to procedures --- .../kernel/impl/enterprise/SecurityLog.java | 41 +- .../enterprise/auth/AuthProcedures.java | 264 +++++++++---- .../auth/EnterpriseAuthManagerFactory.java | 3 +- .../auth/InternalFlatFileRealm.java | 173 +++------ .../auth/MultiRealmAuthManager.java | 30 +- .../auth/AuthProceduresLoggingTest.java | 357 ++++++++++++++++++ .../AuthScenariosInteractionTestBase.java | 51 +-- .../auth/InternalFlatFileRealmTest.java | 27 +- .../auth/MultiRealmAuthManagerTest.java | 6 +- 9 files changed, 717 insertions(+), 235 deletions(-) create mode 100644 enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthProceduresLoggingTest.java diff --git a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/enterprise/SecurityLog.java b/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/enterprise/SecurityLog.java index 2601962d9f6cd..9445a018ff014 100644 --- a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/enterprise/SecurityLog.java +++ b/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/enterprise/SecurityLog.java @@ -25,11 +25,13 @@ import java.util.function.Consumer; import org.neo4j.io.fs.FileSystemAbstraction; +import org.neo4j.kernel.api.security.AuthSubject; import org.neo4j.kernel.configuration.Config; import org.neo4j.logging.FormattedLog; import org.neo4j.logging.Log; import org.neo4j.logging.Logger; +import static org.neo4j.helpers.Strings.escape; import static org.neo4j.io.file.Files.createOrOpenAsOuputStream; import static org.neo4j.kernel.impl.enterprise.configuration.EnterpriseEditionSettings.security_log_filename; @@ -39,10 +41,15 @@ public class SecurityLog implements Log SecurityLog( Config config, FileSystemAbstraction fileSystem ) { - inner = createLog( config, fileSystem ); + this( createLog( config, fileSystem ) ); } - private Log createLog( Config config, FileSystemAbstraction fileSystem ) + public SecurityLog( Log log ) + { + inner = log; + } + + private static Log createLog( Config config, FileSystemAbstraction fileSystem ) { FormattedLog.Builder builder = FormattedLog.withUTCTimeZone(); File logFile = config.get( security_log_filename ); @@ -58,6 +65,11 @@ private Log createLog( Config config, FileSystemAbstraction fileSystem ) return builder.toOutputStream( ouputStream ); } + private static String withSubject( AuthSubject subject, String msg ) + { + return "[" + escape( subject.username() ) + "]: " + msg; + } + @Override public boolean isDebugEnabled() { @@ -88,6 +100,11 @@ public void debug( String format, Object... arguments ) inner.debug( format, arguments ); } + public void debug( AuthSubject subject, String format, Object... arguments ) + { + inner.debug( withSubject( subject, format ), arguments ); + } + @Override public Logger infoLogger() { @@ -112,6 +129,16 @@ public void info( String format, Object... arguments ) inner.info( format, arguments ); } + public void info( AuthSubject subject, String format, Object... arguments ) + { + inner.info( withSubject( subject, format ), arguments ); + } + + public void info( AuthSubject subject, String format ) + { + inner.info( withSubject( subject, format ) ); + } + @Override public Logger warnLogger() { @@ -136,6 +163,11 @@ public void warn( String format, Object... arguments ) inner.warn( format, arguments ); } + public void warn( AuthSubject subject, String format, Object... arguments ) + { + inner.warn( withSubject( subject, format ), arguments ); + } + @Override public Logger errorLogger() { @@ -160,6 +192,11 @@ public void error( String format, Object... arguments ) inner.error( format, arguments ); } + public void error( AuthSubject subject, String format, Object... arguments ) + { + inner.error( withSubject( subject, format ), arguments ); + } + @Override public void bulk( Consumer consumer ) { 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 89c611af45455..a7a50c603e993 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 @@ -45,9 +45,12 @@ import org.neo4j.procedure.Procedure; import static java.util.function.Predicate.isEqual; +import static java.lang.String.format; + import static org.neo4j.kernel.impl.api.security.OverriddenAccessMode.getUsernameFromAccessMode; import static org.neo4j.procedure.Mode.DBMS; +@SuppressWarnings( "unused" ) public class AuthProcedures { public static final String PERMISSION_DENIED = "Permission denied."; @@ -73,8 +76,17 @@ public void createUser( ) throws InvalidArgumentsException, IOException { - StandardEnterpriseAuthSubject adminSubject = ensureAdminAuthSubject(); - adminSubject.getUserManager().newUser( username, password, requirePasswordChange ); + try + { + StandardEnterpriseAuthSubject adminSubject = ensureAdminAuthSubject(); + adminSubject.getUserManager().newUser( username, password, requirePasswordChange ); + securityLog.info( authSubject, "created user `%s`", username ); + } + catch ( Exception e ) + { + securityLog.error( authSubject, "tried to create user `%s`: %s", username, e.getMessage() ); + throw e; + } } @Description( "Change the current user's password." ) @@ -86,22 +98,32 @@ public void changeUserPassword( ) throws InvalidArgumentsException, IOException { - StandardEnterpriseAuthSubject enterpriseSubject = StandardEnterpriseAuthSubject.castOrFail( authSubject ); - if ( enterpriseSubject.doesUsernameMatch( username ) ) - { - enterpriseSubject.setPassword( newPassword, requirePasswordChange ); - } - else if ( !enterpriseSubject.isAdmin() ) + String ownOrOther = format("password for user `%s`", username ); + try { - securityLog.error( "User `%s` tried to change password for user `%s`: %s", - enterpriseSubject.name(), username, PERMISSION_DENIED ); - throw new AuthorizationViolationException( PERMISSION_DENIED ); + StandardEnterpriseAuthSubject enterpriseSubject = StandardEnterpriseAuthSubject.castOrFail( authSubject ); + if ( enterpriseSubject.doesUsernameMatch( username ) ) + { + ownOrOther = "own password"; + enterpriseSubject.setPassword( newPassword, requirePasswordChange ); + securityLog.info( authSubject, "changed own password" ); + } + else if ( !enterpriseSubject.isAdmin() ) + { + throw new AuthorizationViolationException( PERMISSION_DENIED ); + } + else + { + enterpriseSubject.getUserManager().setUserPassword( username, newPassword, requirePasswordChange ); + terminateTransactionsForValidUser( username ); + terminateConnectionsForValidUser( username ); + securityLog.info( authSubject, "changed password for user `%s`", username ); + } } - else + catch ( Exception e ) { - enterpriseSubject.getUserManager().setUserPassword( username, newPassword, requirePasswordChange ); - terminateTransactionsForValidUser( username ); - terminateConnectionsForValidUser( username ); + securityLog.error( authSubject, "tried to change %s: %s", ownOrOther, e.getMessage() ); + throw e; } } @@ -110,8 +132,18 @@ else if ( !enterpriseSubject.isAdmin() ) public void addRoleToUser(@Name( "roleName" ) String roleName, @Name( "username" ) String username ) throws IOException, InvalidArgumentsException { - StandardEnterpriseAuthSubject adminSubject = ensureAdminAuthSubject(); - adminSubject.getUserManager().addRoleToUser( roleName, username ); + try + { + StandardEnterpriseAuthSubject adminSubject = ensureAdminAuthSubject(); + adminSubject.getUserManager().addRoleToUser( roleName, username ); + securityLog.info( authSubject, "added role `%s` to user `%s`", roleName, username ); + } + catch ( Exception e ) + { + securityLog.error( authSubject, "tried to add role `%s` to user `%s`: %s", + roleName, username, e.getMessage() ); + throw e; + } } @Description( "Unassign a role from the user." ) @@ -119,43 +151,85 @@ public void addRoleToUser(@Name( "roleName" ) String roleName, @Name( "username" public void removeRoleFromUser( @Name( "roleName" ) String roleName, @Name( "username" ) String username ) throws InvalidArgumentsException, IOException { - StandardEnterpriseAuthSubject adminSubject = ensureAdminAuthSubject(); - if ( adminSubject.doesUsernameMatch( username ) && roleName.equals( PredefinedRolesBuilder.ADMIN ) ) + try { - throw new InvalidArgumentsException( "Removing yourself (user '" + username + - "') from the admin role is not allowed." ); + StandardEnterpriseAuthSubject adminSubject = ensureAdminAuthSubject(); + if ( adminSubject.doesUsernameMatch( username ) && roleName.equals( PredefinedRolesBuilder.ADMIN ) ) + { + throw new InvalidArgumentsException( + "Removing yourself (user '" + username + "') from the admin role is not allowed." ); + } + adminSubject.getUserManager().removeRoleFromUser( roleName, username ); + securityLog.info( authSubject, "removed role `%s` from user `%s`", roleName, username ); + } + catch ( Exception e ) + { + securityLog.error( authSubject, "tried to remove role `%s` from user `%s`: %s", roleName, username, e + .getMessage() ); + throw e; } - adminSubject.getUserManager().removeRoleFromUser( roleName, username ); } @Description( "Delete the specified user." ) @Procedure( name = "dbms.security.deleteUser", mode = DBMS ) public void deleteUser( @Name( "username" ) String username ) throws InvalidArgumentsException, IOException { - StandardEnterpriseAuthSubject adminSubject = ensureAdminAuthSubject(); - if ( adminSubject.doesUsernameMatch( username ) ) + try + { + StandardEnterpriseAuthSubject adminSubject = ensureAdminAuthSubject(); + if ( adminSubject.doesUsernameMatch( username ) ) + { + throw new InvalidArgumentsException( "Deleting yourself (user '" + username + "') is not allowed." ); + } + adminSubject.getUserManager().deleteUser( username ); + securityLog.info( authSubject, "deleted user `%s`", username ); + } + catch ( Exception e ) + { + securityLog.error( authSubject, "tried to delete user `%s`: %s", username, e.getMessage() ); + throw e; + } + + kickoutUser( username, "deletion" ); + } + + void kickoutUser( String username, String reason ) + { + try + { + terminateTransactionsForValidUser( username ); + terminateConnectionsForValidUser( username ); + } + catch ( Exception e ) { - throw new InvalidArgumentsException( "Deleting yourself (user '" + username + - "') is not allowed." ); + securityLog.error( authSubject, "failed to terminate running transaction and bolt connections for " + + "user `%s` following %s. %s", username, reason, e.getMessage() ); + throw e; } - adminSubject.getUserManager().deleteUser( username ); - terminateTransactionsForValidUser( username ); - terminateConnectionsForValidUser( username ); } @Description( "Suspend the specified user." ) @Procedure( name = "dbms.security.suspendUser", mode = DBMS ) public void suspendUser( @Name( "username" ) String username ) throws IOException, InvalidArgumentsException { - StandardEnterpriseAuthSubject adminSubject = ensureAdminAuthSubject(); - if ( adminSubject.doesUsernameMatch( username ) ) + try { - throw new InvalidArgumentsException( "Suspending yourself (user '" + username + - "') is not allowed." ); + StandardEnterpriseAuthSubject adminSubject = ensureAdminAuthSubject(); + if ( adminSubject.doesUsernameMatch( username ) ) + { + throw new InvalidArgumentsException( "Suspending yourself (user '" + username + + "') is not allowed." ); + } + adminSubject.getUserManager().suspendUser( username ); + securityLog.info( authSubject, "suspended user `%s`", username ); } - adminSubject.getUserManager().suspendUser( username ); - terminateTransactionsForValidUser( username ); - terminateConnectionsForValidUser( username ); + catch ( Exception e ) + { + securityLog.error( authSubject, "tried to suspend user `%s`. %s", username, e.getMessage() ); + throw e; + } + + kickoutUser( username, "suspension" ); } @Description( "Activate a suspended user." ) @@ -165,13 +239,22 @@ public void activateUser( @Name( value = "requirePasswordChange", defaultValue = "true" ) boolean requirePasswordChange ) throws IOException, InvalidArgumentsException { - StandardEnterpriseAuthSubject adminSubject = ensureAdminAuthSubject(); - if ( adminSubject.doesUsernameMatch( username ) ) + try + { + StandardEnterpriseAuthSubject adminSubject = ensureAdminAuthSubject(); + if ( adminSubject.doesUsernameMatch( username ) ) + { + throw new InvalidArgumentsException( "Activating yourself (user '" + username + + "') is not allowed." ); + } + adminSubject.getUserManager().activateUser( username, requirePasswordChange ); + securityLog.info( authSubject, "activated user `%s`", username ); + } + catch ( Exception e ) { - throw new InvalidArgumentsException( "Activating yourself (user '" + username + - "') is not allowed." ); + securityLog.error( authSubject, "tried to activate user `%s`. %s", username, e.getMessage() ); + throw e; } - adminSubject.getUserManager().activateUser( username, requirePasswordChange ); } @Description( "Show the current user." ) @@ -189,31 +272,47 @@ public Stream showCurrentUser() throws InvalidArgumentsException, IO @Procedure( name = "dbms.security.listUsers", mode = DBMS ) public Stream listUsers() throws InvalidArgumentsException, IOException { - StandardEnterpriseAuthSubject adminSubject = ensureAdminAuthSubject(); - EnterpriseUserManager userManager = adminSubject.getUserManager(); - Set users = userManager.getAllUsernames(); - List results = new ArrayList<>(); - for ( String u : users ) + try { - results.add( - new UserResult( u, userManager.getRoleNamesForUser( u ), userManager.getUser( u ).getFlags() ) ); + StandardEnterpriseAuthSubject adminSubject = ensureAdminAuthSubject(); + EnterpriseUserManager userManager = adminSubject.getUserManager(); + Set users = userManager.getAllUsernames(); + List results = new ArrayList<>(); + for ( String u : users ) + { + results.add( + new UserResult( u, userManager.getRoleNamesForUser( u ), userManager.getUser( u ).getFlags() ) ); + } + return results.stream(); + } + catch ( Exception e ) + { + securityLog.error( authSubject, "tried to list users. %s", e.getMessage() ); + throw e; } - return results.stream(); } @Description( "List all available roles." ) @Procedure( name = "dbms.security.listRoles", mode = DBMS ) public Stream listRoles() throws InvalidArgumentsException, IOException { - StandardEnterpriseAuthSubject adminSubject = ensureAdminAuthSubject(); - EnterpriseUserManager userManager = adminSubject.getUserManager(); - Set roles = userManager.getAllRoleNames(); - List results = new ArrayList<>(); - for ( String r : roles ) + try + { + StandardEnterpriseAuthSubject adminSubject = ensureAdminAuthSubject(); + EnterpriseUserManager userManager = adminSubject.getUserManager(); + Set roles = userManager.getAllRoleNames(); + List results = new ArrayList<>(); + for ( String r : roles ) + { + results.add( new RoleResult( r, userManager.getUsernamesForRole( r ) ) ); + } + return results.stream(); + } + catch ( Exception e ) { - results.add( new RoleResult( r, userManager.getUsernamesForRole( r ) ) ); + securityLog.error( authSubject, "tried to list roles. %s", e.getMessage() ); + throw e; } - return results.stream(); } @Description( "List all roles assigned to the specified user." ) @@ -221,8 +320,16 @@ public Stream listRoles() throws InvalidArgumentsException, IOExcept public Stream listRolesForUser( @Name( "username" ) String username ) throws InvalidArgumentsException, IOException { - StandardEnterpriseAuthSubject subject = ensureSelfOrAdminAuthSubject( username ); - return subject.getUserManager().getRoleNamesForUser( username ).stream().map( StringResult::new ); + try + { + StandardEnterpriseAuthSubject subject = ensureSelfOrAdminAuthSubject( username ); + return subject.getUserManager().getRoleNamesForUser( username ).stream().map( StringResult::new ); + } + catch ( Exception e ) + { + securityLog.error( authSubject, "tried to list roles for user `%s`. %s", username, e.getMessage() ); + throw e; + } } @Description( "List all users currently assigned the specified role." ) @@ -230,8 +337,16 @@ public Stream listRolesForUser( @Name( "username" ) String usernam public Stream listUsersForRole( @Name( "roleName" ) String roleName ) throws InvalidArgumentsException, IOException { - StandardEnterpriseAuthSubject adminSubject = ensureAdminAuthSubject(); - return adminSubject.getUserManager().getUsernamesForRole( roleName ).stream().map( StringResult::new ); + try + { + StandardEnterpriseAuthSubject adminSubject = ensureAdminAuthSubject(); + return adminSubject.getUserManager().getUsernamesForRole( roleName ).stream().map( StringResult::new ); + } + catch ( Exception e ) + { + securityLog.error( authSubject, "tried to list users for role `%s`. %s", roleName, e.getMessage() ); + throw e; + } } @Description( "Create a new role." ) @@ -239,7 +354,16 @@ public Stream listUsersForRole( @Name( "roleName" ) String roleNam public void createRole( @Name( "roleName" ) String roleName ) throws InvalidArgumentsException, IOException { - ensureAdminAuthSubject().getUserManager().newRole( roleName ); + try + { + ensureAdminAuthSubject().getUserManager().newRole( roleName ); + securityLog.info( authSubject, "created role `%s`", roleName ); + } + catch ( Exception e ) + { + securityLog.error( authSubject, "tried to create role `%s`. %s", roleName, e.getMessage() ); + throw e; + } } @Description( "Delete the specified role. Any role assignments will be removed." ) @@ -247,9 +371,20 @@ public void createRole( @Name( "roleName" ) String roleName ) public void deleteRole( @Name( "roleName" ) String roleName ) throws InvalidArgumentsException, IOException { - ensureAdminAuthSubject().getUserManager().deleteRole( roleName ); + try + { + ensureAdminAuthSubject().getUserManager().deleteRole( roleName ); + securityLog.info( authSubject, "deleted role `%s`", roleName ); + } + catch ( Exception e ) + { + securityLog.error( authSubject, "tried to delete role `%s`. %s", roleName, e.getMessage() ); + throw e; + } } + // TODO: add security logging once listQueries PR is in + // ====================================================== FROM HERE @Procedure( name = "dbms.security.listTransactions", mode = DBMS ) public Stream listTransactions() throws InvalidArgumentsException, IOException @@ -299,10 +434,11 @@ public Stream terminateConnectionsForUser( @Name( "username" ) return terminateConnectionsForValidUser( username ); } + // ====================================================== TO HERE // ----------------- helpers --------------------- - private Stream terminateTransactionsForValidUser( String username ) + Stream terminateTransactionsForValidUser( String username ) { long terminatedCount = 0; for ( KernelTransactionHandle tx : getActiveTransactions() ) @@ -319,7 +455,7 @@ private Stream terminateTransactionsForValidUser( return Stream.of( new TransactionTerminationResult( username, terminatedCount ) ); } - private Stream terminateConnectionsForValidUser( String username ) + Stream terminateConnectionsForValidUser( String username ) { Long killCount = 0L; for ( ManagedBoltStateMachine connection : getBoltConnectionTracker().getActiveConnections( username ) ) diff --git a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/EnterpriseAuthManagerFactory.java b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/EnterpriseAuthManagerFactory.java index 973de7ba3abba..a9e9aba3654bf 100644 --- a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/EnterpriseAuthManagerFactory.java +++ b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/EnterpriseAuthManagerFactory.java @@ -33,6 +33,7 @@ import org.neo4j.kernel.api.security.AuthManager; import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.impl.util.JobScheduler; +import org.neo4j.kernel.impl.enterprise.SecurityLog; import org.neo4j.logging.Log; import org.neo4j.logging.LogProvider; import org.neo4j.server.security.auth.AuthenticationStrategy; @@ -92,7 +93,7 @@ public AuthManager newInstance( Config config, LogProvider logProvider, Log secu return new MultiRealmAuthManager( internalRealm, realms, new ShiroCaffeineCache.Manager( Ticker.systemTicker(), ttl, maxCapacity ), - securityLog ); + (SecurityLog) securityLog ); } private static InternalFlatFileRealm createInternalRealm( Config config, LogProvider logProvider, diff --git a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/InternalFlatFileRealm.java b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/InternalFlatFileRealm.java index 11cca11049641..6ce6d9e0e726b 100644 --- a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/InternalFlatFileRealm.java +++ b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/InternalFlatFileRealm.java @@ -63,7 +63,6 @@ import org.neo4j.server.security.auth.exception.ConcurrentModificationException; import static java.lang.String.format; -import static org.neo4j.helpers.Strings.escape; /** * Shiro realm wrapping FileUserRepository and FileRoleRepository @@ -84,22 +83,21 @@ class InternalFlatFileRealm extends AuthorizingRealm implements RealmLifecycle, private final AuthenticationStrategy authenticationStrategy; private final boolean authenticationEnabled; private final boolean authorizationEnabled; - private final Log securityLog; private final Map roles; private final JobScheduler jobScheduler; private JobScheduler.JobHandle reloadJobHandle; public InternalFlatFileRealm( UserRepository userRepository, RoleRepository roleRepository, PasswordPolicy passwordPolicy, AuthenticationStrategy authenticationStrategy, - JobScheduler jobScheduler, Log securityLog ) + JobScheduler jobScheduler ) { this( userRepository, roleRepository, passwordPolicy, authenticationStrategy, true, true, - jobScheduler, securityLog ); + jobScheduler ); } InternalFlatFileRealm( UserRepository userRepository, RoleRepository roleRepository, PasswordPolicy passwordPolicy, AuthenticationStrategy authenticationStrategy, - boolean authenticationEnabled, boolean authorizationEnabled, JobScheduler jobScheduler, Log securityLog ) + boolean authenticationEnabled, boolean authorizationEnabled, JobScheduler jobScheduler ) { super(); @@ -112,7 +110,6 @@ public InternalFlatFileRealm( UserRepository userRepository, RoleRepository role this.jobScheduler = jobScheduler; setAuthenticationCachingEnabled( false ); setAuthorizationCachingEnabled( false ); - this.securityLog = securityLog; setCredentialsMatcher( new AllowAllCredentialsMatcher() ); setRolePermissionResolver( new RolePermissionResolver() { @@ -371,26 +368,16 @@ private int numberOfRoles() public User newUser( String username, String initialPassword, boolean requirePasswordChange ) throws IOException, InvalidArgumentsException { - try - { - passwordPolicy.validatePassword( initialPassword ); + passwordPolicy.validatePassword( initialPassword ); - User user = new User.Builder() - .withName( username ) - .withCredentials( Credential.forPassword( initialPassword ) ) - .withRequiredPasswordChange( requirePasswordChange ) - .build(); - userRepository.create( user ); + User user = new User.Builder() + .withName( username ) + .withCredentials( Credential.forPassword( initialPassword ) ) + .withRequiredPasswordChange( requirePasswordChange ) + .build(); + userRepository.create( user ); - securityLog.info( "User created: `%s`" + (requirePasswordChange ? " (password change required)" : ""), - escape( username ) ); - return user; - } - catch ( InvalidArgumentsException e ) - { - securityLog.error( "User creation failed for user `%s`: %s", escape( username ), e.getMessage() ); - throw e; - } + return user; } @Override @@ -449,29 +436,20 @@ public void addRoleToUser( String roleName, String username ) throws IOException assertValidRoleName( roleName ); assertValidUsername( username ); - try + synchronized ( this ) { - synchronized ( this ) + getUser( username ); + RoleRecord role = getRole( roleName ); + RoleRecord newRole = role.augment().withUser( username ).build(); + try { - getUser( username ); - RoleRecord role = getRole( roleName ); - RoleRecord newRole = role.augment().withUser( username ).build(); - try - { - roleRepository.update( role, newRole ); - } - catch ( ConcurrentModificationException e ) - { - // Try again - addRoleToUser( roleName, username ); - } + roleRepository.update( role, newRole ); + } + catch ( ConcurrentModificationException e ) + { + // Try again + addRoleToUser( roleName, username ); } - securityLog.info( "Role `%s` added to user `%s`", escape( roleName ), escape( username ) ); - } - catch ( InvalidArgumentsException e ) - { - securityLog.error( "Role `%s` not added to user `%s`: %s", escape( roleName ), escape( username ), e.getMessage() ); - throw e; } clearCachedAuthorizationInfoForUser( username ); } @@ -482,32 +460,23 @@ public void removeRoleFromUser( String roleName, String username ) throws IOExce assertValidRoleName( roleName ); assertValidUsername( username ); - try + synchronized ( this ) { - synchronized ( this ) - { - getUser( username ); - RoleRecord role = getRole( roleName ); + getUser( username ); + RoleRecord role = getRole( roleName ); - RoleRecord newRole = role.augment().withoutUser( username ).build(); - try - { - roleRepository.update( role, newRole ); - securityLog.info( "Role `%s` removed from user `%s`", escape( roleName ), escape( username ) ); - } - catch ( ConcurrentModificationException e ) - { - // Try again - removeRoleFromUser( roleName, username ); - } + RoleRecord newRole = role.augment().withoutUser( username ).build(); + try + { + roleRepository.update( role, newRole ); + } + catch ( ConcurrentModificationException e ) + { + // Try again + removeRoleFromUser( roleName, username ); } } - catch ( InvalidArgumentsException e ) - { - securityLog.error( "Role `%s` not removed from user `%s`: %s", escape( roleName ), escape( username ), - e.getMessage() ); - throw e; - } + clearCachedAuthorizationInfoForUser( username ); } @@ -515,29 +484,21 @@ public void removeRoleFromUser( String roleName, String username ) throws IOExce public boolean deleteUser( String username ) throws IOException, InvalidArgumentsException { boolean result = false; - try + synchronized ( this ) { - synchronized ( this ) + User user = getUser( username ); + if ( userRepository.delete( user ) ) { - User user = getUser( username ); - if ( userRepository.delete( user ) ) - { - removeUserFromAllRoles( username ); - result = true; - } - else - { - // We should not get here, but if we do the assert will fail and give a nice error msg - getUser( username ); - } + removeUserFromAllRoles( username ); + result = true; + } + else + { + // We should not get here, but if we do the assert will fail and give a nice error msg + getUser( username ); } - clearCacheForUser( username ); - securityLog.info( "User deleted: `%s`", escape( username ) ); - } - catch ( InvalidArgumentsException e ) - { - securityLog.error( "User deletion failed for user `%s`: %s", escape( username ), e.getMessage() ); } + clearCacheForUser( username ); return result; } @@ -556,39 +517,29 @@ public User getUser( String username ) throws InvalidArgumentsException public void setUserPassword( String username, String password, boolean requirePasswordChange ) throws IOException, InvalidArgumentsException { - try - { - User existingUser = getUser( username ); + User existingUser = getUser( username ); - passwordPolicy.validatePassword( password ); + passwordPolicy.validatePassword( password ); - if ( existingUser.credentials().matchesPassword( password ) ) - { - throw new InvalidArgumentsException( "Old password and new password cannot be the same." ); - } + if ( existingUser.credentials().matchesPassword( password ) ) + { + throw new InvalidArgumentsException( "Old password and new password cannot be the same." ); + } - try - { - User updatedUser = existingUser.augment() - .withCredentials( Credential.forPassword( password ) ) - .withRequiredPasswordChange( requirePasswordChange ) - .build(); - userRepository.update( existingUser, updatedUser ); - } - catch ( ConcurrentModificationException e ) - { - // try again - setUserPassword( username, password, requirePasswordChange ); - } + try + { + User updatedUser = existingUser.augment() + .withCredentials( Credential.forPassword( password ) ) + .withRequiredPasswordChange( requirePasswordChange ) + .build(); + userRepository.update( existingUser, updatedUser ); } - catch ( InvalidArgumentsException e ) + catch ( ConcurrentModificationException e ) { - securityLog.error( "Password not changed for user `%s`: %s", escape( username ), e.getMessage() ); - throw e; + // try again + setUserPassword( username, password, requirePasswordChange ); } - securityLog.info( "Password changed for user `%s`.", escape( username ) ); - clearCacheForUser( username ); } diff --git a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/MultiRealmAuthManager.java b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/MultiRealmAuthManager.java index 917c3db0ab6fe..43f23f48cd720 100644 --- a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/MultiRealmAuthManager.java +++ b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/MultiRealmAuthManager.java @@ -35,25 +35,22 @@ import java.util.Collection; import java.util.Map; -import org.neo4j.kernel.api.security.AuthToken; import org.neo4j.kernel.api.security.AuthenticationResult; import org.neo4j.kernel.api.security.exception.InvalidAuthTokenException; import org.neo4j.kernel.enterprise.api.security.EnterpriseAuthSubject; -import org.neo4j.logging.Log; +import org.neo4j.kernel.impl.enterprise.SecurityLog; import org.neo4j.server.security.auth.UserManagerSupplier; -import static org.neo4j.helpers.Strings.escape; - -public class MultiRealmAuthManager implements EnterpriseAuthManager, UserManagerSupplier +class MultiRealmAuthManager implements EnterpriseAuthManager, UserManagerSupplier { private final EnterpriseUserManager userManager; private final Collection realms; private final DefaultSecurityManager securityManager; private final CacheManager cacheManager; - private final Log securityLog; + private final SecurityLog securityLog; MultiRealmAuthManager( EnterpriseUserManager userManager, Collection realms, CacheManager cacheManager, - Log securityLog ) + SecurityLog securityLog ) { this.userManager = userManager; this.realms = realms; @@ -87,15 +84,14 @@ public boolean supports( final Map authToken ) @Override public EnterpriseAuthSubject login( Map authToken ) throws InvalidAuthTokenException { - ShiroSubject subject; + EnterpriseAuthSubject subject; ShiroAuthToken token = new ShiroAuthToken( authToken ); - String username = escape( authToken.get( AuthToken.PRINCIPAL ).toString() ); try { - subject = (ShiroSubject) securityManager.login( null, token ); - securityLog.info( "Login success for user `%s`", username ); + subject = new StandardEnterpriseAuthSubject( this, (ShiroSubject) securityManager.login( null, token ) ); + securityLog.info( subject, "logged in" ); } catch ( UnsupportedTokenException e ) { @@ -104,16 +100,18 @@ public EnterpriseAuthSubject login( Map authToken ) throws Invali catch ( ExcessiveAttemptsException e ) { // NOTE: We only get this with single (internal) realm authentication - subject = new ShiroSubject( securityManager, AuthenticationResult.TOO_MANY_ATTEMPTS ); - securityLog.error( "Login fail for user `%s` - too many failed attempts.", username ); + subject = new StandardEnterpriseAuthSubject( this, + new ShiroSubject( securityManager, AuthenticationResult.TOO_MANY_ATTEMPTS ) ); + securityLog.info( subject, "Login fail for user `%s` - too many failed attempts." ); } catch ( AuthenticationException e ) { - subject = new ShiroSubject( securityManager, AuthenticationResult.FAILURE ); - securityLog.error( "Login fail for user `%s`", username ); + subject = new StandardEnterpriseAuthSubject( this, + new ShiroSubject( securityManager, AuthenticationResult.FAILURE ) ); + securityLog.info( subject, "Login fail for user `%s`" ); } - return new StandardEnterpriseAuthSubject( this, subject ); + return subject; } @Override 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 new file mode 100644 index 0000000000000..b158b76c68771 --- /dev/null +++ b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthProceduresLoggingTest.java @@ -0,0 +1,357 @@ +/* + * Copyright (c) 2002-2016 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Neo4j is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package org.neo4j.server.security.enterprise.auth; + +import java.io.IOException; +import java.util.stream.Stream; + +import org.apache.shiro.mgt.SecurityManager; +import org.junit.Before; +import org.junit.Test; + +import org.neo4j.graphdb.security.AuthorizationViolationException; +import org.neo4j.kernel.api.security.AuthSubject; +import org.neo4j.kernel.api.security.exception.InvalidArgumentsException; +import org.neo4j.kernel.impl.enterprise.SecurityLog; +import org.neo4j.kernel.internal.GraphDatabaseAPI; +import org.neo4j.logging.AssertableLogProvider; +import org.neo4j.server.security.auth.AuthenticationStrategy; +import org.neo4j.server.security.auth.BasicPasswordPolicy; +import org.neo4j.server.security.auth.InMemoryUserRepository; +import org.neo4j.server.security.auth.PasswordPolicy; +import org.neo4j.server.security.auth.UserRepository; + +import static org.mockito.Mockito.mock; + +import static org.neo4j.logging.AssertableLogProvider.inLog; +import static org.neo4j.server.security.enterprise.auth.AuthProcedures.PERMISSION_DENIED; +import static org.neo4j.server.security.enterprise.auth.PredefinedRolesBuilder.ARCHITECT; +import static org.neo4j.server.security.enterprise.auth.PredefinedRolesBuilder.READER; + +public class AuthProceduresLoggingTest +{ + private AuthProcedures authProcedures; + private AssertableLogProvider log = null; + private AuthSubject matsSubject = null; + + @Before + public void setUp() throws Throwable + { + EnterpriseUserManager userManager = getUserManager(); + AuthSubject adminSubject = new TestAuthSubject( "admin", true, userManager ); + matsSubject = new TestAuthSubject( "mats", false, userManager ); + + log = new AssertableLogProvider(); + authProcedures = new TestAuthProcedures(); + authProcedures.authSubject = adminSubject; + authProcedures.securityLog = new SecurityLog( log.getLog( getClass() ) ); + GraphDatabaseAPI api = mock( GraphDatabaseAPI.class ); + authProcedures.graph = api; + } + + private static class TestAuthProcedures extends AuthProcedures + { + @Override + void kickoutUser( String username, String reason ) + { + } + + @Override + Stream terminateTransactionsForValidUser( String username ) + { + return Stream.empty(); + } + + @Override + Stream terminateConnectionsForValidUser( String username ) + { + return Stream.empty(); + } + } + + private EnterpriseUserManager getUserManager() throws Throwable + { + InMemoryRoleRepository roles = new InMemoryRoleRepository(); + PasswordPolicy passwordPolicy = new BasicPasswordPolicy(); + UserRepository users = new InMemoryUserRepository(); + AuthenticationStrategy authStrategy = mock( AuthenticationStrategy.class ); + InternalFlatFileRealm realm = new InternalFlatFileRealm( users, roles, passwordPolicy, authStrategy ); + realm.start(); // creates default user and roles + return realm; + } + + @Test + public void shouldLogCreatingUser() throws Throwable + { + authProcedures.createUser( "andres", "el password", true ); + authProcedures.createUser( "mats", "el password", false ); + + log.assertExactly( + info( "[admin]: created user `%s`", "andres" ), + info( "[admin]: created user `%s`", "mats" ) ); + } + + @Test + public void shouldLogCreatingUserWithBadPassword() throws Throwable + { + catchInvalidArguments( () -> authProcedures.createUser( "andres", "", true ) ); + catchInvalidArguments( () -> authProcedures.createUser( "mats", null, true ) ); + + log.assertExactly( + 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." ) ); + } + + @Test + public void shouldLogDeletingUser() throws Throwable + { + authProcedures.createUser( "andres", "el password", false ); + authProcedures.deleteUser( "andres" ); + + log.assertExactly( + info( "[admin]: created user `%s`", "andres" ), + info( "[admin]: deleted user `%s`", "andres" ) ); + } + + @Test + public void shouldLogDeletingNonExistentUser() throws Throwable + { + catchInvalidArguments( () -> authProcedures.deleteUser( "andres" ) ); + + log.assertExactly( error( "[admin]: tried to delete user `%s`: %s", "andres", "User 'andres' does not exist." ) ); + } + + @Test + public void shouldLogAddingRoleToUser() throws Throwable + { + authProcedures.createUser( "mats", "neo4j", false ); + authProcedures.addRoleToUser( ARCHITECT, "mats" ); + + log.assertExactly( + info( "[admin]: created user `%s`", "mats" ), + info( "[admin]: added role `%s` to user `%s`", ARCHITECT, "mats" ) ); + } + + @Test + public void shouldLogFailureToAddRoleToUser() throws Throwable + { + authProcedures.createUser( "mats", "neo4j", false ); + catchInvalidArguments( () -> authProcedures.addRoleToUser( "null", "mats" ) ); + + log.assertExactly( + info( "[admin]: created user `%s`", "mats" ), + error( "[admin]: tried to add role `%s` to user `%s`: %s", "null", "mats", "Role 'null' does not exist." ) ); + } + + @Test + public void shouldLogRemovalOfRoleFromUser() throws Throwable + { + // Given + authProcedures.createUser( "mats", "neo4j", false ); + authProcedures.addRoleToUser( READER, "mats" ); + log.clear(); + + // When + authProcedures.removeRoleFromUser( READER, "mats" ); + + // Then + log.assertExactly( info( "[admin]: removed role `%s` from user `%s`", READER, "mats" ) ); + } + + @Test + public void shouldLogFailureToRemoveRoleFromUser() throws Throwable + { + // Given + authProcedures.createUser( "mats", "neo4j", false ); + authProcedures.addRoleToUser( READER, "mats" ); + log.clear(); + + // When + catchInvalidArguments( () -> authProcedures.removeRoleFromUser( "notReader", "mats" ) ); + catchInvalidArguments( () -> authProcedures.removeRoleFromUser( READER, "notMats" ) ); + + // Then + log.assertExactly( + error( "[admin]: tried to remove role `%s` from user `%s`: %s", "notReader", "mats", "Role 'notReader' does not exist." ), + error( "[admin]: tried to remove role `%s` from user `%s`: %s", READER, "notMats", "User 'notMats' does not exist." ) + ); + } + + @Test + public void shouldLogPasswordChanges() throws IOException, InvalidArgumentsException + { + // Given + authProcedures.createUser( "mats", "neo4j", true ); + log.clear(); + + // When + authProcedures.changeUserPassword( "mats", "longerPassword", false ); + authProcedures.authSubject = matsSubject; + authProcedures.changeUserPassword( "mats", "evenLongerPassword", false ); + + // Then + log.assertExactly( + info( "[admin]: changed password for user `%s`", "mats" ), + info( "[mats]: changed own password" ) + ); + } + + @Test + public void shouldLogFailureToChangePassword() throws Throwable + { + // Given + authProcedures.createUser( "andres", "neo4j", true ); + log.clear(); + + // When + catchInvalidArguments( () -> authProcedures.changeUserPassword( "andres", "neo4j", false ) ); + catchInvalidArguments( () -> authProcedures.changeUserPassword( "andres", "", false ) ); + catchInvalidArguments( () -> authProcedures.changeUserPassword( "notAndres", "good password", false ) ); + + // Then + log.assertExactly( + error( "[admin]: tried to change %s: %s", "password for user `andres`", "Old password and new password cannot be the same." ), + error( "[admin]: tried to change %s: %s", "password for user `andres`", "A password cannot be empty." ), + error( "[admin]: tried to change %s: %s", "password for user `notAndres`", "User 'notAndres' does not exist." ) + ); + } + + @Test + public void shouldLogFailureToChangeOwnPassword() throws Throwable + { + // Given + authProcedures.createUser( "mats", "neo4j", true ); + authProcedures.authSubject = matsSubject; + log.clear(); + + // When + catchInvalidArguments( () -> authProcedures.changeUserPassword( "mats", "neo4j", false ) ); + catchInvalidArguments( () -> authProcedures.changeUserPassword( "mats", "", false ) ); + + // Then + log.assertExactly( + error( "[mats]: tried to change %s: %s", "own password", "Old password and new password cannot be the same." ), + error( "[mats]: tried to change %s: %s", "own password", "A password cannot be empty." ) + ); + } + + @Test + public void shouldLogUnauthorizedFailureToChangePassword() throws Throwable + { + // Given + authProcedures.createUser( "andres", "neo4j", true ); + log.clear(); + authProcedures.authSubject = matsSubject; + + // When + catchAuthorizationViolation( () -> authProcedures.changeUserPassword( "andres", "otherPw", false ) ); + + // Then + log.assertExactly( + error( "[mats]: tried to change %s: %s", "password for user `andres`", PERMISSION_DENIED ) + ); + } + + private void catchInvalidArguments( CheckedFunction f ) throws Throwable + { + try { f.apply(); } catch (InvalidArgumentsException e) { /*ignore*/ } + } + + private void catchAuthorizationViolation( CheckedFunction f ) throws Throwable + { + try { f.apply(); } catch (AuthorizationViolationException e) { /*ignore*/ } + } + + private AssertableLogProvider.LogMatcher info( String message, String... arguments ) + { + if ( arguments.length == 0 ) + { + return inLog( this.getClass() ).info( message ); + } + return inLog( this.getClass() ).info( message, arguments ); + } + + private AssertableLogProvider.LogMatcher error( String message, String... arguments ) + { + 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; + private final boolean isAdmin; + private final EnterpriseUserManager userManager; + + TestAuthSubject( String name, boolean isAdmin, EnterpriseUserManager userManager ) + { + super(null, new TestShiroSubject( name )); + this.name = name; + this.isAdmin = isAdmin; + this.userManager = userManager; + } + + @Override + public String username() + { + return name; + } + + @Override + public boolean isAdmin() + { + return isAdmin; + } + + @Override + public EnterpriseUserManager getUserManager() + { + return userManager; + } + + @Override + public boolean doesUsernameMatch( String username ) + { + return name.equals( username ); + } + } + + private static class TestShiroSubject extends ShiroSubject + { + private final String name; + + TestShiroSubject( String name ) + { + super( mock( SecurityManager.class ), null ); + this.name = name; + } + + @Override + public Object getPrincipal() + { + return 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 57100cd18c161..892d482a2df5e 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 @@ -114,27 +114,36 @@ public void shouldLogSecurityEvents() throws Exception neo.getLocalGraph().shutdown(); // assert on log content - List allLines = readFullLog(); - - assertThat( allLines, hasItem( containsString( "Login fail for user `mats`" ) ) ); - assertThat( allLines, hasItem( containsString( "User created: `mats`" ) ) ); - assertThat( allLines, hasItem( containsString( "Login success for user `mats`" ) ) ); - assertThat( allLines, hasItem( containsString( "Role `reader` added to user `mats`" ) ) ); - assertThat( allLines, hasItem( containsString( "User `mats` tried to change password for user `neo4j`" ) ) ); - assertThat( allLines, hasItem( containsString( "Password not changed for user `mats`" ) ) ); - assertThat( allLines, hasItem( containsString( "Password changed for user `mats`" ) ) ); - assertThat( allLines, hasItem( containsString( "Role `reader` removed from user `mats`" ) ) ); - assertThat( allLines, hasItem( containsString( "User deleted: `mats`" ) ) ); - } - - private List readFullLog() throws IOException - { - List lines = new ArrayList<>(); - BufferedReader bufferedReader = new BufferedReader( - neo.fileSystem().openAsReader( new File( securityLog.getAbsolutePath() ), Charsets.UTF_8 ) ); - lines.add( bufferedReader.readLine() ); - bufferedReader.lines().forEach( lines::add ); - return lines; + FullLog log = new FullLog(); + + log.assertHasLine( "mats", "logged in" ); + log.assertHasLine( "adminSubject", "created user `mats`" ); + 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); + log.assertHasLine( "mats", "tried to change own password: A password cannot be empty."); + log.assertHasLine( "mats", "changed own password"); + log.assertHasLine( "adminSubject", "removed role `reader` from user `mats`"); + log.assertHasLine( "adminSubject", "deleted user `mats`"); + } + + private class FullLog + { + List lines; + + FullLog() throws IOException + { + lines = new ArrayList<>(); + BufferedReader bufferedReader = new BufferedReader( + neo.fileSystem().openAsReader( new File( securityLog.getAbsolutePath() ), Charsets.UTF_8 ) ); + lines.add( bufferedReader.readLine() ); + bufferedReader.lines().forEach( lines::add ); + } + + void assertHasLine( String subject, String msg ) + { + assertThat( lines, hasItem( containsString( "[" + subject + "]: " + msg ) ) ); + } } /* diff --git a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/InternalFlatFileRealmTest.java b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/InternalFlatFileRealmTest.java index 2b57941e41ea6..b478194f71c4c 100644 --- a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/InternalFlatFileRealmTest.java +++ b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/InternalFlatFileRealmTest.java @@ -36,13 +36,7 @@ import org.neo4j.kernel.api.security.exception.InvalidArgumentsException; import org.neo4j.kernel.api.security.exception.InvalidAuthTokenException; import org.neo4j.kernel.enterprise.api.security.EnterpriseAuthSubject; -import java.io.IOException; - -import com.sun.org.apache.regexp.internal.RE; -import org.junit.Before; -import org.junit.Test; - -import org.neo4j.kernel.api.security.exception.InvalidArgumentsException; +import org.neo4j.kernel.impl.enterprise.SecurityLog; import org.neo4j.kernel.impl.util.JobScheduler; import org.neo4j.logging.AssertableLogProvider; import org.neo4j.logging.Log; @@ -84,7 +78,8 @@ public void setup() throws Throwable List realms = listOf( testRealm ); - authManager = new MultiRealmAuthManager( testRealm, realms, new MemoryConstrainedCacheManager(), actualLog ); + authManager = new MultiRealmAuthManager( testRealm, realms, new MemoryConstrainedCacheManager(), + new SecurityLog( actualLog ) ); authManager.init(); authManager.start(); @@ -128,11 +123,9 @@ private class TestRealm extends InternalFlatFileRealm public TestRealm( UserRepository userRepository, RoleRepository roleRepository, PasswordPolicy passwordPolicy, AuthenticationStrategy authenticationStrategy, JobScheduler jobScheduler, Log securityLog ) { - super( userRepository, roleRepository, passwordPolicy, authenticationStrategy, true, true, - jobScheduler, securityLog ); + super( userRepository, roleRepository, passwordPolicy, authenticationStrategy, jobScheduler, securityLog ); } - boolean takeAuthenticationFlag() { boolean t = authenticationFlag; @@ -275,11 +268,11 @@ public void shouldLogFailureToRemoveRoleFromUser() throws Throwable public void shouldLogPasswordChanges() throws IOException, InvalidArgumentsException { // Given - realm.newUser( "andres", "neo4j", true ); + testRealm.newUser( "andres", "neo4j", true ); log.clear(); // When - realm.setUserPassword( "andres", "longerPassword", false ); + testRealm.setUserPassword( "andres", "longerPassword", false ); // Then log.assertExactly( info( "Password changed for user `%s`.", "andres" ) ); @@ -289,13 +282,13 @@ public void shouldLogPasswordChanges() throws IOException, InvalidArgumentsExcep public void shouldLogFailureToChangePassword() throws IOException, InvalidArgumentsException { // Given - realm.newUser( "andres", "neo4j", true ); + testRealm.newUser( "andres", "neo4j", true ); log.clear(); // When - catchInvalidArguments( () -> realm.setUserPassword( "andres", "neo4j", false ) ); - catchInvalidArguments( () -> realm.setUserPassword( "andres", "", false ) ); - catchInvalidArguments( () -> realm.setUserPassword( "notAndres", "good password", false ) ); + catchInvalidArguments( () -> testRealm.setUserPassword( "andres", "neo4j", false ) ); + catchInvalidArguments( () -> testRealm.setUserPassword( "andres", "", false ) ); + catchInvalidArguments( () -> testRealm.setUserPassword( "notAndres", "good password", false ) ); // Then log.assertExactly( diff --git a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/MultiRealmAuthManagerTest.java b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/MultiRealmAuthManagerTest.java index 5c7b09df490e4..23ac6a300158a 100644 --- a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/MultiRealmAuthManagerTest.java +++ b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/MultiRealmAuthManagerTest.java @@ -30,9 +30,9 @@ import org.neo4j.kernel.api.security.AuthenticationResult; import org.neo4j.kernel.api.security.exception.InvalidArgumentsException; import org.neo4j.kernel.impl.util.JobScheduler; +import org.neo4j.kernel.impl.enterprise.SecurityLog; import org.neo4j.logging.AssertableLogProvider; import org.neo4j.logging.Log; -import org.neo4j.logging.NullLog; import org.neo4j.server.security.auth.AuthenticationStrategy; import org.neo4j.server.security.auth.Credential; import org.neo4j.server.security.auth.InMemoryUserRepository; @@ -70,10 +70,10 @@ public void setUp() throws Throwable InternalFlatFileRealm internalFlatFileRealm = new InternalFlatFileRealm( users, new InMemoryRoleRepository(), mock( PasswordPolicy.class ), - authStrategy, mock( JobScheduler.class ) ); + authStrategy, mock( JobScheduler.class ), log ); manager = new MultiRealmAuthManager( internalFlatFileRealm, Collections.singleton( internalFlatFileRealm ), - new MemoryConstrainedCacheManager(), log ); + new MemoryConstrainedCacheManager(), new SecurityLog( log ) ); manager.init(); userManager = manager.getUserManager();