]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-8134 fix isolation of organizations when dropping global permissions
authorSimon Brandhof <simon.brandhof@sonarsource.com>
Fri, 21 Oct 2016 13:25:28 +0000 (15:25 +0200)
committerSimon Brandhof <simon.brandhof@sonarsource.com>
Fri, 21 Oct 2016 13:58:40 +0000 (15:58 +0200)
on user

server/sonar-server/src/main/java/org/sonar/server/permission/GroupPermissionChanger.java
server/sonar-server/src/main/java/org/sonar/server/permission/UserPermissionChanger.java
server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/DeleteAction.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 6d31338c376983ea85beea79db7259a9c2eff124..2a5be5fbc0df44e09d0e7e928770feef08881538 100644 (file)
@@ -106,7 +106,7 @@ public class GroupPermissionChanger {
       !change.getGroupIdOrAnyone().isAnyone() &&
       !change.getProjectId().isPresent()) {
       // removing global admin permission from group
-      int remaining = dbClient.authorizationDao().countRemainingUserIdsWithGlobalPermissionIfExcludeGroup(dbSession,
+      int remaining = dbClient.authorizationDao().countUsersWithGlobalPermissionExcludingGroup(dbSession,
         change.getOrganizationUuid(), SYSTEM_ADMIN, change.getGroupIdOrAnyone().getId());
 
       if (remaining == 0) {
index c8d397b78e7776e1a5fa9aa8c7387ea7ee869834..61b4e977762e6c289a699be00803b1f685eac3c8 100644 (file)
@@ -92,7 +92,7 @@ public class UserPermissionChanger {
 
   private void checkOtherAdminsExist(DbSession dbSession, UserPermissionChange change) {
     if (SYSTEM_ADMIN.equals(change.getPermission()) && !change.getProjectId().isPresent()) {
-      int remaining = dbClient.authorizationDao().countRemainingUsersWithGlobalPermissionExcludingUser(dbSession,
+      int remaining = dbClient.authorizationDao().countUsersWithGlobalPermissionExcludingUser(dbSession,
         change.getOrganizationUuid(), change.getPermission(), change.getUserId().getId());
       if (remaining == 0) {
         throw new BadRequestException(String.format("Last user with permission '%s'. Permission cannot be removed.", SYSTEM_ADMIN));
index f35473c8b5798860b7ba753fd6ec1890490261c6..5ab66eab3b5570f61330d9c13871bef6cd5df7a8 100644 (file)
@@ -105,7 +105,7 @@ public class DeleteAction implements UserGroupsWsAction {
   }
 
   private void checkNotTryingToDeleteLastAdminGroup(DbSession dbSession, GroupId group) {
-    int remaining = dbClient.authorizationDao().countRemainingUserIdsWithGlobalPermissionIfExcludeGroup(dbSession,
+    int remaining = dbClient.authorizationDao().countUsersWithGlobalPermissionExcludingGroup(dbSession,
       group.getOrganizationUuid(), SYSTEM_ADMIN, group.getId());
 
     checkArgument(remaining > 0, "The last system admin group cannot be deleted");
index c85bf9732d073eb2d5668d8ad9189c0bd733713d..2cd2195ab91a4d19a855cd37d4c825bb30cf3285 100644 (file)
@@ -81,18 +81,18 @@ public class AuthorizationDao implements Dao {
    * The number of users who will still have the permission when the group {@code excludedGroupId}
    * is deleted. The anyone virtual group is not taken into account.
    */
-  public int countRemainingUserIdsWithGlobalPermissionIfExcludeGroup(DbSession dbSession, String organizationUuid,
+  public int countUsersWithGlobalPermissionExcludingGroup(DbSession dbSession, String organizationUuid,
     String permission, long excludedGroupId) {
-    return mapper(dbSession).countRemainingUserIdsWithGlobalPermissionIfExcludeGroup(organizationUuid, permission, excludedGroupId);
+    return mapper(dbSession).countUsersWithGlobalPermissionExcludingGroup(organizationUuid, permission, excludedGroupId);
   }
 
   /**
    * The number of users who will still have the permission when the user {@code excludedUserId}
    * is deleted. The anyone virtual group is not taken into account.
    */
-  public int countRemainingUsersWithGlobalPermissionExcludingUser(DbSession dbSession, String organizationUuid,
-                                                                  String permission, long excludedUSerId) {
-    return mapper(dbSession).countRemainingUsersWithGlobalPermissionExcludingUser(organizationUuid, permission, excludedUSerId);
+  public int countUsersWithGlobalPermissionExcludingUser(DbSession dbSession, String organizationUuid,
+    String permission, long excludedUSerId) {
+    return mapper(dbSession).countUsersWithGlobalPermissionExcludingUser(organizationUuid, permission, excludedUSerId);
   }
 
   public Collection<Long> keepAuthorizedProjectIds(DbSession dbSession, Collection<Long> componentIds, @Nullable Integer userId, String role) {
index 0d83a18acbdc96dbc93c642fd8946b36d3c0c855..00cbcd9e28ed44cb49f48cd2c3f1dd008c5dbd7e 100644 (file)
@@ -37,9 +37,11 @@ public interface AuthorizationMapper {
 
   Set<String> selectRootComponentPermissionsOfAnonymous(@Param("rootComponentId") long rootComponentId);
 
-  int countRemainingUserIdsWithGlobalPermissionIfExcludeGroup(@Param("organizationUuid") String organizationUuid, @Param("permission") String permission, @Param("excludedGroupId") long excludedGroupId);
+  int countUsersWithGlobalPermissionExcludingGroup(@Param("organizationUuid") String organizationUuid,
+    @Param("permission") String permission, @Param("excludedGroupId") long excludedGroupId);
 
-  int countRemainingUsersWithGlobalPermissionExcludingUser(@Param("organizationUuid") String organizationUuid, @Param("permission") String permission, @Param("excludedUserId") long excludedUserId);
+  int countUsersWithGlobalPermissionExcludingUser(@Param("organizationUuid") String organizationUuid, @Param("permission") String permission,
+    @Param("excludedUserId") long excludedUserId);
 
   List<Long> keepAuthorizedProjectIdsForAnonymous(@Param("role") String role, @Param("componentIds") Collection<Long> componentIds);
 
index 28d66d9f5360ccb7c485436e49706f2976407195..37738662baf151d0fee30d72c9f469bb6ea6b67b 100644 (file)
@@ -71,7 +71,7 @@
     gr.group_id is null
   </select>
 
-  <select id="countRemainingUserIdsWithGlobalPermissionIfExcludeGroup" parameterType="map" resultType="int">
+  <select id="countUsersWithGlobalPermissionExcludingGroup" parameterType="map" resultType="int">
     select count(1) from
     (
       select gu.user_id
       select ur.user_id
       from user_roles ur
       where
+      ur.organization_uuid = #{organizationUuid,jdbcType=VARCHAR} and
       ur.resource_id is null and
       ur.role = #{permission,jdbcType=VARCHAR}
     ) remaining
   </select>
 
-  <select id="countRemainingUsersWithGlobalPermissionExcludingUser" parameterType="map" resultType="int">
+  <select id="countUsersWithGlobalPermissionExcludingUser" parameterType="map" resultType="int">
     select count(1) from
     (
     select gu.user_id
     gr.organization_uuid = #{organizationUuid,jdbcType=VARCHAR} and
     gr.role = #{permission,jdbcType=VARCHAR} and
     gr.resource_id is null and
-    gr.group_id is not null
+    gr.group_id is not null and
+    gu.user_id != #{excludedUserId,jdbcType=BIGINT}
 
     union
 
     select ur.user_id
     from user_roles ur
     where
+    ur.organization_uuid = #{organizationUuid,jdbcType=VARCHAR} and
     ur.resource_id is null and
     ur.role = #{permission,jdbcType=VARCHAR} and
     ur.user_id != #{excludedUserId,jdbcType=BIGINT}
index 6a62f5dc51735fcf02eb950292bce1b23b487fcb..e1592e3136561f1ef2f06798753efc266eaaf752 100644 (file)
@@ -173,7 +173,7 @@ public class AuthorizationDaoTest {
   }
 
   @Test
-  public void countRemainingUserIdsWithGlobalPermissionIfExcludeGroup() {
+  public void countUsersWithGlobalPermissionExcludingGroup() {
     // users with global permission "perm1" :
     // - "u1" and "u2" through group "g1"
     // - "u1" and "u3" through group "g2"
@@ -202,27 +202,67 @@ public class AuthorizationDaoTest {
     GroupDto group3 = db.users().insertGroup(org, "g2");
     db.users().insertPermissionOnGroup(group3, "perm1");
 
-    db.users().insertPermissionOnUser(user4, "perm1");
-    db.users().insertPermissionOnUser(user4, "perm2");
-
+    db.users().insertPermissionOnUser(org, user4, "perm1");
+    db.users().insertPermissionOnUser(org, user4, "perm2");
     db.users().insertPermissionOnAnyone(org, "perm1");
 
+    // other organizations are ignored
+    OrganizationDto org2 = db.organizations().insert();
+    db.users().insertPermissionOnUser(org2, user1, "perm1");
+
     // excluding group "g1" -> remain u1, u3 and u4
-    assertThat(underTest.countRemainingUserIdsWithGlobalPermissionIfExcludeGroup(db.getSession(),
+    assertThat(underTest.countUsersWithGlobalPermissionExcludingGroup(db.getSession(),
       org.getUuid(), "perm1", group1.getId())).isEqualTo(3);
 
     // excluding group "g2" -> remain u1, u2 and u4
-    assertThat(underTest.countRemainingUserIdsWithGlobalPermissionIfExcludeGroup(db.getSession(),
+    assertThat(underTest.countUsersWithGlobalPermissionExcludingGroup(db.getSession(),
       org.getUuid(), "perm1", group2.getId())).isEqualTo(3);
 
     // excluding group "g3" -> remain u1, u2, u3 and u4
-    assertThat(underTest.countRemainingUserIdsWithGlobalPermissionIfExcludeGroup(db.getSession(),
+    assertThat(underTest.countUsersWithGlobalPermissionExcludingGroup(db.getSession(),
       org.getUuid(), "perm1", group3.getId())).isEqualTo(4);
 
     // nobody has the permission
-    assertThat(underTest.countRemainingUserIdsWithGlobalPermissionIfExcludeGroup(db.getSession(),
+    assertThat(underTest.countUsersWithGlobalPermissionExcludingGroup(db.getSession(),
       org.getUuid(), "missingPermission", group1.getId())).isEqualTo(0);
+  }
+
+  @Test
+  public void countUsersWithGlobalPermissionExcludingUser() {
+    // group g1 has the permission p1 and has members user1 and user2
+    // user3 has the permission
+    UserDto user1 = db.users().insertUser();
+    UserDto user2 = db.users().insertUser();
+    UserDto user3 = db.users().insertUser();
 
+    OrganizationDto org = db.organizations().insert();
+    GroupDto group1 = db.users().insertGroup(org, "g1");
+    db.users().insertPermissionOnGroup(group1, "p1");
+    db.users().insertPermissionOnGroup(group1, "p2");
+    db.users().insertMember(group1, user1);
+    db.users().insertMember(group1, user2);
+    db.users().insertPermissionOnUser(org, user3, "p1");
+    db.users().insertPermissionOnAnyone(org, "p1");
+
+    // other organizations are ignored
+    OrganizationDto org2 = db.organizations().insert();
+    db.users().insertPermissionOnUser(org2, user1, "p1");
+
+    // excluding user1 -> remain user2 and user3
+    assertThat(underTest.countUsersWithGlobalPermissionExcludingUser(db.getSession(),
+      org.getUuid(), "p1", user1.getId())).isEqualTo(2);
+
+    // excluding user3 -> remain the members of group g1
+    assertThat(underTest.countUsersWithGlobalPermissionExcludingUser(db.getSession(),
+      org.getUuid(), "p1", user3.getId())).isEqualTo(2);
+
+    // excluding unknown user
+    assertThat(underTest.countUsersWithGlobalPermissionExcludingUser(db.getSession(),
+      org.getUuid(), "p1", -1)).isEqualTo(3);
+
+    // nobody has the permission
+    assertThat(underTest.countUsersWithGlobalPermissionExcludingUser(db.getSession(),
+      org.getUuid(), "missingPermission", group1.getId())).isEqualTo(0);
   }
 
   @Test