]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-6912 do not deactivate last organization administrator
authorSimon Brandhof <simon.brandhof@sonarsource.com>
Tue, 15 Nov 2016 19:40:03 +0000 (20:40 +0100)
committerSimon Brandhof <simon.brandhof@sonarsource.com>
Wed, 16 Nov 2016 12:59:29 +0000 (13:59 +0100)
server/sonar-server/src/main/java/org/sonar/server/user/ws/DeactivateAction.java
server/sonar-server/src/test/java/org/sonar/server/user/ws/DeactivateActionTest.java
sonar-db/src/main/java/org/sonar/db/permission/AuthorizationDao.java
sonar-db/src/main/java/org/sonar/db/permission/AuthorizationMapper.java
sonar-db/src/main/resources/org/sonar/db/permission/AuthorizationMapper.xml
sonar-db/src/test/java/org/sonar/db/permission/AuthorizationDaoTest.java

index 5185d935f5b2ef178becb7fe3d4c51ba9cb2249a..121da89409ef5a82e6c0e9fa96110eb65dd0011b 100644 (file)
 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<String> 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<String> 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<String> selectOrganizationsWithNoMoreAdministrators(DbSession dbSession, UserDto user) {
+    Set<String> organizationUuids = dbClient.authorizationDao().selectOrganizationUuidsOfUserWithGlobalPermission(
+      dbSession, user.getId(), SYSTEM_ADMIN);
+    List<String> 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()));
+  }
 }
index 8bf8163eb1b7b3a9ebb998fe7fb97f7a43f9cbf6..7cd0b79fd36b35fec8383119452a8112bb97cce2 100644 (file)
@@ -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();
index 84462aba5ca4b1b2b8191cbfdc009c37839e251b..d64da6db3d9e6b71754674bc706d07f5f849a11e 100644 (file)
@@ -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.
+   * <br/>
+   * Group membership is taken into account. Anonymous privileges are ignored.
+   */
+  public Set<String> selectOrganizationUuidsOfUserWithGlobalPermission(DbSession dbSession, long userId, String permission) {
+    return mapper(dbSession).selectOrganizationUuidsOfUserWithGlobalPermission(userId, permission);
+  }
+
   public Set<Long> keepAuthorizedProjectIds(DbSession dbSession, Collection<Long> componentIds, @Nullable Integer userId, String role) {
     return executeLargeInputsIntoSet(
       componentIds,
index 975bd65c7f7f425ed8c33a5a90b4b45537075485..6a8091f1bd07a9efca889728dbdf2399d25dc602 100644 (file)
@@ -49,6 +49,8 @@ public interface AuthorizationMapper {
   int countUsersWithGlobalPermissionExcludingUserPermission(@Param("organizationUuid") String organizationUuid,
                                                             @Param("permission") String permission, @Param("userId") long userId);
 
+  Set<String> selectOrganizationUuidsOfUserWithGlobalPermission(@Param("userId") long userId, @Param("permission") String permission);
+
   Set<Long> keepAuthorizedProjectIdsForAnonymous(@Param("role") String role, @Param("componentIds") Collection<Long> componentIds);
   
   Set<Long> keepAuthorizedProjectIdsForUser(@Param("userId") long userId, @Param("role") String role, @Param("componentIds") Collection<Long> componentIds);
index d5e441bcbce64e8f135d528bf6c2345ca29a68c1..bf9fdcb54b8c471c8697d58d5855d9efd741297a 100644 (file)
     ) remaining
   </select>
 
+  <select id="selectOrganizationUuidsOfUserWithGlobalPermission" parameterType="map" resultType="String">
+    select gr.organization_uuid
+    from group_roles gr
+    inner join groups_users gu on gr.group_id = gu.group_id
+    where
+    gr.role = #{permission,jdbcType=VARCHAR} and
+    gr.resource_id is null and
+    gr.group_id is not null and
+    gu.user_id = #{userId,jdbcType=BIGINT}
+
+    union
+
+    select ur.organization_uuid
+    from user_roles ur
+    where
+    ur.resource_id is null and
+    ur.role = #{permission,jdbcType=VARCHAR} and
+    ur.user_id = #{userId,jdbcType=BIGINT}
+  </select>
+
   <select id="keepAuthorizedProjectIdsForUser" parameterType="map" resultType="long">
     SELECT gr.resource_id
     FROM group_roles gr
index 42adf47bd9ad33155a7a918d6a7755cfd7fcefcd..0bc7faa939a9fdc93d047120c83cbf2795391a83 100644 (file)
@@ -26,6 +26,7 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.sonar.api.utils.System2;
+import org.sonar.api.web.UserRole;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
 import org.sonar.db.component.ComponentDto;
@@ -35,6 +36,9 @@ import org.sonar.db.user.UserDto;
 
 import static com.google.common.collect.Sets.newHashSet;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.sonar.core.permission.GlobalPermissions.QUALITY_GATE_ADMIN;
+import static org.sonar.core.permission.GlobalPermissions.SCAN_EXECUTION;
+import static org.sonar.core.permission.GlobalPermissions.SYSTEM_ADMIN;
 
 public class AuthorizationDaoTest {
 
@@ -591,4 +595,69 @@ public class AuthorizationDaoTest {
     count = underTest.countUsersWithGlobalPermissionExcludingUserPermission(dbSession, org.getUuid(), DOES_NOT_EXIST,  u2.getId());
     assertThat(count).isEqualTo(0);
   }
+
+  @Test
+  public void selectOrganizationUuidsOfUserWithGlobalPermission_returns_empty_set_if_user_does_not_exist() {
+    // another user
+    db.users().insertPermissionOnUser(user, QUALITY_GATE_ADMIN);
+
+    Set<String> orgUuids = underTest.selectOrganizationUuidsOfUserWithGlobalPermission(dbSession, MISSING_ID, SYSTEM_ADMIN);
+
+    assertThat(orgUuids).isEmpty();
+  }
+
+  @Test
+  public void selectOrganizationUuidsOfUserWithGlobalPermission_returns_empty_set_if_user_does_not_have_permission_at_all() {
+    db.users().insertPermissionOnUser(user, QUALITY_GATE_ADMIN);
+    // user is not part of this group
+    db.users().insertPermissionOnGroup(group1, SCAN_EXECUTION);
+
+    Set<String> orgUuids = underTest.selectOrganizationUuidsOfUserWithGlobalPermission(dbSession, user.getId(), SCAN_EXECUTION);
+
+    assertThat(orgUuids).isEmpty();
+  }
+
+  @Test
+  public void selectOrganizationUuidsOfUserWithGlobalPermission_returns_organizations_on_which_user_has_permission() {
+    db.users().insertPermissionOnGroup(group1, SCAN_EXECUTION);
+    db.users().insertPermissionOnGroup(group2, QUALITY_GATE_ADMIN);
+    db.users().insertMember(group1, user);
+    db.users().insertMember(group2, user);
+
+    Set<String> orgUuids = underTest.selectOrganizationUuidsOfUserWithGlobalPermission(dbSession, user.getId(), SCAN_EXECUTION);
+
+    assertThat(orgUuids).containsExactly(group1.getOrganizationUuid());
+  }
+
+  @Test
+  public void selectOrganizationUuidsOfUserWithGlobalPermission_handles_user_permissions_and_group_permissions() {
+    // org: through group membership
+    db.users().insertPermissionOnGroup(group1, SCAN_EXECUTION);
+    db.users().insertMember(group1, user);
+
+    // org2 : direct user permission
+    OrganizationDto org2 = db.organizations().insert();
+    db.users().insertPermissionOnUser(org2, user, SCAN_EXECUTION);
+
+    // org3 : another permission QUALITY_GATE_ADMIN
+    OrganizationDto org3 = db.organizations().insert();
+    db.users().insertPermissionOnUser(org3, user, QUALITY_GATE_ADMIN);
+
+    // exclude project permission
+    db.users().insertProjectPermissionOnUser(user, UserRole.ADMIN, db.components().insertProject());
+
+    Set<String> orgUuids = underTest.selectOrganizationUuidsOfUserWithGlobalPermission(dbSession, user.getId(), SCAN_EXECUTION);
+
+    assertThat(orgUuids).containsOnly(org.getUuid(), org2.getUuid());
+  }
+
+  @Test
+  public void selectOrganizationUuidsOfUserWithGlobalPermission_ignores_anonymous_permissions() {
+    db.users().insertPermissionOnAnyone(org, SCAN_EXECUTION);
+    db.users().insertPermissionOnUser(org, user, QUALITY_GATE_ADMIN);
+
+    Set<String> orgUuids = underTest.selectOrganizationUuidsOfUserWithGlobalPermission(dbSession, user.getId(), SCAN_EXECUTION);
+
+    assertThat(orgUuids).isEmpty();
+  }
 }