SONAR-8716 fix check of permissions in api/quality_gates/create_condition

diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/CreateConditionAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/CreateConditionAction.java
index b11e23edef2b924e6fc35f1916838e9e7e51bcbd..04f30c713a648941b39a386b000481846940e1e7 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/CreateConditionAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/CreateConditionAction.java
@@ -25,12 +25,14 @@ import org.sonar.api.server.ws.WebService;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.qualitygate.QualityGateConditionDto;
+import org.sonar.server.organization.DefaultOrganizationProvider;
 import org.sonar.server.qualitygate.QualityGateConditionsUpdater;
 import org.sonar.server.user.UserSession;
 import org.sonarqube.ws.WsQualityGates.CreateConditionWsResponse;
 import org.sonarqube.ws.client.qualitygate.CreateConditionRequest;
 
 import static org.sonar.core.permission.GlobalPermissions.QUALITY_GATE_ADMIN;
+import static org.sonar.core.util.Protobuf.setNullable;
 import static org.sonar.server.qualitygate.ws.QualityGatesWs.addConditionParams;
 import static org.sonar.server.ws.WsUtils.writeProtobuf;
 import static org.sonarqube.ws.client.qualitygate.QualityGatesWsParameters.ACTION_CREATE_CONDITION;
@@ -46,11 +48,14 @@ public class CreateConditionAction implements QualityGatesWsAction {
   private final UserSession userSession;
   private final DbClient dbClient;
   private final QualityGateConditionsUpdater qualityGateConditionsUpdater;
+  private final DefaultOrganizationProvider defaultOrganizationProvider;
 
-  public CreateConditionAction(UserSession userSession, DbClient dbClient, QualityGateConditionsUpdater qualityGateConditionsUpdater) {
+  public CreateConditionAction(UserSession userSession, DbClient dbClient, QualityGateConditionsUpdater qualityGateConditionsUpdater,
+    DefaultOrganizationProvider defaultOrganizationProvider) {
     this.userSession = userSession;
     this.dbClient = dbClient;
     this.qualityGateConditionsUpdater = qualityGateConditionsUpdater;
+    this.defaultOrganizationProvider = defaultOrganizationProvider;
   }
 
   @Override
@@ -72,18 +77,15 @@ public class CreateConditionAction implements QualityGatesWsAction {
 
   @Override
   public void handle(Request request, Response response) {
-    userSession.checkPermission(QUALITY_GATE_ADMIN);
+    userSession.checkLoggedIn().checkOrganizationPermission(defaultOrganizationProvider.get().getUuid(), QUALITY_GATE_ADMIN);
 
-    DbSession dbSession = dbClient.openSession(false);
-    try {
+    try (DbSession dbSession = dbClient.openSession(false)) {
       writeProtobuf(doHandle(toWsRequest(request), dbSession), request, response);
       dbSession.commit();
-    } finally {
-      dbClient.closeSession(dbSession);
     }
   }
 
-  private CreateConditionWsResponse doHandle(CreateConditionRequest request, DbSession dbSession){
+  private CreateConditionWsResponse doHandle(CreateConditionRequest request, DbSession dbSession) {
     QualityGateConditionDto condition = qualityGateConditionsUpdater.createCondition(dbSession,
       request.getQualityGateId(),
       request.getMetricKey(),
@@ -96,18 +98,9 @@ public class CreateConditionAction implements QualityGatesWsAction {
       .setId(condition.getId())
       .setMetric(condition.getMetricKey())
       .setOp(condition.getOperator());
-    String warning = condition.getWarningThreshold();
-    if (warning != null) {
-      response.setWarning(warning);
-    }
-    String error = condition.getErrorThreshold();
-    if (error != null) {
-      response.setError(error);
-    }
-    Integer period = condition.getPeriod();
-    if (period != null) {
-      response.setPeriod(period);
-    }
+    setNullable(condition.getWarningThreshold(), response::setWarning);
+    setNullable(condition.getErrorThreshold(), response::setError);
+    setNullable(condition.getPeriod(), response::setPeriod);
     return response.build();
   }
 

diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/CreateConditionActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/CreateConditionActionTest.java
index 1b838b480d56fd7804284a3a4b96075459fa0b80..e2d3560167d998bc3afbb7dd0468f4b444ad400c 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/CreateConditionActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/CreateConditionActionTest.java
@@ -30,14 +30,18 @@ import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.api.utils.System2;
+import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
 import org.sonar.db.metric.MetricDto;
+import org.sonar.db.organization.OrganizationDto;
 import org.sonar.db.qualitygate.QualityGateConditionDto;
 import org.sonar.db.qualitygate.QualityGateDbTester;
 import org.sonar.db.qualitygate.QualityGateDto;
 import org.sonar.server.exceptions.ForbiddenException;
+import org.sonar.server.exceptions.UnauthorizedException;
+import org.sonar.server.organization.TestDefaultOrganizationProvider;
 import org.sonar.server.qualitygate.QualityGateConditionsUpdater;
 import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.ws.TestRequest;
@@ -47,7 +51,6 @@ import org.sonarqube.ws.WsQualityGates.CreateConditionWsResponse;
 
 import static org.assertj.core.api.Java6Assertions.assertThat;
 import static org.sonar.core.permission.GlobalPermissions.QUALITY_GATE_ADMIN;
-import static org.sonar.core.permission.GlobalPermissions.SCAN_EXECUTION;
 import static org.sonar.db.metric.MetricTesting.newMetricDto;
 import static org.sonar.server.computation.task.projectanalysis.metric.Metric.MetricType.PERCENT;
 import static org.sonarqube.ws.client.qualitygate.QualityGatesWsParameters.PARAM_ERROR;
@@ -68,21 +71,19 @@ public class CreateConditionActionTest {
   @Rule
   public DbTester db = DbTester.create(System2.INSTANCE);
 
-  DbClient dbClient = db.getDbClient();
-  DbSession dbSession = db.getSession();
-  QualityGateDbTester qualityGateDbTester = new QualityGateDbTester(db);
-
-  CreateConditionAction underTest = new CreateConditionAction(userSession, dbClient, new QualityGateConditionsUpdater(dbClient));
-
-  QualityGateDto qualityGateDto;
-
-  MetricDto coverageMetricDto = newMetricDto()
+  private TestDefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(db);
+  private DbClient dbClient = db.getDbClient();
+  private DbSession dbSession = db.getSession();
+  private QualityGateDbTester qualityGateDbTester = new QualityGateDbTester(db);
+  private CreateConditionAction underTest = new CreateConditionAction(userSession, dbClient, new QualityGateConditionsUpdater(dbClient), defaultOrganizationProvider);
+  private QualityGateDto qualityGateDto;
+  private MetricDto coverageMetricDto = newMetricDto()
     .setKey("coverage")
     .setShortName("Coverage")
     .setValueType(PERCENT.name())
     .setHidden(false);
 
-  WsActionTester ws = new WsActionTester(underTest);
+  private WsActionTester ws = new WsActionTester(underTest);
 
   @Before
   public void setUp() throws Exception {
@@ -93,7 +94,7 @@ public class CreateConditionActionTest {
 
   @Test
   public void create_warning_condition() throws Exception {
-    setUserAsQualityGateAdmin();
+    logInAsQualityGateAdmin();
 
     CreateConditionWsResponse response = executeRequest(qualityGateDto.getId(), coverageMetricDto.getKey(), "LT", "90", null, null);
 
@@ -102,7 +103,7 @@ public class CreateConditionActionTest {
 
   @Test
   public void create_error_condition() throws Exception {
-    setUserAsQualityGateAdmin();
+    logInAsQualityGateAdmin();
 
     CreateConditionWsResponse response = executeRequest(qualityGateDto.getId(), coverageMetricDto.getKey(), "LT", null, "90", null);
 
@@ -111,7 +112,7 @@ public class CreateConditionActionTest {
 
   @Test
   public void create_condition_over_leak_period() throws Exception {
-    setUserAsQualityGateAdmin();
+    logInAsQualityGateAdmin();
 
     CreateConditionWsResponse response = executeRequest(qualityGateDto.getId(), coverageMetricDto.getKey(), "LT", null, "90", 1);
 
@@ -119,10 +120,35 @@ public class CreateConditionActionTest {
   }
 
   @Test
-  public void fail_when_not_quality_gate_admin() throws Exception {
-    setUserAsNotQualityGateAdmin();
+  public void throw_ForbiddenException_if_not_gate_administrator() throws Exception {
+    userSession.logIn();
 
     expectedException.expect(ForbiddenException.class);
+    expectedException.expectMessage("Insufficient privileges");
+
+    executeRequest(qualityGateDto.getId(), coverageMetricDto.getKey(), "LT", "90", null, null);
+  }
+
+  @Test
+  public void throw_ForbiddenException_if_not_gate_administrator_of_default_organization() throws Exception {
+    // as long as organizations don't support Quality gates, the global permission
+    // is defined on the default organization
+    OrganizationDto org = db.organizations().insert();
+    userSession.logIn().addOrganizationPermission(org, GlobalPermissions.QUALITY_GATE_ADMIN);
+
+    expectedException.expect(ForbiddenException.class);
+    expectedException.expectMessage("Insufficient privileges");
+
+    executeRequest(qualityGateDto.getId(), coverageMetricDto.getKey(), "LT", "90", null, null);
+  }
+
+  @Test
+  public void throw_UnauthorizedException_if_not_logged_in() throws Exception {
+    userSession.anonymous();
+
+    expectedException.expect(UnauthorizedException.class);
+    expectedException.expectMessage("Authentication is required");
+
     executeRequest(qualityGateDto.getId(), coverageMetricDto.getKey(), "LT", "90", null, null);
   }
 
@@ -190,11 +216,7 @@ public class CreateConditionActionTest {
     }
   }
 
-  private void setUserAsQualityGateAdmin() {
-    userSession.logIn("project-admin").setGlobalPermissions(QUALITY_GATE_ADMIN);
-  }
-
-  private void setUserAsNotQualityGateAdmin() {
-    userSession.logIn("not-admin").setGlobalPermissions(SCAN_EXECUTION);
+  private void logInAsQualityGateAdmin() {
+    userSession.logIn().addOrganizationPermission(db.getDefaultOrganization(), QUALITY_GATE_ADMIN);
   }
 }

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 ee35cc1d12862c30d196afd24b4076a86a135568..116f43a05c1db19c26d06e9b7bf4a081917ec7e1 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
@@ -76,7 +76,7 @@ public class QualityGatesWsTest {
       new CopyAction(qGates),
       new DestroyAction(qGates), new RenameAction(qGates),
       new SetAsDefaultAction(qGates), new UnsetDefaultAction(qGates),
-      new CreateConditionAction(null, null, null),
+      new CreateConditionAction(null, null, null, null),
       new UpdateConditionAction(null, null, null),
       new DeleteConditionAction(qGates),
       selectAction,