From 0f024154967af258864b23e7bd13a737bf70cfa9 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Tue, 15 Nov 2016 20:40:03 +0100 Subject: [PATCH] SONAR-6912 do not deactivate last organization administrator --- .../server/user/ws/DeactivateAction.java | 63 +++++++++++++-- .../server/user/ws/DeactivateActionTest.java | 78 +++++++++++++++---- .../sonar/db/permission/AuthorizationDao.java | 11 +++ .../db/permission/AuthorizationMapper.java | 2 + .../db/permission/AuthorizationMapper.xml | 20 +++++ .../db/permission/AuthorizationDaoTest.java | 69 ++++++++++++++++ 6 files changed, 223 insertions(+), 20 deletions(-) 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 5185d935f5b..121da89409e 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 @@ -20,20 +20,27 @@ package org.sonar.server.user.ws; import com.google.common.collect.Sets; +import java.util.ArrayList; +import java.util.List; import java.util.Set; +import java.util.stream.Collectors; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; import org.sonar.api.server.ws.WebService.NewAction; import org.sonar.api.utils.text.JsonWriter; -import org.sonar.core.permission.GlobalPermissions; import org.sonar.db.DbClient; import org.sonar.db.DbSession; +import org.sonar.db.organization.OrganizationDto; import org.sonar.db.user.UserDto; +import org.sonar.server.exceptions.BadRequestException; +import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.user.UserSession; import org.sonar.server.user.index.UserIndexer; +import static java.lang.String.format; import static java.util.Collections.singletonList; +import static org.sonar.core.permission.GlobalPermissions.SYSTEM_ADMIN; import static org.sonar.server.ws.WsUtils.checkFound; import static org.sonar.server.ws.WsUtils.checkRequest; @@ -45,12 +52,15 @@ public class DeactivateAction implements UsersWsAction { private final UserIndexer userIndexer; private final UserSession userSession; private final UserJsonWriter userWriter; + private final DefaultOrganizationProvider defaultOrganizationProvider; - public DeactivateAction(DbClient dbClient, UserIndexer userIndexer, UserSession userSession, UserJsonWriter userWriter) { + public DeactivateAction(DbClient dbClient, UserIndexer userIndexer, UserSession userSession, UserJsonWriter userWriter, + DefaultOrganizationProvider defaultOrganizationProvider) { this.dbClient = dbClient; this.userIndexer = userIndexer; this.userSession = userSession; this.userWriter = userWriter; + this.defaultOrganizationProvider = defaultOrganizationProvider; } @Override @@ -69,16 +79,17 @@ public class DeactivateAction implements UsersWsAction { @Override public void handle(Request request, Response response) throws Exception { - userSession.checkLoggedIn().checkPermission(GlobalPermissions.SYSTEM_ADMIN); + userSession.checkLoggedIn().checkPermission(SYSTEM_ADMIN); String login = request.mandatoryParam(PARAM_LOGIN); checkRequest(!login.equals(userSession.getLogin()), "Self-deactivation is not possible"); - UserDto user; try (DbSession dbSession = dbClient.openSession(false)) { - user = dbClient.userDao().selectByLogin(dbSession, login); + UserDto user = dbClient.userDao().selectByLogin(dbSession, login); checkFound(user, "User '%s' doesn't exist", login); + ensureNotLastAdministrator(dbSession, user); + dbClient.userTokenDao().deleteByLogin(dbSession, login); dbClient.userDao().deactivateUserByLogin(dbSession, login); dbSession.commit(); @@ -90,15 +101,53 @@ public class DeactivateAction implements UsersWsAction { private void writeResponse(Response response, String login) { try (DbSession dbSession = dbClient.openSession(false)) { + UserDto user = dbClient.userDao().selectByLogin(dbSession, login); + // safeguard. It exists as the check has already been done earlier + // when deactivating user + checkFound(user, "User '%s' doesn't exist", login); + JsonWriter json = response.newJsonWriter().beginObject(); json.name("user"); Set groups = Sets.newHashSet(); - UserDto user = dbClient.userDao().selectByLogin(dbSession, login); - checkFound(user, "User '%s' doesn't exist", login); groups.addAll(dbClient.groupMembershipDao().selectGroupsByLogins(dbSession, singletonList(login)).get(login)); userWriter.write(json, user, groups, UserJsonWriter.FIELDS); json.endObject().close(); } } + private void ensureNotLastAdministrator(DbSession dbSession, UserDto user) { + List problematicOrgs = selectOrganizationsWithNoMoreAdministrators(dbSession, user); + if (!problematicOrgs.isEmpty()) { + if (problematicOrgs.size() == 1 && defaultOrganizationProvider.get().getUuid().equals(problematicOrgs.get(0))) { + throw new BadRequestException("User is last administrator, and cannot be deactivated"); + } + String keys = problematicOrgs + .stream() + .map(orgUuid -> selectOrganizationByUuid(dbSession, orgUuid, user)) + .map(OrganizationDto::getKey) + .sorted() + .collect(Collectors.joining(", ")); + throw new BadRequestException(format("User is last administrator of organizations [%s], and cannot be deactivated", keys)); + + } + } + + private List selectOrganizationsWithNoMoreAdministrators(DbSession dbSession, UserDto user) { + Set organizationUuids = dbClient.authorizationDao().selectOrganizationUuidsOfUserWithGlobalPermission( + dbSession, user.getId(), SYSTEM_ADMIN); + List problematicOrganizations = new ArrayList<>(); + for (String organizationUuid : organizationUuids) { + int remaining = dbClient.authorizationDao().countUsersWithGlobalPermissionExcludingUser(dbSession, organizationUuid, SYSTEM_ADMIN, user.getId()); + if (remaining == 0) { + problematicOrganizations.add(organizationUuid); + } + } + return problematicOrganizations; + } + + private OrganizationDto selectOrganizationByUuid(DbSession dbSession, String orgUuid, UserDto user) { + return dbClient.organizationDao() + .selectByUuid(dbSession, orgUuid) + .orElseThrow(() -> new IllegalStateException("Organization with UUID " + orgUuid + " does not exist in DB but is referenced in permissions of user " + user.getLogin())); + } } 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 8bf8163eb1b..7cd0b79fd36 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 @@ -30,17 +30,17 @@ import org.sonar.api.utils.internal.AlwaysIncreasingSystem2; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.DbTester; +import org.sonar.db.organization.OrganizationDto; import org.sonar.db.property.PropertyDto; import org.sonar.db.property.PropertyQuery; import org.sonar.db.user.UserDto; -import org.sonar.db.user.UserTesting; import org.sonar.server.es.EsTester; 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.organization.TestDefaultOrganizationProvider; import org.sonar.server.tester.UserSessionRule; -import org.sonar.server.user.index.UserDoc; import org.sonar.server.user.index.UserIndex; import org.sonar.server.user.index.UserIndexDefinition; import org.sonar.server.user.index.UserIndexer; @@ -50,6 +50,8 @@ import org.sonar.server.ws.WsActionTester; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.sonar.core.permission.GlobalPermissions.SYSTEM_ADMIN; +import static org.sonar.db.organization.OrganizationTesting.newOrganizationDto; +import static org.sonar.db.user.UserTesting.newUserDto; import static org.sonar.db.user.UserTokenTesting.newUserToken; import static org.sonar.test.JsonAssert.assertJson; @@ -69,6 +71,7 @@ public class DeactivateActionTest { @Rule public UserSessionRule userSession = UserSessionRule.standalone(); + private TestDefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(db); private WsActionTester ws; private UserIndex index; private DbClient dbClient = db.getDbClient(); @@ -81,7 +84,7 @@ public class DeactivateActionTest { index = new UserIndex(esTester.client()); userIndexer = new UserIndexer(system2, dbClient, esTester.client()); ws = new WsActionTester(new DeactivateAction( - dbClient, userIndexer, userSession, new UserJsonWriter(userSession))); + dbClient, userIndexer, userSession, new UserJsonWriter(userSession), defaultOrganizationProvider)); } @Test @@ -93,15 +96,17 @@ public class DeactivateActionTest { @Test public void deactivate_user_and_delete_his_related_data() throws Exception { - UserDto user = createUser(); + UserDto user = insertUser(newUserDto().setEmail("john@email.com") + .setLogin("john") + .setName("John") + .setScmAccounts(singletonList("jn"))); loginAsAdmin(); String json = deactivate(user.getLogin()).getInput(); assertJson(json).isSimilarTo(getClass().getResource("DeactivateActionTest/deactivate_user.json")); - UserDoc userDoc = index.getNullableByLogin(user.getLogin()); - assertThat(userDoc.active()).isFalse(); + 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(); @@ -158,17 +163,64 @@ public class DeactivateActionTest { deactivate(""); } + @Test + public void fail_to_deactivate_last_administrator_of_default_organization() throws Exception { + UserDto admin = createUser(); + db.users().insertPermissionOnUser(admin, SYSTEM_ADMIN); + loginAsAdmin(); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage("User is last administrator, and cannot be deactivated"); + + deactivate(admin.getLogin()); + } + + @Test + 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(); + OrganizationDto org1 = db.organizations().insert(newOrganizationDto().setKey("org1")); + OrganizationDto org2 = db.organizations().insert(newOrganizationDto().setKey("org2")); + OrganizationDto org3 = db.organizations().insert(newOrganizationDto().setKey("org3")); + db.users().insertPermissionOnUser(org1, user1, SYSTEM_ADMIN); + db.users().insertPermissionOnUser(org2, user1, SYSTEM_ADMIN); + db.users().insertPermissionOnUser(org3, user1, SYSTEM_ADMIN); + UserDto user2 = createUser(); + db.users().insertPermissionOnUser(org3, user2, SYSTEM_ADMIN); + loginAsAdmin(); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage("User is last administrator of organizations [org1, org2], and cannot be deactivated"); + + deactivate(user1.getLogin()); + } + + @Test + public void administrators_can_be_deactivated_if_there_are_still_other_administrators() throws Exception { + UserDto admin = createUser(); + UserDto anotherAdmin = createUser(); + db.users().insertPermissionOnUser(admin, SYSTEM_ADMIN); + db.users().insertPermissionOnUser(anotherAdmin, SYSTEM_ADMIN); + db.commit(); + loginAsAdmin(); + + deactivate(admin.getLogin()); + + verifyThatUserIsDeactivated(admin.getLogin()); + verifyThatUserExists(anotherAdmin.getLogin()); + } + private UserDto createUser() { - UserDto user = UserTesting.newUserDto() - .setActive(true) - .setEmail("john@email.com") - .setLogin("john") - .setName("John") - .setScmAccounts(singletonList("jn")) + return insertUser(newUserDto()); + } + + private UserDto insertUser(UserDto user) { + user .setCreatedAt(system2.now()) .setUpdatedAt(system2.now()); dbClient.userDao().insert(dbSession, user); - dbClient.userTokenDao().insert(dbSession, newUserToken().setLogin("john")); + dbClient.userTokenDao().insert(dbSession, newUserToken().setLogin(user.getLogin())); dbClient.propertiesDao().saveProperty(dbSession, new PropertyDto().setUserId(user.getId()).setKey("foo").setValue("bar")); dbSession.commit(); userIndexer.index(); diff --git a/sonar-db/src/main/java/org/sonar/db/permission/AuthorizationDao.java b/sonar-db/src/main/java/org/sonar/db/permission/AuthorizationDao.java index 84462aba5ca..d64da6db3d9 100644 --- a/sonar-db/src/main/java/org/sonar/db/permission/AuthorizationDao.java +++ b/sonar-db/src/main/java/org/sonar/db/permission/AuthorizationDao.java @@ -118,6 +118,17 @@ public class AuthorizationDao implements Dao { return mapper(dbSession).countUsersWithGlobalPermissionExcludingUserPermission(organizationUuid, permission, userId); } + /** + * The UUIDs of all the organizations in which the specified user has the specified global permission. An empty + * set is returned if user or permission do not exist. An empty set is also returned if the user is not involved + * in any organization. + *
+ * Group membership is taken into account. Anonymous privileges are ignored. + */ + public Set selectOrganizationUuidsOfUserWithGlobalPermission(DbSession dbSession, long userId, String permission) { + return mapper(dbSession).selectOrganizationUuidsOfUserWithGlobalPermission(userId, permission); + } + public Set keepAuthorizedProjectIds(DbSession dbSession, Collection componentIds, @Nullable Integer userId, String role) { return executeLargeInputsIntoSet( componentIds, diff --git a/sonar-db/src/main/java/org/sonar/db/permission/AuthorizationMapper.java b/sonar-db/src/main/java/org/sonar/db/permission/AuthorizationMapper.java index 975bd65c7f7..6a8091f1bd0 100644 --- a/sonar-db/src/main/java/org/sonar/db/permission/AuthorizationMapper.java +++ b/sonar-db/src/main/java/org/sonar/db/permission/AuthorizationMapper.java @@ -49,6 +49,8 @@ public interface AuthorizationMapper { int countUsersWithGlobalPermissionExcludingUserPermission(@Param("organizationUuid") String organizationUuid, @Param("permission") String permission, @Param("userId") long userId); + Set selectOrganizationUuidsOfUserWithGlobalPermission(@Param("userId") long userId, @Param("permission") String permission); + Set keepAuthorizedProjectIdsForAnonymous(@Param("role") String role, @Param("componentIds") Collection componentIds); Set keepAuthorizedProjectIdsForUser(@Param("userId") long userId, @Param("role") String role, @Param("componentIds") Collection componentIds); diff --git a/sonar-db/src/main/resources/org/sonar/db/permission/AuthorizationMapper.xml b/sonar-db/src/main/resources/org/sonar/db/permission/AuthorizationMapper.xml index d5e441bcbce..bf9fdcb54b8 100644 --- a/sonar-db/src/main/resources/org/sonar/db/permission/AuthorizationMapper.xml +++ b/sonar-db/src/main/resources/org/sonar/db/permission/AuthorizationMapper.xml @@ -169,6 +169,26 @@ ) remaining + +