]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-8956 Prevent removing last org admin member
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Mon, 20 Mar 2017 08:47:27 +0000 (09:47 +0100)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Tue, 21 Mar 2017 12:05:50 +0000 (13:05 +0100)
server/sonar-server/src/main/java/org/sonar/server/organization/ws/RemoveMemberAction.java
server/sonar-server/src/main/java/org/sonar/server/user/ws/DeactivateAction.java
server/sonar-server/src/test/java/org/sonar/server/organization/ws/RemoveMemberActionTest.java
server/sonar-server/src/test/java/org/sonar/server/user/ws/DeactivateActionTest.java

index fa94f6ea922bb99cc837bb87fdde95d93b0ca159..1c31b25fd6066b25191c6e1409516ab6e9090155 100644 (file)
@@ -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");
+  }
+
 }
index 088ce719c233506574c1dfe055dfdb298d3dddd9..6f05d7a64070802aba380042592a44333b1a8aff 100644 (file)
@@ -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<String> selectOrganizationsWithNoMoreAdministrators(DbSession dbSession, UserDto user) {
index 9ed91f312b2d5b3c06ca7e8588fca9d6112b6ab2..b80f5b15f07731c201b8de15938c261b3c8146a2 100644 (file)
@@ -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));
index 2dee740c0b24b550bcd006f34bf46300b4764be4..c5fe0df7568efc1265ee2757ab0a3bb8b98c8b68 100644 (file)
@@ -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());
   }