SONAR-5830 Fix issue when same email is used by multiple users

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 13ba63edc1a301554ee94e0306c78f36eebf980c..0815c69955899a65ff583700798c30fd75dafde9 100644 (file)
--- 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
@@ -21,6 +21,7 @@
 package org.sonar.server.user;
 
 import com.google.common.base.CharMatcher;
+import com.google.common.base.Joiner;
 import com.google.common.base.Predicate;
 import com.google.common.base.Strings;
 import com.google.common.collect.Iterables;
@@ -247,9 +248,15 @@ public class UserUpdater implements ServerComponent {
       if (scmAccount.equals(login) || scmAccount.equals(email)) {
         messages.add(Message.of("user.login_or_email_used_as_scm_account"));
       } else {
-        UserDto matchingUser = dbClient.userDao().selectNullableByScmAccountOrLoginOrName(dbSession, scmAccount);
-        if (matchingUser != null && (existingUser == null || !matchingUser.getId().equals(existingUser.getId()))) {
-          messages.add(Message.of("user.scm_account_already_used", scmAccount, matchingUser.getName(), matchingUser.getLogin()));
+        List<UserDto> matchingUsers = dbClient.userDao().selectNullableByScmAccountOrLoginOrEmail(dbSession, scmAccount);
+        List<String> matchingUsersWithoutExistingUser = newArrayList();
+        for (UserDto matchingUser : matchingUsers) {
+          if (existingUser == null || !matchingUser.getId().equals(existingUser.getId())) {
+            matchingUsersWithoutExistingUser.add(matchingUser.getName() + " (" + matchingUser.getLogin() + ")");
+          }
+        }
+        if (!matchingUsersWithoutExistingUser.isEmpty()) {
+          messages.add(Message.of("user.scm_account_already_used", scmAccount, Joiner.on(", ").join(matchingUsersWithoutExistingUser)));
         }
       }
     }

diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/db/UserDao.java b/server/sonar-server/src/main/java/org/sonar/server/user/db/UserDao.java
index a43857558614b00487c8c22644cd373c3fbc00ea..ba3db9a9c7cb2e1cb49c63e42ccb7fde1c5903ef 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/user/db/UserDao.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/user/db/UserDao.java
@@ -30,6 +30,8 @@ import org.sonar.server.exceptions.NotFoundException;
 
 import javax.annotation.CheckForNull;
 
+import java.util.List;
+
 public class UserDao extends org.sonar.core.user.UserDao implements DaoComponent {
 
   public UserDao(MyBatis mybatis, System2 system2) {
@@ -49,9 +51,8 @@ public class UserDao extends org.sonar.core.user.UserDao implements DaoComponent
     return user;
   }
 
-  @CheckForNull
-  public UserDto selectNullableByScmAccountOrLoginOrName(DbSession session, String scmAccount) {
-    return mapper(session).selectNullableByScmAccountOrLoginOrName(scmAccount);
+  public List<UserDto> selectNullableByScmAccountOrLoginOrEmail(DbSession session, String scmAccountOrLoginOrEmail) {
+    return mapper(session).selectNullableByScmAccountOrLoginOrEmail(scmAccountOrLoginOrEmail);
   }
 
   protected UserMapper mapper(DbSession session) {

diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/index/UserIndex.java b/server/sonar-server/src/main/java/org/sonar/server/user/index/UserIndex.java
index 63ce49d84688afe2ac3faed5485b2506acabfdb5..aa34fcf9b59e98497aeab50ff68e5e5b355f3f44 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/user/index/UserIndex.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/user/index/UserIndex.java
@@ -59,7 +59,7 @@ public class UserIndex implements ServerComponent {
         .should(QueryBuilders.termQuery(UserIndexDefinition.FIELD_LOGIN, scmAccount))
         .should(QueryBuilders.termQuery(UserIndexDefinition.FIELD_EMAIL, scmAccount))
         .should(QueryBuilders.termQuery(UserIndexDefinition.FIELD_SCM_ACCOUNTS, scmAccount)))
-      .setSize(1);
+      .setSize(2);
     SearchHit[] result = request.get().getHits().getHits();
     if (result.length == 1) {
       return new UserDoc(result[0].sourceAsMap());

diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java
index 148e47da4cfabdeae29b31b8d9f5b1bb7343046b..8b9bd54d1ef963459f875d0053cb77eebb9a3af1 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java
@@ -364,14 +364,32 @@ public class UserUpdaterTest {
     try {
       userUpdater.create(NewUser.create()
         .setLogin("marius")
-        .setName("Marius2")
-        .setEmail("marius2@mail.com")
-        .setPassword("password2")
-        .setPasswordConfirmation("password2")
+        .setName("Marius")
+        .setEmail("marius@mail.com")
+        .setPassword("password")
+        .setPasswordConfirmation("password")
         .setScmAccounts(newArrayList("jo")));
       fail();
     } catch (BadRequestException e) {
-      assertThat(e.errors().messages()).containsOnly(Message.of("user.scm_account_already_used", "jo", "John", "john"));
+      assertThat(e.errors().messages()).containsOnly(Message.of("user.scm_account_already_used", "jo", "John (john)"));
+    }
+  }
+
+  @Test
+  public void fail_to_create_user_when_scm_account_is_already_used_by_many_user() throws Exception {
+    db.prepareDbUnit(getClass(), "fail_to_create_user_when_scm_account_is_already_used_by_many_user.xml");
+
+    try {
+      userUpdater.create(NewUser.create()
+        .setLogin("marius")
+        .setName("Marius")
+        .setEmail("marius@mail.com")
+        .setPassword("password")
+        .setPasswordConfirmation("password")
+        .setScmAccounts(newArrayList("john@email.com")));
+      fail();
+    } catch (BadRequestException e) {
+      assertThat(e.errors().messages()).containsOnly(Message.of("user.scm_account_already_used", "john@email.com", "John (john), Technical account (technical-account)"));
     }
   }
 
@@ -727,7 +745,7 @@ public class UserUpdaterTest {
         .setScmAccounts(newArrayList("jo")));
       fail();
     } catch (BadRequestException e) {
-      assertThat(e.errors().messages()).containsOnly(Message.of("user.scm_account_already_used", "jo", "John", "john"));
+      assertThat(e.errors().messages()).containsOnly(Message.of("user.scm_account_already_used", "jo", "John (john)"));
     }
   }
 

diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/db/UserDaoTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/db/UserDaoTest.java
index 4c7ffa0386f2f8bdb6211b486f502485b987e716..683847263667f98d53c948245cf768040eeaa286 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/user/db/UserDaoTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/db/UserDaoTest.java
@@ -30,6 +30,8 @@ import org.sonar.core.persistence.DbTester;
 import org.sonar.core.user.UserDto;
 import org.sonar.server.exceptions.NotFoundException;
 
+import java.util.List;
+
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
@@ -75,12 +77,35 @@ public class UserDaoTest {
   public void select_nullable_by_scm_account() {
     db.prepareDbUnit(getClass(), "select_nullable_by_scm_account.xml");
 
-    assertThat(dao.selectNullableByScmAccountOrLoginOrName(session, "ma").getLogin()).isEqualTo("marius");
-    assertThat(dao.selectNullableByScmAccountOrLoginOrName(session, "marius").getLogin()).isEqualTo("marius");
-    assertThat(dao.selectNullableByScmAccountOrLoginOrName(session, "marius@lesbronzes.fr").getLogin()).isEqualTo("marius");
+    List<UserDto> results = dao.selectNullableByScmAccountOrLoginOrEmail(session, "ma");
+    assertThat(results).hasSize(1);
+    assertThat(results.get(0).getLogin()).isEqualTo("marius");
+
+    results = dao.selectNullableByScmAccountOrLoginOrEmail(session, "marius");
+    assertThat(results).hasSize(1);
+    assertThat(results.get(0).getLogin()).isEqualTo("marius");
+
+    results = dao.selectNullableByScmAccountOrLoginOrEmail(session, "marius@lesbronzes.fr");
+    assertThat(results).hasSize(1);
+    assertThat(results.get(0).getLogin()).isEqualTo("marius");
+
+    results = dao.selectNullableByScmAccountOrLoginOrEmail(session, "marius@lesbronzes.fr");
+    assertThat(results).hasSize(1);
+    assertThat(results.get(0).getLogin()).isEqualTo("marius");
+
+    results = dao.selectNullableByScmAccountOrLoginOrEmail(session, "m");
+    assertThat(results).isEmpty();
+
+    results = dao.selectNullableByScmAccountOrLoginOrEmail(session, "unknown");
+    assertThat(results).isEmpty();
+  }
+
+  @Test
+  public void select_nullable_by_scm_account_return_many_results_when_same_email_is_used_by_many_users() {
+    db.prepareDbUnit(getClass(), "select_nullable_by_scm_account_return_many_results_when_same_email_is_used_by_many_users.xml");
 
-    assertThat(dao.selectNullableByScmAccountOrLoginOrName(session, "m")).isNull();
-    assertThat(dao.selectNullableByScmAccountOrLoginOrName(session, "unknown")).isNull();
+    List<UserDto> results = dao.selectNullableByScmAccountOrLoginOrEmail(session, "marius@lesbronzes.fr");
+    assertThat(results).hasSize(2);
   }
 
   @Test

diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/index/UserIndexTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/index/UserIndexTest.java
index f795df89b0a4341e17e8ae19ad2a8e23829c725e..61be62bdbfd23f02b551cdcb138ce46b07177cb3 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/user/index/UserIndexTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/index/UserIndexTest.java
@@ -103,4 +103,11 @@ public class UserIndexTest {
     assertThat(index.getNullableByScmAccount("unknown")).isNull();
   }
 
+  @Test
+  public void get_nullable_by_scm_account_return_null_when_two_users_have_same_email() throws Exception {
+    esTester.putDocuments(UserIndexDefinition.INDEX, UserIndexDefinition.TYPE_USER, this.getClass(), "user1.json", "user3-with-same-email-as-user1.json");
+
+    assertThat(index.getNullableByScmAccount("user1@mail.com")).isNull();
+  }
+
 }

diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/fail_to_create_user_when_scm_account_is_already_used_by_many_user.xml b/server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/fail_to_create_user_when_scm_account_is_already_used_by_many_user.xml
new file mode 100644 (file)
index 0000000..4bcf30b
--- /dev/null
+++ b/server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/fail_to_create_user_when_scm_account_is_already_used_by_many_user.xml
@@ -0,0 +1,10 @@
+<dataset>
+
+  <users id="101" login="john" name="John" email="john@email.com" active="[true]" scm_accounts=",jo," created_at="1418215735482" updated_at="1418215735485"
+         salt="79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8365" crypted_password="650d2261c98361e2f67f90ce5c65a95e7d8ea2fg"/>
+
+  <users id="102" login="technical-account" name="Technical account" email="john@email.com" active="[true]" scm_accounts="[null]" created_at="1418215735482"
+         updated_at="1418215735485"
+         salt="79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8365" crypted_password="650d2261c98361e2f67f90ce5c65a95e7d8ea2fg"/>
+
+</dataset>

diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/db/UserDaoTest/select_nullable_by_scm_account_return_many_results_when_same_email_is_used_by_many_users.xml b/server/sonar-server/src/test/resources/org/sonar/server/user/db/UserDaoTest/select_nullable_by_scm_account_return_many_results_when_same_email_is_used_by_many_users.xml
new file mode 100644 (file)
index 0000000..8ea78cf
--- /dev/null
+++ b/server/sonar-server/src/test/resources/org/sonar/server/user/db/UserDaoTest/select_nullable_by_scm_account_return_many_results_when_same_email_is_used_by_many_users.xml
@@ -0,0 +1,9 @@
+<dataset>
+
+  <users id="101" login="marius" name="Marius" email="marius@lesbronzes.fr" active="[true]" scm_accounts=",ma,marius33," created_at="1418215735482" updated_at="1418215735485"
+         salt="79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8365" crypted_password="650d2261c98361e2f67f90ce5c65a95e7d8ea2fg"/>
+  <users id="102" login="sbrandhof" name="Simon Brandhof" email="marius@lesbronzes.fr" active="[true]" scm_accounts="[null]" created_at="1418215735482"
+         updated_at="1418215735485"
+         salt="79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8366" crypted_password="650d2261c98361e2f67f90ce5c65a95e7d8ea2fh"/>
+
+</dataset>

diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/index/UserIndexTest/user3-with-same-email-as-user1.json b/server/sonar-server/src/test/resources/org/sonar/server/user/index/UserIndexTest/user3-with-same-email-as-user1.json
new file mode 100644 (file)
index 0000000..01bfa37
--- /dev/null
+++ b/server/sonar-server/src/test/resources/org/sonar/server/user/index/UserIndexTest/user3-with-same-email-as-user1.json
@@ -0,0 +1,10 @@
+{
+  "login": "user3",
+  "name": "User3",
+  "email": "user1@mail.com",
+  "active": true,
+  "scmAccounts": ["u3"],
+  "createdAt": 1500000000000,
+  "updatedAt": 1500000000000
+}
+

diff --git a/sonar-core/src/main/java/org/sonar/core/user/UserMapper.java b/sonar-core/src/main/java/org/sonar/core/user/UserMapper.java
index 48fda4f8dc81de978014fadab28f07476f19cf93..80c44052ea938fdad8f9c6923591ac97a3a2739f 100644 (file)
--- a/sonar-core/src/main/java/org/sonar/core/user/UserMapper.java
+++ b/sonar-core/src/main/java/org/sonar/core/user/UserMapper.java
@@ -31,8 +31,12 @@ public interface UserMapper {
   @CheckForNull
   UserDto selectByLogin(String login);
 
+  /**
+   * Search for a user by SCM account, login or email.
+   * Can return multiple results if an email is used by many users (For instance, technical account can use the same email as a none technical account)
+   */
   @CheckForNull
-  UserDto selectNullableByScmAccountOrLoginOrName(String scmAccount);
+  List<UserDto> selectNullableByScmAccountOrLoginOrEmail(String scmAccountOrLoginOrEmail);
 
   @CheckForNull
   UserDto selectUser(long userId);

diff --git a/sonar-core/src/main/resources/org/sonar/core/user/UserMapper.xml b/sonar-core/src/main/resources/org/sonar/core/user/UserMapper.xml
index a50be05abc98dfe4de2ae49db0d54ec70d9be5e3..16b370594d7ad8bf0e257eb5cf4e0bb15daefc61 100644 (file)
--- a/sonar-core/src/main/resources/org/sonar/core/user/UserMapper.xml
+++ b/sonar-core/src/main/resources/org/sonar/core/user/UserMapper.xml
@@ -23,7 +23,7 @@
     WHERE u.login=#{login}
   </select>
 
-  <select id="selectNullableByScmAccountOrLoginOrName" parameterType="String" resultType="User">
+  <select id="selectNullableByScmAccountOrLoginOrEmail" parameterType="String" resultType="User">
     SELECT
     <include refid="userColumns"/>
     FROM users u

diff --git a/sonar-core/src/main/resources/org/sonar/l10n/core.properties b/sonar-core/src/main/resources/org/sonar/l10n/core.properties
index 964a145b46ad8c4ce80956fbcea2f8f13ec03d09..ef44688c9c8b804a587640c143349c1e10e13928 100644 (file)
--- a/sonar-core/src/main/resources/org/sonar/l10n/core.properties
+++ b/sonar-core/src/main/resources/org/sonar/l10n/core.properties
@@ -1991,7 +1991,7 @@ user.bad_login=Use only letters, numbers, and .-_@ please.
 user.password_doesnt_match_confirmation=Password doesn't match confirmation.
 user.reactivated=The user '{0}' has been reactivated.
 user.add_scm_account=Add SCM account
-user.scm_account_already_used=The scm account '{0}' is already used by user '{1} ({2})'
+user.scm_account_already_used=The scm account '{0}' is already used by user(s) : '{1}'
 user.login_or_email_used_as_scm_account=Login and email are automatically considered as SCM accounts