From 45fd241517891ce953671eeecf5397fca4520dd8 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 12 Mar 2016 08:16:57 +0100 Subject: [PATCH] Update settings in PasswordSecurity upon reload --- src/main/java/fr/xephi/authme/AuthMe.java | 11 +++-- .../authme/security/PasswordSecurity.java | 18 ++++--- .../AbstractResourceClosingTest.java | 2 +- .../authme/security/PasswordSecurityTest.java | 48 ++++++++++++++++--- 4 files changed, 61 insertions(+), 18 deletions(-) diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index 005297426..81e8781d8 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -248,8 +248,7 @@ public class AuthMe extends JavaPlugin { } MigrationService.changePlainTextToSha256(newSettings, database, new SHA256()); - passwordSecurity = new PasswordSecurity(getDataSource(), newSettings.getProperty(SecuritySettings.PASSWORD_HASH), - Bukkit.getPluginManager(), newSettings.getProperty(SecuritySettings.SUPPORT_OLD_PASSWORD_HASH)); + passwordSecurity = new PasswordSecurity(getDataSource(), newSettings, Bukkit.getPluginManager()); ipAddressManager = new IpAddressManager(newSettings); // Set up the permissions manager and command handler @@ -338,10 +337,14 @@ public class AuthMe extends JavaPlugin { */ public void reload() throws Exception { newSettings.reload(); + // We do not change database type for consistency issues, but we'll output a note in the logs + if (!newSettings.getProperty(DatabaseSettings.BACKEND).equals(database.getType())) { + ConsoleLogger.info("Note: cannot change database type during /authme reload"); + } + database.reload(); messages.reload(newSettings.getMessagesFile()); - // TODO ljacqu 20160309: change static interface of Spawn + passwordSecurity.reload(newSettings); Spawn.reload(); - // setupDatabase(newSettings); } /** diff --git a/src/main/java/fr/xephi/authme/security/PasswordSecurity.java b/src/main/java/fr/xephi/authme/security/PasswordSecurity.java index 1a8278e12..da506b6bb 100644 --- a/src/main/java/fr/xephi/authme/security/PasswordSecurity.java +++ b/src/main/java/fr/xephi/authme/security/PasswordSecurity.java @@ -4,6 +4,8 @@ import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.events.PasswordEncryptionEvent; import fr.xephi.authme.security.crypts.EncryptionMethod; import fr.xephi.authme.security.crypts.HashedPassword; +import fr.xephi.authme.settings.NewSetting; +import fr.xephi.authme.settings.properties.SecuritySettings; import org.bukkit.plugin.PluginManager; /** @@ -11,17 +13,16 @@ import org.bukkit.plugin.PluginManager; */ public class PasswordSecurity { + private HashAlgorithm algorithm; + private boolean supportOldAlgorithm; private final DataSource dataSource; - private final HashAlgorithm algorithm; private final PluginManager pluginManager; - private final boolean supportOldAlgorithm; - public PasswordSecurity(DataSource dataSource, HashAlgorithm algorithm, - PluginManager pluginManager, boolean supportOldAlgorithm) { + public PasswordSecurity(DataSource dataSource, NewSetting settings, PluginManager pluginManager) { + this.algorithm = settings.getProperty(SecuritySettings.PASSWORD_HASH); + this.supportOldAlgorithm = settings.getProperty(SecuritySettings.SUPPORT_OLD_PASSWORD_HASH); this.dataSource = dataSource; - this.algorithm = algorithm; this.pluginManager = pluginManager; - this.supportOldAlgorithm = supportOldAlgorithm; } public HashedPassword computeHash(String password, String playerName) { @@ -46,6 +47,11 @@ public class PasswordSecurity { || supportOldAlgorithm && compareWithAllEncryptionMethods(password, hashedPassword, playerLowerCase); } + public void reload(NewSetting settings) { + this.algorithm = settings.getProperty(SecuritySettings.PASSWORD_HASH); + this.supportOldAlgorithm = settings.getProperty(SecuritySettings.SUPPORT_OLD_PASSWORD_HASH); + } + /** * Compare the given hash with all available encryption methods to support * the migration to a new encryption method. Upon a successful match, the password diff --git a/src/test/java/fr/xephi/authme/datasource/AbstractResourceClosingTest.java b/src/test/java/fr/xephi/authme/datasource/AbstractResourceClosingTest.java index 328dfc6d5..f5ba58e85 100644 --- a/src/test/java/fr/xephi/authme/datasource/AbstractResourceClosingTest.java +++ b/src/test/java/fr/xephi/authme/datasource/AbstractResourceClosingTest.java @@ -58,7 +58,7 @@ import static org.mockito.Mockito.verify; public abstract class AbstractResourceClosingTest { /** List of DataSource method names not to test. */ - private static final Set IGNORED_METHODS = ImmutableSet.of("close", "getType"); + private static final Set IGNORED_METHODS = ImmutableSet.of("reload", "close", "getType"); /** Collection of values to use to call methods with the parameters they expect. */ private static final Map, Object> PARAM_VALUES = getDefaultParameters(); diff --git a/src/test/java/fr/xephi/authme/security/PasswordSecurityTest.java b/src/test/java/fr/xephi/authme/security/PasswordSecurityTest.java index 81fdebea3..de223e6f2 100644 --- a/src/test/java/fr/xephi/authme/security/PasswordSecurityTest.java +++ b/src/test/java/fr/xephi/authme/security/PasswordSecurityTest.java @@ -1,5 +1,6 @@ package fr.xephi.authme.security; +import fr.xephi.authme.ReflectionTestUtils; import fr.xephi.authme.cache.auth.PlayerAuth; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.events.PasswordEncryptionEvent; @@ -7,6 +8,8 @@ import fr.xephi.authme.security.crypts.HashedPassword; import fr.xephi.authme.security.crypts.EncryptionMethod; import fr.xephi.authme.security.crypts.JOOMLA; import fr.xephi.authme.security.crypts.PHPBB; +import fr.xephi.authme.settings.NewSetting; +import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.util.WrapperMock; import org.bukkit.event.Event; import org.bukkit.plugin.PluginManager; @@ -73,7 +76,8 @@ public class PasswordSecurityTest { given(dataSource.getPassword(playerName)).willReturn(password); given(method.comparePassword(clearTextPass, password, playerLowerCase)).willReturn(true); - PasswordSecurity security = new PasswordSecurity(dataSource, HashAlgorithm.BCRYPT, pluginManager, false); + PasswordSecurity security = + new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.BCRYPT, false), pluginManager); // when boolean result = security.comparePassword(clearTextPass, playerName); @@ -96,7 +100,8 @@ public class PasswordSecurityTest { PlayerAuth auth = mock(PlayerAuth.class); given(dataSource.getPassword(playerName)).willReturn(password); given(method.comparePassword(clearTextPass, password, playerLowerCase)).willReturn(false); - PasswordSecurity security = new PasswordSecurity(dataSource, HashAlgorithm.CUSTOM, pluginManager, false); + PasswordSecurity security = + new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.CUSTOM, false), pluginManager); // when boolean result = security.comparePassword(clearTextPass, playerName); @@ -115,7 +120,8 @@ public class PasswordSecurityTest { String clearTextPass = "tables"; given(dataSource.getPassword(playerName)).willReturn(null); - PasswordSecurity security = new PasswordSecurity(dataSource, HashAlgorithm.MD5, pluginManager, false); + PasswordSecurity security = + new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.MD5, false), pluginManager); // when boolean result = security.comparePassword(clearTextPass, playerName); @@ -142,7 +148,8 @@ public class PasswordSecurityTest { given(dataSource.getPassword(argThat(equalToIgnoringCase(playerName)))).willReturn(password); given(method.comparePassword(clearTextPass, password, playerLowerCase)).willReturn(false); given(method.computeHash(clearTextPass, playerLowerCase)).willReturn(newPassword); - PasswordSecurity security = new PasswordSecurity(dataSource, HashAlgorithm.MD5, pluginManager, true); + PasswordSecurity security = + new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.MD5, true), pluginManager); // when boolean result = security.comparePassword(clearTextPass, playerName); @@ -166,7 +173,8 @@ public class PasswordSecurityTest { String usernameLowerCase = username.toLowerCase(); HashedPassword hashedPassword = new HashedPassword("$T$est#Hash", "__someSalt__"); given(method.computeHash(password, usernameLowerCase)).willReturn(hashedPassword); - PasswordSecurity security = new PasswordSecurity(dataSource, HashAlgorithm.JOOMLA, pluginManager, true); + PasswordSecurity security = + new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.JOOMLA, true), pluginManager); // when HashedPassword result = security.computeHash(password, username); @@ -187,7 +195,8 @@ public class PasswordSecurityTest { String username = "someone12"; HashedPassword hashedPassword = new HashedPassword("~T!est#Hash", "__someSalt__"); given(method.computeHash(password, username)).willReturn(hashedPassword); - PasswordSecurity security = new PasswordSecurity(dataSource, HashAlgorithm.JOOMLA, pluginManager, true); + PasswordSecurity security = + new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.JOOMLA, true), pluginManager); // when HashedPassword result = security.computeHash(HashAlgorithm.PHPBB, password, username); @@ -209,7 +218,8 @@ public class PasswordSecurityTest { HashedPassword hashedPassword = new HashedPassword("~T!est#Hash"); given(method.computeHash(password, username)).willReturn(hashedPassword); given(method.hasSeparateSalt()).willReturn(true); - PasswordSecurity security = new PasswordSecurity(dataSource, HashAlgorithm.XAUTH, pluginManager, false); + PasswordSecurity security = + new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.XAUTH, false), pluginManager); // when boolean result = security.comparePassword(password, hashedPassword, username); @@ -221,4 +231,28 @@ public class PasswordSecurityTest { verify(method, never()).comparePassword(anyString(), any(HashedPassword.class), anyString()); } + @Test + public void shouldReloadSettings() { + // given + PasswordSecurity passwordSecurity = + new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.BCRYPT, false), pluginManager); + NewSetting updatedSettings = mockSettings(HashAlgorithm.MD5, true); + + // when + passwordSecurity.reload(updatedSettings); + + // then + assertThat(ReflectionTestUtils.getFieldValue(PasswordSecurity.class, passwordSecurity, "algorithm"), + equalTo((Object) HashAlgorithm.MD5)); + assertThat(ReflectionTestUtils.getFieldValue(PasswordSecurity.class, passwordSecurity, "supportOldAlgorithm"), + equalTo((Object) Boolean.TRUE)); + } + + private static NewSetting mockSettings(HashAlgorithm algorithm, boolean supportOldPassword) { + NewSetting settings = mock(NewSetting.class); + given(settings.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(algorithm); + given(settings.getProperty(SecuritySettings.SUPPORT_OLD_PASSWORD_HASH)).willReturn(supportOldPassword); + return settings; + } + }