From 4108de32c8e7850b90fecd87ccdcbbb71cdc464d Mon Sep 17 00:00:00 2001 From: Aurelien Poscia Date: Mon, 10 Jul 2023 11:15:56 +0200 Subject: [PATCH] SONAR-19785 update SQ groups permissions from GitHub teams permissions --- .../java/org/sonar/db/entity/EntityDto.java | 5 + .../org/sonar/db/portfolio/PortfolioDto.java | 1 + .../java/org/sonar/db/project/ProjectDto.java | 1 + .../java/org/sonar/db/user/UserDbTester.java | 16 ++- .../server/project/VisibilityService.java | 9 +- .../server/permission/GroupUuidOrAnyone.java | 24 +++- .../permission/PermissionServiceImpl.java | 7 +- .../permission/GroupPermissionChangerIT.java | 110 +++++++----------- .../permission/ws/BasePermissionWsIT.java | 2 +- .../permission/GroupPermissionChange.java | 14 ++- .../permission/GroupPermissionChanger.java | 46 +++----- .../server/permission/PermissionUpdater.java | 57 +++++++-- .../server/permission/ws/AddGroupAction.java | 11 +- .../server/permission/ws/AddUserAction.java | 4 +- .../permission/ws/PermissionWsSupport.java | 12 +- .../permission/ws/RemoveGroupAction.java | 13 ++- .../permission/ws/RemoveUserAction.java | 4 +- .../ws/template/AddGroupToTemplateAction.java | 2 +- .../RemoveGroupFromTemplateAction.java | 2 +- .../server/usergroups/ws/GroupWsSupport.java | 11 +- 20 files changed, 195 insertions(+), 156 deletions(-) diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/entity/EntityDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/entity/EntityDto.java index e6d20322ab5..5a4edcba038 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/entity/EntityDto.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/entity/EntityDto.java @@ -105,4 +105,9 @@ public class EntityDto { public int hashCode() { return Objects.hash(uuid); } + + public T setPrivate(boolean aPrivate) { + isPrivate = aPrivate; + return (T) this; + } } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/portfolio/PortfolioDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/portfolio/PortfolioDto.java index 50df9b0895e..69124b36603 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/portfolio/PortfolioDto.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/portfolio/PortfolioDto.java @@ -138,6 +138,7 @@ public class PortfolioDto extends EntityDto { return setKee(key); } + @Override public PortfolioDto setPrivate(boolean aPrivate) { isPrivate = aPrivate; return this; diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/project/ProjectDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/project/ProjectDto.java index 81190a9f61a..dac8567ad3d 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/project/ProjectDto.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/project/ProjectDto.java @@ -69,6 +69,7 @@ public class ProjectDto extends EntityDto { return setKee(key); } + @Override public ProjectDto setPrivate(boolean aPrivate) { isPrivate = aPrivate; return this; diff --git a/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/user/UserDbTester.java b/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/user/UserDbTester.java index 774ae5eff78..e2b220e596b 100644 --- a/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/user/UserDbTester.java +++ b/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/user/UserDbTester.java @@ -45,6 +45,7 @@ import org.sonar.db.scim.ScimUserDto; import static com.google.common.base.Preconditions.checkArgument; import static java.util.Arrays.stream; +import static java.util.stream.Collectors.toSet; import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.apache.commons.lang.math.RandomUtils.nextLong; import static org.sonar.db.permission.GlobalPermission.ADMINISTER; @@ -241,12 +242,18 @@ public class UserDbTester { return insertPermissionOnAnyone(permission.getKey()); } + public Set insertPermissionsOnGroup(GroupDto group, String... permissions) { + return stream(permissions) + .map(p -> insertPermissionOnGroup(group, p)) + .collect(toSet()); + } + public GroupPermissionDto insertPermissionOnGroup(GroupDto group, String permission) { GroupPermissionDto dto = new GroupPermissionDto() .setUuid(Uuids.createFast()) .setGroupUuid(group.getUuid()) .setRole(permission); - db.getDbClient().groupPermissionDao().insert(db.getSession(), dto, (EntityDto) null, null); + db.getDbClient().groupPermissionDao().insert(db.getSession(), dto, null, null); db.commit(); return dto; } @@ -321,6 +328,12 @@ public class UserDbTester { return dto; } + public Set insertEntityPermissionsOnGroup(GroupDto group, EntityDto entity, String... permissions) { + return stream(permissions) + .map(permission -> insertEntityPermissionOnGroup(group, permission, entity)) + .collect(toSet()); + } + public GroupPermissionDto insertEntityPermissionOnGroup(GroupDto group, String permission, EntityDto entity) { checkArgument(entity.isPrivate() || !PUBLIC_PERMISSIONS.contains(permission), "%s can't be granted on a public entity (project or portfolio)", permission); @@ -426,7 +439,6 @@ public class UserDbTester { return db.getDbClient().userPermissionDao().selectEntityPermissionsOfUser(db.getSession(), user.getUuid(), entityUuid); } - private static List toListOfGlobalPermissions(List keys) { return keys .stream() diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/project/VisibilityService.java b/server/sonar-server-common/src/main/java/org/sonar/server/project/VisibilityService.java index d9e57b1672e..8768b1d5625 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/project/VisibilityService.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/project/VisibilityService.java @@ -51,13 +51,6 @@ public class VisibilityService { this.indexers = indexers; this.uuidFactory = uuidFactory; } - public void changeVisibility(String entityUuid, boolean isPrivate) { - try (DbSession dbSession = dbClient.openSession(false)) { - EntityDto entityDto = dbClient.entityDao().selectByUuid(dbSession, entityUuid) - .orElseThrow(() -> new IllegalStateException("Component must be a project, a portfolio or an application")); - changeVisibility(entityDto, isPrivate); - } - } public void changeVisibility(EntityDto entityDto, boolean isPrivate) { try (DbSession dbSession = dbClient.openSession(false)) { @@ -90,13 +83,13 @@ public class VisibilityService { if (entity.isProjectOrApp()) { dbClient.projectDao().updateVisibility(dbSession, entity.getUuid(), newIsPrivate); - dbClient.branchDao().selectByProjectUuid(dbSession, entity.getUuid()).stream() .filter(branch -> !branch.isMain()) .forEach(branch -> dbClient.componentDao().setPrivateForBranchUuidWithoutAuditLog(dbSession, branch.getUuid(), newIsPrivate)); } else { dbClient.portfolioDao().updateVisibilityByPortfolioUuid(dbSession, entity.getUuid(), newIsPrivate); } + entity.setPrivate(newIsPrivate); } private void updatePermissionsToPrivate(DbSession dbSession, EntityDto entity) { diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/permission/GroupUuidOrAnyone.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/permission/GroupUuidOrAnyone.java index 026e38fa1bb..2d92846e617 100644 --- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/permission/GroupUuidOrAnyone.java +++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/permission/GroupUuidOrAnyone.java @@ -19,6 +19,8 @@ */ package org.sonar.server.permission; +import java.util.Objects; +import java.util.Optional; import javax.annotation.CheckForNull; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; @@ -49,11 +51,29 @@ public class GroupUuidOrAnyone { return uuid; } - public static GroupUuidOrAnyone from(GroupDto dto) { - return new GroupUuidOrAnyone(dto.getUuid()); + public static GroupUuidOrAnyone from(@Nullable GroupDto dto) { + String groupUuid = Optional.ofNullable(dto).map(GroupDto::getUuid).orElse(null); + return new GroupUuidOrAnyone(groupUuid); } public static GroupUuidOrAnyone forAnyone() { return new GroupUuidOrAnyone(null); } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + GroupUuidOrAnyone that = (GroupUuidOrAnyone) o; + return Objects.equals(uuid, that.uuid); + } + + @Override + public int hashCode() { + return Objects.hash(uuid); + } } diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/permission/PermissionServiceImpl.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/permission/PermissionServiceImpl.java index f8f75ebd7eb..30514238375 100644 --- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/permission/PermissionServiceImpl.java +++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/permission/PermissionServiceImpl.java @@ -19,7 +19,10 @@ */ package org.sonar.server.permission; +import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import javax.annotation.concurrent.Immutable; import org.sonar.api.resources.Qualifiers; import org.sonar.api.resources.ResourceTypes; @@ -28,14 +31,14 @@ import org.sonar.db.permission.GlobalPermission; @Immutable public class PermissionServiceImpl implements PermissionService { - private static final List ALL_PROJECT_PERMISSIONS = List.of( + public static final Set ALL_PROJECT_PERMISSIONS = Collections.unmodifiableSet(new LinkedHashSet<>(List.of( UserRole.ADMIN, UserRole.CODEVIEWER, UserRole.ISSUE_ADMIN, UserRole.SECURITYHOTSPOT_ADMIN, UserRole.SCAN, UserRole.USER - ); + ))); private static final List ALL_GLOBAL_PERMISSIONS = List.of(GlobalPermission.values()); diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/GroupPermissionChangerIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/GroupPermissionChangerIT.java index f192a7ee679..07179bf33e7 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/GroupPermissionChangerIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/GroupPermissionChangerIT.java @@ -19,6 +19,7 @@ */ package org.sonar.server.permission; +import java.util.Set; import org.apache.commons.lang.StringUtils; import org.junit.Before; import org.junit.Rule; @@ -29,6 +30,7 @@ import org.sonar.api.utils.System2; import org.sonar.api.web.UserRole; import org.sonar.core.util.SequenceUuidFactory; import org.sonar.core.util.Uuids; +import org.sonar.db.DbSession; import org.sonar.db.DbTester; import org.sonar.db.component.ResourceTypesRule; import org.sonar.db.permission.GlobalPermission; @@ -68,28 +70,23 @@ public class GroupPermissionChangerIT { @Test public void apply_adds_global_permission_to_group() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.from(group); - - apply(new GroupPermissionChange(PermissionChange.Operation.ADD, ADMINISTER_QUALITY_PROFILES.getKey(), null, groupUuid, permissionService)); + apply(new GroupPermissionChange(PermissionChange.Operation.ADD, ADMINISTER_QUALITY_PROFILES.getKey(), null, group, permissionService)); assertThat(db.users().selectGroupPermissions(group, null)).containsOnly(ADMINISTER_QUALITY_PROFILES.getKey()); } @Test public void apply_adds_global_permission_to_group_AnyOne() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.forAnyone(); - - apply(new GroupPermissionChange(PermissionChange.Operation.ADD, ADMINISTER_QUALITY_PROFILES.getKey(), null, groupUuid, permissionService)); + apply(new GroupPermissionChange(PermissionChange.Operation.ADD, ADMINISTER_QUALITY_PROFILES.getKey(), null, null, permissionService)); assertThat(db.users().selectAnyonePermissions(null)).containsOnly(ADMINISTER_QUALITY_PROFILES.getKey()); } @Test public void apply_fails_with_BadRequestException_when_adding_any_permission_to_group_AnyOne_on_private_project() { - GroupUuidOrAnyone anyOneGroup = GroupUuidOrAnyone.forAnyone(); permissionService.getAllProjectPermissions() .forEach(perm -> { - GroupPermissionChange change = new GroupPermissionChange(PermissionChange.Operation.ADD, perm, privateProject, anyOneGroup, permissionService); + GroupPermissionChange change = new GroupPermissionChange(PermissionChange.Operation.ADD, perm, privateProject, null, permissionService); try { apply(change); fail("a BadRequestException should have been thrown"); @@ -104,10 +101,9 @@ public class GroupPermissionChangerIT { permissionService.getAllProjectPermissions() .forEach(this::unsafeInsertProjectPermissionOnAnyone); - GroupUuidOrAnyone anyOneGroup = GroupUuidOrAnyone.forAnyone(); permissionService.getAllProjectPermissions() .forEach(perm -> { - apply(new GroupPermissionChange(PermissionChange.Operation.REMOVE, perm, privateProject, anyOneGroup, permissionService)); + apply(new GroupPermissionChange(PermissionChange.Operation.REMOVE, perm, privateProject, null, permissionService)); assertThat(db.users().selectAnyonePermissions(privateProject.getUuid())).contains(perm); }); @@ -139,9 +135,8 @@ public class GroupPermissionChangerIT { } private void applyAddsPermissionToGroupOnPrivateProject(String permission) { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.from(group); - apply(new GroupPermissionChange(PermissionChange.Operation.ADD, permission, privateProject, groupUuid, permissionService)); + apply(new GroupPermissionChange(PermissionChange.Operation.ADD, permission, privateProject, group, permissionService)); assertThat(db.users().selectGroupPermissions(group, null)).isEmpty(); assertThat(db.users().selectGroupPermissions(group, privateProject)).containsOnly(permission); @@ -173,73 +168,61 @@ public class GroupPermissionChangerIT { } private void applyRemovesPermissionFromGroupOnPrivateProject(String permission) { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.from(group); db.users().insertEntityPermissionOnGroup(group, permission, privateProject); - apply(new GroupPermissionChange(PermissionChange.Operation.ADD, permission, privateProject, groupUuid, permissionService)); + apply(new GroupPermissionChange(PermissionChange.Operation.ADD, permission, privateProject, group, permissionService), permission); assertThat(db.users().selectGroupPermissions(group, privateProject)).containsOnly(permission); } @Test public void apply_has_no_effect_when_adding_USER_permission_to_group_AnyOne_on_a_public_project() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.forAnyone(); - - apply(new GroupPermissionChange(PermissionChange.Operation.ADD, UserRole.USER, publicProject, groupUuid, permissionService)); + apply(new GroupPermissionChange(PermissionChange.Operation.ADD, UserRole.USER, publicProject, null, permissionService)); assertThat(db.users().selectAnyonePermissions(publicProject.getUuid())).isEmpty(); } @Test public void apply_has_no_effect_when_adding_CODEVIEWER_permission_to_group_AnyOne_on_a_public_project() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.forAnyone(); - - apply(new GroupPermissionChange(PermissionChange.Operation.ADD, UserRole.CODEVIEWER, publicProject, groupUuid, permissionService)); + apply(new GroupPermissionChange(PermissionChange.Operation.ADD, UserRole.CODEVIEWER, publicProject, null, permissionService)); assertThat(db.users().selectAnyonePermissions(publicProject.getUuid())).isEmpty(); } @Test public void apply_fails_with_BadRequestException_when_adding_permission_ADMIN_to_group_AnyOne_on_a_public_project() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.forAnyone(); - - assertThatThrownBy(() -> apply(new GroupPermissionChange(PermissionChange.Operation.ADD, UserRole.ADMIN, publicProject, groupUuid, permissionService))) + GroupPermissionChange change = new GroupPermissionChange(PermissionChange.Operation.ADD, UserRole.ADMIN, publicProject, null, permissionService); + assertThatThrownBy(() -> apply(change)) .isInstanceOf(BadRequestException.class) .hasMessage("It is not possible to add the 'admin' permission to group 'Anyone'."); } @Test public void apply_adds_permission_ISSUE_ADMIN_to_group_AnyOne_on_a_public_project() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.forAnyone(); - - apply(new GroupPermissionChange(PermissionChange.Operation.ADD, UserRole.ISSUE_ADMIN, publicProject, groupUuid, permissionService)); + apply(new GroupPermissionChange(PermissionChange.Operation.ADD, UserRole.ISSUE_ADMIN, publicProject, null, permissionService)); assertThat(db.users().selectAnyonePermissions(publicProject.getUuid())).containsOnly(UserRole.ISSUE_ADMIN); } @Test public void apply_adds_permission_SCAN_EXECUTION_to_group_AnyOne_on_a_public_project() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.forAnyone(); - - apply(new GroupPermissionChange(PermissionChange.Operation.ADD, GlobalPermission.SCAN.getKey(), publicProject, groupUuid, permissionService)); + apply(new GroupPermissionChange(PermissionChange.Operation.ADD, GlobalPermission.SCAN.getKey(), publicProject, null, permissionService)); assertThat(db.users().selectAnyonePermissions(publicProject.getUuid())).containsOnly(GlobalPermission.SCAN.getKey()); } @Test public void apply_fails_with_BadRequestException_when_removing_USER_permission_from_group_AnyOne_on_a_public_project() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.forAnyone(); - - assertThatThrownBy(() -> apply(new GroupPermissionChange(PermissionChange.Operation.REMOVE, UserRole.USER, publicProject, groupUuid, permissionService))) + GroupPermissionChange change = new GroupPermissionChange(PermissionChange.Operation.REMOVE, UserRole.USER, publicProject, null, permissionService); + assertThatThrownBy(() -> apply(change)) .isInstanceOf(BadRequestException.class) .hasMessage("Permission user can't be removed from a public component"); } @Test public void apply_fails_with_BadRequestException_when_removing_CODEVIEWER_permission_from_group_AnyOne_on_a_public_project() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.forAnyone(); - - assertThatThrownBy(() -> apply(new GroupPermissionChange(PermissionChange.Operation.REMOVE, UserRole.CODEVIEWER, publicProject, groupUuid, permissionService))) + GroupPermissionChange change = new GroupPermissionChange(PermissionChange.Operation.REMOVE, UserRole.CODEVIEWER, publicProject, null, permissionService); + assertThatThrownBy(() -> apply(change)) .isInstanceOf(BadRequestException.class) .hasMessage("Permission codeviewer can't be removed from a public component"); } @@ -260,37 +243,32 @@ public class GroupPermissionChangerIT { } private void applyRemovesPermissionFromGroupAnyOneOnAPublicProject(String permission) { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.forAnyone(); db.users().insertEntityPermissionOnAnyone(permission, publicProject); - apply(new GroupPermissionChange(PermissionChange.Operation.REMOVE, permission, publicProject, groupUuid, permissionService)); + apply(new GroupPermissionChange(PermissionChange.Operation.REMOVE, permission, publicProject, null, permissionService), permission); assertThat(db.users().selectAnyonePermissions(publicProject.getUuid())).isEmpty(); } @Test public void apply_fails_with_BadRequestException_when_removing_USER_permission_from_a_group_on_a_public_project() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.from(group); - - assertThatThrownBy(() -> apply(new GroupPermissionChange(PermissionChange.Operation.REMOVE, UserRole.USER, publicProject, groupUuid, permissionService))) + GroupPermissionChange change = new GroupPermissionChange(PermissionChange.Operation.REMOVE, UserRole.USER, publicProject, group, permissionService); + assertThatThrownBy(() -> apply(change)) .isInstanceOf(BadRequestException.class) .hasMessage("Permission user can't be removed from a public component"); } @Test public void apply_fails_with_BadRequestException_when_removing_CODEVIEWER_permission_from_a_group_on_a_public_project() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.from(group); - - assertThatThrownBy(() -> apply(new GroupPermissionChange(PermissionChange.Operation.REMOVE, UserRole.CODEVIEWER, publicProject, groupUuid, permissionService))) + GroupPermissionChange change = new GroupPermissionChange(PermissionChange.Operation.REMOVE, UserRole.CODEVIEWER, publicProject, group, permissionService); + assertThatThrownBy(() -> apply(change)) .isInstanceOf(BadRequestException.class) .hasMessage("Permission codeviewer can't be removed from a public component"); } @Test public void add_permission_to_anyone() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.forAnyone(); - - apply(new GroupPermissionChange(PermissionChange.Operation.ADD, ADMINISTER_QUALITY_PROFILES.getKey(), null, groupUuid, permissionService)); + apply(new GroupPermissionChange(PermissionChange.Operation.ADD, ADMINISTER_QUALITY_PROFILES.getKey(), null, null, permissionService)); assertThat(db.users().selectGroupPermissions(group, null)).isEmpty(); assertThat(db.users().selectAnyonePermissions(null)).containsOnly(ADMINISTER_QUALITY_PROFILES.getKey()); @@ -298,24 +276,21 @@ public class GroupPermissionChangerIT { @Test public void do_nothing_when_adding_permission_that_already_exists() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.from(group); db.users().insertPermissionOnGroup(group, ADMINISTER_QUALITY_GATES); - apply(new GroupPermissionChange(PermissionChange.Operation.ADD, ADMINISTER_QUALITY_GATES.getKey(), null, groupUuid, permissionService)); + apply(new GroupPermissionChange(PermissionChange.Operation.ADD, ADMINISTER_QUALITY_GATES.getKey(), null, group, permissionService)); assertThat(db.users().selectGroupPermissions(group, null)).containsOnly(ADMINISTER_QUALITY_GATES.getKey()); } @Test public void fail_to_add_global_permission_but_SCAN_and_ADMIN_on_private_project() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.from(group); - permissionService.getGlobalPermissions().stream() .map(GlobalPermission::getKey) .filter(perm -> !UserRole.ADMIN.equals(perm) && !GlobalPermission.SCAN.getKey().equals(perm)) .forEach(perm -> { try { - new GroupPermissionChange(PermissionChange.Operation.ADD, perm, privateProject, groupUuid, permissionService); + new GroupPermissionChange(PermissionChange.Operation.ADD, perm, privateProject, group, permissionService); fail("a BadRequestException should have been thrown for permission " + perm); } catch (BadRequestException e) { assertThat(e).hasMessage("Invalid project permission '" + perm + @@ -326,14 +301,12 @@ public class GroupPermissionChangerIT { @Test public void fail_to_add_global_permission_but_SCAN_and_ADMIN_on_public_project() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.from(group); - permissionService.getGlobalPermissions().stream() .map(GlobalPermission::getKey) .filter(perm -> !UserRole.ADMIN.equals(perm) && !GlobalPermission.SCAN.getKey().equals(perm)) .forEach(perm -> { try { - new GroupPermissionChange(PermissionChange.Operation.ADD, perm, publicProject, groupUuid, permissionService); + new GroupPermissionChange(PermissionChange.Operation.ADD, perm, publicProject, group, permissionService); fail("a BadRequestException should have been thrown for permission " + perm); } catch (BadRequestException e) { assertThat(e).hasMessage("Invalid project permission '" + perm + @@ -344,14 +317,12 @@ public class GroupPermissionChangerIT { @Test public void fail_to_add_project_permission_but_SCAN_and_ADMIN_on_global_group() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.from(group); - permissionService.getAllProjectPermissions() .stream() .filter(perm -> !GlobalPermission.SCAN.getKey().equals(perm) && !GlobalPermission.ADMINISTER.getKey().equals(perm)) .forEach(permission -> { try { - new GroupPermissionChange(PermissionChange.Operation.ADD, permission, null, groupUuid, permissionService); + new GroupPermissionChange(PermissionChange.Operation.ADD, permission, null, group, permissionService); fail("a BadRequestException should have been thrown for permission " + permission); } catch (BadRequestException e) { assertThat(e).hasMessage("Invalid global permission '" + permission + "'. Valid values are [admin, gateadmin, profileadmin, provisioning, scan]"); @@ -361,23 +332,23 @@ public class GroupPermissionChangerIT { @Test public void remove_permission_from_group() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.from(group); db.users().insertPermissionOnGroup(group, ADMINISTER_QUALITY_GATES); db.users().insertPermissionOnGroup(group, PROVISION_PROJECTS); - apply(new GroupPermissionChange(PermissionChange.Operation.REMOVE, ADMINISTER_QUALITY_GATES.getKey(), null, groupUuid, permissionService)); + apply(new GroupPermissionChange(PermissionChange.Operation.REMOVE, ADMINISTER_QUALITY_GATES.getKey(), null, group, permissionService), ADMINISTER_QUALITY_GATES.getKey(), + PROVISION_PROJECTS.getKey()); assertThat(db.users().selectGroupPermissions(group, null)).containsOnly(PROVISION_PROJECTS.getKey()); } @Test public void remove_project_permission_from_group() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.from(group); db.users().insertPermissionOnGroup(group, ADMINISTER_QUALITY_GATES); db.users().insertEntityPermissionOnGroup(group, UserRole.ISSUE_ADMIN, privateProject); db.users().insertEntityPermissionOnGroup(group, UserRole.CODEVIEWER, privateProject); - apply(new GroupPermissionChange(PermissionChange.Operation.REMOVE, UserRole.ISSUE_ADMIN, privateProject, groupUuid, permissionService)); + apply(new GroupPermissionChange(PermissionChange.Operation.REMOVE, UserRole.ISSUE_ADMIN, privateProject, group, permissionService), UserRole.ISSUE_ADMIN, + UserRole.CODEVIEWER); assertThat(db.users().selectGroupPermissions(group, null)).containsOnly(ADMINISTER_QUALITY_GATES.getKey()); assertThat(db.users().selectGroupPermissions(group, privateProject)).containsOnly(UserRole.CODEVIEWER); @@ -385,9 +356,7 @@ public class GroupPermissionChangerIT { @Test public void do_not_fail_if_removing_a_permission_that_does_not_exist() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.from(group); - - apply(new GroupPermissionChange(PermissionChange.Operation.REMOVE, UserRole.ISSUE_ADMIN, privateProject, groupUuid, permissionService)); + apply(new GroupPermissionChange(PermissionChange.Operation.REMOVE, UserRole.ISSUE_ADMIN, privateProject, group, permissionService)); assertThat(db.users().selectGroupPermissions(group, null)).isEmpty(); assertThat(db.users().selectGroupPermissions(group, privateProject)).isEmpty(); @@ -395,28 +364,29 @@ public class GroupPermissionChangerIT { @Test public void fail_to_remove_admin_permission_if_no_more_admins() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.from(group); db.users().insertPermissionOnGroup(group, ADMINISTER); - assertThatThrownBy(() -> underTest.apply(db.getSession(), new GroupPermissionChange(PermissionChange.Operation.REMOVE, ADMINISTER.getKey(), null, groupUuid, permissionService))) + GroupPermissionChange change = new GroupPermissionChange(PermissionChange.Operation.REMOVE, ADMINISTER.getKey(), null, group, permissionService); + Set permission = Set.of("admin"); + DbSession session = db.getSession(); + assertThatThrownBy(() -> underTest.apply(session, permission, change)) .isInstanceOf(BadRequestException.class) .hasMessage("Last group with permission 'admin'. Permission cannot be removed."); } @Test public void remove_admin_group_if_still_other_admins() { - GroupUuidOrAnyone groupUuid = GroupUuidOrAnyone.from(group); db.users().insertPermissionOnGroup(group, ADMINISTER); UserDto admin = db.users().insertUser(); db.users().insertGlobalPermissionOnUser(admin, ADMINISTER); - apply(new GroupPermissionChange(PermissionChange.Operation.REMOVE, ADMINISTER.getKey(), null, groupUuid, permissionService)); + apply(new GroupPermissionChange(PermissionChange.Operation.REMOVE, ADMINISTER.getKey(), null, group, permissionService), ADMINISTER.getKey()); assertThat(db.users().selectGroupPermissions(group, null)).isEmpty(); } - private void apply(GroupPermissionChange change) { - underTest.apply(db.getSession(), change); + private void apply(GroupPermissionChange change, String... existingPermissions) { + underTest.apply(db.getSession(), Set.of(existingPermissions), change); db.commit(); } diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/BasePermissionWsIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/BasePermissionWsIT.java index 600cd56fe5f..2d02b766cf2 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/BasePermissionWsIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/BasePermissionWsIT.java @@ -84,7 +84,7 @@ public abstract class BasePermissionWsIT { return new PermissionUpdater( new IndexersImpl(new PermissionIndexer(db.getDbClient(), es.client())), new UserPermissionChanger(db.getDbClient(), new SequenceUuidFactory()), - new GroupPermissionChanger(db.getDbClient(), new SequenceUuidFactory())); + new GroupPermissionChanger(db.getDbClient(), new SequenceUuidFactory()), db.getDbClient()); } protected TestRequest newRequest() { diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/GroupPermissionChange.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/GroupPermissionChange.java index b4e3ec65afe..32f6e9147ee 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/GroupPermissionChange.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/GroupPermissionChange.java @@ -19,20 +19,26 @@ */ package org.sonar.server.permission; +import java.util.Optional; import javax.annotation.Nullable; import org.sonar.db.entity.EntityDto; +import org.sonar.db.user.GroupDto; public class GroupPermissionChange extends PermissionChange { - private final GroupUuidOrAnyone group; + private final GroupDto groupDto; public GroupPermissionChange(Operation operation, String permission, @Nullable EntityDto entityDto, - GroupUuidOrAnyone group, PermissionService permissionService) { + @Nullable GroupDto groupDto, PermissionService permissionService) { super(operation, permission, entityDto, permissionService); - this.group = group; + this.groupDto = groupDto; } public GroupUuidOrAnyone getGroupUuidOrAnyone() { - return group; + return GroupUuidOrAnyone.from(groupDto); + } + + public Optional getGroupName() { + return Optional.ofNullable(groupDto).map(GroupDto::getName); } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/GroupPermissionChanger.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/GroupPermissionChanger.java index 6b08f4e7b85..9a971d52560 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/GroupPermissionChanger.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/GroupPermissionChanger.java @@ -19,8 +19,7 @@ */ package org.sonar.server.permission; -import java.util.List; -import java.util.Optional; +import java.util.Set; import org.sonar.api.web.UserRole; import org.sonar.core.util.UuidFactory; import org.sonar.db.DbClient; @@ -28,7 +27,6 @@ import org.sonar.db.DbSession; import org.sonar.db.entity.EntityDto; import org.sonar.db.permission.GlobalPermission; import org.sonar.db.permission.GroupPermissionDto; -import org.sonar.db.user.GroupDto; import static com.google.common.base.Preconditions.checkNotNull; import static java.lang.String.format; @@ -46,15 +44,21 @@ public class GroupPermissionChanger { this.uuidFactory = uuidFactory; } - public boolean apply(DbSession dbSession, GroupPermissionChange change) { + public boolean apply(DbSession dbSession, Set existingPermissions, GroupPermissionChange change) { ensureConsistencyWithVisibility(change); if (isImplicitlyAlreadyDone(change)) { return false; } switch (change.getOperation()) { case ADD: + if (existingPermissions.contains(change.getPermission())) { + return false; + } return addPermission(dbSession, change); case REMOVE: + if (!existingPermissions.contains(change.getPermission())) { + return false; + } return removePermission(dbSession, change); default: throw new UnsupportedOperationException("Unsupported permission change: " + change.getOperation()); @@ -111,24 +115,18 @@ public class GroupPermissionChanger { } private boolean addPermission(DbSession dbSession, GroupPermissionChange change) { - if (loadExistingPermissions(dbSession, change).contains(change.getPermission())) { - return false; - } - validateNotAnyoneAndAdminPermission(change.getPermission(), change.getGroupUuidOrAnyone()); String groupUuid = change.getGroupUuidOrAnyone().getUuid(); + String groupName = change.getGroupName().orElse(null); + GroupPermissionDto addedDto = new GroupPermissionDto() .setUuid(uuidFactory.create()) .setRole(change.getPermission()) .setGroupUuid(groupUuid) .setEntityName(change.getProjectName()) - .setEntityUuid(change.getProjectUuid()); - - Optional.ofNullable(groupUuid) - .map(uuid -> dbClient.groupDao().selectByUuid(dbSession, groupUuid)) - .map(GroupDto::getName) - .ifPresent(addedDto::setGroupName); + .setEntityUuid(change.getProjectUuid()) + .setGroupName(groupName); dbClient.groupPermissionDao().insert(dbSession, addedDto, change.getEntity(), null); return true; @@ -140,16 +138,9 @@ public class GroupPermissionChanger { } private boolean removePermission(DbSession dbSession, GroupPermissionChange change) { - if (!loadExistingPermissions(dbSession, change).contains(change.getPermission())) { - return false; - } checkIfRemainingGlobalAdministrators(dbSession, change); String groupUuid = change.getGroupUuidOrAnyone().getUuid(); - String groupName = Optional.ofNullable(groupUuid) - .map(uuid -> dbClient.groupDao().selectByUuid(dbSession, uuid)) - .map(GroupDto::getName) - .orElse(null); - + String groupName = change.getGroupName().orElse(null); dbClient.groupPermissionDao().delete(dbSession, change.getPermission(), groupUuid, @@ -158,17 +149,6 @@ public class GroupPermissionChanger { return true; } - private List loadExistingPermissions(DbSession dbSession, GroupPermissionChange change) { - String projectUuid = change.getProjectUuid(); - if (projectUuid != null) { - return dbClient.groupPermissionDao().selectEntityPermissionsOfGroup(dbSession, - change.getGroupUuidOrAnyone().getUuid(), - projectUuid); - } - return dbClient.groupPermissionDao().selectGlobalPermissionsOfGroup(dbSession, - change.getGroupUuidOrAnyone().getUuid()); - } - private void checkIfRemainingGlobalAdministrators(DbSession dbSession, GroupPermissionChange change) { GroupUuidOrAnyone groupUuidOrAnyone = change.getGroupUuidOrAnyone(); if (GlobalPermission.ADMINISTER.getKey().equals(change.getPermission()) && diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/PermissionUpdater.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/PermissionUpdater.java index 3564da99516..6878a2d7657 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/PermissionUpdater.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/PermissionUpdater.java @@ -21,10 +21,18 @@ package org.sonar.server.permission; import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Set; +import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.server.es.Indexers; +import static java.util.stream.Collectors.groupingBy; +import static org.sonar.api.utils.Preconditions.checkState; +import static org.sonar.server.es.Indexers.EntityEvent.PERMISSION_CHANGE; + /** * Add or remove global/project permissions to a group. This class does not verify that caller has administration right on the related project. */ @@ -33,34 +41,59 @@ public class PermissionUpdater { private final Indexers indexers; private final UserPermissionChanger userPermissionChanger; private final GroupPermissionChanger groupPermissionChanger; + private final DbClient dbClient; public PermissionUpdater(Indexers indexers, - UserPermissionChanger userPermissionChanger, GroupPermissionChanger groupPermissionChanger) { + UserPermissionChanger userPermissionChanger, GroupPermissionChanger groupPermissionChanger, DbClient dbClient) { this.indexers = indexers; this.userPermissionChanger = userPermissionChanger; this.groupPermissionChanger = groupPermissionChanger; + this.dbClient = dbClient; } - public void apply(DbSession dbSession, Collection changes) { + public void applyForUser(DbSession dbSession, Collection changes) { List projectOrViewUuids = new ArrayList<>(); - for (PermissionChange change : changes) { - boolean changed = doApply(dbSession, change); + for (UserPermissionChange change : changes) { + boolean changed = userPermissionChanger.apply(dbSession, change); String projectUuid = change.getProjectUuid(); if (changed && projectUuid != null) { projectOrViewUuids.add(projectUuid); } } - indexers.commitAndIndexOnEntityEvent(dbSession, projectOrViewUuids, Indexers.EntityEvent.PERMISSION_CHANGE); + indexers.commitAndIndexOnEntityEvent(dbSession, projectOrViewUuids, PERMISSION_CHANGE); } - private boolean doApply(DbSession dbSession, PermissionChange change) { - if (change instanceof UserPermissionChange userPermissionChange) { - return userPermissionChanger.apply(dbSession, userPermissionChange); - } - if (change instanceof GroupPermissionChange groupPermissionChange) { - return groupPermissionChanger.apply(dbSession, groupPermissionChange); + public void applyForGroups(DbSession dbSession, Collection groupsPermissionChanges) { + checkState(groupsPermissionChanges.stream().map(PermissionChange::getProjectUuid).distinct().count() <= 1, + "Only one project per group of changes is supported"); + + List projectOrViewUuids = new ArrayList<>(); + Map> groupUuidToChanges = groupsPermissionChanges.stream().collect(groupingBy(GroupPermissionChange::getGroupUuidOrAnyone)); + groupUuidToChanges.values().forEach(groupPermissionChanges -> applyForSingleGroup(dbSession, projectOrViewUuids, groupPermissionChanges)); + + indexers.commitAndIndexOnEntityEvent(dbSession, projectOrViewUuids, PERMISSION_CHANGE); + } + + private void applyForSingleGroup(DbSession dbSession, List projectOrViewUuids, List groupPermissionChanges) { + GroupPermissionChange anyGroupPermissionChange = groupPermissionChanges.iterator().next(); + Set existingPermissions = loadExistingEntityPermissions(dbSession, anyGroupPermissionChange); + for (GroupPermissionChange groupPermissionChange : groupPermissionChanges) { + if (doApplyForGroup(dbSession, existingPermissions, groupPermissionChange) && groupPermissionChange.getProjectUuid() != null) { + projectOrViewUuids.add(groupPermissionChange.getProjectUuid()); + } } - throw new UnsupportedOperationException("Unsupported permission change: " + change.getClass()); + } + private boolean doApplyForGroup(DbSession dbSession, Set existingPermissions, GroupPermissionChange change) { + return groupPermissionChanger.apply(dbSession, existingPermissions, change); + } + + private Set loadExistingEntityPermissions(DbSession dbSession, GroupPermissionChange change) { + String projectUuid = change.getProjectUuid(); + String groupUuid = change.getGroupUuidOrAnyone().getUuid(); + if (projectUuid != null) { + return new HashSet<>(dbClient.groupPermissionDao().selectEntityPermissionsOfGroup(dbSession, groupUuid, projectUuid)); + } + return new HashSet<>(dbClient.groupPermissionDao().selectGlobalPermissionsOfGroup(dbSession, groupUuid)); } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/AddGroupAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/AddGroupAction.java index 95b5b869798..7b8be1df85f 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/AddGroupAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/AddGroupAction.java @@ -27,8 +27,8 @@ import org.sonar.api.server.ws.WebService; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.entity.EntityDto; +import org.sonar.db.user.GroupDto; import org.sonar.server.permission.GroupPermissionChange; -import org.sonar.server.permission.GroupUuidOrAnyone; import org.sonar.server.permission.PermissionChange; import org.sonar.server.permission.PermissionService; import org.sonar.server.permission.PermissionUpdater; @@ -84,15 +84,16 @@ public class AddGroupAction implements PermissionsWsAction { @Override public void handle(Request request, Response response) throws Exception { try (DbSession dbSession = dbClient.openSession(false)) { - GroupUuidOrAnyone group = wsSupport.findGroup(dbSession, request); + GroupDto groupDto = wsSupport.findGroupDtoOrNullIfAnyone(dbSession, request); EntityDto project = wsSupport.findEntity(dbSession, request); wsSupport.checkPermissionManagementAccess(userSession, project); - PermissionChange change = new GroupPermissionChange( + GroupPermissionChange change = new GroupPermissionChange( PermissionChange.Operation.ADD, request.mandatoryParam(PARAM_PERMISSION), project, - group, permissionService); - permissionUpdater.apply(dbSession, List.of(change)); + groupDto, + permissionService); + permissionUpdater.applyForGroups(dbSession, List.of(change)); } response.noContent(); } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/AddUserAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/AddUserAction.java index 3c78c50cc02..ea55cb5c31c 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/AddUserAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/AddUserAction.java @@ -90,12 +90,12 @@ public class AddUserAction implements PermissionsWsAction { checkProjectAdmin(userSession, configuration, entity); UserId user = wsSupport.findUser(dbSession, userLogin); - PermissionChange change = new UserPermissionChange( + UserPermissionChange change = new UserPermissionChange( PermissionChange.Operation.ADD, request.mandatoryParam(PARAM_PERMISSION), entity, user, permissionService); - permissionUpdater.apply(dbSession, singletonList(change)); + permissionUpdater.applyForUser(dbSession, singletonList(change)); } response.noContent(); } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/PermissionWsSupport.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/PermissionWsSupport.java index 5b34d7a2905..71b54559be1 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/PermissionWsSupport.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/PermissionWsSupport.java @@ -20,8 +20,8 @@ package org.sonar.server.permission.ws; import java.util.Optional; -import javax.annotation.CheckForNull; import java.util.Set; +import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.sonar.api.config.Configuration; import org.sonar.api.resources.Qualifiers; @@ -89,11 +89,17 @@ public class PermissionWsSupport { return null; } - public GroupUuidOrAnyone findGroup(DbSession dbSession, Request request) { + public GroupUuidOrAnyone findGroupUuidOrAnyone(DbSession dbSession, Request request) { String groupName = request.mandatoryParam(PARAM_GROUP_NAME); return groupWsSupport.findGroupOrAnyone(dbSession, groupName); } + @CheckForNull + public GroupDto findGroupDtoOrNullIfAnyone(DbSession dbSession, Request request) { + String groupName = request.mandatoryParam(PARAM_GROUP_NAME); + return groupWsSupport.findGroupDtoOrNullIfAnyone(dbSession, groupName); + } + public UserId findUser(DbSession dbSession, String login) { UserDto dto = ofNullable(dbClient.userDao().selectActiveUserByLogin(dbSession, login)) .orElseThrow(() -> new NotFoundException(format("User with login '%s' is not found'", login))); @@ -149,7 +155,7 @@ public class PermissionWsSupport { } public static boolean isUpdatingBrowsePermissionOnPrivateProject(String permission, @Nullable EntityDto entityDto) { - return entityDto != null && entityDto.isPrivate() && permission.equals(UserRole.USER) ; + return entityDto != null && entityDto.isPrivate() && permission.equals(UserRole.USER); } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/RemoveGroupAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/RemoveGroupAction.java index 0a5364dcef4..64994cfa858 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/RemoveGroupAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/RemoveGroupAction.java @@ -26,6 +26,7 @@ import org.sonar.api.server.ws.WebService; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.entity.EntityDto; +import org.sonar.db.user.GroupDto; import org.sonar.server.permission.GroupPermissionChange; import org.sonar.server.permission.GroupUuidOrAnyone; import org.sonar.server.permission.PermissionChange; @@ -50,7 +51,7 @@ public class RemoveGroupAction implements PermissionsWsAction { private final PermissionService permissionService; public RemoveGroupAction(DbClient dbClient, UserSession userSession, PermissionUpdater permissionUpdater, PermissionWsSupport wsSupport, - WsParameters wsParameters, PermissionService permissionService) { + WsParameters wsParameters, PermissionService permissionService) { this.dbClient = dbClient; this.userSession = userSession; this.permissionUpdater = permissionUpdater; @@ -85,21 +86,21 @@ public class RemoveGroupAction implements PermissionsWsAction { @Override public void handle(Request request, Response response) throws Exception { try (DbSession dbSession = dbClient.openSession(false)) { - GroupUuidOrAnyone group = wsSupport.findGroup(dbSession, request); EntityDto entity = wsSupport.findEntity(dbSession, request); + GroupDto groupDto = wsSupport.findGroupDtoOrNullIfAnyone(dbSession, request); wsSupport.checkPermissionManagementAccess(userSession, entity); String permission = request.mandatoryParam(PARAM_PERMISSION); - wsSupport.checkRemovingOwnBrowsePermissionOnPrivateProject(dbSession, userSession, entity, permission, group); + wsSupport.checkRemovingOwnBrowsePermissionOnPrivateProject(dbSession, userSession, entity, permission, GroupUuidOrAnyone.from(groupDto)); - PermissionChange change = new GroupPermissionChange( + GroupPermissionChange change = new GroupPermissionChange( PermissionChange.Operation.REMOVE, permission, entity, - group, + groupDto, permissionService); - permissionUpdater.apply(dbSession, singletonList(change)); + permissionUpdater.applyForGroups(dbSession, singletonList(change)); } response.noContent(); } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/RemoveUserAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/RemoveUserAction.java index f90f036efc0..59cc08ea547 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/RemoveUserAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/RemoveUserAction.java @@ -87,12 +87,12 @@ public class RemoveUserAction implements PermissionsWsAction { EntityDto entity = wsSupport.findEntity(dbSession, request); wsSupport.checkRemovingOwnBrowsePermissionOnPrivateProject(userSession, entity, permission, user); wsSupport.checkPermissionManagementAccess(userSession, entity); - PermissionChange change = new UserPermissionChange( + UserPermissionChange change = new UserPermissionChange( PermissionChange.Operation.REMOVE, permission, entity, user, permissionService); - permissionUpdater.apply(dbSession, singletonList(change)); + permissionUpdater.applyForUser(dbSession, singletonList(change)); response.noContent(); } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/template/AddGroupToTemplateAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/template/AddGroupToTemplateAction.java index 7e4034bbecf..2169267d3a7 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/template/AddGroupToTemplateAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/template/AddGroupToTemplateAction.java @@ -78,7 +78,7 @@ public class AddGroupToTemplateAction implements PermissionsWsAction { public void handle(Request request, Response response) { try (DbSession dbSession = dbClient.openSession(false)) { String permission = request.mandatoryParam(PARAM_PERMISSION); - GroupUuidOrAnyone group = support.findGroup(dbSession, request); + GroupUuidOrAnyone group = support.findGroupUuidOrAnyone(dbSession, request); checkRequest(!ADMINISTER.getKey().equals(permission) || !group.isAnyone(), format("It is not possible to add the '%s' permission to the group 'Anyone'.", permission)); diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/template/RemoveGroupFromTemplateAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/template/RemoveGroupFromTemplateAction.java index 396c198c3ff..410539c5a03 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/template/RemoveGroupFromTemplateAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/template/RemoveGroupFromTemplateAction.java @@ -76,7 +76,7 @@ public class RemoveGroupFromTemplateAction implements PermissionsWsAction { String permission = request.mandatoryParam(PARAM_PERMISSION); PermissionTemplateDto template = wsSupport.findTemplate(dbSession, WsTemplateRef.fromRequest(request)); checkGlobalAdmin(userSession); - GroupUuidOrAnyone group = wsSupport.findGroup(dbSession, request); + GroupUuidOrAnyone group = wsSupport.findGroupUuidOrAnyone(dbSession, request); dbClient.permissionTemplateDao().deleteGroupPermission(dbSession, template.getUuid(), group.getUuid(), permission, template.getName(), request.param(PARAM_GROUP_NAME)); diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/GroupWsSupport.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/GroupWsSupport.java index 15034dec15e..0278e20b7e6 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/GroupWsSupport.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/GroupWsSupport.java @@ -20,6 +20,7 @@ package org.sonar.server.usergroups.ws; import java.util.Optional; +import javax.annotation.CheckForNull; import org.sonar.api.security.DefaultGroups; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.WebService; @@ -41,7 +42,6 @@ import static org.sonar.server.exceptions.NotFoundException.checkFoundWithOption */ public class GroupWsSupport { - static final String PARAM_GROUP_ID = "id"; static final String PARAM_GROUP_NAME = "name"; static final String PARAM_GROUP_CURRENT_NAME = "currentName"; static final String PARAM_GROUP_DESCRIPTION = "description"; @@ -76,8 +76,15 @@ public class GroupWsSupport { return findGroupDto(dbSession, groupName); } - public GroupDto findGroupDto(DbSession dbSession, String groupName) { + @CheckForNull + public GroupDto findGroupDtoOrNullIfAnyone(DbSession dbSession, String groupName) { + if (DefaultGroups.isAnyone(groupName)) { + return null; + } + return findGroupDto(dbSession, groupName); + } + public GroupDto findGroupDto(DbSession dbSession, String groupName) { Optional group = dbClient.groupDao().selectByName(dbSession, groupName); checkFoundWithOptional(group, "No group with name '%s'", groupName); return group.get(); -- 2.39.5