]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-12720 api/hotspots/change_status requires HotspotAdmin permission
authorSébastien Lesaint <sebastien.lesaint@sonarsource.com>
Mon, 16 Dec 2019 16:27:55 +0000 (17:27 +0100)
committerSonarTech <sonartech@sonarsource.com>
Mon, 13 Jan 2020 19:46:29 +0000 (20:46 +0100)
server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/AddCommentAction.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/AssignAction.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ChangeStatusAction.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotWsSupport.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ShowAction.java
server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ChangeStatusActionTest.java
sonar-core/src/main/resources/org/sonar/l10n/core.properties

index eaf476c1e3e1ab62011597fdcde16cb99a4fff57..4a2be100abc84d8087ec458ed11ee65ec507db98 100644 (file)
@@ -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();
index 55be25702d413a1e5efccd3d718229376376ebe5..6a9467171a18f4243b02a5723b8fb1647c64bfc7 100644 (file)
@@ -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();
index ffb8048b85a8af0a6714c9aad0904683ca0ceb54..8e0ca3a2e1430e1a5dcf76e35f22684d6a603456 100644 (file)
@@ -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);
index c4e1b2f7709b9cf0b00e583e800ba92b226b16e2..8420f32ceefbfb60ac12668d3f23548798d49abe 100644 (file)
@@ -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;
   }
index d2d5085764ab3119bb82f2e50635bab71be8e61b..7b5c3f5c1e5cde0337c05daf1cdc19908c3b623a 100644 (file)
@@ -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);
index a6192e51885e23c953d05beadb415ea7f1838314..eb613ae5bbb3ce840e1a11a88188f233f2904122 100644 (file)
@@ -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));
index 34fb7b43a10835943a4f45097691dfe62e6243e4..3ddc3dce610f7eff471cc33264e90ea62ac07edc 100644 (file)
@@ -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