diff --git a/awx/main/access.py b/awx/main/access.py index 15af5b2ff9..e8607f6f74 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: @@ -444,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() @@ -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: @@ -1480,7 +1475,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 64f3b4146e..287919ad31 100644 --- a/awx/main/tests/functional/test_rbac_inventory.py +++ b/awx/main/tests/functional/test_rbac_inventory.py @@ -134,6 +134,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) @@ -177,29 +178,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): @@ -218,6 +196,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) @@ -242,42 +221,29 @@ def test_inventory_update_org_admin(inventory_update, org_admin): @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 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())