SONAR-9426 Set external identity info when reactivating local user

4 files changed, 31 insertions, 19 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/UserIdentityAuthenticator.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/UserIdentityAuthenticator.java
index 396cea50b03..58f35204e38 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/authentication/UserIdentityAuthenticator.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/UserIdentityAuthenticator.java
@@ -121,8 +121,7 @@ public class UserIdentityAuthenticator {
     UpdateUser update = UpdateUser.create(userDto.getLogin())
       .setEmail(identity.getEmail())
       .setName(identity.getName())
-      .setExternalIdentity(new ExternalIdentity(provider.getKey(), identity.getProviderLogin()))
-      .setPassword(null);
+      .setExternalIdentity(new ExternalIdentity(provider.getKey(), identity.getProviderLogin()));
     userUpdater.updateAndCommit(dbSession, update, u -> syncGroups(dbSession, identity, u));
   }
 
diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/UserUpdater.java b/server/sonar-server/src/main/java/org/sonar/server/user/UserUpdater.java
index b29394d3845..cc14249f719 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/user/UserUpdater.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/user/UserUpdater.java
@@ -112,15 +112,11 @@ public class UserUpdater {
     UpdateUser updateUser = UpdateUser.create(login)
       .setName(newUser.name())
       .setEmail(newUser.email())
-      .setScmAccounts(newUser.scmAccounts());
+      .setScmAccounts(newUser.scmAccounts())
+      .setExternalIdentity(newUser.externalIdentity());
     if (newUser.password() != null) {
       updateUser.setPassword(newUser.password());
     }
-    if (newUser.externalIdentity() != null) {
-      updateUser.setExternalIdentity(newUser.externalIdentity());
-    }
-    // Hack to allow to change the password of the user
-    existingUser.setLocal(true);
     setOnboarded(existingUser);
     updateDto(dbSession, updateUser, existingUser);
     updateUser(dbSession, existingUser);
@@ -213,8 +209,6 @@ public class UserUpdater {
     ExternalIdentity externalIdentity = updateUser.externalIdentity();
     if (updateUser.isExternalIdentityChanged() && !isSameExternalIdentity(userDto, externalIdentity)) {
       setExternalIdentity(userDto, externalIdentity);
-      userDto.setSalt(null);
-      userDto.setCryptedPassword(null);
       return true;
     }
     return false;
@@ -222,7 +216,7 @@ public class UserUpdater {
 
   private static boolean updatePassword(UpdateUser updateUser, UserDto userDto, List<String> messages) {
     String password = updateUser.password();
-    if (!updateUser.isExternalIdentityChanged() && updateUser.isPasswordChanged() && validatePasswords(password, messages) && checkPasswordChangeAllowed(userDto, messages)) {
+    if (updateUser.isPasswordChanged() && validatePasswords(password, messages) && checkPasswordChangeAllowed(userDto, messages)) {
       setEncryptedPassword(password, userDto);
       return true;
     }
@@ -248,10 +242,9 @@ public class UserUpdater {
   }
 
   private static boolean isSameExternalIdentity(UserDto dto, @Nullable ExternalIdentity externalIdentity) {
-    return (externalIdentity == null && dto.getExternalIdentity() == null) ||
-      (externalIdentity != null
-        && Objects.equals(dto.getExternalIdentity(), externalIdentity.getId())
-        && Objects.equals(dto.getExternalIdentityProvider(), externalIdentity.getProvider()));
+    return externalIdentity != null
+      && Objects.equals(dto.getExternalIdentity(), externalIdentity.getId())
+      && Objects.equals(dto.getExternalIdentityProvider(), externalIdentity.getProvider());
   }
 
   private static void setExternalIdentity(UserDto dto, @Nullable ExternalIdentity externalIdentity) {
@@ -263,6 +256,8 @@ public class UserUpdater {
       dto.setExternalIdentity(externalIdentity.getId());
       dto.setExternalIdentityProvider(externalIdentity.getProvider());
       dto.setLocal(false);
+      dto.setSalt(null);
+      dto.setCryptedPassword(null);
     }
   }
 
diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterCreateTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterCreateTest.java
index bd5118bc61b..8bedc870c2e 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterCreateTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterCreateTest.java
@@ -637,7 +637,7 @@ public class UserUpdaterCreateTest {
   }
 
   @Test
-  public void update_external_provider_when_reactivating_user() {
+  public void reactivate_user_with_external_provider() {
     db.users().insertUser(newDisabledUser(DEFAULT_LOGIN)
       .setLocal(true));
     createDefaultGroup();
@@ -651,9 +651,29 @@ public class UserUpdaterCreateTest {
     session.commit();
 
     UserDto dto = dbClient.userDao().selectByLogin(session, DEFAULT_LOGIN);
+    assertThat(dto.isLocal()).isFalse();
     assertThat(dto.getExternalIdentity()).isEqualTo("john");
     assertThat(dto.getExternalIdentityProvider()).isEqualTo("github");
-    assertThat(dto.isLocal()).isFalse();
+  }
+
+  @Test
+  public void reactivate_user_with_local_provider() {
+    db.users().insertUser(newDisabledUser(DEFAULT_LOGIN)
+      .setLocal(true));
+    createDefaultGroup();
+
+    underTest.createAndCommit(db.getSession(), NewUser.builder()
+      .setLogin(DEFAULT_LOGIN)
+      .setName("Marius2")
+      .setPassword("password")
+      .build(), u -> {
+    });
+    session.commit();
+
+    UserDto dto = dbClient.userDao().selectByLogin(session, DEFAULT_LOGIN);
+    assertThat(dto.isLocal()).isTrue();
+    assertThat(dto.getExternalIdentity()).isEqualTo(DEFAULT_LOGIN);
+    assertThat(dto.getExternalIdentityProvider()).isEqualTo("sonarqube");
   }
 
   @Test
diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java
index a6dab47c7c0..592fb3ad914 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java
@@ -124,7 +124,6 @@ public class UserUpdaterUpdateTest {
     underTest.updateAndCommit(session, UpdateUser.create(DEFAULT_LOGIN)
       .setName("Marius2")
       .setEmail("marius2@email.com")
-      .setPassword(null)
       .setExternalIdentity(new ExternalIdentity("github", "john")), u -> {
       });
 
@@ -142,7 +141,6 @@ public class UserUpdaterUpdateTest {
     underTest.updateAndCommit(session, UpdateUser.create(DEFAULT_LOGIN)
       .setName("Marius2")
       .setEmail("marius2@email.com")
-      .setPassword(null)
       .setExternalIdentity(new ExternalIdentity("github", "john")), u -> {
       });