SONAR-12720 api/hotspots/change_status requires HotspotAdmin permission

7 files changed, 90 insertions, 26 deletions

diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/AddCommentAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/AddCommentAction.java
index eaf476c1e3e..4a2be100abc 100644
--- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/AddCommentAction.java
+++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/AddCommentAction.java
@@ -22,6 +22,7 @@ package org.sonar.server.hotspot.ws;
 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.web.UserRole;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.IssueChangeContext;
 import org.sonar.core.util.Uuids;
@@ -54,7 +55,8 @@ public class AddCommentAction implements HotspotsWsAction {
       .createAction("add_comment")
       .setHandler(this)
       .setPost(true)
-      .setDescription("Add a comment to a Security Hotpot.")
+      .setDescription("Add a comment to a Security Hotpot.<br/>" +
+        "Requires authentication and the following permission: 'Browse' on the project of the specified Security Hotspot.")
       .setSince("8.1")
       .setInternal(true);
 
@@ -76,7 +78,7 @@ public class AddCommentAction implements HotspotsWsAction {
     String comment = request.mandatoryParam(PARAM_COMMENT);
     try (DbSession dbSession = dbClient.openSession(false)) {
       IssueDto hotspot = hotspotWsSupport.loadHotspot(dbSession, hotspotKey);
-      hotspotWsSupport.loadAndCheckProject(dbSession, hotspot);
+      hotspotWsSupport.loadAndCheckProject(dbSession, hotspot, UserRole.USER);
 
       DefaultIssue defaultIssue = hotspot.toDefaultIssue();
       IssueChangeContext context = hotspotWsSupport.newIssueChangeContext();
diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/AssignAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/AssignAction.java
index 55be25702d4..6a9467171a1 100644
--- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/AssignAction.java
+++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/AssignAction.java
@@ -100,7 +100,7 @@ public class AssignAction implements HotspotsWsAction {
       IssueDto hotspotDto = hotspotWsSupport.loadHotspot(dbSession, hotspotKey);
 
       checkIfHotspotToReview(hotspotDto);
-      hotspotWsSupport.loadAndCheckProject(dbSession, hotspotDto);
+      hotspotWsSupport.loadAndCheckProject(dbSession, hotspotDto, UserRole.USER);
       UserDto assignee = getAssignee(dbSession, login);
 
       IssueChangeContext context = hotspotWsSupport.newIssueChangeContext();
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
index ffb8048b85a..8e0ca3a2e14 100644
--- 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
@@ -25,6 +25,7 @@ import org.sonar.api.issue.DefaultTransitions;
 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.web.UserRole;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.IssueChangeContext;
 import org.sonar.core.util.Uuids;
@@ -70,7 +71,8 @@ public class ChangeStatusAction implements HotspotsWsAction {
       .createAction("change_status")
       .setHandler(this)
       .setPost(true)
-      .setDescription("Change the status of a Security Hotpot.")
+      .setDescription("Change the status of a Security Hotpot.<br/>" +
+        "Requires the 'Administer Security Hotspot' permission.")
       .setSince("8.1")
       .setInternal(true);
 
@@ -99,7 +101,7 @@ public class ChangeStatusAction implements HotspotsWsAction {
     String newResolution = resolutionParam(request, newStatus);
     try (DbSession dbSession = dbClient.openSession(false)) {
       IssueDto hotspot = hotspotWsSupport.loadHotspot(dbSession, hotspotKey);
-      hotspotWsSupport.loadAndCheckProject(dbSession, hotspot);
+      hotspotWsSupport.loadAndCheckProject(dbSession, hotspot, UserRole.SECURITYHOTSPOT_ADMIN);
 
       if (needStatusUpdate(hotspot, newStatus, newResolution)) {
         String transitionKey = toTransitionKey(newStatus, newResolution);
diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotWsSupport.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotWsSupport.java
index c4e1b2f7709..8420f32ceef 100644
--- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotWsSupport.java
+++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotWsSupport.java
@@ -22,7 +22,6 @@ package org.sonar.server.hotspot.ws;
 import java.util.Date;
 import org.sonar.api.rules.RuleType;
 import org.sonar.api.utils.System2;
-import org.sonar.api.web.UserRole;
 import org.sonar.core.issue.IssueChangeContext;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
@@ -55,13 +54,13 @@ public class HotspotWsSupport {
       .orElseThrow(() -> new NotFoundException(format("Hotspot '%s' does not exist", hotspotKey)));
   }
 
-  ComponentDto loadAndCheckProject(DbSession dbSession, IssueDto hotspot) {
+  ComponentDto loadAndCheckProject(DbSession dbSession, IssueDto hotspot, String userRole) {
     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);
+    userSession.checkComponentPermission(userRole, project);
 
     return project;
   }
diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ShowAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ShowAction.java
index d2d5085764a..7b5c3f5c1e5 100644
--- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ShowAction.java
+++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ShowAction.java
@@ -29,6 +29,7 @@ import org.sonar.api.rule.RuleKey;
 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.web.UserRole;
 import org.sonar.core.util.Uuids;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
@@ -192,7 +193,7 @@ public class ShowAction implements HotspotsWsAction {
   private Components loadComponents(DbSession dbSession, IssueDto hotspot) {
     String componentUuid = hotspot.getComponentUuid();
 
-    ComponentDto project = hotspotWsSupport.loadAndCheckProject(dbSession, hotspot);
+    ComponentDto project = hotspotWsSupport.loadAndCheckProject(dbSession, hotspot, UserRole.USER);
 
     checkArgument(componentUuid != null, "Hotspot '%s' has no component", hotspot.getKee());
     boolean hotspotOnProject = Objects.equals(project.uuid(), componentUuid);
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
index a6192e51885..eb613ae5bbb 100644
--- 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
@@ -267,25 +267,75 @@ public class ChangeStatusActionTest {
   }
 
   @Test
-  @UseDataProvider("validStatusAndResolutions")
-  public void fails_with_ForbiddenException_if_project_is_private_and_not_allowed(String status, @Nullable String resolution) {
+  @UseDataProvider("anyPublicProjectPermissionButHotspotAdmin")
+  public void fails_with_ForbiddenException_if_project_is_public_and_user_has_no_HotspotAdmin_permission_on_it(String permission) {
+    ComponentDto project = dbTester.components().insertPublicProject();
+    userSessionRule.logIn().registerComponents(project)
+      .addProjectPermission(permission, project);
+    ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
+    RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT);
+    IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule));
+
+    Arrays.stream(validStatusAndResolutions())
+      .forEach(o -> {
+        String status = (String) o[0];
+        String resolution = (String) o[1];
+
+        TestRequest request = newRequest(hotspot, status, resolution, NO_COMMENT);
+        assertThatThrownBy(request::execute)
+          .isInstanceOf(ForbiddenException.class)
+          .hasMessage("Insufficient privileges");
+      });
+  }
+
+  @DataProvider
+  public static Object[][] anyPublicProjectPermissionButHotspotAdmin() {
+    return new Object[][] {
+      {UserRole.ADMIN},
+      {UserRole.ISSUE_ADMIN},
+      {UserRole.SCAN}
+    };
+  }
+
+  @Test
+  @UseDataProvider("anyPrivateProjectPermissionButHotspotAdmin")
+  public void fails_with_ForbiddenException_if_project_is_private_and_has_no_IssueAdmin_permission_on_it(String permission) {
     ComponentDto project = dbTester.components().insertPrivateProject();
-    userSessionRule.logIn().registerComponents(project);
+    userSessionRule.logIn().registerComponents(project)
+      .addProjectPermission(permission, 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, NO_COMMENT);
 
-    assertThatThrownBy(request::execute)
-      .isInstanceOf(ForbiddenException.class)
-      .hasMessage("Insufficient privileges");
+    Arrays.stream(validStatusAndResolutions())
+      .forEach(o -> {
+        String status = (String) o[0];
+        String resolution = (String) o[1];
+
+        TestRequest request = newRequest(hotspot, status, resolution, NO_COMMENT);
+        assertThatThrownBy(request::execute)
+          .isInstanceOf(ForbiddenException.class)
+          .hasMessage("Insufficient privileges");
+      });
+  }
+
+  @DataProvider
+  public static Object[][] anyPrivateProjectPermissionButHotspotAdmin() {
+    return new Object[][] {
+      {UserRole.USER},
+      {UserRole.ADMIN},
+      {UserRole.ISSUE_ADMIN},
+      {UserRole.CODEVIEWER},
+      {UserRole.SCAN}
+    };
   }
 
   @Test
   @UseDataProvider("validStatusAndResolutions")
-  public void succeeds_on_public_project(String status, @Nullable String resolution) {
+  public void succeeds_on_public_project_with_HotspotAdmin_permission(String status, @Nullable String resolution) {
     ComponentDto project = dbTester.components().insertPublicProject();
-    userSessionRule.logIn().registerComponents(project);
+    userSessionRule.logIn().registerComponents(project)
+      .addProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN, project);
     ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
     RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT);
     IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule));
@@ -295,9 +345,10 @@ public class ChangeStatusActionTest {
 
   @Test
   @UseDataProvider("validStatusAndResolutions")
-  public void succeeds_on_private_project_with_permission(String status, @Nullable String resolution) {
+  public void succeeds_on_private_project_with_HotspotAdmin_permission(String status, @Nullable String resolution) {
     ComponentDto project = dbTester.components().insertPrivateProject();
-    userSessionRule.logIn().registerComponents(project).addProjectPermission(UserRole.USER, project);
+    userSessionRule.logIn().registerComponents(project)
+      .addProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN, project);
     ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
     RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT);
     IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule));
@@ -309,7 +360,8 @@ public class ChangeStatusActionTest {
   @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);
+    userSessionRule.logIn().registerComponents(project)
+      .addProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN, 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));
@@ -325,7 +377,9 @@ public class ChangeStatusActionTest {
     long now = RANDOM.nextInt(232_323);
     when(system2.now()).thenReturn(now);
     ComponentDto project = dbTester.components().insertPublicProject();
-    userSessionRule.logIn().registerComponents(project);
+    userSessionRule.logIn().registerComponents(project)
+      .addProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN, 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));
@@ -371,7 +425,9 @@ public class ChangeStatusActionTest {
     long now = RANDOM.nextInt(232_323);
     when(system2.now()).thenReturn(now);
     ComponentDto project = dbTester.components().insertPublicProject();
-    userSessionRule.logIn().registerComponents(project);
+    userSessionRule.logIn().registerComponents(project)
+      .addProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN, 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));
@@ -418,7 +474,9 @@ public class ChangeStatusActionTest {
     long now = RANDOM.nextInt(232_323);
     when(system2.now()).thenReturn(now);
     ComponentDto project = dbTester.components().insertPublicProject();
-    userSessionRule.logIn().registerComponents(project);
+    userSessionRule.logIn().registerComponents(project)
+      .addProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN, project);
+    ;
     ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
     RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT);
     IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule).setStatus(currentStatus).setResolution(currentResolution));
@@ -469,7 +527,9 @@ public class ChangeStatusActionTest {
     long now = RANDOM.nextInt(232_323);
     when(system2.now()).thenReturn(now);
     ComponentDto project = dbTester.components().insertPublicProject();
-    userSessionRule.logIn().registerComponents(project);
+    userSessionRule.logIn().registerComponents(project)
+      .addProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN, 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));
diff --git a/sonar-core/src/main/resources/org/sonar/l10n/core.properties b/sonar-core/src/main/resources/org/sonar/l10n/core.properties
index 34fb7b43a10..3ddc3dce610 100644
--- a/sonar-core/src/main/resources/org/sonar/l10n/core.properties
+++ b/sonar-core/src/main/resources/org/sonar/l10n/core.properties
@@ -2286,7 +2286,7 @@ projects_role.admin.desc=Access project settings and perform administration task
 projects_role.issueadmin=Administer Issues
 projects_role.issueadmin.desc=Change the type and severity of issues, resolve issues as being "fixed", "won't fix" or "false-positive" (users also need "Browse" permission).
 projects_role.securityhotspotadmin=Administer Security Hotspots
-projects_role.securityhotspotadmin.desc=Open a Vulnerability from a Security Hotspot. Resolved a Security Hotspot as reviewed, set it as in review or reset it as to review (users also need Browse permission).
+projects_role.securityhotspotadmin.desc=Resolve a Security Hotspot as reviewed (fixed or safe), reset it as to review (users also need Browse permission).
 projects_role.user=Browse
 projects_role.user.desc=Access a project, browse its measures and issues, confirm issues, change the assignee, comment on issues and change tags.
 projects_role.codeviewer=See Source Code