From ee8215dc526ff023754eb20faf33ef89a5fc36f9 Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Thu, 23 Jun 2016 11:01:22 -0400 Subject: [PATCH 1/4] Removed InventoryGroup roles --- awx/main/access.py | 17 ++---- awx/main/migrations/0008_v300_rbac_changes.py | 25 -------- awx/main/migrations/_rbac.py | 1 - awx/main/models/inventory.py | 19 ------ awx/main/tests/functional/test_rbac_core.py | 19 ------ .../tests/functional/test_rbac_inventory.py | 58 ++++--------------- 6 files changed, 18 insertions(+), 121 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index e96a9f3b97..e1c5345a5b 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -384,12 +384,7 @@ class HostAccess(BaseAccess): def get_queryset(self): inv_qs = Inventory.accessible_objects(self.user, 'read_role') - group_qs = Group.accessible_objects(self.user, 'read_role').exclude(inventory__in=inv_qs) - if group_qs.count(): - qs = self.model.objects.filter(Q(inventory__in=inv_qs) | Q(groups__in=group_qs)) - else: - qs = self.model.objects.filter(inventory__in=inv_qs) - + qs = self.model.objects.filter(inventory__in=inv_qs) qs = qs.select_related('created_by', 'modified_by', 'inventory', 'last_job__job_template', 'last_job_host_summary__job') @@ -397,7 +392,7 @@ class HostAccess(BaseAccess): return qs def can_read(self, obj): - return obj and any(self.user in grp.read_role for grp in obj.groups.all()) or self.user in obj.inventory.read_role + return obj and self.user in obj.inventory.read_role def can_add(self, data): if not data or 'inventory' not in data: @@ -506,9 +501,9 @@ class InventorySourceAccess(BaseAccess): def can_read(self, obj): if obj and obj.group: - return self.user in obj.group.read_role + return self.user.can_access(Group, 'read', obj.group) elif obj and obj.inventory: - return self.user in obj.inventory.read_role + return self.user.can_access(Inventory, 'read', obj.inventory) else: return False @@ -519,7 +514,7 @@ class InventorySourceAccess(BaseAccess): def can_change(self, obj, data): # Checks for admin or change permission on group. if obj and obj.group: - return self.user in obj.group.update_role + return self.user.can_access(Group, 'change', obj.group, None) # Can't change inventory sources attached to only the inventory, since # these are created automatically from the management command. else: @@ -1455,7 +1450,7 @@ class ActivityStreamAccess(BaseAccess): inventory_set = Inventory.accessible_objects(self.user, 'read_role') credential_set = Credential.accessible_objects(self.user, 'read_role') organization_set = Organization.accessible_objects(self.user, 'read_role') - group_set = Group.accessible_objects(self.user, 'read_role') + group_set = Group.objects.filter(inventory__in=inventory_set) project_set = Project.accessible_objects(self.user, 'read_role') jt_set = JobTemplate.accessible_objects(self.user, 'read_role') team_set = Team.accessible_objects(self.user, 'read_role') diff --git a/awx/main/migrations/0008_v300_rbac_changes.py b/awx/main/migrations/0008_v300_rbac_changes.py index 50ce375f5f..7e3b2ad1f1 100644 --- a/awx/main/migrations/0008_v300_rbac_changes.py +++ b/awx/main/migrations/0008_v300_rbac_changes.py @@ -166,31 +166,6 @@ class Migration(migrations.Migration): name='read_role', field=awx.main.fields.ImplicitRoleField(related_name='+', parent_role=[b'organization.auditor_role', b'organization.member_role', b'admin_role'], to='main.Role', null=b'True'), ), - migrations.AddField( - model_name='group', - name='admin_role', - field=awx.main.fields.ImplicitRoleField(related_name='+', parent_role=[b'inventory.admin_role', b'parents.admin_role'], to='main.Role', null=b'True'), - ), - migrations.AddField( - model_name='group', - name='adhoc_role', - field=awx.main.fields.ImplicitRoleField(related_name='+', parent_role=[b'inventory.adhoc_role', b'parents.adhoc_role', b'admin_role'], to='main.Role', null=b'True'), - ), - migrations.AddField( - model_name='group', - name='use_role', - field=awx.main.fields.ImplicitRoleField(related_name='+', parent_role=[b'inventory.use_role', b'parents.use_role', b'adhoc_role'], to='main.Role', null=b'True'), - ), - migrations.AddField( - model_name='group', - name='update_role', - field=awx.main.fields.ImplicitRoleField(related_name='+', parent_role=[b'inventory.update_role', b'parents.update_role', b'admin_role'], to='main.Role', null=b'True'), - ), - migrations.AddField( - model_name='group', - name='read_role', - field=awx.main.fields.ImplicitRoleField(related_name='+', parent_role=[b'inventory.read_role', b'parents.read_role', b'use_role', b'update_role', b'admin_role'], to='main.Role', null=b'True'), - ), migrations.AddField( model_name='inventory', name='admin_role', diff --git a/awx/main/migrations/_rbac.py b/awx/main/migrations/_rbac.py index ce271c8de3..cba2c598e1 100644 --- a/awx/main/migrations/_rbac.py +++ b/awx/main/migrations/_rbac.py @@ -43,7 +43,6 @@ def create_roles(apps, schema_editor): 'Organization', 'Team', 'Inventory', - 'Group', 'Project', 'Credential', 'CustomInventoryScript', diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py index 6153a63731..af67727583 100644 --- a/awx/main/models/inventory.py +++ b/awx/main/models/inventory.py @@ -513,25 +513,6 @@ class Group(CommonModelNameNotUnique, ResourceMixin): editable=False, help_text=_('Inventory source(s) that created or modified this group.'), ) - admin_role = ImplicitRoleField( - parent_role=['inventory.admin_role', 'parents.admin_role'], - ) - update_role = ImplicitRoleField( - parent_role=['inventory.update_role', 'parents.update_role', 'admin_role'], - ) - adhoc_role = ImplicitRoleField( - parent_role=['inventory.adhoc_role', 'parents.adhoc_role', 'admin_role'], - ) - use_role = ImplicitRoleField( - parent_role=['inventory.use_role', 'parents.use_role', 'adhoc_role'], - ) - read_role = ImplicitRoleField(parent_role=[ - 'inventory.read_role', - 'parents.read_role', - 'use_role', - 'update_role', - 'admin_role' - ]) def __unicode__(self): return self.name diff --git a/awx/main/tests/functional/test_rbac_core.py b/awx/main/tests/functional/test_rbac_core.py index 9405eb8639..75c89643ad 100644 --- a/awx/main/tests/functional/test_rbac_core.py +++ b/awx/main/tests/functional/test_rbac_core.py @@ -78,25 +78,6 @@ def test_team_symantics(organization, team, alice): team.member_role.members.remove(alice) assert alice not in organization.auditor_role -@pytest.mark.django_db -def test_auto_m2m_adjustments(organization, inventory, group_factory, alice): - 'Ensures the auto role reparenting is working correctly through m2m maps' - g1 = group_factory(name='g1') - g1.admin_role.members.add(alice) - assert alice in g1.admin_role - g2 = group_factory(name='g2') - assert alice not in g2.admin_role - - g2.parents.add(g1) - assert alice in g2.admin_role - g2.parents.remove(g1) - assert alice not in g2.admin_role - - g1.children.add(g2) - assert alice in g2.admin_role - g1.children.remove(g2) - assert alice not in g2.admin_role - @pytest.mark.django_db def test_auto_field_adjustments(organization, inventory, team, alice): diff --git a/awx/main/tests/functional/test_rbac_inventory.py b/awx/main/tests/functional/test_rbac_inventory.py index 843b530c62..9c9ab2a1ab 100644 --- a/awx/main/tests/functional/test_rbac_inventory.py +++ b/awx/main/tests/functional/test_rbac_inventory.py @@ -130,6 +130,7 @@ def test_inventory_auditor(inventory, permissions, user, team): assert u in inventory.read_role assert u not in inventory.admin_role + @pytest.mark.django_db def test_inventory_updater(inventory, permissions, user, team): u = user('updater', False) @@ -173,29 +174,6 @@ def test_inventory_executor(inventory, permissions, user, team): assert team.member_role.is_ancestor_of(inventory.update_role) is False assert team.member_role.is_ancestor_of(inventory.use_role) -@pytest.mark.django_db -def test_group_parent_admin(group_factory, permissions, user): - u = user('admin', False) - parent1 = group_factory('parent-1') - parent2 = group_factory('parent-2') - childA = group_factory('child-1') - - parent1.admin_role.members.add(u) - assert u in parent1.admin_role - assert u not in parent2.admin_role - assert u not in childA.admin_role - - childA.parents.add(parent1) - assert u in childA.admin_role - - childA.parents.remove(parent1) - assert u not in childA.admin_role - - parent2.children.add(childA) - assert u not in childA.admin_role - - parent2.admin_role.members.add(u) - assert u in childA.admin_role @pytest.mark.django_db def test_access_admin(organization, inventory, user): @@ -214,6 +192,7 @@ def test_access_admin(organization, inventory, user): assert access.can_delete(inventory) assert access.can_run_ad_hoc_commands(inventory) + @pytest.mark.django_db def test_access_auditor(organization, inventory, user): u = user('admin', False) @@ -234,42 +213,29 @@ def test_access_auditor(organization, inventory, user): @pytest.mark.django_db -def test_host_access(organization, inventory, user, group_factory): +def test_host_access(organization, inventory, group, user, group_factory): other_inventory = organization.inventories.create(name='other-inventory') inventory_admin = user('inventory_admin', False) - my_group = group_factory('my-group') - not_my_group = group_factory('not-my-group') - group_admin = user('group_admin', False) inventory_admin_access = HostAccess(inventory_admin) - group_admin_access = HostAccess(group_admin) - h1 = Host.objects.create(inventory=inventory, name='host1') - h2 = Host.objects.create(inventory=inventory, name='host2') - h1.groups.add(my_group) - h2.groups.add(not_my_group) + host = Host.objects.create(inventory=inventory, name='host1') + host.groups.add(group) - assert inventory_admin_access.can_read(h1) is False - assert group_admin_access.can_read(h1) is False + assert inventory_admin_access.can_read(host) is False inventory.admin_role.members.add(inventory_admin) - my_group.admin_role.members.add(group_admin) - assert inventory_admin_access.can_read(h1) - assert inventory_admin_access.can_read(h2) - assert group_admin_access.can_read(h1) - assert group_admin_access.can_read(h2) is False + assert inventory_admin_access.can_read(host) - my_group.hosts.remove(h1) + group.hosts.remove(host) - assert inventory_admin_access.can_read(h1) - assert group_admin_access.can_read(h1) is False + assert inventory_admin_access.can_read(host) - h1.inventory = other_inventory - h1.save() + host.inventory = other_inventory + host.save() - assert inventory_admin_access.can_read(h1) is False - assert group_admin_access.can_read(h1) is False + assert inventory_admin_access.can_read(host) is False From 7aa31bb51b7b08f9e3954cfb605134c6b181f23f Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Thu, 23 Jun 2016 11:15:08 -0400 Subject: [PATCH 2/4] Updated old test case to allow org admins to create inventory scripts This was added in https://github.com/ansible/ansible-tower/commit/e9fe45389d7287d1b817d10389367214ab440690 --- awx/main/tests/old/inventory.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/awx/main/tests/old/inventory.py b/awx/main/tests/old/inventory.py index 301f6e4e9e..9c4dee8294 100644 --- a/awx/main/tests/old/inventory.py +++ b/awx/main/tests/old/inventory.py @@ -285,10 +285,10 @@ class InventoryTest(BaseTest): got = self.get(inventory_scripts, expect=200, auth=self.get_super_credentials()) self.assertEquals(got['count'], 1) - new_failed_script = dict(name="Shouldfail", description="This test should fail", script=TEST_SIMPLE_INVENTORY_SCRIPT, organization=self.organizations[0].id) - self.post(inventory_scripts, data=new_failed_script, expect=403, auth=self.get_normal_credentials()) + new_failed_script = dict(name="Should not fail", description="This test should not fail", script=TEST_SIMPLE_INVENTORY_SCRIPT, organization=self.organizations[0].id) + self.post(inventory_scripts, data=new_failed_script, expect=201, auth=self.get_normal_credentials()) - failed_no_shebang = dict(name="ShouldAlsoFail", descript="This test should also fail", script=TEST_SIMPLE_INVENTORY_SCRIPT_WITHOUT_HASHBANG, + failed_no_shebang = dict(name="ShouldFail", descript="This test should fail", script=TEST_SIMPLE_INVENTORY_SCRIPT_WITHOUT_HASHBANG, organization=self.organizations[0].id) self.post(inventory_scripts, data=failed_no_shebang, expect=400, auth=self.get_super_credentials()) From 392a7dab0d1c2e5decc73d96a0ab35e9d951efca Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Thu, 23 Jun 2016 11:50:14 -0400 Subject: [PATCH 3/4] flake8 --- awx/api/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index a172f1d4e9..2f2af5e6fb 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1279,7 +1279,7 @@ class CustomInventoryScriptSerializer(BaseSerializer): if obj is None: return ret request = self.context.get('request', None) - if not request.user in obj.admin_role: + if request.user not in obj.admin_role: ret['script'] = None return ret From f6ebf80ebaaf5d6f04e9469093155297f085a286 Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Thu, 23 Jun 2016 11:56:48 -0400 Subject: [PATCH 4/4] Fixed Group queryset --- awx/main/access.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/access.py b/awx/main/access.py index e1c5345a5b..ddc4fe0716 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -439,7 +439,7 @@ class GroupAccess(BaseAccess): model = Group def get_queryset(self): - qs = self.model.accessible_objects(self.user, 'read_role') + qs = Group.objects.filter(inventory__in=Inventory.accessible_objects(self.user, 'read_role')) qs = qs.select_related('created_by', 'modified_by', 'inventory') return qs.prefetch_related('parents', 'children', 'inventory_source').all()