#1095 Update SMF hash algorithm to generate salt as SMF does

- The salt isn't used for password hashing but SMF requires that there be one to generate the authentication cookie. This does not yet enable registration from Minecraft: SMF has other non-null columns that need to be tackled. This is a first step.

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

@@ -1,12 +1,47 @@
 package fr.xephi.authme.security.crypts;
 
 import fr.xephi.authme.security.HashUtils;
+import fr.xephi.authme.security.crypts.description.HasSalt;
+import fr.xephi.authme.security.crypts.description.Recommendation;
+import fr.xephi.authme.security.crypts.description.SaltType;
+import fr.xephi.authme.security.crypts.description.Usage;
+import fr.xephi.authme.util.RandomStringUtils;
 
-public class Smf extends UsernameSaltMethod {
+/**
+ * Hashing algorithm for SMF forums.
+ * <p>
+ * The hash algorithm is {@code sha1(strtolower($username) . $password)}. However, an additional four-character
+ * salt is generated for each user, used to generate the login cookie. Therefore, this implementation generates a salt
+ * and declares that it requires a separate salt (the SMF members table has a not-null constraint on the salt column).
+ *
+ * @see <a href="https://www.simplemachines.org/">Simple Machines Forum</a>
+ */
+@Recommendation(Usage.DO_NOT_USE)
+@HasSalt(SaltType.USERNAME)
+public class Smf implements EncryptionMethod {
 
     @Override
     public HashedPassword computeHash(String password, String name) {
-        return new HashedPassword(HashUtils.sha1(name.toLowerCase() + password));
+        return new HashedPassword(computeHash(password, null, name), generateSalt());
     }
 
+    @Override
+    public String computeHash(String password, String salt, String name) {
+        return HashUtils.sha1(name.toLowerCase() + password);
+    }
+
+    @Override
+    public boolean comparePassword(String password, HashedPassword hashedPassword, String name) {
+        return computeHash(password, null, name).equals(hashedPassword.getHash());
+    }
+
+    @Override
+    public String generateSalt() {
+        return RandomStringUtils.generate(4);
+    }
+
+    @Override
+    public boolean hasSeparateSalt() {
+        return true;
+    }
 }

src/test/java/fr/xephi/authme/security/crypts/AbstractEncryptionMethodTest.java

@@ -64,10 +64,7 @@ public abstract class AbstractEncryptionMethodTest {
      */
     public AbstractEncryptionMethodTest(EncryptionMethod method, String hash0,
                                         String hash1, String hash2, String hash3) {
-        if (method.hasSeparateSalt()) {
-            throw new UnsupportedOperationException("Test must be initialized with HashedPassword objects if "
-                + "the salt is stored separately. Use the other constructor");
-        }
+        verifyCorrectConstructorIsUsed(method, false);
         this.method = method;
 
         hashes = ImmutableMap.of(
@@ -89,10 +86,7 @@ public abstract class AbstractEncryptionMethodTest {
      */
     public AbstractEncryptionMethodTest(EncryptionMethod method, HashedPassword result0, HashedPassword result1,
                                         HashedPassword result2, HashedPassword result3) {
-        if (!method.hasSeparateSalt()) {
-            throw new UnsupportedOperationException("Salt is not stored separately, so test should be initialized"
-                + " with the password hashes only. Use the other constructor");
-        }
+        verifyCorrectConstructorIsUsed(method, true);
         this.method = method;
 
         hashes = ImmutableMap.of(
@@ -107,6 +101,16 @@ public abstract class AbstractEncryptionMethodTest {
         TestHelper.setupLogger();
     }
 
+    protected void verifyCorrectConstructorIsUsed(EncryptionMethod method, boolean isConstructorWithSalt) {
+        if (isConstructorWithSalt && !method.hasSeparateSalt()) {
+            throw new UnsupportedOperationException("Salt is not stored separately, so test should be initialized"
+                + " with the password hashes only. Use the other constructor");
+        } else if (!isConstructorWithSalt && method.hasSeparateSalt()) {
+            throw new UnsupportedOperationException("Test must be initialized with HashedPassword objects if "
+                + "the salt is stored separately. Use the other constructor");
+        }
+    }
+
     @Test
     public void testGivenPasswords() {
         // Start with the 2nd to last password if we skip long tests

src/test/java/fr/xephi/authme/security/crypts/SmfTest.java

@@ -1,5 +1,11 @@
 package fr.xephi.authme.security.crypts;
 
+import org.junit.Test;
+
+import static fr.xephi.authme.AuthMeMatchers.stringWithLength;
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.Assert.assertThat;
+
 /**
  * Test for {@link Smf}.
  */
@@ -13,4 +19,25 @@ public class SmfTest extends AbstractEncryptionMethodTest {
             "03cca5af1eb0a93be47777651b2e7be4fd5d537d"); // âË_3(íù*
     }
 
+    @Override
+    protected void verifyCorrectConstructorIsUsed(EncryptionMethod method, boolean isSaltConstructor) {
+        // Smf declares to use a separate salt, but it's not used in the actual hashing mechanism, see Smf class Javadoc
+        assertThat(method.hasSeparateSalt(), equalTo(true));
+        assertThat(isSaltConstructor, equalTo(false));
+    }
+
+    @Test
+    public void shouldGenerateFourCharSalt() {
+        // given
+        EncryptionMethod method = new Smf();
+
+        // when / then
+        assertThat(method.hasSeparateSalt(), equalTo(true));
+        for (int i = 0; i < 3; ++i) {
+            HashedPassword hash = method.computeHash("pw", "name");
+            String salt = hash.getSalt();
+            assertThat(salt, stringWithLength(4));
+            assertThat(salt.matches("[a-z0-9]{4}"), equalTo(true));
+        }
+    }
 }