SONAR-6949 Implements bcrypt hash for password

Extract hash mechanism into a single class LocalAuthentication
Implements SHA1 (deprecated) and bcrypt hash
Set bcrypt as default
Update the hash of a user during authentication if hash method was SHA1

4 files changed, 9 insertions, 27 deletions

diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDto.java
index 1baa5f72892..32b5e69e835 100644
--- a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDto.java
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDto.java
@@ -25,11 +25,8 @@ import java.util.ArrayList;
 import java.util.List;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
-import org.apache.commons.codec.digest.DigestUtils;
 import org.sonar.core.user.DefaultUser;
 
-import static java.util.Objects.requireNonNull;
-
 /**
  * @since 3.2
  */
@@ -44,8 +41,11 @@ public class UserDto {
   private String scmAccounts;
   private String externalIdentity;
   private String externalIdentityProvider;
+  // Hashed password that may be null in case of external authentication
   private String cryptedPassword;
+  // Salt used for SHA1, null when bcrypt is used or for external authentication
   private String salt;
+  // Hash method used to generate cryptedPassword, my be null in case of external authentication
   private String hashMethod;
   private Long createdAt;
   private Long updatedAt;
@@ -192,7 +192,7 @@ public class UserDto {
     return hashMethod;
   }
 
-  public UserDto setHashMethod(String hashMethod) {
+  public UserDto setHashMethod(@Nullable String hashMethod) {
     this.hashMethod = hashMethod;
     return this;
   }
@@ -260,12 +260,6 @@ public class UserDto {
     return this;
   }
 
-  public static String encryptPassword(String password, String salt) {
-    requireNonNull(password, "Password cannot be empty");
-    requireNonNull(salt, "Salt cannot be empty");
-    return DigestUtils.sha1Hex("--" + salt + "--" + password + "--");
-  }
-
   public DefaultUser toUser() {
     return new DefaultUser()
       .setLogin(login)
diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/user/UserMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/user/UserMapper.xml
index d699cf7ba2a..97909a40307 100644
--- a/server/sonar-db-dao/src/main/resources/org/sonar/db/user/UserMapper.xml
+++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/user/UserMapper.xml
@@ -221,6 +221,7 @@
         onboarded = #{user.onboarded, jdbcType=BOOLEAN},
         salt = #{user.salt, jdbcType=VARCHAR},
         crypted_password = #{user.cryptedPassword, jdbcType=BIGINT},
+        hash_method = #{user.hashMethod, jdbcType=VARCHAR},
         updated_at = #{now, jdbcType=BIGINT},
         homepage_type = #{user.homepageType, jdbcType=VARCHAR},
         homepage_parameter = #{user.homepageParameter, jdbcType=VARCHAR}
diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/user/UserDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/user/UserDaoTest.java
index 1286768f703..e5899b9d08a 100644
--- a/server/sonar-db-dao/src/test/java/org/sonar/db/user/UserDaoTest.java
+++ b/server/sonar-db-dao/src/test/java/org/sonar/db/user/UserDaoTest.java
@@ -319,6 +319,7 @@ public class UserDaoTest {
       .setOnboarded(true)
       .setSalt("1234")
       .setCryptedPassword("abcd")
+      .setHashMethod("SHA1")
       .setExternalIdentity("johngithub")
       .setExternalIdentityProvider("github")
       .setLocal(true)
@@ -340,6 +341,7 @@ public class UserDaoTest {
     assertThat(user.getScmAccounts()).isEqualTo(",jo.hn,john2,");
     assertThat(user.getSalt()).isEqualTo("1234");
     assertThat(user.getCryptedPassword()).isEqualTo("abcd");
+    assertThat(user.getHashMethod()).isEqualTo("SHA1");
     assertThat(user.getExternalIdentity()).isEqualTo("johngithub");
     assertThat(user.getExternalIdentityProvider()).isEqualTo("github");
     assertThat(user.isLocal()).isTrue();
@@ -368,6 +370,7 @@ public class UserDaoTest {
       .setOnboarded(true)
       .setSalt("12345")
       .setCryptedPassword("abcde")
+      .setHashMethod("BCRYPT")
       .setExternalIdentity("johngithub")
       .setExternalIdentityProvider("github")
       .setLocal(false)
@@ -386,6 +389,7 @@ public class UserDaoTest {
     assertThat(reloaded.getScmAccounts()).isEqualTo(",jo.hn,john2,johndoo,");
     assertThat(reloaded.getSalt()).isEqualTo("12345");
     assertThat(reloaded.getCryptedPassword()).isEqualTo("abcde");
+    assertThat(reloaded.getHashMethod()).isEqualTo("BCRYPT");
     assertThat(reloaded.getExternalIdentity()).isEqualTo("johngithub");
     assertThat(reloaded.getExternalIdentityProvider()).isEqualTo("github");
     assertThat(reloaded.isLocal()).isFalse();
diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/user/UserDtoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/user/UserDtoTest.java
index f34a9c226ed..15613d0f59f 100644
--- a/server/sonar-db-dao/src/test/java/org/sonar/db/user/UserDtoTest.java
+++ b/server/sonar-db-dao/src/test/java/org/sonar/db/user/UserDtoTest.java
@@ -46,21 +46,4 @@ public class UserDtoTest {
     assertThat(UserDto.decodeScmAccounts("\nfoo\n")).containsOnly("foo");
     assertThat(UserDto.decodeScmAccounts("\nfoo\nbar\n")).containsOnly("foo", "bar");
   }
-
-  @Test
-  public void encrypt_password() {
-    assertThat(UserDto.encryptPassword("PASSWORD", "0242b0b4c0a93ddfe09dd886de50bc25ba000b51")).isEqualTo("540e4fc4be4e047db995bc76d18374a5b5db08cc");
-  }
-
-  @Test
-  public void fail_to_encrypt_password_when_password_is_null() {
-    expectedException.expect(NullPointerException.class);
-    UserDto.encryptPassword(null, "salt");
-  }
-
-  @Test
-  public void fail_to_encrypt_password_when_salt_is_null() {
-    expectedException.expect(NullPointerException.class);
-    UserDto.encryptPassword("password", null);
-  }
 }