diff --git a/awx/main/models/rbac.py b/awx/main/models/rbac.py index 3ceec36d17..b7f70aafec 100644 --- a/awx/main/models/rbac.py +++ b/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): diff --git a/awx/main/tests/functional/conftest.py b/awx/main/tests/functional/conftest.py index 01071ba947..e0d793d190 100644 --- a/awx/main/tests/functional/conftest.py +++ b/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 diff --git a/awx/main/tests/functional/test_rbac_role.py b/awx/main/tests/functional/test_rbac_role.py index 16ad46f8db..96484f9fee 100644 --- a/awx/main/tests/functional/test_rbac_role.py +++ b/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)