From e818daa74f4ca9004176024c7ac042a570a7f10f Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Tue, 5 Jul 2016 14:01:46 -0400 Subject: [PATCH 1/5] Ensure system auditors/admins can see all roles Partial fix for #2744 --- awx/main/models/rbac.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/models/rbac.py b/awx/main/models/rbac.py index 697d51f1e2..633c60c175 100644 --- a/awx/main/models/rbac.py +++ b/awx/main/models/rbac.py @@ -75,7 +75,7 @@ def check_singleton(func): if user in sys_admin or user in sys_audit: if len(args) == 2: return args[1] - return user.roles.all() + return Roles.objects.all() return func(*args, **kwargs) return wrapper From a126736332383306f68ce28e9eae503f517ecf06 Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Tue, 5 Jul 2016 15:06:25 -0400 Subject: [PATCH 2/5] Expand role visibility such that you can always see all roles on any objects you can see Completes #2774 --- awx/main/models/rbac.py | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/awx/main/models/rbac.py b/awx/main/models/rbac.py index 633c60c175..b16a777a77 100644 --- a/awx/main/models/rbac.py +++ b/awx/main/models/rbac.py @@ -382,9 +382,10 @@ class Role(models.Model): qs = Role.objects.extra( where = [''' %(roles_table)s.id IN ( - SELECT descendent_id FROM %(ancestors_table)s WHERE ancestor_id IN (%(ids)s) - UNION - SELECT ancestor_id FROM %(ancestors_table)s WHERE descendent_id IN (%(ids)s) + SELECT t2.ancestor_id + FROM %(ancestors_table)s as t1 + LEFT JOIN %(ancestors_table)s as t2 ON (t1.descendent_id = t2.descendent_id) + WHERE t1.ancestor_id IN (%(ids)s) ) ''' % sql_params] ) @@ -393,23 +394,7 @@ class Role(models.Model): @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 - WHERE (descendent_id = %(roles_table)s.id AND ancestor_id IN (%(ids)s)) - OR (ancestor_id = %(roles_table)s.id AND descendent_id IN (%(ids)s)) - ) ''' % sql_params] - ) - return qs + return roles_qs.filter(id__in=Role.visible_roles(user)) @staticmethod def singleton(name): From b94902d9701309115ee703983a64947b2d32d370 Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Wed, 6 Jul 2016 12:20:52 -0400 Subject: [PATCH 3/5] filter_visible_roles performance enhancement --- awx/main/models/rbac.py | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/awx/main/models/rbac.py b/awx/main/models/rbac.py index b16a777a77..b60228d1dc 100644 --- a/awx/main/models/rbac.py +++ b/awx/main/models/rbac.py @@ -382,10 +382,10 @@ class Role(models.Model): qs = Role.objects.extra( where = [''' %(roles_table)s.id IN ( - SELECT t2.ancestor_id - FROM %(ancestors_table)s as t1 - LEFT JOIN %(ancestors_table)s as t2 ON (t1.descendent_id = t2.descendent_id) - WHERE t1.ancestor_id IN (%(ids)s) + 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] ) @@ -394,7 +394,24 @@ class Role(models.Model): @staticmethod @check_singleton def filter_visible_roles(user, roles_qs): - return roles_qs.filter(id__in=Role.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.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] + ) + return qs @staticmethod def singleton(name): From b4810f6486dd26735a142df4288595c7a4cdca5b Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Wed, 6 Jul 2016 13:38:43 -0400 Subject: [PATCH 4/5] Typo --- awx/main/models/rbac.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/models/rbac.py b/awx/main/models/rbac.py index b60228d1dc..5e040b85a1 100644 --- a/awx/main/models/rbac.py +++ b/awx/main/models/rbac.py @@ -75,7 +75,7 @@ def check_singleton(func): if user in sys_admin or user in sys_audit: if len(args) == 2: return args[1] - return Roles.objects.all() + return Role.objects.all() return func(*args, **kwargs) return wrapper From 015085fe15628b7c9ae0bcea075a37384116f54e Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Wed, 6 Jul 2016 13:38:52 -0400 Subject: [PATCH 5/5] Added role visibility tests --- awx/main/tests/functional/test_rbac_api.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/awx/main/tests/functional/test_rbac_api.py b/awx/main/tests/functional/test_rbac_api.py index e6f5959355..d5ccbdf5d0 100644 --- a/awx/main/tests/functional/test_rbac_api.py +++ b/awx/main/tests/functional/test_rbac_api.py @@ -57,6 +57,28 @@ def test_get_roles_list_user(organization, inventory, team, get, user): assert inventory.admin_role.id not in role_hash assert team.member_role.id not in role_hash +@pytest.mark.django_db +def test_roles_visibility(get, organization, project, admin, alice, bob): + Role.singleton('system_auditor').members.add(alice) + assert get(reverse('api:role_list') + '?id=%d' % project.update_role.id, user=admin).data['count'] == 1 + assert get(reverse('api:role_list') + '?id=%d' % project.update_role.id, user=alice).data['count'] == 1 + assert get(reverse('api:role_list') + '?id=%d' % project.update_role.id, user=bob).data['count'] == 0 + organization.auditor_role.members.add(bob) + assert get(reverse('api:role_list') + '?id=%d' % project.update_role.id, user=bob).data['count'] == 1 + +@pytest.mark.django_db +def test_roles_filter_visibility(get, organization, project, admin, alice, bob): + Role.singleton('system_auditor').members.add(alice) + project.update_role.members.add(admin) + + assert get(reverse('api:user_roles_list', args=(admin.id,)) + '?id=%d' % project.update_role.id, user=admin).data['count'] == 1 + assert get(reverse('api:user_roles_list', args=(admin.id,)) + '?id=%d' % project.update_role.id, user=alice).data['count'] == 1 + assert get(reverse('api:user_roles_list', args=(admin.id,)) + '?id=%d' % project.update_role.id, user=bob).data['count'] == 0 + organization.auditor_role.members.add(bob) + assert get(reverse('api:user_roles_list', args=(admin.id,)) + '?id=%d' % project.update_role.id, user=bob).data['count'] == 1 + organization.auditor_role.members.remove(bob) + project.use_role.members.add(bob) # sibling role should still grant visibility + assert get(reverse('api:user_roles_list', args=(admin.id,)) + '?id=%d' % project.update_role.id, user=bob).data['count'] == 1 @pytest.mark.django_db def test_cant_create_role(post, admin):