SONAR-10088 Forbid deleting built-in quality gate condition

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 45bc460ddb5b57623fd8c47087297b3549862a53..3353e082d85dd1ac5615a91d7aa72005c5bbe5e1 100644 (file)
--- 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 df5a1a52363fa01078c3bff7662dacd7c3b546ce..16b98a21ca4ad92481432dc84b8c9780f7a0da86 100644 (file)
--- 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 9eaf85f4a1b05ee49299000a859573bec497350e..16e5720729485bd743996d9984b3316fcf7f7758 100644 (file)
--- 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();