From 4b5dcfc7085fab0201437330a8d500b7052ee618 Mon Sep 17 00:00:00 2001 From: Wojtek Wajerowicz <115081248+wojciech-wajerowicz-sonarsource@users.noreply.github.com> Date: Wed, 15 Mar 2023 11:48:02 +0100 Subject: [PATCH] SONAR-18534 Support filter=displayName eq "displayName" --- .../java/org/sonar/db/scim/ScimGroupDao.java | 8 +- .../org/sonar/db/scim/ScimGroupMapper.java | 4 +- .../org/sonar/db/scim/ScimGroupQuery.java | 65 +++++++++++++++ .../org/sonar/db/scim/ScimGroupMapper.xml | 10 ++- .../org/sonar/db/scim/ScimGroupDaoTest.java | 79 ++++++++++++++---- .../org/sonar/db/scim/ScimGroupQueryTest.java | 82 +++++++++++++++++++ 6 files changed, 223 insertions(+), 25 deletions(-) create mode 100644 server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimGroupQuery.java create mode 100644 server/sonar-db-dao/src/test/java/org/sonar/db/scim/ScimGroupQueryTest.java diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimGroupDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimGroupDao.java index 83c37d5a5d0..b6962213c8a 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimGroupDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimGroupDao.java @@ -37,8 +37,8 @@ public class ScimGroupDao implements Dao { return mapper(dbSession).findAll(); } - public List findScimGroups(DbSession dbSession, int offset, int limit) { - return mapper(dbSession).findScimGroups(new RowBounds(offset, limit)); + public List findScimGroups(DbSession dbSession, ScimGroupQuery query, int offset, int limit) { + return mapper(dbSession).findScimGroups(query, new RowBounds(offset, limit)); } public Optional findByScimUuid(DbSession dbSession, String scimGroupUuid) { @@ -49,8 +49,8 @@ public class ScimGroupDao implements Dao { return Optional.ofNullable(mapper(dbSession).findByGroupUuid(groupUuid)); } - public int countScimGroups(DbSession dbSession) { - return mapper(dbSession).countScimGroups(); + public int countScimGroups(DbSession dbSession, ScimGroupQuery query) { + return mapper(dbSession).countScimGroups(query); } public ScimGroupDto enableScimForGroup(DbSession dbSession, String groupUuid) { diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimGroupMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimGroupMapper.java index 715d68b40da..391ec3faa5c 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimGroupMapper.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimGroupMapper.java @@ -34,9 +34,9 @@ public interface ScimGroupMapper { @CheckForNull ScimGroupDto findByGroupUuid(@Param("groupUuid") String groupUuid); - List findScimGroups(RowBounds rowBounds); + List findScimGroups(@Param("query") ScimGroupQuery query, RowBounds rowBounds); - int countScimGroups(); + int countScimGroups(@Param("query") ScimGroupQuery query); void insert(@Param("scimGroupDto") ScimGroupDto scimGroupDto); diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimGroupQuery.java b/server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimGroupQuery.java new file mode 100644 index 00000000000..d62de9adee4 --- /dev/null +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/scim/ScimGroupQuery.java @@ -0,0 +1,65 @@ +/* + * SonarQube + * Copyright (C) 2009-2023 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.db.scim; + +import com.google.common.annotations.VisibleForTesting; +import java.util.Optional; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import javax.annotation.Nullable; + +import static java.util.regex.Pattern.CASE_INSENSITIVE; +import static org.apache.commons.lang.StringUtils.isBlank; + +public class ScimGroupQuery { + private static final Pattern DISPLAY_NAME_FILTER_PATTERN = Pattern.compile("^displayName\\s+eq\\s+\"([^\"]*?)\"$", CASE_INSENSITIVE); + private static final String UNSUPPORTED_FILTER = "Unsupported filter or value: %s. The only supported filter and operator is 'displayName eq \"displayName\""; + @VisibleForTesting + static final ScimGroupQuery ALL = new ScimGroupQuery(null); + + private final String displayName; + + @VisibleForTesting + protected ScimGroupQuery(@Nullable String displayName) { + this.displayName = displayName; + } + + public static ScimGroupQuery fromScimFilter(@Nullable String filter) { + if (isBlank(filter)) { + return ALL; + } + String userName = getDisplayNameFromFilter(filter) + .orElseThrow(() -> new IllegalArgumentException(String.format(UNSUPPORTED_FILTER, filter))); + + return new ScimGroupQuery(userName); + } + + private static Optional getDisplayNameFromFilter(String filter) { + Matcher matcher = DISPLAY_NAME_FILTER_PATTERN.matcher(filter.trim()); + return matcher.find() + ? Optional.of(matcher.group(1)) + : Optional.empty(); + + } + + public String getDisplayName() { + return displayName; + } +} diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/scim/ScimGroupMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/scim/ScimGroupMapper.xml index 86dea4d8772..17461b227cd 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/scim/ScimGroupMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/scim/ScimGroupMapper.xml @@ -34,12 +34,20 @@ select from scim_groups s + + inner join groups g on g.uuid=s.group_uuid + where g.name = #{query.displayName,jdbcType=VARCHAR} + order by s.scim_uuid asc diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/scim/ScimGroupDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/scim/ScimGroupDaoTest.java index 6b59f414501..a69b54e89e5 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/scim/ScimGroupDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/scim/ScimGroupDaoTest.java @@ -26,11 +26,13 @@ import java.util.Collection; import java.util.List; import java.util.Map; import java.util.stream.Collectors; -import java.util.stream.Stream; +import java.util.stream.IntStream; +import org.apache.commons.lang.StringUtils; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.sonar.db.DbTester; +import org.sonar.db.user.GroupDto; import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.assertj.core.api.Assertions.assertThat; @@ -40,6 +42,7 @@ import static org.assertj.core.groups.Tuple.tuple; @RunWith(DataProviderRunner.class) public class ScimGroupDaoTest { + private static final String DISPLAY_NAME_FILTER = "displayName eq \"group2\""; @Rule public DbTester db = DbTester.create(); private final ScimGroupDao scimGroupDao = db.getDbClient().scimGroupDao(); @@ -60,8 +63,7 @@ public class ScimGroupDaoTest { .extracting(ScimGroupDto::getGroupUuid, ScimGroupDto::getScimGroupUuid) .containsExactlyInAnyOrder( tuple(scimGroup1.getGroupUuid(), scimGroup1.getScimGroupUuid()), - tuple(scimGroup2.getGroupUuid(), scimGroup2.getScimGroupUuid()) - ); + tuple(scimGroup2.getGroupUuid(), scimGroup2.getScimGroupUuid())); } @Test @@ -69,12 +71,12 @@ public class ScimGroupDaoTest { int totalScimGroups = 15; generateScimGroups(totalScimGroups); - assertThat(scimGroupDao.countScimGroups(db.getSession())).isEqualTo(totalScimGroups); + assertThat(scimGroupDao.countScimGroups(db.getSession(), ScimGroupQuery.ALL)).isEqualTo(totalScimGroups); } @Test public void countScimGroups_shouldReturnZero_whenNoScimGroups() { - assertThat(scimGroupDao.countScimGroups(db.getSession())).isZero(); + assertThat(scimGroupDao.countScimGroups(db.getSession(), ScimGroupQuery.ALL)).isZero(); } @DataProvider @@ -91,13 +93,49 @@ public class ScimGroupDaoTest { @Test @UseDataProvider("paginationData") - public void findScimGroups_whenPaginationAndStartIndex_shouldReturnTheCorrectNumberOfScimGroups(int totalScimGroups, int offset, int pageSize, List expectedScimGroupUuids) { + public void findScimGroups_whenPaginationAndStartIndex_shouldReturnTheCorrectNumberOfScimGroups(int totalScimGroups, int offset, int pageSize, + List expectedScimGroupUuidSuffixes) { generateScimGroups(totalScimGroups); - List scimUserDtos = scimGroupDao.findScimGroups(db.getSession(), offset, pageSize); + List scimGroupDtos = scimGroupDao.findScimGroups(db.getSession(), ScimGroupQuery.ALL, offset, pageSize); - List scimGroupsUuids = toScimGroupsUuids(scimUserDtos); - assertThat(scimGroupsUuids).containsExactlyElementsOf(expectedScimGroupUuids); + List actualScimGroupsUuids = toScimGroupsUuids(scimGroupDtos); + List expectedScimGroupUuids = toExpectedscimGroupUuids(expectedScimGroupUuidSuffixes); + assertThat(actualScimGroupsUuids).containsExactlyElementsOf(expectedScimGroupUuids); + } + + private static List toExpectedscimGroupUuids(List expectedScimGroupUuidSuffixes) { + return expectedScimGroupUuidSuffixes.stream() + .map(expectedScimGroupUuidSuffix -> "scim_uuid_Scim Group" + expectedScimGroupUuidSuffix) + .collect(Collectors.toList()); + } + + @Test + public void findScimGroups_whenFilteringByDisplayName_shouldReturnTheExpectedScimGroups() { + insertGroupAndScimGroup("group1"); + insertGroupAndScimGroup("group2"); + ScimGroupQuery query = ScimGroupQuery.fromScimFilter(DISPLAY_NAME_FILTER); + + List scimGroups = scimGroupDao.findScimGroups(db.getSession(), query, 0, 100); + + assertThat(scimGroups).hasSize(1); + assertThat(scimGroups.get(0).getScimGroupUuid()).isEqualTo(createScimGroupUuid("group2")); + } + + @Test + public void countScimGroups_whenFilteringByDisplayName_shouldReturnCorrectCount() { + insertGroupAndScimGroup("group1"); + insertGroupAndScimGroup("group2"); + ScimGroupQuery query = ScimGroupQuery.fromScimFilter(DISPLAY_NAME_FILTER); + + int groupCount = scimGroupDao.countScimGroups(db.getSession(), query); + + assertThat(groupCount).isEqualTo(1); + } + + private void insertGroupAndScimGroup(String groupName) { + GroupDto groupDto = insertGroup(groupName); + insertScimGroup(createScimGroupUuid(groupName), groupDto.getUuid()); } @Test @@ -110,26 +148,31 @@ public class ScimGroupDaoTest { public void getManagedGroupsSqlFilter_whenFilterByManagedIsFalse_returnsCorrectQuery() { String filterNonManagedUser = scimGroupDao.getManagedGroupSqlFilter(false); assertThat(filterNonManagedUser).isEqualTo("not exists (select group_uuid from scim_groups sg where sg.group_uuid = uuid)"); + } + private List generateScimGroups(int totalScimGroups) { + return IntStream.range(1, totalScimGroups + 1) + .mapToObj(i -> insertGroup(createGroupName(i))) + .map(groupDto -> insertScimGroup(createScimGroupUuid(groupDto.getName()), groupDto.getUuid())) + .toList(); } - private void generateScimGroups(int totalScimGroups) { - List allScimGroups = Stream.iterate(1, i -> i + 1) - .map(i -> insertScimGroup(i.toString())) - .limit(totalScimGroups) - .collect(Collectors.toList()); - assertThat(allScimGroups).hasSize(totalScimGroups); + private static String createGroupName(int i) { + return "Scim Group" + i; } - private ScimGroupDto insertScimGroup(String scimGroupUuid) { - return insertScimGroup(scimGroupUuid, randomAlphanumeric(40)); + private GroupDto insertGroup(String name) { + return db.users().insertGroup(name); + } + + private static String createScimGroupUuid(String groupName) { + return StringUtils.substring("scim_uuid_" + groupName, 0, 40); } private ScimGroupDto insertScimGroup(String scimGroupUuid, String groupUuid) { ScimGroupDto scimGroupDto = new ScimGroupDto(scimGroupUuid, groupUuid); Map data = Map.of("scim_uuid", scimGroupDto.getScimGroupUuid(), "group_uuid", scimGroupDto.getGroupUuid()); db.executeInsert("scim_groups", data); - return scimGroupDto; } diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/scim/ScimGroupQueryTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/scim/ScimGroupQueryTest.java new file mode 100644 index 00000000000..b6c0b5bc4c2 --- /dev/null +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/scim/ScimGroupQueryTest.java @@ -0,0 +1,82 @@ +/* + * SonarQube + * Copyright (C) 2009-2023 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.db.scim; + +import com.tngtech.java.junit.dataprovider.DataProvider; +import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import com.tngtech.java.junit.dataprovider.UseDataProvider; +import org.junit.Test; +import org.junit.runner.RunWith; + +import static java.lang.String.format; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; + +@RunWith(DataProviderRunner.class) +public class ScimGroupQueryTest { + + @DataProvider + public static Object[][] filterData() { + ScimGroupQuery queryWithDisplayName = new ScimGroupQuery("testGroup"); + return new Object[][] { + {"displayName eq \"testGroup\"", queryWithDisplayName}, + {" displayName eq \"testGroup\" ", queryWithDisplayName}, + {"displayName eq \"testGroup\"", queryWithDisplayName}, + {"DIsPlaynaMe eq \"testGroup\"", queryWithDisplayName}, + {"displayName EQ \"testGroup\"", queryWithDisplayName}, + {null, ScimGroupQuery.ALL}, + {"", ScimGroupQuery.ALL} + }; + } + + @Test + @UseDataProvider("filterData") + public void fromScimFilter_shouldCorrectlyResolveProperties(String filter, ScimGroupQuery expected) { + ScimGroupQuery scimGroupQuery = ScimGroupQuery.fromScimFilter(filter); + + assertThat(scimGroupQuery).usingRecursiveComparison().isEqualTo(expected); + } + + @DataProvider + public static Object[][] unsupportedFilterData() { + return new Object[][] { + {"otherProp eq \"testGroup\""}, + {"displayName eq \"testGroup\" or displayName eq \"testGroup2\""}, + {"displayName eq \"testGroup\" and email eq \"test.user2@okta.local\""}, + {"displayName eq \"testGroup\"xjdkfgldkjfhg"} + }; + } + + @Test + @UseDataProvider("unsupportedFilterData") + public void fromScimFilter_shouldThrowAnException(String filter) { + assertThatIllegalArgumentException() + .isThrownBy(() -> ScimGroupQuery.fromScimFilter(filter)) + .withMessage(format("Unsupported filter or value: %s. The only supported filter and operator is 'displayName eq \"displayName\"", filter)); + } + + @Test + public void empty_shouldHaveNoProperties() { + ScimGroupQuery scimGroupQuery = ScimGroupQuery.ALL; + + assertThat(scimGroupQuery.getDisplayName()).isNull(); + } + +} -- 2.39.5