Fix issue when permission is set only on user

6 files changed, 99 insertions, 63 deletions

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 @@
       <foreach collection="componentKeys" open="(" close=")" item="element" index="index" separator=" or " >p.kee=#{element}</foreach>
     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 @@
     <foreach collection="componentIds" open="(" close=")" item="element" index="index" separator=" or ">gr.resource_id=#{element}</foreach>
     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<Long> 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,
@@ -73,13 +76,61 @@ public class AuthorizationDaoTest extends AbstractDaoTestCase {
   }
 
   @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.<Long>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.<Long>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.<Long>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<Long> 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<Long> 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<Long> 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<String> 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<String> 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<String> 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<String> 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<String> 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<String> 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 @@
-<dataset>
-
-  <!-- user 100 has no direct grant access, but is in the group 200 that has the role "user"
-  on the all the projects  -->
-  <user_roles id="1" user_id="100" resource_id="999" role="user"/>
-  <groups_users user_id="100" group_id="200"/>
-  <group_roles id="1" group_id="200" resource_id="[null]" role="user"/>
-
-  <projects id="301" kee="pj-w-snapshot:package" root_id="300" uuid="ABCD" module_uuid="DEFG"/>
-  <projects id="302" kee="pj-w-snapshot:file" root_id="300" uuid="BCDE" module_uuid="DEFG"/>
-  <projects id="303" kee="pj-w-snapshot:other" root_id="300" uuid="CDEF" module_uuid="DEFG"/>
-  <projects id="300" kee="pj-w-snapshot" uuid="DEFG" module_uuid="[null]"/>
-  <projects id="400" kee="pj-wo-snapshot" uuid="EFGH" module_uuid="[null]"/>
-
-</dataset>
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 @@
+<dataset>
+
+  <groups_users user_id="100" group_id="200"/>
+  <group_roles id="1" group_id="[null]" resource_id="300" role="user"/>
+  <group_roles id="2" group_id="200" resource_id="400" role="codeviewer"/>
+
+  <projects id="300" kee="pj-w-snapshot" uuid="DEFG" module_uuid="[null]" enabled="[true]"/>
+  <projects id="400" kee="pj-wo-snapshot" uuid="EFGH" module_uuid="[null]" enabled="[true]"/>
+
+</dataset>
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 @@
+<dataset>
+
+  <groups_users user_id="100" group_id="200"/>
+  <group_roles id="1" group_id="200" resource_id="300" role="user"/>
+  <group_roles id="2" group_id="200" resource_id="400" role="codeviewer"/>
+
+  <projects id="300" kee="pj-w-snapshot" uuid="DEFG" module_uuid="[null]" enabled="[true]"/>
+  <projects id="400" kee="pj-wo-snapshot" uuid="EFGH" module_uuid="[null]" enabled="[true]"/>
+
+</dataset>
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 @@
+<dataset>
+
+  <!-- user 100 has the role "user" on the project 300 -->
+  <user_roles id="1" user_id="100" resource_id="300" role="user"/>
+  <user_roles id="2" user_id="100" resource_id="400" role="codeviewer"/>
+
+  <projects id="300" kee="pj-w-snapshot" uuid="DEFG" module_uuid="[null]" enabled="[true]"/>
+  <projects id="400" kee="pj-wo-snapshot" uuid="EFGH" module_uuid="[null]" enabled="[true]"/>
+
+</dataset>