diff --git a/src/main/java/fr/xephi/authme/initialization/AuthMeServiceInitializer.java b/src/main/java/fr/xephi/authme/initialization/AuthMeServiceInitializer.java index 576ea2277..c2241c11d 100644 --- a/src/main/java/fr/xephi/authme/initialization/AuthMeServiceInitializer.java +++ b/src/main/java/fr/xephi/authme/initialization/AuthMeServiceInitializer.java @@ -37,6 +37,7 @@ public class AuthMeServiceInitializer { public AuthMeServiceInitializer(String... allowedPackages) { ALLOWED_PACKAGES = ImmutableSet.copyOf(allowedPackages); objects = new HashMap<>(); + objects.put(getClass(), this); } /** diff --git a/src/main/java/fr/xephi/authme/security/PasswordSecurity.java b/src/main/java/fr/xephi/authme/security/PasswordSecurity.java index 458c99699..4a525531b 100644 --- a/src/main/java/fr/xephi/authme/security/PasswordSecurity.java +++ b/src/main/java/fr/xephi/authme/security/PasswordSecurity.java @@ -2,34 +2,43 @@ package fr.xephi.authme.security; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.events.PasswordEncryptionEvent; +import fr.xephi.authme.initialization.AuthMeServiceInitializer; 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; +import javax.annotation.PostConstruct; import javax.inject.Inject; -import java.lang.reflect.Constructor; -import java.lang.reflect.InvocationTargetException; /** * Manager class for password-related operations. */ public class PasswordSecurity { - private final NewSetting settings; - private HashAlgorithm algorithm; - private boolean supportOldAlgorithm; - private final DataSource dataSource; - private final PluginManager pluginManager; + @Inject + private NewSetting settings; @Inject - public PasswordSecurity(DataSource dataSource, NewSetting settings, PluginManager pluginManager) { - this.settings = settings; + private DataSource dataSource; + + @Inject + private PluginManager pluginManager; + + @Inject + private AuthMeServiceInitializer initializer; + + private HashAlgorithm algorithm; + private boolean supportOldAlgorithm; + + /** + * Load or reload the configuration. + */ + @PostConstruct + public void reload() { this.algorithm = settings.getProperty(SecuritySettings.PASSWORD_HASH); this.supportOldAlgorithm = settings.getProperty(SecuritySettings.SUPPORT_OLD_PASSWORD_HASH); - this.dataSource = dataSource; - this.pluginManager = pluginManager; } /** @@ -75,14 +84,6 @@ public class PasswordSecurity { || supportOldAlgorithm && compareWithAllEncryptionMethods(password, hashedPassword, playerLowerCase); } - /** - * Reload the configuration. - */ - public void reload() { - 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 @@ -97,7 +98,7 @@ public class PasswordSecurity { private boolean compareWithAllEncryptionMethods(String password, HashedPassword hashedPassword, String playerName) { for (HashAlgorithm algorithm : HashAlgorithm.values()) { if (!HashAlgorithm.CUSTOM.equals(algorithm)) { - EncryptionMethod method = initializeEncryptionMethod(algorithm, settings); + EncryptionMethod method = initializeEncryptionMethod(algorithm); if (methodMatches(method, password, hashedPassword, playerName)) { hashPasswordForNewAlgorithm(password, playerName); return true; @@ -135,7 +136,7 @@ public class PasswordSecurity { * @return The encryption method */ private EncryptionMethod initializeEncryptionMethodWithEvent(HashAlgorithm algorithm, String playerName) { - EncryptionMethod method = initializeEncryptionMethod(algorithm, settings); + EncryptionMethod method = initializeEncryptionMethod(algorithm); PasswordEncryptionEvent event = new PasswordEncryptionEvent(method, playerName); pluginManager.callEvent(event); return event.getMethod(); @@ -145,30 +146,14 @@ public class PasswordSecurity { * Initialize the encryption method associated with the given hash algorithm. * * @param algorithm The algorithm to retrieve the encryption method for - * @param settings The settings instance to pass to the constructor if required * * @return The associated encryption method, or null if CUSTOM / deprecated */ - public static EncryptionMethod initializeEncryptionMethod(HashAlgorithm algorithm, - NewSetting settings) { - try { - if (HashAlgorithm.CUSTOM.equals(algorithm) || HashAlgorithm.PLAINTEXT.equals(algorithm)) { - return null; - } - Constructor constructor = algorithm.getClazz().getConstructors()[0]; - Class[] parameters = constructor.getParameterTypes(); - if (parameters.length == 0) { - return (EncryptionMethod) constructor.newInstance(); - } else if (parameters.length == 1 && parameters[0] == NewSetting.class) { - return (EncryptionMethod) constructor.newInstance(settings); - } else { - throw new UnsupportedOperationException("Did not find default constructor or constructor with settings " - + "parameter in class " + algorithm.getClazz().getSimpleName()); - } - } catch (InstantiationException | IllegalAccessException | InvocationTargetException e) { - throw new UnsupportedOperationException("Constructor for '" + algorithm.getClazz().getSimpleName() - + "' could not be invoked. (Is there no default constructor?)", e); + public EncryptionMethod initializeEncryptionMethod(HashAlgorithm algorithm) { + if (HashAlgorithm.CUSTOM.equals(algorithm) || HashAlgorithm.PLAINTEXT.equals(algorithm)) { + return null; } + return initializer.newInstance(algorithm.getClazz()); } private void hashPasswordForNewAlgorithm(String password, String playerName) { diff --git a/src/main/java/fr/xephi/authme/security/crypts/BCRYPT.java b/src/main/java/fr/xephi/authme/security/crypts/BCRYPT.java index a114adc09..67d8c794e 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/BCRYPT.java +++ b/src/main/java/fr/xephi/authme/security/crypts/BCRYPT.java @@ -9,13 +9,15 @@ import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.properties.HooksSettings; import fr.xephi.authme.util.StringUtils; +import javax.inject.Inject; @Recommendation(Usage.RECOMMENDED) // provided the salt length is >= 8 -@HasSalt(value = SaltType.TEXT) // length depends on Settings.bCryptLog2Rounds +@HasSalt(value = SaltType.TEXT) // length depends on the bcryptLog2Rounds setting public class BCRYPT implements EncryptionMethod { private final int bCryptLog2Rounds; + @Inject public BCRYPT(NewSetting settings) { this.bCryptLog2Rounds = settings.getProperty(HooksSettings.BCRYPT_LOG2_ROUND); } diff --git a/src/main/java/fr/xephi/authme/security/crypts/SALTED2MD5.java b/src/main/java/fr/xephi/authme/security/crypts/SALTED2MD5.java index 27066f7a2..bf67432b8 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/SALTED2MD5.java +++ b/src/main/java/fr/xephi/authme/security/crypts/SALTED2MD5.java @@ -8,14 +8,17 @@ import fr.xephi.authme.security.crypts.description.Usage; import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.properties.SecuritySettings; +import javax.inject.Inject; + import static fr.xephi.authme.security.HashUtils.md5; @Recommendation(Usage.ACCEPTABLE) // presuming that length is something sensible (>= 8) -@HasSalt(value = SaltType.TEXT) // length defined by Settings.saltLength +@HasSalt(value = SaltType.TEXT) // length defined by the doubleMd5SaltLength setting public class SALTED2MD5 extends SeparateSaltMethod { private final int saltLength; + @Inject public SALTED2MD5(NewSetting settings) { saltLength = settings.getProperty(SecuritySettings.DOUBLE_MD5_SALT_LENGTH); } diff --git a/src/test/java/fr/xephi/authme/security/HashAlgorithmIntegrationTest.java b/src/test/java/fr/xephi/authme/security/HashAlgorithmIntegrationTest.java index 976a7a445..30940c345 100644 --- a/src/test/java/fr/xephi/authme/security/HashAlgorithmIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/security/HashAlgorithmIntegrationTest.java @@ -1,5 +1,6 @@ package fr.xephi.authme.security; +import fr.xephi.authme.initialization.AuthMeServiceInitializer; import fr.xephi.authme.security.crypts.EncryptionMethod; import fr.xephi.authme.security.crypts.HashedPassword; import fr.xephi.authme.settings.NewSetting; @@ -13,8 +14,6 @@ import java.util.HashSet; import java.util.Set; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; import static org.mockito.BDDMockito.given; @@ -25,13 +24,15 @@ import static org.mockito.Mockito.mock; */ public class HashAlgorithmIntegrationTest { - private static NewSetting settings; + private static AuthMeServiceInitializer initializer; @BeforeClass public static void setUpWrapper() { - settings = mock(NewSetting.class); + NewSetting settings = mock(NewSetting.class); given(settings.getProperty(HooksSettings.BCRYPT_LOG2_ROUND)).willReturn(8); given(settings.getProperty(SecuritySettings.DOUBLE_MD5_SALT_LENGTH)).willReturn(16); + initializer = new AuthMeServiceInitializer(); + initializer.register(NewSetting.class, settings); } @Test @@ -55,9 +56,7 @@ public class HashAlgorithmIntegrationTest { // given / when / then for (HashAlgorithm algorithm : HashAlgorithm.values()) { if (!HashAlgorithm.CUSTOM.equals(algorithm) && !HashAlgorithm.PLAINTEXT.equals(algorithm)) { - EncryptionMethod method = PasswordSecurity.initializeEncryptionMethod(algorithm, settings); - assertThat("Encryption method for algorithm '" + algorithm + "' is not null", - method, not(nullValue())); + EncryptionMethod method = initializer.newInstance(algorithm.getClazz()); HashedPassword hashedPassword = method.computeHash("pwd", "name"); assertThat("Salt should not be null if method.hasSeparateSalt(), and vice versa. Method: '" + method + "'", StringUtils.isEmpty(hashedPassword.getSalt()), equalTo(!method.hasSeparateSalt())); diff --git a/src/test/java/fr/xephi/authme/security/HashUtilsTest.java b/src/test/java/fr/xephi/authme/security/HashUtilsTest.java index e515fdbba..c7ef1ccd2 100644 --- a/src/test/java/fr/xephi/authme/security/HashUtilsTest.java +++ b/src/test/java/fr/xephi/authme/security/HashUtilsTest.java @@ -18,7 +18,7 @@ public class HashUtilsTest { /** * List of passwords whose hash is provided to the class to test against. */ - public static final String[] GIVEN_PASSWORDS = {"", "password", "PassWord1", "&^%te$t?Pw@_"}; + private static final String[] GIVEN_PASSWORDS = {"", "password", "PassWord1", "&^%te$t?Pw@_"}; @Test public void shouldHashMd5() { diff --git a/src/test/java/fr/xephi/authme/security/PasswordSecurityTest.java b/src/test/java/fr/xephi/authme/security/PasswordSecurityTest.java index 470b8a2d5..f3d577dc9 100644 --- a/src/test/java/fr/xephi/authme/security/PasswordSecurityTest.java +++ b/src/test/java/fr/xephi/authme/security/PasswordSecurityTest.java @@ -4,6 +4,7 @@ import fr.xephi.authme.ReflectionTestUtils; import fr.xephi.authme.TestHelper; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.events.PasswordEncryptionEvent; +import fr.xephi.authme.initialization.AuthMeServiceInitializer; import fr.xephi.authme.security.crypts.EncryptionMethod; import fr.xephi.authme.security.crypts.HashedPassword; import fr.xephi.authme.security.crypts.JOOMLA; @@ -30,7 +31,6 @@ import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.argThat; import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -41,12 +41,20 @@ import static org.mockito.Mockito.verify; @RunWith(MockitoJUnitRunner.class) public class PasswordSecurityTest { + private AuthMeServiceInitializer initializer; + + @Mock + private NewSetting settings; + @Mock private PluginManager pluginManager; + @Mock private DataSource dataSource; + @Mock private EncryptionMethod method; + private Class caughtClassInEvent; @BeforeClass @@ -71,6 +79,10 @@ public class PasswordSecurityTest { return null; } }).when(pluginManager).callEvent(any(Event.class)); + initializer = new AuthMeServiceInitializer(new String[]{}); + initializer.register(NewSetting.class, settings); + initializer.register(DataSource.class, dataSource); + initializer.register(PluginManager.class, pluginManager); } @Test @@ -84,8 +96,8 @@ public class PasswordSecurityTest { given(dataSource.getPassword(playerName)).willReturn(password); given(method.comparePassword(clearTextPass, password, playerLowerCase)).willReturn(true); - PasswordSecurity security = - new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.BCRYPT, false), pluginManager); + initSettings(HashAlgorithm.BCRYPT, false); + PasswordSecurity security = initializer.newInstance(PasswordSecurity.class); // when boolean result = security.comparePassword(clearTextPass, playerName); @@ -107,8 +119,8 @@ public class PasswordSecurityTest { given(dataSource.getPassword(playerName)).willReturn(password); given(method.comparePassword(clearTextPass, password, playerLowerCase)).willReturn(false); - PasswordSecurity security = - new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.CUSTOM, false), pluginManager); + initSettings(HashAlgorithm.CUSTOM, false); + PasswordSecurity security = initializer.newInstance(PasswordSecurity.class); // when boolean result = security.comparePassword(clearTextPass, playerName); @@ -127,8 +139,8 @@ public class PasswordSecurityTest { String clearTextPass = "tables"; given(dataSource.getPassword(playerName)).willReturn(null); - PasswordSecurity security = - new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.MD5, false), pluginManager); + initSettings(HashAlgorithm.MD5, false); + PasswordSecurity security = initializer.newInstance(PasswordSecurity.class); // when boolean result = security.comparePassword(clearTextPass, playerName); @@ -155,8 +167,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, mockSettings(HashAlgorithm.MD5, true), pluginManager); + initSettings(HashAlgorithm.MD5, true); + PasswordSecurity security = initializer.newInstance(PasswordSecurity.class); // when boolean result = security.comparePassword(clearTextPass, playerName); @@ -180,8 +192,8 @@ public class PasswordSecurityTest { String clearTextPass = "someInvalidPassword"; given(dataSource.getPassword(playerName)).willReturn(password); given(method.comparePassword(clearTextPass, password, playerName)).willReturn(false); - PasswordSecurity security = - new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.MD5, true), pluginManager); + initSettings(HashAlgorithm.MD5, true); + PasswordSecurity security = initializer.newInstance(PasswordSecurity.class); // when boolean result = security.comparePassword(clearTextPass, playerName); @@ -199,8 +211,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, mockSettings(HashAlgorithm.JOOMLA, true), pluginManager); + initSettings(HashAlgorithm.JOOMLA, true); + PasswordSecurity security = initializer.newInstance(PasswordSecurity.class); // when HashedPassword result = security.computeHash(password, username); @@ -222,8 +234,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, mockSettings(HashAlgorithm.XAUTH, false), pluginManager); + initSettings(HashAlgorithm.XAUTH, false); + PasswordSecurity security = initializer.newInstance(PasswordSecurity.class); // when boolean result = security.comparePassword(password, hashedPassword, username); @@ -238,8 +250,8 @@ public class PasswordSecurityTest { @Test public void shouldReloadSettings() { // given - NewSetting settings = mockSettings(HashAlgorithm.BCRYPT, false); - PasswordSecurity passwordSecurity = new PasswordSecurity(dataSource, settings, pluginManager); + initSettings(HashAlgorithm.BCRYPT, false); + PasswordSecurity passwordSecurity = initializer.newInstance(PasswordSecurity.class); given(settings.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(HashAlgorithm.MD5); given(settings.getProperty(SecuritySettings.SUPPORT_OLD_PASSWORD_HASH)).willReturn(true); @@ -253,13 +265,11 @@ public class PasswordSecurityTest { equalTo((Object) Boolean.TRUE)); } - private static NewSetting mockSettings(HashAlgorithm algorithm, boolean supportOldPassword) { - NewSetting settings = mock(NewSetting.class); + private void initSettings(HashAlgorithm algorithm, boolean supportOldPassword) { given(settings.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(algorithm); given(settings.getProperty(SecuritySettings.SUPPORT_OLD_PASSWORD_HASH)).willReturn(supportOldPassword); given(settings.getProperty(HooksSettings.BCRYPT_LOG2_ROUND)).willReturn(8); given(settings.getProperty(SecuritySettings.DOUBLE_MD5_SALT_LENGTH)).willReturn(16); - return settings; } }