Merge pull request #647 from AlanCoding/no_sql

remove raw SQL in visible_roles

awx/main/models/rbac.py

@@ -372,48 +372,26 @@ class Role(models.Model):
 
 
     @staticmethod
-    @check_singleton
     def visible_roles(user):
-        sql_params = {
-            'ancestors_table': Role.ancestors.through._meta.db_table,
-            'parents_table': Role.parents.through._meta.db_table,
-            'roles_table': Role._meta.db_table,
-            'ids': ','.join(str(x) for x in user.roles.values_list('id', flat=True)),
-        }
-
-        qs = Role.objects.extra(
-            where = ['''
-                    %(roles_table)s.id IN (
-                        SELECT DISTINCT visible_roles_t2.ancestor_id
-                          FROM %(ancestors_table)s as visible_roles_t1
-                               LEFT JOIN %(ancestors_table)s as visible_roles_t2 ON (visible_roles_t1.descendent_id = visible_roles_t2.descendent_id)
-                         WHERE visible_roles_t1.ancestor_id IN (%(ids)s)
-                    )
-                    ''' % sql_params]
-        )
-        return qs
+        return Role.filter_visible_roles(user, Role.objects.all())
 
     @staticmethod
     @check_singleton
     def filter_visible_roles(user, roles_qs):
-        sql_params = {
-            'ancestors_table': Role.ancestors.through._meta.db_table,
-            'parents_table': Role.parents.through._meta.db_table,
-            'roles_table': Role._meta.db_table,
-            'ids': ','.join(str(x) for x in user.roles.all().values_list('id', flat=True))
-        }
-
-        qs = roles_qs.extra(
-            where = ['''
-                EXISTS (
-                    SELECT 1
-                      FROM %(ancestors_table)s as visible_roles_t1
-                           LEFT JOIN %(ancestors_table)s as visible_roles_t2 ON (visible_roles_t1.descendent_id = visible_roles_t2.descendent_id)
-                     WHERE visible_roles_t1.ancestor_id = %(roles_table)s.id
-                           AND visible_roles_t2.ancestor_id IN (%(ids)s)
-                ) ''' % sql_params]
+        '''
+        Visible roles include all roles that are ancestors of any
+        roles that the user has access to.
+        Case in point - organization auditor_role must see all roles
+        in their organization, but some of those roles descend from
+        organization admin_role, but not auditor_role.
+        '''
+        return roles_qs.filter(
+            id__in=RoleAncestorEntry.objects.filter(
+                descendent__in=RoleAncestorEntry.objects.filter(
+                    ancestor_id__in=list(user.roles.values_list('id', flat=True))
+                ).values_list('descendent', flat=True)
+            ).distinct().values_list('ancestor', flat=True)
         )
-        return qs
 
     @staticmethod
     def singleton(name):

awx/main/tests/functional/conftest.py

@@ -378,7 +378,7 @@ def admin(user):
 
 @pytest.fixture
 def system_auditor(user):
-    u = user(False)
+    u = user('an-auditor', False)
     Role.singleton('system_auditor').members.add(u)
     return u
 

awx/main/tests/functional/test_rbac_role.py

@@ -4,6 +4,7 @@ from awx.main.access import (
     RoleAccess,
     UserAccess,
     TeamAccess)
+from awx.main.models import Role
 
 
 @pytest.mark.django_db
@@ -32,3 +33,20 @@ def test_role_access_attach(rando, inventory):
     inventory.read_role.members.add(rando)
     access = RoleAccess(rando)
     assert not access.can_attach(inventory.admin_role, rando, 'members', None)
+
+
+@pytest.mark.django_db
+def test_visible_roles(admin_user, system_auditor, rando, organization, project):
+    '''
+    system admin & system auditor fixtures needed to create system roles
+    '''
+    organization.auditor_role.members.add(rando)
+    access = RoleAccess(rando)
+
+    assert rando not in organization.admin_role
+    assert access.can_read(organization.admin_role)
+    assert organization.admin_role in Role.visible_roles(rando)
+
+    assert rando not in project.admin_role
+    assert access.can_read(project.admin_role)
+    assert project.admin_role in Role.visible_roles(rando)