From 4a70f91143e2073d38dfd690e190c40b61c5817e Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Mon, 24 Sep 2018 13:34:10 +0200 Subject: [PATCH] SONAR-11302 Generate random login when identity login is null --- .../UserIdentityAuthenticatorImpl.java | 53 ++- .../java/org/sonar/server/user/NewUser.java | 3 +- .../org/sonar/server/user/UserUpdater.java | 34 +- .../sonar/server/user/ws/CreateAction.java | 2 + .../UserIdentityAuthenticatorImplTest.java | 97 +++- .../server/user/UserUpdaterCreateTest.java | 49 +- .../user/UserUpdaterReactivateTest.java | 18 + .../server/user/ws/CreateActionTest.java | 17 +- .../server/user/ws/SearchActionTest.java | 434 +++++++++--------- .../user/ws/SearchActionTest/five_users.json | 59 --- .../user/ws/SearchActionTest/page_one.json | 24 - .../user/ws/SearchActionTest/page_two.json | 24 - .../user/ws/SearchActionTest/user_one.json | 20 - .../ws/SearchActionTest/user_with_groups.json | 19 - .../server/authentication/UserIdentity.java | 19 +- .../authentication/UserIdentityTest.java | 17 +- 16 files changed, 451 insertions(+), 438 deletions(-) delete mode 100644 server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/five_users.json delete mode 100644 server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/page_one.json delete mode 100644 server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/page_two.json delete mode 100644 server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/user_one.json delete mode 100644 server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/user_with_groups.json diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/UserIdentityAuthenticatorImpl.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/UserIdentityAuthenticatorImpl.java index a5247b533b9..be21b9b02b0 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/authentication/UserIdentityAuthenticatorImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/UserIdentityAuthenticatorImpl.java @@ -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 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 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 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 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 userGroups = new HashSet<>(dbClient.groupMembershipDao().selectGroupsByLogins(dbSession, singletonList(userLogin)).get(userLogin)); Set 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; + } + } diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/NewUser.java b/server/sonar-server/src/main/java/org/sonar/server/user/NewUser.java index 7a8a976d97a..4da0ba3b664 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/NewUser.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/NewUser.java @@ -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; } 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 d968fdf9da5..9086a9c453b 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 @@ -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 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 messages = newArrayList(); boolean changed = updateLogin(dbSession, update, dto, messages); diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ws/CreateAction.java b/server/sonar-server/src/main/java/org/sonar/server/user/ws/CreateAction.java index adebf8d1a82..97b13625df4 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/ws/CreateAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/ws/CreateAction.java @@ -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"); diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/UserIdentityAuthenticatorImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/UserIdentityAuthenticatorImplTest.java index c58afab14bc..bee36526a43 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/authentication/UserIdentityAuthenticatorImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/UserIdentityAuthenticatorImplTest.java @@ -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(); 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 f12c7485428..c231333770a 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 @@ -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 diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterReactivateTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterReactivateTest.java index 26534693671..d677e4fbd7d 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterReactivateTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterReactivateTest.java @@ -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)); diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ws/CreateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ws/CreateActionTest.java index 6ca46d0fa5d..8812dde814e 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/ws/CreateActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/ws/CreateActionTest.java @@ -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); diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java index 2e6f056e861..77e6c2bbc8f 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java @@ -19,38 +19,33 @@ */ 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 injectUsers(int numberOfUsers) { - List 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 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 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 index ff3f77667da..00000000000 --- a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/five_users.json +++ /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 index 3985d561cb1..00000000000 --- a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/page_one.json +++ /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 index 8d2c5f7d62d..00000000000 --- a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/page_two.json +++ /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 index 383ea295686..00000000000 --- a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/user_one.json +++ /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 index f9ffeda9b6f..00000000000 --- a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/user_with_groups.json +++ /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 - } - ] -} diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/authentication/UserIdentity.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/authentication/UserIdentity.java index 943400d8380..a1462c949f0 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/authentication/UserIdentity.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/authentication/UserIdentity.java @@ -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 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) { diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/server/authentication/UserIdentityTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/server/authentication/UserIdentityTest.java index ae0e56d89f9..a5daad2df3a 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/server/authentication/UserIdentityTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/server/authentication/UserIdentityTest.java @@ -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); -- 2.39.5