From f718bf655922dae9692b0cd8eafd0e3c981faa64 Mon Sep 17 00:00:00 2001 From: Antoine Vigneau Date: Thu, 6 Jul 2023 17:35:22 +0200 Subject: [PATCH] SONAR-19787 Restrict permissions changes on managed instances --- .../management/DelegatingManagedServices.java | 15 +- .../management/ManagedInstanceService.java | 5 +- .../management/ManagedProjectService.java | 2 +- .../DelegatingManagedServicesTest.java | 40 ++++- .../permission/ws/AddGroupActionIT.java | 28 ++- .../server/permission/ws/AddUserActionIT.java | 26 ++- .../permission/ws/BasePermissionWsIT.java | 3 +- .../permission/ws/RemoveGroupActionIT.java | 27 ++- .../permission/ws/RemoveUserActionIT.java | 28 ++- .../ws/template/DeleteTemplateActionIT.java | 3 +- .../project/ws/UpdateVisibilityActionIT.java | 16 +- .../server/user/ws/DeactivateActionIT.java | 21 ++- .../management/ManagedInstanceChecker.java | 31 +++- .../server/permission/ws/AddGroupAction.java | 14 +- .../server/permission/ws/AddUserAction.java | 18 +- .../permission/ws/PermissionWsSupport.java | 5 +- .../permission/ws/RemoveGroupAction.java | 17 +- .../permission/ws/RemoveUserAction.java | 25 ++- .../project/ws/UpdateVisibilityAction.java | 18 +- .../server/user/ws/DeactivateAction.java | 19 +- .../ManagedInstanceCheckerTest.java | 170 +++++++++++++++++- 21 files changed, 437 insertions(+), 94 deletions(-) diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/management/DelegatingManagedServices.java b/server/sonar-server-common/src/main/java/org/sonar/server/management/DelegatingManagedServices.java index 6f243218308..7234e84a893 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/management/DelegatingManagedServices.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/management/DelegatingManagedServices.java @@ -83,9 +83,16 @@ public class DelegatingManagedServices implements ManagedInstanceService, Manage } @Override - public boolean isUserManaged(DbSession dbSession, String login) { + public boolean isUserManaged(DbSession dbSession, String userUuid) { return findManagedInstanceService() - .map(managedInstanceService -> managedInstanceService.isUserManaged(dbSession, login)) + .map(managedInstanceService -> managedInstanceService.isUserManaged(dbSession, userUuid)) + .orElse(false); + } + + @Override + public boolean isGroupManaged(DbSession dbSession, String groupUuid) { + return findManagedInstanceService() + .map(managedInstanceService -> managedInstanceService.isGroupManaged(dbSession, groupUuid)) .orElse(false); } @@ -104,9 +111,9 @@ public class DelegatingManagedServices implements ManagedInstanceService, Manage } @Override - public boolean isProjectManaged(DbSession dbSession, String projectKey) { + public boolean isProjectManaged(DbSession dbSession, String projectUuid) { return findManagedProjectService() - .map(managedProjectService -> managedProjectService.isProjectManaged(dbSession, projectKey)) + .map(managedProjectService -> managedProjectService.isProjectManaged(dbSession, projectUuid)) .orElse(false); } diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/management/ManagedInstanceService.java b/server/sonar-server-common/src/main/java/org/sonar/server/management/ManagedInstanceService.java index dcce08c7d0a..628bfcb1549 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/management/ManagedInstanceService.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/management/ManagedInstanceService.java @@ -39,5 +39,8 @@ public interface ManagedInstanceService { String getManagedGroupsSqlFilter(boolean filterByManaged); - boolean isUserManaged(DbSession dbSession, String login); + boolean isUserManaged(DbSession dbSession, String userUuid); + + boolean isGroupManaged(DbSession dbSession, String groupUuid); + } diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/management/ManagedProjectService.java b/server/sonar-server-common/src/main/java/org/sonar/server/management/ManagedProjectService.java index b3d765ba0aa..101e2454020 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/management/ManagedProjectService.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/management/ManagedProjectService.java @@ -23,6 +23,6 @@ import org.sonar.db.DbSession; public interface ManagedProjectService { - boolean isProjectManaged(DbSession dbSession, String projectKey); + boolean isProjectManaged(DbSession dbSession, String projectUuid); } diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/management/DelegatingManagedServicesTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/management/DelegatingManagedServicesTest.java index 042f46750b4..e7a7bbccfd8 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/management/DelegatingManagedServicesTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/management/DelegatingManagedServicesTest.java @@ -117,14 +117,28 @@ public class DelegatingManagedServicesTest { public void isUserManaged_delegatesToRightService_andPropagateAnswer() { DelegatingManagedServices managedInstanceService = new DelegatingManagedServices(Set.of(new NeverManagedInstanceService(), new AlwaysManagedInstanceService())); - assertThat(managedInstanceService.isUserManaged(dbSession, "login")).isTrue(); + assertThat(managedInstanceService.isUserManaged(dbSession, "whatever")).isTrue(); } @Test public void isUserManaged_whenNoDelegates_returnsFalse() { DelegatingManagedServices managedInstanceService = new DelegatingManagedServices(Set.of()); - assertThat(managedInstanceService.isUserManaged(dbSession, "login")).isFalse(); + assertThat(managedInstanceService.isUserManaged(dbSession, "whatever")).isFalse(); + } + + @Test + public void isGroupManaged_delegatesToRightService_andPropagateAnswer() { + DelegatingManagedServices managedInstanceService = new DelegatingManagedServices(Set.of(new NeverManagedInstanceService(), new AlwaysManagedInstanceService())); + + assertThat(managedInstanceService.isGroupManaged(dbSession, "whatever")).isTrue(); + } + + @Test + public void isGroupManaged_whenNoDelegates_returnsFalse() { + DelegatingManagedServices managedInstanceService = new DelegatingManagedServices(Set.of()); + + assertThat(managedInstanceService.isGroupManaged(dbSession, "whatever")).isFalse(); } @Test @@ -206,14 +220,14 @@ public class DelegatingManagedServicesTest { public void isProjectManaged_whenManagedInstanceServices_shouldDelegatesToRightService() { DelegatingManagedServices managedInstanceService = new DelegatingManagedServices(Set.of(new NeverManagedInstanceService(), new AlwaysManagedInstanceService())); - assertThat(managedInstanceService.isProjectManaged(dbSession, "project_key")).isTrue(); + assertThat(managedInstanceService.isProjectManaged(dbSession, "whatever")).isTrue(); } @Test public void isProjectManaged_whenManagedNoInstanceServices_returnsFalse() { DelegatingManagedServices managedInstanceService = NO_MANAGED_SERVICES; - assertThat(managedInstanceService.isProjectManaged(dbSession, "project_key")).isFalse(); + assertThat(managedInstanceService.isProjectManaged(dbSession, "whatever")).isFalse(); } private static class NeverManagedInstanceService implements ManagedInstanceService, ManagedProjectService { @@ -249,12 +263,17 @@ public class DelegatingManagedServicesTest { } @Override - public boolean isUserManaged(DbSession dbSession, String login) { + public boolean isUserManaged(DbSession dbSession, String userUuid) { + return false; + } + + @Override + public boolean isGroupManaged(DbSession dbSession, String groupUuid) { return false; } @Override - public boolean isProjectManaged(DbSession dbSession, String projectKey) { + public boolean isProjectManaged(DbSession dbSession, String projectUuid) { return false; } } @@ -292,12 +311,17 @@ public class DelegatingManagedServicesTest { } @Override - public boolean isUserManaged(DbSession dbSession, String login) { + public boolean isUserManaged(DbSession dbSession, String userUuid) { + return true; + } + + @Override + public boolean isGroupManaged(DbSession dbSession, String groupUuid) { return true; } @Override - public boolean isProjectManaged(DbSession dbSession, String projectKey) { + public boolean isProjectManaged(DbSession dbSession, String projectUuid) { return true; } } diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/AddGroupActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/AddGroupActionIT.java index f78ddee35d2..43de8249cda 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/AddGroupActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/AddGroupActionIT.java @@ -36,13 +36,19 @@ import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.exceptions.ServerException; +import org.sonar.server.management.ManagedInstanceChecker; import org.sonar.server.permission.PermissionService; import org.sonar.server.permission.PermissionServiceImpl; +import org.sonar.server.ws.TestRequest; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.fail; import static org.assertj.core.api.Assertions.tuple; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; import static org.sonar.db.component.ComponentTesting.newDirectory; import static org.sonar.db.component.ComponentTesting.newFileDto; import static org.sonar.db.component.ComponentTesting.newSubPortfolio; @@ -59,10 +65,11 @@ public class AddGroupActionIT extends BasePermissionWsIT { private final ResourceTypes resourceTypes = new ResourceTypesRule().setRootQualifiers(Qualifiers.PROJECT); private final PermissionService permissionService = new PermissionServiceImpl(resourceTypes); private final WsParameters wsParameters = new WsParameters(permissionService); + private final ManagedInstanceChecker managedInstanceChecker = mock(ManagedInstanceChecker.class); @Override protected AddGroupAction buildWsAction() { - return new AddGroupAction(db.getDbClient(), userSession, newPermissionUpdater(), newPermissionWsSupport(), wsParameters, permissionService); + return new AddGroupAction(db.getDbClient(), userSession, newPermissionUpdater(), newPermissionWsSupport(), wsParameters, permissionService, managedInstanceChecker); } @Test @@ -202,6 +209,25 @@ public class AddGroupActionIT extends BasePermissionWsIT { .hasMessage("Entity not found"); } + @Test + public void fail_when_project_is_managed_and_no_permissions_update() { + GroupDto group = db.users().insertGroup("whatever"); + ProjectDto project = db.components().insertPrivateProject().getProjectDto(); + + doThrow(new IllegalStateException("Managed project")).when(managedInstanceChecker).throwIfProjectIsManaged(any(), eq(project.getUuid())); + + TestRequest request = newRequest() + .setParam(PARAM_GROUP_NAME, group.getName()) + .setParam(PARAM_PROJECT_KEY, project.getKey()) + .setParam(PARAM_PERMISSION, UserRole.CODEVIEWER); + + assertThatThrownBy(request::execute) + .isInstanceOf(IllegalStateException.class) + .hasMessage("Managed project"); + + assertThat(db.users().selectGroupPermissions(group, project)).doesNotContain(UserRole.CODEVIEWER); + } + @Test public void adding_a_project_permission_fails_if_project_is_not_set() { GroupDto group = db.users().insertGroup("sonar-administrators"); diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/AddUserActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/AddUserActionIT.java index 89f28ed70a2..e61488348d5 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/AddUserActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/AddUserActionIT.java @@ -35,12 +35,16 @@ import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.exceptions.ServerException; +import org.sonar.server.management.ManagedInstanceChecker; import org.sonar.server.permission.PermissionService; import org.sonar.server.permission.PermissionServiceImpl; import org.sonar.server.ws.TestRequest; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.sonar.db.component.ComponentTesting.newDirectory; import static org.sonar.db.component.ComponentTesting.newFileDto; @@ -57,6 +61,7 @@ public class AddUserActionIT extends BasePermissionWsIT { private final PermissionService permissionService = new PermissionServiceImpl(resourceTypes); private final WsParameters wsParameters = new WsParameters(permissionService); private final Configuration configuration = mock(Configuration.class); + private final ManagedInstanceChecker managedInstanceChecker = mock(ManagedInstanceChecker.class); @Before public void setUp() { @@ -65,7 +70,8 @@ public class AddUserActionIT extends BasePermissionWsIT { @Override protected AddUserAction buildWsAction() { - return new AddUserAction(db.getDbClient(), userSession, newPermissionUpdater(), newPermissionWsSupport(), wsParameters, permissionService, configuration); + return new AddUserAction(db.getDbClient(), userSession, newPermissionUpdater(), newPermissionWsSupport(), + wsParameters, permissionService, configuration, managedInstanceChecker); } @Test @@ -206,6 +212,24 @@ public class AddUserActionIT extends BasePermissionWsIT { .isInstanceOf(NotFoundException.class); } + @Test + public void fail_when_project_is_managed_and_no_permissions_update() { + ProjectDto project = db.components().insertPrivateProject().getProjectDto(); + + doThrow(new IllegalStateException("Managed project")).when(managedInstanceChecker).throwIfProjectIsManaged(any(), eq(project.getUuid())); + + TestRequest request = newRequest() + .setParam(PARAM_USER_LOGIN, user.getLogin()) + .setParam(PARAM_PROJECT_KEY, project.getKey()) + .setParam(PARAM_PERMISSION, UserRole.CODEVIEWER); + + assertThatThrownBy(request::execute) + .isInstanceOf(IllegalStateException.class) + .hasMessage("Managed project"); + + assertThat(db.users().selectEntityPermissionOfUser(user, project.getUuid())).doesNotContain(UserRole.CODEVIEWER); + } + @Test public void fail_when_get_request() { loginAsAdmin(); 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 2d02b766cf2..91e1f855f3d 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 @@ -29,7 +29,6 @@ import org.sonar.db.DbClient; import org.sonar.db.DbTester; import org.sonar.db.component.ResourceTypesRule; import org.sonar.db.permission.template.PermissionTemplateDto; -import org.sonar.server.component.ComponentFinder; import org.sonar.server.es.EsTester; import org.sonar.server.es.IndexersImpl; import org.sonar.server.permission.GroupPermissionChanger; @@ -73,7 +72,7 @@ public abstract class BasePermissionWsIT { protected PermissionWsSupport newPermissionWsSupport() { DbClient dbClient = db.getDbClient(); - return new PermissionWsSupport(dbClient, configuration, new ComponentFinder(dbClient, newRootResourceTypes()), newGroupWsSupport()); + return new PermissionWsSupport(dbClient, configuration, newGroupWsSupport()); } protected ResourceTypesRule newRootResourceTypes() { diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/RemoveGroupActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/RemoveGroupActionIT.java index 49db2fdfd46..b5e85e7be69 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/RemoveGroupActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/RemoveGroupActionIT.java @@ -40,6 +40,7 @@ import org.sonar.db.user.UserDto; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.management.ManagedInstanceChecker; import org.sonar.server.permission.PermissionService; import org.sonar.server.permission.PermissionServiceImpl; import org.sonar.server.ws.TestRequest; @@ -47,6 +48,10 @@ import org.sonar.server.ws.TestRequest; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.tuple; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; import static org.sonar.db.component.ComponentTesting.newDirectory; import static org.sonar.db.component.ComponentTesting.newFileDto; import static org.sonar.db.component.ComponentTesting.newSubPortfolio; @@ -62,6 +67,7 @@ public class RemoveGroupActionIT extends BasePermissionWsIT { private final ResourceTypes resourceTypes = new ResourceTypesRule().setRootQualifiers(Qualifiers.PROJECT); private final PermissionService permissionService = new PermissionServiceImpl(resourceTypes); private final WsParameters wsParameters = new WsParameters(permissionService); + private final ManagedInstanceChecker managedInstanceChecker = mock(ManagedInstanceChecker.class); @Before public void setUp() { @@ -70,7 +76,7 @@ public class RemoveGroupActionIT extends BasePermissionWsIT { @Override protected RemoveGroupAction buildWsAction() { - return new RemoveGroupAction(db.getDbClient(), userSession, newPermissionUpdater(), newPermissionWsSupport(), wsParameters, permissionService); + return new RemoveGroupAction(db.getDbClient(), userSession, newPermissionUpdater(), newPermissionWsSupport(), wsParameters, permissionService, managedInstanceChecker); } @Test @@ -223,6 +229,25 @@ public class RemoveGroupActionIT extends BasePermissionWsIT { .hasMessage("Entity not found"); } + @Test + public void wsAction_whenGroupAndProjectAreManaged_shouldFailAndNotRemovePermissions() { + ProjectDto project = db.components().insertPrivateProject().getProjectDto(); + db.users().insertEntityPermissionOnGroup(aGroup, UserRole.CODEVIEWER, project); + + doThrow(new IllegalStateException("Managed project and group")).when(managedInstanceChecker).throwIfGroupAndProjectAreManaged(any(), eq(aGroup.getUuid()), eq(project.getUuid())); + + TestRequest request = newRequest() + .setParam(PARAM_GROUP_NAME, aGroup.getName()) + .setParam(PARAM_PROJECT_KEY, project.getKey()) + .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey()); + + assertThatThrownBy(request::execute) + .isInstanceOf(IllegalStateException.class) + .hasMessage("Managed project and group"); + + assertThat(db.users().selectGroupPermissions(aGroup, project)).containsOnly(UserRole.CODEVIEWER); + } + @Test public void wsAction_whenGroupNameIsMissing_shouldFail() { loginAsAdmin(); diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/RemoveUserActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/RemoveUserActionIT.java index e215eda1fe5..8af083dccb3 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/RemoveUserActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/RemoveUserActionIT.java @@ -33,6 +33,7 @@ import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.exceptions.ServerException; +import org.sonar.server.management.ManagedInstanceChecker; import org.sonar.server.permission.PermissionService; import org.sonar.server.permission.PermissionServiceImpl; import org.sonar.server.ws.TestRequest; @@ -40,6 +41,10 @@ import org.sonar.server.ws.TestRequest; import static java.util.Objects.requireNonNull; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; import static org.sonar.db.component.ComponentTesting.newDirectory; import static org.sonar.db.component.ComponentTesting.newFileDto; import static org.sonar.db.component.ComponentTesting.newSubPortfolio; @@ -56,6 +61,7 @@ public class RemoveUserActionIT extends BasePermissionWsIT { private final ResourceTypes resourceTypes = new ResourceTypesRule().setRootQualifiers(Qualifiers.PROJECT); private final PermissionService permissionService = new PermissionServiceImpl(resourceTypes); private final WsParameters wsParameters = new WsParameters(permissionService); + private final ManagedInstanceChecker managedInstanceChecker = mock(ManagedInstanceChecker.class); @Before public void setUp() { @@ -64,7 +70,7 @@ public class RemoveUserActionIT extends BasePermissionWsIT { @Override protected RemoveUserAction buildWsAction() { - return new RemoveUserAction(db.getDbClient(), userSession, newPermissionUpdater(), newPermissionWsSupport(), wsParameters, permissionService); + return new RemoveUserAction(db.getDbClient(), userSession, newPermissionUpdater(), newPermissionWsSupport(), wsParameters, permissionService, managedInstanceChecker); } @Test @@ -255,6 +261,26 @@ public class RemoveUserActionIT extends BasePermissionWsIT { .hasMessage("Entity not found"); } + @Test + public void wsAction_whenProjectAndUserAreManaged_shouldThrowAndNotRemovePermissions() { + ProjectDto project = db.components().insertPrivateProject().getProjectDto(); + db.users().insertProjectPermissionOnUser(user, UserRole.CODEVIEWER, project); + + doThrow(new IllegalStateException("Managed project")).when(managedInstanceChecker).throwIfUserAndProjectAreManaged(any(), eq(user.getUuid()), eq(project.getUuid())); + + loginAsAdmin(); + TestRequest request = newRequest() + .setParam(PARAM_USER_LOGIN, user.getLogin()) + .setParam(PARAM_PROJECT_ID, project.getUuid()) + .setParam(PARAM_PERMISSION, UserRole.CODEVIEWER); + + assertThatThrownBy(request::execute) + .isInstanceOf(IllegalStateException.class) + .hasMessage("Managed project"); + + assertThat(db.users().selectEntityPermissionOfUser(user, project.getUuid())).containsOnly(UserRole.CODEVIEWER); + } + @Test public void wsAction_whenGetRequest_shouldFail() { loginAsAdmin(); diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/template/DeleteTemplateActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/template/DeleteTemplateActionIT.java index 3fb170edc0a..ecf781380b5 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/template/DeleteTemplateActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/template/DeleteTemplateActionIT.java @@ -34,7 +34,6 @@ import org.sonar.db.user.GroupDto; import org.sonar.db.user.GroupTesting; import org.sonar.db.user.UserDto; import org.sonar.db.user.UserTesting; -import org.sonar.server.component.ComponentFinder; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; @@ -76,7 +75,7 @@ public class DeleteTemplateActionIT { public void setUp() { GroupWsSupport groupWsSupport = new GroupWsSupport(dbClient, new DefaultGroupFinder(db.getDbClient())); this.underTest = new WsActionTester(new DeleteTemplateAction(dbClient, userSession, - new PermissionWsSupport(dbClient, configuration, new ComponentFinder(dbClient, resourceTypes), groupWsSupport), defaultTemplatesResolver)); + new PermissionWsSupport(dbClient, configuration, groupWsSupport), defaultTemplatesResolver)); } @Test diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/project/ws/UpdateVisibilityActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/project/ws/UpdateVisibilityActionIT.java index c59d664d213..6f68d7cba98 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/project/ws/UpdateVisibilityActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/project/ws/UpdateVisibilityActionIT.java @@ -59,7 +59,7 @@ import org.sonar.server.es.TestIndexers; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.UnauthorizedException; -import org.sonar.server.management.DelegatingManagedServices; +import org.sonar.server.management.ManagedInstanceChecker; import org.sonar.server.permission.PermissionService; import org.sonar.server.permission.PermissionServiceImpl; import org.sonar.server.permission.index.FooIndexDefinition; @@ -77,6 +77,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.sonar.api.CoreProperties.CORE_ALLOW_PERMISSION_MANAGEMENT_FOR_PROJECT_ADMINS_PROPERTY; @@ -109,10 +110,10 @@ public class UpdateVisibilityActionIT { private final DbSession dbSession = dbTester.getSession(); private final TestIndexers projectIndexers = new TestIndexers(); private final Configuration configuration = mock(Configuration.class); - private final DelegatingManagedServices delegatingManagedServices = mock(DelegatingManagedServices.class); + private final ManagedInstanceChecker managedInstanceChecker = mock(ManagedInstanceChecker.class); private final VisibilityService visibilityService = new VisibilityService(dbClient, projectIndexers, new SequenceUuidFactory()); - private final UpdateVisibilityAction underTest = new UpdateVisibilityAction(dbClient, userSessionRule, configuration, visibilityService, delegatingManagedServices); + private final UpdateVisibilityAction underTest = new UpdateVisibilityAction(dbClient, userSessionRule, configuration, visibilityService, managedInstanceChecker); private final WsActionTester ws = new WsActionTester(underTest); private final Random random = new Random(); @@ -309,16 +310,17 @@ public class UpdateVisibilityActionIT { } @Test - public void execute_throws_BadRequestException_if_entity_is_project_and_managed() { - when(delegatingManagedServices.isProjectManaged(any(), eq(MANAGED_PROJECT_KEY))).thenReturn(true); + public void execute_throws_if_project_is_managed() { ProjectDto project = dbTester.components().insertPublicProject(p -> p.setKey(MANAGED_PROJECT_KEY)).getProjectDto(); request.setParam(PARAM_PROJECT, project.getKey()) .setParam(PARAM_VISIBILITY, Visibility.PUBLIC.getLabel()); userSessionRule.addProjectPermission(UserRole.ADMIN, project); + doThrow(new IllegalStateException("Managed project")).when(managedInstanceChecker).throwIfProjectIsManaged(any(), eq(project.getUuid())); + assertThatThrownBy(request::execute) - .isInstanceOf(BadRequestException.class) - .hasMessage("Cannot change visibility of a managed project"); + .isInstanceOf(IllegalStateException.class) + .hasMessage("Managed project"); } @Test diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/DeactivateActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/DeactivateActionIT.java index 665db995126..2afc43c82d4 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/DeactivateActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/DeactivateActionIT.java @@ -48,7 +48,7 @@ import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.exceptions.UnauthorizedException; -import org.sonar.server.management.ManagedInstanceService; +import org.sonar.server.management.ManagedInstanceChecker; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.user.ExternalIdentity; import org.sonar.server.ws.TestRequest; @@ -57,12 +57,11 @@ import org.sonar.server.ws.WsActionTester; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; import static org.sonar.db.property.PropertyTesting.newUserPropertyDto; import static org.sonar.test.JsonAssert.assertJson; @@ -79,10 +78,9 @@ public class DeactivateActionIT { private final DbSession dbSession = db.getSession(); private final UserAnonymizer userAnonymizer = new UserAnonymizer(db.getDbClient(), () -> "anonymized"); private final UserDeactivator userDeactivator = new UserDeactivator(dbClient, userAnonymizer); - private final ManagedInstanceService managedInstanceService = mock(ManagedInstanceService.class); + private final ManagedInstanceChecker managedInstanceChecker = mock(ManagedInstanceChecker.class); - private final WsActionTester ws = new WsActionTester(new DeactivateAction(dbClient, userSession, new UserJsonWriter(userSession), userDeactivator, managedInstanceService - )); + private final WsActionTester ws = new WsActionTester(new DeactivateAction(dbClient, userSession, new UserJsonWriter(userSession), userDeactivator, managedInstanceChecker)); @Test public void deactivate_user_and_delete_their_related_data() { @@ -434,15 +432,16 @@ public class DeactivateActionIT { } @Test - public void handle_whenUserManagedAndInstanceManaged_shouldThrowBadRequestException() { + public void handle_whenUserManagedAndInstanceManaged_shouldThrow() { createAdminUser(); logInAsSystemAdministrator(); UserDto user = db.users().insertUser(); - when(managedInstanceService.isUserManaged(any(), eq(user.getLogin()))).thenReturn(true); + doThrow(new IllegalStateException("User managed")).when(managedInstanceChecker).throwIfUserIsManaged(any(), eq(user.getUuid())); - assertThatExceptionOfType(BadRequestException.class) - .isThrownBy(() -> deactivate(user.getLogin())) - .withMessage("Operation not allowed when the instance is externally managed."); + String login = user.getLogin(); + assertThatThrownBy(() -> deactivate(login)) + .isInstanceOf(IllegalStateException.class) + .hasMessage("User managed"); } @Test diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/management/ManagedInstanceChecker.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/management/ManagedInstanceChecker.java index ce27b4b2b3a..b5183f5a000 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/management/ManagedInstanceChecker.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/management/ManagedInstanceChecker.java @@ -19,17 +19,44 @@ */ package org.sonar.server.management; +import org.sonar.db.DbSession; import org.sonar.server.exceptions.BadRequestException; public class ManagedInstanceChecker { + private static final String INSTANCE_EXCEPTION_MESSAGE = "Operation not allowed when the instance is externally managed."; + private static final String PROJECT_EXCEPTION_MESSAGE = "Operation not allowed when the project is externally managed."; + private final ManagedInstanceService managedInstanceService; + private final ManagedProjectService managedProjectService; - public ManagedInstanceChecker(ManagedInstanceService managedInstanceService) { + public ManagedInstanceChecker(ManagedInstanceService managedInstanceService, ManagedProjectService managedProjectService) { this.managedInstanceService = managedInstanceService; + this.managedProjectService = managedProjectService; } public void throwIfInstanceIsManaged() { - BadRequestException.checkRequest(!managedInstanceService.isInstanceExternallyManaged(), "Operation not allowed when the instance is externally managed."); + BadRequestException.checkRequest(!managedInstanceService.isInstanceExternallyManaged(), INSTANCE_EXCEPTION_MESSAGE); + } + + public void throwIfProjectIsManaged(DbSession dbSession, String projectUuid) { + BadRequestException.checkRequest(!managedProjectService.isProjectManaged(dbSession, projectUuid), PROJECT_EXCEPTION_MESSAGE); + } + + public void throwIfUserIsManaged(DbSession dbSession, String userUuid) { + BadRequestException.checkRequest(!managedInstanceService.isUserManaged(dbSession, userUuid), INSTANCE_EXCEPTION_MESSAGE); } + + public void throwIfUserAndProjectAreManaged(DbSession dbSession, String userUuid, String projectUuid) { + boolean isUserManaged = managedInstanceService.isUserManaged(dbSession, userUuid); + boolean isProjectManaged = managedProjectService.isProjectManaged(dbSession, projectUuid); + BadRequestException.checkRequest(!(isUserManaged && isProjectManaged), PROJECT_EXCEPTION_MESSAGE); + } + + public void throwIfGroupAndProjectAreManaged(DbSession dbSession, String groupUuid, String projectUuid) { + boolean isGroupManaged = managedInstanceService.isGroupManaged(dbSession, groupUuid); + boolean isProjectManaged = managedProjectService.isProjectManaged(dbSession, projectUuid); + BadRequestException.checkRequest(!(isGroupManaged && isProjectManaged), PROJECT_EXCEPTION_MESSAGE); + } + } 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 7b8be1df85f..bb66f1c1af0 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 @@ -28,6 +28,7 @@ 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.management.ManagedInstanceChecker; import org.sonar.server.permission.GroupPermissionChange; import org.sonar.server.permission.PermissionChange; import org.sonar.server.permission.PermissionService; @@ -47,15 +48,17 @@ public class AddGroupAction implements PermissionsWsAction { private final PermissionWsSupport wsSupport; private final WsParameters wsParameters; private final PermissionService permissionService; + private final ManagedInstanceChecker managedInstanceChecker; public AddGroupAction(DbClient dbClient, UserSession userSession, PermissionUpdater permissionUpdater, PermissionWsSupport wsSupport, - WsParameters wsParameters, PermissionService permissionService) { + WsParameters wsParameters, PermissionService permissionService, ManagedInstanceChecker managedInstanceChecker) { this.dbClient = dbClient; this.userSession = userSession; this.permissionUpdater = permissionUpdater; this.wsSupport = wsSupport; this.wsParameters = wsParameters; this.permissionService = permissionService; + this.managedInstanceChecker = managedInstanceChecker; } @Override @@ -85,12 +88,15 @@ public class AddGroupAction implements PermissionsWsAction { public void handle(Request request, Response response) throws Exception { try (DbSession dbSession = dbClient.openSession(false)) { GroupDto groupDto = wsSupport.findGroupDtoOrNullIfAnyone(dbSession, request); - EntityDto project = wsSupport.findEntity(dbSession, request); - wsSupport.checkPermissionManagementAccess(userSession, project); + EntityDto entityDto = wsSupport.findEntity(dbSession, request); + if (entityDto != null && entityDto.isProject()) { + managedInstanceChecker.throwIfProjectIsManaged(dbSession, entityDto.getUuid()); + } + wsSupport.checkPermissionManagementAccess(userSession, entityDto); GroupPermissionChange change = new GroupPermissionChange( PermissionChange.Operation.ADD, request.mandatoryParam(PARAM_PERMISSION), - project, + entityDto, groupDto, permissionService); permissionUpdater.applyForGroups(dbSession, List.of(change)); 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 ea55cb5c31c..df0a68a3866 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 @@ -27,6 +27,7 @@ import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.entity.EntityDto; import org.sonar.db.user.UserId; +import org.sonar.server.management.ManagedInstanceChecker; import org.sonar.server.permission.PermissionChange; import org.sonar.server.permission.PermissionService; import org.sonar.server.permission.PermissionUpdater; @@ -51,9 +52,10 @@ public class AddUserAction implements PermissionsWsAction { private final WsParameters wsParameters; private final PermissionService permissionService; private final Configuration configuration; + private final ManagedInstanceChecker managedInstanceChecker; public AddUserAction(DbClient dbClient, UserSession userSession, PermissionUpdater permissionUpdater, PermissionWsSupport wsSupport, - WsParameters wsParameters, PermissionService permissionService, Configuration configuration) { + WsParameters wsParameters, PermissionService permissionService, Configuration configuration, ManagedInstanceChecker managedInstanceChecker) { this.dbClient = dbClient; this.userSession = userSession; this.permissionUpdater = permissionUpdater; @@ -61,6 +63,7 @@ public class AddUserAction implements PermissionsWsAction { this.wsParameters = wsParameters; this.permissionService = permissionService; this.configuration = configuration; + this.managedInstanceChecker = managedInstanceChecker; } @Override @@ -86,17 +89,22 @@ public class AddUserAction implements PermissionsWsAction { public void handle(Request request, Response response) throws Exception { try (DbSession dbSession = dbClient.openSession(false)) { String userLogin = request.mandatoryParam(PARAM_USER_LOGIN); - EntityDto entity = wsSupport.findEntity(dbSession, request); - checkProjectAdmin(userSession, configuration, entity); + EntityDto entityDto = wsSupport.findEntity(dbSession, request); + if (entityDto != null && entityDto.isProject()) { + managedInstanceChecker.throwIfProjectIsManaged(dbSession, entityDto.getUuid()); + } + checkProjectAdmin(userSession, configuration, entityDto); UserId user = wsSupport.findUser(dbSession, userLogin); UserPermissionChange change = new UserPermissionChange( PermissionChange.Operation.ADD, request.mandatoryParam(PARAM_PERMISSION), - entity, - user, permissionService); + entityDto, + user, + permissionService); 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 71b54559be1..540dad487a5 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 @@ -35,7 +35,6 @@ import org.sonar.db.user.GroupDto; import org.sonar.db.user.UserDto; import org.sonar.db.user.UserId; import org.sonar.db.user.UserIdDto; -import org.sonar.server.component.ComponentFinder; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.permission.GroupUuidOrAnyone; @@ -57,15 +56,13 @@ public class PermissionWsSupport { private static final String ERROR_REMOVING_OWN_BROWSE_PERMISSION = "Permission 'Browse' cannot be removed from a private project for a project administrator."; private final DbClient dbClient; - private final ComponentFinder componentFinder; private final GroupWsSupport groupWsSupport; private final Configuration configuration; - public PermissionWsSupport(DbClient dbClient, Configuration configuration, ComponentFinder componentFinder, GroupWsSupport groupWsSupport) { + public PermissionWsSupport(DbClient dbClient, Configuration configuration, GroupWsSupport groupWsSupport) { this.dbClient = dbClient; this.configuration = configuration; - this.componentFinder = componentFinder; this.groupWsSupport = groupWsSupport; } 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 64994cfa858..dd876848574 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 @@ -27,6 +27,7 @@ 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.management.ManagedInstanceChecker; import org.sonar.server.permission.GroupPermissionChange; import org.sonar.server.permission.GroupUuidOrAnyone; import org.sonar.server.permission.PermissionChange; @@ -49,15 +50,17 @@ public class RemoveGroupAction implements PermissionsWsAction { private final PermissionWsSupport wsSupport; private final WsParameters wsParameters; private final PermissionService permissionService; + private final ManagedInstanceChecker managedInstanceChecker; public RemoveGroupAction(DbClient dbClient, UserSession userSession, PermissionUpdater permissionUpdater, PermissionWsSupport wsSupport, - WsParameters wsParameters, PermissionService permissionService) { + WsParameters wsParameters, PermissionService permissionService, ManagedInstanceChecker managedInstanceChecker) { this.dbClient = dbClient; this.userSession = userSession; this.permissionUpdater = permissionUpdater; this.wsSupport = wsSupport; this.wsParameters = wsParameters; this.permissionService = permissionService; + this.managedInstanceChecker = managedInstanceChecker; } @Override @@ -86,18 +89,20 @@ public class RemoveGroupAction implements PermissionsWsAction { @Override public void handle(Request request, Response response) throws Exception { try (DbSession dbSession = dbClient.openSession(false)) { - EntityDto entity = wsSupport.findEntity(dbSession, request); + EntityDto entityDto = wsSupport.findEntity(dbSession, request); GroupDto groupDto = wsSupport.findGroupDtoOrNullIfAnyone(dbSession, request); - - wsSupport.checkPermissionManagementAccess(userSession, entity); + if (entityDto != null && entityDto.isProject() && groupDto != null) { + managedInstanceChecker.throwIfGroupAndProjectAreManaged(dbSession, groupDto.getUuid(), entityDto.getUuid()); + } + wsSupport.checkPermissionManagementAccess(userSession, entityDto); String permission = request.mandatoryParam(PARAM_PERMISSION); - wsSupport.checkRemovingOwnBrowsePermissionOnPrivateProject(dbSession, userSession, entity, permission, GroupUuidOrAnyone.from(groupDto)); + wsSupport.checkRemovingOwnBrowsePermissionOnPrivateProject(dbSession, userSession, entityDto, permission, GroupUuidOrAnyone.from(groupDto)); GroupPermissionChange change = new GroupPermissionChange( PermissionChange.Operation.REMOVE, permission, - entity, + entityDto, groupDto, permissionService); permissionUpdater.applyForGroups(dbSession, singletonList(change)); 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 59cc08ea547..76a26d23e41 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 @@ -26,6 +26,7 @@ import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.entity.EntityDto; import org.sonar.db.user.UserId; +import org.sonar.server.management.ManagedInstanceChecker; import org.sonar.server.permission.PermissionChange; import org.sonar.server.permission.PermissionService; import org.sonar.server.permission.PermissionUpdater; @@ -48,15 +49,17 @@ public class RemoveUserAction implements PermissionsWsAction { private final PermissionWsSupport wsSupport; private final WsParameters wsParameters; private final PermissionService permissionService; + private final ManagedInstanceChecker managedInstanceChecker; public RemoveUserAction(DbClient dbClient, UserSession userSession, PermissionUpdater permissionUpdater, PermissionWsSupport wsSupport, - WsParameters wsParameters, PermissionService permissionService) { + WsParameters wsParameters, PermissionService permissionService, ManagedInstanceChecker managedInstanceChecker) { this.dbClient = dbClient; this.userSession = userSession; this.permissionUpdater = permissionUpdater; this.wsSupport = wsSupport; this.wsParameters = wsParameters; this.permissionService = permissionService; + this.managedInstanceChecker = managedInstanceChecker; } @Override @@ -81,19 +84,25 @@ public class RemoveUserAction implements PermissionsWsAction { @Override public void handle(Request request, Response response) throws Exception { try (DbSession dbSession = dbClient.openSession(false)) { - UserId user = wsSupport.findUser(dbSession, request.mandatoryParam(PARAM_USER_LOGIN)); + UserId userIdDto = wsSupport.findUser(dbSession, request.mandatoryParam(PARAM_USER_LOGIN)); String permission = request.mandatoryParam(PARAM_PERMISSION); - wsSupport.checkRemovingOwnAdminRight(userSession, user, permission); - EntityDto entity = wsSupport.findEntity(dbSession, request); - wsSupport.checkRemovingOwnBrowsePermissionOnPrivateProject(userSession, entity, permission, user); - wsSupport.checkPermissionManagementAccess(userSession, entity); + wsSupport.checkRemovingOwnAdminRight(userSession, userIdDto, permission); + + EntityDto entityDto = wsSupport.findEntity(dbSession, request); + if (entityDto != null) { + managedInstanceChecker.throwIfUserAndProjectAreManaged(dbSession, userIdDto.getUuid(), entityDto.getUuid()); + } + wsSupport.checkRemovingOwnBrowsePermissionOnPrivateProject(userSession, entityDto, permission, userIdDto); + wsSupport.checkPermissionManagementAccess(userSession, entityDto); UserPermissionChange change = new UserPermissionChange( PermissionChange.Operation.REMOVE, permission, - entity, - user, permissionService); + entityDto, + userIdDto, + permissionService); permissionUpdater.applyForUser(dbSession, singletonList(change)); response.noContent(); } } + } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/project/ws/UpdateVisibilityAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/project/ws/UpdateVisibilityAction.java index 2dd8219571d..5148c6b6ea5 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/project/ws/UpdateVisibilityAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/project/ws/UpdateVisibilityAction.java @@ -27,7 +27,7 @@ import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.entity.EntityDto; import org.sonar.server.exceptions.BadRequestException; -import org.sonar.server.management.DelegatingManagedServices; +import org.sonar.server.management.ManagedInstanceChecker; import org.sonar.server.project.Visibility; import org.sonar.server.project.VisibilityService; import org.sonar.server.user.UserSession; @@ -36,7 +36,6 @@ import org.sonarqube.ws.client.project.ProjectsWsParameters; import static org.sonar.api.CoreProperties.CORE_ALLOW_PERMISSION_MANAGEMENT_FOR_PROJECT_ADMINS_DEFAULT_VALUE; import static org.sonar.api.CoreProperties.CORE_ALLOW_PERMISSION_MANAGEMENT_FOR_PROJECT_ADMINS_PROPERTY; import static org.sonar.api.web.UserRole.ADMIN; -import static org.sonar.server.exceptions.BadRequestException.checkRequest; import static org.sonar.server.user.AbstractUserSession.insufficientPrivilegesException; import static org.sonar.server.ws.KeyExamples.KEY_PROJECT_EXAMPLE_001; import static org.sonarqube.ws.client.project.ProjectsWsParameters.PARAM_PROJECT; @@ -48,15 +47,15 @@ public class UpdateVisibilityAction implements ProjectsWsAction { private final UserSession userSession; private final Configuration configuration; private final VisibilityService visibilityService; - private final DelegatingManagedServices delegatingInstanceService; + private final ManagedInstanceChecker managedInstanceChecker; public UpdateVisibilityAction(DbClient dbClient, UserSession userSession, Configuration configuration, - VisibilityService visibilityService, DelegatingManagedServices delegatingInstanceService) { + VisibilityService visibilityService, ManagedInstanceChecker managedInstanceChecker) { this.dbClient = dbClient; this.userSession = userSession; this.configuration = configuration; this.visibilityService = visibilityService; - this.delegatingInstanceService = delegatingInstanceService; + this.managedInstanceChecker = managedInstanceChecker; } public void define(WebService.NewController context) { @@ -103,12 +102,9 @@ public class UpdateVisibilityAction implements ProjectsWsAction { if (!isProjectAdmin || (!isGlobalAdmin && !allowChangingPermissionsByProjectAdmins)) { throw insufficientPrivilegesException(); } - checkRequest(!isManagedProject(dbSession, entityDto), "Cannot change visibility of a managed project"); - } - - - private boolean isManagedProject(DbSession dbSession, EntityDto entityDto) { - return entityDto.isProject() && delegatingInstanceService.isProjectManaged(dbSession, entityDto.getKey()); + if (entityDto.isProject()) { + managedInstanceChecker.throwIfProjectIsManaged(dbSession, entityDto.getUuid()); + } } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/DeactivateAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/DeactivateAction.java index 7c31f901187..3dfa62dab2b 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/DeactivateAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/DeactivateAction.java @@ -29,7 +29,7 @@ import org.sonar.api.utils.text.JsonWriter; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.user.UserDto; -import org.sonar.server.management.ManagedInstanceService; +import org.sonar.server.management.ManagedInstanceChecker; import org.sonar.server.user.UserSession; import static java.util.Collections.singletonList; @@ -45,15 +45,15 @@ public class DeactivateAction implements UsersWsAction { private final UserSession userSession; private final UserJsonWriter userWriter; private final UserDeactivator userDeactivator; - private final ManagedInstanceService managedInstanceService; + private final ManagedInstanceChecker managedInstanceChecker; public DeactivateAction(DbClient dbClient, UserSession userSession, UserJsonWriter userWriter, - UserDeactivator userDeactivator, ManagedInstanceService managedInstanceService) { + UserDeactivator userDeactivator, ManagedInstanceChecker managedInstanceChecker) { this.dbClient = dbClient; this.userSession = userSession; this.userWriter = userWriter; this.userDeactivator = userDeactivator; - this.managedInstanceService = managedInstanceService; + this.managedInstanceChecker = managedInstanceChecker; } @Override @@ -84,19 +84,18 @@ public class DeactivateAction implements UsersWsAction { String login = request.mandatoryParam(PARAM_LOGIN); checkRequest(!login.equals(userSession.getLogin()), "Self-deactivation is not possible"); try (DbSession dbSession = dbClient.openSession(false)) { - preventManagedUserDeactivationIfManagedInstance(dbSession, login); + UserDto userDto = dbClient.userDao().selectByLogin(dbSession, login); + if (userDto != null) { + managedInstanceChecker.throwIfUserIsManaged(dbSession, userDto.getUuid()); + } boolean shouldAnonymize = request.mandatoryParamAsBoolean(PARAM_ANONYMIZE); - UserDto userDto = shouldAnonymize + userDto = shouldAnonymize ? userDeactivator.deactivateUserWithAnonymization(dbSession, login) : userDeactivator.deactivateUser(dbSession, login); writeResponse(response, userDto.getLogin()); } } - private void preventManagedUserDeactivationIfManagedInstance(DbSession dbSession, String login) { - checkRequest(!managedInstanceService.isUserManaged(dbSession, login), "Operation not allowed when the instance is externally managed."); - } - private void writeResponse(Response response, String login) { try (DbSession dbSession = dbClient.openSession(false)) { UserDto user = dbClient.userDao().selectByLogin(dbSession, login); diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/management/ManagedInstanceCheckerTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/management/ManagedInstanceCheckerTest.java index 827bab8002f..16959847dcb 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/management/ManagedInstanceCheckerTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/management/ManagedInstanceCheckerTest.java @@ -24,32 +24,194 @@ import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import org.sonar.db.DbSession; +import org.sonar.db.project.ProjectDto; +import org.sonar.db.user.GroupDto; +import org.sonar.db.user.UserDto; import org.sonar.server.exceptions.BadRequestException; import static org.assertj.core.api.Assertions.assertThatNoException; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) public class ManagedInstanceCheckerTest { + private static final String INSTANCE_EXCEPTION_MESSAGE = "Operation not allowed when the instance is externally managed."; + private static final String PROJECT_EXCEPTION_MESSAGE = "Operation not allowed when the project is externally managed."; + + @Mock + private DbSession dbSession; @Mock private ManagedInstanceService managedInstanceService; - + @Mock + private ManagedProjectService managedProjectService; @InjectMocks private ManagedInstanceChecker managedInstanceChecker; @Test - public void throwIfInstanceIsManaged_whenInstanceExternallyManaged_throws() { + public void throwIfInstanceIsManaged_whenInstanceExternallyManaged_shouldThrow() { when(managedInstanceService.isInstanceExternallyManaged()).thenReturn(true); + assertThatThrownBy(() -> managedInstanceChecker.throwIfInstanceIsManaged()) .isInstanceOf(BadRequestException.class) - .hasMessage("Operation not allowed when the instance is externally managed."); + .hasMessage(INSTANCE_EXCEPTION_MESSAGE); } @Test - public void throwIfInstanceIsManaged_whenInstanceNotExternallyManaged_doesntThrow() { + public void throwIfInstanceIsManaged_whenInstanceNotExternallyManaged_shouldNotThrow() { when(managedInstanceService.isInstanceExternallyManaged()).thenReturn(false); + assertThatNoException().isThrownBy(() -> managedInstanceChecker.throwIfInstanceIsManaged()); } + + @Test + public void throwIfProjectIsManaged_whenProjectIsManaged_shouldThrow() { + ProjectDto projectDto = mockManagedProject(); + + String projectUuid = projectDto.getUuid(); + assertThatThrownBy(() -> managedInstanceChecker.throwIfProjectIsManaged(dbSession, projectUuid)) + .isInstanceOf(BadRequestException.class) + .hasMessage(PROJECT_EXCEPTION_MESSAGE); + } + + @Test + public void throwIfProjectIsManaged_whenProjectIsNotManaged_shouldNotThrow() { + ProjectDto projectDto = mockNotManagedProject(); + + assertThatNoException().isThrownBy(() -> managedInstanceChecker.throwIfProjectIsManaged(dbSession, projectDto.getUuid())); + } + + @Test + public void throwIfUserIsManaged_whenUserIsManaged_shouldThrow() { + UserDto userDto = mockManagedUser(); + + String userUuid = userDto.getUuid(); + assertThatThrownBy(() -> managedInstanceChecker.throwIfUserIsManaged(dbSession, userUuid)) + .isInstanceOf(BadRequestException.class) + .hasMessage(INSTANCE_EXCEPTION_MESSAGE); + } + + @Test + public void throwIfUserIsManaged_whenUserIsNotManaged_shouldNotThrow() { + UserDto userDto = mockNotManagedUser(); + + assertThatNoException().isThrownBy(() -> managedInstanceChecker.throwIfUserIsManaged(dbSession, userDto.getUuid())); + } + + @Test + public void throwIfUserAndProjectAreManaged_whenUserAndProjectAreManaged_shouldThrow() { + ProjectDto projectDto = mockManagedProject(); + UserDto userDto = mockManagedUser(); + + String userUuid = userDto.getUuid(); + String projectUuid = projectDto.getUuid(); + assertThatThrownBy(() -> managedInstanceChecker.throwIfUserAndProjectAreManaged(dbSession, userUuid, projectUuid)) + .isInstanceOf(BadRequestException.class) + .hasMessage(PROJECT_EXCEPTION_MESSAGE); + } + + @Test + public void throwIfUserAndProjectAreManaged_whenOnlyUserIsManaged_shouldNotThrow() { + ProjectDto projectDto = mockNotManagedProject(); + UserDto userDto = mockManagedUser(); + + assertThatNoException().isThrownBy(() -> managedInstanceChecker.throwIfUserAndProjectAreManaged(dbSession, userDto.getUuid(), projectDto.getUuid())); + } + + @Test + public void throwIfUserAndProjectAreManaged_whenOnlyProjectIsManaged_shouldNotThrow() { + ProjectDto projectDto = mockManagedProject(); + UserDto userDto = mockNotManagedUser(); + + assertThatNoException().isThrownBy(() -> managedInstanceChecker.throwIfUserAndProjectAreManaged(dbSession, userDto.getUuid(), projectDto.getUuid())); + } + + @Test + public void throwIfUserAndProjectAreManaged_whenNothingIsManaged_shouldNotThrow() { + ProjectDto projectDto = mockNotManagedProject(); + UserDto userDto = mockNotManagedUser(); + + assertThatNoException().isThrownBy(() -> managedInstanceChecker.throwIfUserAndProjectAreManaged(dbSession, userDto.getUuid(), projectDto.getUuid())); + } + + @Test + public void throwIfGroupAndProjectAreManaged_whenGroupAndProjectAreManaged_shouldThrow() { + ProjectDto projectDto = mockManagedProject(); + GroupDto groupDto = mockManagedGroup(); + + String groupDtoUuid = groupDto.getUuid(); + String projectDtoUuid = projectDto.getUuid(); + assertThatThrownBy(() -> managedInstanceChecker.throwIfGroupAndProjectAreManaged(dbSession, groupDtoUuid, projectDtoUuid)) + .isInstanceOf(BadRequestException.class) + .hasMessage(PROJECT_EXCEPTION_MESSAGE); + } + + @Test + public void throwIfGroupAndProjectAreManaged_whenOnlyGroupIsManaged_shouldNotThrow() { + ProjectDto projectDto = mockNotManagedProject(); + GroupDto groupDto = mockManagedGroup(); + + assertThatNoException().isThrownBy(() -> managedInstanceChecker.throwIfGroupAndProjectAreManaged(dbSession, groupDto.getUuid(), projectDto.getUuid())); + } + + @Test + public void throwIfGroupAndProjectAreManaged_whenOnlyProjectIsManaged_shouldNotThrow() { + ProjectDto projectDto = mockManagedProject(); + GroupDto groupDto = mockNotManagedGroup(); + + assertThatNoException().isThrownBy(() -> managedInstanceChecker.throwIfGroupAndProjectAreManaged(dbSession, groupDto.getUuid(), projectDto.getUuid())); + } + + @Test + public void throwIfGroupAndProjectAreManaged_whenNothingIsManaged_shouldNotThrow() { + ProjectDto projectDto = mockNotManagedProject(); + GroupDto groupDto = mockNotManagedGroup(); + + assertThatNoException().isThrownBy(() -> managedInstanceChecker.throwIfGroupAndProjectAreManaged(dbSession, groupDto.getUuid(), projectDto.getUuid())); + } + + private ProjectDto mockManagedProject() { + return mockProject(true); + } + + private ProjectDto mockNotManagedProject() { + return mockProject(false); + } + + private ProjectDto mockProject(boolean isManaged) { + ProjectDto projectDto = mock(ProjectDto.class); + when(managedProjectService.isProjectManaged(dbSession, projectDto.getUuid())).thenReturn(isManaged); + return projectDto; + } + + private UserDto mockManagedUser() { + return mockUser(true); + } + + private UserDto mockNotManagedUser() { + return mockUser(false); + } + + private UserDto mockUser(boolean isManaged) { + UserDto userDto = mock(UserDto.class); + when(managedInstanceService.isUserManaged(dbSession, userDto.getUuid())).thenReturn(isManaged); + return userDto; + } + + private GroupDto mockManagedGroup() { + return mockGroup(true); + } + + private GroupDto mockNotManagedGroup() { + return mockGroup(false); + } + + private GroupDto mockGroup(boolean isManaged) { + GroupDto groupDto = mock(GroupDto.class); + when(managedInstanceService.isGroupManaged(dbSession, groupDto.getUuid())).thenReturn(isManaged); + return groupDto; + } + } -- 2.39.5