From 4e3fb97d0434535d83ee1f9df97a6d1e657fe9d4 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Wed, 1 Apr 2015 17:28:45 +0200 Subject: [PATCH] Fix issue when permission is set only on user --- .../sonar/core/user/AuthorizationMapper.xml | 6 +- .../sonar/core/user/AuthorizationDaoTest.java | 111 ++++++++++-------- ...group_should_have_global_authorization.xml | 15 --- ...p_authorized_project_ids_for_anonymous.xml | 10 ++ .../keep_authorized_project_ids_for_group.xml | 10 ++ .../keep_authorized_project_ids_for_user.xml | 10 ++ 6 files changed, 99 insertions(+), 63 deletions(-) delete mode 100644 sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/group_should_have_global_authorization.xml create mode 100644 sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/keep_authorized_project_ids_for_anonymous.xml create mode 100644 sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/keep_authorized_project_ids_for_group.xml create mode 100644 sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/keep_authorized_project_ids_for_user.xml diff --git a/sonar-core/src/main/resources/org/sonar/core/user/AuthorizationMapper.xml b/sonar-core/src/main/resources/org/sonar/core/user/AuthorizationMapper.xml index 1e757eaf58b..52601598415 100644 --- a/sonar-core/src/main/resources/org/sonar/core/user/AuthorizationMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/user/AuthorizationMapper.xml @@ -13,7 +13,8 @@ p.kee=#{element} UNION SELECT p.kee - FROM user_roles ur, projects p + FROM user_roles ur + INNER JOIN projects p on p.id = ur.resource_id WHERE ur.role=#{role} and ur.user_id=#{userId} and @@ -40,7 +41,8 @@ gr.resource_id=#{element} UNION SELECT p.id - FROM user_roles ur, projects p + FROM user_roles ur + INNER JOIN projects p on p.id = ur.resource_id WHERE ur.role=#{role} and ur.user_id=#{userId} and diff --git a/sonar-core/src/test/java/org/sonar/core/user/AuthorizationDaoTest.java b/sonar-core/src/test/java/org/sonar/core/user/AuthorizationDaoTest.java index 605cfc756e2..3389d1f6204 100644 --- a/sonar-core/src/test/java/org/sonar/core/user/AuthorizationDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/user/AuthorizationDaoTest.java @@ -34,14 +34,18 @@ import static org.assertj.core.api.Assertions.assertThat; public class AuthorizationDaoTest extends AbstractDaoTestCase { private static final int USER = 100; - private static final Long PROJECT_ID = 300L, EMPTY_PROJECT_ID = 400L; + private static final Long PROJECT_ID = 300L, PROJECT_ID_WITHOUT_SNAPSHOT = 400L; private static final String PROJECT = "pj-w-snapshot"; + private static final String PROJECT_WIHOUT_SNAPSHOT = "pj-wo-snapshot"; DbSession session; + AuthorizationDao authorization; + @Before public void setUp() throws Exception { session = getMyBatis().openSession(false); + authorization = new AuthorizationDao(getMyBatis()); } @After @@ -54,12 +58,11 @@ public class AuthorizationDaoTest extends AbstractDaoTestCase { // but user is not in an authorized group setupData("user_should_be_authorized"); - AuthorizationDao authorization = new AuthorizationDao(getMyBatis()); Collection componentIds = authorization.keepAuthorizedProjectIds(session, - Sets.newHashSet(PROJECT_ID, EMPTY_PROJECT_ID), + Sets.newHashSet(PROJECT_ID, PROJECT_ID_WITHOUT_SNAPSHOT), USER, "user"); - assertThat(componentIds).containsOnly(PROJECT_ID, EMPTY_PROJECT_ID); + assertThat(componentIds).containsOnly(PROJECT_ID, PROJECT_ID_WITHOUT_SNAPSHOT); // user does not have the role "admin" componentIds = authorization.keepAuthorizedProjectIds(session, @@ -72,14 +75,62 @@ public class AuthorizationDaoTest extends AbstractDaoTestCase { USER, "admin")).isEmpty(); } + @Test + public void keep_authorized_project_ids_for_user() { + setupData("keep_authorized_project_ids_for_user"); + + assertThat(authorization.keepAuthorizedProjectIds(session, Sets.newHashSet(PROJECT_ID, PROJECT_ID_WITHOUT_SNAPSHOT), USER, "user")).containsOnly(PROJECT_ID); + + // user does not have the role "admin" + assertThat(authorization.keepAuthorizedProjectIds(session, Sets.newHashSet(PROJECT_ID), USER, "admin")).isEmpty(); + + // Empty list + assertThat(authorization.keepAuthorizedProjectIds(session, Collections.emptySet(), USER, "admin")).isEmpty(); + } + + @Test + public void keep_authorized_project_ids_for_group() { + setupData("keep_authorized_project_ids_for_group"); + + assertThat(authorization.keepAuthorizedProjectIds(session, Sets.newHashSet(PROJECT_ID, PROJECT_ID_WITHOUT_SNAPSHOT), USER, "user")).containsOnly(PROJECT_ID); + + // user does not have the role "admin" + assertThat(authorization.keepAuthorizedProjectIds(session, Sets.newHashSet(PROJECT_ID), USER, "admin")).isEmpty(); + + // Empty list + assertThat(authorization.keepAuthorizedProjectIds(session, Collections.emptySet(), USER, "admin")).isEmpty(); + } + + @Test + public void keep_authorized_project_ids_for_anonymous() { + setupData("keep_authorized_project_ids_for_anonymous"); + + assertThat(authorization.keepAuthorizedProjectIds(session, Sets.newHashSet(PROJECT_ID, PROJECT_ID_WITHOUT_SNAPSHOT), null, "user")).containsOnly(PROJECT_ID); + + // user does not have the role "admin" + assertThat(authorization.keepAuthorizedProjectIds(session, Sets.newHashSet(PROJECT_ID), null, "admin")).isEmpty(); + + // Empty list + assertThat(authorization.keepAuthorizedProjectIds(session, Collections.emptySet(), null, "admin")).isEmpty(); + } + @Test public void is_authorized_component_key_for_user() { - // but user is not in an authorized group - setupData("user_should_be_authorized"); + setupData("keep_authorized_project_ids_for_user"); + + assertThat(authorization.isAuthorizedComponentKey(PROJECT, USER, "user")).isTrue(); + assertThat(authorization.isAuthorizedComponentKey(PROJECT_WIHOUT_SNAPSHOT, USER, "user")).isFalse(); - AuthorizationDao authorization = new AuthorizationDao(getMyBatis()); + // user does not have the role "admin" + assertThat(authorization.isAuthorizedComponentKey(PROJECT, USER, "admin")).isFalse(); + } + + @Test + public void is_authorized_component_key_for_group() { + setupData("keep_authorized_project_ids_for_group"); assertThat(authorization.isAuthorizedComponentKey(PROJECT, USER, "user")).isTrue(); + assertThat(authorization.isAuthorizedComponentKey(PROJECT_WIHOUT_SNAPSHOT, USER, "user")).isFalse(); // user does not have the role "admin" assertThat(authorization.isAuthorizedComponentKey(PROJECT, USER, "admin")).isFalse(); @@ -87,11 +138,10 @@ public class AuthorizationDaoTest extends AbstractDaoTestCase { @Test public void is_authorized_component_key_for_anonymous() { - setupData("anonymous_should_be_authorized"); - - AuthorizationDao authorization = new AuthorizationDao(getMyBatis()); + setupData("keep_authorized_project_ids_for_anonymous"); assertThat(authorization.isAuthorizedComponentKey(PROJECT, null, "user")).isTrue(); + assertThat(authorization.isAuthorizedComponentKey(PROJECT_WIHOUT_SNAPSHOT, null, "user")).isFalse(); assertThat(authorization.isAuthorizedComponentKey(PROJECT, null, "admin")).isFalse(); } @@ -100,35 +150,15 @@ public class AuthorizationDaoTest extends AbstractDaoTestCase { // user is in an authorized group setupData("group_should_be_authorized"); - AuthorizationDao authorization = new AuthorizationDao(getMyBatis()); - Collection componentIds = authorization.keepAuthorizedProjectIds(session, - Sets.newHashSet(PROJECT_ID, EMPTY_PROJECT_ID), - USER, "user"); - - assertThat(componentIds).containsOnly(PROJECT_ID, EMPTY_PROJECT_ID); - - // group does not have the role "admin" - componentIds = authorization.keepAuthorizedProjectIds(session, - Sets.newHashSet(PROJECT_ID, EMPTY_PROJECT_ID), - USER, "admin"); - assertThat(componentIds).isEmpty(); - } - - @Test - public void group_should_have_global_authorization() { - // user is in a group that has authorized access to all projects - setupData("group_should_have_global_authorization"); - - AuthorizationDao authorization = new AuthorizationDao(getMyBatis()); Collection componentIds = authorization.keepAuthorizedProjectIds(session, - Sets.newHashSet(PROJECT_ID, EMPTY_PROJECT_ID), + Sets.newHashSet(PROJECT_ID, PROJECT_ID_WITHOUT_SNAPSHOT), USER, "user"); - assertThat(componentIds).containsOnly(PROJECT_ID, EMPTY_PROJECT_ID); + assertThat(componentIds).containsOnly(PROJECT_ID, PROJECT_ID_WITHOUT_SNAPSHOT); // group does not have the role "admin" componentIds = authorization.keepAuthorizedProjectIds(session, - Sets.newHashSet(PROJECT_ID, EMPTY_PROJECT_ID), + Sets.newHashSet(PROJECT_ID, PROJECT_ID_WITHOUT_SNAPSHOT), USER, "admin"); assertThat(componentIds).isEmpty(); } @@ -137,12 +167,11 @@ public class AuthorizationDaoTest extends AbstractDaoTestCase { public void anonymous_should_be_authorized() { setupData("anonymous_should_be_authorized"); - AuthorizationDao authorization = new AuthorizationDao(getMyBatis()); Collection componentIds = authorization.keepAuthorizedProjectIds(session, - Sets.newHashSet(PROJECT_ID, EMPTY_PROJECT_ID), + Sets.newHashSet(PROJECT_ID, PROJECT_ID_WITHOUT_SNAPSHOT), null, "user"); - assertThat(componentIds).containsOnly(PROJECT_ID, EMPTY_PROJECT_ID); + assertThat(componentIds).containsOnly(PROJECT_ID, PROJECT_ID_WITHOUT_SNAPSHOT); // group does not have the role "admin" componentIds = authorization.keepAuthorizedProjectIds(session, @@ -155,7 +184,6 @@ public class AuthorizationDaoTest extends AbstractDaoTestCase { public void should_return_root_project_keys_for_user() { setupData("should_return_root_project_keys_for_user"); - AuthorizationDao authorization = new AuthorizationDao(getMyBatis()); Collection rootProjectIds = authorization.selectAuthorizedRootProjectsKeys(USER, "user"); assertThat(rootProjectIds).containsOnly(PROJECT); @@ -170,7 +198,6 @@ public class AuthorizationDaoTest extends AbstractDaoTestCase { // but user is not in an authorized group setupData("should_return_root_project_keys_for_group"); - AuthorizationDao authorization = new AuthorizationDao(getMyBatis()); Collection rootProjectIds = authorization.selectAuthorizedRootProjectsKeys(USER, "user"); assertThat(rootProjectIds).containsOnly(PROJECT); @@ -184,7 +211,6 @@ public class AuthorizationDaoTest extends AbstractDaoTestCase { public void should_return_root_project_keys_for_anonymous() { setupData("should_return_root_project_keys_for_anonymous"); - AuthorizationDao authorization = new AuthorizationDao(getMyBatis()); Collection rootProjectIds = authorization.selectAuthorizedRootProjectsKeys(null, "user"); assertThat(rootProjectIds).containsOnly(PROJECT); @@ -198,7 +224,6 @@ public class AuthorizationDaoTest extends AbstractDaoTestCase { public void should_return_root_project_uuids_for_user() { setupData("should_return_root_project_keys_for_user"); - AuthorizationDao authorization = new AuthorizationDao(getMyBatis()); Collection rootProjectUuids = authorization.selectAuthorizedRootProjectsUuids(USER, "user"); assertThat(rootProjectUuids).containsOnly("ABCD"); @@ -213,7 +238,6 @@ public class AuthorizationDaoTest extends AbstractDaoTestCase { // but user is not in an authorized group setupData("should_return_root_project_keys_for_group"); - AuthorizationDao authorization = new AuthorizationDao(getMyBatis()); Collection rootProjectUuids = authorization.selectAuthorizedRootProjectsUuids(USER, "user"); assertThat(rootProjectUuids).containsOnly("ABCD"); @@ -227,7 +251,6 @@ public class AuthorizationDaoTest extends AbstractDaoTestCase { public void should_return_root_project_uuids_for_anonymous() { setupData("should_return_root_project_keys_for_anonymous"); - AuthorizationDao authorization = new AuthorizationDao(getMyBatis()); Collection rootProjectUuids = authorization.selectAuthorizedRootProjectsUuids(null, "user"); assertThat(rootProjectUuids).containsOnly("ABCD"); @@ -241,7 +264,6 @@ public class AuthorizationDaoTest extends AbstractDaoTestCase { public void should_return_user_global_permissions() { setupData("should_return_user_global_permissions"); - AuthorizationDao authorization = new AuthorizationDao(getMyBatis()); assertThat(authorization.selectGlobalPermissions("john")).containsOnly("user", "admin"); assertThat(authorization.selectGlobalPermissions("arthur")).containsOnly("user"); assertThat(authorization.selectGlobalPermissions("none")).isEmpty(); @@ -251,7 +273,6 @@ public class AuthorizationDaoTest extends AbstractDaoTestCase { public void should_return_group_global_permissions() { setupData("should_return_group_global_permissions"); - AuthorizationDao authorization = new AuthorizationDao(getMyBatis()); assertThat(authorization.selectGlobalPermissions("john")).containsOnly("user", "admin"); assertThat(authorization.selectGlobalPermissions("arthur")).containsOnly("user"); assertThat(authorization.selectGlobalPermissions("none")).isEmpty(); @@ -261,7 +282,6 @@ public class AuthorizationDaoTest extends AbstractDaoTestCase { public void should_return_global_permissions_for_anonymous() { setupData("should_return_global_permissions_for_anonymous"); - AuthorizationDao authorization = new AuthorizationDao(getMyBatis()); assertThat(authorization.selectGlobalPermissions(null)).containsOnly("user", "admin"); } @@ -269,7 +289,6 @@ public class AuthorizationDaoTest extends AbstractDaoTestCase { public void should_return_global_permissions_for_group_anyone() throws Exception { setupData("should_return_global_permissions_for_group_anyone"); - AuthorizationDao authorization = new AuthorizationDao(getMyBatis()); assertThat(authorization.selectGlobalPermissions("anyone_user")).containsOnly("user", "profileadmin"); } diff --git a/sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/group_should_have_global_authorization.xml b/sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/group_should_have_global_authorization.xml deleted file mode 100644 index c5cd325ea5e..00000000000 --- a/sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/group_should_have_global_authorization.xml +++ /dev/null @@ -1,15 +0,0 @@ - - - - - - - - - - - - - - diff --git a/sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/keep_authorized_project_ids_for_anonymous.xml b/sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/keep_authorized_project_ids_for_anonymous.xml new file mode 100644 index 00000000000..1c21104a7b6 --- /dev/null +++ b/sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/keep_authorized_project_ids_for_anonymous.xml @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/keep_authorized_project_ids_for_group.xml b/sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/keep_authorized_project_ids_for_group.xml new file mode 100644 index 00000000000..17e6323ccd6 --- /dev/null +++ b/sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/keep_authorized_project_ids_for_group.xml @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/keep_authorized_project_ids_for_user.xml b/sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/keep_authorized_project_ids_for_user.xml new file mode 100644 index 00000000000..515adaa8f48 --- /dev/null +++ b/sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/keep_authorized_project_ids_for_user.xml @@ -0,0 +1,10 @@ + + + + + + + + + + -- 2.39.5