From 9595eaec8b143b6f228fa63dbba7e5d202cda6fc Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Mon, 16 Dec 2019 17:27:55 +0100 Subject: [PATCH] SONAR-12720 api/hotspots/change_status requires HotspotAdmin permission --- .../server/hotspot/ws/AddCommentAction.java | 6 +- .../sonar/server/hotspot/ws/AssignAction.java | 2 +- .../server/hotspot/ws/ChangeStatusAction.java | 6 +- .../server/hotspot/ws/HotspotWsSupport.java | 5 +- .../sonar/server/hotspot/ws/ShowAction.java | 3 +- .../hotspot/ws/ChangeStatusActionTest.java | 92 +++++++++++++++---- .../resources/org/sonar/l10n/core.properties | 2 +- 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.
" + + "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.
" + + "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 -- 2.39.5