From e1bb13c356ba329aaa5f144a539867e0150f9828 Mon Sep 17 00:00:00 2001 From: Wojtek Wajerowicz <115081248+wojciech-wajerowicz-sonarsource@users.noreply.github.com> Date: Wed, 8 Mar 2023 16:13:35 +0100 Subject: [PATCH] SONAR-18654 prevent editing groups when instance is managed. --- .../server/usergroups/ws/AddUserActionIT.java | 19 ++++++- .../server/usergroups/ws/CreateActionIT.java | 17 +++++- .../server/usergroups/ws/DeleteActionIT.java | 53 +++++++++++++++++- .../usergroups/ws/RemoveUserActionIT.java | 16 +++++- .../server/usergroups/ws/UpdateActionIT.java | 18 +++++- .../management/ManagedInstanceChecker.java | 35 ++++++++++++ .../server/usergroups/ws/AddUserAction.java | 8 ++- .../server/usergroups/ws/CreateAction.java | 7 ++- .../server/usergroups/ws/DeleteAction.java | 16 +++++- .../usergroups/ws/ManagedInstanceChecker.java | 17 ------ .../usergroups/ws/RemoveUserAction.java | 9 ++- .../server/usergroups/ws/UpdateAction.java | 9 ++- .../usergroups/ws/UserGroupsModule.java | 2 + .../ManagedInstanceCheckerTest.java | 55 +++++++++++++++++++ 14 files changed, 249 insertions(+), 32 deletions(-) create mode 100644 server/sonar-webserver-webapi/src/main/java/org/sonar/server/management/ManagedInstanceChecker.java delete mode 100644 server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/ManagedInstanceChecker.java create mode 100644 server/sonar-webserver-webapi/src/test/java/org/sonar/server/management/ManagedInstanceCheckerTest.java diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/AddUserActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/AddUserActionIT.java index 63b9bc81f47..eae0bc78ecb 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/AddUserActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/AddUserActionIT.java @@ -27,8 +27,10 @@ import org.sonar.api.server.ws.WebService.Action; import org.sonar.db.DbTester; import org.sonar.db.user.GroupDto; import org.sonar.db.user.UserDto; +import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.exceptions.UnauthorizedException; +import org.sonar.server.management.ManagedInstanceChecker; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.usergroups.DefaultGroupFinder; import org.sonar.server.ws.TestRequest; @@ -39,6 +41,8 @@ import static java.net.HttpURLConnection.HTTP_NO_CONTENT; 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.Mockito.doThrow; +import static org.mockito.Mockito.mock; import static org.sonar.db.permission.GlobalPermission.ADMINISTER; import static org.sonar.server.usergroups.ws.GroupWsSupport.PARAM_GROUP_NAME; import static org.sonar.server.usergroups.ws.GroupWsSupport.PARAM_LOGIN; @@ -50,7 +54,9 @@ public class AddUserActionIT { @Rule public UserSessionRule userSession = UserSessionRule.standalone(); - private final WsActionTester ws = new WsActionTester(new AddUserAction(db.getDbClient(), userSession, newGroupWsSupport())); + private ManagedInstanceChecker managedInstanceChecker = mock(ManagedInstanceChecker.class); + + private final WsActionTester ws = new WsActionTester(new AddUserAction(db.getDbClient(), userSession, newGroupWsSupport(), managedInstanceChecker)); @Test public void verify_definition() { @@ -212,6 +218,17 @@ public class AddUserActionIT { .hasMessage("Default group cannot be found"); } + @Test + public void fail_if_instance_is_externally_managed() { + loginAsAdmin(); + BadRequestException exception = BadRequestException.create("Not allowed"); + doThrow(exception).when(managedInstanceChecker).throwIfInstanceIsManaged(); + TestRequest testRequest = newRequest() + .setParam("name", "long-desc"); + assertThatThrownBy(testRequest::execute) + .isEqualTo(exception); + } + private void executeRequest(GroupDto groupDto, UserDto userDto) { newRequest() .setParam(PARAM_GROUP_NAME, groupDto.getName()) diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/CreateActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/CreateActionIT.java index 91a5963a31e..42043b7af36 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/CreateActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/CreateActionIT.java @@ -31,12 +31,16 @@ import org.sonar.db.user.GroupDto; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.ServerException; +import org.sonar.server.management.ManagedInstanceChecker; import org.sonar.server.tester.UserSessionRule; +import org.sonar.server.ws.TestRequest; import org.sonar.server.ws.WsActionTester; 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.Mockito.doThrow; +import static org.mockito.Mockito.mock; import static org.sonar.db.permission.GlobalPermission.ADMINISTER; public class CreateActionIT { @@ -44,8 +48,9 @@ public class CreateActionIT { public DbTester db = DbTester.create(System2.INSTANCE); @Rule public UserSessionRule userSession = UserSessionRule.standalone(); + private ManagedInstanceChecker managedInstanceChecker = mock(ManagedInstanceChecker.class); - private final CreateAction underTest = new CreateAction(db.getDbClient(), userSession, newGroupService()); + private final CreateAction underTest = new CreateAction(db.getDbClient(), userSession, newGroupService(), managedInstanceChecker); private final WsActionTester tester = new WsActionTester(underTest); @Test @@ -173,6 +178,16 @@ public class CreateActionIT { .isInstanceOf(IllegalArgumentException.class); } + @Test + public void fail_if_instance_is_externally_managed() { + loginAsAdmin(); + BadRequestException exception = BadRequestException.create("Not allowed"); + doThrow(exception).when(managedInstanceChecker).throwIfInstanceIsManaged(); + TestRequest testRequest = tester.newRequest(); + assertThatThrownBy(testRequest::execute) + .isEqualTo(exception); + } + private void loginAsAdmin() { userSession.logIn().addPermission(ADMINISTER); } diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/DeleteActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/DeleteActionIT.java index 46fe34bfde0..81fa9575690 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/DeleteActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/DeleteActionIT.java @@ -19,6 +19,8 @@ */ package org.sonar.server.usergroups.ws; +import java.util.Map; +import java.util.Set; import org.junit.Rule; import org.junit.Test; import org.sonar.api.impl.utils.AlwaysIncreasingSystem2; @@ -26,6 +28,7 @@ import org.sonar.api.server.ws.Change; import org.sonar.api.server.ws.WebService.Action; import org.sonar.api.web.UserRole; import org.sonar.core.util.UuidFactoryImpl; +import org.sonar.db.DbSession; import org.sonar.db.DbTester; import org.sonar.db.component.ComponentDbTester; import org.sonar.db.component.ComponentDto; @@ -35,7 +38,9 @@ import org.sonar.db.qualitygate.QualityGateDto; import org.sonar.db.qualityprofile.QProfileDto; import org.sonar.db.user.GroupDto; import org.sonar.db.user.UserDto; +import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.management.ManagedInstanceService; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.TestRequest; import org.sonar.server.ws.TestResponse; @@ -45,6 +50,10 @@ 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.assertj.core.api.Assertions.tuple; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import static org.sonar.db.permission.GlobalPermission.ADMINISTER; import static org.sonar.server.usergroups.ws.GroupWsSupport.PARAM_GROUP_NAME; @@ -57,7 +66,9 @@ public class DeleteActionIT { private final ComponentDbTester componentTester = new ComponentDbTester(db); private final GroupService groupService = new GroupService(db.getDbClient(), UuidFactoryImpl.INSTANCE); - private final WsActionTester ws = new WsActionTester(new DeleteAction(db.getDbClient(), userSession, groupService)); + + private final ManagedInstanceService managedInstanceService = mock(ManagedInstanceService.class); + private final WsActionTester ws = new WsActionTester(new DeleteAction(db.getDbClient(), userSession, groupService, managedInstanceService)); @Test public void verify_definition() { @@ -273,6 +284,46 @@ public class DeleteActionIT { assertThat(db.users().selectGroupPermissions(adminGroup2, null)).hasSize(1); } + @Test + public void delete_local_group_when_instance_is_managed_shouldSucceed() { + when(managedInstanceService.isInstanceExternallyManaged()).thenReturn(true); + + addAdmin(); + insertDefaultGroup(); + GroupDto group = insertGroupAndMockIsManaged(false); + + loginAsAdmin(); + TestResponse response = newRequest() + .setParam(PARAM_GROUP_NAME, group.getName()) + .execute(); + + assertThat(response.getStatus()).isEqualTo(204); + } + + @Test + public void fail_to_delete_managed_group_when_instance_is_managed() { + when(managedInstanceService.isInstanceExternallyManaged()).thenReturn(true); + addAdmin(); + insertDefaultGroup(); + GroupDto group = insertGroupAndMockIsManaged(true); + + loginAsAdmin(); + TestRequest request = newRequest() + .setParam(PARAM_GROUP_NAME, group.getName()); + + assertThatThrownBy(request::execute) + .isInstanceOf(BadRequestException.class) + .hasMessage("Deleting managed groups is not allowed."); + + } + + private GroupDto insertGroupAndMockIsManaged(boolean isManaged) { + GroupDto group = db.users().insertGroup(); + when(managedInstanceService.getGroupUuidToManaged(any(DbSession.class), eq(Set.of(group.getUuid())))) + .thenReturn(Map.of(group.getUuid(), isManaged)); + return group; + } + private void executeDeleteGroupRequest(GroupDto adminGroup1) { newRequest() .setParam(PARAM_GROUP_NAME, adminGroup1.getName()) diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/RemoveUserActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/RemoveUserActionIT.java index a8cef179bf1..088f91aef2d 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/RemoveUserActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/RemoveUserActionIT.java @@ -30,6 +30,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.tester.UserSessionRule; import org.sonar.server.usergroups.DefaultGroupFinder; import org.sonar.server.ws.TestRequest; @@ -40,6 +41,8 @@ import static java.net.HttpURLConnection.HTTP_NO_CONTENT; 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.Mockito.doThrow; +import static org.mockito.Mockito.mock; import static org.sonar.db.permission.GlobalPermission.ADMINISTER; import static org.sonar.server.usergroups.ws.GroupWsSupport.PARAM_GROUP_NAME; import static org.sonar.server.usergroups.ws.GroupWsSupport.PARAM_LOGIN; @@ -51,8 +54,9 @@ public class RemoveUserActionIT { @Rule public UserSessionRule userSession = UserSessionRule.standalone(); + private ManagedInstanceChecker managedInstanceChecker = mock(ManagedInstanceChecker.class); private final WsActionTester ws = new WsActionTester( - new RemoveUserAction(db.getDbClient(), userSession, new GroupWsSupport(db.getDbClient(), new DefaultGroupFinder(db.getDbClient())))); + new RemoveUserAction(db.getDbClient(), userSession, new GroupWsSupport(db.getDbClient(), new DefaultGroupFinder(db.getDbClient())), managedInstanceChecker)); @Test public void verify_definition() { @@ -212,6 +216,16 @@ public class RemoveUserActionIT { .hasMessage("Default group 'sonar-users' cannot be used to perform this action"); } + @Test + public void fail_if_instance_is_externally_managed() { + loginAsAdmin(); + BadRequestException exception = BadRequestException.create("Not allowed"); + doThrow(exception).when(managedInstanceChecker).throwIfInstanceIsManaged(); + TestRequest testRequest = newRequest(); + assertThatThrownBy(testRequest::execute) + .isEqualTo(exception); + } + private TestRequest newRequest() { return ws.newRequest(); } diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/UpdateActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/UpdateActionIT.java index e77c5088a13..c065b8e5300 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/UpdateActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/usergroups/ws/UpdateActionIT.java @@ -32,12 +32,15 @@ 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.tester.UserSessionRule; import org.sonar.server.ws.TestRequest; import org.sonar.server.ws.WsActionTester; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; import static org.sonar.db.permission.GlobalPermission.ADMINISTER; import static org.sonar.server.usergroups.ws.GroupWsSupport.PARAM_GROUP_CURRENT_NAME; import static org.sonar.server.usergroups.ws.GroupWsSupport.PARAM_GROUP_DESCRIPTION; @@ -51,8 +54,10 @@ public class UpdateActionIT { @Rule public UserSessionRule userSession = UserSessionRule.standalone(); + private ManagedInstanceChecker managedInstanceChecker = mock(ManagedInstanceChecker.class); + private final WsActionTester ws = new WsActionTester( - new UpdateAction(db.getDbClient(), userSession, new GroupService(db.getDbClient(), UuidFactoryImpl.INSTANCE))); + new UpdateAction(db.getDbClient(), userSession, new GroupService(db.getDbClient(), UuidFactoryImpl.INSTANCE), managedInstanceChecker)); @Test public void verify_definition() { @@ -187,6 +192,7 @@ public class UpdateActionIT { TestRequest request = newRequest() .setParam(PARAM_GROUP_NAME, "newname"); + loginAsAdmin(); assertThatThrownBy(request::execute) .isInstanceOf(IllegalArgumentException.class) .hasMessage("The 'currentName' parameter is missing"); @@ -259,6 +265,16 @@ public class UpdateActionIT { .hasMessage("Default group 'sonar-users' cannot be used to perform this action"); } + @Test + public void fail_if_instance_is_externally_managed() { + loginAsAdmin(); + BadRequestException exception = BadRequestException.create("Not allowed"); + doThrow(exception).when(managedInstanceChecker).throwIfInstanceIsManaged(); + TestRequest testRequest = newRequest(); + assertThatThrownBy(testRequest::execute) + .isEqualTo(exception); + } + private TestRequest newRequest() { return ws.newRequest(); } 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 new file mode 100644 index 00000000000..ce27b4b2b3a --- /dev/null +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/management/ManagedInstanceChecker.java @@ -0,0 +1,35 @@ +/* + * SonarQube + * Copyright (C) 2009-2023 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.management; + +import org.sonar.server.exceptions.BadRequestException; + +public class ManagedInstanceChecker { + + private final ManagedInstanceService managedInstanceService; + + public ManagedInstanceChecker(ManagedInstanceService managedInstanceService) { + this.managedInstanceService = managedInstanceService; + } + + public void throwIfInstanceIsManaged() { + BadRequestException.checkRequest(!managedInstanceService.isInstanceExternallyManaged(), "Operation not allowed when the instance is externally managed."); + } +} diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/AddUserAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/AddUserAction.java index 51ddfd119bc..dfd97224ee6 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/AddUserAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/AddUserAction.java @@ -29,6 +29,7 @@ import org.sonar.db.DbSession; import org.sonar.db.user.GroupDto; import org.sonar.db.user.UserDto; import org.sonar.db.user.UserGroupDto; +import org.sonar.server.management.ManagedInstanceChecker; import org.sonar.server.user.UserSession; import static java.lang.String.format; @@ -44,11 +45,13 @@ public class AddUserAction implements UserGroupsWsAction { private final DbClient dbClient; private final UserSession userSession; private final GroupWsSupport support; + private final ManagedInstanceChecker managedInstanceChecker; - public AddUserAction(DbClient dbClient, UserSession userSession, GroupWsSupport support) { + public AddUserAction(DbClient dbClient, UserSession userSession, GroupWsSupport support, ManagedInstanceChecker managedInstanceChecker) { this.dbClient = dbClient; this.userSession = userSession; this.support = support; + this.managedInstanceChecker = managedInstanceChecker; } @Override @@ -71,8 +74,9 @@ public class AddUserAction implements UserGroupsWsAction { @Override public void handle(Request request, Response response) throws Exception { try (DbSession dbSession = dbClient.openSession(false)) { - GroupDto group = support.findGroupDto(dbSession, request); userSession.checkLoggedIn().checkPermission(ADMINISTER); + managedInstanceChecker.throwIfInstanceIsManaged(); + GroupDto group = support.findGroupDto(dbSession, request); String login = request.mandatoryParam(PARAM_LOGIN); UserDto user = dbClient.userDao().selectActiveUserByLogin(dbSession, login); diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/CreateAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/CreateAction.java index 2448bbfdae5..cfc53523bfb 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/CreateAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/CreateAction.java @@ -27,6 +27,7 @@ import org.sonar.api.server.ws.WebService.NewController; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.user.GroupDto; +import org.sonar.server.management.ManagedInstanceChecker; import org.sonar.server.user.UserSession; import org.sonarqube.ws.UserGroups; @@ -44,11 +45,13 @@ public class CreateAction implements UserGroupsWsAction { private final DbClient dbClient; private final UserSession userSession; private final GroupService groupService; + private final ManagedInstanceChecker managedInstanceChecker; - public CreateAction(DbClient dbClient, UserSession userSession, GroupService groupService) { + public CreateAction(DbClient dbClient, UserSession userSession, GroupService groupService, ManagedInstanceChecker managedInstanceService) { this.dbClient = dbClient; this.userSession = userSession; this.groupService = groupService; + this.managedInstanceChecker = managedInstanceService; } @Override @@ -81,7 +84,7 @@ public class CreateAction implements UserGroupsWsAction { try (DbSession dbSession = dbClient.openSession(false)) { userSession.checkPermission(ADMINISTER); - + managedInstanceChecker.throwIfInstanceIsManaged(); String groupName = request.mandatoryParam(PARAM_GROUP_NAME); String groupDescription = request.param(PARAM_GROUP_DESCRIPTION); GroupDto group = groupService.createGroup(dbSession, groupName, groupDescription); diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/DeleteAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/DeleteAction.java index 91f6e62b2f0..bc16c54cca6 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/DeleteAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/DeleteAction.java @@ -19,6 +19,7 @@ */ package org.sonar.server.usergroups.ws; +import java.util.Set; import org.sonar.api.server.ws.Change; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; @@ -28,7 +29,9 @@ import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.permission.GlobalPermission; import org.sonar.db.user.GroupDto; +import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.management.ManagedInstanceService; import org.sonar.server.user.UserSession; import static java.lang.String.format; @@ -40,11 +43,13 @@ public class DeleteAction implements UserGroupsWsAction { private final DbClient dbClient; private final UserSession userSession; private final GroupService groupService; + private final ManagedInstanceService managedInstanceService; - public DeleteAction(DbClient dbClient, UserSession userSession, GroupService groupService) { + public DeleteAction(DbClient dbClient, UserSession userSession, GroupService groupService, ManagedInstanceService managedInstanceService) { this.dbClient = dbClient; this.userSession = userSession; this.groupService = groupService; + this.managedInstanceService = managedInstanceService; } @Override @@ -67,8 +72,8 @@ public class DeleteAction implements UserGroupsWsAction { public void handle(Request request, Response response) throws Exception { try (DbSession dbSession = dbClient.openSession(false)) { userSession.checkPermission(GlobalPermission.ADMINISTER); - GroupDto group = findGroupOrThrow(request, dbSession); + checkIfInstanceAndGroupAreManaged(dbSession, group); groupService.delete(dbSession, group); dbSession.commit(); @@ -76,6 +81,13 @@ public class DeleteAction implements UserGroupsWsAction { } } + private void checkIfInstanceAndGroupAreManaged(DbSession dbSession, GroupDto group) { + boolean isGroupManaged = managedInstanceService.getGroupUuidToManaged(dbSession, Set.of(group.getUuid())).getOrDefault(group.getUuid(), false); + if (isGroupManaged) { + throw BadRequestException.create("Deleting managed groups is not allowed."); + } + } + private GroupDto findGroupOrThrow(Request request, DbSession dbSession) { String groupName = request.mandatoryParam(PARAM_GROUP_NAME); return groupService.findGroup(dbSession, groupName) diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/ManagedInstanceChecker.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/ManagedInstanceChecker.java deleted file mode 100644 index ed87df8c2ba..00000000000 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/ManagedInstanceChecker.java +++ /dev/null @@ -1,17 +0,0 @@ -package org.sonar.server.usergroups.ws; - -import org.sonar.server.exceptions.BadRequestException; -import org.sonar.server.management.ManagedInstanceService; - -public class ManagedInstanceChecker { - - private final ManagedInstanceService managedInstanceService; - - public ManagedInstanceChecker(ManagedInstanceService managedInstanceService) { - this.managedInstanceService = managedInstanceService; - } - - public void checkInstanceIsNotExternallyManaged() { - BadRequestException.checkRequest(!managedInstanceService.isInstanceExternallyManaged(), "Operation not allowed when instance is externally managed."); - } -} diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/RemoveUserAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/RemoveUserAction.java index a7ef68faaa8..7e4d301fd76 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/RemoveUserAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/RemoveUserAction.java @@ -29,6 +29,7 @@ import org.sonar.db.DbSession; import org.sonar.db.permission.GlobalPermission; import org.sonar.db.user.GroupDto; import org.sonar.db.user.UserDto; +import org.sonar.server.management.ManagedInstanceChecker; import org.sonar.server.user.UserSession; import static java.lang.String.format; @@ -45,10 +46,13 @@ public class RemoveUserAction implements UserGroupsWsAction { private final UserSession userSession; private final GroupWsSupport support; - public RemoveUserAction(DbClient dbClient, UserSession userSession, GroupWsSupport support) { + private final ManagedInstanceChecker managedInstanceChecker; + + public RemoveUserAction(DbClient dbClient, UserSession userSession, GroupWsSupport support, ManagedInstanceChecker managedInstanceChecker) { this.dbClient = dbClient; this.userSession = userSession; this.support = support; + this.managedInstanceChecker = managedInstanceChecker; } @Override @@ -73,8 +77,9 @@ public class RemoveUserAction implements UserGroupsWsAction { userSession.checkLoggedIn(); try (DbSession dbSession = dbClient.openSession(false)) { - GroupDto group = support.findGroupDto(dbSession, request); userSession.checkPermission(GlobalPermission.ADMINISTER); + managedInstanceChecker.throwIfInstanceIsManaged(); + GroupDto group = support.findGroupDto(dbSession, request); support.checkGroupIsNotDefault(dbSession, group); String login = request.mandatoryParam(PARAM_LOGIN); diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/UpdateAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/UpdateAction.java index 7c31c49b206..e67a3ae8887 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/UpdateAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/UpdateAction.java @@ -29,6 +29,7 @@ import org.sonar.db.DbSession; import org.sonar.db.user.GroupDto; import org.sonar.db.user.UserMembershipQuery; import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.management.ManagedInstanceChecker; import org.sonar.server.user.UserSession; import org.sonarqube.ws.UserGroups; @@ -49,10 +50,13 @@ public class UpdateAction implements UserGroupsWsAction { private final UserSession userSession; private final GroupService groupService; - public UpdateAction(DbClient dbClient, UserSession userSession, GroupService groupService) { + private final ManagedInstanceChecker managedInstanceChecker; + + public UpdateAction(DbClient dbClient, UserSession userSession, GroupService groupService, ManagedInstanceChecker managedInstanceChecker) { this.dbClient = dbClient; this.userSession = userSession; this.groupService = groupService; + this.managedInstanceChecker = managedInstanceChecker; } @Override @@ -92,12 +96,13 @@ public class UpdateAction implements UserGroupsWsAction { @Override public void handle(Request request, Response response) throws Exception { try (DbSession dbSession = dbClient.openSession(false)) { + userSession.checkPermission(ADMINISTER); + managedInstanceChecker.throwIfInstanceIsManaged(); String currentName = request.mandatoryParam(PARAM_GROUP_CURRENT_NAME); GroupDto group = dbClient.groupDao().selectByName(dbSession, currentName) .orElseThrow(() -> new NotFoundException(format("Could not find a user group with name '%s'.", currentName))); - userSession.checkPermission(ADMINISTER); String newName = request.param(PARAM_GROUP_NAME); String description = request.param(PARAM_GROUP_DESCRIPTION); diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/UserGroupsModule.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/UserGroupsModule.java index 9ca4ba90692..b4dd9c03a1f 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/UserGroupsModule.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/UserGroupsModule.java @@ -20,6 +20,7 @@ package org.sonar.server.usergroups.ws; import org.sonar.core.platform.Module; +import org.sonar.server.management.ManagedInstanceChecker; public class UserGroupsModule extends Module { @@ -28,6 +29,7 @@ public class UserGroupsModule extends Module { add( UserGroupsWs.class, GroupWsSupport.class, + ManagedInstanceChecker.class, GroupService.class, // actions SearchAction.class, 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 new file mode 100644 index 00000000000..827bab8002f --- /dev/null +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/management/ManagedInstanceCheckerTest.java @@ -0,0 +1,55 @@ +/* + * SonarQube + * Copyright (C) 2009-2023 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.management; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +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.when; + +@RunWith(MockitoJUnitRunner.class) +public class ManagedInstanceCheckerTest { + + @Mock + private ManagedInstanceService managedInstanceService; + + @InjectMocks + private ManagedInstanceChecker managedInstanceChecker; + + @Test + public void throwIfInstanceIsManaged_whenInstanceExternallyManaged_throws() { + when(managedInstanceService.isInstanceExternallyManaged()).thenReturn(true); + assertThatThrownBy(() -> managedInstanceChecker.throwIfInstanceIsManaged()) + .isInstanceOf(BadRequestException.class) + .hasMessage("Operation not allowed when the instance is externally managed."); + } + + @Test + public void throwIfInstanceIsManaged_whenInstanceNotExternallyManaged_doesntThrow() { + when(managedInstanceService.isInstanceExternallyManaged()).thenReturn(false); + assertThatNoException().isThrownBy(() -> managedInstanceChecker.throwIfInstanceIsManaged()); + } +} -- 2.39.5