]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-8134 isolate deletion of permissions
authorSimon Brandhof <simon.brandhof@sonarsource.com>
Sat, 15 Oct 2016 08:24:46 +0000 (10:24 +0200)
committerSimon Brandhof <simon.brandhof@sonarsource.com>
Sun, 16 Oct 2016 17:10:48 +0000 (19:10 +0200)
server/sonar-server/src/main/java/org/sonar/server/permission/UserPermissionChanger.java
sonar-db/src/main/java/org/sonar/db/permission/PermissionRepository.java
sonar-db/src/main/java/org/sonar/db/permission/UserPermissionDao.java
sonar-db/src/main/java/org/sonar/db/permission/UserPermissionMapper.java
sonar-db/src/main/resources/org/sonar/db/permission/UserPermissionMapper.xml
sonar-db/src/test/java/org/sonar/db/permission/UserPermissionDaoTest.java
sonar-db/src/test/java/org/sonar/db/permission/template/PermissionTemplateCharacteristicDaoTest.java

index 87f26dc7c59d711974646976edffb37b5896eb8e..5de1bd1521a0f572edbf525af7399e24c09860b4 100644 (file)
@@ -19,6 +19,7 @@
  */
 package org.sonar.server.permission;
 
+import java.util.Optional;
 import java.util.Set;
 import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.db.DbClient;
@@ -47,7 +48,12 @@ public class UserPermissionChanger {
         break;
       case REMOVE:
         checkOtherAdminUsersExist(dbSession, change);
-        dbClient.userPermissionDao().delete(dbSession, change.getUserId().getLogin(), change.getProjectUuid(), change.getPermission());
+        Optional<ProjectId> projectId = change.getProjectId();
+        if (projectId.isPresent()) {
+          dbClient.userPermissionDao().deleteProjectPermission(dbSession, change.getUserId().getId(), change.getPermission(), projectId.get().getId());
+        } else {
+          dbClient.userPermissionDao().deleteGlobalPermission(dbSession, change.getUserId().getId(), change.getPermission(), change.getOrganizationUuid());
+        }
         break;
       default:
         throw new UnsupportedOperationException("Unsupported permission change: " + change.getOperation());
index f9f00b66b074bea0c60a251f4788bc3c8abea8da..05e2a39f85cdb0f10db11e8b6059f78ee7473103 100644 (file)
@@ -64,7 +64,7 @@ public class PermissionRepository {
     PermissionTemplate template = dbClient.permissionTemplateDao().selectPermissionTemplateWithPermissions(session, templateUuid);
     updateProjectAuthorizationDate(session, project.getId());
     dbClient.groupPermissionDao().deleteByRootComponentId(session, project.getId());
-    dbClient.userPermissionDao().delete(session, null, project.uuid(), null);
+    dbClient.userPermissionDao().deleteProjectPermissions(session, project.getId());
 
     List<PermissionTemplateUserDto> usersPermissions = template.getUserPermissions();
     String organizationUuid = template.getTemplate().getOrganizationUuid();
index 648ea0afd32d68335eea9a93e0d25498d21527cd..6772ff8f3ea03e74c5003c674ab26123665df493 100644 (file)
@@ -32,7 +32,6 @@ import org.sonar.db.DbSession;
 import static com.google.common.base.Preconditions.checkArgument;
 import static java.util.Arrays.asList;
 import static java.util.Collections.emptyList;
-import static org.apache.commons.lang.StringUtils.isNotEmpty;
 import static org.sonar.db.DatabaseUtils.executeLargeInputs;
 
 public class UserPermissionDao implements Dao {
@@ -106,21 +105,24 @@ public class UserPermissionDao implements Dao {
   }
 
   /**
-   * Delete permissions for a user, permissions for a project, or a mix of them. In all cases
-   * scope can be restricted to a specified permission.
-   *
-   * Examples:
-   * <ul>
-   *   <li>{@code delete(dbSession, "marius", null, null)} deletes all permissions of Marius, including global and project permissions</li>
-   *   <li>{@code delete(dbSession, null, "ABC", null)} deletes all permissions of project ABC</li>
-   *   <li>{@code delete(dbSession, "marius", "ABC", null)} deletes the permissions of Marius on project "ABC"</li>
-   * </ul>
-   * 
-   * @see UserPermissionMapper#delete(String, String, String)
+   * Removes a single global permission from user
+   */
+  public void deleteGlobalPermission(DbSession dbSession, long userId, String permission, String organizationUuid) {
+    mapper(dbSession).deleteGlobalPermission(userId, permission, organizationUuid);
+  }
+
+  /**
+   * Removes a single project permission from user
+   */
+  public void deleteProjectPermission(DbSession dbSession, long userId, String permission, long projectId) {
+    mapper(dbSession).deleteProjectPermission(userId, permission, projectId);
+  }
+
+  /**
+   * Deletes all the permissions defined on a project
    */
-  public void delete(DbSession dbSession, @Nullable String login, @Nullable String projectUuid, @Nullable String permission) {
-    checkArgument(isNotEmpty(login) || isNotEmpty(projectUuid), "At least one of login or project must be set");
-    mapper(dbSession).delete(login, projectUuid, permission);
+  public void deleteProjectPermissions(DbSession dbSession, long projectId) {
+    mapper(dbSession).deleteProjectPermissions(projectId);
   }
 
   private static UserPermissionMapper mapper(DbSession dbSession) {
index 5da6ff35ff41c7e300a91e253ed06c05cf428605..fffc729fac1b37a274345894ea1f1f62acac24a9 100644 (file)
@@ -52,10 +52,12 @@ public interface UserPermissionMapper {
 
   void insert(UserPermissionDto dto);
 
-  /**
-   * Delete permissions by user and/or by project. In both cases scope can be restricted to a specified permission
-   */
-  void delete(@Nullable @Param("login") String login, @Nullable @Param("projectUuid") String projectUuid, @Nullable @Param("permission") String permission);
-
   int countRowsByRootComponentId(@Param("rootComponentId") long projectId);
+
+  void deleteGlobalPermission(@Param("userId") long userId, @Param("permission") String permission, @Param("organizationUuid") String organizationUuid);
+
+  void deleteProjectPermission(@Param("userId") long userId, @Param("permission") String permission, @Param("projectId") long projectId);
+
+  void deleteProjectPermissions(@Param("projectId") long projectId);
+
 }
index a134e797cb6f2f7ccb1726bde908c94615391ef7..901f66063c5d786b060cb692cdf72a987b81b497 100644 (file)
     )
   </insert>
 
-  <delete id="delete" parameterType="map">
+  <delete id="deleteGlobalPermission" parameterType="map">
     delete from user_roles
-    <where>
-      <choose>
-        <when test="login != null">
-          and user_id = (select id from users where login = #{login})
-          <if test="projectUuid != null">
-          and resource_id = (select id from projects where uuid = #{projectUuid})
-          </if>
-        </when>
-        <otherwise>
-          and resource_id = (select id from projects where uuid = #{projectUuid})
-        </otherwise>
-      </choose>
-      <if test="permission != null">
-        and role = #{permission}
-      </if>
-    </where>
+    where
+    role = #{permission,jdbcType=VARCHAR} and
+    user_id = #{userId,jdbcType=BIGINT} and
+    organization_uuid = #{organizationUuid,jdbcType=VARCHAR} and
+    resource_id is null
+  </delete>
+
+  <delete id="deleteProjectPermission" parameterType="map">
+    delete from user_roles
+    where
+    role = #{permission,jdbcType=VARCHAR} and
+    user_id = #{userId,jdbcType=BIGINT} and
+    resource_id = #{projectId,jdbcType=BIGINT}
+  </delete>
+
+  <delete id="deleteProjectPermissions" parameterType="map">
+    delete from user_roles
+    where
+    resource_id = #{projectId,jdbcType=BIGINT}
   </delete>
 </mapper>
index e056456ec4a8e4b4a7f0ef9b6a76089f49e97646..52e16f21b43ffe9c55be1c70ea84423244a18d2f 100644 (file)
@@ -32,6 +32,7 @@ import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
 import org.sonar.db.component.ComponentDto;
+import org.sonar.db.organization.OrganizationDto;
 import org.sonar.db.user.UserDto;
 
 import static java.util.Arrays.asList;
@@ -69,10 +70,10 @@ public class UserPermissionDaoTest {
 
   @Test
   public void select_global_permissions() {
-    UserPermissionDto global1 = insertGlobalPermission(SYSTEM_ADMIN, user1.getId());
-    UserPermissionDto global2 = insertGlobalPermission(SYSTEM_ADMIN, user2.getId());
-    UserPermissionDto global3 = insertGlobalPermission(PROVISIONING, user2.getId());
-    UserPermissionDto project1Perm = insertProjectPermission(USER, user3.getId(), this.project1.getId());
+    UserPermissionDto global1 = addGlobalPermissionOnDefaultOrganization(SYSTEM_ADMIN, user1);
+    UserPermissionDto global2 = addGlobalPermissionOnDefaultOrganization(SYSTEM_ADMIN, user2);
+    UserPermissionDto global3 = addGlobalPermissionOnDefaultOrganization(PROVISIONING, user2);
+    UserPermissionDto project1Perm = addProjectPermissionOnDefaultOrganization(USER, user3, project1);
 
     // global permissions of users who has at least one global permission, ordered by user name then permission
     PermissionQuery query = PermissionQuery.builder().withAtLeastOnePermission().build();
@@ -132,11 +133,11 @@ public class UserPermissionDaoTest {
 
   @Test
   public void select_project_permissions() {
-    insertGlobalPermission(SYSTEM_ADMIN, user1.getId());
-    UserPermissionDto perm1 = insertProjectPermission(USER, user1.getId(), project1.getId());
-    UserPermissionDto perm2 = insertProjectPermission(ISSUE_ADMIN, user1.getId(), project1.getId());
-    UserPermissionDto perm3 = insertProjectPermission(ISSUE_ADMIN, user2.getId(), project1.getId());
-    insertProjectPermission(ISSUE_ADMIN, user3.getId(), project2.getId());
+    addGlobalPermissionOnDefaultOrganization(SYSTEM_ADMIN, user1);
+    UserPermissionDto perm1 = addProjectPermissionOnDefaultOrganization(USER, user1, project1);
+    UserPermissionDto perm2 = addProjectPermissionOnDefaultOrganization(ISSUE_ADMIN, user1, project1);
+    UserPermissionDto perm3 = addProjectPermissionOnDefaultOrganization(ISSUE_ADMIN, user2, project1);
+    addProjectPermissionOnDefaultOrganization(ISSUE_ADMIN, user3, project2);
 
     // project permissions of users who has at least one permission on this project
     PermissionQuery query = PermissionQuery.builder().withAtLeastOnePermission().setComponentUuid(project1.uuid()).build();
@@ -185,11 +186,11 @@ public class UserPermissionDaoTest {
 
   @Test
   public void countUsersByProjectPermission() {
-    insertGlobalPermission(SYSTEM_ADMIN, user1.getId());
-    insertProjectPermission(USER, user1.getId(), project1.getId());
-    insertProjectPermission(ISSUE_ADMIN, user1.getId(), project1.getId());
-    insertProjectPermission(ISSUE_ADMIN, user2.getId(), project1.getId());
-    insertProjectPermission(ISSUE_ADMIN, user2.getId(), project2.getId());
+    addGlobalPermissionOnDefaultOrganization(SYSTEM_ADMIN, user1);
+    addProjectPermissionOnDefaultOrganization(USER, user1, project1);
+    addProjectPermissionOnDefaultOrganization(ISSUE_ADMIN, user1, project1);
+    addProjectPermissionOnDefaultOrganization(ISSUE_ADMIN, user2, project1);
+    addProjectPermissionOnDefaultOrganization(ISSUE_ADMIN, user2, project2);
 
     // no projects -> return empty list
     assertThat(underTest.countUsersByProjectPermission(dbSession, Collections.emptyList())).isEmpty();
@@ -208,10 +209,10 @@ public class UserPermissionDaoTest {
 
   @Test
   public void selectLogins() {
-    insertProjectPermission(USER, user1.getId(), project1.getId());
-    insertProjectPermission(USER, user2.getId(), project1.getId());
-    insertProjectPermission(ISSUE_ADMIN, user2.getId(), project1.getId());
-    insertProjectPermission(ISSUE_ADMIN, user2.getId(), project2.getId());
+    addProjectPermissionOnDefaultOrganization(USER, user1, project1);
+    addProjectPermissionOnDefaultOrganization(USER, user2, project1);
+    addProjectPermissionOnDefaultOrganization(ISSUE_ADMIN, user2, project1);
+    addProjectPermissionOnDefaultOrganization(ISSUE_ADMIN, user2, project2);
 
     // logins are ordered by user name: user2 ("Marie") then user1 ("Marius")
     PermissionQuery query = PermissionQuery.builder().setComponentUuid(project1.uuid()).withAtLeastOnePermission().build();
@@ -225,11 +226,11 @@ public class UserPermissionDaoTest {
 
   @Test
   public void selectPermissionsByLogin() {
-    insertGlobalPermission(SYSTEM_ADMIN, user1.getId());
-    insertProjectPermission(USER, user1.getId(), project1.getId());
-    insertProjectPermission(USER, user2.getId(), project1.getId());
-    insertProjectPermission(ISSUE_ADMIN, user2.getId(), project1.getId());
-    insertProjectPermission(ISSUE_ADMIN, user2.getId(), project2.getId());
+    addGlobalPermissionOnDefaultOrganization(SYSTEM_ADMIN, user1);
+    addProjectPermissionOnDefaultOrganization(USER, user1, project1);
+    addProjectPermissionOnDefaultOrganization(USER, user2, project1);
+    addProjectPermissionOnDefaultOrganization(ISSUE_ADMIN, user2, project1);
+    addProjectPermissionOnDefaultOrganization(ISSUE_ADMIN, user2, project2);
 
     // user1 has one global permission and user2 has no global permissions
     assertThat(underTest.selectPermissionsByLogin(dbSession, user1.getLogin(), null)).hasSize(1);
@@ -247,51 +248,68 @@ public class UserPermissionDaoTest {
   }
 
   @Test
-  public void delete_by_project() {
-    insertGlobalPermission(SYSTEM_ADMIN, user1.getId());
-    insertProjectPermission(USER, user1.getId(), project1.getId());
-    insertProjectPermission(ISSUE_ADMIN, user2.getId(), project1.getId());
-    insertProjectPermission(ISSUE_ADMIN, user2.getId(), project2.getId());
-
-    underTest.delete(dbSession, null, project1.uuid(), null);
-
-    assertThat(dbTester.countSql(dbSession, "select count(id) from user_roles where resource_id=" + project1.getId())).isEqualTo(0);
-    // remains global permission and project2 permission
-    assertThat(dbTester.countRowsOfTable(dbSession, "user_roles")).isEqualTo(2);
+  public void deleteGlobalPermission() {
+    addGlobalPermissionOnDefaultOrganization("perm1", user1);
+    addGlobalPermissionOnDefaultOrganization("perm2", user1);
+    addProjectPermissionOnDefaultOrganization("perm1", user1, project1);
+    addProjectPermissionOnDefaultOrganization("perm3", user2, project1);
+    addProjectPermissionOnDefaultOrganization("perm4", user2, project2);
+
+    // user2 does not have global permissions -> do nothing
+    underTest.deleteGlobalPermission(dbSession, user2.getId(), "perm1", dbTester.getDefaultOrganization().getUuid());
+    assertThat(dbTester.countRowsOfTable(dbSession, "user_roles")).isEqualTo(5);
+
+    // global permission is not granted -> do nothing
+    underTest.deleteGlobalPermission(dbSession, user1.getId(), "notGranted", dbTester.getDefaultOrganization().getUuid());
+    assertThat(dbTester.countRowsOfTable(dbSession, "user_roles")).isEqualTo(5);
+
+    // permission is on project -> do nothing
+    underTest.deleteGlobalPermission(dbSession, user1.getId(), "perm3", dbTester.getDefaultOrganization().getUuid());
+    assertThat(dbTester.countRowsOfTable(dbSession, "user_roles")).isEqualTo(5);
+
+    // global permission on another organization-> do nothing
+    underTest.deleteGlobalPermission(dbSession, user1.getId(), "notGranted", "anotherOrg");
+    assertThat(dbTester.countRowsOfTable(dbSession, "user_roles")).isEqualTo(5);
+
+    // global permission exists -> delete it, but not the project permission with the same name !
+    underTest.deleteGlobalPermission(dbSession, user1.getId(), "perm1", dbTester.getDefaultOrganization().getUuid());
+    assertThat(dbTester.countSql(dbSession, "select count(id) from user_roles where role='perm1' and resource_id is null")).isEqualTo(0);
+    assertThat(dbTester.countRowsOfTable(dbSession, "user_roles")).isEqualTo(4);
   }
 
   @Test
-  public void delete_by_user() {
-    insertGlobalPermission(SYSTEM_ADMIN, user1.getId());
-    insertProjectPermission(USER, user1.getId(), project1.getId());
-    insertProjectPermission(ISSUE_ADMIN, user2.getId(), project1.getId());
-    insertProjectPermission(ISSUE_ADMIN, user2.getId(), project2.getId());
-
-    underTest.delete(dbSession, user1.getLogin(), null, null);
-
-    assertThat(dbTester.countSql(dbSession, "select count(id) from user_roles where user_id=" + user1.getId())).isEqualTo(0);
-    // remains user2 permissions
-    assertThat(dbTester.countRowsOfTable(dbSession, "user_roles")).isEqualTo(2);
+  public void deleteProjectPermission() {
+    addGlobalPermissionOnDefaultOrganization("perm", user1);
+    addProjectPermissionOnDefaultOrganization("perm", user1, project1);
+    addProjectPermissionOnDefaultOrganization("perm", user1, project2);
+    addProjectPermissionOnDefaultOrganization("perm", user2, project1);
+
+    // no such provision -> ignore
+    underTest.deleteProjectPermission(dbSession, user1.getId(), "anotherPerm", project1.getId());
+    assertThat(dbTester.countRowsOfTable(dbSession, "user_roles")).isEqualTo(4);
+
+    underTest.deleteProjectPermission(dbSession, user1.getId(), "perm", project1.getId());
+    assertThatProjectPermissionDoesNotExist(user1, "perm", project1);
+    assertThat(dbTester.countRowsOfTable(dbSession, "user_roles")).isEqualTo(3);
   }
 
   @Test
-  public void delete_specific_permission() {
-    insertGlobalPermission(SYSTEM_ADMIN, user1.getId());
-    insertProjectPermission(USER, user1.getId(), project1.getId());
-    insertProjectPermission(ISSUE_ADMIN, user2.getId(), project1.getId());
-    insertProjectPermission(ISSUE_ADMIN, user2.getId(), project2.getId());
-
-    underTest.delete(dbSession, user1.getLogin(), project1.uuid(), USER);
+  public void deleteProjectPermissions() {
+    addGlobalPermissionOnDefaultOrganization("perm", user1);
+    addProjectPermissionOnDefaultOrganization("perm", user1, project1);
+    addProjectPermissionOnDefaultOrganization("perm", user2, project1);
+    addProjectPermissionOnDefaultOrganization("perm", user1, project2);
 
-    assertThat(dbTester.countRowsOfTable(dbSession, "user_roles")).isEqualTo(3);
-    assertThat(dbTester.countSql(dbSession, "select count(id) from user_roles where user_id=" + user1.getId())).isEqualTo(1);
-    assertThat(dbTester.countSql(dbSession, "select count(id) from user_roles where role='" + SYSTEM_ADMIN + "' and user_id=" + user1.getId())).isEqualTo(1);
+    underTest.deleteProjectPermissions(dbSession, project1.getId());
+    assertThat(dbTester.countRowsOfTable(dbSession, "user_roles")).isEqualTo(2);
+    assertThatProjectHasNoPermissions(project1);
   }
 
+
   @Test
   public void projectHasPermissions() {
-    insertGlobalPermission(SYSTEM_ADMIN, user1.getId());
-    insertProjectPermission(USER, user1.getId(), project1.getId());
+    addGlobalPermissionOnDefaultOrganization(SYSTEM_ADMIN, user1);
+    addProjectPermissionOnDefaultOrganization(USER, user1, project1);
 
     assertThat(underTest.hasRootComponentPermissions(dbSession, project1.getId())).isTrue();
     assertThat(underTest.hasRootComponentPermissions(dbSession, project2.getId())).isFalse();
@@ -332,17 +350,33 @@ public class UserPermissionDaoTest {
     }
   }
 
-  private UserPermissionDto insertGlobalPermission(String permission, long userId) {
-    UserPermissionDto dto = new UserPermissionDto(dbTester.getDefaultOrganization().getUuid(), permission, userId, null);
+  private UserPermissionDto addGlobalPermissionOnDefaultOrganization(String permission, UserDto user) {
+    return addGlobalPermission(dbTester.getDefaultOrganization(), permission, user);
+  }
+
+  private UserPermissionDto addGlobalPermission(OrganizationDto org, String permission, UserDto user) {
+    UserPermissionDto dto = new UserPermissionDto(org.getUuid(), permission, user.getId(), null);
     underTest.insert(dbSession, dto);
     dbTester.commit();
     return dto;
   }
 
-  private UserPermissionDto insertProjectPermission(String permission, long userId, long projectId) {
-    UserPermissionDto dto = new UserPermissionDto(dbTester.getDefaultOrganization().getUuid(), permission, userId, projectId);
+  private UserPermissionDto addProjectPermissionOnDefaultOrganization(String permission, UserDto user, ComponentDto project) {
+    return addProjectPermission(dbTester.getDefaultOrganization(), permission, user, project);
+  }
+
+  private UserPermissionDto addProjectPermission(OrganizationDto org, String permission, UserDto user, ComponentDto project) {
+    UserPermissionDto dto = new UserPermissionDto(org.getUuid(), permission, user.getId(), project.getId());
     underTest.insert(dbSession, dto);
     dbTester.commit();
     return dto;
   }
+
+  private void assertThatProjectPermissionDoesNotExist(UserDto user, String permission, ComponentDto project) {
+    assertThat(dbTester.countSql(dbSession, "select count(id) from user_roles where role='" + permission + "' and user_id=" + user.getId() + " and resource_id=" + project.getId())).isEqualTo(0);
+  }
+
+  private void assertThatProjectHasNoPermissions(ComponentDto project) {
+    assertThat(dbTester.countSql(dbSession, "select count(id) from user_roles where resource_id=" + project.getId())).isEqualTo(0);
+  }
 }
index 4b91d1660d8a4af6e287aabcf87114319e979b6b..996118eac508e46fbebc0347d1b3da8dccc39cb0 100644 (file)
@@ -40,10 +40,8 @@ public class PermissionTemplateCharacteristicDaoTest {
   public ExpectedException expectedException = ExpectedException.none();
   @Rule
   public DbTester db = DbTester.create(System2.INSTANCE);
-  DbSession dbSession = db.getSession();
-  PermissionTemplateCharacteristicMapper mapper = dbSession.getMapper(PermissionTemplateCharacteristicMapper.class);
-
-  PermissionTemplateCharacteristicDao underTest = new PermissionTemplateCharacteristicDao();
+  private DbSession dbSession = db.getSession();
+  private PermissionTemplateCharacteristicDao underTest = new PermissionTemplateCharacteristicDao();
 
   @Test
   public void selectByTemplateId_filter_by_template_id() {