From ba0c85a09f2c3d4430fdb3e7703b0b59e1d82414 Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Tue, 16 Aug 2016 15:32:31 +0200 Subject: [PATCH] Ensure set-password respects same default user behaviour as server --- .../neo4j/test/TestGraphDatabaseFactory.java | 1 - .../admin/security/SetPasswordCommand.java | 1 + .../security/SetPasswordCommandTest.java | 70 ++++++++++++++----- .../configuration/ConfigLoaderTest.java | 8 +-- .../auth/InternalFlatFileRealm.java | 27 +++++-- 5 files changed, 77 insertions(+), 30 deletions(-) diff --git a/community/kernel/src/test/java/org/neo4j/test/TestGraphDatabaseFactory.java b/community/kernel/src/test/java/org/neo4j/test/TestGraphDatabaseFactory.java index e51ce5d14582d..8c0c1681692a2 100644 --- a/community/kernel/src/test/java/org/neo4j/test/TestGraphDatabaseFactory.java +++ b/community/kernel/src/test/java/org/neo4j/test/TestGraphDatabaseFactory.java @@ -87,7 +87,6 @@ private void configure( GraphDatabaseBuilder builder, File storeDir ) super.configure( builder ); // Reduce the default page cache memory size to 8 mega-bytes for test databases. builder.setConfig( GraphDatabaseSettings.pagecache_memory, "8m" ); - builder.setConfig( GraphDatabaseSettings.auth_store, tempFile( "auth" ).toString() ); builder.setConfig( GraphDatabaseSettings.logs_directory, new File( storeDir, "logs" ).getAbsolutePath() ); } diff --git a/community/security/src/main/java/org/neo4j/commandline/admin/security/SetPasswordCommand.java b/community/security/src/main/java/org/neo4j/commandline/admin/security/SetPasswordCommand.java index 9146a75d669d1..d3be55bfaf1d3 100644 --- a/community/security/src/main/java/org/neo4j/commandline/admin/security/SetPasswordCommand.java +++ b/community/security/src/main/java/org/neo4j/commandline/admin/security/SetPasswordCommand.java @@ -107,6 +107,7 @@ public void execute( String[] args ) throws IncorrectUsage, CommandFailed userRepository.start(); PasswordPolicy passwordPolicy = new BasicPasswordPolicy(); BasicAuthManager authManager = new BasicAuthManager( userRepository, passwordPolicy, systemUTC() ); + authManager.start(); // required to setup default users try { authManager.setUserPassword( username, password ); diff --git a/community/security/src/test/java/org/neo4j/commandline/admin/security/SetPasswordCommandTest.java b/community/security/src/test/java/org/neo4j/commandline/admin/security/SetPasswordCommandTest.java index e935de36c22c9..da69f7464589b 100644 --- a/community/security/src/test/java/org/neo4j/commandline/admin/security/SetPasswordCommandTest.java +++ b/community/security/src/test/java/org/neo4j/commandline/admin/security/SetPasswordCommandTest.java @@ -113,14 +113,30 @@ public void shouldFailWitNonExistingUser() throws Exception } @Test - public void shouldRunSetPasswordCommandWithExistinguser() throws Throwable + public void shouldRunSetPasswordCommandWithDefaultUser() throws Throwable { - // Given - new user that requires password change + // Given - no created user + + // When - the admin command sets the password File graphDir = testDir.graphDbDir(); + File confDir = new File( graphDir, "conf" ); + SetPasswordCommand setPasswordCommand = + new SetPasswordCommand( graphDir.toPath(), confDir.toPath(), mock( OutsideWorld.class ) ); + setPasswordCommand.execute( new String[]{"neo4j", "abc"} ); + + // Then - the default user does not require a password change + assertUserDoesNotRequirePasswordChange( "neo4j" ); + } + + @Test + public void shouldRunSetPasswordCommandWithExistingUser() throws Throwable + { + // Given - new user that requires password change createTestUser("neo4j", "neo4j"); assertUserRequiresPasswordChange( "neo4j" ); // When - the admin command sets the password + File graphDir = testDir.graphDbDir(); File confDir = new File( graphDir, "conf" ); SetPasswordCommand setPasswordCommand = new SetPasswordCommand( graphDir.toPath(), confDir.toPath(), mock( OutsideWorld.class ) ); @@ -134,9 +150,9 @@ public void shouldRunSetPasswordCommandWithExistinguser() throws Throwable public void shouldRunSetPasswordCommandWithoutExistingUser() throws Throwable { // Given - no user - File graphDir = testDir.graphDbDir(); // When - the admin command sets the password + File graphDir = testDir.graphDbDir(); File confDir = new File( graphDir, "conf" ); SetPasswordCommand setPasswordCommand = new SetPasswordCommand( graphDir.toPath(), confDir.toPath(), mock( OutsideWorld.class ) ); @@ -150,11 +166,11 @@ public void shouldRunSetPasswordCommandWithoutExistingUser() throws Throwable public void shouldFailToRunSetPasswordCommandWithoutExistingUser() throws Throwable { // Given - no user - File graphDir = testDir.graphDbDir(); // When - the admin command sets the password try { + File graphDir = testDir.graphDbDir(); File confDir = new File( graphDir, "conf" ); SetPasswordCommand setPasswordCommand = new SetPasswordCommand( graphDir.toPath(), confDir.toPath(), mock( OutsideWorld.class ) ); @@ -170,9 +186,7 @@ public void shouldFailToRunSetPasswordCommandWithoutExistingUser() throws Throwa @Test public void shouldRunAdminToolWithSetPasswordCommandAndNoArgs() throws Throwable { - // Given a user that requires password change - createTestUser("neo4j", "neo4j"); - assertUserRequiresPasswordChange( "neo4j" ); + // Given only default user // When running the neo4j-admin tool with incorrect parameters Path homeDir = testDir.graphDbDir().toPath(); @@ -189,7 +203,6 @@ public void shouldRunAdminToolWithSetPasswordCommandAndNoArgs() throws Throwable verify( out ).stdErrLine( " you specify the option --create=true." ); verify( out ).stdErrLine( "Missing arguments: expected username and password" ); verify( out ).exit( 1 ); - assertUserRequiresPasswordChange( "neo4j" ); } @Test @@ -202,11 +215,11 @@ public void shouldRunAdminToolWithSetPasswordCommandAndArgsButNoUser() throws Th Path configDir = testDir.directory( "conf" ).toPath(); OutsideWorld out = mock( OutsideWorld.class ); AdminTool tool = new AdminTool( CommandLocator.fromServiceLocator(), out, true ); - tool.execute( homeDir, configDir, "set-password", "neo4j", "abc" ); + tool.execute( homeDir, configDir, "set-password", "another", "abc" ); // Then we get error output and user still requires password change verify( out, times( 0 ) ).stdOutLine( anyString() ); - verify( out ).stdErrLine( "command failed: Failed to set password for 'neo4j': User 'neo4j' does not exist" ); + verify( out ).stdErrLine( "command failed: Failed to set password for 'another': User 'another' does not exist" ); verify( out ).exit( 1 ); } @@ -220,20 +233,18 @@ public void shouldRunAdminToolWithSetPasswordCommandAndArgsButNoUserAndCreateFal Path configDir = testDir.directory( "conf" ).toPath(); OutsideWorld out = mock( OutsideWorld.class ); AdminTool tool = new AdminTool( CommandLocator.fromServiceLocator(), out, true ); - tool.execute( homeDir, configDir, "set-password", "neo4j", "abc", "--create=false" ); + tool.execute( homeDir, configDir, "set-password", "another", "abc", "--create=false" ); // Then we get error output and user still requires password change verify( out, times( 0 ) ).stdOutLine( anyString() ); - verify( out ).stdErrLine( "command failed: Failed to set password for 'neo4j': User 'neo4j' does not exist" ); + verify( out ).stdErrLine( "command failed: Failed to set password for 'another': User 'another' does not exist" ); verify( out ).exit( 1 ); } @Test - public void shouldRunAdminToolWithSetPasswordCommandAndExistingUser() throws Throwable + public void shouldRunAdminToolWithSetPasswordCommandAndDefaultUser() throws Throwable { - // Given a user that requires password change - createTestUser("neo4j", "neo4j"); - assertUserRequiresPasswordChange( "neo4j" ); + // Given default state (only default user) // When running the neo4j-admin tool with correct parameters Path homeDir = testDir.graphDbDir().toPath(); @@ -249,6 +260,27 @@ public void shouldRunAdminToolWithSetPasswordCommandAndExistingUser() throws Thr assertUserDoesNotRequirePasswordChange( "neo4j" ); } + @Test + public void shouldRunAdminToolWithSetPasswordCommandAndExistingUser() throws Throwable + { + // Given a user that requires password change + createTestUser("another", "neo4j"); + assertUserRequiresPasswordChange( "another" ); + + // When running the neo4j-admin tool with correct parameters + Path homeDir = testDir.graphDbDir().toPath(); + Path configDir = testDir.directory( "conf" ).toPath(); + OutsideWorld out = mock( OutsideWorld.class ); + AdminTool tool = new AdminTool( CommandLocator.fromServiceLocator(), out, true ); + tool.execute( homeDir, configDir, "set-password", "another", "abc" ); + + // Then we get no error output and the user no longer requires password change + verify( out ).stdOutLine( "Changed password for user 'another'" ); + verify( out, times( 0 ) ).stdErrLine( anyString() ); + verify( out ).exit( 0 ); + assertUserDoesNotRequirePasswordChange( "another" ); + } + @Test public void shouldRunAdminToolWithSetPasswordCommandAndNoExistingUser() throws Throwable { @@ -259,13 +291,13 @@ public void shouldRunAdminToolWithSetPasswordCommandAndNoExistingUser() throws T Path configDir = testDir.directory( "conf" ).toPath(); OutsideWorld out = mock( OutsideWorld.class ); AdminTool tool = new AdminTool( CommandLocator.fromServiceLocator(), out, true ); - tool.execute( homeDir, configDir, "set-password", "--create=true", "neo4j", "abc" ); + tool.execute( homeDir, configDir, "set-password", "--create=true", "another", "abc" ); // Then we get no error output and the user no longer requires password change - verify( out ).stdOutLine( "Created new user 'neo4j'" ); + verify( out ).stdOutLine( "Created new user 'another'" ); verify( out, times( 0 ) ).stdErrLine( anyString() ); verify( out ).exit( 0 ); - assertUserDoesNotRequirePasswordChange( "neo4j" ); + assertUserDoesNotRequirePasswordChange( "another" ); } @Test diff --git a/community/server/src/test/java/org/neo4j/server/configuration/ConfigLoaderTest.java b/community/server/src/test/java/org/neo4j/server/configuration/ConfigLoaderTest.java index 38f7dbb9b973f..ed488e50f592a 100644 --- a/community/server/src/test/java/org/neo4j/server/configuration/ConfigLoaderTest.java +++ b/community/server/src/test/java/org/neo4j/server/configuration/ConfigLoaderTest.java @@ -202,8 +202,8 @@ public void shouldDefaultToCorrectValueForAuthStoreLocation() throws IOException .build(); Config config = configLoader.loadConfig( Optional.of( folder.getRoot() ), configFile ); - assertThat( config.get( GraphDatabaseSettings.auth_store ), - is( new File( folder.getRoot(), "data/dbms/auth" ).getAbsoluteFile() ) ); + assertThat( config.get( DatabaseManagementSystemSettings.auth_store_directory ), + is( new File( folder.getRoot(), "data/dbms" ).getAbsoluteFile() ) ); } @Test @@ -214,8 +214,8 @@ public void shouldSetAValueForAuthStoreLocation() throws IOException .build(); Config config = configLoader.loadConfig( Optional.of( folder.getRoot() ), configFile ); - assertThat( config.get( GraphDatabaseSettings.auth_store ), - is( new File( folder.getRoot(), "the-data-dir/dbms/auth" ).getAbsoluteFile() ) ); + assertThat( config.get( DatabaseManagementSystemSettings.auth_store_directory ), + is( new File( folder.getRoot(), "the-data-dir/dbms" ).getAbsoluteFile() ) ); } @Test 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 d708b9ed2c371..468c149c0f91f 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 @@ -132,10 +132,24 @@ public void start() throws Throwable userRepository.start(); roleRepository.start(); - ensureDefaultUsersAndRoles(); + ensureDefaultUsers(); + ensureDefaultRoles(); } - private void ensureDefaultUsersAndRoles() throws IOException, InvalidArgumentsException + /* Adds neo4j user if no users exist */ + private void ensureDefaultUsers() throws IOException, InvalidArgumentsException + { + if ( authenticationEnabled || authorizationEnabled ) + { + if ( numberOfUsers() == 0 ) + { + newUser( "neo4j", "neo4j", true ); + } + } + } + + /* Builds all predefined roles if no roles exist. Adds 'neo4j' to admin role if no admin is assigned */ + private void ensureDefaultRoles() throws IOException, InvalidArgumentsException { if ( authenticationEnabled || authorizationEnabled ) { @@ -146,11 +160,12 @@ private void ensureDefaultUsersAndRoles() throws IOException, InvalidArgumentsEx newRole( role ); } } - if ( numberOfUsers() == 0 ) + if ( this.getUsernamesForRole( PredefinedRolesBuilder.ADMIN ).size() == 0 ) { - newUser( "neo4j", "neo4j", true ); - // Make the default user admin for now - addUserToRole( "neo4j", PredefinedRolesBuilder.ADMIN ); + if ( getAllUsernames().contains( "neo4j" ) ) + { + addUserToRole( "neo4j", PredefinedRolesBuilder.ADMIN ); + } } } }