From b8a687d4de61a78ba5529ad5554b7a10b10dc674 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Mon, 26 Jun 2017 17:02:30 +0200 Subject: [PATCH] SONAR-9444 make private at least one missing perm user or codeviewer --- ...keComponentsPrivateBasedOnPermissions.java | 27 +++-- ...mponentsPrivateBasedOnPermissionsTest.java | 106 ++++++++++++++---- 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 -- 2.39.5