Fix #440 Hash algo's sometimes skipped for old algorithm support

- Fix check that discards potentially trying all encryption methods if password didn't match
- Wrap call to encryption method properly to avoid calling methods with hasSeparateSalt() = true and a null salt

src/main/java/fr/xephi/authme/security/PasswordSecurity.java

@@ -41,15 +41,8 @@ public class PasswordSecurity {
 
     public boolean comparePassword(String password, HashedPassword hashedPassword, String playerName) {
         EncryptionMethod method = initializeEncryptionMethod(algorithm, playerName);
-        // User is not in data source, so the result will invariably be wrong because an encryption
-        // method with hasSeparateSalt() == true NEEDS the salt to evaluate the password
-        String salt = hashedPassword.getSalt();
-        if (method.hasSeparateSalt() && salt == null) {
-            return false;
-        }
-
         String playerLowerCase = playerName.toLowerCase();
-        return method.comparePassword(password, hashedPassword, playerLowerCase)
+        return methodMatches(method, password, hashedPassword, playerLowerCase)
             || supportOldAlgorithm && compareWithAllEncryptionMethods(password, hashedPassword, playerLowerCase);
     }
 
@@ -69,7 +62,7 @@ public class PasswordSecurity {
         for (HashAlgorithm algorithm : HashAlgorithm.values()) {
             if (!HashAlgorithm.CUSTOM.equals(algorithm)) {
                 EncryptionMethod method = initializeEncryptionMethodWithoutEvent(algorithm);
-                if (method != null && method.comparePassword(password, hashedPassword, playerName)) {
+                if (methodMatches(method, password, hashedPassword, playerName)) {
                     hashPasswordForNewAlgorithm(password, playerName);
                     return true;
                 }
@@ -78,6 +71,22 @@ public class PasswordSecurity {
         return false;
     }
 
+    /**
+     * Verify with the given encryption method whether the password matches the hash after checking that
+     * the method can be called safely with the given data.
+     *
+     * @param method The encryption method to use
+     * @param password The password to check
+     * @param hashedPassword The hash to check against
+     * @param playerName The name of the player
+     * @return True if the password matched, false otherwise
+     */
+    private static boolean methodMatches(EncryptionMethod method, String password,
+                                         HashedPassword hashedPassword, String playerName) {
+        return method != null && (!method.hasSeparateSalt() || hashedPassword.getSalt() != null)
+            && method.comparePassword(password, hashedPassword, playerName);
+    }
+
     /**
      * Get the encryption method from the given {@link HashAlgorithm} value and emit a
      * {@link PasswordEncryptionEvent}. The encryption method from the event is then returned,

src/main/java/fr/xephi/authme/security/crypts/WBB4.java

@@ -1,7 +1,9 @@
 package fr.xephi.authme.security.crypts;
 
+import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.security.crypts.description.Recommendation;
 import fr.xephi.authme.security.crypts.description.Usage;
+import fr.xephi.authme.util.StringUtils;
 
 @Recommendation(Usage.DOES_NOT_WORK)
 public class WBB4 extends HexSaltedMethod {
@@ -13,7 +15,12 @@ public class WBB4 extends HexSaltedMethod {
 
     @Override
     public boolean comparePassword(String password, HashedPassword hashedPassword, String playerName) {
-        return BCRYPT.checkpw(password, hashedPassword.getHash(), 2);
+        try {
+            return BCRYPT.checkpw(password, hashedPassword.getHash(), 2);
+        } catch (IllegalArgumentException e) {
+            ConsoleLogger.showError("WBB4 compare password returned: " + StringUtils.formatException(e));
+        }
+        return false;
     }
 
     @Override

src/test/java/fr/xephi/authme/security/PasswordSecurityTest.java

@@ -7,6 +7,7 @@ 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.util.WrapperMock;
 import org.bukkit.event.Event;
 import org.bukkit.plugin.PluginManager;
 import org.junit.Before;
@@ -42,6 +43,7 @@ public class PasswordSecurityTest {
 
     @Before
     public void setUpMocks() {
+        WrapperMock.createInstance();
         pluginManager = mock(PluginManager.class);
         dataSource = mock(DataSource.class);
         method = mock(EncryptionMethod.class);
@@ -209,7 +211,7 @@ 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, true);
+        PasswordSecurity security = new PasswordSecurity(dataSource, HashAlgorithm.XAUTH, pluginManager, false);
 
         // when
         boolean result = security.comparePassword(password, hashedPassword, username);