]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-11302 Generate random login when identity login is null
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Mon, 24 Sep 2018 11:34:10 +0000 (13:34 +0200)
committersonartech <sonartech@sonarsource.com>
Wed, 10 Oct 2018 07:23:01 +0000 (09:23 +0200)
16 files changed:
server/sonar-server/src/main/java/org/sonar/server/authentication/UserIdentityAuthenticatorImpl.java
server/sonar-server/src/main/java/org/sonar/server/user/NewUser.java
server/sonar-server/src/main/java/org/sonar/server/user/UserUpdater.java
server/sonar-server/src/main/java/org/sonar/server/user/ws/CreateAction.java
server/sonar-server/src/test/java/org/sonar/server/authentication/UserIdentityAuthenticatorImplTest.java
server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterCreateTest.java
server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterReactivateTest.java
server/sonar-server/src/test/java/org/sonar/server/user/ws/CreateActionTest.java
server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java
server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/five_users.json [deleted file]
server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/page_one.json [deleted file]
server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/page_two.json [deleted file]
server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/user_one.json [deleted file]
server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/user_with_groups.json [deleted file]
sonar-plugin-api/src/main/java/org/sonar/api/server/authentication/UserIdentity.java
sonar-plugin-api/src/test/java/org/sonar/api/server/authentication/UserIdentityTest.java

index a5247b533b9e30b006c1aead80f3f2ae407838de..be21b9b02b08a13607002fc322b7eb8f9412547e 100644 (file)
@@ -98,39 +98,47 @@ public class UserIdentityAuthenticatorImpl implements UserIdentityAuthenticator
 
   @CheckForNull
   private UserDto getUser(DbSession dbSession, UserIdentity userIdentity, IdentityProvider provider) {
-    String externalId = userIdentity.getProviderId();
-    UserDto user = dbClient.userDao().selectByExternalIdAndIdentityProvider(dbSession, externalId == null ? userIdentity.getProviderLogin() : externalId, provider.getKey());
+    UserDto user = dbClient.userDao().selectByExternalIdAndIdentityProvider(dbSession, getProviderIdOrProviderLogin(userIdentity), provider.getKey());
+    if (user != null) {
+      return user;
+    }
     // We need to search by login because :
-    // 1. external id may have not been set before,
-    // 2. user may have been provisioned,
-    // 3. user may have been disabled.
-    return user != null ? user : dbClient.userDao().selectByLogin(dbSession, userIdentity.getLogin());
+    // 1. user may have been provisioned,
+    // 2. user may have been disabled.
+    String login = userIdentity.getLogin();
+    if (login == null) {
+      return null;
+    }
+    return dbClient.userDao().selectByLogin(dbSession, login);
+  }
+
+  private UserDto registerNewUser(DbSession dbSession, @Nullable UserDto disabledUser, UserIdentityAuthenticatorParameters authenticatorParameters) {
+    Optional<UserDto> otherUserToIndex = detectEmailUpdate(dbSession, authenticatorParameters);
+    NewUser newUser = createNewUser(authenticatorParameters);
+    if (disabledUser == null) {
+      return userUpdater.createAndCommit(dbSession, newUser, u -> syncGroups(dbSession, authenticatorParameters.getUserIdentity(), u), toArray(otherUserToIndex));
+    }
+    return userUpdater.reactivateAndCommit(dbSession, disabledUser, newUser, u -> syncGroups(dbSession, authenticatorParameters.getUserIdentity(), u), toArray(otherUserToIndex));
   }
 
   private UserDto registerExistingUser(DbSession dbSession, UserDto userDto, UserIdentityAuthenticatorParameters authenticatorParameters) {
     UpdateUser update = new UpdateUser()
-      .setLogin(authenticatorParameters.getUserIdentity().getLogin())
       .setEmail(authenticatorParameters.getUserIdentity().getEmail())
       .setName(authenticatorParameters.getUserIdentity().getName())
       .setExternalIdentity(new ExternalIdentity(
         authenticatorParameters.getProvider().getKey(),
         authenticatorParameters.getUserIdentity().getProviderLogin(),
         authenticatorParameters.getUserIdentity().getProviderId()));
+    String login = authenticatorParameters.getUserIdentity().getLogin();
+    if (login != null) {
+      update.setLogin(login);
+    }
     detectLoginUpdate(dbSession, userDto, update, authenticatorParameters);
     Optional<UserDto> otherUserToIndex = detectEmailUpdate(dbSession, authenticatorParameters);
     userUpdater.updateAndCommit(dbSession, userDto, update, u -> syncGroups(dbSession, authenticatorParameters.getUserIdentity(), u), toArray(otherUserToIndex));
     return userDto;
   }
 
-  private UserDto registerNewUser(DbSession dbSession, @Nullable UserDto disabledUser, UserIdentityAuthenticatorParameters authenticatorParameters) {
-    Optional<UserDto> otherUserToIndex = detectEmailUpdate(dbSession, authenticatorParameters);
-    NewUser newUser = createNewUser(authenticatorParameters);
-    if (disabledUser == null) {
-      return userUpdater.createAndCommit(dbSession, newUser, u -> syncGroups(dbSession, authenticatorParameters.getUserIdentity(), u), toArray(otherUserToIndex));
-    }
-    return userUpdater.reactivateAndCommit(dbSession, disabledUser, newUser, u -> syncGroups(dbSession, authenticatorParameters.getUserIdentity(), u), toArray(otherUserToIndex));
-  }
-
   private Optional<UserDto> detectEmailUpdate(DbSession dbSession, UserIdentityAuthenticatorParameters authenticatorParameters) {
     String email = authenticatorParameters.getUserIdentity().getEmail();
     if (email == null) {
@@ -147,7 +155,7 @@ public class UserIdentityAuthenticatorImpl implements UserIdentityAuthenticator
     UserDto existingUser = existingUsers.get(0);
     if (existingUser == null
       || Objects.equals(existingUser.getLogin(), authenticatorParameters.getUserIdentity().getLogin())
-      || (Objects.equals(existingUser.getExternalId(), authenticatorParameters.getUserIdentity().getProviderId())
+      || (Objects.equals(existingUser.getExternalId(), getProviderIdOrProviderLogin(authenticatorParameters.getUserIdentity()))
         && Objects.equals(existingUser.getExternalIdentityProvider(), authenticatorParameters.getProvider().getKey()))) {
       return Optional.empty();
     }
@@ -197,7 +205,7 @@ public class UserIdentityAuthenticatorImpl implements UserIdentityAuthenticator
     if (!userIdentity.shouldSyncGroups()) {
       return;
     }
-    String userLogin = userIdentity.getLogin();
+    String userLogin = userDto.getLogin();
     Set<String> userGroups = new HashSet<>(dbClient.groupMembershipDao().selectGroupsByLogins(dbSession, singletonList(userLogin)).get(userLogin));
     Set<String> identityGroups = userIdentity.getGroups();
     LOGGER.debug("List of groups returned by the identity provider '{}'", identityGroups);
@@ -245,7 +253,7 @@ public class UserIdentityAuthenticatorImpl implements UserIdentityAuthenticator
     if (!authenticatorParameters.getProvider().allowsUsersToSignUp()) {
       throw AuthenticationException.newBuilder()
         .setSource(authenticatorParameters.getSource())
-        .setLogin(authenticatorParameters.getUserIdentity().getLogin())
+        .setLogin(authenticatorParameters.getUserIdentity().getProviderLogin())
         .setMessage(format("User signup disabled for provider '%s'", identityProviderKey))
         .setPublicMessage(format("'%s' users are not allowed to sign up", identityProviderKey))
         .build();
@@ -269,7 +277,7 @@ public class UserIdentityAuthenticatorImpl implements UserIdentityAuthenticator
   private static AuthenticationException generateExistingEmailError(UserIdentityAuthenticatorParameters authenticatorParameters, String email) {
     return AuthenticationException.newBuilder()
       .setSource(authenticatorParameters.getSource())
-      .setLogin(authenticatorParameters.getUserIdentity().getLogin())
+      .setLogin(authenticatorParameters.getUserIdentity().getProviderLogin())
       .setMessage(format("Email '%s' is already used", email))
       .setPublicMessage(format(
         "You can't sign up because email '%s' is already used by an existing user. This means that you probably already registered with another account.",
@@ -277,4 +285,9 @@ public class UserIdentityAuthenticatorImpl implements UserIdentityAuthenticator
       .build();
   }
 
+  private static String getProviderIdOrProviderLogin(UserIdentity userIdentity) {
+    String providerId = userIdentity.getProviderId();
+    return providerId == null ? userIdentity.getProviderLogin() : providerId;
+  }
+
 }
index 7a8a976d97abf3a3e7f440e96597bf6a70e3e91b..4da0ba3b6648682309b8b1a4b7467ad00059a9e8 100644 (file)
@@ -44,6 +44,7 @@ public class NewUser {
     this.externalIdentity = builder.externalIdentity;
   }
 
+  @CheckForNull
   public String login() {
     return login;
   }
@@ -103,7 +104,7 @@ public class NewUser {
     private String password;
     private ExternalIdentity externalIdentity;
 
-    public Builder setLogin(String login) {
+    public Builder setLogin(@Nullable String login) {
       this.login = login;
       return this;
     }
index d968fdf9da5af4c7451572c77525e015de9fa260..9086a9c453be0fa911dafa970a2094a1ad018671 100644 (file)
@@ -29,6 +29,7 @@ import java.util.Objects;
 import java.util.function.Consumer;
 import java.util.stream.Stream;
 import javax.annotation.Nullable;
+import org.apache.commons.lang.math.RandomUtils;
 import org.sonar.api.config.Configuration;
 import org.sonar.api.platform.NewUserHandler;
 import org.sonar.api.server.ServerSide;
@@ -54,6 +55,7 @@ import static java.util.Arrays.stream;
 import static java.util.stream.Stream.concat;
 import static org.sonar.api.CoreProperties.DEFAULT_ISSUE_ASSIGNEE;
 import static org.sonar.core.config.CorePropertyDefinitions.ONBOARDING_TUTORIAL_SHOW_TO_NEW_USERS;
+import static org.sonar.core.util.Slug.slugify;
 import static org.sonar.core.util.stream.MoreCollectors.toList;
 import static org.sonar.server.ws.WsUtils.checkRequest;
 
@@ -67,7 +69,7 @@ public class UserUpdater {
   private static final String NAME_PARAM = "Name";
   private static final String EMAIL_PARAM = "Email";
 
-  private static final int LOGIN_MIN_LENGTH = 2;
+  public static final int LOGIN_MIN_LENGTH = 2;
   public static final int LOGIN_MAX_LENGTH = 255;
   public static final int EMAIL_MAX_LENGTH = 100;
   public static final int NAME_MAX_LENGTH = 200;
@@ -83,8 +85,8 @@ public class UserUpdater {
   private final LocalAuthentication localAuthentication;
 
   public UserUpdater(NewUserNotifier newUserNotifier, DbClient dbClient, UserIndexer userIndexer, OrganizationFlags organizationFlags,
-                     DefaultOrganizationProvider defaultOrganizationProvider, OrganizationUpdater organizationUpdater, DefaultGroupFinder defaultGroupFinder, Configuration config,
-                     LocalAuthentication localAuthentication) {
+    DefaultOrganizationProvider defaultOrganizationProvider, OrganizationUpdater organizationUpdater, DefaultGroupFinder defaultGroupFinder, Configuration config,
+    LocalAuthentication localAuthentication) {
     this.newUserNotifier = newUserNotifier;
     this.dbClient = dbClient;
     this.userIndexer = userIndexer;
@@ -109,13 +111,17 @@ public class UserUpdater {
 
   private void reactivateUser(DbSession dbSession, UserDto disabledUser, NewUser newUser) {
     UpdateUser updateUser = new UpdateUser()
-      .setLogin(newUser.login())
       .setName(newUser.name())
       .setEmail(newUser.email())
       .setScmAccounts(newUser.scmAccounts())
       .setExternalIdentity(newUser.externalIdentity());
-    if (newUser.password() != null) {
-      updateUser.setPassword(newUser.password());
+    String login = newUser.login();
+    if (login != null) {
+      updateUser.setLogin(login);
+    }
+    String password = newUser.password();
+    if (password != null) {
+      updateUser.setPassword(password);
     }
     setOnboarded(disabledUser);
     updateDto(dbSession, updateUser, disabledUser);
@@ -148,7 +154,9 @@ public class UserUpdater {
     List<String> messages = new ArrayList<>();
 
     String login = newUser.login();
-    if (validateLoginFormat(login, messages)) {
+    if (isNullOrEmpty(login)) {
+      userDto.setLogin(generateUniqueLogin(dbSession, newUser.name()));
+    } else if (validateLoginFormat(login, messages)) {
       checkLoginUniqueness(dbSession, login);
       userDto.setLogin(login);
     }
@@ -180,6 +188,18 @@ public class UserUpdater {
     return userDto;
   }
 
+  private String generateUniqueLogin(DbSession dbSession, String userName) {
+    String slugName = slugify(userName);
+    for (int i = 0; i < 10; i++) {
+      String login = slugName + RandomUtils.nextInt(100_000);
+      UserDto existingUser = dbClient.userDao().selectByLogin(dbSession, login);
+      if (existingUser == null) {
+        return login;
+      }
+    }
+    throw new IllegalStateException("Cannot create unique login for user name " + userName);
+  }
+
   private boolean updateDto(DbSession dbSession, UpdateUser update, UserDto dto) {
     List<String> messages = newArrayList();
     boolean changed = updateLogin(dbSession, update, dto, messages);
index adebf8d1a82ef926891841ae85571cbd29931887..97b13625df43235886354868d9889b662b9c93a2 100644 (file)
@@ -44,6 +44,7 @@ import static org.sonar.core.util.Protobuf.setNullable;
 import static org.sonar.server.user.ExternalIdentity.SQ_AUTHORITY;
 import static org.sonar.server.user.UserUpdater.EMAIL_MAX_LENGTH;
 import static org.sonar.server.user.UserUpdater.LOGIN_MAX_LENGTH;
+import static org.sonar.server.user.UserUpdater.LOGIN_MIN_LENGTH;
 import static org.sonar.server.user.UserUpdater.NAME_MAX_LENGTH;
 import static org.sonar.server.user.ws.EmailValidator.isValidIfPresent;
 import static org.sonar.server.ws.WsUtils.writeProtobuf;
@@ -85,6 +86,7 @@ public class CreateAction implements UsersWsAction {
 
     action.createParam(PARAM_LOGIN)
       .setRequired(true)
+      .setMinimumLength(LOGIN_MIN_LENGTH)
       .setMaximumLength(LOGIN_MAX_LENGTH)
       .setDescription("User login")
       .setExampleValue("myuser");
index c58afab14bc5a9e3a9b88e89a29117b9bd9d07b6..bee36526a4339613aea9a2fc47dff78ab353ec82 100644 (file)
@@ -133,6 +133,33 @@ public class UserIdentityAuthenticatorImplTest {
     checkGroupMembership(user);
   }
 
+  @Test
+  public void authenticate_new_user_generate_login_when_no_login_provided() {
+    organizationFlags.setEnabled(true);
+
+    underTest.authenticate(UserIdentityAuthenticatorParameters.builder()
+      .setUserIdentity(UserIdentity.builder()
+        .setProviderId("ABCD")
+        .setProviderLogin("johndoo")
+        .setName("John Doe")
+        .setEmail("john@email.com")
+        .build())
+      .setProvider(IDENTITY_PROVIDER)
+      .setSource(Source.realm(BASIC, IDENTITY_PROVIDER.getName()))
+      .setExistingEmailStrategy(ExistingEmailStrategy.FORBID)
+      .setUpdateLoginStrategy(UpdateLoginStrategy.ALLOW)
+      .build());
+
+    UserDto user = db.getDbClient().userDao().selectByEmail(db.getSession(), "john@email.com").get(0);
+    assertThat(user).isNotNull();
+    assertThat(user.isActive()).isTrue();
+    assertThat(user.getLogin()).isNotEqualTo("John Doe").startsWith("john-doe");
+    assertThat(user.getEmail()).isEqualTo("john@email.com");
+    assertThat(user.getExternalLogin()).isEqualTo("johndoo");
+    assertThat(user.getExternalIdentityProvider()).isEqualTo("github");
+    assertThat(user.getExternalId()).isEqualTo("ABCD");
+  }
+
   @Test
   public void authenticate_new_user_with_groups() {
     organizationFlags.setEnabled(true);
@@ -281,7 +308,7 @@ public class UserIdentityAuthenticatorImplTest {
     Source source = Source.realm(AuthenticationEvent.Method.FORM, IDENTITY_PROVIDER.getName());
 
     expectedException.expect(authenticationException().from(source)
-      .withLogin(USER_IDENTITY.getLogin())
+      .withLogin(USER_IDENTITY.getProviderLogin())
       .andPublicMessage("You can't sign up because email 'john@email.com' is already used by an existing user. " +
         "This means that you probably already registered with another account."));
     expectedException.expectMessage("Email 'john@email.com' is already used");
@@ -303,7 +330,7 @@ public class UserIdentityAuthenticatorImplTest {
     Source source = Source.realm(AuthenticationEvent.Method.FORM, IDENTITY_PROVIDER.getName());
 
     expectedException.expect(authenticationException().from(source)
-      .withLogin(USER_IDENTITY.getLogin())
+      .withLogin(USER_IDENTITY.getProviderLogin())
       .andPublicMessage("You can't sign up because email 'john@email.com' is already used by an existing user. " +
         "This means that you probably already registered with another account."));
     expectedException.expectMessage("Email 'john@email.com' is already used");
@@ -327,7 +354,7 @@ public class UserIdentityAuthenticatorImplTest {
       .setAllowsUsersToSignUp(false);
     Source source = Source.realm(AuthenticationEvent.Method.FORM, identityProvider.getName());
 
-    expectedException.expect(authenticationException().from(source).withLogin(USER_IDENTITY.getLogin()).andPublicMessage("'github' users are not allowed to sign up"));
+    expectedException.expect(authenticationException().from(source).withLogin(USER_IDENTITY.getProviderLogin()).andPublicMessage("'github' users are not allowed to sign up"));
     expectedException.expectMessage("User signup disabled for provider 'github'");
 
     underTest.authenticate(UserIdentityAuthenticatorParameters.builder()
@@ -340,7 +367,7 @@ public class UserIdentityAuthenticatorImplTest {
   }
 
   @Test
-  public void authenticate_existing_user_matching_login() {
+  public void authenticate_and_update_existing_user_matching_login() {
     db.users().insertUser(u -> u
       .setLogin(USER_LOGIN)
       .setName("Old name")
@@ -363,7 +390,7 @@ public class UserIdentityAuthenticatorImplTest {
   }
 
   @Test
-  public void authenticate_existing_user_matching_external_id() {
+  public void authenticate_and_update_existing_user_matching_external_id() {
     UserDto user = db.users().insertUser(u -> u
       .setLogin("Old login")
       .setName("Old name")
@@ -414,6 +441,32 @@ public class UserIdentityAuthenticatorImplTest {
         true);
   }
 
+  @Test
+  public void authenticate_existing_user_and_update_only_identity_provider_key() {
+    UserDto user = db.users().insertUser(u -> u
+      .setLogin(USER_LOGIN)
+      .setName(USER_IDENTITY.getName())
+      .setEmail(USER_IDENTITY.getEmail())
+      .setExternalId(USER_IDENTITY.getProviderId())
+      .setExternalLogin(USER_IDENTITY.getProviderLogin())
+      .setExternalIdentityProvider("old identity provider"));
+
+    underTest.authenticate(UserIdentityAuthenticatorParameters.builder()
+      .setUserIdentity(USER_IDENTITY)
+      .setProvider(IDENTITY_PROVIDER)
+      .setSource(Source.local(BASIC))
+      .setExistingEmailStrategy(ExistingEmailStrategy.FORBID)
+      .setUpdateLoginStrategy(UpdateLoginStrategy.ALLOW)
+      .build());
+
+    assertThat(db.getDbClient().userDao().selectByUuid(db.getSession(), user.getUuid()))
+      .extracting(UserDto::getLogin, UserDto::getName, UserDto::getEmail, UserDto::getExternalId, UserDto::getExternalLogin, UserDto::getExternalIdentityProvider,
+        UserDto::isActive)
+      .containsExactlyInAnyOrder(USER_LOGIN, USER_IDENTITY.getName(), USER_IDENTITY.getEmail(), USER_IDENTITY.getProviderId(), USER_IDENTITY.getProviderLogin(),
+        IDENTITY_PROVIDER.getKey(),
+        true);
+  }
+
   @Test
   public void authenticate_existing_user_matching_login_when_external_id_is_null() {
     UserDto user = db.users().insertUser(u -> u
@@ -444,6 +497,32 @@ public class UserIdentityAuthenticatorImplTest {
       .contains(user.getLogin(), "John", "john@email.com", "johndoo", "johndoo", "github", true);
   }
 
+  @Test
+  public void authenticate_existing_user_when_login_is_not_provided() {
+    UserDto user = db.users().insertUser(u -> u.setExternalIdentityProvider(IDENTITY_PROVIDER.getKey()));
+
+    underTest.authenticate(UserIdentityAuthenticatorParameters.builder()
+      .setUserIdentity(UserIdentity.builder()
+        .setProviderId(user.getExternalId())
+        .setProviderLogin(user.getExternalLogin())
+        // No login provided
+        .setName(user.getName())
+        .setEmail(user.getEmail())
+        .build())
+      .setProvider(IDENTITY_PROVIDER)
+      .setSource(Source.local(BASIC))
+      .setExistingEmailStrategy(ExistingEmailStrategy.FORBID)
+      .setUpdateLoginStrategy(UpdateLoginStrategy.ALLOW)
+      .build());
+
+    // No new user is created
+    assertThat(db.countRowsOfTable(db.getSession(), "users")).isEqualTo(1);
+    assertThat(db.getDbClient().userDao().selectByUuid(db.getSession(), user.getUuid()))
+      .extracting(UserDto::getLogin, UserDto::getName, UserDto::getEmail, UserDto::getExternalId, UserDto::getExternalLogin, UserDto::getExternalIdentityProvider,
+        UserDto::isActive)
+      .contains(user.getLogin(), user.getName(), user.getEmail(), user.getExternalId(), user.getExternalLogin(), user.getExternalIdentityProvider(), true);
+  }
+
   @Test
   public void authenticate_existing_user_with_login_update_and_strategy_is_ALLOW() {
     UserDto user = db.users().insertUser(u -> u
@@ -650,7 +729,7 @@ public class UserIdentityAuthenticatorImplTest {
       .build();
 
     expectedException.expect(authenticationException().from(Source.realm(AuthenticationEvent.Method.FORM, IDENTITY_PROVIDER.getName()))
-      .withLogin(userIdentity.getLogin())
+      .withLogin(userIdentity.getProviderLogin())
       .andPublicMessage("You can't sign up because email 'john@email.com' is already used by an existing user. " +
         "This means that you probably already registered with another account."));
     expectedException.expectMessage("Email 'john@email.com' is already used");
@@ -667,10 +746,12 @@ public class UserIdentityAuthenticatorImplTest {
   @Test
   public void does_not_fail_to_authenticate_user_when_email_has_not_changed_and_strategy_is_FORBID() {
     organizationFlags.setEnabled(true);
-    UserDto currentUser = db.users().insertUser(u -> u.setEmail("john@email.com"));
+    UserDto currentUser = db.users().insertUser(u -> u.setEmail("john@email.com")
+      .setExternalIdentityProvider(IDENTITY_PROVIDER.getKey()));
     UserIdentity userIdentity = UserIdentity.builder()
       .setLogin(currentUser.getLogin())
-      .setProviderLogin("johndoo")
+      .setProviderId(currentUser.getExternalId())
+      .setProviderLogin(currentUser.getExternalLogin())
       .setName("John")
       .setEmail("john@email.com")
       .build();
index f12c7485428c4ee079274c5ce77cd176de42b170..c231333770a510640ceb237d88a59b70b84a3fbe 100644 (file)
@@ -147,6 +147,35 @@ public class UserUpdaterCreateTest {
     assertThat(dto.isActive()).isTrue();
   }
 
+  @Test
+  public void create_user_generates_unique_login_no_login_provided() {
+    createDefaultGroup();
+
+    UserDto user = underTest.createAndCommit(db.getSession(), NewUser.builder()
+      .setName("John Doe")
+      .build(), u -> {
+      });
+
+    UserDto dto = dbClient.userDao().selectByLogin(session, user.getLogin());
+    assertThat(dto.getLogin()).startsWith("john-doe");
+    assertThat(dto.getName()).isEqualTo("John Doe");
+  }
+
+  @Test
+  public void create_user_generates_unique_login_when_login_is_emnpty() {
+    createDefaultGroup();
+
+    UserDto user = underTest.createAndCommit(db.getSession(), NewUser.builder()
+      .setLogin("")
+      .setName("John Doe")
+      .build(), u -> {
+      });
+
+    UserDto dto = dbClient.userDao().selectByLogin(session, user.getLogin());
+    assertThat(dto.getLogin()).startsWith("john-doe");
+    assertThat(dto.getName()).isEqualTo("John Doe");
+  }
+
   @Test
   public void create_user_with_sq_authority_when_no_authority_set() {
     createDefaultGroup();
@@ -191,7 +220,7 @@ public class UserUpdaterCreateTest {
     underTest.createAndCommit(db.getSession(), NewUser.builder()
       .setLogin("user")
       .setName("User")
-      .setExternalIdentity(new ExternalIdentity(SQ_AUTHORITY, "user", null))
+      .setExternalIdentity(new ExternalIdentity(SQ_AUTHORITY, "user", "user"))
       .build(), u -> {
       });
 
@@ -293,20 +322,6 @@ public class UserUpdaterCreateTest {
     assertThat(es.getIds(UserIndexDefinition.INDEX_TYPE_USER)).containsExactlyInAnyOrder(created.getUuid(), otherUser.getUuid());
   }
 
-  @Test
-  public void fail_to_create_user_with_missing_login() {
-    expectedException.expect(BadRequestException.class);
-    expectedException.expectMessage("Login can't be empty");
-
-    underTest.createAndCommit(db.getSession(), NewUser.builder()
-      .setLogin(null)
-      .setName("Marius")
-      .setEmail("marius@mail.com")
-      .setPassword("password")
-      .build(), u -> {
-      });
-  }
-
   @Test
   public void fail_to_create_user_with_invalid_login() {
     expectedException.expect(BadRequestException.class);
@@ -417,7 +432,7 @@ public class UserUpdaterCreateTest {
         });
       fail();
     } catch (BadRequestException e) {
-      assertThat(e.errors()).hasSize(3);
+      assertThat(e.errors()).containsExactlyInAnyOrder("Name can't be empty", "Password can't be empty");
     }
   }
 
@@ -515,7 +530,7 @@ public class UserUpdaterCreateTest {
       .setName("User")
       .setExternalIdentity(new ExternalIdentity(existingUser.getExternalIdentityProvider(), existingUser.getExternalLogin(), existingUser.getExternalId()))
       .build(), u -> {
-    });
+      });
   }
 
   @Test
index 265346936719de9e3f59bc3ddd2e6e44228d6593..d677e4fbd7d44d756d15a6a6edb39dc4f9b715c6 100644 (file)
@@ -101,6 +101,24 @@ public class UserUpdaterReactivateTest {
     assertThat(reloaded.getUpdatedAt()).isGreaterThan(user.getCreatedAt());
   }
 
+  @Test
+  public void reactivate_user_without_providing_login() {
+    UserDto user = db.users().insertUser(u -> u.setActive(false));
+    createDefaultGroup();
+
+    underTest.reactivateAndCommit(db.getSession(), user, NewUser.builder()
+        .setName("Marius2")
+        .setEmail("marius2@mail.com")
+        .setPassword("password2")
+        .build(),
+      u -> {
+      });
+
+    UserDto reloaded = dbClient.userDao().selectByUuid(session, user.getUuid());
+    assertThat(reloaded.isActive()).isTrue();
+    assertThat(reloaded.getLogin()).isEqualTo(user.getLogin());
+  }
+
   @Test
   public void reactivate_user_not_having_password() {
     UserDto user = db.users().insertDisabledUser(u -> u.setSalt(null).setCryptedPassword(null));
index 6ca46d0fa5db3b105fa664584f0b3e3d68b3bebb..8812dde814e4ae35b780f37e73506210add9ce33 100644 (file)
@@ -44,7 +44,6 @@ import org.sonar.server.organization.TestOrganizationFlags;
 import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.user.NewUserNotifier;
 import org.sonar.server.user.UserUpdater;
-import org.sonar.server.user.index.UserIndex;
 import org.sonar.server.user.index.UserIndexDefinition;
 import org.sonar.server.user.index.UserIndexer;
 import org.sonar.server.user.ws.CreateAction.CreateRequest;
@@ -84,7 +83,6 @@ public class CreateActionTest {
   @Rule
   public ExpectedException expectedException = ExpectedException.none();
 
-  private UserIndex index = new UserIndex(es.client(), System2.INSTANCE);
   private UserIndexer userIndexer = new UserIndexer(db.getDbClient(), es.client());
   private GroupDto defaultGroupInDefaultOrg;
   private DefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(db);
@@ -302,6 +300,19 @@ public class CreateActionTest {
       .build());
   }
 
+  @Test
+  public void fail_when_login_is_too_short() {
+    logInAsSystemAdministrator();
+
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("'login' length (1) is shorter than the minimum authorized (2)");
+    call(CreateRequest.builder()
+      .setLogin("a")
+      .setName("John")
+      .setPassword("1234")
+      .build());
+  }
+
   @Test
   public void fail_when_missing_name() {
     logInAsSystemAdministrator();
@@ -358,7 +369,7 @@ public class CreateActionTest {
   }
 
   @Test
-  public void throw_ForbiddenException_if_not_system_administrator() throws Exception {
+  public void throw_ForbiddenException_if_not_system_administrator() {
     userSessionRule.logIn().setNonSystemAdministrator();
 
     expectedException.expect(ForbiddenException.class);
index 2e6f056e861455338e9b4d0dff879eb2aac1de50..77e6c2bbc8f2f67604ceab2f9ee89a7f43b1e362 100644 (file)
  */
 package org.sonar.server.user.ws;
 
-import java.util.ArrayList;
-import java.util.List;
-import java.util.function.Consumer;
 import java.util.stream.IntStream;
 import org.junit.Rule;
 import org.junit.Test;
 import org.sonar.api.server.ws.WebService.Param;
 import org.sonar.api.utils.System2;
-import org.sonar.db.DbClient;
-import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
 import org.sonar.db.user.GroupDto;
 import org.sonar.db.user.UserDto;
-import org.sonar.db.user.UserGroupDto;
 import org.sonar.server.es.EsTester;
 import org.sonar.server.issue.ws.AvatarResolverImpl;
 import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.user.index.UserIndex;
 import org.sonar.server.user.index.UserIndexer;
-import org.sonar.server.ws.WsTester;
+import org.sonar.server.ws.WsActionTester;
+import org.sonarqube.ws.Common.Paging;
+import org.sonarqube.ws.Users.SearchWsResponse;
+import org.sonarqube.ws.Users.SearchWsResponse.User;
 
-import static com.google.common.collect.Lists.newArrayList;
+import static java.util.Arrays.asList;
 import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.sonar.db.user.GroupTesting.newGroupDto;
+import static org.assertj.core.api.Assertions.tuple;
 import static org.sonar.test.JsonAssert.assertJson;
 
 public class SearchActionTest {
 
-  private System2 system2 = System2.INSTANCE;
-
   @Rule
   public EsTester es = EsTester.create();
 
@@ -58,71 +53,32 @@ public class SearchActionTest {
   public UserSessionRule userSession = UserSessionRule.standalone();
 
   @Rule
-  public DbTester db = DbTester.create(system2);
+  public DbTester db = DbTester.create();
 
-  private DbClient dbClient = db.getDbClient();
-  private DbSession dbSession = db.getSession();
-  private UserIndex index = new UserIndex(es.client(), system2);
-  private UserIndexer userIndexer = new UserIndexer(dbClient, es.client());
-  private WsTester ws = new WsTester(new UsersWs(new SearchAction(userSession, index, dbClient, new AvatarResolverImpl())));
+  private UserIndex index = new UserIndex(es.client(), System2.INSTANCE);
+  private UserIndexer userIndexer = new UserIndexer(db.getDbClient(), es.client());
+  private WsActionTester ws = new WsActionTester(new SearchAction(userSession, index, db.getDbClient(), new AvatarResolverImpl()));
 
   @Test
-  public void test_json_example() throws Exception {
-    UserDto fmallet = db.users().insertUser(u -> u.setLogin("fmallet").setName("Freddy Mallet").setEmail("f@m.com")
-      .setActive(true)
-      .setLocal(true)
-      .setScmAccounts(emptyList())
-      .setExternalLogin("fmallet")
-      .setExternalIdentityProvider("sonarqube"));
-    UserDto simon = db.users().insertUser(u -> u.setLogin("sbrandhof").setName("Simon").setEmail("s.brandhof@company.tld")
-      .setActive(true)
-      .setLocal(false)
-      .setExternalLogin("sbrandhof@ldap.com")
-      .setExternalIdentityProvider("LDAP")
-      .setScmAccounts(newArrayList("simon.brandhof", "s.brandhof@company.tld")));
-    GroupDto sonarUsers = db.users().insertGroup(newGroupDto().setName("sonar-users"));
-    GroupDto sonarAdministrators = db.users().insertGroup(newGroupDto().setName("sonar-administrators"));
-    dbClient.userGroupDao().insert(dbSession, new UserGroupDto().setUserId(simon.getId()).setGroupId(sonarUsers.getId()));
-    dbClient.userGroupDao().insert(dbSession, new UserGroupDto().setUserId(fmallet.getId()).setGroupId(sonarUsers.getId()));
-    dbClient.userGroupDao().insert(dbSession, new UserGroupDto().setUserId(fmallet.getId()).setGroupId(sonarAdministrators.getId()));
-    db.commit();
-    for (int i = 0; i < 3; i++) {
-      db.users().insertToken(simon);
-    }
-    db.users().insertToken(fmallet);
+  public void search_for_all_users() {
+    UserDto user1 = db.users().insertUser();
+    UserDto user2 = db.users().insertUser();
     userIndexer.indexOnStartup(null);
-    loginAsSystemAdministrator();
-
-    String response = ws.newGetRequest("api/users", "search").execute().outputAsString();
-
-    assertJson(response).isSimilarTo(getClass().getResource("search-example.json"));
-  }
-
-  @Test
-  public void search_empty() throws Exception {
-    loginAsSimpleUser();
-    ws.newGetRequest("api/users", "search").execute().assertJson("{\n" +
-      "  \"paging\": {\n" +
-      "    \"pageIndex\": 1,\n" +
-      "    \"pageSize\": 50,\n" +
-      "    \"total\": 0\n" +
-      "  },\n" +
-      "  \"users\": []\n" +
-      "}");
-  }
+    userSession.logIn();
 
-  @Test
-  public void search_without_parameters() throws Exception {
-    loginAsSimpleUser();
-    injectUsers(5);
+    SearchWsResponse response = ws.newRequest()
+      .executeProtobuf(SearchWsResponse.class);
 
-    ws.newGetRequest("api/users", "search").execute().assertJson(getClass(), "five_users.json");
+    assertThat(response.getUsersList())
+      .extracting(User::getLogin, User::getName)
+      .containsExactlyInAnyOrder(
+        tuple(user1.getLogin(), user1.getName()),
+        tuple(user2.getLogin(), user2.getName()));
   }
 
   @Test
-  public void search_with_query() throws Exception {
-    loginAsSimpleUser();
-    injectUsers(5);
+  public void search_with_query() {
+    userSession.logIn();
     UserDto user = db.users().insertUser(u -> u
       .setLogin("user-%_%-login")
       .setName("user-name")
@@ -131,190 +87,238 @@ public class SearchActionTest {
       .setScmAccounts(singletonList("user1")));
     userIndexer.indexOnStartup(null);
 
-    ws.newGetRequest("api/users", "search").setParam("q", "user-%_%-").execute().assertJson(getClass(), "user_one.json");
-    ws.newGetRequest("api/users", "search").setParam("q", "user@MAIL.com").execute().assertJson(getClass(), "user_one.json");
-    ws.newGetRequest("api/users", "search").setParam("q", "user-name").execute().assertJson(getClass(), "user_one.json");
+    assertThat(ws.newRequest()
+      .setParam("q", "user-%_%-")
+      .executeProtobuf(SearchWsResponse.class).getUsersList())
+        .extracting(User::getLogin)
+        .containsExactlyInAnyOrder(user.getLogin());
+    assertThat(ws.newRequest()
+      .setParam("q", "user@MAIL.com")
+      .executeProtobuf(SearchWsResponse.class).getUsersList())
+        .extracting(User::getLogin)
+        .containsExactlyInAnyOrder(user.getLogin());
+    assertThat(ws.newRequest()
+      .setParam("q", "user-name")
+      .executeProtobuf(SearchWsResponse.class).getUsersList())
+        .extracting(User::getLogin)
+        .containsExactlyInAnyOrder(user.getLogin());
   }
 
   @Test
-  public void search_with_paging() throws Exception {
-    loginAsSimpleUser();
-    injectUsers(10);
+  public void return_avatar() {
+    UserDto user = db.users().insertUser(u -> u.setEmail("john@doe.com"));
+    userIndexer.indexOnStartup(null);
+    userSession.logIn();
+
+    SearchWsResponse response = ws.newRequest()
+      .executeProtobuf(SearchWsResponse.class);
 
-    ws.newGetRequest("api/users", "search").setParam(Param.PAGE_SIZE, "5").execute().assertJson(getClass(), "page_one.json");
-    ws.newGetRequest("api/users", "search").setParam(Param.PAGE_SIZE, "5").setParam(Param.PAGE, "2").execute().assertJson(getClass(), "page_two.json");
+    assertThat(response.getUsersList())
+      .extracting(User::getLogin, User::getAvatar)
+      .containsExactlyInAnyOrder(tuple(user.getLogin(), "6a6c19fea4a3676970167ce51f39e6ee"));
   }
 
   @Test
-  public void search_with_fields() throws Exception {
-    loginAsSimpleUser();
-    injectUsers(1);
-
-    assertThat(ws.newGetRequest("api/users", "search").execute().outputAsString())
-      .contains("login")
-      .contains("name")
-      .contains("avatar")
-      .contains("scmAccounts")
-      .doesNotContain("groups");
-
-    assertThat(ws.newGetRequest("api/users", "search").setParam(Param.FIELDS, "").execute().outputAsString())
-      .contains("login")
-      .contains("name")
-      .contains("avatar")
-      .contains("scmAccounts")
-      .doesNotContain("groups");
-
-    assertThat(ws.newGetRequest("api/users", "search").setParam(Param.FIELDS, "scmAccounts").execute().outputAsString())
-      .contains("login")
-      .doesNotContain("name")
-      .doesNotContain("avatar")
-      .contains("scmAccounts")
-      .doesNotContain("groups");
-
-    assertThat(ws.newGetRequest("api/users", "search").setParam(Param.FIELDS, "groups").execute().outputAsString())
-      .contains("login")
-      .doesNotContain("name")
-      .doesNotContain("avatar")
-      .doesNotContain("scmAccounts")
-      .doesNotContain("groups");
-
-    loginAsSystemAdministrator();
-
-    assertThat(ws.newGetRequest("api/users", "search").execute().outputAsString())
-      .contains("login")
-      .contains("name")
-      .contains("email")
-      .contains("avatar")
-      .contains("scmAccounts")
-      .contains("groups");
-
-    assertThat(ws.newGetRequest("api/users", "search").setParam(Param.FIELDS, "groups").execute().outputAsString())
-      .contains("login")
-      .doesNotContain("name")
-      .doesNotContain("email")
-      .doesNotContain("avatar")
-      .doesNotContain("scmAccounts")
-      .contains("groups");
+  public void return_scm_accounts() {
+    UserDto user = db.users().insertUser(u -> u.setScmAccounts(asList("john1", "john2")));
+    userIndexer.indexOnStartup(null);
+    userSession.logIn();
+
+    SearchWsResponse response = ws.newRequest()
+      .executeProtobuf(SearchWsResponse.class);
+
+    assertThat(response.getUsersList())
+      .extracting(User::getLogin, u -> u.getScmAccounts().getScmAccountsList())
+      .containsExactlyInAnyOrder(tuple(user.getLogin(), asList("john1", "john2")));
   }
 
   @Test
-  public void search_with_groups() throws Exception {
-    loginAsSystemAdministrator();
-    injectUsers(1);
+  public void return_tokens_count() {
+    UserDto user = db.users().insertUser();
+    db.users().insertToken(user);
+    db.users().insertToken(user);
+    userIndexer.indexOnStartup(null);
+    userSession.logIn();
+
+    SearchWsResponse response = ws.newRequest()
+      .executeProtobuf(SearchWsResponse.class);
 
-    ws.newGetRequest("api/users", "search").execute().assertJson(getClass(), "user_with_groups.json");
+    assertThat(response.getUsersList())
+      .extracting(User::getLogin, User::getTokensCount)
+      .containsExactlyInAnyOrder(tuple(user.getLogin(), 2));
   }
 
   @Test
-  public void does_not_return_email_when_not_when_system_administer() throws Exception {
-    loginAsSimpleUser();
-    insertUser(user -> user.setLogin("john").setName("John").setEmail("john@email.com"));
-
-    ws.newGetRequest("api/users", "search").execute().assertJson(
-      "{" +
-        "  \"users\": [" +
-        "    {" +
-        "      \"login\": \"john\"," +
-        "      \"name\": \"John\"," +
-        "      \"avatar\": \"41193cdbffbf06be0cdf231b28c54b18\"" +
-        "    }" +
-        "  ]" +
-        "}");
+  public void return_email_only_when_system_administer() {
+    UserDto user = db.users().insertUser();
+    userIndexer.indexOnStartup(null);
+
+    userSession.logIn().setSystemAdministrator();
+    assertThat(ws.newRequest()
+      .executeProtobuf(SearchWsResponse.class).getUsersList())
+        .extracting(User::getLogin, User::getEmail)
+        .containsExactlyInAnyOrder(tuple(user.getLogin(), user.getEmail()));
+
+    userSession.logIn();
+    assertThat(ws.newRequest()
+      .executeProtobuf(SearchWsResponse.class).getUsersList())
+        .extracting(User::getLogin, User::hasEmail)
+        .containsExactlyInAnyOrder(tuple(user.getLogin(), false));
   }
 
   @Test
-  public void return_email_and_avatar_when_system_administer() throws Exception {
-    loginAsSystemAdministrator();
-    insertUser(user -> user.setLogin("john").setName("John").setEmail("john@email.com"));
-
-    ws.newGetRequest("api/users", "search").execute().assertJson(
-      "{" +
-        "  \"users\": [" +
-        "    {" +
-        "      \"login\": \"john\"," +
-        "      \"name\": \"John\"," +
-        "      \"email\": \"john@email.com\"," +
-        "      \"avatar\": \"41193cdbffbf06be0cdf231b28c54b18\"" +
-        "    }" +
-        "  ]" +
-        "}");
+  public void return_user_not_having_email() {
+    UserDto user = db.users().insertUser(u -> u.setEmail(null));
+    userIndexer.indexOnStartup(null);
+    userSession.logIn().setSystemAdministrator();
+
+    SearchWsResponse response = ws.newRequest()
+      .executeProtobuf(SearchWsResponse.class);
+
+    assertThat(response.getUsersList())
+      .extracting(User::getLogin, User::hasEmail)
+      .containsExactlyInAnyOrder(tuple(user.getLogin(), false));
   }
 
   @Test
-  public void does_not_fail_when_user_has_no_email() throws Exception {
-    loginAsSystemAdministrator();
-    insertUser(user -> user.setLogin("john").setName("John").setEmail(null));
-
-    ws.newGetRequest("api/users", "search").execute().assertJson(
-      "{" +
-        "  \"users\": [" +
-        "    {" +
-        "      \"login\": \"john\"," +
-        "      \"name\": \"John\"" +
-        "    }" +
-        "  ]" +
-        "}");
+  public void return_groups_only_when_system_administer() {
+    UserDto user = db.users().insertUser();
+    GroupDto group1 = db.users().insertGroup(db.getDefaultOrganization(), "group1");
+    GroupDto group2 = db.users().insertGroup(db.getDefaultOrganization(), "group2");
+    GroupDto group3 = db.users().insertGroup(db.getDefaultOrganization(), "group3");
+    db.users().insertMember(group1, user);
+    db.users().insertMember(group2, user);
+    userIndexer.indexOnStartup(null);
+
+    userSession.logIn().setSystemAdministrator();
+    assertThat(ws.newRequest()
+      .executeProtobuf(SearchWsResponse.class).getUsersList())
+        .extracting(User::getLogin, u -> u.getGroups().getGroupsList())
+        .containsExactlyInAnyOrder(tuple(user.getLogin(), asList(group1.getName(), group2.getName())));
+
+    userSession.logIn();
+    assertThat(ws.newRequest()
+      .executeProtobuf(SearchWsResponse.class).getUsersList())
+        .extracting(User::getLogin, User::hasGroups)
+        .containsExactlyInAnyOrder(tuple(user.getLogin(), false));
   }
 
   @Test
-  public void only_return_login_and_name_when_not_logged() throws Exception {
+  public void only_return_login_and_name_when_not_logged() {
+    UserDto user = db.users().insertUser();
+    db.users().insertToken(user);
+    GroupDto group = db.users().insertGroup(db.getDefaultOrganization());
+    db.users().insertMember(group, user);
+    userIndexer.indexOnStartup(null);
     userSession.anonymous();
 
-    db.users().insertUser(u -> u.setLogin("john").setName("John").setEmail("john@email.com"));
-    dbSession.commit();
-    userIndexer.indexOnStartup(null);
+    SearchWsResponse response = ws.newRequest()
+      .executeProtobuf(SearchWsResponse.class);
 
-    ws.newGetRequest("api/users", "search").execute().assertJson(
-      "{" +
-        "  \"users\": [" +
-        "    {" +
-        "      \"login\": \"john\"," +
-        "      \"name\": \"John\"" +
-        "    }" +
-        "  ]" +
-        "}");
+    assertThat(response.getUsersList())
+      .extracting(User::getLogin, User::getName, User::hasTokensCount, User::hasScmAccounts, User::hasAvatar, User::hasGroups)
+      .containsExactlyInAnyOrder(tuple(user.getLogin(), user.getName(), false, false, false, false));
   }
 
-  private List<UserDto> injectUsers(int numberOfUsers) {
-    List<UserDto> userDtos = new ArrayList<>();
-    GroupDto group1 = db.users().insertGroup(newGroupDto().setName("sonar-users"));
-    GroupDto group2 = db.users().insertGroup(newGroupDto().setName("sonar-admins"));
-    for (int index = 0; index < numberOfUsers; index++) {
-      String email = String.format("user-%d@mail.com", index);
-      String login = String.format("user-%d", index);
-      String name = String.format("User %d", index);
-      List<String> scmAccounts = singletonList(String.format("user-%d", index));
-
-      UserDto userDto = db.users().insertUser(u -> u.setActive(true)
-        .setEmail(email)
-        .setLogin(login)
-        .setName(name)
-        .setScmAccounts(scmAccounts)
-        .setLocal(true)
-        .setExternalLogin(login)
-        .setExternalIdentityProvider("sonarqube"));
-      userDtos.add(userDto);
-
-      IntStream.range(0, index).forEach(i -> db.users().insertToken(userDto, t -> t.setName(String.format("%s-%d", login, i))));
-      db.users().insertMember(group1, userDto);
-      db.users().insertMember(group2, userDto);
-    }
+  @Test
+  public void search_with_fields() {
+    UserDto user = db.users().insertUser();
+    GroupDto group = db.users().insertGroup(db.getDefaultOrganization());
+    db.users().insertMember(group, user);
     userIndexer.indexOnStartup(null);
-    return userDtos;
+    userSession.logIn().setSystemAdministrator();
+
+    assertThat(ws.newRequest()
+      .setParam(Param.FIELDS, "scmAccounts")
+      .executeProtobuf(SearchWsResponse.class)
+      .getUsersList())
+        .extracting(User::getLogin, User::hasName, User::hasScmAccounts, User::hasAvatar, User::hasGroups)
+        .containsExactlyInAnyOrder(tuple(user.getLogin(), false, true, false, false));
+    assertThat(ws.newRequest()
+      .setParam(Param.FIELDS, "groups")
+      .executeProtobuf(SearchWsResponse.class)
+      .getUsersList())
+        .extracting(User::getLogin, User::hasName, User::hasScmAccounts, User::hasAvatar, User::hasGroups)
+        .containsExactlyInAnyOrder(tuple(user.getLogin(), false, false, false, true));
+    assertThat(ws.newRequest()
+      .setParam(Param.FIELDS, "")
+      .executeProtobuf(SearchWsResponse.class)
+      .getUsersList())
+        .extracting(User::getLogin, User::hasName, User::hasScmAccounts, User::hasAvatar, User::hasGroups)
+        .containsExactlyInAnyOrder(tuple(user.getLogin(), true, true, true, true));
+    assertThat(ws.newRequest()
+      .executeProtobuf(SearchWsResponse.class)
+      .getUsersList())
+        .extracting(User::getLogin, User::hasName, User::hasScmAccounts, User::hasAvatar, User::hasGroups)
+        .containsExactlyInAnyOrder(tuple(user.getLogin(), true, true, true, true));
   }
 
-  private UserDto insertUser(Consumer<UserDto> populateUserDto) {
-    UserDto user = db.users().insertUser(populateUserDto);
+  @Test
+  public void search_with_paging() {
+    userSession.logIn();
+    IntStream.rangeClosed(0, 9).forEach(i -> db.users().insertUser(u -> u.setLogin("user-" + i).setName("User " + i)));
     userIndexer.indexOnStartup(null);
-    return user;
-  }
 
-  private void loginAsSystemAdministrator() {
-    userSession.logIn().setSystemAdministrator();
+    SearchWsResponse response = ws.newRequest()
+      .setParam(Param.PAGE_SIZE, "5")
+      .executeProtobuf(SearchWsResponse.class);
+    assertThat(response.getUsersList())
+      .extracting(User::getLogin)
+      .containsExactly("user-0", "user-1", "user-2", "user-3", "user-4");
+    assertThat(response.getPaging())
+      .extracting(Paging::getPageIndex, Paging::getPageSize, Paging::getTotal)
+      .containsExactly(1, 5, 10);
+
+    response = ws.newRequest()
+      .setParam(Param.PAGE_SIZE, "5")
+      .setParam(Param.PAGE, "2")
+      .executeProtobuf(SearchWsResponse.class);
+    assertThat(response.getUsersList())
+      .extracting(User::getLogin)
+      .containsExactly("user-5", "user-6", "user-7", "user-8", "user-9");
+    assertThat(response.getPaging())
+      .extracting(Paging::getPageIndex, Paging::getPageSize, Paging::getTotal)
+      .containsExactly(2, 5, 10);
   }
 
-  private void loginAsSimpleUser() {
+  @Test
+  public void return_empty_result_when_no_user() {
     userSession.logIn();
+
+    SearchWsResponse response = ws.newRequest()
+      .executeProtobuf(SearchWsResponse.class);
+
+    assertThat(response.getUsersList()).isEmpty();
+    assertThat(response.getPaging().getTotal()).isZero();
+  }
+
+  @Test
+  public void test_json_example() {
+    UserDto fmallet = db.users().insertUser(u -> u.setLogin("fmallet").setName("Freddy Mallet").setEmail("f@m.com")
+      .setLocal(true)
+      .setScmAccounts(emptyList())
+      .setExternalLogin("fmallet")
+      .setExternalIdentityProvider("sonarqube"));
+    UserDto simon = db.users().insertUser(u -> u.setLogin("sbrandhof").setName("Simon").setEmail("s.brandhof@company.tld")
+      .setLocal(false)
+      .setExternalLogin("sbrandhof@ldap.com")
+      .setExternalIdentityProvider("LDAP")
+      .setScmAccounts(asList("simon.brandhof", "s.brandhof@company.tld")));
+    GroupDto sonarUsers = db.users().insertGroup(db.getDefaultOrganization(), "sonar-users");
+    GroupDto sonarAdministrators = db.users().insertGroup(db.getDefaultOrganization(), "sonar-administrators");
+    db.users().insertMember(sonarUsers, simon);
+    db.users().insertMember(sonarUsers, fmallet);
+    db.users().insertMember(sonarAdministrators, fmallet);
+    db.users().insertToken(simon);
+    db.users().insertToken(simon);
+    db.users().insertToken(simon);
+    db.users().insertToken(fmallet);
+    userIndexer.indexOnStartup(null);
+    userSession.logIn().setSystemAdministrator();
+
+    String response = ws.newRequest().execute().getInput();
+
+    assertJson(response).isSimilarTo(getClass().getResource("search-example.json"));
   }
 
 }
diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/five_users.json b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/five_users.json
deleted file mode 100644 (file)
index ff3f776..0000000
+++ /dev/null
@@ -1,59 +0,0 @@
-{
-  "paging": {
-    "pageIndex": 1,
-    "pageSize": 50,
-    "total": 5
-  },
-  "users": [
-    {
-      "login": "user-0",
-      "name": "User 0",
-      "avatar": "7df9eb5cfa0b534812fd8e77c06bdccf",
-      "scmAccounts": [
-        "user-0"
-      ],
-      "tokensCount": 0,
-      "local": true
-    },
-    {
-      "login": "user-1",
-      "name": "User 1",
-      "avatar": "071241157c57e21956c73081138ecb6e",
-      "scmAccounts": [
-        "user-1"
-      ],
-      "tokensCount": 1,
-      "local": true
-    },
-    {
-      "login": "user-2",
-      "name": "User 2",
-      "avatar": "25c3b9b5609693983e1f9ed80820d7aa",
-      "scmAccounts": [
-        "user-2"
-      ],
-      "tokensCount": 2,
-      "local": true
-    },
-    {
-      "login": "user-3",
-      "name": "User 3",
-      "avatar": "61061cc5edb4b771d22509bc47eeaccf",
-      "scmAccounts": [
-        "user-3"
-      ],
-      "tokensCount": 3,
-      "local": true
-    },
-    {
-      "login": "user-4",
-      "name": "User 4",
-      "avatar": "07a317a07eb517ddf02bc592491ddf1b",
-      "scmAccounts": [
-        "user-4"
-      ],
-      "tokensCount": 4,
-      "local": true
-    }
-  ]
-}
diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/page_one.json b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/page_one.json
deleted file mode 100644 (file)
index 3985d56..0000000
+++ /dev/null
@@ -1,24 +0,0 @@
-{
-  "paging": {
-    "pageIndex": 1,
-    "pageSize": 5,
-    "total": 10
-  },
-  "users": [
-    {
-      "login": "user-0"
-    },
-    {
-      "login": "user-1"
-    },
-    {
-      "login": "user-2"
-    },
-    {
-      "login": "user-3"
-    },
-    {
-      "login": "user-4"
-    }
-  ]
-}
diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/page_two.json b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/page_two.json
deleted file mode 100644 (file)
index 8d2c5f7..0000000
+++ /dev/null
@@ -1,24 +0,0 @@
-{
-  "paging": {
-    "pageIndex": 2,
-    "pageSize": 5,
-    "total": 10
-  },
-  "users": [
-    {
-      "login": "user-5"
-    },
-    {
-      "login": "user-6"
-    },
-    {
-      "login": "user-7"
-    },
-    {
-      "login": "user-8"
-    },
-    {
-      "login": "user-9"
-    }
-  ]
-}
diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/user_one.json b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/user_one.json
deleted file mode 100644 (file)
index 383ea29..0000000
+++ /dev/null
@@ -1,20 +0,0 @@
-{
-  "paging": {
-    "pageIndex": 1,
-    "pageSize": 50,
-    "total": 1
-  },
-  "users": [
-    {
-      "login": "user-%_%-login",
-      "name": "user-name",
-      "avatar": "6ad193f57f79ac444c3621370da955e9",
-      "active": true,
-      "scmAccounts": [
-        "user1"
-      ],
-      "tokensCount": 0,
-      "local": true
-    }
-  ]
-}
diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/user_with_groups.json b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/user_with_groups.json
deleted file mode 100644 (file)
index f9ffeda..0000000
+++ /dev/null
@@ -1,19 +0,0 @@
-{
-  "paging": {
-    "pageIndex": 1,
-    "pageSize": 50,
-    "total": 1
-  },
-  "users": [
-    {
-      "login": "user-0",
-      "name": "User 0",
-      "email": "user-0@mail.com",
-      "scmAccounts": [
-        "user-0"
-      ],
-      "groups": ["sonar-admins", "sonar-users"],
-      "local": true
-    }
-  ]
-}
index 943400d8380e9461ee3534825a95d012fa4820e9..a1462c949f0268a4b4f2a3949a18b832aab9d829 100644 (file)
@@ -28,6 +28,7 @@ import org.sonar.api.user.UserGroupValidation;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static java.util.Objects.requireNonNull;
+import static org.apache.commons.lang.StringUtils.isBlank;
 import static org.apache.commons.lang.StringUtils.isNotBlank;
 
 /**
@@ -38,10 +39,13 @@ import static org.apache.commons.lang.StringUtils.isNotBlank;
 @Immutable
 public final class UserIdentity {
 
+  @Nullable
   private final String id;
   private final String providerLogin;
+  @Nullable
   private final String login;
   private final String name;
+  @Nullable
   private final String email;
   private final boolean groupsProvided;
   private final Set<String> groups;
@@ -77,9 +81,13 @@ public final class UserIdentity {
   }
 
   /**
-   * Non-blank user login, unique for the SonarQube platform.
+   * User login, unique for the SonarQube platform.
    * If two {@link IdentityProvider} define two users with the same login, then users are considered as identical.
+   *
+   * Since 7.4, a unique login will be generated if result is null and the user referenced by {@link #getProviderId()}
+   * or {@link #getProviderLogin()} does not already exist.
    */
+  @CheckForNull
   public String getLogin() {
     return login;
   }
@@ -153,9 +161,9 @@ public final class UserIdentity {
     }
 
     /**
-     * @see UserIdentity#getLogin() ()
+     * @see UserIdentity#getLogin()
      */
-    public Builder setLogin(String login) {
+    public Builder setLogin(@Nullable String login) {
       this.login = login;
       return this;
     }
@@ -217,9 +225,8 @@ public final class UserIdentity {
       checkArgument(providerLogin.length() <= 255, "Provider login size is incorrect (maximum 255 characters)");
     }
 
-    private static void validateLogin(String login) {
-      checkArgument(isNotBlank(login), "User login must not be blank");
-      checkArgument(login.length() <= 255 && login.length() >= 2, "User login size is incorrect (Between 2 and 255 characters)");
+    private static void validateLogin(@Nullable String login) {
+      checkArgument(isBlank(login) || (login.length() <= 255 && login.length() >= 2), "User login size is incorrect (Between 2 and 255 characters)");
     }
 
     private static void validateName(String name) {
index ae0e56d89f984d5cf8b4807a9469bf4deac0b975..a5daad2df3a512c97f0577da5f9aece926dacb54 100644 (file)
@@ -55,14 +55,13 @@ public class UserIdentityTest {
   public void create_user_with_minimum_fields() {
     UserIdentity underTest = UserIdentity.builder()
       .setProviderLogin("john")
-      .setLogin("1234")
       .setName("John")
       .build();
 
-    assertThat(underTest.getProviderId()).isNull();
     assertThat(underTest.getProviderLogin()).isEqualTo("john");
-    assertThat(underTest.getLogin()).isEqualTo("1234");
     assertThat(underTest.getName()).isEqualTo("John");
+    assertThat(underTest.getProviderId()).isNull();
+    assertThat(underTest.getLogin()).isNull();
     assertThat(underTest.getEmail()).isNull();
     assertThat(underTest.shouldSyncGroups()).isFalse();
     assertThat(underTest.getGroups()).isEmpty();
@@ -80,18 +79,6 @@ public class UserIdentityTest {
       .build();
   }
 
-  @Test
-  public void fail_when_login_is_empty() {
-    thrown.expect(IllegalArgumentException.class);
-    thrown.expectMessage("User login must not be blank");
-    UserIdentity.builder()
-      .setProviderLogin("john")
-      .setLogin("")
-      .setName("John")
-      .setEmail("john@email.com")
-      .build();
-  }
-
   @Test
   public void fail_when_login_is_too_long() {
     thrown.expect(IllegalArgumentException.class);