From: Sébastien Lesaint Date: Thu, 5 Dec 2019 13:15:34 +0000 (+0100) Subject: SONAR-12719 add WS api/hotspots/change_status X-Git-Tag: 8.2.0.32929~207 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=29a5dec5cff33ddb418d09a743b8f95999e0a867;p=sonarqube.git SONAR-12719 add WS api/hotspots/change_status --- diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/IssueWorkflow.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/IssueWorkflow.java index c8c56bb9dc6..0c666bc43f7 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/IssueWorkflow.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/IssueWorkflow.java @@ -36,6 +36,7 @@ import static com.google.common.base.Preconditions.checkState; import static org.sonar.api.issue.Issue.RESOLUTION_FALSE_POSITIVE; import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; import static org.sonar.api.issue.Issue.RESOLUTION_REMOVED; +import static org.sonar.api.issue.Issue.RESOLUTION_SAFE; import static org.sonar.api.issue.Issue.RESOLUTION_WONT_FIX; import static org.sonar.api.issue.Issue.STATUS_CLOSED; import static org.sonar.api.issue.Issue.STATUS_CONFIRMED; @@ -158,17 +159,42 @@ public class IssueWorkflow implements Startable { } private static void buildSecurityHotspotTransitions(StateMachine.Builder builder) { + // hotspot reviewed as fixed, either from TO_REVIEW or from REVIEWED-SAFE + Transition.TransitionBuilder reviewedAsFixedBuilder = Transition.builder(DefaultTransitions.RESOLVE_AS_REVIEWED) + .to(STATUS_REVIEWED) + .conditions(new HasType(RuleType.SECURITY_HOTSPOT)) + .functions(new SetResolution(RESOLUTION_FIXED)) + .requiredProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN); builder - .transition(Transition.builder(DefaultTransitions.RESOLVE_AS_REVIEWED) - .from(STATUS_TO_REVIEW).to(STATUS_REVIEWED) + .transition(reviewedAsFixedBuilder + .from(STATUS_TO_REVIEW) + .conditions(new HasType(RuleType.SECURITY_HOTSPOT)) + .build()) + .transition(reviewedAsFixedBuilder + .from(STATUS_REVIEWED) + .conditions(new HasType(RuleType.SECURITY_HOTSPOT), new HasResolution(RESOLUTION_SAFE)) + .build()); + + // hotspot reviewed as safe, either from TO_REVIEW or from REVIEWED-FIXED + Transition.TransitionBuilder resolveAsSafeTransitionBuilder = Transition.builder(DefaultTransitions.RESOLVE_AS_SAFE) + .to(STATUS_REVIEWED) + .functions(new SetResolution(RESOLUTION_SAFE)) + .requiredProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN); + builder + .transition(resolveAsSafeTransitionBuilder + .from(STATUS_TO_REVIEW) .conditions(new HasType(RuleType.SECURITY_HOTSPOT)) - .functions(new SetResolution(RESOLUTION_FIXED)) - .requiredProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN) .build()) + .transition(resolveAsSafeTransitionBuilder + .from(STATUS_REVIEWED) + .conditions(new HasType(RuleType.SECURITY_HOTSPOT), new HasResolution(RESOLUTION_FIXED)) + .build()); + // put hotspot back into TO_REVIEW + builder .transition(Transition.builder(DefaultTransitions.RESET_AS_TO_REVIEW) .from(STATUS_REVIEWED).to(STATUS_TO_REVIEW) - .conditions(new HasType(RuleType.SECURITY_HOTSPOT)) + .conditions(new HasType(RuleType.SECURITY_HOTSPOT), new HasResolution(RESOLUTION_FIXED, RESOLUTION_SAFE)) .functions(new SetResolution(null)) .requiredProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN) .build()); diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/IssueWorkflowForSecurityHotspotsTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/IssueWorkflowForSecurityHotspotsTest.java index 5b915f3f6b4..4979ecfcff5 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/IssueWorkflowForSecurityHotspotsTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/IssueWorkflowForSecurityHotspotsTest.java @@ -27,188 +27,298 @@ import java.util.Calendar; import java.util.Collection; import java.util.Date; import java.util.List; +import java.util.stream.Stream; +import javax.annotation.Nullable; import org.apache.commons.lang.time.DateUtils; import org.junit.Test; import org.junit.runner.RunWith; -import org.sonar.api.issue.DefaultTransitions; +import org.sonar.api.issue.Issue; import org.sonar.api.rule.RuleKey; -import org.sonar.api.rules.RuleType; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.FieldDiffs; import org.sonar.core.issue.IssueChangeContext; import org.sonar.core.util.stream.MoreCollectors; import org.sonar.server.issue.IssueFieldsSetter; +import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.api.issue.DefaultTransitions.RESET_AS_TO_REVIEW; +import static org.sonar.api.issue.DefaultTransitions.RESOLVE_AS_REVIEWED; +import static org.sonar.api.issue.DefaultTransitions.RESOLVE_AS_SAFE; import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; import static org.sonar.api.issue.Issue.RESOLUTION_REMOVED; +import static org.sonar.api.issue.Issue.RESOLUTION_SAFE; import static org.sonar.api.issue.Issue.STATUS_CLOSED; -import static org.sonar.api.issue.Issue.STATUS_RESOLVED; import static org.sonar.api.issue.Issue.STATUS_REVIEWED; import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW; +import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT; import static org.sonar.db.rule.RuleTesting.XOO_X1; @RunWith(DataProviderRunner.class) public class IssueWorkflowForSecurityHotspotsTest { - private static final String[] ALL_STATUSES_LEADING_TO_CLOSED = new String[] {STATUS_TO_REVIEW, STATUS_RESOLVED}; - - private static final String[] SUPPORTED_RESOLUTIONS_FOR_UNCLOSING = new String[] {RESOLUTION_FIXED, RESOLUTION_REMOVED}; + private static final IssueChangeContext SOME_CHANGE_CONTEXT = IssueChangeContext.createUser(new Date(), "USER1"); private IssueFieldsSetter updater = new IssueFieldsSetter(); private IssueWorkflow underTest = new IssueWorkflow(new FunctionExecutor(updater), updater); + @Test + @UseDataProvider("anyResolutionIncludingNone") + public void to_review_hotspot_with_any_resolution_can_be_resolved_as_safe_or_fixed(@Nullable String resolution) { + underTest.start(); + DefaultIssue hotspot = newHotspot(STATUS_TO_REVIEW, resolution); + + List transitions = underTest.outTransitions(hotspot); + + assertThat(keys(transitions)).containsExactlyInAnyOrder(RESOLVE_AS_REVIEWED, RESOLVE_AS_SAFE); + } + @DataProvider - public static Object[][] allStatusesLeadingToClosed() { - return Arrays.stream(ALL_STATUSES_LEADING_TO_CLOSED) + public static Object[][] anyResolutionIncludingNone() { + return Stream.of( + Issue.RESOLUTIONS.stream(), + Issue.SECURITY_HOTSPOT_RESOLUTIONS.stream(), + Stream.of(randomAlphabetic(12), null)) + .flatMap(t -> t) .map(t -> new Object[] {t}) .toArray(Object[][]::new); } - private static DefaultIssue newClosedIssue(String resolution) { - return new DefaultIssue() - .setKey("ABCDE") - .setRuleKey(RuleKey.of("js", "S001")) - .setResolution(resolution) - .setStatus(STATUS_CLOSED) - .setNew(false) - .setCloseDate(new Date(5_999_999L)); + @Test + public void reviewed_as_fixed_hotspot_can_be_resolved_as_safe_or_put_back_to_review() { + underTest.start(); + DefaultIssue hotspot = newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED); + + List transitions = underTest.outTransitions(hotspot); + + assertThat(keys(transitions)).containsExactlyInAnyOrder(RESOLVE_AS_SAFE, RESET_AS_TO_REVIEW); + } + + @Test + public void reviewed_as_safe_hotspot_can_be_resolved_as_fixed_or_put_back_to_review() { + underTest.start(); + DefaultIssue hotspot = newHotspot(STATUS_REVIEWED, RESOLUTION_SAFE); + + List transitions = underTest.outTransitions(hotspot); + + assertThat(keys(transitions)).containsExactlyInAnyOrder(RESOLVE_AS_REVIEWED, RESET_AS_TO_REVIEW); } - private static void setStatusPreviousToClosed(DefaultIssue issue, String previousStatus) { - addStatusChange(issue, new Date(), previousStatus, STATUS_CLOSED); + @Test + @UseDataProvider("anyResolutionButSafeOrFixed") + public void reviewed_with_any_resolution_but_safe_or_fixed_can_not_be_changed(String resolution) { + underTest.start(); + DefaultIssue hotspot = newHotspot(STATUS_REVIEWED, resolution); + + List transitions = underTest.outTransitions(hotspot); + + assertThat(transitions).isEmpty(); } - private static void addStatusChange(DefaultIssue issue, Date date, String previousStatus, String newStatus) { - issue.addChange(new FieldDiffs().setCreationDate(date).setDiff("status", previousStatus, newStatus)); + @DataProvider + public static Object[][] anyResolutionButSafeOrFixed() { + return Stream.of( + Issue.RESOLUTIONS.stream(), + Issue.SECURITY_HOTSPOT_RESOLUTIONS.stream(), + Stream.of(randomAlphabetic(12))) + .flatMap(t -> t) + .filter(t -> !RESOLUTION_FIXED.equals(t)) + .filter(t -> !RESOLUTION_SAFE.equals(t)) + .map(t -> new Object[] {t}) + .toArray(Object[][]::new); } @Test - public void list_out_transitions_in_status_to_review() { + public void doManualTransition_to_review_hostpot_is_resolved_as_fixed() { underTest.start(); - DefaultIssue issue = new DefaultIssue().setType(RuleType.SECURITY_HOTSPOT).setStatus(STATUS_TO_REVIEW); + DefaultIssue hotspot = newHotspot(STATUS_TO_REVIEW, null); - List transitions = underTest.outTransitions(issue); + boolean result = underTest.doManualTransition(hotspot, RESOLVE_AS_REVIEWED, SOME_CHANGE_CONTEXT); - assertThat(keys(transitions)).containsExactlyInAnyOrder("resolveasreviewed"); + assertThat(result).isTrue(); + assertThat(hotspot.getStatus()).isEqualTo(STATUS_REVIEWED); + assertThat(hotspot.resolution()).isEqualTo(RESOLUTION_FIXED); } @Test - public void list_out_transitions_in_status_reviewed() { + public void doManualTransition_reviewed_as_safe_hostpot_is_resolved_as_fixed() { underTest.start(); - DefaultIssue issue = new DefaultIssue().setType(RuleType.SECURITY_HOTSPOT).setStatus(STATUS_REVIEWED); + DefaultIssue hotspot = newHotspot(STATUS_REVIEWED, RESOLUTION_SAFE); - List transitions = underTest.outTransitions(issue); + boolean result = underTest.doManualTransition(hotspot, RESOLVE_AS_REVIEWED, SOME_CHANGE_CONTEXT); - assertThat(keys(transitions)).containsExactlyInAnyOrder("resetastoreview"); + assertThat(result).isTrue(); + assertThat(hotspot.getStatus()).isEqualTo(STATUS_REVIEWED); + assertThat(hotspot.resolution()).isEqualTo(RESOLUTION_FIXED); } @Test - public void resolve_as_reviewed_from_to_review() { + public void doManualTransition_to_review_hostpot_is_resolved_as_safe() { underTest.start(); - DefaultIssue issue = new DefaultIssue() - .setType(RuleType.SECURITY_HOTSPOT) - .setStatus(STATUS_TO_REVIEW); + DefaultIssue hotspot = newHotspot(STATUS_TO_REVIEW, null); - boolean result = underTest.doManualTransition(issue, DefaultTransitions.RESOLVE_AS_REVIEWED, IssueChangeContext.createUser(new Date(), "USER1")); + boolean result = underTest.doManualTransition(hotspot, RESOLVE_AS_SAFE, SOME_CHANGE_CONTEXT); assertThat(result).isTrue(); - assertThat(issue.getStatus()).isEqualTo(STATUS_REVIEWED); - assertThat(issue.resolution()).isEqualTo(RESOLUTION_FIXED); + assertThat(hotspot.getStatus()).isEqualTo(STATUS_REVIEWED); + assertThat(hotspot.resolution()).isEqualTo(RESOLUTION_SAFE); + } + + @Test + public void doManualTransition_reviewed_as_fixed_hostpot_is_resolved_as_safe() { + underTest.start(); + DefaultIssue hotspot = newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED); + + boolean result = underTest.doManualTransition(hotspot, RESOLVE_AS_SAFE, SOME_CHANGE_CONTEXT); + + assertThat(result).isTrue(); + assertThat(hotspot.getStatus()).isEqualTo(STATUS_REVIEWED); + assertThat(hotspot.resolution()).isEqualTo(RESOLUTION_SAFE); + } + + @Test + public void doManualTransition_reviewed_as_fixed_hostpot_is_put_back_to_review() { + underTest.start(); + DefaultIssue hotspot = newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED); + + boolean result = underTest.doManualTransition(hotspot, RESET_AS_TO_REVIEW, SOME_CHANGE_CONTEXT); + + assertThat(result).isTrue(); + assertThat(hotspot.getStatus()).isEqualTo(STATUS_TO_REVIEW); + assertThat(hotspot.resolution()).isNull(); + } + + @Test + public void doManualTransition_reviewed_as_safe_hostpot_is_put_back_to_review() { + underTest.start(); + DefaultIssue hotspot = newHotspot(STATUS_REVIEWED, RESOLUTION_SAFE); + + boolean result = underTest.doManualTransition(hotspot, RESET_AS_TO_REVIEW, SOME_CHANGE_CONTEXT); + + assertThat(result).isTrue(); + assertThat(hotspot.getStatus()).isEqualTo(STATUS_TO_REVIEW); + assertThat(hotspot.resolution()).isNull(); } @Test public void reset_as_to_review_from_reviewed() { underTest.start(); - DefaultIssue issue = new DefaultIssue() - .setType(RuleType.SECURITY_HOTSPOT) - .setStatus(STATUS_REVIEWED) - .setResolution(RESOLUTION_FIXED); + DefaultIssue hotspot = newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED); - boolean result = underTest.doManualTransition(issue, DefaultTransitions.RESET_AS_TO_REVIEW, IssueChangeContext.createUser(new Date(), "USER1")); + boolean result = underTest.doManualTransition(hotspot, RESET_AS_TO_REVIEW, SOME_CHANGE_CONTEXT); assertThat(result).isTrue(); - assertThat(issue.type()).isEqualTo(RuleType.SECURITY_HOTSPOT); - assertThat(issue.getStatus()).isEqualTo(STATUS_TO_REVIEW); - assertThat(issue.resolution()).isNull(); + assertThat(hotspot.type()).isEqualTo(SECURITY_HOTSPOT); + assertThat(hotspot.getStatus()).isEqualTo(STATUS_TO_REVIEW); + assertThat(hotspot.resolution()).isNull(); } @Test public void automatically_close_resolved_security_hotspots_in_status_to_review() { underTest.start(); - DefaultIssue issue = new DefaultIssue() - .setType(RuleType.SECURITY_HOTSPOT) - .setResolution(null) - .setStatus(STATUS_TO_REVIEW) + DefaultIssue hotspot = newHotspot(STATUS_TO_REVIEW, null) .setNew(false) .setBeingClosed(true); Date now = new Date(); - underTest.doAutomaticTransition(issue, IssueChangeContext.createScan(now)); + underTest.doAutomaticTransition(hotspot, IssueChangeContext.createScan(now)); - assertThat(issue.resolution()).isEqualTo(RESOLUTION_FIXED); - assertThat(issue.status()).isEqualTo(STATUS_CLOSED); - assertThat(issue.closeDate()).isNotNull(); - assertThat(issue.updateDate()).isEqualTo(DateUtils.truncate(now, Calendar.SECOND)); + assertThat(hotspot.resolution()).isEqualTo(RESOLUTION_FIXED); + assertThat(hotspot.status()).isEqualTo(STATUS_CLOSED); + assertThat(hotspot.closeDate()).isNotNull(); + assertThat(hotspot.updateDate()).isEqualTo(DateUtils.truncate(now, Calendar.SECOND)); } @Test - public void automatically_close_resolved_security_hotspots_in_status_reviewed() { + @UseDataProvider("safeOrFixedResolutions") + public void automatically_close_hotspot_resolved_as_fixed_or_safe(String resolution) { underTest.start(); - DefaultIssue issue = new DefaultIssue() - .setType(RuleType.SECURITY_HOTSPOT) - .setResolution(RESOLUTION_FIXED) - .setStatus(STATUS_REVIEWED) + DefaultIssue hotspot = newHotspot(STATUS_REVIEWED, resolution) .setNew(false) .setBeingClosed(true); Date now = new Date(); - underTest.doAutomaticTransition(issue, IssueChangeContext.createScan(now)); + underTest.doAutomaticTransition(hotspot, IssueChangeContext.createScan(now)); - assertThat(issue.resolution()).isEqualTo(RESOLUTION_FIXED); - assertThat(issue.status()).isEqualTo(STATUS_CLOSED); - assertThat(issue.closeDate()).isNotNull(); - assertThat(issue.updateDate()).isEqualTo(DateUtils.truncate(now, Calendar.SECOND)); + assertThat(hotspot.resolution()).isEqualTo(RESOLUTION_FIXED); + assertThat(hotspot.status()).isEqualTo(STATUS_CLOSED); + assertThat(hotspot.closeDate()).isNotNull(); + assertThat(hotspot.updateDate()).isEqualTo(DateUtils.truncate(now, Calendar.SECOND)); + } + + @DataProvider + public static Object[][] safeOrFixedResolutions() { + return new Object[][] { + {RESOLUTION_SAFE}, + {RESOLUTION_FIXED} + }; } @Test @UseDataProvider("allStatusesLeadingToClosed") public void do_not_automatically_reopen_closed_issues_of_security_hotspots(String previousStatus) { - DefaultIssue[] issues = Arrays.stream(SUPPORTED_RESOLUTIONS_FOR_UNCLOSING) + DefaultIssue[] hotspots = Stream.of(RESOLUTION_FIXED, RESOLUTION_REMOVED) .map(resolution -> { - DefaultIssue issue = newClosedIssue(resolution); + DefaultIssue issue = newClosedHotspot(resolution); setStatusPreviousToClosed(issue, previousStatus); - issue.setType(RuleType.SECURITY_HOTSPOT); return issue; }) .toArray(DefaultIssue[]::new); Date now = new Date(); underTest.start(); - Arrays.stream(issues).forEach(issue -> { - underTest.doAutomaticTransition(issue, IssueChangeContext.createScan(now)); + Arrays.stream(hotspots).forEach(hotspot -> { + underTest.doAutomaticTransition(hotspot, IssueChangeContext.createScan(now)); - assertThat(issue.status()).isEqualTo(STATUS_CLOSED); - assertThat(issue.updateDate()).isNull(); + assertThat(hotspot.status()).isEqualTo(STATUS_CLOSED); + assertThat(hotspot.updateDate()).isNull(); }); } + @DataProvider + public static Object[][] allStatusesLeadingToClosed() { + return Stream.of(STATUS_TO_REVIEW, STATUS_REVIEWED) + .map(t -> new Object[] {t}) + .toArray(Object[][]::new); + } + @Test public void doAutomaticTransition_does_nothing_on_security_hotspots_in_to_review_status() { - DefaultIssue issue = new DefaultIssue() + DefaultIssue hotspot = newHotspot(STATUS_TO_REVIEW, null) .setKey("ABCDE") - .setRuleKey(XOO_X1) - .setResolution(null) - .setStatus(STATUS_TO_REVIEW); + .setRuleKey(XOO_X1); underTest.start(); - underTest.doAutomaticTransition(issue, IssueChangeContext.createScan(new Date())); + underTest.doAutomaticTransition(hotspot, IssueChangeContext.createScan(new Date())); - assertThat(issue.status()).isEqualTo(STATUS_TO_REVIEW); - assertThat(issue.resolution()).isNull(); + assertThat(hotspot.status()).isEqualTo(STATUS_TO_REVIEW); + assertThat(hotspot.resolution()).isNull(); } private Collection keys(List transitions) { return transitions.stream().map(Transition::key).collect(MoreCollectors.toList()); } + + private static void setStatusPreviousToClosed(DefaultIssue hotspot, String previousStatus) { + addStatusChange(hotspot, new Date(), previousStatus, STATUS_CLOSED); + } + + private static void addStatusChange(DefaultIssue issue, Date date, String previousStatus, String newStatus) { + issue.addChange(new FieldDiffs().setCreationDate(date).setDiff("status", previousStatus, newStatus)); + } + + private static DefaultIssue newClosedHotspot(String resolution) { + return newHotspot(STATUS_CLOSED, resolution) + .setKey("ABCDE") + .setRuleKey(RuleKey.of("js", "S001")) + .setNew(false) + .setCloseDate(new Date(5_999_999L)); + } + + private static DefaultIssue newHotspot(String status, @Nullable String resolution) { + return new DefaultIssue() + .setType(SECURITY_HOTSPOT) + .setStatus(status) + .setResolution(resolution); + } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ChangeStatusAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ChangeStatusAction.java new file mode 100644 index 00000000000..3da4564254a --- /dev/null +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ChangeStatusAction.java @@ -0,0 +1,157 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 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.server.hotspot.ws; + +import java.util.Date; +import java.util.Objects; +import org.sonar.api.issue.DefaultTransitions; +import org.sonar.api.rules.RuleType; +import org.sonar.api.server.ws.Request; +import org.sonar.api.server.ws.Response; +import org.sonar.api.server.ws.WebService; +import org.sonar.api.utils.System2; +import org.sonar.api.web.UserRole; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.IssueChangeContext; +import org.sonar.core.util.Uuids; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.component.ComponentDto; +import org.sonar.db.issue.IssueDto; +import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.issue.TransitionService; +import org.sonar.server.issue.ws.IssueUpdater; +import org.sonar.server.user.UserSession; + +import static com.google.common.base.Preconditions.checkArgument; +import static java.lang.String.format; +import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; +import static org.sonar.api.issue.Issue.SECURITY_HOTSPOT_RESOLUTIONS; +import static org.sonar.api.issue.Issue.STATUS_REVIEWED; +import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW; + +public class ChangeStatusAction implements HotspotsWsAction { + + private static final String PARAM_HOTSPOT_KEY = "hotspot"; + private static final String PARAM_RESOLUTION = "resolution"; + private static final String PARAM_STATUS = "status"; + + private final DbClient dbClient; + private final UserSession userSession; + private final TransitionService transitionService; + private final System2 system2; + private final IssueUpdater issueUpdater; + + public ChangeStatusAction(DbClient dbClient, UserSession userSession, TransitionService transitionService, System2 system2, IssueUpdater issueUpdater) { + this.dbClient = dbClient; + this.userSession = userSession; + this.transitionService = transitionService; + this.system2 = system2; + this.issueUpdater = issueUpdater; + } + + @Override + public void define(WebService.NewController controller) { + WebService.NewAction action = controller + .createAction("change_status") + .setHandler(this) + .setPost(true) + .setDescription("Change the status of a Security Hotpot.") + .setSince("8.1") + .setInternal(true); + + action.createParam(PARAM_HOTSPOT_KEY) + .setDescription("Key of the Security Hotspot") + .setExampleValue(Uuids.UUID_EXAMPLE_03) + .setRequired(true); + action.createParam(PARAM_STATUS) + .setDescription("New status of the Security Hotspot.") + .setPossibleValues(STATUS_TO_REVIEW, STATUS_REVIEWED) + .setRequired(true); + action.createParam(PARAM_RESOLUTION) + .setDescription("Resolution of the Security Hotspot when new status is " + STATUS_REVIEWED + ", otherwise must not be set.") + .setPossibleValues(SECURITY_HOTSPOT_RESOLUTIONS); + } + + @Override + public void handle(Request request, Response response) throws Exception { + userSession.checkLoggedIn(); + + String hotspotKey = request.mandatoryParam(PARAM_HOTSPOT_KEY); + String newStatus = request.mandatoryParam(PARAM_STATUS); + String newResolution = resolutionParam(request, newStatus); + try (DbSession dbSession = dbClient.openSession(false)) { + IssueDto hotspot = dbClient.issueDao().selectByKey(dbSession, hotspotKey) + .filter(t -> t.getType() == RuleType.SECURITY_HOTSPOT.getDbConstant()) + .orElseThrow(() -> new NotFoundException(format("Hotspot '%s' does not exist", hotspotKey))); + loadAndCheckProject(dbSession, hotspot); + + if (needStatusUpdate(hotspot, newStatus, newResolution)) { + String transitionKey = toTransitionKey(newStatus, newResolution); + doTransition(dbSession, hotspot, transitionKey); + } + response.noContent(); + } + } + + private static String resolutionParam(Request request, String newStatus) { + String resolution = request.param(PARAM_RESOLUTION); + checkArgument(STATUS_REVIEWED.equals(newStatus) || resolution == null, + "Parameter '%s' must not be specified when Parameter '%s' has value '%s'", + PARAM_RESOLUTION, PARAM_STATUS, STATUS_TO_REVIEW); + checkArgument(STATUS_TO_REVIEW.equals(newStatus) || resolution != null, + "Parameter '%s' must be specified when Parameter '%s' has value '%s'", + PARAM_RESOLUTION, PARAM_STATUS, STATUS_REVIEWED); + return resolution; + } + + private void loadAndCheckProject(DbSession dbSession, IssueDto hotspot) { + String projectUuid = hotspot.getProjectUuid(); + checkArgument(projectUuid != null, "Hotspot '%s' has no project", hotspot.getKee()); + + ComponentDto project = dbClient.componentDao().selectByUuid(dbSession, projectUuid) + .orElseThrow(() -> new NotFoundException(format("Project with uuid '%s' does not exist", projectUuid))); + userSession.checkComponentPermission(UserRole.USER, project); + } + + private static boolean needStatusUpdate(IssueDto hotspot, String newStatus, String newResolution) { + return !(hotspot.getStatus().equals(newStatus) && Objects.equals(hotspot.getResolution(), newResolution)); + } + + private static String toTransitionKey(String newStatus, String newResolution) { + if (STATUS_TO_REVIEW.equals(newStatus)) { + return DefaultTransitions.RESET_AS_TO_REVIEW; + } + if (STATUS_REVIEWED.equals(newStatus) && RESOLUTION_FIXED.equals(newResolution)) { + return DefaultTransitions.RESOLVE_AS_REVIEWED; + } + return DefaultTransitions.RESOLVE_AS_SAFE; + } + + private void doTransition(DbSession session, IssueDto issueDto, String transitionKey) { + DefaultIssue defaultIssue = issueDto.toDefaultIssue(); + IssueChangeContext context = IssueChangeContext.createUser(new Date(system2.now()), userSession.getUuid()); + transitionService.checkTransitionPermission(transitionKey, defaultIssue); + if (transitionService.doTransition(defaultIssue, context, transitionKey)) { + issueUpdater.saveIssueAndPreloadSearchResponseData(session, defaultIssue, context, true); + } + } + +} diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotsWsModule.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotsWsModule.java index 2a437420b81..555ed0eec62 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotsWsModule.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotsWsModule.java @@ -28,6 +28,7 @@ public class HotspotsWsModule extends Module { HotspotWsResponseFormatter.class, SearchAction.class, ShowAction.class, + ChangeStatusAction.class, HotspotsWs.class); } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java index c17846dfbca..d67f119cbd7 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java @@ -82,6 +82,7 @@ import static java.util.function.Function.identity; import static java.util.stream.Collectors.toMap; import static org.sonar.api.issue.DefaultTransitions.OPEN_AS_VULNERABILITY; import static org.sonar.api.issue.DefaultTransitions.REOPEN; +import static org.sonar.api.issue.DefaultTransitions.RESOLVE_AS_REVIEWED; import static org.sonar.api.issue.DefaultTransitions.SET_AS_IN_REVIEW; import static org.sonar.api.rule.Severity.BLOCKER; import static org.sonar.api.rules.RuleType.BUG; @@ -145,8 +146,7 @@ public class BulkChangeAction implements IssuesWsAction { "Requires authentication.") .setSince("3.7") .setChangelog( - new Change("8.1", OPEN_AS_VULNERABILITY + " transition is no more supported"), - new Change("8.1", SET_AS_IN_REVIEW + " transition is no more supported"), + new Change("8.1", format("transitions '%s', '%s' and '%s' are no more supported", SET_AS_IN_REVIEW, RESOLVE_AS_REVIEWED, OPEN_AS_VULNERABILITY)), new Change("6.3", "'actions' parameter is ignored")) .setHandler(this) .setResponseExample(getClass().getResource("bulk_change-example.json")) diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/DoTransitionAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/DoTransitionAction.java index 7f65f6bbd05..c8eed8f4431 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/DoTransitionAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/DoTransitionAction.java @@ -75,8 +75,7 @@ public class DoTransitionAction implements IssuesWsAction { "The transitions involving security hotspots require the permission 'Administer Security Hotspot'.") .setSince("3.6") .setChangelog( - new Change("8.1", OPEN_AS_VULNERABILITY + " transition is no more supported"), - new Change("8.1", SET_AS_IN_REVIEW + " transition is no more supported"), + new Change("8.1", format("transitions '%s' and '%s' are no more supported", SET_AS_IN_REVIEW, OPEN_AS_VULNERABILITY)), new Change("7.8", format("added '%s', %s, %s and %s transitions for security hotspots ", SET_AS_IN_REVIEW, RESOLVE_AS_REVIEWED, OPEN_AS_VULNERABILITY, RESET_AS_TO_REVIEW)), new Change("7.3", "added transitions for security hotspots"), new Change("6.5", "the database ids of the components are removed from the response"), diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ChangeStatusActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ChangeStatusActionTest.java new file mode 100644 index 00000000000..de0b1162bf1 --- /dev/null +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ChangeStatusActionTest.java @@ -0,0 +1,480 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 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.server.hotspot.ws; + +import com.tngtech.java.junit.dataprovider.DataProvider; +import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import com.tngtech.java.junit.dataprovider.UseDataProvider; +import java.util.Arrays; +import java.util.Date; +import java.util.Objects; +import java.util.Random; +import java.util.function.Consumer; +import java.util.stream.Stream; +import javax.annotation.Nullable; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentMatcher; +import org.sonar.api.issue.DefaultTransitions; +import org.sonar.api.issue.Issue; +import org.sonar.api.rules.RuleType; +import org.sonar.api.utils.System2; +import org.sonar.api.web.UserRole; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.IssueChangeContext; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.DbTester; +import org.sonar.db.component.ComponentDto; +import org.sonar.db.issue.IssueDto; +import org.sonar.db.issue.IssueTesting; +import org.sonar.db.rule.RuleDefinitionDto; +import org.sonar.db.rule.RuleTesting; +import org.sonar.server.exceptions.ForbiddenException; +import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.exceptions.UnauthorizedException; +import org.sonar.server.issue.TransitionService; +import org.sonar.server.issue.ws.IssueUpdater; +import org.sonar.server.tester.UserSessionRule; +import org.sonar.server.ws.TestRequest; +import org.sonar.server.ws.WsActionTester; + +import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; +import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; +import static org.sonar.api.issue.Issue.RESOLUTION_SAFE; +import static org.sonar.api.issue.Issue.STATUS_REVIEWED; +import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW; +import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT; +import static org.sonar.db.component.ComponentTesting.newFileDto; + +@RunWith(DataProviderRunner.class) +public class ChangeStatusActionTest { + private static final Random RANDOM = new Random(); + + @Rule + public DbTester dbTester = DbTester.create(System2.INSTANCE); + @Rule + public UserSessionRule userSessionRule = UserSessionRule.standalone(); + + private DbClient dbClient = dbTester.getDbClient(); + private TransitionService transitionService = mock(TransitionService.class); + private IssueUpdater issueUpdater = mock(IssueUpdater.class); + private System2 system2 = mock(System2.class); + private ChangeStatusAction underTest = new ChangeStatusAction(dbClient, userSessionRule, transitionService, system2, issueUpdater); + private WsActionTester actionTester = new WsActionTester(underTest); + + @Test + public void ws_is_internal() { + assertThat(actionTester.getDef().isInternal()).isTrue(); + } + + @Test + public void fails_with_UnauthorizedException_if_user_is_anonymous() { + userSessionRule.anonymous(); + + TestRequest request = actionTester.newRequest(); + + assertThatThrownBy(request::execute) + .isInstanceOf(UnauthorizedException.class) + .hasMessage("Authentication is required"); + } + + @Test + public void fails_with_IAE_if_parameter_hotspot_is_missing() { + userSessionRule.logIn(); + TestRequest request = actionTester.newRequest(); + + assertThatThrownBy(request::execute) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("The 'hotspot' parameter is missing"); + } + + @Test + public void fails_with_IAE_if_parameter_status_is_missing() { + String key = randomAlphabetic(12); + userSessionRule.logIn(); + TestRequest request = actionTester.newRequest() + .setParam("hotspot", key); + + assertThatThrownBy(request::execute) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("The 'status' parameter is missing"); + } + + @Test + @UseDataProvider("badStatuses") + public void fail_with_IAE_if_status_value_is_neither_REVIEWED_nor_TO_REVIEW(String badStatus) { + String key = randomAlphabetic(12); + userSessionRule.logIn(); + TestRequest request = actionTester.newRequest() + .setParam("hotspot", key) + .setParam("status", badStatus); + + assertThatThrownBy(request::execute) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Value of parameter 'status' (" + badStatus + ") must be one of: [TO_REVIEW, REVIEWED]"); + } + + @DataProvider + public static Object[][] badStatuses() { + return Stream.concat( + Issue.STATUSES.stream() + .filter(t -> !t.equals(STATUS_TO_REVIEW)) + .filter(t -> !t.equals(STATUS_REVIEWED)), + Stream.of(randomAlphabetic(22), "")) + .map(t -> new Object[] {t}) + .toArray(Object[][]::new); + } + + @Test + @UseDataProvider("badResolutions") + public void fail_with_IAE_if_resolution_value_is_neither_FIXED_nor_SAFE(String validStatus, String badResolution) { + String key = randomAlphabetic(12); + userSessionRule.logIn(); + TestRequest request = actionTester.newRequest() + .setParam("hotspot", key) + .setParam("status", validStatus) + .setParam("resolution", badResolution); + + assertThatThrownBy(request::execute) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Value of parameter 'resolution' (" + badResolution + ") must be one of: [FIXED, SAFE]"); + } + + @DataProvider + public static Object[][] badResolutions() { + return Stream.of(STATUS_TO_REVIEW, STATUS_REVIEWED) + .flatMap(t -> Stream.concat(Issue.RESOLUTIONS.stream(), Issue.SECURITY_HOTSPOT_RESOLUTIONS.stream()) + .filter(r -> !r.equals(RESOLUTION_FIXED)) + .filter(r -> !r.equals(RESOLUTION_SAFE)) + .map(r -> new Object[] {t, r})) + .toArray(Object[][]::new); + } + + @Test + @UseDataProvider("validResolutions") + public void fail_with_IAE_if_status_is_TO_REVIEW_and_resolution_is_set(String resolution) { + String key = randomAlphabetic(12); + userSessionRule.logIn(); + TestRequest request = actionTester.newRequest() + .setParam("hotspot", key) + .setParam("status", STATUS_TO_REVIEW) + .setParam("resolution", resolution); + + assertThatThrownBy(request::execute) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Parameter 'resolution' must not be specified when Parameter 'status' has value 'TO_REVIEW'"); + } + + @DataProvider + public static Object[][] validResolutions() { + return new Object[][] { + {RESOLUTION_FIXED}, + {RESOLUTION_SAFE} + }; + } + + public void fail_with_IAE_if_status_is_RESOLVED_and_resolution_is_not_set() { + String key = randomAlphabetic(12); + userSessionRule.logIn(); + TestRequest request = actionTester.newRequest() + .setParam("hotspot", key) + .setParam("status", Issue.STATUS_RESOLVED); + + assertThatThrownBy(request::execute) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Parameter 'resolution' must not be specified when Parameter 'status' has value 'TO_REVIEW'"); + } + + @Test + @UseDataProvider("validStatusAndResolutions") + public void fails_with_NotFoundException_if_hotspot_does_not_exist(String status, @Nullable String resolution) { + String key = randomAlphabetic(12); + userSessionRule.logIn(); + TestRequest request = actionTester.newRequest() + .setParam("hotspot", key) + .setParam("status", status); + if (resolution != null) { + request.setParam("resolution", resolution); + } + + assertThatThrownBy(request::execute) + .isInstanceOf(NotFoundException.class) + .hasMessage("Hotspot '%s' does not exist", key); + } + + @DataProvider + public static Object[][] validStatusAndResolutions() { + return new Object[][] { + {STATUS_TO_REVIEW, null}, + {STATUS_REVIEWED, RESOLUTION_FIXED}, + {STATUS_REVIEWED, RESOLUTION_SAFE} + }; + } + + @Test + @UseDataProvider("ruleTypesButHotspot") + public void fails_with_NotFoundException_if_issue_is_not_a_hotspot(String status, @Nullable String resolution, RuleType ruleType) { + ComponentDto project = dbTester.components().insertPublicProject(); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(ruleType); + IssueDto notAHotspot = dbTester.issues().insertIssue(IssueTesting.newIssue(rule, project, file).setType(ruleType)); + userSessionRule.logIn(); + TestRequest request = newRequest(notAHotspot, status, resolution); + + assertThatThrownBy(request::execute) + .isInstanceOf(NotFoundException.class) + .hasMessage("Hotspot '%s' does not exist", notAHotspot.getKey()); + } + + @DataProvider + public static Object[][] ruleTypesButHotspot() { + return Arrays.stream(RuleType.values()) + .filter(t -> t != SECURITY_HOTSPOT) + .flatMap(t -> Arrays.stream(validStatusAndResolutions()).map(u -> new Object[] {u[0], u[1], t})) + .toArray(Object[][]::new); + } + + @Test + @UseDataProvider("validStatusAndResolutions") + public void fails_with_ForbiddenException_if_project_is_private_and_not_allowed(String status, @Nullable String resolution) { + ComponentDto project = dbTester.components().insertPrivateProject(); + userSessionRule.logIn().registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule)); + TestRequest request = newRequest(hotspot, status, resolution); + + assertThatThrownBy(request::execute) + .isInstanceOf(ForbiddenException.class) + .hasMessage("Insufficient privileges"); + } + + @Test + @UseDataProvider("validStatusAndResolutions") + public void succeeds_on_public_project(String status, @Nullable String resolution) { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.logIn().registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule)); + + newRequest(hotspot, status, resolution).execute().assertNoContent(); + } + + @Test + @UseDataProvider("validStatusAndResolutions") + public void succeeds_on_private_project_with_permission(String status, @Nullable String resolution) { + ComponentDto project = dbTester.components().insertPrivateProject(); + userSessionRule.logIn().registerComponents(project).addProjectPermission(UserRole.USER, project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule)); + + newRequest(hotspot, status, resolution).execute().assertNoContent(); + } + + @Test + @UseDataProvider("validStatusAndResolutions") + public void no_effect_and_success_if_hotspot_already_has_specified_status_and_resolution(String status, @Nullable String resolution) { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.logIn().registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule).setStatus(status).setResolution(resolution)); + + newRequest(hotspot, status, resolution).execute().assertNoContent(); + verifyNoInteractions(transitionService, issueUpdater); + } + + @Test + @UseDataProvider("reviewedResolutionsAndExpectedTransitionKey") + public void success_to_change_hostpot_to_review_into_reviewed_status(String resolution, String expectedTransitionKey, boolean transitionDone) { + long now = RANDOM.nextInt(232_323); + when(system2.now()).thenReturn(now); + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.logIn().registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule).setStatus(STATUS_TO_REVIEW).setResolution(null)); + when(transitionService.doTransition(any(), any(), any())).thenReturn(transitionDone); + + newRequest(hotspot, STATUS_REVIEWED, resolution).execute().assertNoContent(); + IssueChangeContext issueChangeContext = IssueChangeContext.createUser(new Date(now), userSessionRule.getUuid()); + DefaultIssueMatcher defaultIssueMatcher = new DefaultIssueMatcher(hotspot); + verify(transitionService).doTransition( + argThat(defaultIssueMatcher), + eq(issueChangeContext), + eq(expectedTransitionKey)); + if (transitionDone) { + verify(issueUpdater).saveIssueAndPreloadSearchResponseData( + any(DbSession.class), + argThat(defaultIssueMatcher), + eq(issueChangeContext), + eq(true)); + } else { + verifyNoInteractions(issueUpdater); + } + } + + @DataProvider + public static Object[][] reviewedResolutionsAndExpectedTransitionKey() { + return new Object[][] { + {RESOLUTION_FIXED, DefaultTransitions.RESOLVE_AS_REVIEWED, true}, + {RESOLUTION_FIXED, DefaultTransitions.RESOLVE_AS_REVIEWED, false}, + {RESOLUTION_SAFE, DefaultTransitions.RESOLVE_AS_SAFE, true}, + {RESOLUTION_SAFE, DefaultTransitions.RESOLVE_AS_SAFE, false} + }; + } + + @Test + @UseDataProvider("reviewedResolutions") + public void success_to_change_reviewed_hotspot_back_to_to_review(String resolution, boolean transitionDone) { + long now = RANDOM.nextInt(232_323); + when(system2.now()).thenReturn(now); + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.logIn().registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule).setStatus(STATUS_REVIEWED).setResolution(resolution)); + when(transitionService.doTransition(any(), any(), any())).thenReturn(transitionDone); + + newRequest(hotspot, STATUS_TO_REVIEW, null).execute().assertNoContent(); + IssueChangeContext issueChangeContext = IssueChangeContext.createUser(new Date(now), userSessionRule.getUuid()); + DefaultIssueMatcher defaultIssueMatcher = new DefaultIssueMatcher(hotspot); + verify(transitionService).doTransition( + argThat(defaultIssueMatcher), + eq(issueChangeContext), + eq(DefaultTransitions.RESET_AS_TO_REVIEW)); + if (transitionDone) { + verify(issueUpdater).saveIssueAndPreloadSearchResponseData( + any(DbSession.class), + argThat(defaultIssueMatcher), + eq(issueChangeContext), + eq(true)); + } else { + verifyNoInteractions(issueUpdater); + } + } + + @DataProvider + public static Object[][] reviewedResolutions() { + return new Object[][] { + {RESOLUTION_FIXED, true}, + {RESOLUTION_FIXED, false}, + {RESOLUTION_SAFE, true}, + {RESOLUTION_SAFE, false} + }; + } + + private static class DefaultIssueMatcher implements ArgumentMatcher { + private final DefaultIssue expected; + + private DefaultIssueMatcher(IssueDto issueDto) { + this.expected = issueDto.toDefaultIssue(); + } + + @Override + public boolean matches(DefaultIssue that) { + if (expected == that) { + return true; + } + if (that == null || expected.getClass() != that.getClass()) { + return false; + } + return expected.manualSeverity() == that.manualSeverity() && + expected.isFromExternalRuleEngine() == that.isFromExternalRuleEngine() && + expected.isNew() == that.isNew() && + expected.isCopied() == that.isCopied() && + expected.isBeingClosed() == that.isBeingClosed() && + expected.isOnDisabledRule() == that.isOnDisabledRule() && + expected.isChanged() == that.isChanged() && + expected.mustSendNotifications() == that.mustSendNotifications() && + Objects.equals(expected.key(), that.key()) && + expected.type() == that.type() && + Objects.equals(expected.componentUuid(), that.componentUuid()) && + Objects.equals(expected.componentKey(), that.componentKey()) && + Objects.equals(expected.moduleUuid(), that.moduleUuid()) && + Objects.equals(expected.moduleUuidPath(), that.moduleUuidPath()) && + Objects.equals(expected.projectUuid(), that.projectUuid()) && + Objects.equals(expected.projectKey(), that.projectKey()) && + Objects.equals(expected.ruleKey(), that.ruleKey()) && + Objects.equals(expected.language(), that.language()) && + Objects.equals(expected.severity(), that.severity()) && + Objects.equals(expected.message(), that.message()) && + Objects.equals(expected.line(), that.line()) && + Objects.equals(expected.gap(), that.gap()) && + Objects.equals(expected.effort(), that.effort()) && + Objects.equals(expected.status(), that.status()) && + Objects.equals(expected.resolution(), that.resolution()) && + Objects.equals(expected.assignee(), that.assignee()) && + Objects.equals(expected.checksum(), that.checksum()) && + Objects.equals(expected.attributes(), that.attributes()) && + Objects.equals(expected.authorLogin(), that.authorLogin()) && + Objects.equals(expected.comments(), that.comments()) && + Objects.equals(expected.tags(), that.tags()) && + Objects.equals(expected.getLocations(), that.getLocations()) && + Objects.equals(expected.creationDate(), that.creationDate()) && + Objects.equals(expected.updateDate(), that.updateDate()) && + Objects.equals(expected.closeDate(), that.closeDate()) && + Objects.equals(expected.currentChange(), that.currentChange()) && + Objects.equals(expected.changes(), that.changes()) && + Objects.equals(expected.selectedAt(), that.selectedAt()); + } + } + + private static IssueDto newHotspot(ComponentDto project, ComponentDto file, RuleDefinitionDto rule) { + return IssueTesting.newIssue(rule, project, file).setType(SECURITY_HOTSPOT); + } + + private TestRequest newRequest(IssueDto hotspot, String newStatus, @Nullable String newResolution) { + TestRequest res = actionTester.newRequest() + .setParam("hotspot", hotspot.getKey()) + .setParam("status", newStatus); + if (newResolution != null) { + res.setParam("resolution", newResolution); + } + return res; + } + + private RuleDefinitionDto newRule(RuleType ruleType) { + return newRule(ruleType, t -> { + }); + } + + private RuleDefinitionDto newRule(RuleType ruleType, Consumer populate) { + RuleDefinitionDto ruleDefinition = RuleTesting.newRule() + .setType(ruleType); + populate.accept(ruleDefinition); + dbTester.rules().insert(ruleDefinition); + return ruleDefinition; + } + +} diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/HotspotsWsModuleTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/HotspotsWsModuleTest.java index a9c839b56df..dea04b193f0 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/HotspotsWsModuleTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/HotspotsWsModuleTest.java @@ -30,7 +30,7 @@ public class HotspotsWsModuleTest { public void verify_count_of_added_components() { ComponentContainer container = new ComponentContainer(); new HotspotsWsModule().configure(container); - assertThat(container.size()).isEqualTo(COMPONENTS_IN_EMPTY_COMPONENT_CONTAINER + 4); + assertThat(container.size()).isEqualTo(COMPONENTS_IN_EMPTY_COMPONENT_CONTAINER + 5); } } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/DefaultTransitions.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/DefaultTransitions.java index 58279aa676e..a371baef2b7 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/issue/DefaultTransitions.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/DefaultTransitions.java @@ -48,15 +48,20 @@ public interface DefaultTransitions { /** * @since 7.8 + * @deprecated since 8.1, security hotspots can no longer be converted to vulnerabilities */ - String RESOLVE_AS_REVIEWED = "resolveasreviewed"; + @Deprecated + String OPEN_AS_VULNERABILITY = "openasvulnerability"; /** * @since 7.8 - * @deprecated since 8.1, security hotspots can no longer be converted to vulnerabilities */ - @Deprecated - String OPEN_AS_VULNERABILITY = "openasvulnerability"; + String RESOLVE_AS_REVIEWED = "resolveasreviewed"; + + /** + * @since 8.1 + */ + String RESOLVE_AS_SAFE = "resolveassafe"; /** * @since 7.8 diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issue.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issue.java index 6c831ca61f6..687973fa054 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issue.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issue.java @@ -74,8 +74,16 @@ public interface Issue extends Serializable { */ String RESOLUTION_WONT_FIX = "WONTFIX"; + /** + * Security Hotspot has been reviewed and resolved as safe. + * @since 8.1 + */ + String RESOLUTION_SAFE = "SAFE"; + List RESOLUTIONS = unmodifiableList(asList(RESOLUTION_FALSE_POSITIVE, RESOLUTION_WONT_FIX, RESOLUTION_FIXED, RESOLUTION_REMOVED)); + List SECURITY_HOTSPOT_RESOLUTIONS = unmodifiableList(asList(RESOLUTION_FIXED, RESOLUTION_SAFE)); + /** * @since 7.8 */