#432 Use injector instantiate hash algorithms

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);
     }
 
     /**

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;
+    @Inject
-    private HashAlgorithm algorithm;
+    private NewSetting settings;
-    private boolean supportOldAlgorithm;
-    private final DataSource dataSource;
-    private final PluginManager pluginManager;
 
     @Inject
-    public PasswordSecurity(DataSource dataSource, NewSetting settings, PluginManager pluginManager) {
+    private DataSource dataSource;
-        this.settings = settings;
+
+    @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,
+    public EncryptionMethod initializeEncryptionMethod(HashAlgorithm algorithm) {
-                                                              NewSetting settings) {
+        if (HashAlgorithm.CUSTOM.equals(algorithm) || HashAlgorithm.PLAINTEXT.equals(algorithm)) {
-        try {
+            return null;
-            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);
         }
+        return initializer.newInstance(algorithm.getClazz());
     }
 
     private void hashPasswordForNewAlgorithm(String password, String playerName) {

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);
     }

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);
     }

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);
+                EncryptionMethod method = initializer.newInstance(algorithm.getClazz());
-                assertThat("Encryption method for algorithm '" + algorithm + "' is not null",
-                    method, not(nullValue()));
                 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()));

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() {

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 =
+        initSettings(HashAlgorithm.BCRYPT, false);
-            new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.BCRYPT, false), pluginManager);
+        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 =
+        initSettings(HashAlgorithm.CUSTOM, false);
-            new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.CUSTOM, false), pluginManager);
+        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 =
+        initSettings(HashAlgorithm.MD5, false);
-            new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.MD5, false), pluginManager);
+        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 =
+        initSettings(HashAlgorithm.MD5, true);
-            new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.MD5, true), pluginManager);
+        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 =
+        initSettings(HashAlgorithm.MD5, true);
-            new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.MD5, true), pluginManager);
+        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 =
+        initSettings(HashAlgorithm.JOOMLA, true);
-            new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.JOOMLA, true), pluginManager);
+        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 =
+        initSettings(HashAlgorithm.XAUTH, false);
-            new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.XAUTH, false), pluginManager);
+        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);
+        initSettings(HashAlgorithm.BCRYPT, false);
-        PasswordSecurity passwordSecurity = new PasswordSecurity(dataSource, settings, pluginManager);
+        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) {
+    private void initSettings(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);
         given(settings.getProperty(HooksSettings.BCRYPT_LOG2_ROUND)).willReturn(8);
         given(settings.getProperty(SecuritySettings.DOUBLE_MD5_SALT_LENGTH)).willReturn(16);
-        return settings;
     }
 
 }