From 6276c7e052741b8175b6b1b7d28618b3b9c8ecdf Mon Sep 17 00:00:00 2001 From: Teryk Bellahsene Date: Thu, 22 Jun 2017 18:12:55 +0200 Subject: [PATCH] SONAR-9448 Sanitize api/qualityprofiles/projects --- .../sonar/server/ce/ws/ActivityAction.java | 2 +- .../server/projecttag/ws/SearchAction.java | 2 +- .../qualityprofile/ws/ProjectsAction.java | 133 +++++++++--------- ...le-projects.json => projects-example.json} | 0 .../qualityprofile/ws/ProjectsActionTest.java | 63 +++++++-- .../qualityprofile/ws/QProfilesWsTest.java | 9 -- .../org/sonar/api/server/ws/WebService.java | 31 ++-- 7 files changed, 139 insertions(+), 101 deletions(-) rename server/sonar-server/src/main/resources/org/sonar/server/qualityprofile/ws/{example-projects.json => projects-example.json} (100%) diff --git a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ActivityAction.java b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ActivityAction.java index 777e51746ad..2e034f91dae 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ActivityAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ActivityAction.java @@ -147,7 +147,7 @@ public class ActivityAction implements CeWsAction { .setDescription("Deprecated parameter") .setDeprecatedSince("5.5") .setDeprecatedKey("pageIndex", "5.4"); - action.addPageSize(100, MAX_PAGE_SIZE); + action.createPageSize(100, MAX_PAGE_SIZE); } @Override diff --git a/server/sonar-server/src/main/java/org/sonar/server/projecttag/ws/SearchAction.java b/server/sonar-server/src/main/java/org/sonar/server/projecttag/ws/SearchAction.java index ab915cd61c2..2300eaf507c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/projecttag/ws/SearchAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/projecttag/ws/SearchAction.java @@ -47,7 +47,7 @@ public class SearchAction implements ProjectTagsWsAction { .setHandler(this); action.addSearchQuery("off", "tags"); - action.addPageSize(10, 100); + action.createPageSize(10, 100); } @Override diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ProjectsAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ProjectsAction.java index bf1b85c41a8..332b45c6a92 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ProjectsAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ProjectsAction.java @@ -19,12 +19,10 @@ */ package org.sonar.server.qualityprofile.ws; -import com.google.common.collect.Collections2; -import com.google.common.collect.Iterables; -import com.google.common.collect.Lists; import java.util.Collection; import java.util.List; -import org.apache.commons.lang.builder.CompareToBuilder; +import java.util.Set; +import org.sonar.api.server.ws.Change; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService.NewAction; @@ -34,8 +32,7 @@ import org.sonar.api.server.ws.WebService.SelectionMode; import org.sonar.api.utils.Paging; import org.sonar.api.utils.text.JsonWriter; import org.sonar.api.web.UserRole; -import org.sonar.core.util.NonNullInputFunction; -import org.sonar.core.util.Uuids; +import org.sonar.core.util.stream.MoreCollectors; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.organization.OrganizationDto; @@ -44,14 +41,15 @@ import org.sonar.db.qualityprofile.QProfileDto; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.user.UserSession; +import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Comparator.comparing; import static org.sonar.api.utils.Paging.forPageIndex; +import static org.sonar.core.util.Uuids.UUID_EXAMPLE_01; +import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters.PARAM_PROFILE; public class ProjectsAction implements QProfileWsAction { - private static final String PARAM_KEY = "key"; - private static final String PARAM_QUERY = "query"; - private static final String PARAM_PAGE_SIZE = "pageSize"; - private static final String PARAM_PAGE = "page"; + private static final int MAX_PAGE_SIZE = 500; private final DbClient dbClient; private final UserSession userSession; @@ -65,66 +63,64 @@ public class ProjectsAction implements QProfileWsAction { @Override public void define(NewController controller) { - NewAction projects = controller.createAction("projects") + NewAction action = controller.createAction("projects") .setSince("5.2") .setHandler(this) - .setDescription("List projects with their association status regarding a quality profile.
" + - "Since 6.0, 'uuid' response field is deprecated and replaced by 'id'
" + - "Since 6.0, 'key' reponse field has been added to return the project key") - .setResponseExample(getClass().getResource("example-projects.json")); - projects.createParam(PARAM_KEY) - .setDescription("A quality profile key.") + .setDescription("List projects with their association status regarding a quality profile") + .setResponseExample(getClass().getResource("projects-example.json")); + + action.setChangelog( + new Change("6.5", "'id' response field is deprecated"), + new Change("6.0", "'uuid' response field is deprecated and replaced by 'id'"), + new Change("6.0", "'key' response field has been added to return the project key")); + + action.createParam(PARAM_PROFILE) + .setDescription("Quality profile key.") + .setDeprecatedKey("key", "6.5") .setRequired(true) - .setExampleValue(Uuids.UUID_EXAMPLE_01); - projects.addSelectionModeParam(); - projects.createParam(PARAM_QUERY) - .setDescription("If specified, return only projects whose name match the query."); - projects.createParam(PARAM_PAGE_SIZE) - .setDescription("Size for the paging to apply.").setDefaultValue(100); - projects.createParam(PARAM_PAGE) - .setDescription("Index of the page to display.").setDefaultValue(1); + .setExampleValue(UUID_EXAMPLE_01); + action.addSelectionModeParam(); + + action.createSearchQuery("sonar", "projects") + .setDeprecatedKey("query", "6.5"); + + action.createPageParam() + .setDeprecatedKey("page", "6.5"); + + action.createPageSize(100, MAX_PAGE_SIZE); } @Override public void handle(Request request, Response response) throws Exception { - String profileKey = request.mandatoryParam(PARAM_KEY); + String profileKey = request.mandatoryParam(PARAM_PROFILE); try (DbSession session = dbClient.openSession(false)) { checkProfileExists(profileKey, session); String selected = request.param(Param.SELECTED); - String query = request.param(PARAM_QUERY); - int pageSize = request.mandatoryParamAsInt(PARAM_PAGE_SIZE); - int page = request.mandatoryParamAsInt(PARAM_PAGE); - - List projects = loadProjects(profileKey, session, selected, query); - projects.sort((o1, o2) -> new CompareToBuilder() - // First, sort by name - .append(o1.getProjectName(), o2.getProjectName()) - // Then by UUID to disambiguate - .append(o1.getProjectUuid(), o2.getProjectUuid()) - .toComparison()); - - Collection projectIds = Collections2.transform(projects, new NonNullInputFunction() { - @Override - protected Long doApply(ProjectQprofileAssociationDto input) { - return input.getProjectId(); - } - }); - - Collection authorizedProjectIds = dbClient.authorizationDao().keepAuthorizedProjectIds(session, projectIds, userSession.getUserId(), UserRole.USER); - Iterable authorizedProjects = Iterables.filter(projects, input -> authorizedProjectIds.contains(input.getProjectId())); - - Paging paging = forPageIndex(page).withPageSize(pageSize).andTotal(authorizedProjectIds.size()); - - List pagedAuthorizedProjects = Lists.newArrayList(authorizedProjects); - if (pagedAuthorizedProjects.size() <= paging.offset()) { - pagedAuthorizedProjects = Lists.newArrayList(); - } else if (pagedAuthorizedProjects.size() > paging.pageSize()) { - int endIndex = Math.min(paging.offset() + pageSize, pagedAuthorizedProjects.size()); - pagedAuthorizedProjects = pagedAuthorizedProjects.subList(paging.offset(), endIndex); - } - - writeProjects(response.newJsonWriter(), pagedAuthorizedProjects, paging); + String query = request.param(Param.TEXT_QUERY); + int page = request.mandatoryParamAsInt(Param.PAGE); + int pageSize = request.mandatoryParamAsInt(Param.PAGE_SIZE); + checkArgument(pageSize <= MAX_PAGE_SIZE, "The '%s' parameter must be less than %s", Param.PAGE_SIZE, MAX_PAGE_SIZE); + + List projects = loadAllProjects(profileKey, session, selected, query).stream() + .sorted(comparing(ProjectQprofileAssociationDto::getProjectName) + .thenComparing(ProjectQprofileAssociationDto::getProjectUuid)) + .collect(MoreCollectors.toList()); + + Collection projectUuids = projects.stream() + .map(ProjectQprofileAssociationDto::getProjectUuid) + .collect(MoreCollectors.toSet()); + + Set authorizedProjectUuids = dbClient.authorizationDao().keepAuthorizedProjectUuids(session, projectUuids, userSession.getUserId(), UserRole.USER); + Paging paging = forPageIndex(page).withPageSize(pageSize).andTotal(authorizedProjectUuids.size()); + + List authorizedProjects = projects.stream() + .filter(input -> authorizedProjectUuids.contains(input.getProjectUuid())) + .skip(paging.offset()) + .limit(paging.pageSize()) + .collect(MoreCollectors.toList()); + + writeProjects(response, authorizedProjects, paging); } } @@ -134,22 +130,26 @@ public class ProjectsAction implements QProfileWsAction { } } - private List loadProjects(String profileKey, DbSession session, String selected, String query) { + private List loadAllProjects(String profileKey, DbSession session, String selected, String query) { QProfileDto profile = dbClient.qualityProfileDao().selectByUuid(session, profileKey); OrganizationDto organization = wsSupport.getOrganization(session, profile); - List projects = Lists.newArrayList(); + List projects; SelectionMode selectionMode = SelectionMode.fromParam(selected); + if (SelectionMode.SELECTED == selectionMode) { - projects.addAll(dbClient.qualityProfileDao().selectSelectedProjects(session, organization, profile, query)); + projects = dbClient.qualityProfileDao().selectSelectedProjects(session, organization, profile, query); } else if (SelectionMode.DESELECTED == selectionMode) { - projects.addAll(dbClient.qualityProfileDao().selectDeselectedProjects(session, organization, profile, query)); + projects = dbClient.qualityProfileDao().selectDeselectedProjects(session, organization, profile, query); } else { - projects.addAll(dbClient.qualityProfileDao().selectProjectAssociations(session, organization, profile, query)); + projects = dbClient.qualityProfileDao().selectProjectAssociations(session, organization, profile, query); } + return projects; } - private static void writeProjects(JsonWriter json, List projects, Paging paging) { + private static void writeProjects(Response response, List projects, Paging paging) { + JsonWriter json = response.newJsonWriter(); + json.beginObject(); json.name("results").beginArray(); for (ProjectQprofileAssociationDto project : projects) { @@ -164,6 +164,7 @@ public class ProjectsAction implements QProfileWsAction { } json.endArray(); json.prop("more", paging.hasNextPage()); - json.endObject().close(); + json.endObject(); + json.close(); } } diff --git a/server/sonar-server/src/main/resources/org/sonar/server/qualityprofile/ws/example-projects.json b/server/sonar-server/src/main/resources/org/sonar/server/qualityprofile/ws/projects-example.json similarity index 100% rename from server/sonar-server/src/main/resources/org/sonar/server/qualityprofile/ws/example-projects.json rename to server/sonar-server/src/main/resources/org/sonar/server/qualityprofile/ws/projects-example.json diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ProjectsActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ProjectsActionTest.java index 579c73ce642..ad57f5db450 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ProjectsActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ProjectsActionTest.java @@ -22,6 +22,9 @@ package org.sonar.server.qualityprofile.ws; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.sonar.api.server.ws.WebService; +import org.sonar.api.server.ws.WebService.Param; import org.sonar.api.utils.System2; import org.sonar.api.web.UserRole; import org.sonar.db.DbClient; @@ -40,8 +43,13 @@ import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.WsTester; import org.sonar.server.ws.WsTester.TestRequest; +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters.PARAM_PROFILE; + public class ProjectsActionTest { + @Rule + public ExpectedException expectedException = ExpectedException.none(); @Rule public DbTester db = DbTester.create(System2.INSTANCE); @Rule @@ -86,7 +94,7 @@ public class ProjectsActionTest { dbSession.commit(); - newRequest().setParam("key", xooP1.getKee()).setParam("selected", "selected").execute().assertJson(this.getClass(), "authorized_selected.json"); + newRequest().setParam(PARAM_PROFILE, xooP1.getKee()).setParam("selected", "selected").execute().assertJson(this.getClass(), "authorized_selected.json"); } @Test @@ -101,20 +109,20 @@ public class ProjectsActionTest { dbSession.commit(); - newRequest().setParam("key", xooP1.getKee()).setParam("selected", "selected").setParam("pageSize", "2") + newRequest().setParam(PARAM_PROFILE, xooP1.getKee()).setParam("selected", "selected").setParam(Param.PAGE_SIZE, "2") .execute().assertJson(this.getClass(), "selected_page1.json"); - newRequest().setParam("key", xooP1.getKee()).setParam("selected", "selected").setParam("pageSize", "2").setParam("page", "2") + newRequest().setParam(PARAM_PROFILE, xooP1.getKee()).setParam("selected", "selected").setParam(Param.PAGE_SIZE, "2").setParam(Param.PAGE, "2") .execute().assertJson(this.getClass(), "selected_page2.json"); - newRequest().setParam("key", xooP1.getKee()).setParam("selected", "selected").setParam("pageSize", "2").setParam("page", "3") + newRequest().setParam(PARAM_PROFILE, xooP1.getKee()).setParam("selected", "selected").setParam(Param.PAGE_SIZE, "2").setParam(Param.PAGE, "3") .execute().assertJson(this.getClass(), "empty.json"); - newRequest().setParam("key", xooP1.getKee()).setParam("selected", "selected").setParam("pageSize", "2").setParam("page", "4") + newRequest().setParam(PARAM_PROFILE, xooP1.getKee()).setParam("selected", "selected").setParam(Param.PAGE_SIZE, "2").setParam(Param.PAGE, "4") .execute().assertJson(this.getClass(), "empty.json"); - newRequest().setParam("key", xooP1.getKee()).setParam("selected", "selected").setParam("pageSize", "3").setParam("page", "1") + newRequest().setParam(PARAM_PROFILE, xooP1.getKee()).setParam("selected", "selected").setParam(Param.PAGE_SIZE, "3").setParam(Param.PAGE, "1") .execute().assertJson(this.getClass(), "selected_ps3_page1.json"); - newRequest().setParam("key", xooP1.getKee()).setParam("selected", "selected").setParam("pageSize", "3").setParam("page", "2") + newRequest().setParam(PARAM_PROFILE, xooP1.getKee()).setParam("selected", "selected").setParam(Param.PAGE_SIZE, "3").setParam(Param.PAGE, "2") .execute().assertJson(this.getClass(), "selected_ps3_page2.json"); - newRequest().setParam("key", xooP1.getKee()).setParam("selected", "selected").setParam("pageSize", "3").setParam("page", "3") + newRequest().setParam(PARAM_PROFILE, xooP1.getKee()).setParam("selected", "selected").setParam(Param.PAGE_SIZE, "3").setParam(Param.PAGE, "3") .execute().assertJson(this.getClass(), "empty.json"); } @@ -130,7 +138,7 @@ public class ProjectsActionTest { dbSession.commit(); - newRequest().setParam("key", xooP1.getKee()).setParam("selected", "deselected").execute().assertJson(this.getClass(), "deselected.json"); + newRequest().setParam(PARAM_PROFILE, xooP1.getKee()).setParam("selected", "deselected").execute().assertJson(this.getClass(), "deselected.json"); } @Test @@ -147,7 +155,7 @@ public class ProjectsActionTest { dbSession.commit(); - newRequest().setParam("key", xooP1.getKee()).setParam("selected", "all").execute().assertJson(this.getClass(), "all.json"); + newRequest().setParam(PARAM_PROFILE, xooP1.getKee()).setParam("selected", "all").execute().assertJson(this.getClass(), "all.json"); } @Test @@ -162,12 +170,14 @@ public class ProjectsActionTest { dbSession.commit(); - newRequest().setParam("key", xooP1.getKee()).setParam("selected", "all").setParam("query", "project t").execute().assertJson(this.getClass(), "all_filtered.json"); + newRequest().setParam(PARAM_PROFILE, xooP1.getKee()).setParam("selected", "all").setParam(Param.TEXT_QUERY, "project t").execute().assertJson(this.getClass(), "all_filtered.json"); } - @Test(expected = NotFoundException.class) + @Test public void should_fail_on_nonexistent_profile() throws Exception { - newRequest().setParam("key", "unknown").setParam("selected", "all").execute(); + expectedException.expect(NotFoundException.class); + + newRequest().setParam(PARAM_PROFILE, "unknown").setParam("selected", "all").execute(); } @Test @@ -184,7 +194,32 @@ public class ProjectsActionTest { dbSession.commit(); - newRequest().setParam("key", xooP1.getKee()).setParam("selected", "all").execute().assertJson(this.getClass(), "return_deprecated_uuid_field.json"); + newRequest().setParam(PARAM_PROFILE, xooP1.getKee()).setParam("selected", "all").execute().assertJson(this.getClass(), "return_deprecated_uuid_field.json"); + } + + @Test + public void fail_if_page_size_greater_than_500() throws Exception { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("The 'ps' parameter must be less than 500"); + + newRequest().setParam(PARAM_PROFILE, xooP1.getKee()).setParam(Param.PAGE_SIZE, "501").execute(); + } + + @Test + public void definition() { + WebService.Action definition = wsTester.action("api/qualityprofiles", "projects"); + + assertThat(definition.key()).isEqualTo("projects"); + assertThat(definition.responseExampleAsString()).isNotEmpty(); + assertThat(definition.params()).extracting(Param::key).containsExactlyInAnyOrder("profile", "p", "ps", "q", "selected"); + Param profile = definition.param("profile"); + assertThat(profile.deprecatedKey()).isEqualTo("key"); + Param page = definition.param("p"); + assertThat(page.deprecatedKey()).isEqualTo("page"); + Param pageSize = definition.param("ps"); + assertThat(pageSize.deprecatedKey()).isEqualTo("pageSize"); + Param query = definition.param("q"); + assertThat(query.deprecatedKey()).isEqualTo("query"); } private void createProfiles() { diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsTest.java index 8414f13a89c..1c547efa587 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsTest.java @@ -106,15 +106,6 @@ public class QProfilesWsTest { assertThat(setDefault.param("organization").since()).isEqualTo("6.4"); } - @Test - public void define_projects_action() { - WebService.Action projects = controller.action("projects"); - assertThat(projects).isNotNull(); - assertThat(projects.isPost()).isFalse(); - assertThat(projects.params()).hasSize(5); - assertThat(projects.responseExampleAsString()).isNotEmpty(); - } - @Test public void define_create_action() { WebService.Action create = controller.action("create"); diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java index c958463ee22..84cb57735c6 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java @@ -374,27 +374,25 @@ public interface WebService extends Definable { * Note the maximum is a documentation only feature. It does not check anything. */ public NewAction addPagingParams(int defaultPageSize, int maxPageSize) { - addPageParam(); - addPageSize(defaultPageSize, maxPageSize); + createPageParam(); + createPageSize(defaultPageSize, maxPageSize); return this; } - public NewAction addPageParam() { - createParam(Param.PAGE) + public NewParam createPageParam() { + return createParam(Param.PAGE) .setDescription("1-based page number") .setExampleValue("42") .setDeprecatedKey("pageIndex", "5.2") .setDefaultValue("1"); - return this; } - public NewAction addPageSize(int defaultPageSize, int maxPageSize) { - createParam(Param.PAGE_SIZE) + public NewParam createPageSize(int defaultPageSize, int maxPageSize) { + return createParam(Param.PAGE_SIZE) .setDescription("Page size. Must be greater than 0 and less than " + maxPageSize) .setExampleValue("20") .setDeprecatedKey("pageSize", "5.2") .setDefaultValue(String.valueOf(defaultPageSize)); - return this; } /** @@ -421,11 +419,24 @@ public interface WebService extends Definable { *

*/ public NewAction addSearchQuery(String exampleValue, String... pluralFields) { + createSearchQuery(exampleValue, pluralFields); + return this; + } + + /** + * + * Creates the parameter {@link org.sonar.api.server.ws.WebService.Param#TEXT_QUERY}, which is + * used to search for a subset of fields containing the supplied string. + *

+ * The fields must be in the plural form (ex: "names", "keys"). + *

+ */ + public NewParam createSearchQuery(String exampleValue, String... pluralFields) { String actionDescription = format("Limit search to %s that contain the supplied string.", Joiner.on(" or ").join(pluralFields)); - createParam(Param.TEXT_QUERY) + + return createParam(Param.TEXT_QUERY) .setDescription(actionDescription) .setExampleValue(exampleValue); - return this; } /** -- 2.39.5