From 9997b7a557b0032283d8fd4813210c4a45e010dc Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Tue, 25 Apr 2017 13:53:23 +0200 Subject: [PATCH] SONAR-9089 account project visibility when applying perm template --- .../template/PermissionTemplateDbTester.java | 4 + .../permission/PermissionTemplateService.java | 40 +++- .../PermissionTemplateServiceTest.java | 212 +++++++++++++++++- .../template/BulkApplyTemplateActionTest.java | 67 ++++-- 4 files changed, 289 insertions(+), 34 deletions(-) diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/permission/template/PermissionTemplateDbTester.java b/server/sonar-db-dao/src/test/java/org/sonar/db/permission/template/PermissionTemplateDbTester.java index aa4f22b07b6..b206ca77a0c 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/permission/template/PermissionTemplateDbTester.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/permission/template/PermissionTemplateDbTester.java @@ -78,6 +78,10 @@ public class PermissionTemplateDbTester { db.commit(); } + public void addProjectCreatorToTemplate(PermissionTemplateDto permissionTemplate, String permission) { + addProjectCreatorToTemplate(permissionTemplate.getId(), permission); + } + public void addProjectCreatorToTemplate(long templateId, String permission) { dbClient.permissionTemplateCharacteristicDao().insert(dbSession, newPermissionTemplateCharacteristicDto() .setWithProjectCreator(true) diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateService.java b/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateService.java index 858338adc78..875d1b0a93a 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateService.java @@ -31,6 +31,7 @@ import org.apache.commons.lang.StringUtils; import org.sonar.api.resources.Qualifiers; import org.sonar.api.server.ServerSide; import org.sonar.core.component.ComponentKeys; +import org.sonar.core.permission.ProjectPermissions; import org.sonar.core.util.stream.MoreCollectors; import org.sonar.db.DbClient; import org.sonar.db.DbSession; @@ -69,11 +70,11 @@ public class PermissionTemplateService { this.userSession = userSession; this.defaultTemplatesResolver = defaultTemplatesResolver; } - + public boolean wouldUserHaveScanPermissionWithDefaultTemplate(DbSession dbSession, - String organizationUuid, @Nullable Integer userId, - @Nullable String branch, String projectKey, - String qualifier) { + String organizationUuid, @Nullable Integer userId, + @Nullable String branch, String projectKey, + String qualifier) { if (userSession.hasPermission(OrganizationPermission.SCAN, organizationUuid)) { return true; } @@ -141,20 +142,26 @@ public class PermissionTemplateService { List usersPermissions = dbClient.permissionTemplateDao().selectUserPermissionsByTemplateId(dbSession, template.getId()); String organizationUuid = template.getOrganizationUuid(); usersPermissions + .stream() + .filter(up -> permissionValidForProject(project, up.getPermission())) .forEach(up -> { UserPermissionDto dto = new UserPermissionDto(organizationUuid, up.getPermission(), up.getUserId(), project.getId()); dbClient.userPermissionDao().insert(dbSession, dto); }); List groupsPermissions = dbClient.permissionTemplateDao().selectGroupPermissionsByTemplateId(dbSession, template.getId()); - groupsPermissions.forEach(gp -> { - GroupPermissionDto dto = new GroupPermissionDto() - .setOrganizationUuid(organizationUuid) - .setGroupId(isAnyone(gp.getGroupName()) ? null : gp.getGroupId()) - .setRole(gp.getPermission()) - .setResourceId(project.getId()); - dbClient.groupPermissionDao().insert(dbSession, dto); - }); + groupsPermissions + .stream() + .filter(gp -> groupNameValidForProject(project, gp.getGroupName())) + .filter(gp -> permissionValidForProject(project, gp.getPermission())) + .forEach(gp -> { + GroupPermissionDto dto = new GroupPermissionDto() + .setOrganizationUuid(organizationUuid) + .setGroupId(isAnyone(gp.getGroupName()) ? null : gp.getGroupId()) + .setRole(gp.getPermission()) + .setResourceId(project.getId()); + dbClient.groupPermissionDao().insert(dbSession, dto); + }); List characteristics = dbClient.permissionTemplateCharacteristicDao().selectByTemplateIds(dbSession, asList(template.getId())); if (projectCreatorUserId != null) { @@ -164,6 +171,7 @@ public class PermissionTemplateService { .collect(java.util.stream.Collectors.toSet()); characteristics.stream() .filter(PermissionTemplateCharacteristicDto::getWithProjectCreator) + .filter(up -> permissionValidForProject(project, up.getPermission())) .filter(characteristic -> !permissionsForCurrentUserAlreadyInDb.contains(characteristic.getPermission())) .forEach(c -> { UserPermissionDto dto = new UserPermissionDto(organizationUuid, c.getPermission(), projectCreatorUserId, project.getId()); @@ -172,6 +180,14 @@ public class PermissionTemplateService { } } + private static boolean permissionValidForProject(ComponentDto project, String permission) { + return project.isPrivate() || !ProjectPermissions.PUBLIC_PERMISSIONS.contains(permission); + } + + private static boolean groupNameValidForProject(ComponentDto project, String groupName) { + return !project.isPrivate() || !isAnyone(groupName); + } + /** * Return the permission template for the given component. If no template key pattern match then consider default * template for the component qualifier. diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/PermissionTemplateServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/PermissionTemplateServiceTest.java index 3445f219e6d..ae714b8162e 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/permission/PermissionTemplateServiceTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/permission/PermissionTemplateServiceTest.java @@ -21,12 +21,15 @@ package org.sonar.server.permission; import java.util.List; import javax.annotation.Nullable; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.sonar.api.resources.Qualifiers; import org.sonar.api.utils.internal.AlwaysIncreasingSystem2; import org.sonar.api.web.UserRole; +import org.sonar.core.permission.GlobalPermissions; +import org.sonar.core.permission.ProjectPermissions; import org.sonar.db.DbSession; import org.sonar.db.DbTester; import org.sonar.db.component.ComponentDto; @@ -57,15 +60,215 @@ public class PermissionTemplateServiceTest { private PermissionTemplateDbTester templateDb = dbTester.permissionTemplates(); private DbSession session = dbTester.getSession(); private PermissionIndexer permissionIndexer = mock(PermissionIndexer.class); + + private OrganizationDto organization; + private ComponentDto privateProject; + private ComponentDto publicProject; + private GroupDto group; + private UserDto user; + private UserDto creator; + private PermissionTemplateService underTest = new PermissionTemplateService(dbTester.getDbClient(), permissionIndexer, userSession, defaultTemplatesResolver); + @Before + public void setUp() throws Exception { + organization = dbTester.organizations().insert(); + privateProject = dbTester.components().insertPrivateProject(organization); + publicProject = dbTester.components().insertPublicProject(organization); + group = dbTester.users().insertGroup(organization); + user = dbTester.users().insertUser(); + creator = dbTester.users().insertUser(); + } + + @Test + public void apply_does_not_insert_permission_to_group_AnyOne_when_applying_template_on_private_project() { + PermissionTemplateDto permissionTemplate = dbTester.permissionTemplates().insertTemplate(organization); + dbTester.permissionTemplates().addAnyoneToTemplate(permissionTemplate, "p1"); + + underTest.apply(session, permissionTemplate, singletonList(privateProject)); + + assertThat(selectProjectPermissionsOfGroup(organization, null, privateProject)).isEmpty(); + } + + @Test + public void apply_default_does_not_insert_permission_to_group_AnyOne_when_applying_template_on_private_project() { + PermissionTemplateDto permissionTemplate = dbTester.permissionTemplates().insertTemplate(organization); + dbTester.permissionTemplates().addAnyoneToTemplate(permissionTemplate, "p1"); + dbTester.organizations().setDefaultTemplates(organization, permissionTemplate.getUuid(), null); + + underTest.applyDefault(session, organization.getUuid(), privateProject, creator.getId()); + + assertThat(selectProjectPermissionsOfGroup(organization, null, privateProject)).isEmpty(); + } + + @Test + public void apply_inserts_permissions_to_group_AnyOne_but_USER_and_CODEVIEWER_when_applying_template_on_public_project() { + PermissionTemplateDto permissionTemplate = dbTester.permissionTemplates().insertTemplate(organization); + ProjectPermissions.ALL + .forEach(perm -> dbTester.permissionTemplates().addAnyoneToTemplate(permissionTemplate, perm)); + dbTester.permissionTemplates().addAnyoneToTemplate(permissionTemplate, "p1"); + + underTest.apply(session, permissionTemplate, singletonList(publicProject)); + + assertThat(selectProjectPermissionsOfGroup(organization, null, publicProject)) + .containsOnly("p1", UserRole.ADMIN, UserRole.ISSUE_ADMIN, GlobalPermissions.SCAN_EXECUTION); + } + + @Test + public void applyDefault_inserts_permissions_to_group_AnyOne_but_USER_and_CODEVIEWER_when_applying_template_on_public_project() { + PermissionTemplateDto permissionTemplate = dbTester.permissionTemplates().insertTemplate(organization); + ProjectPermissions.ALL + .forEach(perm -> dbTester.permissionTemplates().addAnyoneToTemplate(permissionTemplate, perm)); + dbTester.permissionTemplates().addAnyoneToTemplate(permissionTemplate, "p1"); + dbTester.organizations().setDefaultTemplates(organization, permissionTemplate.getUuid(), null); + + underTest.applyDefault(session, organization.getUuid(), publicProject, null); + + assertThat(selectProjectPermissionsOfGroup(organization, null, publicProject)) + .containsOnly("p1", UserRole.ADMIN, UserRole.ISSUE_ADMIN, GlobalPermissions.SCAN_EXECUTION); + } + + @Test + public void apply_inserts_any_permissions_to_group_when_applying_template_on_private_project() { + PermissionTemplateDto permissionTemplate = dbTester.permissionTemplates().insertTemplate(organization); + ProjectPermissions.ALL + .forEach(perm -> dbTester.permissionTemplates().addGroupToTemplate(permissionTemplate, group, perm)); + dbTester.permissionTemplates().addGroupToTemplate(permissionTemplate, group, "p1"); + + underTest.apply(session, permissionTemplate, singletonList(privateProject)); + + assertThat(selectProjectPermissionsOfGroup(organization, group, privateProject)) + .containsOnly("p1", UserRole.USER, UserRole.CODEVIEWER, UserRole.ADMIN, UserRole.ISSUE_ADMIN, GlobalPermissions.SCAN_EXECUTION); + } + + @Test + public void applyDefault_inserts_any_permissions_to_group_when_applying_template_on_private_project() { + PermissionTemplateDto permissionTemplate = dbTester.permissionTemplates().insertTemplate(organization); + ProjectPermissions.ALL + .forEach(perm -> dbTester.permissionTemplates().addGroupToTemplate(permissionTemplate, group, perm)); + dbTester.permissionTemplates().addGroupToTemplate(permissionTemplate, group, "p1"); + dbTester.organizations().setDefaultTemplates(organization, permissionTemplate.getUuid(), null); + + underTest.applyDefault(session, organization.getUuid(), privateProject, null); + + assertThat(selectProjectPermissionsOfGroup(organization, group, privateProject)) + .containsOnly("p1", UserRole.USER, UserRole.CODEVIEWER, UserRole.ADMIN, UserRole.ISSUE_ADMIN, GlobalPermissions.SCAN_EXECUTION); + } + + @Test + public void apply_inserts_permissions_to_group_but_USER_and_CODEVIEWER_when_applying_template_on_public_project() { + PermissionTemplateDto permissionTemplate = dbTester.permissionTemplates().insertTemplate(organization); + ProjectPermissions.ALL + .forEach(perm -> dbTester.permissionTemplates().addGroupToTemplate(permissionTemplate, group, perm)); + dbTester.permissionTemplates().addGroupToTemplate(permissionTemplate, group, "p1"); + + underTest.apply(session, permissionTemplate, singletonList(publicProject)); + + assertThat(selectProjectPermissionsOfGroup(organization, group, publicProject)) + .containsOnly("p1", UserRole.ADMIN, UserRole.ISSUE_ADMIN, GlobalPermissions.SCAN_EXECUTION); + } + + @Test + public void applyDefault_inserts_permissions_to_group_but_USER_and_CODEVIEWER_when_applying_template_on_public_project() { + PermissionTemplateDto permissionTemplate = dbTester.permissionTemplates().insertTemplate(organization); + ProjectPermissions.ALL + .forEach(perm -> dbTester.permissionTemplates().addGroupToTemplate(permissionTemplate, group, perm)); + dbTester.permissionTemplates().addGroupToTemplate(permissionTemplate, group, "p1"); + dbTester.organizations().setDefaultTemplates(organization, permissionTemplate.getUuid(), null); + + underTest.applyDefault(session, organization.getUuid(), publicProject, null); + + assertThat(selectProjectPermissionsOfGroup(organization, group, publicProject)) + .containsOnly("p1", UserRole.ADMIN, UserRole.ISSUE_ADMIN, GlobalPermissions.SCAN_EXECUTION); + } + + @Test + public void apply_inserts_permissions_to_user_but_USER_and_CODEVIEWER_when_applying_template_on_public_project() { + PermissionTemplateDto permissionTemplate = dbTester.permissionTemplates().insertTemplate(organization); + ProjectPermissions.ALL + .forEach(perm -> dbTester.permissionTemplates().addUserToTemplate(permissionTemplate, user, perm)); + dbTester.permissionTemplates().addUserToTemplate(permissionTemplate, user, "p1"); + + underTest.apply(session, permissionTemplate, singletonList(publicProject)); + + assertThat(selectProjectPermissionsOfUser(user, publicProject)) + .containsOnly("p1", UserRole.ADMIN, UserRole.ISSUE_ADMIN, GlobalPermissions.SCAN_EXECUTION); + } + + @Test + public void applyDefault_inserts_permissions_to_user_but_USER_and_CODEVIEWER_when_applying_template_on_public_project() { + PermissionTemplateDto permissionTemplate = dbTester.permissionTemplates().insertTemplate(organization); + ProjectPermissions.ALL + .forEach(perm -> dbTester.permissionTemplates().addUserToTemplate(permissionTemplate, user, perm)); + dbTester.permissionTemplates().addUserToTemplate(permissionTemplate, user, "p1"); + dbTester.organizations().setDefaultTemplates(organization, permissionTemplate.getUuid(), null); + + underTest.applyDefault(session, organization.getUuid(), publicProject, null); + + assertThat(selectProjectPermissionsOfUser(user, publicProject)) + .containsOnly("p1", UserRole.ADMIN, UserRole.ISSUE_ADMIN, GlobalPermissions.SCAN_EXECUTION); + } + + @Test + public void apply_inserts_any_permissions_to_user_when_applying_template_on_private_project() { + PermissionTemplateDto permissionTemplate = dbTester.permissionTemplates().insertTemplate(organization); + ProjectPermissions.ALL + .forEach(perm -> dbTester.permissionTemplates().addUserToTemplate(permissionTemplate, user, perm)); + dbTester.permissionTemplates().addUserToTemplate(permissionTemplate, user, "p1"); + + underTest.apply(session, permissionTemplate, singletonList(privateProject)); + + assertThat(selectProjectPermissionsOfUser(user, privateProject)) + .containsOnly("p1", UserRole.USER, UserRole.CODEVIEWER, UserRole.ADMIN, UserRole.ISSUE_ADMIN, GlobalPermissions.SCAN_EXECUTION); + } + + @Test + public void applyDefault_inserts_any_permissions_to_user_when_applying_template_on_private_project() { + PermissionTemplateDto permissionTemplate = dbTester.permissionTemplates().insertTemplate(organization); + ProjectPermissions.ALL + .forEach(perm -> dbTester.permissionTemplates().addUserToTemplate(permissionTemplate, user, perm)); + dbTester.permissionTemplates().addUserToTemplate(permissionTemplate, user, "p1"); + dbTester.organizations().setDefaultTemplates(organization, permissionTemplate.getUuid(), null); + + underTest.applyDefault(session, organization.getUuid(), privateProject, null); + + assertThat(selectProjectPermissionsOfUser(user, privateProject)) + .containsOnly("p1", UserRole.USER, UserRole.CODEVIEWER, UserRole.ADMIN, UserRole.ISSUE_ADMIN, GlobalPermissions.SCAN_EXECUTION); + } + + @Test + public void applyDefault_inserts_permissions_to_ProjectCreator_but_USER_and_CODEVIEWER_when_applying_template_on_public_project() { + PermissionTemplateDto permissionTemplate = dbTester.permissionTemplates().insertTemplate(organization); + ProjectPermissions.ALL + .forEach(perm -> dbTester.permissionTemplates().addProjectCreatorToTemplate(permissionTemplate, perm)); + dbTester.permissionTemplates().addProjectCreatorToTemplate(permissionTemplate, "p1"); + dbTester.organizations().setDefaultTemplates(organization, permissionTemplate.getUuid(), null); + + underTest.applyDefault(session, organization.getUuid(), publicProject, user.getId()); + + assertThat(selectProjectPermissionsOfUser(user, publicProject)) + .containsOnly("p1", UserRole.ADMIN, UserRole.ISSUE_ADMIN, GlobalPermissions.SCAN_EXECUTION); + } + + @Test + public void applyDefault_inserts_any_permissions_to_ProjectCreator_when_applying_template_on_private_project() { + PermissionTemplateDto permissionTemplate = dbTester.permissionTemplates().insertTemplate(organization); + ProjectPermissions.ALL + .forEach(perm -> dbTester.permissionTemplates().addProjectCreatorToTemplate(permissionTemplate, perm)); + dbTester.permissionTemplates().addProjectCreatorToTemplate(permissionTemplate, "p1"); + dbTester.organizations().setDefaultTemplates(organization, permissionTemplate.getUuid(), null); + + underTest.applyDefault(session, organization.getUuid(), privateProject, user.getId()); + + assertThat(selectProjectPermissionsOfUser(user, privateProject)) + .containsOnly("p1", UserRole.USER, UserRole.CODEVIEWER, UserRole.ADMIN, UserRole.ISSUE_ADMIN, GlobalPermissions.SCAN_EXECUTION); + } + @Test public void apply_permission_template() { - OrganizationDto organization = dbTester.organizations().insert(); ComponentDto project = dbTester.components().insertPrivateProject(organization); GroupDto adminGroup = dbTester.users().insertGroup(organization); GroupDto userGroup = dbTester.users().insertGroup(organization); - UserDto user = dbTester.users().insertUser(); dbTester.users().insertPermissionOnGroup(adminGroup, "admin"); dbTester.users().insertPermissionOnGroup(userGroup, "user"); dbTester.users().insertPermissionOnUser(organization, user, "admin"); @@ -87,7 +290,7 @@ public class PermissionTemplateServiceTest { assertThat(selectProjectPermissionsOfGroup(organization, adminGroup, project)).containsOnly("admin", "issueadmin"); assertThat(selectProjectPermissionsOfGroup(organization, userGroup, project)).containsOnly("user", "codeviewer"); - assertThat(selectProjectPermissionsOfGroup(organization, null, project)).containsOnly("user", "codeviewer"); + assertThat(selectProjectPermissionsOfGroup(organization, null, project)).isEmpty(); assertThat(selectProjectPermissionsOfUser(user, project)).containsOnly("admin"); checkAuthorizationUpdatedAtIsUpdated(project); @@ -105,9 +308,6 @@ public class PermissionTemplateServiceTest { @Test public void would_user_have_scan_permission_with_default_permission_template() { - OrganizationDto organization = dbTester.organizations().insert(); - UserDto user = dbTester.users().insertUser(); - GroupDto group = dbTester.users().insertGroup(organization); dbTester.users().insertMember(group, user); PermissionTemplateDto template = templateDb.insertTemplate(organization); dbTester.organizations().setDefaultTemplates(template, null); diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/BulkApplyTemplateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/BulkApplyTemplateActionTest.java index 278282d9068..929eb39b464 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/BulkApplyTemplateActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/BulkApplyTemplateActionTest.java @@ -99,14 +99,16 @@ public class BulkApplyTemplateActionTest extends BasePermissionWsTest