From: Julien Lancelot Date: Mon, 20 Mar 2017 08:47:27 +0000 (+0100) Subject: SONAR-8956 Prevent removing last org admin member X-Git-Tag: 6.4-RC1~698 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=705fce4ebde2c7287d6e49f2aebb6e47d26e1f8e;p=sonarqube.git SONAR-8956 Prevent removing last org admin member --- diff --git a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/RemoveMemberAction.java b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/RemoveMemberAction.java index fa94f6ea922..1c31b25fd60 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/RemoveMemberAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/RemoveMemberAction.java @@ -27,17 +27,18 @@ import org.sonar.api.server.ws.WebService.NewController; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.organization.OrganizationDto; -import org.sonar.db.permission.OrganizationPermission; import org.sonar.db.user.UserDto; import org.sonar.server.user.UserSession; import static java.util.Collections.singletonList; import static org.sonar.api.CoreProperties.DEFAULT_ISSUE_ASSIGNEE; +import static org.sonar.db.permission.OrganizationPermission.ADMINISTER; import static org.sonar.server.organization.ws.OrganizationsWsSupport.PARAM_LOGIN; import static org.sonar.server.organization.ws.OrganizationsWsSupport.PARAM_ORGANIZATION; import static org.sonar.server.ws.KeyExamples.KEY_ORG_EXAMPLE_001; import static org.sonar.server.ws.WsUtils.checkFound; import static org.sonar.server.ws.WsUtils.checkFoundWithOptional; +import static org.sonar.server.ws.WsUtils.checkRequest; public class RemoveMemberAction implements OrganizationsWsAction { private final DbClient dbClient; @@ -79,21 +80,19 @@ public class RemoveMemberAction implements OrganizationsWsAction { try (DbSession dbSession = dbClient.openSession(false)) { OrganizationDto organization = checkFoundWithOptional(dbClient.organizationDao().selectByKey(dbSession, organizationKey), "Organization '%s' is not found", organizationKey); - String organizationUuid = organization.getUuid(); UserDto user = checkFound(dbClient.userDao().selectActiveUserByLogin(dbSession, login), "User '%s' is not found", login); - int userId = user.getId(); + userSession.checkPermission(ADMINISTER, organization); - userSession.checkPermission(OrganizationPermission.ADMINISTER, organization); - - dbClient.organizationMemberDao().select(dbSession, organizationUuid, userId) - .ifPresent(om -> removeMember(dbSession, organizationUuid, user)); + dbClient.organizationMemberDao().select(dbSession, organization.getUuid(), user.getId()) + .ifPresent(om -> removeMember(dbSession, organization, user)); } - response.noContent(); } - private void removeMember(DbSession dbSession, String organizationUuid, UserDto user) { + private void removeMember(DbSession dbSession, OrganizationDto organization, UserDto user) { + ensureLastAdminIsNotRemoved(dbSession, organization, user); int userId = user.getId(); + String organizationUuid = organization.getUuid(); dbClient.userPermissionDao().deleteOrganizationMemberPermissions(dbSession, organizationUuid, userId); dbClient.permissionTemplateDao().deleteUserPermissionsByOrganization(dbSession, organizationUuid, userId); dbClient.userGroupDao().deleteByOrganizationAndUser(dbSession, organizationUuid, userId); @@ -103,4 +102,11 @@ public class RemoveMemberAction implements OrganizationsWsAction { dbClient.organizationMemberDao().delete(dbSession, organizationUuid, userId); dbSession.commit(); } + + private void ensureLastAdminIsNotRemoved(DbSession dbSession, OrganizationDto organizationDto, UserDto user) { + int remainingAdmins = dbClient.authorizationDao().countUsersWithGlobalPermissionExcludingUser(dbSession, + organizationDto.getUuid(), ADMINISTER.getKey(), user.getId()); + checkRequest(remainingAdmins > 0, "The last administrator member cannot be removed"); + } + } diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ws/DeactivateAction.java b/server/sonar-server/src/main/java/org/sonar/server/user/ws/DeactivateAction.java index 088ce719c23..6f05d7a6407 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/ws/DeactivateAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/ws/DeactivateAction.java @@ -129,7 +129,7 @@ public class DeactivateAction implements UsersWsAction { .map(OrganizationDto::getKey) .sorted() .collect(Collectors.joining(", ")); - throw BadRequestException.create(format("User is last administrator of organizations [%s], and cannot be deactivated", keys)); + throw BadRequestException.create(format("User '%s' is last administrator of organizations [%s], and cannot be deactivated", user.getLogin(), keys)); } private List selectOrganizationsWithNoMoreAdministrators(DbSession dbSession, UserDto user) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/RemoveMemberActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/RemoveMemberActionTest.java index 9ed91f312b2..b80f5b15f07 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/RemoveMemberActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/RemoveMemberActionTest.java @@ -38,6 +38,7 @@ import org.sonar.db.property.PropertyDto; import org.sonar.db.property.PropertyQuery; import org.sonar.db.user.GroupDto; 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.tester.UserSessionRule; @@ -76,9 +77,13 @@ public class RemoveMemberActionTest { @Before public void setUp() { organization = db.organizations().insert(); + project = db.components().insertProject(organization); + user = db.users().insertUser(); db.organizations().addMember(organization, user); - project = db.components().insertProject(organization); + + UserDto adminUser = db.users().insertAdminByUserPermission(organization); + db.organizations().addMember(organization, adminUser); } @Test @@ -158,8 +163,10 @@ public class RemoveMemberActionTest { call(organization.getKey(), user.getLogin()); - assertThat(dbClient.permissionTemplateDao().selectUserPermissionsByTemplateId(dbSession, template.getId())).extracting(PermissionTemplateUserDto::getUserId).containsOnly(anotherUser.getId()); - assertThat(dbClient.permissionTemplateDao().selectUserPermissionsByTemplateId(dbSession, anotherTemplate.getId())).extracting(PermissionTemplateUserDto::getUserId).containsOnly(user.getId()); + assertThat(dbClient.permissionTemplateDao().selectUserPermissionsByTemplateId(dbSession, template.getId())).extracting(PermissionTemplateUserDto::getUserId) + .containsOnly(anotherUser.getId()); + assertThat(dbClient.permissionTemplateDao().selectUserPermissionsByTemplateId(dbSession, anotherTemplate.getId())).extracting(PermissionTemplateUserDto::getUserId) + .containsOnly(user.getId()); } @Test @@ -281,6 +288,34 @@ public class RemoveMemberActionTest { call(organization.getKey(), user.getLogin()); } + @Test + public void remove_org_admin_is_allowed_when_another_org_admin_exists() throws Exception { + OrganizationDto anotherOrganization = db.organizations().insert(); + UserDto admin1 = db.users().insertAdminByUserPermission(anotherOrganization); + db.organizations().addMember(anotherOrganization, admin1); + UserDto admin2 = db.users().insertAdminByUserPermission(anotherOrganization); + db.organizations().addMember(anotherOrganization, admin2); + + call(anotherOrganization.getKey(), admin1.getLogin()); + + assertNotAMember(anotherOrganization.getUuid(), admin1.getId()); + assertMember(anotherOrganization.getUuid(), admin2.getId()); + } + + @Test + public void fail_to_remove_last_organization_admin() { + OrganizationDto anotherOrganization = db.organizations().insert(); + UserDto admin = db.users().insertAdminByUserPermission(anotherOrganization); + db.organizations().addMember(anotherOrganization, admin); + UserDto user = db.users().insertUser(); + db.organizations().addMember(anotherOrganization, user); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage("The last administrator member cannot be removed"); + + call(anotherOrganization.getKey(), admin.getLogin()); + } + private TestResponse call(@Nullable String organizationKey, @Nullable String login) { TestRequest request = ws.newRequest(); setNullable(organizationKey, o -> request.setParam(PARAM_ORGANIZATION, o)); diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ws/DeactivateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ws/DeactivateActionTest.java index 2dee740c0b2..c5fe0df7568 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/ws/DeactivateActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/ws/DeactivateActionTest.java @@ -81,13 +81,6 @@ public class DeactivateActionTest { private WsActionTester ws = new WsActionTester(new DeactivateAction( dbClient, userIndexer, userSession, new UserJsonWriter(userSession), defaultOrganizationProvider)); - @Test - public void test_definition() { - assertThat(ws.getDef().isPost()).isTrue(); - assertThat(ws.getDef().isInternal()).isFalse(); - assertThat(ws.getDef().params()).hasSize(1); - } - @Test public void deactivate_user_and_delete_his_related_data() throws Exception { UserDto user = insertUser(newUserDto() @@ -100,12 +93,10 @@ public class DeactivateActionTest { String json = deactivate(user.getLogin()).getInput(); // scm accounts, groups and email are deleted - assertJson(json).isSimilarTo(ws.getDef().responseExampleAsString()); - assertThat(index.getNullableByLogin(user.getLogin()).active()).isFalse(); verifyThatUserIsDeactivated(user.getLogin()); assertThat(dbClient.userTokenDao().selectByLogin(dbSession, user.getLogin())).isEmpty(); - assertThat(dbClient.propertiesDao().selectByQuery(PropertyQuery.builder().setUserId(user.getId().intValue()).build(), dbSession)).isEmpty(); + assertThat(dbClient.propertiesDao().selectByQuery(PropertyQuery.builder().setUserId(user.getId()).build(), dbSession)).isEmpty(); } @Test @@ -191,7 +182,7 @@ public class DeactivateActionTest { public void fail_to_deactivate_last_administrator_of_organization() throws Exception { // user1 is the unique administrator of org1 and org2. // user1 and user2 are both administrators of org3 - UserDto user1 = createUser(); + UserDto user1 = insertUser(newUserDto().setLogin("test")); OrganizationDto org1 = db.organizations().insert(newOrganizationDto().setKey("org1")); OrganizationDto org2 = db.organizations().insert(newOrganizationDto().setKey("org2")); OrganizationDto org3 = db.organizations().insert(newOrganizationDto().setKey("org3")); @@ -203,7 +194,7 @@ public class DeactivateActionTest { logInAsSystemAdministrator(); expectedException.expect(BadRequestException.class); - expectedException.expectMessage("User is last administrator of organizations [org1, org2], and cannot be deactivated"); + expectedException.expectMessage("User 'test' is last administrator of organizations [org1, org2], and cannot be deactivated"); deactivate(user1.getLogin()); } @@ -223,6 +214,27 @@ public class DeactivateActionTest { verifyThatUserExists(anotherAdmin.getLogin()); } + @Test + public void test_definition() { + assertThat(ws.getDef().isPost()).isTrue(); + assertThat(ws.getDef().isInternal()).isFalse(); + assertThat(ws.getDef().params()).hasSize(1); + } + + @Test + public void test_example() throws Exception { + UserDto user = insertUser(newUserDto() + .setLogin("ada.lovelace") + .setEmail("ada.lovelace@noteg.com") + .setName("Ada Lovelace") + .setScmAccounts(singletonList("al"))); + logInAsSystemAdministrator(); + + String json = deactivate(user.getLogin()).getInput(); + + assertJson(json).isSimilarTo(ws.getDef().responseExampleAsString()); + } + private UserDto createUser() { return insertUser(newUserDto()); }