From 32dcc4c93fbcde717187551ad4a104a7290fcf92 Mon Sep 17 00:00:00 2001 From: Aurelien Poscia Date: Tue, 28 Feb 2023 17:08:58 +0100 Subject: [PATCH] SONAR-18532 Add support for group membership PUT in /api/scim/v2/Groups --- .../java/org/sonar/db/scim/ScimUserDaoIT.java | 105 +++++++++++++++--- .../java/org/sonar/db/scim/ScimUserDao.java | 36 +++++- .../java/org/sonar/db/scim/ScimUserQuery.java | 35 +++++- .../main/java/org/sonar/db/user/UserDao.java | 1 + .../org/sonar/db/scim/ScimUserMapper.xml | 32 +++++- .../java/org/sonar/db/user/UserDbTester.java | 12 ++ 6 files changed, 196 insertions(+), 25 deletions(-) diff --git a/server/sonar-db-dao/src/it/java/org/sonar/db/scim/ScimUserDaoIT.java b/server/sonar-db-dao/src/it/java/org/sonar/db/scim/ScimUserDaoIT.java index fb99c012799..6f90e777a36 100644 --- a/server/sonar-db-dao/src/it/java/org/sonar/db/scim/ScimUserDaoIT.java +++ b/server/sonar-db-dao/src/it/java/org/sonar/db/scim/ScimUserDaoIT.java @@ -26,9 +26,9 @@ import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.IntStream; -import java.util.stream.Stream; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -64,7 +64,6 @@ public class ScimUserDaoIT { assertThat(scimUserDtos).hasSize(2) .map(scimUserDto -> new ScimUserTestData(scimUserDto.getScimUserUuid(), scimUserDto.getUserUuid())) .containsExactlyInAnyOrder(scimUser1TestData, scimUser2TestData); - } @Test @@ -109,7 +108,7 @@ public class ScimUserDaoIT { {9, 3, 3, List.of("4", "5", "6")}, {9, 7, 3, List.of("8", "9")}, {5, 5, 20, List.of()}, - {5, 0, 0, List.of()} + {5, 0, 0, List.of()}, }; } @@ -154,18 +153,17 @@ public class ScimUserDaoIT { @Test public void countScimUsers_shoudReturnCorrectNumberOfScimUser_whenFilteredByScimUserName() { - inserScimUsersWithUsers(List.of("TEST_A", "TEST_B", "TEST_B_BIS", "TEST_C", "TEST_D")); + insertScimUsersWithUsers(List.of("TEST_A", "TEST_B", "TEST_B_BIS", "TEST_C", "TEST_D")); ScimUserQuery scimUserQuery = ScimUserQuery.builder().userName("test_b").build(); assertThat(scimUserDao.countScimUsers(dbSession, scimUserQuery)).isEqualTo(1); } - private void generateScimUsers(int totalScimUsers) { - List allScimUsers = Stream.iterate(1, i -> i + 1) - .map(i -> insertScimUser(i.toString())) - .limit(totalScimUsers) + private List generateScimUsers(int totalScimUsers) { + List userNames = IntStream.range(0, totalScimUsers) + .mapToObj(i -> "username_" + i) .collect(Collectors.toList()); - assertThat(allScimUsers).hasSize(totalScimUsers); + return insertScimUsersWithUsers(userNames); } @Test @@ -191,7 +189,7 @@ public class ScimUserDaoIT { @Test @UseDataProvider("filterData") public void findScimUsers_whenFilteringByUserName_shouldReturnTheExpectedScimUsers(String search, List userLogins, List expectedScimUserUuids) { - inserScimUsersWithUsers(userLogins); + insertScimUsersWithUsers(userLogins); ScimUserQuery query = ScimUserQuery.builder().userName(search).build(); List scimUsersByQuery = scimUserDao.findScimUsers(dbSession, query, 0, 100); @@ -200,6 +198,81 @@ public class ScimUserDaoIT { assertThat(scimUsersUuids).containsExactlyElementsOf(expectedScimUserUuids); } + @Test + public void findScimUsers_whenFilteringByScimUuidsWithLongRange_shouldReturnTheExpectedScimUsers() { + generateScimUsers(3000); + Set expectedScimUserUuids = generateStrings(1, 2050); + + ScimUserQuery query = ScimUserQuery.builder().scimUserUuids(expectedScimUserUuids).build(); + + List scimUsersByQuery = scimUserDao.findScimUsers(dbSession, query, 0, Integer.MAX_VALUE); + + List scimUsersUuids = toScimUsersUuids(scimUsersByQuery); + assertThat(scimUsersByQuery) + .hasSameSizeAs(scimUsersUuids) + .extracting(ScimUserDto::getScimUserUuid) + .containsAll(expectedScimUserUuids); + } + + @Test + public void findScimUsers_whenFilteringByScimUuidsAndUserName_shouldReturnTheExpectedScimUser() { + Set scimUserUuids = generateScimUsers(10).stream() + .map(ScimUserTestData::getScimUserUuid) + .collect(Collectors.toSet()); + + ScimUserQuery query = ScimUserQuery.builder().scimUserUuids(scimUserUuids).userName("username_5").build(); + + List scimUsersByQuery = scimUserDao.findScimUsers(dbSession, query, 0, Integer.MAX_VALUE); + + assertThat(scimUsersByQuery).hasSize(1) + .extracting(ScimUserDto::getScimUserUuid) + .contains("6"); + } + + @Test + public void findScimUsers_whenFilteringByUserUuidsWithLongRange_shouldReturnTheExpectedScimUsers() { + List scimUsersTestData = generateScimUsers(3000); + Set allUsersUuid = scimUsersTestData.stream() + .map(ScimUserTestData::getUserUuid) + .collect(Collectors.toSet()); + + ScimUserQuery query = ScimUserQuery.builder().userUuids(allUsersUuid).build(); + + List scimUsersByQuery = scimUserDao.findScimUsers(dbSession, query, 0, Integer.MAX_VALUE); + + assertThat(scimUsersByQuery) + .hasSameSizeAs(allUsersUuid) + .extracting(ScimUserDto::getUserUuid) + .containsAll(allUsersUuid); + } + + @Test + public void findScimUsers_whenFilteringByUserUuidsAndUserName_shouldReturnTheExpectedScimUser() { + List scimUsersTestData = generateScimUsers(10); + Set allUsersUuid = scimUsersTestData.stream() + .map(ScimUserTestData::getUserUuid) + .collect(Collectors.toSet()); + + ScimUserQuery query = ScimUserQuery.builder().userUuids(allUsersUuid).userName("username_5").build(); + + List scimUsersByQuery = scimUserDao.findScimUsers(dbSession, query, 0, Integer.MAX_VALUE); + + assertThat(scimUsersByQuery).hasSize(1) + .extracting(ScimUserDto::getScimUserUuid) + .contains("6"); + } + + private static Set generateStrings(int startInclusive, int endExclusive) { + return generateStrings(startInclusive, endExclusive, ""); + } + + private static Set generateStrings(int startInclusive, int endExclusive, String prefix) { + return IntStream.range(startInclusive, endExclusive) + .mapToObj(String::valueOf) + .map(string -> prefix + string) + .collect(Collectors.toSet()); + } + @Test public void deleteByUserUuid_shouldDeleteScimUser() { ScimUserTestData scimUserTestData = insertScimUser("scimUser"); @@ -229,14 +302,15 @@ public class ScimUserDaoIT { assertThatCode(() -> scimUserDao.deleteByUserUuid(dbSession, randomAlphanumeric(6))).doesNotThrowAnyException(); } - private void inserScimUsersWithUsers(List userLogins) { - IntStream.range(0, userLogins.size()) - .forEachOrdered(i -> insertScimUserWithUser(userLogins.get(i), String.valueOf(i + 1))); + private List insertScimUsersWithUsers(List userLogins) { + return IntStream.range(0, userLogins.size()) + .mapToObj(i -> insertScimUserWithUser(userLogins.get(i), String.valueOf(i + 1))) + .collect(Collectors.toList()); } - private void insertScimUserWithUser(String userLogin, String scimUuid) { + private ScimUserTestData insertScimUserWithUser(String userLogin, String scimUuid) { UserDto userDto = db.users().insertUser(u -> u.setExternalId(userLogin)); - insertScimUser(scimUuid, userDto.getUuid()); + return insertScimUser(scimUuid, userDto.getUuid()); } private ScimUserTestData insertScimUser(String scimUserUuid) { @@ -247,7 +321,6 @@ public class ScimUserDaoIT { ScimUserTestData scimUserTestData = new ScimUserTestData(scimUserUuid, userUuid); Map data = Map.of("scim_uuid", scimUserTestData.getScimUserUuid(), "user_uuid", scimUserTestData.getUserUuid()); db.executeInsert("scim_users", data); - return scimUserTestData; } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimUserDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimUserDao.java index 9476e7d842d..d2a9fb25da1 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimUserDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimUserDao.java @@ -19,13 +19,18 @@ */ package org.sonar.db.scim; +import java.util.HashSet; import java.util.List; import java.util.Optional; -import org.sonar.core.util.UuidFactory; +import java.util.function.BiFunction; import org.apache.ibatis.session.RowBounds; +import org.sonar.core.util.UuidFactory; import org.sonar.db.Dao; import org.sonar.db.DbSession; +import static com.google.common.base.Preconditions.checkState; +import static org.sonar.db.DatabaseUtils.executeLargeInputs; + public class ScimUserDao implements Dao { private final UuidFactory uuidFactory; @@ -45,6 +50,7 @@ public class ScimUserDao implements Dao { return Optional.ofNullable(mapper(dbSession).findByUserUuid(userUuid)); } + public ScimUserDto enableScimForUser(DbSession dbSession, String userUuid) { ScimUserDto scimUserDto = new ScimUserDto(uuidFactory.create(), userUuid); mapper(dbSession).insert(scimUserDto); @@ -52,9 +58,37 @@ public class ScimUserDao implements Dao { } public List findScimUsers(DbSession dbSession, ScimUserQuery scimUserQuery, int offset, int limit) { + checkState(scimUserQuery.getUserUuids() == null || scimUserQuery.getScimUserUuids() == null, + "Only one of userUuids & scimUserUuids request parameter is supported."); + if (scimUserQuery.getScimUserUuids() != null) { + return executeLargeInputs( + scimUserQuery.getScimUserUuids(), + partialSetOfUsers -> createPartialQuery(scimUserQuery, partialSetOfUsers, + (builder, scimUserUuids) -> builder.scimUserUuids(new HashSet<>(scimUserUuids)), + dbSession, offset, limit) + ); + } + if (scimUserQuery.getUserUuids() != null) { + return executeLargeInputs( + scimUserQuery.getUserUuids(), + partialSetOfUsers -> createPartialQuery(scimUserQuery, partialSetOfUsers, + (builder, userUuids) -> builder.userUuids(new HashSet<>(userUuids)), + dbSession, offset, limit) + ); + } return mapper(dbSession).findScimUsers(scimUserQuery, new RowBounds(offset, limit)); } + private static List createPartialQuery(ScimUserQuery completeQuery, List strings, + BiFunction, ScimUserQuery.ScimUserQueryBuilder> queryModifier, + DbSession dbSession, int offset, int limit) { + + ScimUserQuery.ScimUserQueryBuilder partialScimUserQuery = ScimUserQuery.builder() + .userName(completeQuery.getUserName()); + partialScimUserQuery = queryModifier.apply(partialScimUserQuery, strings); + return mapper(dbSession).findScimUsers(partialScimUserQuery.build(), new RowBounds(offset, limit)); + } + public int countScimUsers(DbSession dbSession, ScimUserQuery scimUserQuery) { return mapper(dbSession).countScimUsers(scimUserQuery); } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimUserQuery.java b/server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimUserQuery.java index 35908f2ec14..99ba5e05211 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimUserQuery.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimUserQuery.java @@ -20,6 +20,7 @@ package org.sonar.db.scim; import java.util.Optional; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.annotation.CheckForNull; @@ -33,9 +34,13 @@ public class ScimUserQuery { private static final String UNSUPPORTED_FILTER = "Unsupported filter value: %s. Format should be 'userName eq \"username\"'"; private final String userName; + private final Set scimUserUuids; + private final Set userUuids; - private ScimUserQuery(String userName) { + private ScimUserQuery(@Nullable String userName, @Nullable Set scimUserUuids, @Nullable Set userUuids) { this.userName = userName; + this.scimUserUuids = scimUserUuids; + this.userUuids = userUuids; } @CheckForNull @@ -43,6 +48,16 @@ public class ScimUserQuery { return userName; } + @CheckForNull + public Set getScimUserUuids() { + return scimUserUuids; + } + + @CheckForNull + public Set getUserUuids() { + return userUuids; + } + public static ScimUserQuery empty() { return builder().build(); } @@ -72,18 +87,30 @@ public class ScimUserQuery { public static final class ScimUserQueryBuilder { private String userName; + private Set scimUserUuids; + private Set userUuids; private ScimUserQueryBuilder() { } - public ScimUserQueryBuilder userName(String userName) { + public ScimUserQueryBuilder userName(@Nullable String userName) { this.userName = userName; return this; } - public ScimUserQuery build() { - return new ScimUserQuery(userName); + + public ScimUserQueryBuilder scimUserUuids(Set scimUserUuids) { + this.scimUserUuids = scimUserUuids; + return this; } + public ScimUserQueryBuilder userUuids(Set userUuids) { + this.userUuids = userUuids; + return this; + } + + public ScimUserQuery build() { + return new ScimUserQuery(userName, scimUserUuids, userUuids); + } } } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDao.java index 8387c648d9b..1949fff8071 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDao.java @@ -78,6 +78,7 @@ public class UserDao implements Dao { /** * Select users by uuids, including disabled users. An empty list is returned * if list of uuids is empty, without any db round trips. + * @return */ public List selectByUuids(DbSession session, Collection uuids) { return executeLargeInputs(uuids, mapper(session)::selectByUuids); diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/scim/ScimUserMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/scim/ScimUserMapper.xml index 734cbb15a26..a973a535c6b 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/scim/ScimUserMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/scim/ScimUserMapper.xml @@ -22,6 +22,17 @@ scim_uuid = #{scimUserUuid,jdbcType=VARCHAR} + + + - select count(1) from scim_users s + inner join users u on u.uuid=s.user_uuid + where 1=1 - inner join users u on u.uuid=s.user_uuid - where lower(u.external_id) like lower(#{query.userName,jdbcType=VARCHAR}) escape '/' + and lower(u.external_id) like lower(#{query.userName,jdbcType=VARCHAR}) escape '/' + + + and s.scim_uuid in + + #{scimUserUuid, jdbcType=VARCHAR} + + + + and s.user_uuid in + + #{userUuid, jdbcType=VARCHAR} + diff --git a/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/user/UserDbTester.java b/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/user/UserDbTester.java index 190d0299478..40f77ebcecd 100644 --- a/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/user/UserDbTester.java +++ b/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/user/UserDbTester.java @@ -39,6 +39,7 @@ import org.sonar.db.permission.GroupPermissionDto; import org.sonar.db.permission.UserPermissionDto; import org.sonar.db.project.ProjectDto; import org.sonar.db.scim.ScimGroupDto; +import org.sonar.db.scim.ScimUserDto; import static com.google.common.base.Preconditions.checkArgument; import static java.util.Arrays.stream; @@ -89,6 +90,12 @@ public class UserDbTester { return updatedUser; } + public ScimUserDto enableScimForUser(UserDto userDto) { + ScimUserDto scimUSerDto = db.getDbClient().scimUserDao().enableScimForUser(db.getSession(), userDto.getUuid()); + db.commit(); + return scimUSerDto; + } + public UserDto insertAdminByUserPermission() { UserDto user = insertUser(); insertGlobalPermissionOnUser(user, ADMINISTER); @@ -175,6 +182,11 @@ public class UserDbTester { db.commit(); } + public List findMembers(GroupDto group) { + Set userUuidsInGroup = db.getDbClient().userGroupDao().selectUserUuidsInGroup(db.getSession(), group.getUuid()); + return db.getDbClient().userDao().selectByUuids(db.getSession(), userUuidsInGroup); + } + public List selectGroupUuidsOfUser(UserDto user) { return db.getDbClient().groupMembershipDao().selectGroupUuidsByUserUuid(db.getSession(), user.getUuid()); } -- 2.39.5