From: Julien Lancelot Date: Wed, 4 Apr 2018 08:15:20 +0000 (+0200) Subject: SONAR-10510 Pull Request analysis fails with "Execute analysis" permission only X-Git-Tag: 7.5~1415 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=e3ab105183d7b7678340b2cca8902929fda8200d;p=sonarqube.git SONAR-10510 Pull Request analysis fails with "Execute analysis" permission only * 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 --- 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 index 00000000000..380371b2311 --- /dev/null +++ b/server/sonar-qa-util/src/main/java/org/sonarqube/qa/util/PermissionTester.java @@ -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... populators) { + return generateTemplate(null, populators); + } + + @SafeVarargs + public final PermissionTemplate generateTemplate(@Nullable Organizations.Organization organization, Consumer... 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(); + } + +} diff --git a/server/sonar-qa-util/src/main/java/org/sonarqube/qa/util/Tester.java b/server/sonar-qa-util/src/main/java/org/sonarqube/qa/util/Tester.java index b97c2638fb3..715020b7812 100644 --- a/server/sonar-qa-util/src/main/java/org/sonarqube/qa/util/Tester.java +++ b/server/sonar-qa-util/src/main/java/org/sonarqube/qa/util/Tester.java @@ -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); + } } } diff --git a/server/sonar-qa-util/src/main/java/org/sonarqube/qa/util/TesterSession.java b/server/sonar-qa-util/src/main/java/org/sonarqube/qa/util/TesterSession.java index d0e3afdcbd2..b3f2fbcbb3c 100644 --- a/server/sonar-qa-util/src/main/java/org/sonarqube/qa/util/TesterSession.java +++ b/server/sonar-qa-util/src/main/java/org/sonarqube/qa/util/TesterSession.java @@ -43,4 +43,6 @@ public interface TesterSession { WebhookTester webhooks(); + PermissionTester permissions(); + } diff --git a/server/sonar-server/src/main/java/org/sonar/server/branch/pr/ws/ListAction.java b/server/sonar-server/src/main/java/org/sonar/server/branch/pr/ws/ListAction.java index a3ec277faa3..62618d2e359 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/branch/pr/ws/ListAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/branch/pr/ws/ListAction.java @@ -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.
" + - "Requires 'Administer' rights on the specified project.") + "One of the following permissions is required: " + + "
    " + + "
  • 'Browse' rights on the specified project
    • " + + "
  • 'Execute Analysis' rights on the specified project
    • " + + "
") .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 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 mergeBranchesByUuid, - @Nullable LiveMeasureDto qualityGateMeasure, BranchStatistics branchStatistics, @Nullable String analysisDate) { + @Nullable LiveMeasureDto qualityGateMeasure, BranchStatistics branchStatistics, @Nullable String analysisDate) { Optional mergeBranch = Optional.ofNullable(mergeBranchesByUuid.get(branch.getMergeBranchUuid())); ProjectPullRequests.PullRequest.Builder builder = ProjectPullRequests.PullRequest.newBuilder(); diff --git a/server/sonar-server/src/test/java/org/sonar/server/branch/pr/ws/ListActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/branch/pr/ws/ListActionTest.java index 3ab729c882a..434d1ca5b46 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/branch/pr/ws/ListActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/branch/pr/ws/ListActionTest.java @@ -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(); + } + } diff --git a/tests/src/test/java/org/sonarqube/tests/analysis/PermissionTest.java b/tests/src/test/java/org/sonarqube/tests/analysis/PermissionTest.java index c12a62753eb..1e1ea906665 100644 --- a/tests/src/test/java/org/sonarqube/tests/analysis/PermissionTest.java +++ b/tests/src/test/java/org/sonarqube/tests/analysis/PermissionTest.java @@ -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; } }