SONAR-9444 make private at least one missing perm user or codeviewer

2 files changed, 102 insertions, 31 deletions

diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v64/MakeComponentsPrivateBasedOnPermissions.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v64/MakeComponentsPrivateBasedOnPermissions.java
index 32aa0a5dac1..2df2bd5273a 100644
--- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v64/MakeComponentsPrivateBasedOnPermissions.java
+++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v64/MakeComponentsPrivateBasedOnPermissions.java
@@ -68,14 +68,25 @@ public class MakeComponentsPrivateBasedOnPermissions extends DataChange {
       " p.scope = ?" +
       " and p.qualifier in (?, ?)" +
       " and p.private = ?" +
-      " and not exists (" +
-      "   select" +
-      "     1" +
-      "   from group_roles gr" +
-      "   where " +
-      "     gr.resource_id = p.id" +
-      "     and gr.group_id is null" +
-      "     and gr.role in (?, ?)" +
+      " and (" +
+      "   not exists (" +
+      "     select" +
+      "       1" +
+      "     from group_roles gr" +
+      "     where " +
+      "       gr.resource_id = p.id" +
+      "       and gr.group_id is null" +
+      "       and gr.role = ?" +
+      "   ) " +
+      "   or not exists (" +
+      "     select" +
+      "       1" +
+      "     from group_roles gr" +
+      "     where " +
+      "       gr.resource_id = p.id" +
+      "       and gr.group_id is null" +
+      "       and gr.role = ?" +
+      "   )" +
       " )" +
       // trees with only permissions to group must not be made private
       " and (" +
diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v64/MakeComponentsPrivateBasedOnPermissionsTest.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v64/MakeComponentsPrivateBasedOnPermissionsTest.java
index 6c8d3189fa9..8ad82c6b4f2 100644
--- a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v64/MakeComponentsPrivateBasedOnPermissionsTest.java
+++ b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v64/MakeComponentsPrivateBasedOnPermissionsTest.java
@@ -46,8 +46,6 @@ public class MakeComponentsPrivateBasedOnPermissionsTest {
   private final String randomPublicConditionRole = random.nextBoolean() ? ROLE_CODEVIEWER : ROLE_USER;
   private final String randomQualifier = random.nextBoolean() ? PROJECT_QUALIFIER : VIEW_QUALIFIER;
   private final String randomRole = "role_" + random.nextInt(12);
-  private final int randomUserId = random.nextInt(500);
-  private final int randomGroupId = random.nextInt(500);
   private MakeComponentsPrivateBasedOnPermissions underTest = new MakeComponentsPrivateBasedOnPermissions(db.database());
 
   @Test
@@ -56,10 +54,10 @@ public class MakeComponentsPrivateBasedOnPermissionsTest {
   }
 
   @Test
-  public void execute_makes_project_private_if_group_AnyOne_has_global_permission_USER() throws SQLException {
-    long pId = insertRootComponent("p1", false);
-    insertGroupPermission(ROLE_USER, null, null);
-    insertGroupPermission(randomRole, pId, randomGroupId);
+  public void execute_makes_project_private_if_Anyone_has_only_user_permission_and_project_has_at_least_one_other_group_permission() throws SQLException {
+    long pId1 = insertRootComponent("p1", false);
+    insertGroupPermission(ROLE_USER, pId1, null);
+    insertGroupPermission("foo", pId1, random.nextInt(10));
 
     underTest.execute();
 
@@ -67,10 +65,10 @@ public class MakeComponentsPrivateBasedOnPermissionsTest {
   }
 
   @Test
-  public void execute_makes_project_private_if_group_AnyOne_has_global_permission_BROWSE() throws SQLException {
-    long pId = insertRootComponent("p1", false);
-    insertGroupPermission(ROLE_CODEVIEWER, null, null);
-    insertUserPermission(randomRole, pId, randomUserId);
+  public void execute_makes_project_private_if_Anyone_has_only_user_permission_and_project_has_one_user_permission() throws SQLException {
+    long pId1 = insertRootComponent("p1", false);
+    insertGroupPermission(ROLE_USER, pId1, null);
+    insertUserPermission("foo", pId1, random.nextInt(10));
 
     underTest.execute();
 
@@ -78,9 +76,20 @@ public class MakeComponentsPrivateBasedOnPermissionsTest {
   }
 
   @Test
-  public void execute_makes_project_private_if_group_other_than_AnyOne_has_permission_BROWSE_on_other_project() throws SQLException {
+  public void execute_keeps_project_public_if_Anyone_has_only_user_permission_and_project_has_no_user_nor_other_group_permission() throws SQLException {
+    long pId1 = insertRootComponent("p1", false);
+    insertGroupPermission(ROLE_USER, pId1, null);
+
+    underTest.execute();
+
+    assertThat(isPrivate("p1")).isFalse();
+  }
+
+  @Test
+  public void execute_makes_project_private_if_Anyone_has_only_codeviewer_permission_and_project_has_one_other_group_permission() throws SQLException {
     long pId1 = insertRootComponent("p1", false);
-    insertGroupPermission(ROLE_CODEVIEWER, pId1, random.nextInt(30));
+    insertGroupPermission(ROLE_CODEVIEWER, pId1, null);
+    insertGroupPermission("foo", pId1, random.nextInt(10));
 
     underTest.execute();
 
@@ -88,9 +97,10 @@ public class MakeComponentsPrivateBasedOnPermissionsTest {
   }
 
   @Test
-  public void execute_makes_project_private_if_group_other_than_AnyOne_has_permission_USER_on_other_project() throws SQLException {
+  public void execute_makes_project_private_if_Anyone_has_only_codeviewer_permission_and_project_has_one_user_permission() throws SQLException {
     long pId1 = insertRootComponent("p1", false);
-    insertGroupPermission(ROLE_USER, pId1, random.nextInt(30));
+    insertGroupPermission(ROLE_CODEVIEWER, pId1, null);
+    insertUserPermission("foo", pId1, random.nextInt(10));
 
     underTest.execute();
 
@@ -98,9 +108,53 @@ public class MakeComponentsPrivateBasedOnPermissionsTest {
   }
 
   @Test
-  public void execute_keeps_project_public_if_group_AnyOne_has_permission_USER_on_it() throws SQLException {
+  public void execute_keeps_project_public_if_Anyone_has_only_codeviewer_permission_and_project_has_no_user_nor_other_group_permission() throws SQLException {
+    long pId1 = insertRootComponent("p1", false);
+    insertGroupPermission(ROLE_CODEVIEWER, pId1, null);
+
+    underTest.execute();
+
+    assertThat(isPrivate("p1")).isFalse();
+  }
+
+  @Test
+  public void execute_makes_project_private_if_Anyone_has_neither_user_nor_codeviewer_permission_and_project_has_one_other_group_permission() throws SQLException {
+    long pId1 = insertRootComponent("p1", false);
+    insertGroupPermission(randomRole, pId1, null);
+    insertGroupPermission("foo", pId1, random.nextInt(10));
+
+    underTest.execute();
+
+    assertThat(isPrivate("p1")).isTrue();
+  }
+
+  @Test
+  public void execute_makes_project_private_if_Anyone_has_neither_user_nor_codeviewer_permission_and_project_has_one_user_permission() throws SQLException {
+    long pId1 = insertRootComponent("p1", false);
+    insertGroupPermission(randomRole, pId1, null);
+    insertUserPermission("foo", pId1, random.nextInt(10));
+
+    underTest.execute();
+
+    assertThat(isPrivate("p1")).isTrue();
+  }
+
+  @Test
+  public void execute_keeps_project_public_if_Anyone_has_neither_user_nor_codeviewer_permission_and_project_has_no_user_nor_other_group_permission() throws SQLException {
+    long pId1 = insertRootComponent("p1", false);
+    insertGroupPermission("foo", pId1, null);
+
+    underTest.execute();
+
+    assertThat(isPrivate("p1")).isFalse();
+  }
+
+  @Test
+  public void execute_keeps_project_public_if_Anyone_has_both_user_and_codeviewer_permission_and_project_has_one_other_group_permission() throws SQLException {
     long pId1 = insertRootComponent("p1", false);
     insertGroupPermission(ROLE_USER, pId1, null);
+    insertGroupPermission(ROLE_CODEVIEWER, pId1, null);
+    insertGroupPermission("foo", pId1, random.nextInt(10));
 
     underTest.execute();
 
@@ -108,9 +162,11 @@ public class MakeComponentsPrivateBasedOnPermissionsTest {
   }
 
   @Test
-  public void execute_keeps_project_public_if_group_AnyOne_has_permission_BROWSE_on_it() throws SQLException {
+  public void execute_keeps_project_public_if_Anyone_has_both_user_and_codeviewer_permission_and_project_has_user_permission() throws SQLException {
     long pId1 = insertRootComponent("p1", false);
+    insertGroupPermission(ROLE_USER, pId1, null);
     insertGroupPermission(ROLE_CODEVIEWER, pId1, null);
+    insertUserPermission("foo", pId1, random.nextInt(10));
 
     underTest.execute();
 
@@ -118,9 +174,10 @@ public class MakeComponentsPrivateBasedOnPermissionsTest {
   }
 
   @Test
-  public void execute_keeps_project_public_if_only_group_AnyOne_has_permission_on_it() throws SQLException {
+  public void execute_keeps_project_public_if_Anyone_has_both_user_and_codeviewer_permission_and_project_has_no_user_nor_other_group_permission() throws SQLException {
     long pId1 = insertRootComponent("p1", false);
-    insertGroupPermission(randomRole, pId1, null);
+    insertGroupPermission(ROLE_USER, pId1, null);
+    insertGroupPermission(ROLE_CODEVIEWER, pId1, null);
 
     underTest.execute();
 
@@ -128,7 +185,7 @@ public class MakeComponentsPrivateBasedOnPermissionsTest {
   }
 
   @Test
-  public void execute_keeps_project_public_if_project_has_no_permission() throws SQLException {
+  public void execute_keeps_project_public_if_it_has_no_user_nor_group_permission_at_all() throws SQLException {
     insertRootComponent("p1", false);
 
     underTest.execute();
@@ -137,20 +194,23 @@ public class MakeComponentsPrivateBasedOnPermissionsTest {
   }
 
   @Test
-  public void execute_does_not_change_private_projects_to_public_when_they_actually_should_be_because_they_have_USER_or_BROWSE_on_group_Anyone() throws SQLException {
-    long p1Id = insertRootComponent("p1", true);
-    long p2Id = insertRootComponent("p2", true);
-    long p3Id = insertRootComponent("p3", true);
+  public void execute_does_not_change_private_projects_to_public_when_they_actually_should_be() throws SQLException {
+    long p1Id = insertRootComponent("p1", true); // both user and codeviewer
+    long p2Id = insertRootComponent("p2", true); // only user but no other permission
+    long p3Id = insertRootComponent("p3", true); // only codeviewer but no other permission
+    long p4Id = insertRootComponent("p4", true); // neither codeviewer nor user but no other permission
     insertGroupPermission(ROLE_CODEVIEWER, p1Id, null);
     insertGroupPermission(ROLE_USER, p1Id, null);
     insertGroupPermission(ROLE_CODEVIEWER, p2Id, null);
     insertGroupPermission(ROLE_USER, p3Id, null);
+    insertGroupPermission(randomRole, p4Id, null);
 
     underTest.execute();
 
     assertThat(isPrivate("p1")).isTrue();
     assertThat(isPrivate("p2")).isTrue();
     assertThat(isPrivate("p3")).isTrue();
+    assertThat(isPrivate("p4")).isTrue();
   }
 
   @Test