]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-18266 - Prevent project admin from removing own browse right
authorAntoine Vinot <antoine.vinot@sonarsource.com>
Fri, 2 Jun 2023 10:49:31 +0000 (12:49 +0200)
committersonartech <sonartech@sonarsource.com>
Mon, 5 Jun 2023 20:02:47 +0000 (20:02 +0000)
server/sonar-db-dao/src/it/java/org/sonar/db/permission/GroupPermissionDaoIT.java
server/sonar-db-dao/src/main/java/org/sonar/db/permission/GroupPermissionDao.java
server/sonar-db-dao/src/main/java/org/sonar/db/permission/GroupPermissionMapper.java
server/sonar-db-dao/src/main/resources/org/sonar/db/permission/GroupPermissionMapper.xml
server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/RemoveGroupActionIT.java
server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/RemoveUserActionIT.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/PermissionWsSupport.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/RemoveGroupAction.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/permission/ws/RemoveUserAction.java

index 9c2aa8cc6f967d7b8e5514ab908c902c3a9acf86..acbf2284ec3bd902ed052094567b6f9455b86269 100644 (file)
@@ -24,6 +24,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Random;
+import java.util.Set;
 import java.util.stream.IntStream;
 import java.util.stream.Stream;
 import org.junit.Rule;
@@ -40,6 +41,7 @@ import org.sonar.db.user.GroupDto;
 import static java.util.Arrays.asList;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
 import static org.assertj.core.api.Assertions.tuple;
 import static org.sonar.api.security.DefaultGroups.ANYONE;
 import static org.sonar.db.permission.PermissionQuery.DEFAULT_PAGE_SIZE;
@@ -130,7 +132,7 @@ public class GroupPermissionDaoIT {
       db.users().insertProjectPermissionOnGroup(group, GlobalPermission.SCAN.getKey(), project);
     });
     String lastGroupName = "Group-" + (DEFAULT_PAGE_SIZE + 1);
-    db.users().insertPermissionOnGroup(db.users().selectGroup(lastGroupName).get(), GlobalPermission.SCAN);
+    db.users().insertPermissionOnGroup(db.users().selectGroup(lastGroupName).orElseGet(() -> fail("group not found")), GlobalPermission.SCAN);
 
     assertThat(underTest.selectGroupNamesByQuery(dbSession, newQuery().build()))
       .hasSize(DEFAULT_PAGE_SIZE)
@@ -146,7 +148,7 @@ public class GroupPermissionDaoIT {
     });
     ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
     String lastGroupName = "Group-" + (DEFAULT_PAGE_SIZE + 1);
-    db.users().insertProjectPermissionOnGroup(db.users().selectGroup(lastGroupName).get(), GlobalPermission.SCAN.getKey(), project);
+    db.users().insertProjectPermissionOnGroup(db.users().selectGroup(lastGroupName).orElseGet(() -> fail("group not found")), GlobalPermission.SCAN.getKey(), project);
 
     assertThat(underTest.selectGroupNamesByQuery(dbSession, newQuery()
       .setComponent(project)
@@ -298,24 +300,24 @@ public class GroupPermissionDaoIT {
     db.users().insertPermissionOnAnyone(GlobalPermission.SCAN);
     db.users().insertPermissionOnAnyone(GlobalPermission.PROVISION_PROJECTS);
 
-    assertThat(underTest.selectByGroupUuids(dbSession, asList(group1.getUuid()), null))
+    assertThat(underTest.selectByGroupUuids(dbSession, List.of(group1.getUuid()), null))
       .extracting(GroupPermissionDto::getGroupUuid, GroupPermissionDto::getRole, GroupPermissionDto::getComponentUuid)
       .containsOnly(tuple(group1.getUuid(), GlobalPermission.SCAN.getKey(), null));
 
-    assertThat(underTest.selectByGroupUuids(dbSession, asList(group2.getUuid()), null)).isEmpty();
+    assertThat(underTest.selectByGroupUuids(dbSession, List.of(group2.getUuid()), null)).isEmpty();
 
-    assertThat(underTest.selectByGroupUuids(dbSession, asList(group3.getUuid()), null))
+    assertThat(underTest.selectByGroupUuids(dbSession, List.of(group3.getUuid()), null))
       .extracting(GroupPermissionDto::getGroupUuid, GroupPermissionDto::getRole, GroupPermissionDto::getComponentUuid)
       .containsOnly(tuple(group3.getUuid(), GlobalPermission.ADMINISTER.getKey(), null));
 
-    assertThat(underTest.selectByGroupUuids(dbSession, asList(ANYONE_UUID), null))
+    assertThat(underTest.selectByGroupUuids(dbSession, List.of(ANYONE_UUID), null))
       .extracting(GroupPermissionDto::getGroupUuid, GroupPermissionDto::getRole, GroupPermissionDto::getComponentUuid)
       .containsOnly(
         tuple(ANYONE_UUID, GlobalPermission.SCAN.getKey(), null),
         tuple(ANYONE_UUID, GlobalPermission.PROVISION_PROJECTS.getKey(), null));
 
-    assertThat(underTest.selectByGroupUuids(dbSession, asList(group1.getUuid(), group2.getUuid(), ANYONE_UUID), null)).hasSize(3);
-    assertThat(underTest.selectByGroupUuids(dbSession, asList(MISSING_UUID), null)).isEmpty();
+    assertThat(underTest.selectByGroupUuids(dbSession, List.of(group1.getUuid(), group2.getUuid(), ANYONE_UUID), null)).hasSize(3);
+    assertThat(underTest.selectByGroupUuids(dbSession, List.of(MISSING_UUID), null)).isEmpty();
     assertThat(underTest.selectByGroupUuids(dbSession, Collections.emptyList(), null)).isEmpty();
   }
 
@@ -594,7 +596,7 @@ public class GroupPermissionDaoIT {
     ComponentDto project = randomPublicOrPrivateProject();
     GroupDto group1 = db.users().insertGroup();
     GroupDto group2 = db.users().insertGroup();
-    GroupDto group3 = db.users().insertGroup();
+    db.users().insertGroup();
     db.users().insertProjectPermissionOnGroup(group1, "p1", project);
     db.users().insertProjectPermissionOnGroup(group2, "p2", project);
 
@@ -966,6 +968,23 @@ public class GroupPermissionDaoIT {
     assertThat(underTest.deleteByRootComponentUuidAndPermission(dbSession, "p1", project)).isZero();
   }
 
+  @Test
+  public void selectGroupUuidsWithPermissionOnProject_shouldReturnOnlyGroupsWithSpecifiedPermission() {
+    GroupDto group1 = db.users().insertGroup();
+    GroupDto group2 = db.users().insertGroup();
+    GroupDto group3 = db.users().insertGroup();
+    ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
+    ComponentDto otherProject = db.components().insertPrivateProject().getMainBranchComponent();
+    String anyPermission = "any_permission";
+    db.users().insertProjectPermissionOnGroup(group1, anyPermission, project);
+    db.users().insertProjectPermissionOnGroup(group2, "otherPermission", project);
+    db.users().insertProjectPermissionOnGroup(group3, anyPermission, otherProject);
+
+    Set<String> results = underTest.selectGroupUuidsWithPermissionOnProject(dbSession, project.uuid(), anyPermission);
+
+    assertThat(results).containsOnly(group1.getUuid());
+  }
+
   private Collection<String> getGlobalPermissionsForAnyone() {
     return getPermissions("group_uuid is null and component_uuid is null");
   }
index 469fff519b9628816fd72285bfd6d6e640905bcb..8d01bcaf79d4f99ec5576d6433ac40eebe0cc9a1 100644 (file)
@@ -129,6 +129,13 @@ public class GroupPermissionDao implements Dao {
     return mapper(session).selectGroupUuidsWithPermissionOnProjectBut(projectUuid, permission);
   }
 
+  /**
+   * Lists group uuids that have the selected permission on the specified project.
+   */
+  public Set<String> selectGroupUuidsWithPermissionOnProject(DbSession session, String projectUuid, String permission) {
+    return mapper(session).selectGroupUuidsWithPermissionOnProject(projectUuid, permission);
+  }
+
   public void insert(DbSession dbSession, GroupPermissionDto groupPermissionDto, @Nullable ComponentDto componentDto, @Nullable PermissionTemplateDto permissionTemplateDto) {
     mapper(dbSession).insert(groupPermissionDto);
 
index d4771c3efd183980981d5efc6af84123a69e16cb..89e575baff8ee27ac777a7d706fba448c2b4357c 100644 (file)
@@ -58,6 +58,8 @@ public interface GroupPermissionMapper {
    */
   Set<String> selectGroupUuidsWithPermissionOnProjectBut(@Param("projectUuid") String projectUuid, @Param("role") String permission);
 
+  Set<String> selectGroupUuidsWithPermissionOnProject(@Param("projectUuid") String projectUuid, @Param("role") String permission);
+
   int deleteByRootComponentUuid(@Param("rootComponentUuid") String rootComponentUuid);
 
   int deleteByRootComponentUuidAndGroupUuid(@Param("rootComponentUuid") String rootComponentUuid, @Nullable @Param("groupUuid") String groupUuid);
index d062673b736435d780292e6f5bb7fc2a1a4b5224..dbaeb9425869a120a39ae5d1f54adcc1bd97dd4a 100644 (file)
       )
   </select>
 
+   <select id="selectGroupUuidsWithPermissionOnProject" resultType="string">
+    select
+      distinct gr1.group_uuid
+    from
+      group_roles gr1
+    where
+      gr1.component_uuid = #{projectUuid,jdbcType=VARCHAR}
+      and gr1.group_uuid is not null
+      and gr1.role = #{role,jdbcType=VARCHAR}
+  </select>
+
   <insert id="insert" parameterType="GroupPermission">
     insert into group_roles (
     uuid,
index 5b9fcfd9d9d7f8f484c7e4d10bb4a907b6b21c37..201a5cee1e2f2953e01991f6ab0fda22b6505922 100644 (file)
  */
 package org.sonar.server.permission.ws;
 
+import com.tngtech.java.junit.dataprovider.DataProviderRunner;
 import org.junit.Before;
 import org.junit.Test;
+import org.junit.runner.RunWith;
 import org.sonar.api.resources.Qualifiers;
 import org.sonar.api.resources.ResourceTypes;
 import org.sonar.api.server.ws.Change;
@@ -32,11 +34,13 @@ import org.sonar.db.component.ResourceTypesRule;
 import org.sonar.db.permission.GlobalPermission;
 import org.sonar.db.permission.GroupPermissionDto;
 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.permission.PermissionService;
 import org.sonar.server.permission.PermissionServiceImpl;
+import org.sonar.server.ws.TestRequest;
 
 import static java.lang.String.format;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -50,12 +54,13 @@ import static org.sonarqube.ws.client.permission.PermissionsWsParameters.PARAM_P
 import static org.sonarqube.ws.client.permission.PermissionsWsParameters.PARAM_PROJECT_ID;
 import static org.sonarqube.ws.client.permission.PermissionsWsParameters.PARAM_PROJECT_KEY;
 
+@RunWith(DataProviderRunner.class)
 public class RemoveGroupActionIT extends BasePermissionWsIT<RemoveGroupAction> {
 
   private GroupDto aGroup;
-  private ResourceTypes resourceTypes = new ResourceTypesRule().setRootQualifiers(Qualifiers.PROJECT);
-  private PermissionService permissionService = new PermissionServiceImpl(resourceTypes);
-  private WsParameters wsParameters = new WsParameters(permissionService);
+  private final ResourceTypes resourceTypes = new ResourceTypesRule().setRootQualifiers(Qualifiers.PROJECT);
+  private final PermissionService permissionService = new PermissionServiceImpl(resourceTypes);
+  private final WsParameters wsParameters = new WsParameters(permissionService);
 
   @Before
   public void setUp() {
@@ -68,7 +73,7 @@ public class RemoveGroupActionIT extends BasePermissionWsIT<RemoveGroupAction> {
   }
 
   @Test
-  public void verify_definition() {
+  public void wsAction_shouldHaveDefinition() {
     Action wsDef = wsTester.getDef();
 
     assertThat(wsDef.isInternal()).isFalse();
@@ -80,7 +85,7 @@ public class RemoveGroupActionIT extends BasePermissionWsIT<RemoveGroupAction> {
   }
 
   @Test
-  public void remove_permission_using_group_name() {
+  public void wsAction_shouldRemoveGlobalPermission() {
     db.users().insertPermissionOnGroup(aGroup, GlobalPermission.ADMINISTER);
     db.users().insertPermissionOnGroup(aGroup, GlobalPermission.PROVISION_PROJECTS);
     loginAsAdmin();
@@ -94,7 +99,7 @@ public class RemoveGroupActionIT extends BasePermissionWsIT<RemoveGroupAction> {
   }
 
   @Test
-  public void remove_project_permission() {
+  public void wsAction_shouldRemoveProjectPermission() {
     ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
     db.users().insertPermissionOnGroup(aGroup, GlobalPermission.ADMINISTER);
     db.users().insertProjectPermissionOnGroup(aGroup, UserRole.ADMIN, project);
@@ -112,7 +117,7 @@ public class RemoveGroupActionIT extends BasePermissionWsIT<RemoveGroupAction> {
   }
 
   @Test
-  public void remove_with_view_uuid() {
+  public void wsAction_whenUsingViewUuid_shouldRemovePermission() {
     ComponentDto view = db.components().insertPrivatePortfolio();
     db.users().insertPermissionOnGroup(aGroup, GlobalPermission.ADMINISTER);
     db.users().insertProjectPermissionOnGroup(aGroup, UserRole.ADMIN, view);
@@ -130,7 +135,7 @@ public class RemoveGroupActionIT extends BasePermissionWsIT<RemoveGroupAction> {
   }
 
   @Test
-  public void remove_with_project_key() {
+  public void wsAction_whenUsingProjectKey_shouldRemovePermission() {
     ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
     db.users().insertPermissionOnGroup(aGroup, GlobalPermission.ADMINISTER);
     db.users().insertProjectPermissionOnGroup(aGroup, UserRole.ADMIN, project);
@@ -148,36 +153,33 @@ public class RemoveGroupActionIT extends BasePermissionWsIT<RemoveGroupAction> {
   }
 
   @Test
-  public void fail_to_remove_last_admin_permission() {
+  public void wsAction_whenLastAdminPermission_shouldFail() {
     db.users().insertPermissionOnGroup(aGroup, GlobalPermission.ADMINISTER);
     db.users().insertPermissionOnGroup(aGroup, GlobalPermission.PROVISION_PROJECTS);
     loginAsAdmin();
 
     String administerPermission = GlobalPermission.ADMINISTER.getKey();
-    assertThatThrownBy(() -> {
-      executeRequest(aGroup, administerPermission);
-    })
+    assertThatThrownBy(() -> executeRequest(aGroup, administerPermission))
       .isInstanceOf(BadRequestException.class)
       .hasMessage("Last group with permission 'admin'. Permission cannot be removed.");
   }
 
   @Test
-  public void fail_when_project_does_not_exist() {
+  public void wsAction_whenProjectNotFound_shouldFail() {
     loginAsAdmin();
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_GROUP_NAME, aGroup.getName())
-        .setParam(PARAM_PROJECT_ID, "unknown-project-uuid")
-        .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey())
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_GROUP_NAME, aGroup.getName())
+      .setParam(PARAM_PROJECT_ID, "unknown-project-uuid")
+      .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey());
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(NotFoundException.class)
       .hasMessage("Project id 'unknown-project-uuid' not found");
   }
 
   @Test
-  public void fail_when_project_project_permission_without_project() {
+  public void wsAction_whenUsingProjectPermissionWithoutProject_shouldFail() {
     loginAsAdmin();
 
     assertThatThrownBy(() -> executeRequest(aGroup, UserRole.ISSUE_ADMIN))
@@ -186,7 +188,7 @@ public class RemoveGroupActionIT extends BasePermissionWsIT<RemoveGroupAction> {
   }
 
   @Test
-  public void fail_when_component_is_a_directory() {
+  public void wsAction_whenComponentIsDirectory_shouldFail() {
     ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
     ComponentDto file = db.components().insertComponent(newDirectory(project, "A/B"));
 
@@ -194,7 +196,7 @@ public class RemoveGroupActionIT extends BasePermissionWsIT<RemoveGroupAction> {
   }
 
   @Test
-  public void fail_when_component_is_a_file() {
+  public void wsAction_whenComponentIsFile_shouldFail() {
     ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
     ComponentDto file = db.components().insertComponent(newFileDto(project, null, "file-uuid"));
 
@@ -202,7 +204,7 @@ public class RemoveGroupActionIT extends BasePermissionWsIT<RemoveGroupAction> {
   }
 
   @Test
-  public void fail_when_component_is_a_subview() {
+  public void wsAction_whenComponentIsSubview_shouldFail() {
     ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
     ComponentDto file = db.components().insertComponent(newSubPortfolio(project));
 
@@ -212,56 +214,50 @@ public class RemoveGroupActionIT extends BasePermissionWsIT<RemoveGroupAction> {
   private void failIfComponentIsNotAProjectOrView(ComponentDto file) {
     loginAsAdmin();
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_GROUP_NAME, aGroup.getName())
-        .setParam(PARAM_PROJECT_ID, file.uuid())
-        .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey())
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_GROUP_NAME, aGroup.getName())
+      .setParam(PARAM_PROJECT_ID, file.uuid())
+      .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey());
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(BadRequestException.class)
       .hasMessage("Component '" + file.getKey() + "' (id: " + file.uuid() + ") must be a project or a view.");
   }
 
   @Test
-  public void fail_when_group_name_is_missing() {
+  public void wsAction_whenGroupNameIsMissing_shouldFail() {
     loginAsAdmin();
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey())
-        .execute();
-    })
+    TestRequest testRequest = newRequest().setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey());
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(IllegalArgumentException.class)
       .hasMessage("The 'groupName' parameter is missing");
   }
 
   @Test
-  public void fail_when_permission_name_and_id_are_missing() {
+  public void wsAction_whenPermissionNameAndIdMissing_shouldFail() {
     loginAsAdmin();
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_GROUP_NAME, aGroup.getName())
-        .execute();
-    })
+    TestRequest testRequest = newRequest().setParam(PARAM_GROUP_NAME, aGroup.getName());
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(IllegalArgumentException.class)
       .hasMessage("The 'permission' parameter is missing");
   }
 
   @Test
-  public void fail_when_project_uuid_and_project_key_are_provided() {
+  public void wsAction_whenProjectUuidAndProjectKeyAreProvided_shouldFail() {
     ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
     loginAsAdmin();
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_GROUP_NAME, aGroup.getName())
-        .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey())
-        .setParam(PARAM_PROJECT_ID, project.uuid())
-        .setParam(PARAM_PROJECT_KEY, project.getKey())
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_GROUP_NAME, aGroup.getName())
+      .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey())
+      .setParam(PARAM_PROJECT_ID, project.uuid())
+      .setParam(PARAM_PROJECT_KEY, project.getKey());
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(BadRequestException.class)
       .hasMessage("Project id or project key can be provided, not both.");
   }
@@ -274,38 +270,33 @@ public class RemoveGroupActionIT extends BasePermissionWsIT<RemoveGroupAction> {
   }
 
   @Test
-  public void removing_global_permission_fails_if_not_system_administrator() {
+  public void wsAction_whenRemovingGlobalPermissionAndNotSystemAdmin_shouldFail() {
     userSession.logIn();
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_GROUP_NAME, aGroup.getName())
-        .setParam(PARAM_PERMISSION, GlobalPermission.PROVISION_PROJECTS.getKey())
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_GROUP_NAME, aGroup.getName())
+      .setParam(PARAM_PERMISSION, GlobalPermission.PROVISION_PROJECTS.getKey());
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(ForbiddenException.class);
   }
 
   @Test
-  public void removing_project_permission_fails_if_not_administrator_of_project() {
+  public void wsAction_whenRemovingProjectPermissionAndNotProjectAdmin_shouldFail() {
     ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
     userSession.logIn();
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_GROUP_NAME, aGroup.getName())
-        .setParam(PARAM_PERMISSION, GlobalPermission.PROVISION_PROJECTS.getKey())
-        .setParam(PARAM_PROJECT_KEY, project.getKey())
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_GROUP_NAME, aGroup.getName())
+      .setParam(PARAM_PERMISSION, GlobalPermission.PROVISION_PROJECTS.getKey())
+      .setParam(PARAM_PROJECT_KEY, project.getKey());
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(ForbiddenException.class);
   }
 
-  /**
-   * User is project administrator but not system administrator
-   */
   @Test
-  public void removing_project_permission_is_allowed_to_project_administrators() {
+  public void wsAction_whenRemovingProjectPermissionAsProjectAdminButNotSystemAdmin_shouldRemovePermission() {
     ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
     db.users().insertProjectPermissionOnGroup(aGroup, UserRole.CODEVIEWER, project);
     db.users().insertProjectPermissionOnGroup(aGroup, UserRole.ISSUE_ADMIN, project);
@@ -321,7 +312,7 @@ public class RemoveGroupActionIT extends BasePermissionWsIT<RemoveGroupAction> {
   }
 
   @Test
-  public void no_effect_when_removing_any_permission_from_group_AnyOne_on_a_private_project() {
+  public void wsAction_whenRemovingAnyPermissionFromGroupAnyoneOnPrivateProject_shouldHaveNoEffect() {
     ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
     permissionService.getAllProjectPermissions()
       .forEach(perm -> unsafeInsertProjectPermissionOnAnyone(perm, project));
@@ -340,89 +331,126 @@ public class RemoveGroupActionIT extends BasePermissionWsIT<RemoveGroupAction> {
   }
 
   @Test
-  public void fail_when_removing_USER_permission_from_group_AnyOne_on_a_public_project() {
+  public void wsAction_whenRemovingBrowsePermissionFromGroupAnyoneOnPublicProject_shouldFail() {
     ComponentDto project = db.components().insertPublicProject().getMainBranchComponent();
     userSession.logIn().addProjectPermission(UserRole.ADMIN, project);
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_GROUP_NAME, "anyone")
-        .setParam(PARAM_PROJECT_ID, project.uuid())
-        .setParam(PARAM_PERMISSION, UserRole.USER)
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_GROUP_NAME, "anyone")
+      .setParam(PARAM_PROJECT_ID, project.uuid())
+      .setParam(PARAM_PERMISSION, UserRole.USER);
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(BadRequestException.class)
       .hasMessage("Permission user can't be removed from a public component");
   }
 
   @Test
-  public void fail_when_removing_CODEVIEWER_permission_from_group_AnyOne_on_a_public_project() {
+  public void wsAction_whenRemovingCodeviewerPermissionFromGroupAnyoneOnPublicProject_shouldFail() {
     ComponentDto project = db.components().insertPublicProject().getMainBranchComponent();
     userSession.logIn().addProjectPermission(UserRole.ADMIN, project);
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_GROUP_NAME, "anyone")
-        .setParam(PARAM_PROJECT_ID, project.uuid())
-        .setParam(PARAM_PERMISSION, UserRole.CODEVIEWER)
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_GROUP_NAME, "anyone")
+      .setParam(PARAM_PROJECT_ID, project.uuid())
+      .setParam(PARAM_PERMISSION, UserRole.CODEVIEWER);
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(BadRequestException.class)
       .hasMessage("Permission codeviewer can't be removed from a public component");
   }
 
   @Test
-  public void fail_when_removing_USER_permission_from_group_on_a_public_project() {
+  public void wsAction_whenRemovingBrowsePermissionFromGroupOnPublicProject_shouldFail() {
     GroupDto group = db.users().insertGroup();
     ComponentDto project = db.components().insertPublicProject().getMainBranchComponent();
     userSession.logIn().addProjectPermission(UserRole.ADMIN, project);
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_GROUP_NAME, group.getName())
-        .setParam(PARAM_PROJECT_ID, project.uuid())
-        .setParam(PARAM_PERMISSION, UserRole.USER)
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_GROUP_NAME, group.getName())
+      .setParam(PARAM_PROJECT_ID, project.uuid())
+      .setParam(PARAM_PERMISSION, UserRole.USER);
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(BadRequestException.class)
       .hasMessage("Permission user can't be removed from a public component");
   }
 
   @Test
-  public void fail_when_removing_CODEVIEWER_permission_from_group_on_a_public_project() {
+  public void wsAction_whenRemovingCodeviewerPermissionFromGroupOnPublicProject() {
     GroupDto group = db.users().insertGroup();
     ComponentDto project = db.components().insertPublicProject().getMainBranchComponent();
     userSession.logIn().addProjectPermission(UserRole.ADMIN, project);
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_GROUP_NAME, group.getName())
-        .setParam(PARAM_PROJECT_ID, project.uuid())
-        .setParam(PARAM_PERMISSION, UserRole.CODEVIEWER)
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_GROUP_NAME, group.getName())
+      .setParam(PARAM_PROJECT_ID, project.uuid())
+      .setParam(PARAM_PERMISSION, UserRole.CODEVIEWER);
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(BadRequestException.class)
       .hasMessage("Permission codeviewer can't be removed from a public component");
   }
 
   @Test
-  public void fail_when_using_branch_uuid() {
+  public void wsAction_whenUsingBranchUuid_shouldFail() {
     GroupDto group = db.users().insertGroup();
     ComponentDto project = db.components().insertPublicProject().getMainBranchComponent();
     userSession.logIn().addProjectPermission(UserRole.ADMIN, project);
     ComponentDto branch = db.components().insertProjectBranch(project);
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_PROJECT_ID, branch.uuid())
-        .setParam(PARAM_GROUP_NAME, group.getName())
-        .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey())
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_PROJECT_ID, branch.uuid())
+      .setParam(PARAM_GROUP_NAME, group.getName())
+      .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey());
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(NotFoundException.class)
       .hasMessage(format("Project id '%s' not found", branch.uuid()));
   }
 
+  @Test
+  public void wsAction_whenRemovingLastOwnBrowsePermissionForPrivateProject_shouldFail() {
+    ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
+    UserDto user = db.users().insertUser();
+    GroupDto projectAdminGroup = db.users().insertGroup();
+    db.users().insertProjectPermissionOnGroup(projectAdminGroup, UserRole.USER, project);
+    db.users().insertProjectPermissionOnGroup(projectAdminGroup, UserRole.ADMIN, project);
+
+    userSession.logIn(user).setGroups(projectAdminGroup).addProjectPermission(UserRole.USER, project).addProjectPermission(UserRole.ADMIN, project);
+
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_PROJECT_ID, project.uuid())
+      .setParam(PARAM_GROUP_NAME, projectAdminGroup.getName())
+      .setParam(PARAM_PERMISSION, UserRole.USER);
+
+    assertThatThrownBy(testRequest::execute)
+      .isInstanceOf(BadRequestException.class)
+      .hasMessage("Permission 'Browse' cannot be removed from a private project for a project administrator.");
+  }
+
+  @Test
+  public void wsAction_whenRemovingOwnBrowsePermissionAndHavePermissionFromOtherGroup_shouldRemovePermission() {
+    ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
+    UserDto user = db.users().insertUser();
+    GroupDto projectAdminGroup = db.users().insertGroup();
+    GroupDto otherProjectAdminGroup = db.users().insertGroup();
+    db.users().insertProjectPermissionOnGroup(projectAdminGroup, UserRole.USER, project);
+    db.users().insertProjectPermissionOnGroup(projectAdminGroup, UserRole.ADMIN, project);
+    db.users().insertProjectPermissionOnGroup(otherProjectAdminGroup, UserRole.USER, project);
+    db.users().insertProjectPermissionOnGroup(otherProjectAdminGroup, UserRole.ADMIN, project);
+    userSession.logIn(user).setGroups(projectAdminGroup, otherProjectAdminGroup).addProjectPermission(UserRole.USER, project).addProjectPermission(UserRole.ADMIN, project);
+
+    newRequest()
+      .setParam(PARAM_PROJECT_ID, project.uuid())
+      .setParam(PARAM_GROUP_NAME, projectAdminGroup.getName())
+      .setParam(PARAM_PERMISSION, UserRole.USER)
+      .execute();
+
+    assertThat(db.users().selectGroupPermissions(projectAdminGroup, project)).containsOnly(UserRole.ADMIN);
+    assertThat(db.users().selectGroupPermissions(otherProjectAdminGroup, project)).containsExactlyInAnyOrder(UserRole.USER, UserRole.ADMIN);
+  }
+
   private void unsafeInsertProjectPermissionOnAnyone(String perm, ComponentDto project) {
     GroupPermissionDto dto = new GroupPermissionDto()
       .setUuid(Uuids.createFast())
index 229227f87ad812478b8a39e07924906bb11fb64a..477a487d4807f42cc7369a0217c8aa6d4438b561 100644 (file)
@@ -37,6 +37,7 @@ import org.sonar.server.permission.PermissionServiceImpl;
 import org.sonar.server.ws.TestRequest;
 
 import static java.lang.String.format;
+import static java.util.Objects.requireNonNull;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.sonar.db.component.ComponentTesting.newDirectory;
@@ -49,14 +50,12 @@ import static org.sonarqube.ws.client.permission.PermissionsWsParameters.PARAM_U
 
 public class RemoveUserActionIT extends BasePermissionWsIT<RemoveUserAction> {
 
-  private static final String A_PROJECT_UUID = "project-uuid";
-  private static final String A_PROJECT_KEY = "project-key";
   private static final String A_LOGIN = "ray.bradbury";
 
   private UserDto user;
-  private ResourceTypes resourceTypes = new ResourceTypesRule().setRootQualifiers(Qualifiers.PROJECT);
-  private PermissionService permissionService = new PermissionServiceImpl(resourceTypes);
-  private WsParameters wsParameters = new WsParameters(permissionService);
+  private final ResourceTypes resourceTypes = new ResourceTypesRule().setRootQualifiers(Qualifiers.PROJECT);
+  private final PermissionService permissionService = new PermissionServiceImpl(resourceTypes);
+  private final WsParameters wsParameters = new WsParameters(permissionService);
 
   @Before
   public void setUp() {
@@ -69,7 +68,7 @@ public class RemoveUserActionIT extends BasePermissionWsIT<RemoveUserAction> {
   }
 
   @Test
-  public void remove_permission_from_user() {
+  public void wsAction_shouldRemovePermissionFromUser() {
     db.users().insertGlobalPermissionOnUser(user, GlobalPermission.PROVISION_PROJECTS);
     db.users().insertGlobalPermissionOnUser(user, GlobalPermission.ADMINISTER_QUALITY_GATES);
     loginAsAdmin();
@@ -83,7 +82,7 @@ public class RemoveUserActionIT extends BasePermissionWsIT<RemoveUserAction> {
   }
 
   @Test
-  public void admin_can_not_remove_his_global_admin_right() {
+  public void wsAction_whenAdminRemoveOwnGlobalAdminRight_shouldFail() {
     db.users().insertGlobalPermissionOnUser(user, GlobalPermission.ADMINISTER);
     loginAsAdmin();
     UserDto admin = db.users().insertUser(userSession.getLogin());
@@ -93,13 +92,13 @@ public class RemoveUserActionIT extends BasePermissionWsIT<RemoveUserAction> {
       .setParam(PARAM_USER_LOGIN, userSession.getLogin())
       .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey());
 
-    assertThatThrownBy(() -> request.execute())
+    assertThatThrownBy(request::execute)
       .isInstanceOf(BadRequestException.class)
       .hasMessage("As an admin, you can't remove your own admin right");
   }
 
   @Test
-  public void project_admin_can_not_remove_his_project_admin_right() {
+  public void wsAction_whenProjectAdminRemoveOwnProjectAdminRight_shouldFail() {
     loginAsAdmin();
     UserDto admin = db.users().insertUser(userSession.getLogin());
     ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
@@ -110,28 +109,44 @@ public class RemoveUserActionIT extends BasePermissionWsIT<RemoveUserAction> {
       .setParam(PARAM_PROJECT_ID, project.uuid())
       .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey());
 
-    assertThatThrownBy(() -> request.execute())
+    assertThatThrownBy(request::execute)
       .isInstanceOf(BadRequestException.class)
       .hasMessage("As an admin, you can't remove your own admin right");
   }
 
   @Test
-  public void fail_to_remove_admin_permission_if_last_admin() {
+  public void wsAction_whenPrivateProjectAdminRemovesOwnBrowsePermission_shouldFail() {
+    loginAsAdmin();
+    UserDto admin = db.users().insertUser(requireNonNull(userSession.getLogin()));
+    ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
+    db.users().insertProjectPermissionOnUser(admin, GlobalPermission.ADMINISTER.getKey(), project);
+
+    TestRequest request = newRequest()
+      .setParam(PARAM_USER_LOGIN, userSession.getLogin())
+      .setParam(PARAM_PROJECT_ID, project.uuid())
+      .setParam(PARAM_PERMISSION, UserRole.USER);
+
+    assertThatThrownBy(request::execute)
+      .isInstanceOf(BadRequestException.class)
+      .hasMessage("Permission 'Browse' cannot be removed from a private project for a project administrator.");
+  }
+
+  @Test
+  public void wsAction_whenRemoveAdminPermissionAndLastAdmin_shouldFail() {
     db.users().insertGlobalPermissionOnUser(user, GlobalPermission.ADMINISTER);
     loginAsAdmin();
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_USER_LOGIN, user.getLogin())
-        .setParam(PARAM_PERMISSION, UserRole.ADMIN)
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_USER_LOGIN, user.getLogin())
+      .setParam(PARAM_PERMISSION, UserRole.ADMIN);
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(BadRequestException.class)
       .hasMessage("Last user with permission 'admin'. Permission cannot be removed.");
   }
 
   @Test
-  public void remove_permission_from_project() {
+  public void wsAction_whenProject_shouldRemovePermission() {
     ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
     db.users().insertProjectPermissionOnUser(user, UserRole.CODEVIEWER, project);
     db.users().insertProjectPermissionOnUser(user, UserRole.ISSUE_ADMIN, project);
@@ -147,7 +162,7 @@ public class RemoveUserActionIT extends BasePermissionWsIT<RemoveUserAction> {
   }
 
   @Test
-  public void remove_with_project_key() {
+  public void wsAction_whenUsingProjectKey_shouldRemovePermission() {
     ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
     db.users().insertProjectPermissionOnUser(user, UserRole.ISSUE_ADMIN, project);
     db.users().insertProjectPermissionOnUser(user, UserRole.CODEVIEWER, project);
@@ -163,7 +178,7 @@ public class RemoveUserActionIT extends BasePermissionWsIT<RemoveUserAction> {
   }
 
   @Test
-  public void remove_with_view_uuid() {
+  public void wsAction_whenUsingViewUuid_shouldRemovePermission() {
     ComponentDto view = db.components().insertPrivatePortfolio();
     db.users().insertProjectPermissionOnUser(user, UserRole.ISSUE_ADMIN, view);
     db.users().insertProjectPermissionOnUser(user, UserRole.ADMIN, view);
@@ -179,34 +194,32 @@ public class RemoveUserActionIT extends BasePermissionWsIT<RemoveUserAction> {
   }
 
   @Test
-  public void fail_when_project_does_not_exist() {
+  public void wsAction_whenProjectNotFound_shouldFail() {
     loginAsAdmin();
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_USER_LOGIN, user.getLogin())
-        .setParam(PARAM_PROJECT_ID, "unknown-project-uuid")
-        .setParam(PARAM_PERMISSION, UserRole.ISSUE_ADMIN)
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_USER_LOGIN, user.getLogin())
+      .setParam(PARAM_PROJECT_ID, "unknown-project-uuid")
+      .setParam(PARAM_PERMISSION, UserRole.ISSUE_ADMIN);
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(NotFoundException.class);
   }
 
   @Test
-  public void fail_when_project_permission_without_permission() {
+  public void wsAction_whenRemovingProjectPermissionWithoutProject_shouldFail() {
     loginAsAdmin();
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_USER_LOGIN, user.getLogin())
-        .setParam(PARAM_PERMISSION, UserRole.ISSUE_ADMIN)
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_USER_LOGIN, user.getLogin())
+      .setParam(PARAM_PERMISSION, UserRole.ISSUE_ADMIN);
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(BadRequestException.class);
   }
 
   @Test
-  public void fail_when_component_is_a_directory() {
+  public void wsAction_whenComponentIsDirectory_shouldFail() {
     ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
     ComponentDto file = db.components().insertComponent(newDirectory(project, "A/B"));
 
@@ -214,7 +227,7 @@ public class RemoveUserActionIT extends BasePermissionWsIT<RemoveUserAction> {
   }
 
   @Test
-  public void fail_when_component_is_a_file() {
+  public void wsAction_whenComponentIsFile_shouldFail() {
     ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
     ComponentDto file = db.components().insertComponent(newFileDto(project, null, "file-uuid"));
 
@@ -222,7 +235,7 @@ public class RemoveUserActionIT extends BasePermissionWsIT<RemoveUserAction> {
   }
 
   @Test
-  public void fail_when_component_is_a_subview() {
+  public void wsAction_whenComponentIsSubview_shouldFail() {
     ComponentDto portfolio = db.components().insertPrivatePortfolio();
     ComponentDto file = db.components().insertComponent(newSubPortfolio(portfolio));
 
@@ -232,97 +245,88 @@ public class RemoveUserActionIT extends BasePermissionWsIT<RemoveUserAction> {
   private void failIfComponentIsNotAProjectOrView(ComponentDto file) {
     loginAsAdmin();
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_USER_LOGIN, user.getLogin())
-        .setParam(PARAM_PROJECT_ID, file.uuid())
-        .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey())
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_USER_LOGIN, user.getLogin())
+      .setParam(PARAM_PROJECT_ID, file.uuid())
+      .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey());
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(BadRequestException.class)
       .hasMessage("Component '" + file.getKey() + "' (id: " + file.uuid() + ") must be a project or a view.");
   }
 
   @Test
-  public void fail_when_get_request() {
+  public void wsAction_whenGetRequest_shouldFail() {
     loginAsAdmin();
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setMethod("GET")
-        .setParam(PARAM_USER_LOGIN, "george.orwell")
-        .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey())
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setMethod("GET")
+      .setParam(PARAM_USER_LOGIN, "george.orwell")
+      .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey());
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(ServerException.class);
   }
 
   @Test
-  public void fail_when_user_login_is_missing() {
+  public void wsAction_whenUserLoginIsMissing_shouldFail() {
     loginAsAdmin();
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey())
-        .execute();
-    })
+    TestRequest testRequest = newRequest().setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey());
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(IllegalArgumentException.class);
   }
 
   @Test
-  public void fail_when_permission_is_missing() {
+  public void wsAction_whenPermissionIsMissing_shouldFail() {
     loginAsAdmin();
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_USER_LOGIN, user.getLogin())
-        .execute();
-    })
+    TestRequest testRequest = newRequest().setParam(PARAM_USER_LOGIN, user.getLogin());
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(IllegalArgumentException.class);
   }
 
   @Test
-  public void fail_when_project_uuid_and_project_key_are_provided() {
+  public void wsAction_whenProjectUuidAndProjectKeyProvided_shouldFail() {
     ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
     loginAsAdmin();
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey())
-        .setParam(PARAM_USER_LOGIN, user.getLogin())
-        .setParam(PARAM_PROJECT_ID, project.uuid())
-        .setParam(PARAM_PROJECT_KEY, project.getKey())
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey())
+      .setParam(PARAM_USER_LOGIN, user.getLogin())
+      .setParam(PARAM_PROJECT_ID, project.uuid())
+      .setParam(PARAM_PROJECT_KEY, project.getKey());
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(BadRequestException.class)
       .hasMessage("Project id or project key can be provided, not both.");
   }
 
   @Test
-  public void removing_global_permission_fails_if_not_system_administrator() {
+  public void wsAction_whenGlobalPermissionAndNotSystemAdmin_shouldFail() {
     userSession.logIn();
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_USER_LOGIN, user.getLogin())
-        .setParam(PARAM_PERMISSION, GlobalPermission.PROVISION_PROJECTS.getKey())
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_USER_LOGIN, user.getLogin())
+      .setParam(PARAM_PERMISSION, GlobalPermission.PROVISION_PROJECTS.getKey());
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(ForbiddenException.class);
   }
 
   @Test
-  public void removing_project_permission_fails_if_not_administrator_of_project() {
+  public void wsAction_whenProjectPermissionAndNotProjectAdmin_shouldFail() {
     ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
     userSession.logIn();
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_USER_LOGIN, user.getLogin())
-        .setParam(PARAM_PERMISSION, UserRole.ISSUE_ADMIN)
-        .setParam(PARAM_PROJECT_KEY, project.getKey())
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_USER_LOGIN, user.getLogin())
+      .setParam(PARAM_PERMISSION, UserRole.ISSUE_ADMIN)
+      .setParam(PARAM_PROJECT_KEY, project.getKey());
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(ForbiddenException.class);
   }
 
@@ -330,7 +334,7 @@ public class RemoveUserActionIT extends BasePermissionWsIT<RemoveUserAction> {
    * User is project administrator but not system administrator
    */
   @Test
-  public void removing_project_permission_is_allowed_to_project_administrators() {
+  public void wsAction_whenProjectPermissionAndProjectAdmin_shouldRemovePermission() {
     ComponentDto project = db.components().insertPrivateProject().getMainBranchComponent();
     db.users().insertProjectPermissionOnUser(user, UserRole.CODEVIEWER, project);
     db.users().insertProjectPermissionOnUser(user, UserRole.ISSUE_ADMIN, project);
@@ -346,51 +350,48 @@ public class RemoveUserActionIT extends BasePermissionWsIT<RemoveUserAction> {
   }
 
   @Test
-  public void fail_when_removing_USER_permission_on_a_public_project() {
+  public void wsAction_whenBrowsePermissionAndPublicProject_shouldFail() {
     ComponentDto project = db.components().insertPublicProject().getMainBranchComponent();
     userSession.logIn().addProjectPermission(UserRole.ADMIN, project);
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_USER_LOGIN, user.getLogin())
-        .setParam(PARAM_PROJECT_ID, project.uuid())
-        .setParam(PARAM_PERMISSION, UserRole.USER)
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_USER_LOGIN, user.getLogin())
+      .setParam(PARAM_PROJECT_ID, project.uuid())
+      .setParam(PARAM_PERMISSION, UserRole.USER);
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(BadRequestException.class)
       .hasMessage("Permission user can't be removed from a public component");
 
   }
 
   @Test
-  public void fail_when_removing_CODEVIEWER_permission_on_a_public_project() {
+  public void wsAction_whenCodeviewerPermissionAndPublicProject_shouldFail() {
     ComponentDto project = db.components().insertPublicProject().getMainBranchComponent();
     userSession.logIn().addProjectPermission(UserRole.ADMIN, project);
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_USER_LOGIN, user.getLogin())
-        .setParam(PARAM_PROJECT_ID, project.uuid())
-        .setParam(PARAM_PERMISSION, UserRole.CODEVIEWER)
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_USER_LOGIN, user.getLogin())
+      .setParam(PARAM_PROJECT_ID, project.uuid())
+      .setParam(PARAM_PERMISSION, UserRole.CODEVIEWER);
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(BadRequestException.class)
       .hasMessage("Permission codeviewer can't be removed from a public component");
   }
 
   @Test
-  public void fail_when_using_branch_uuid() {
+  public void wsAction_whenUsingBranchUuid_shouldFail() {
     ComponentDto project = db.components().insertPublicProject().getMainBranchComponent();
     userSession.logIn().addProjectPermission(UserRole.ADMIN, project);
     ComponentDto branch = db.components().insertProjectBranch(project);
 
-    assertThatThrownBy(() -> {
-      newRequest()
-        .setParam(PARAM_PROJECT_ID, branch.uuid())
-        .setParam(PARAM_USER_LOGIN, user.getLogin())
-        .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey())
-        .execute();
-    })
+    TestRequest testRequest = newRequest()
+      .setParam(PARAM_PROJECT_ID, branch.uuid())
+      .setParam(PARAM_USER_LOGIN, user.getLogin())
+      .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey());
+
+    assertThatThrownBy(testRequest::execute)
       .isInstanceOf(NotFoundException.class)
       .hasMessage(format("Project id '%s' not found", branch.uuid()));
   }
index e740e7410ce00a792050424164242301e44ef75a..963e6734aebf6378af1253d3dbdbfc5ba552f96b 100644 (file)
 package org.sonar.server.permission.ws;
 
 import java.util.Optional;
+import java.util.Set;
 import javax.annotation.Nullable;
 import org.sonar.api.config.Configuration;
 import org.sonar.api.server.ws.Request;
+import org.sonar.api.web.UserRole;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.component.ComponentDto;
 import org.sonar.db.permission.template.PermissionTemplateDto;
+import org.sonar.db.user.GroupDto;
 import org.sonar.db.user.UserDto;
 import org.sonar.db.user.UserId;
 import org.sonar.db.user.UserIdDto;
 import org.sonar.server.component.ComponentFinder;
+import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.permission.GroupUuidOrAnyone;
 import org.sonar.server.permission.ws.template.WsTemplateRef;
@@ -41,18 +45,22 @@ import org.sonarqube.ws.client.permission.PermissionsWsParameters;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static java.lang.String.format;
 import static java.util.Optional.ofNullable;
+import static org.sonar.db.permission.GlobalPermission.ADMINISTER;
 import static org.sonar.server.exceptions.NotFoundException.checkFound;
 import static org.sonar.server.permission.PermissionPrivilegeChecker.checkProjectAdmin;
 import static org.sonarqube.ws.client.permission.PermissionsWsParameters.PARAM_GROUP_NAME;
 
 public class PermissionWsSupport {
 
+  private static final String ERROR_REMOVING_OWN_BROWSE_PERMISSION = "Permission 'Browse' cannot be removed from a private project for a project administrator.";
+
   private final DbClient dbClient;
   private final ComponentFinder componentFinder;
   private final GroupWsSupport groupWsSupport;
   private final Configuration configuration;
 
   public PermissionWsSupport(DbClient dbClient, Configuration configuration, ComponentFinder componentFinder, GroupWsSupport groupWsSupport) {
+
     this.dbClient = dbClient;
     this.configuration = configuration;
     this.componentFinder = componentFinder;
@@ -103,4 +111,41 @@ public class PermissionWsSupport {
     }
   }
 
+  public void checkRemovingOwnAdminRight(UserSession userSession, UserId user, String permission) {
+    if (ADMINISTER.getKey().equals(permission) && isRemovingOwnPermission(userSession, user)) {
+      throw BadRequestException.create("As an admin, you can't remove your own admin right");
+    }
+  }
+
+  private static boolean isRemovingOwnPermission(UserSession userSession, UserId user) {
+    return user.getLogin().equals(userSession.getLogin());
+  }
+
+  public void checkRemovingOwnBrowsePermissionOnPrivateProject(DbSession dbSession, UserSession userSession, @Nullable ComponentDto project, String permission,
+    GroupUuidOrAnyone group) {
+
+    if (userSession.isSystemAdministrator() || group.isAnyone() || !isUpdatingBrowsePermissionOnPrivateProject(permission, project)) {
+      return;
+    }
+
+    Set<String> groupUuidsWithPermission = dbClient.groupPermissionDao().selectGroupUuidsWithPermissionOnProject(dbSession, project.uuid(), UserRole.USER);
+    boolean isUserInAnotherGroupWithPermissionForThisProject = userSession.getGroups().stream()
+      .map(GroupDto::getUuid)
+      .anyMatch(groupDtoUuid -> groupUuidsWithPermission.contains(groupDtoUuid) && !groupDtoUuid.equals(group.getUuid()));
+
+    if (!isUserInAnotherGroupWithPermissionForThisProject) {
+      throw BadRequestException.create(ERROR_REMOVING_OWN_BROWSE_PERMISSION);
+    }
+  }
+
+  public void checkRemovingOwnBrowsePermissionOnPrivateProject(UserSession userSession, @Nullable ComponentDto project, String permission, UserId user) {
+    if (isUpdatingBrowsePermissionOnPrivateProject(permission, project) && user.getLogin().equals(userSession.getLogin())) {
+      throw BadRequestException.create(ERROR_REMOVING_OWN_BROWSE_PERMISSION);
+    }
+  }
+
+  public static boolean isUpdatingBrowsePermissionOnPrivateProject(String permission, @Nullable ComponentDto project) {
+    return project != null && project.isPrivate() && permission.equals(UserRole.USER) ;
+  }
+
 }
index d150cfc5d4b3c794d652e604c5ef1ae6e9c705a9..875f16b2b5a7440bb5cc4517c6131d3851c7a0dc 100644 (file)
@@ -19,7 +19,6 @@
  */
 package org.sonar.server.permission.ws;
 
-import java.util.Optional;
 import org.sonar.api.server.ws.Change;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
@@ -87,17 +86,22 @@ public class RemoveGroupAction implements PermissionsWsAction {
   public void handle(Request request, Response response) throws Exception {
     try (DbSession dbSession = dbClient.openSession(false)) {
       GroupUuidOrAnyone group = wsSupport.findGroup(dbSession, request);
-      Optional<ComponentDto> project = wsSupport.findProject(dbSession, request);
+      ComponentDto project = wsSupport.findProject(dbSession, request).orElse(null);
 
-      wsSupport.checkPermissionManagementAccess(userSession, project.orElse(null));
+      wsSupport.checkPermissionManagementAccess(userSession, project);
+
+      String permission = request.mandatoryParam(PARAM_PERMISSION);
+      wsSupport.checkRemovingOwnBrowsePermissionOnPrivateProject(dbSession, userSession, project, permission, group);
 
       PermissionChange change = new GroupPermissionChange(
         PermissionChange.Operation.REMOVE,
-        request.mandatoryParam(PARAM_PERMISSION),
-        project.orElse(null),
-        group, permissionService);
+        permission,
+        project,
+        group,
+        permissionService);
       permissionUpdater.apply(dbSession, singletonList(change));
     }
     response.noContent();
   }
+
 }
index ff12b6d75c6d479a66984cff8a08444e697a8f5c..b87f6ee19209e911efe6de1f4a59253dc846b8fb 100644 (file)
@@ -19,7 +19,6 @@
  */
 package org.sonar.server.permission.ws;
 
-import java.util.Optional;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
@@ -27,7 +26,6 @@ import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.component.ComponentDto;
 import org.sonar.db.user.UserId;
-import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.permission.PermissionChange;
 import org.sonar.server.permission.PermissionService;
 import org.sonar.server.permission.PermissionUpdater;
@@ -35,7 +33,6 @@ import org.sonar.server.permission.UserPermissionChange;
 import org.sonar.server.user.UserSession;
 
 import static java.util.Collections.singletonList;
-import static org.sonar.db.permission.GlobalPermission.ADMINISTER;
 import static org.sonar.server.permission.ws.WsParameters.createProjectParameters;
 import static org.sonar.server.permission.ws.WsParameters.createUserLoginParameter;
 import static org.sonarqube.ws.client.permission.PermissionsWsParameters.PARAM_PERMISSION;
@@ -86,15 +83,14 @@ public class RemoveUserAction implements PermissionsWsAction {
     try (DbSession dbSession = dbClient.openSession(false)) {
       UserId user = wsSupport.findUser(dbSession, request.mandatoryParam(PARAM_USER_LOGIN));
       String permission = request.mandatoryParam(PARAM_PERMISSION);
-      if (ADMINISTER.getKey().equals(permission) && user.getLogin().equals(userSession.getLogin())) {
-        throw BadRequestException.create("As an admin, you can't remove your own admin right");
-      }
-      Optional<ComponentDto> project = wsSupport.findProject(dbSession, request);
-      wsSupport.checkPermissionManagementAccess(userSession, project.orElse(null));
+      wsSupport.checkRemovingOwnAdminRight(userSession, user, permission);
+      ComponentDto project = wsSupport.findProject(dbSession, request).orElse(null);
+      wsSupport.checkRemovingOwnBrowsePermissionOnPrivateProject(userSession, project, permission, user);
+      wsSupport.checkPermissionManagementAccess(userSession, project);
       PermissionChange change = new UserPermissionChange(
         PermissionChange.Operation.REMOVE,
         permission,
-        project.orElse(null),
+        project,
         user, permissionService);
       permissionUpdater.apply(dbSession, singletonList(change));
       response.noContent();