From 011eb8ec61f878002dafa6ba2bb67782f33d6a5b Mon Sep 17 00:00:00 2001 From: Guillaume Jambet Date: Fri, 13 Apr 2018 11:47:54 +0200 Subject: [PATCH] SONAR-10545 do not allow action on external issues --- .../server/issue/ws/DoTransitionAction.java | 3 ++- .../sonar/server/issue/ws/SearchAction.java | 3 ++- .../server/issue/ws/SearchResponseFormat.java | 10 +++++----- .../server/issue/ws/SearchResponseLoader.java | 10 +++++----- .../issue/ws/DoTransitionActionTest.java | 18 ++++++++++++++++++ .../server/issue/ws/SearchActionTest.java | 2 +- 6 files changed, 33 insertions(+), 13 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/DoTransitionAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/DoTransitionAction.java index 501d7f03fa7..5edebd304e3 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/DoTransitionAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/DoTransitionAction.java @@ -37,7 +37,7 @@ import org.sonar.server.issue.IssueFinder; import org.sonar.server.issue.IssueUpdater; import org.sonar.server.issue.TransitionService; import org.sonar.server.user.UserSession; - +import static org.sonar.server.ws.WsUtils.checkRequest; import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_DO_TRANSITION; import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_ISSUE; import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_TRANSITION; @@ -92,6 +92,7 @@ public class DoTransitionAction implements IssuesWsAction { String issue = request.mandatoryParam(PARAM_ISSUE); try (DbSession dbSession = dbClient.openSession(false)) { IssueDto issueDto = issueFinder.getByKey(dbSession, issue); + checkRequest(!issueDto.isExternal(), "Transition are not allowed on issues imported from external rule engines"); SearchResponseData preloadedSearchResponseData = doTransition(dbSession, issueDto, request.mandatoryParam(PARAM_TRANSITION)); responseWriter.write(issue, preloadedSearchResponseData, request, response); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java index 893880956fb..ba1e8c683e3 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java @@ -61,6 +61,7 @@ import org.sonar.server.issue.index.IssueIndex; import org.sonar.server.user.UserSession; import org.sonarqube.ws.Issues.SearchWsResponse; +import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.Iterables.concat; import static com.google.common.collect.Sets.newHashSet; @@ -384,7 +385,7 @@ public class SearchAction implements IssuesWsAction { // Must be done after loading of data as the "hidden" facet "debt" // can be used to get total debt. facets = reorderFacets(facets, options.getFacets()); - replaceRuleIdsByRuleKeys(facets, data.getRules() == null ? emptyList() : data.getRules()); + replaceRuleIdsByRuleKeys(facets, firstNonNull(data.getRules(), emptyList())); // FIXME allow long in Paging Paging paging = forPageIndex(options.getPage()).withPageSize(options.getLimit()).andTotal((int) result.getHits().getTotalHits()); diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseFormat.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseFormat.java index e55422e4b7d..fdfcd8ef618 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseFormat.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseFormat.java @@ -60,9 +60,11 @@ import org.sonarqube.ws.Issues.SearchWsResponse; import org.sonarqube.ws.Issues.Transitions; import org.sonarqube.ws.Issues.Users; +import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Strings.emptyToNull; import static com.google.common.base.Strings.nullToEmpty; +import static java.util.Collections.emptyList; import static org.sonar.api.rule.RuleKey.EXTERNAL_RULE_REPO_PREFIX; import static org.sonar.core.util.Protobuf.setNullable; @@ -302,11 +304,9 @@ public class SearchResponseFormat { private Common.Rules.Builder formatRules(SearchResponseData data) { Common.Rules.Builder wsRules = Common.Rules.newBuilder(); - List rules = data.getRules(); - if (rules != null) { - for (RuleDefinitionDto rule : rules) { - wsRules.addRules(commonFormat.formatRule(rule)); - } + List rules = firstNonNull(data.getRules(), emptyList()); + for (RuleDefinitionDto rule : rules) { + wsRules.addRules(commonFormat.formatRule(rule)); } return wsRules; } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseLoader.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseLoader.java index 9c793b935de..43fcfd09f76 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseLoader.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseLoader.java @@ -236,15 +236,15 @@ public class SearchResponseLoader { .stream() .filter(ComponentDto::isRootProject) .collect(MoreCollectors.uniqueIndex(ComponentDto::projectUuid)); - for (IssueDto dto : result.getIssues()) { + for (IssueDto issueDto : result.getIssues()) { // so that IssueDto can be used. if (collector.contains(ACTIONS)) { - ComponentDto project = componentsByProjectUuid.get(dto.getProjectUuid()); - result.addActions(dto.getKey(), listAvailableActions(dto, project)); + ComponentDto project = componentsByProjectUuid.get(issueDto.getProjectUuid()); + result.addActions(issueDto.getKey(), listAvailableActions(issueDto, project)); } - if (collector.contains(TRANSITIONS)) { + if (collector.contains(TRANSITIONS) && !issueDto.isExternal()) { // TODO workflow and action engines must not depend on org.sonar.api.issue.Issue but on a generic interface - DefaultIssue issue = dto.toDefaultIssue(); + DefaultIssue issue = issueDto.toDefaultIssue(); result.addTransitions(issue.key(), transitionService.listTransitions(issue)); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/DoTransitionActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/DoTransitionActionTest.java index a6cced6e61d..e724bd99d7f 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/DoTransitionActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/DoTransitionActionTest.java @@ -38,6 +38,7 @@ import org.sonar.db.rule.RuleDbTester; import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.db.rule.RuleDto; import org.sonar.server.es.EsTester; +import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.exceptions.UnauthorizedException; @@ -73,6 +74,7 @@ import static org.sonar.api.web.UserRole.CODEVIEWER; import static org.sonar.api.web.UserRole.USER; import static org.sonar.db.component.ComponentTesting.newFileDto; import static org.sonar.db.issue.IssueTesting.newDto; +import static org.sonar.db.rule.RuleTesting.newRule; import static org.sonar.db.rule.RuleTesting.newRuleDto; public class DoTransitionActionTest { @@ -187,6 +189,22 @@ public class DoTransitionActionTest { call("ISSUE_KEY", "confirm"); } + @Test + public void fail_if_external_issue() { + expectedException.expect(BadRequestException.class); + + RuleDefinitionDto rule = ruleDbTester.insert(newRule() + .setIsExternal(true)); + IssueDto issueDto = issueDbTester.insertIssue(newIssue() + .setStatus(STATUS_OPEN) + .setResolution(null) + .setRuleId(rule.getId()) + .setRuleKey(rule.getRuleKey(), rule.getRepositoryKey())); + userSession.logIn("john").addProjectPermission(USER, project, file); + + call(issueDto.getKey(), "confirm"); + } + private TestResponse call(@Nullable String issueKey, @Nullable String transition) { TestRequest request = tester.newRequest(); if (issueKey != null) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionTest.java index 8a866b9d327..8bd4ed073ea 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionTest.java @@ -189,7 +189,7 @@ public class SearchActionTest { ComponentDto project = insertComponent(ComponentTesting.newPublicProjectDto(otherOrganization2, "PROJECT_ID").setDbKey("PROJECT_KEY")); indexPermissions(); ComponentDto file = insertComponent(newFileDto(project, null, "FILE_ID").setDbKey("FILE_KEY")); - IssueDto issue = IssueTesting.newDto(newRule(), file, project) + IssueDto issue = IssueTesting.newDto(newExternalRule(), file, project) .setKee("82fd47d4-b650-4037-80bc-7b112bd4eac2") .setEffort(10L) .setLine(42) -- 2.39.5