]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-10510 Pull Request analysis fails with "Execute analysis" permission only
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Wed, 4 Apr 2018 08:15:20 +0000 (10:15 +0200)
committerSonarTech <sonartech@sonarsource.com>
Wed, 4 Apr 2018 13:18:42 +0000 (15:18 +0200)
* Use Tester in PermissionTest
* Sanitize permission ITs
* Add ITs to check pr analysis using only scan permission
* Fix permission in api/project_pull_requests/list
To access api/project_pull_requests/list, either 'Browse' or 'Execute Analysis' permissions is required

server/sonar-qa-util/src/main/java/org/sonarqube/qa/util/PermissionTester.java [new file with mode: 0644]
server/sonar-qa-util/src/main/java/org/sonarqube/qa/util/Tester.java
server/sonar-qa-util/src/main/java/org/sonarqube/qa/util/TesterSession.java
server/sonar-server/src/main/java/org/sonar/server/branch/pr/ws/ListAction.java
server/sonar-server/src/test/java/org/sonar/server/branch/pr/ws/ListActionTest.java
tests/src/test/java/org/sonarqube/tests/analysis/PermissionTest.java

diff --git a/server/sonar-qa-util/src/main/java/org/sonarqube/qa/util/PermissionTester.java b/server/sonar-qa-util/src/main/java/org/sonarqube/qa/util/PermissionTester.java
new file mode 100644 (file)
index 0000000..380371b
--- /dev/null
@@ -0,0 +1,99 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2018 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.sonarqube.qa.util;
+
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Consumer;
+import javax.annotation.Nullable;
+import org.sonarqube.ws.Organizations;
+import org.sonarqube.ws.Permissions.PermissionTemplate;
+import org.sonarqube.ws.Projects.CreateWsResponse.Project;
+import org.sonarqube.ws.Users;
+import org.sonarqube.ws.client.permissions.AddUserToTemplateRequest;
+import org.sonarqube.ws.client.permissions.ApplyTemplateRequest;
+import org.sonarqube.ws.client.permissions.CreateTemplateRequest;
+import org.sonarqube.ws.client.permissions.PermissionsService;
+
+import static com.sonar.orchestrator.container.Server.ADMIN_LOGIN;
+import static java.util.Arrays.stream;
+
+public class PermissionTester {
+
+  private static final AtomicInteger ID_GENERATOR = new AtomicInteger();
+
+  private final TesterSession session;
+
+  PermissionTester(TesterSession session) {
+    this.session = session;
+  }
+
+  @SafeVarargs
+  public final PermissionTemplate generateTemplate(Consumer<CreateTemplateRequest>... populators) {
+    return generateTemplate(null, populators);
+  }
+
+  @SafeVarargs
+  public final PermissionTemplate generateTemplate(@Nullable Organizations.Organization organization, Consumer<CreateTemplateRequest>... populators) {
+    int id = ID_GENERATOR.getAndIncrement();
+    String name = "template" + id;
+    CreateTemplateRequest request = new CreateTemplateRequest()
+      .setName(name)
+      .setOrganization(organization != null ? organization.getKey() : null);
+    stream(populators).forEach(p -> p.accept(request));
+    PermissionTemplate template = service().createTemplate(request).getPermissionTemplate();
+    // Give browse and admin permissions to admin in order to allow admin wsclient to perform any operation on created projects
+    addUserToTemplate(organization, ADMIN_LOGIN, template, "user");
+    addUserToTemplate(organization, ADMIN_LOGIN, template, "admin");
+    return template;
+  }
+
+  public void addUserToTemplate(Users.CreateWsResponse.User user, PermissionTemplate template, String permission) {
+    addUserToTemplate(null, user, template, permission);
+  }
+
+  public void addUserToTemplate(@Nullable Organizations.Organization organization, Users.CreateWsResponse.User user, PermissionTemplate template, String permission) {
+    addUserToTemplate(organization, user.getLogin(), template, permission);
+  }
+
+  private void addUserToTemplate(@Nullable Organizations.Organization organization, String userLogin, PermissionTemplate template, String permission) {
+    service().addUserToTemplate(new AddUserToTemplateRequest()
+      .setOrganization(organization != null ? organization.getKey() : null)
+      .setLogin(userLogin)
+      .setTemplateName(template.getName())
+      .setPermission(permission));
+  }
+
+  public void applyTemplate(PermissionTemplate template, Project project) {
+    applyTemplate(null, template, project);
+  }
+
+  public void applyTemplate(@Nullable Organizations.Organization organization, PermissionTemplate template, Project project) {
+    service().applyTemplate(
+      new ApplyTemplateRequest()
+        .setOrganization(organization != null ? organization.getKey() : null)
+        .setTemplateName(template.getName())
+        .setProjectKey(project.getKey()));
+  }
+
+  public PermissionsService service() {
+    return session.wsClient().permissions();
+  }
+
+}
index b97c2638fb3305a91221d7f561be501a7e7ff10f..715020b78120caf93bebf2141599dabec99ddbd0 100644 (file)
@@ -213,6 +213,11 @@ public class Tester extends ExternalResource implements TesterSession {
     return rootSession.webhooks();
   }
 
+  @Override
+  public PermissionTester permissions() {
+    return rootSession.permissions();
+  }
+
   private static class TesterSessionImpl implements TesterSession {
     private final WsClient client;
 
@@ -273,5 +278,10 @@ public class Tester extends ExternalResource implements TesterSession {
     public WebhookTester webhooks() {
       return new WebhookTester(this);
     }
+
+    @Override
+    public PermissionTester permissions() {
+      return new PermissionTester(this);
+    }
   }
 }
index d0e3afdcbd29355c5e6cbf98512efb4ae9a2606d..b3f2fbcbb3c891671dadfc7b034901e1e81bc61f 100644 (file)
@@ -43,4 +43,6 @@ public interface TesterSession {
 
   WebhookTester webhooks();
 
+  PermissionTester permissions();
+
 }
index a3ec277faa34641335e47efb08eef5c4515eef34..62618d2e359727eb18b32ab7d921f5bccdc4c856 100644 (file)
@@ -28,13 +28,13 @@ import javax.annotation.Nullable;
 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.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.component.BranchDto;
 import org.sonar.db.component.ComponentDto;
 import org.sonar.db.component.SnapshotDto;
 import org.sonar.db.measure.LiveMeasureDto;
+import org.sonar.db.permission.OrganizationPermission;
 import org.sonar.db.protobuf.DbProjectBranches;
 import org.sonar.server.component.ComponentFinder;
 import org.sonar.server.issue.index.BranchStatistics;
@@ -48,12 +48,15 @@ import static java.util.Objects.requireNonNull;
 import static org.sonar.api.measures.CoreMetrics.ALERT_STATUS_KEY;
 import static org.sonar.api.resources.Qualifiers.PROJECT;
 import static org.sonar.api.utils.DateUtils.formatDateTime;
+import static org.sonar.api.web.UserRole.USER;
+import static org.sonar.core.permission.GlobalPermissions.SCAN_EXECUTION;
 import static org.sonar.core.util.Protobuf.setNullable;
 import static org.sonar.core.util.stream.MoreCollectors.toList;
 import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex;
 import static org.sonar.db.component.BranchType.PULL_REQUEST;
 import static org.sonar.server.branch.pr.ws.PullRequestsWs.addProjectParam;
 import static org.sonar.server.branch.pr.ws.PullRequestsWsParameters.PARAM_PROJECT;
+import static org.sonar.server.user.AbstractUserSession.insufficientPrivilegesException;
 import static org.sonar.server.ws.WsUtils.writeProtobuf;
 
 public class ListAction implements PullRequestWsAction {
@@ -75,7 +78,11 @@ public class ListAction implements PullRequestWsAction {
     WebService.NewAction action = context.createAction("list")
       .setSince("7.1")
       .setDescription("List the pull requests of a project.<br/>" +
-        "Requires 'Administer' rights on the specified project.")
+        "One of the following permissions is required: " +
+        "<ul>" +
+        "<li>'Browse' rights on the specified project</li>" +
+        "<li>'Execute Analysis' rights on the specified project</li>" +
+        "</ul>")
       .setResponseExample(getClass().getResource("list-example.json"))
       .setHandler(this);
 
@@ -88,7 +95,7 @@ public class ListAction implements PullRequestWsAction {
 
     try (DbSession dbSession = dbClient.openSession(false)) {
       ComponentDto project = componentFinder.getByKey(dbSession, projectKey);
-      userSession.checkComponentPermission(UserRole.USER, project);
+      checkPermission(project);
       checkArgument(project.isEnabled() && PROJECT.equals(project.qualifier()), "Invalid project key");
 
       List<BranchDto> pullRequests = dbClient.branchDao().selectByComponent(dbSession, project).stream()
@@ -109,14 +116,23 @@ public class ListAction implements PullRequestWsAction {
 
       ProjectPullRequests.ListWsResponse.Builder protobufResponse = ProjectPullRequests.ListWsResponse.newBuilder();
       pullRequests
-        .forEach(b -> addPullRequest(protobufResponse, b, mergeBranchesByUuid, qualityGateMeasuresByComponentUuids.get(b.getUuid()),  branchStatisticsByBranchUuid.get(b.getUuid()),
+        .forEach(b -> addPullRequest(protobufResponse, b, mergeBranchesByUuid, qualityGateMeasuresByComponentUuids.get(b.getUuid()), branchStatisticsByBranchUuid.get(b.getUuid()),
           analysisDateByBranchUuid.get(b.getUuid())));
       writeProtobuf(protobufResponse.build(), request, response);
     }
   }
 
+  private void checkPermission(ComponentDto component) {
+    if (userSession.hasComponentPermission(USER, component) ||
+      userSession.hasComponentPermission(SCAN_EXECUTION, component) ||
+      userSession.hasPermission(OrganizationPermission.SCAN, component.getOrganizationUuid())) {
+      return;
+    }
+    throw insufficientPrivilegesException();
+  }
+
   private static void addPullRequest(ProjectPullRequests.ListWsResponse.Builder response, BranchDto branch, Map<String, BranchDto> mergeBranchesByUuid,
-                                     @Nullable LiveMeasureDto qualityGateMeasure, BranchStatistics branchStatistics, @Nullable String analysisDate) {
+    @Nullable LiveMeasureDto qualityGateMeasure, BranchStatistics branchStatistics, @Nullable String analysisDate) {
     Optional<BranchDto> mergeBranch = Optional.ofNullable(mergeBranchesByUuid.get(branch.getMergeBranchUuid()));
 
     ProjectPullRequests.PullRequest.Builder builder = ProjectPullRequests.PullRequest.newBuilder();
index 3ab729c882a6cdfc377580babd10e3e49761ff7c..434d1ca5b46bbdd01c86e7aac1ad13f63d3ee72c 100644 (file)
@@ -36,10 +36,12 @@ import org.sonar.db.component.ComponentTesting;
 import org.sonar.db.component.ResourceTypesRule;
 import org.sonar.db.metric.MetricDto;
 import org.sonar.db.organization.OrganizationDto;
+import org.sonar.db.permission.OrganizationPermission;
 import org.sonar.db.protobuf.DbProjectBranches;
 import org.sonar.db.rule.RuleDefinitionDto;
 import org.sonar.server.component.ComponentFinder;
 import org.sonar.server.es.EsTester;
+import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.issue.index.IssueIndex;
 import org.sonar.server.issue.index.IssueIndexDefinition;
@@ -66,6 +68,8 @@ import static org.sonar.api.rules.RuleType.CODE_SMELL;
 import static org.sonar.api.rules.RuleType.VULNERABILITY;
 import static org.sonar.api.utils.DateUtils.dateToLong;
 import static org.sonar.api.utils.DateUtils.parseDateTime;
+import static org.sonar.api.web.UserRole.CODEVIEWER;
+import static org.sonar.core.permission.GlobalPermissions.SCAN_EXECUTION;
 import static org.sonar.db.component.BranchType.LONG;
 import static org.sonar.db.component.BranchType.PULL_REQUEST;
 import static org.sonar.db.component.SnapshotTesting.newAnalysis;
@@ -265,7 +269,8 @@ public class ListActionTest {
       .executeProtobuf(ListWsResponse.class);
 
     assertThat(response.getPullRequestsList().stream().map(PullRequest::getStatus))
-      .extracting(Status::getQualityGateStatus, Status::hasBugs, Status::getBugs, Status::hasVulnerabilities, Status::getVulnerabilities, Status::hasCodeSmells, Status::getCodeSmells)
+      .extracting(Status::getQualityGateStatus, Status::hasBugs, Status::getBugs, Status::hasVulnerabilities, Status::getVulnerabilities, Status::hasCodeSmells,
+        Status::getCodeSmells)
       .containsExactlyInAnyOrder(tuple("ERROR", true, 1L, true, 2L, true, 3L));
   }
 
@@ -335,6 +340,64 @@ public class ListActionTest {
         tuple(true, lastAnalysisPullRequest));
   }
 
+  @Test
+  public void does_not_fail_when_only_browse_permission_on_project() {
+    ComponentDto project = db.components().insertMainBranch();
+    db.components().insertProjectBranch(project,
+      b -> b.setKey("123")
+        .setBranchType(PULL_REQUEST)
+        .setMergeBranchUuid(project.uuid())
+        .setPullRequestData(DbProjectBranches.PullRequestData.newBuilder().setBranch("feature/bar").build()));
+    userSession.logIn().addProjectPermission(UserRole.USER, project);
+
+    ListWsResponse response = ws.newRequest()
+      .setParam("project", project.getKey())
+      .executeProtobuf(ListWsResponse.class);
+
+    assertThat(response.getPullRequestsList())
+      .extracting(PullRequest::getKey)
+      .containsExactlyInAnyOrder("123");
+  }
+
+  @Test
+  public void does_not_fail_when_only_scan_permission_on_project() {
+    ComponentDto project = db.components().insertMainBranch();
+    db.components().insertProjectBranch(project,
+      b -> b.setKey("123")
+        .setBranchType(PULL_REQUEST)
+        .setMergeBranchUuid(project.uuid())
+        .setPullRequestData(DbProjectBranches.PullRequestData.newBuilder().setBranch("feature/bar").build()));
+    userSession.logIn().addProjectPermission(SCAN_EXECUTION, project);
+
+    ListWsResponse response = ws.newRequest()
+      .setParam("project", project.getKey())
+      .executeProtobuf(ListWsResponse.class);
+
+    assertThat(response.getPullRequestsList())
+      .extracting(PullRequest::getKey)
+      .containsExactlyInAnyOrder("123");
+  }
+
+  @Test
+  public void does_not_fail_when_only_scan_permission_on_organization() {
+    OrganizationDto organization = db.organizations().insert();
+    userSession.logIn().addPermission(OrganizationPermission.SCAN, organization);
+    ComponentDto project = db.components().insertMainBranch(organization);
+    db.components().insertProjectBranch(project,
+      b -> b.setKey("123")
+        .setBranchType(PULL_REQUEST)
+        .setMergeBranchUuid(project.uuid())
+        .setPullRequestData(DbProjectBranches.PullRequestData.newBuilder().setBranch("feature/bar").build()));
+
+    ListWsResponse response = ws.newRequest()
+      .setParam("project", project.getKey())
+      .executeProtobuf(ListWsResponse.class);
+
+    assertThat(response.getPullRequestsList())
+      .extracting(PullRequest::getKey)
+      .containsExactlyInAnyOrder("123");
+  }
+
   @Test
   public void fail_when_using_branch_db_key() throws Exception {
     OrganizationDto organization = db.organizations().insert();
@@ -382,4 +445,21 @@ public class ListActionTest {
       .execute();
   }
 
+  @Test
+  public void fail_when_not_having_right_permission() {
+    ComponentDto project = db.components().insertMainBranch();
+    db.components().insertProjectBranch(project,
+      b -> b.setKey("123")
+        .setBranchType(PULL_REQUEST)
+        .setMergeBranchUuid(project.uuid())
+        .setPullRequestData(DbProjectBranches.PullRequestData.newBuilder().setBranch("feature/bar").build()));
+    userSession.logIn().addProjectPermission(CODEVIEWER, project);
+
+    expectedException.expect(ForbiddenException.class);
+
+    ws.newRequest()
+      .setParam("project", project.getDbKey())
+      .execute();
+  }
+
 }
index c12a62753ebe03bc077a2078e78c37e7c12cb16b..1e1ea906665a6564f401d998e81e0b2e9e0afe02 100644 (file)
@@ -22,65 +22,43 @@ package org.sonarqube.tests.analysis;
 import com.sonar.orchestrator.Orchestrator;
 import com.sonar.orchestrator.build.BuildResult;
 import com.sonar.orchestrator.build.SonarScanner;
-import org.sonarqube.tests.Category3Suite;
-import javax.annotation.Nullable;
-import org.junit.After;
 import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
+import org.sonarqube.qa.util.Tester;
+import org.sonarqube.tests.Category3Suite;
+import org.sonarqube.ws.Permissions;
+import org.sonarqube.ws.Projects.CreateWsResponse.Project;
 import org.sonarqube.ws.UserTokens;
-import org.sonarqube.ws.client.WsClient;
+import org.sonarqube.ws.Users.CreateWsResponse.User;
 import org.sonarqube.ws.client.permissions.AddUserRequest;
 import org.sonarqube.ws.client.usertokens.GenerateRequest;
-import org.sonarqube.ws.client.usertokens.RevokeRequest;
-import org.sonarqube.ws.client.usertokens.UserTokensService;
-import util.user.UserRule;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static util.ItUtils.newAdminWsClient;
 import static util.ItUtils.projectDir;
-import static util.ItUtils.resetSettings;
-import static util.ItUtils.setServerProperty;
 
 public class PermissionTest {
 
-  private static final String A_LOGIN = "a_login";
-  private static final String A_PASSWORD = "a_password";
-
   @ClassRule
   public static Orchestrator orchestrator = Category3Suite.ORCHESTRATOR;
 
   @Rule
-  public UserRule userRule = UserRule.from(orchestrator);
-
-  private WsClient adminWsClient;
-  private UserTokensService userTokensWsClient;
+  public Tester tester = new Tester(orchestrator).disableOrganizations();
 
   @Before
   public void setUp() {
-    orchestrator.resetData();
-
     // enforce scanners to be authenticated
-    setServerProperty(orchestrator, "sonar.forceAuthentication", "true");
-
-    adminWsClient = newAdminWsClient(orchestrator);
-    userTokensWsClient = adminWsClient.userTokens();
-  }
-
-  @After
-  public void tearDown() {
-    resetSettings(orchestrator, null, "sonar.forceAuthentication");
-    userRule.resetUsers();
+    tester.settings().setGlobalSetting("sonar.forceAuthentication", "true");
   }
 
   @Test
   public void scanner_can_authenticate_with_authentication_token() {
-    createUserWithProvisioningAndScanPermissions();
+    User user = createUserWithProvisioningAndScanPermissions();
 
     String tokenName = "For test";
-    UserTokens.GenerateWsResponse generateWsResponse = userTokensWsClient.generate(new GenerateRequest()
-      .setLogin(A_LOGIN)
+    UserTokens.GenerateWsResponse generateWsResponse = tester.wsClient().userTokens().generate(new GenerateRequest()
+      .setLogin(user.getLogin())
       .setName(tokenName));
     SonarScanner sampleProject = SonarScanner.create(projectDir("shared/xoo-sample"));
     sampleProject.setProperties(
@@ -90,7 +68,6 @@ public class PermissionTest {
     BuildResult buildResult = orchestrator.executeBuild(sampleProject);
 
     assertThat(buildResult.isSuccess()).isTrue();
-    userTokensWsClient.revoke(new RevokeRequest().setLogin(A_LOGIN).setName(tokenName));
   }
 
   @Test
@@ -110,7 +87,7 @@ public class PermissionTest {
    */
   @Test
   public void scanner_can_authenticate_with_login_password() {
-    createUserWithProvisioningAndScanPermissions();
+    User user = createUserWithProvisioningAndScanPermissions();
 
     orchestrator.getServer().provisionProject("sample", "xoo-sample");
 
@@ -130,29 +107,27 @@ public class PermissionTest {
       "Not authorized. Please check the properties sonar.login and sonar.password.");
 
     buildResult = scan("shared/xoo-sample",
-      "sonar.login", A_LOGIN,
-      "sonar.password", A_PASSWORD);
+      "sonar.login", user.getLogin(),
+      "sonar.password", user.getLogin());
     assertThat(buildResult.getLastStatus()).isEqualTo(0);
   }
 
   @Test
-  public void run_scanner_with_user_having_scan_permission_only_on_project() {
-    userRule.createUser(A_LOGIN, A_PASSWORD);
-    orchestrator.getServer().provisionProject("sample", "sample");
-    addUserPermission(A_LOGIN, "scan", "sample");
+  public void run_scanner_with_user_having_only_scan_permission_on_project() {
+    User user = tester.users().generate();
+    Project project = tester.projects().provision(c -> c.setVisibility("private"));
+    Permissions.PermissionTemplate template = tester.permissions().generateTemplate();
+    tester.permissions().addUserToTemplate(user, template, "scan");
+    tester.permissions().applyTemplate(template, project);
 
-    BuildResult buildResult = scanQuietly("shared/xoo-sample", "sonar.login", A_LOGIN, "sonar.password", A_PASSWORD);
+    BuildResult buildResult = scanQuietly("shared/xoo-sample",
+      "sonar.projectKey", project.getKey(),
+      "sonar.login", user.getLogin(),
+      "sonar.password", user.getLogin());
 
     assertThat(buildResult.isSuccess()).isTrue();
   }
 
-  private void addUserPermission(String login, String permission, @Nullable String projectKey) {
-    adminWsClient.permissions().addUser(new AddUserRequest()
-      .setLogin(login)
-      .setPermission(permission)
-      .setProjectKey(projectKey));
-  }
-
   private BuildResult scan(String projectPath, String... props) {
     SonarScanner scanner = configureScanner(projectPath, props);
     return orchestrator.executeBuild(scanner);
@@ -168,10 +143,11 @@ public class PermissionTest {
       .setProperties(props);
   }
 
-  private void createUserWithProvisioningAndScanPermissions() {
-    userRule.createUser(A_LOGIN, A_PASSWORD);
-    addUserPermission(A_LOGIN, "provisioning", null);
-    addUserPermission(A_LOGIN, "scan", null);
+  private User createUserWithProvisioningAndScanPermissions() {
+    User user = tester.users().generate();
+    tester.wsClient().permissions().addUser(new AddUserRequest().setLogin(user.getLogin()).setPermission("provisioning"));
+    tester.wsClient().permissions().addUser(new AddUserRequest().setLogin(user.getLogin()).setPermission("scan"));
+    return user;
   }
 
 }