diff --git a/awx/api/views/__init__.py b/awx/api/views/__init__.py index ce2810129a..9bc8bad286 100644 --- a/awx/api/views/__init__.py +++ b/awx/api/views/__init__.py @@ -797,7 +797,7 @@ class TeamProjectsList(SubListAPIView): role = ObjectRole.objects.filter(object_id=team.id, content_type=parent_ct, role_definition=rd).first() if role is None: # Team has no permissions, therefore team has no projects - return self.model.none() + return self.model.objects.none() else: project_qs = self.model.accessible_objects(self.request.user, 'read_role') return project_qs.filter(id__in=RoleEvaluation.objects.filter(content_type_id=model_ct.id, role=role).values_list('object_id')) diff --git a/awx/main/models/rbac.py b/awx/main/models/rbac.py index ff38686efb..a137c381fd 100644 --- a/awx/main/models/rbac.py +++ b/awx/main/models/rbac.py @@ -665,8 +665,6 @@ def sync_parents_to_new_rbac(instance, action, model, pk_set, reverse, **kwargs) elif action == 'post_clear': raise RuntimeError('Clearing of role members not supported') - from awx.main.models.organization import Team - if reverse: parent_role = instance else: @@ -680,15 +678,17 @@ def sync_parents_to_new_rbac(instance, action, model, pk_set, reverse, **kwargs) # To a fault, we want to avoid running this if triggered from implicit_parents management # we only want to do anything if we know for sure this is a non-implicit team role - if parent_role.role_field not in ('member_role', 'admin_role') or parent_role.content_type.model != 'team': - return + if parent_role.role_field == 'member_role' and parent_role.content_type.model == 'team': + # Team internal parents are member_role->read_role and admin_role->member_role + # for the same object, this parenting will also be implicit_parents management + # do nothing for internal parents, but OTHER teams may still be assigned permissions to a team + if (child_role.content_type_id == parent_role.content_type_id) and (child_role.object_id == parent_role.object_id): + return - # Team member role is a parent of its read role so we want to avoid this - if child_role.role_field == 'read_role' and child_role.content_type.model == 'team': - return + from awx.main.models.organization import Team - team = Team.objects.get(pk=parent_role.object_id) - give_or_remove_permission(child_role, team, giving=is_giving) + team = Team.objects.get(pk=parent_role.object_id) + give_or_remove_permission(child_role, team, giving=is_giving) m2m_changed.connect(sync_members_to_new_rbac, Role.members.through) diff --git a/awx/main/tests/functional/dab_rbac/test_translation_layer.py b/awx/main/tests/functional/dab_rbac/test_translation_layer.py index 18a1db4028..0d2c2c7245 100644 --- a/awx/main/tests/functional/dab_rbac/test_translation_layer.py +++ b/awx/main/tests/functional/dab_rbac/test_translation_layer.py @@ -1,7 +1,9 @@ +from unittest import mock + import pytest from awx.main.models.rbac import get_role_from_object_role, give_creator_permissions -from awx.main.models import User, Organization, WorkflowJobTemplate, WorkflowJobTemplateNode +from awx.main.models import User, Organization, WorkflowJobTemplate, WorkflowJobTemplateNode, Team from awx.api.versioning import reverse from ansible_base.rbac.models import RoleUserAssignment @@ -81,3 +83,25 @@ def test_creator_permission(rando, admin_user, inventory): give_creator_permissions(rando, inventory) assert rando in inventory.admin_role assert rando in inventory.admin_role.members.all() + + +@pytest.mark.django_db +def test_team_team_read_role(rando, team, admin_user, post): + orgs = [Organization.objects.create(name=f'foo-{i}') for i in range(2)] + teams = [Team.objects.create(name=f'foo-{i}', organization=orgs[i]) for i in range(2)] + teams[1].member_role.members.add(rando) + + # give second team read permission to first team through the API for regression testing + url = reverse('api:role_teams_list', kwargs={'pk': teams[0].read_role.pk, 'version': 'v2'}) + post(url, {'id': teams[1].id}, user=admin_user) + + # user should be able to view the first team + assert rando in teams[0].read_role + + +@pytest.mark.django_db +def test_implicit_parents_no_assignments(organization): + """Through the normal course of creating models, we should not be changing DAB RBAC permissions""" + with mock.patch('awx.main.models.rbac.give_or_remove_permission') as mck: + Team.objects.create(name='random team', organization=organization) + mck.assert_not_called() diff --git a/awx/main/tests/functional/test_fixture_factories.py b/awx/main/tests/functional/test_fixture_factories.py index 1af7b66246..5792197177 100644 --- a/awx/main/tests/functional/test_fixture_factories.py +++ b/awx/main/tests/functional/test_fixture_factories.py @@ -50,13 +50,13 @@ def test_org_factory_roles(organization_factory): teams=['team1', 'team2'], users=['team1:foo', 'bar'], projects=['baz', 'bang'], - roles=['team2.member_role:foo', 'team1.admin_role:bar', 'team1.admin_role:team2.admin_role', 'baz.admin_role:foo'], + roles=['team2.member_role:foo', 'team1.admin_role:bar', 'team1.member_role:team2.admin_role', 'baz.admin_role:foo'], ) assert objects.users.bar in objects.teams.team2.admin_role assert objects.users.foo in objects.projects.baz.admin_role assert objects.users.foo in objects.teams.team1.member_role - assert objects.teams.team2.admin_role in objects.teams.team1.admin_role.children.all() + assert objects.teams.team2.admin_role in objects.teams.team1.member_role.children.all() @pytest.mark.django_db