From ac411892a8fd09a4a422218efafb6f9f233a7f90 Mon Sep 17 00:00:00 2001 From: Teryk Bellahsene Date: Wed, 4 Oct 2017 18:03:14 +0200 Subject: [PATCH] SONAR-9445 Permission 'Administer Quality Gates' is mandatory and sufficient to delete a QG condition --- .../server/qualitygate/QualityGateModule.java | 2 + .../server/qualitygate/QualityGates.java | 16 --- .../qualitygate/ws/DeleteConditionAction.java | 34 +++-- .../qualitygate/ws/QualityGatesWsSupport.java | 51 +++++++ .../qualitygate/QualityGateModuleTest.java | 2 +- .../server/qualitygate/QualityGatesTest.java | 15 -- .../ws/DeleteConditionActionTest.java | 131 ++++++++++++++++++ .../qualitygate/ws/QualityGatesWsTest.java | 19 +-- 8 files changed, 211 insertions(+), 59 deletions(-) create mode 100644 server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/QualityGatesWsSupport.java create mode 100644 server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/DeleteConditionActionTest.java diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGateModule.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGateModule.java index 45d5c94045b..6e97f2355f1 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGateModule.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGateModule.java @@ -31,6 +31,7 @@ import org.sonar.server.qualitygate.ws.GetByProjectAction; import org.sonar.server.qualitygate.ws.ListAction; import org.sonar.server.qualitygate.ws.ProjectStatusAction; import org.sonar.server.qualitygate.ws.QualityGatesWs; +import org.sonar.server.qualitygate.ws.QualityGatesWsSupport; import org.sonar.server.qualitygate.ws.RenameAction; import org.sonar.server.qualitygate.ws.SearchAction; import org.sonar.server.qualitygate.ws.SelectAction; @@ -49,6 +50,7 @@ public class QualityGateModule extends Module { QgateProjectFinder.class, QualityGateFinder.class, // WS + QualityGatesWsSupport.class, QualityGatesWs.class, ListAction.class, SearchAction.class, diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGates.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGates.java index 085db85ca0b..b21c82cd664 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGates.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGates.java @@ -170,14 +170,6 @@ public class QualityGates { } } - public void deleteCondition(Long condId) { - checkIsSystemAdministrator(); - try (DbSession dbSession = dbClient.openSession(false)) { - conditionDao.delete(getNonNullCondition(dbSession, condId), dbSession); - dbSession.commit(); - } - } - public void dissociateProject(DbSession dbSession, ComponentDto project) { checkProjectAdmin(project); propertiesDao.deleteProjectProperty(SONAR_QUALITYGATE_PROPERTY, project.getId(), dbSession); @@ -220,14 +212,6 @@ public class QualityGates { } } - private QualityGateConditionDto getNonNullCondition(DbSession dbSession, long id) { - QualityGateConditionDto condition = conditionDao.selectById(id, dbSession); - if (condition == null) { - throw new NotFoundException("There is no condition with id=" + id); - } - return condition; - } - private void validateQualityGate(DbSession dbSession, @Nullable Long updatingQgateId, @Nullable String name) { List errors = new ArrayList<>(); if (Strings.isNullOrEmpty(name)) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/DeleteConditionAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/DeleteConditionAction.java index 01f702add1b..afb3d91eb85 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/DeleteConditionAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/DeleteConditionAction.java @@ -22,27 +22,37 @@ package org.sonar.server.qualitygate.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.server.qualitygate.QualityGates; -import org.sonarqube.ws.client.qualitygate.QualityGatesWsParameters; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.organization.OrganizationDto; +import org.sonar.server.user.UserSession; + +import static org.sonar.db.permission.OrganizationPermission.ADMINISTER_QUALITY_GATES; +import static org.sonarqube.ws.client.qualitygate.QualityGatesWsParameters.PARAM_ID; public class DeleteConditionAction implements QualityGatesWsAction { - private final QualityGates qualityGates; + private final DbClient dbClient; + private final UserSession userSession; + private final QualityGatesWsSupport wsSupport; - public DeleteConditionAction(QualityGates qualityGates) { - this.qualityGates = qualityGates; + public DeleteConditionAction(UserSession userSession, DbClient dbClient, QualityGatesWsSupport wsSupport) { + this.userSession = userSession; + this.dbClient = dbClient; + this.wsSupport = wsSupport; } @Override public void define(WebService.NewController controller) { WebService.NewAction createCondition = controller.createAction("delete_condition") - .setDescription("Delete a condition from a quality gate. Require Administer Quality Gates permission") + .setDescription("Delete a condition from a quality gate.
" + + "Requires the 'Administer Quality Gates' permission") .setPost(true) .setSince("4.3") .setHandler(this); createCondition - .createParam(QualityGatesWsParameters.PARAM_ID) + .createParam(PARAM_ID) .setRequired(true) .setDescription("Condition ID") .setExampleValue("2"); @@ -50,8 +60,14 @@ public class DeleteConditionAction implements QualityGatesWsAction { @Override public void handle(Request request, Response response) { - qualityGates.deleteCondition(QualityGatesWs.parseId(request, QualityGatesWsParameters.PARAM_ID)); - response.noContent(); + long conditionId = request.mandatoryParamAsLong(PARAM_ID); + try (DbSession dbSession = dbClient.openSession(false)) { + OrganizationDto organization = wsSupport.getOrganization(dbSession); + userSession.checkPermission(ADMINISTER_QUALITY_GATES, organization); + dbClient.gateConditionDao().delete(wsSupport.getCondition(dbSession, conditionId), dbSession); + dbSession.commit(); + response.noContent(); + } } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/QualityGatesWsSupport.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/QualityGatesWsSupport.java new file mode 100644 index 00000000000..f6e945b3783 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/QualityGatesWsSupport.java @@ -0,0 +1,51 @@ +/* + * SonarQube + * Copyright (C) 2009-2017 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.qualitygate.ws; + +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.organization.OrganizationDto; +import org.sonar.db.qualitygate.QualityGateConditionDto; +import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.organization.DefaultOrganizationProvider; + +import static java.lang.String.format; +import static org.sonar.server.ws.WsUtils.checkFound; + +public class QualityGatesWsSupport { + private final DbClient dbClient; + private final DefaultOrganizationProvider organizationProvider; + + public QualityGatesWsSupport(DbClient dbClient, DefaultOrganizationProvider organizationProvider) { + this.dbClient = dbClient; + this.organizationProvider = organizationProvider; + } + + QualityGateConditionDto getCondition(DbSession dbSession, long id) { + return checkFound(dbClient.gateConditionDao().selectById(id, dbSession), "No quality gate condition with id '%d'", id); + } + + OrganizationDto getOrganization(DbSession dbSession) { + String organizationKey = organizationProvider.get().getKey(); + return dbClient.organizationDao().selectByKey(dbSession, organizationKey) + .orElseThrow(() -> new NotFoundException(format("No organization with key '%s'", organizationKey))); + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGateModuleTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGateModuleTest.java index 18a06580b42..622d9fc6236 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGateModuleTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGateModuleTest.java @@ -29,6 +29,6 @@ public class QualityGateModuleTest { public void verify_count_of_added_components() { ComponentContainer container = new ComponentContainer(); new QualityGateModule().configure(container); - assertThat(container.size()).isEqualTo(23 + 2); + assertThat(container.size()).isEqualTo(24 + 2); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGatesTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGatesTest.java index fca5ac6eda7..16a67be383f 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGatesTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGatesTest.java @@ -284,21 +284,6 @@ public class QualityGatesTest { underTest.listConditions(qGateId); } - @Test - public void should_delete_condition() { - long idToDelete = QUALITY_GATE_ID; - QualityGateConditionDto toDelete = new QualityGateConditionDto().setId(idToDelete); - when(conditionDao.selectById(idToDelete, dbSession)).thenReturn(toDelete); - underTest.deleteCondition(idToDelete); - verify(conditionDao).selectById(idToDelete, dbSession); - verify(conditionDao).delete(toDelete, dbSession); - } - - @Test(expected = NotFoundException.class) - public void should_fail_delete_condition() { - underTest.deleteCondition(QUALITY_GATE_ID); - } - @Test public void should_copy_qgate() { String name = "Atlantis"; diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/DeleteConditionActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/DeleteConditionActionTest.java new file mode 100644 index 00000000000..1dd44cefb04 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/DeleteConditionActionTest.java @@ -0,0 +1,131 @@ +/* + * SonarQube + * Copyright (C) 2009-2017 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.qualitygate.ws; + +import java.util.Collection; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.sonar.api.server.ws.WebService; +import org.sonar.api.server.ws.WebService.Param; +import org.sonar.db.DbTester; +import org.sonar.db.metric.MetricDto; +import org.sonar.db.qualitygate.QualityGateConditionDto; +import org.sonar.db.qualitygate.QualityGateDto; +import org.sonar.server.exceptions.ForbiddenException; +import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.organization.TestDefaultOrganizationProvider; +import org.sonar.server.tester.UserSessionRule; +import org.sonar.server.ws.TestResponse; +import org.sonar.server.ws.WsActionTester; + +import static java.net.HttpURLConnection.HTTP_NO_CONTENT; +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.db.permission.OrganizationPermission.ADMINISTER; +import static org.sonar.db.permission.OrganizationPermission.ADMINISTER_QUALITY_GATES; +import static org.sonarqube.ws.client.qualitygate.QualityGatesWsParameters.PARAM_ID; + +public class DeleteConditionActionTest { + + @Rule + public DbTester db = DbTester.create(); + @Rule + public UserSessionRule userSession = UserSessionRule.standalone(); + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + private TestDefaultOrganizationProvider organizationProvider = TestDefaultOrganizationProvider.from(db); + + private WsActionTester ws = new WsActionTester(new DeleteConditionAction(userSession, db.getDbClient(), new QualityGatesWsSupport(db.getDbClient(), organizationProvider))); + + @Test + public void definition() { + WebService.Action definition = ws.getDef(); + + assertThat(definition.since()).isEqualTo("4.3"); + assertThat(definition.isPost()).isTrue(); + assertThat(definition.isInternal()).isFalse(); + assertThat(definition.params()).extracting(Param::key).containsExactlyInAnyOrder("id"); + + Param id = definition.param("id"); + assertThat(id.isRequired()).isTrue(); + } + + @Test + public void delete_condition() { + userSession.addPermission(ADMINISTER_QUALITY_GATES, db.getDefaultOrganization()); + QualityGateDto qualityGate = db.qualityGates().insertQualityGate(); + MetricDto metric = db.measures().insertMetric(); + QualityGateConditionDto qualityGateCondition = db.qualityGates().addCondition(qualityGate, metric); + + call(qualityGateCondition.getId()); + + assertThat(searchConditionsOf(qualityGate)).isEmpty(); + } + + @Test + public void no_content() { + userSession.addPermission(ADMINISTER_QUALITY_GATES, db.getDefaultOrganization()); + QualityGateDto qualityGate = db.qualityGates().insertQualityGate(); + MetricDto metric = db.measures().insertMetric(); + QualityGateConditionDto qualityGateCondition = db.qualityGates().addCondition(qualityGate, metric); + + TestResponse result = call(qualityGateCondition.getId()); + + assertThat(result.getStatus()).isEqualTo(HTTP_NO_CONTENT); + } + + @Test + public void fail_if_not_quality_gate_administrator() { + userSession.addPermission(ADMINISTER, db.getDefaultOrganization()); + QualityGateDto qualityGate = db.qualityGates().insertQualityGate(); + MetricDto metric = db.measures().insertMetric(); + QualityGateConditionDto qualityGateCondition = db.qualityGates().addCondition(qualityGate, metric); + + expectedException.expect(ForbiddenException.class); + + call(qualityGateCondition.getId()); + } + + @Test + public void fail_if_condition_id_is_not_found() { + userSession.addPermission(ADMINISTER_QUALITY_GATES, db.getDefaultOrganization()); + QualityGateDto qualityGate = db.qualityGates().insertQualityGate(); + MetricDto metric = db.measures().insertMetric(); + QualityGateConditionDto qualityGateCondition = db.qualityGates().addCondition(qualityGate, metric); + long unknownConditionId = qualityGateCondition.getId() + 42L; + + expectedException.expect(NotFoundException.class); + expectedException.expectMessage("No quality gate condition with id '" + unknownConditionId + "'"); + + call(unknownConditionId); + } + + private TestResponse call(long qualityGateConditionId) { + return ws.newRequest() + .setParam(PARAM_ID, String.valueOf(qualityGateConditionId)) + .execute(); + } + + private Collection searchConditionsOf(QualityGateDto qualityGate) { + return db.getDbClient().gateConditionDao().selectForQualityGate(db.getSession(), qualityGate.getId()); + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/QualityGatesWsTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/QualityGatesWsTest.java index 547e7abaaa9..0996909c233 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/QualityGatesWsTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/QualityGatesWsTest.java @@ -78,7 +78,7 @@ public class QualityGatesWsTest { new SetAsDefaultAction(qGates), new UnsetDefaultAction(qGates), new CreateConditionAction(null, null, null, null), new UpdateConditionAction(null, null, null, null), - new DeleteConditionAction(qGates), + new DeleteConditionAction(null, null, null), selectAction, new DeselectAction(qGates, mock(DbClient.class), mock(ComponentFinder.class)), new AppAction(null, null, null))); @@ -182,14 +182,6 @@ public class QualityGatesWsTest { assertThat(updateCondition.param("period")).isNotNull(); assertThat(updateCondition.isInternal()).isFalse(); - Action deleteCondition = controller.action("delete_condition"); - assertThat(deleteCondition).isNotNull(); - assertThat(deleteCondition.handler()).isNotNull(); - assertThat(deleteCondition.since()).isEqualTo("4.3"); - assertThat(deleteCondition.isPost()).isTrue(); - assertThat(deleteCondition.param("id")).isNotNull(); - assertThat(deleteCondition.isInternal()).isFalse(); - Action appInit = controller.action("app"); assertThat(appInit.isInternal()).isTrue(); } @@ -313,15 +305,6 @@ public class QualityGatesWsTest { tester.newGetRequest("api/qualitygates", "show").setParam("id", "12345").setParam("name", "Polop").execute(); } - @Test - public void delete_condition_nominal() throws Exception { - long condId = 12345L; - tester.newPostRequest("api/qualitygates", "delete_condition") - .setParam("id", Long.toString(condId)) - .execute() - .assertNoContent(); - } - @Test public void search_with_query() throws Exception { long gateId = 12345L; -- 2.39.5