From 9d8776164aeb898c5b85f4c46038b21e6a2c7e3b Mon Sep 17 00:00:00 2001 From: Guillaume Jambet Date: Mon, 27 Nov 2017 10:41:07 +0100 Subject: [PATCH] SONAR-10088 Forbid deleting built-in quality gate condition --- .../qualitygate/ws/DeleteConditionAction.java | 20 ++++++----- .../ws/DeleteConditionActionTest.java | 34 +++++++++++++++++-- .../qualitygate/ws/QualityGatesWsTest.java | 3 +- 3 files changed, 44 insertions(+), 13 deletions(-) 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 45bc460ddb5..3353e082d85 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 @@ -24,20 +24,18 @@ import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; import org.sonar.db.DbClient; import org.sonar.db.DbSession; -import org.sonar.db.organization.OrganizationDto; -import org.sonar.server.user.UserSession; +import org.sonar.db.qualitygate.QualityGateConditionDto; +import org.sonar.db.qualitygate.QualityGateDto; -import static org.sonar.db.permission.OrganizationPermission.ADMINISTER_QUALITY_GATES; +import static com.google.common.base.Preconditions.checkState; import static org.sonar.server.qualitygate.ws.QualityGatesWsParameters.PARAM_ID; public class DeleteConditionAction implements QualityGatesWsAction { private final DbClient dbClient; - private final UserSession userSession; private final QualityGatesWsSupport wsSupport; - public DeleteConditionAction(UserSession userSession, DbClient dbClient, QualityGatesWsSupport wsSupport) { - this.userSession = userSession; + public DeleteConditionAction(DbClient dbClient, QualityGatesWsSupport wsSupport) { this.dbClient = dbClient; this.wsSupport = wsSupport; } @@ -62,9 +60,13 @@ public class DeleteConditionAction implements QualityGatesWsAction { public void handle(Request request, Response response) { 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); + + QualityGateConditionDto condition = wsSupport.getCondition(dbSession, conditionId); + QualityGateDto qualityGateDto = dbClient.qualityGateDao().selectById(dbSession, condition.getQualityGateId()); + checkState(qualityGateDto != null, "Condition '%s' is linked to an unknown quality gate '%s'", conditionId, condition.getQualityGateId()); + wsSupport.checkCanEdit(qualityGateDto); + + dbClient.gateConditionDao().delete(condition, dbSession); dbSession.commit(); response.noContent(); } 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 index df5a1a52363..16b98a21ca4 100644 --- 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 @@ -37,6 +37,8 @@ import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.TestResponse; import org.sonar.server.ws.WsActionTester; +import static java.lang.String.format; +import static java.lang.String.valueOf; import static java.net.HttpURLConnection.HTTP_NO_CONTENT; import static org.assertj.core.api.Assertions.assertThat; import static org.sonar.db.permission.OrganizationPermission.ADMINISTER; @@ -55,7 +57,7 @@ public class DeleteConditionActionTest { private TestDefaultOrganizationProvider organizationProvider = TestDefaultOrganizationProvider.from(db); private WsActionTester ws = new WsActionTester( - new DeleteConditionAction(userSession, db.getDbClient(), new QualityGatesWsSupport(db.getDbClient(), userSession, organizationProvider))); + new DeleteConditionAction(db.getDbClient(), new QualityGatesWsSupport(db.getDbClient(), userSession, organizationProvider))); @Test public void definition() { @@ -94,6 +96,21 @@ public class DeleteConditionActionTest { assertThat(result.getStatus()).isEqualTo(HTTP_NO_CONTENT); } + @Test + public void fail_if_built_in_quality_gate() { + userSession.addPermission(ADMINISTER_QUALITY_GATES, db.getDefaultOrganization()); + QualityGateDto qualityGate = db.qualityGates().insertQualityGate(qg -> qg.setBuiltIn(true)); + MetricDto metric = db.measures().insertMetric(); + QualityGateConditionDto qualityGateCondition = db.qualityGates().addCondition(qualityGate, metric); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage(format("Operation forbidden for built-in Quality Gate '%s'", qualityGate.getName())); + + ws.newRequest() + .setParam(PARAM_ID, valueOf(qualityGateCondition.getId())) + .execute(); + } + @Test public void fail_if_not_quality_gate_administrator() { userSession.addPermission(ADMINISTER, db.getDefaultOrganization()); @@ -120,9 +137,22 @@ public class DeleteConditionActionTest { call(unknownConditionId); } + @Test + public void fail_when_condition_match_unknown_quality_gate() { + userSession.addPermission(ADMINISTER_QUALITY_GATES, db.getDefaultOrganization()); + QualityGateConditionDto condition = new QualityGateConditionDto().setQualityGateId(123L); + db.getDbClient().gateConditionDao().insert(condition, db.getSession()); + db.commit(); + + expectedException.expect(IllegalStateException.class); + expectedException.expectMessage(format("Condition '%s' is linked to an unknown quality gate '%s'", condition.getId(), 123L)); + + call(condition.getId()); + } + private TestResponse call(long qualityGateConditionId) { return ws.newRequest() - .setParam(PARAM_ID, String.valueOf(qualityGateConditionId)) + .setParam(PARAM_ID, valueOf(qualityGateConditionId)) .execute(); } 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 9eaf85f4a1b..16e57207294 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 @@ -72,7 +72,6 @@ public class QualityGatesWsTest { new CopyAction(qGates), new DestroyAction(qGates), new SetAsDefaultAction(qGates), - new DeleteConditionAction(null, null, null), selectAction, new DeselectAction(qGates, mock(DbClient.class), mock(ComponentFinder.class)))); } @@ -83,7 +82,7 @@ public class QualityGatesWsTest { assertThat(controller).isNotNull(); assertThat(controller.path()).isEqualTo("api/qualitygates"); assertThat(controller.description()).isNotEmpty(); - assertThat(controller.actions()).hasSize(9); + assertThat(controller.actions()).hasSize(8); Action copy = controller.action("copy"); assertThat(copy).isNotNull(); -- 2.39.5