diff --git a/awx/main/access.py b/awx/main/access.py index 250ce99a66..6d615e8677 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -645,23 +645,22 @@ class UserAccess(BaseAccess): return False def can_attach(self, obj, sub_obj, relationship, *args, **kwargs): - if not settings.MANAGE_ORGANIZATION_AUTH and not self.user.is_superuser: - return False - - # Reverse obj and sub_obj, defer to RoleAccess if this is a role assignment. + # The only thing that a User should ever have attached is a Role if relationship == 'roles': role_access = RoleAccess(self.user) return role_access.can_attach(sub_obj, obj, 'members', *args, **kwargs) - return super(UserAccess, self).can_attach(obj, sub_obj, relationship, *args, **kwargs) + + logger.error('Unexpected attempt to associate {} with a user.'.format(sub_obj)) + return False def can_unattach(self, obj, sub_obj, relationship, *args, **kwargs): - if not settings.MANAGE_ORGANIZATION_AUTH and not self.user.is_superuser: - return False - + # The only thing that a User should ever have to be unattached is a Role if relationship == 'roles': role_access = RoleAccess(self.user) return role_access.can_unattach(sub_obj, obj, 'members', *args, **kwargs) - return super(UserAccess, self).can_unattach(obj, sub_obj, relationship, *args, **kwargs) + + logger.error('Unexpected attempt to de-associate {} from a user.'.format(sub_obj)) + return False class OAuth2ApplicationAccess(BaseAccess): @@ -772,6 +771,11 @@ class OrganizationAccess(NotificationAttachMixin, BaseAccess): return True def can_attach(self, obj, sub_obj, relationship, *args, **kwargs): + # If the request is updating the membership, check the membership role permissions instead + if relationship in ('member_role.members', 'admin_role.members'): + rel_role = getattr(obj, relationship.split('.')[0]) + return RoleAccess(self.user).can_attach(rel_role, sub_obj, 'members', *args, **kwargs) + if relationship == "instance_groups": if self.user.is_superuser: return True @@ -779,6 +783,11 @@ class OrganizationAccess(NotificationAttachMixin, BaseAccess): return super(OrganizationAccess, self).can_attach(obj, sub_obj, relationship, *args, **kwargs) def can_unattach(self, obj, sub_obj, relationship, *args, **kwargs): + # If the request is updating the membership, check the membership role permissions instead + if relationship in ('member_role.members', 'admin_role.members'): + rel_role = getattr(obj, relationship.split('.')[0]) + return RoleAccess(self.user).can_unattach(rel_role, sub_obj, 'members', *args, **kwargs) + if relationship == "instance_groups": return self.can_attach(obj, sub_obj, relationship, *args, **kwargs) return super(OrganizationAccess, self).can_attach(obj, sub_obj, relationship, *args, **kwargs) @@ -1299,6 +1308,12 @@ class TeamAccess(BaseAccess): *args, **kwargs) if self.user.is_superuser: return True + + # If the request is updating the membership, check the membership role permissions instead + if relationship in ('member_role.members', 'admin_role.members'): + rel_role = getattr(obj, relationship.split('.')[0]) + return RoleAccess(self.user).can_attach(rel_role, sub_obj, 'members', *args, **kwargs) + return super(TeamAccess, self).can_attach(obj, sub_obj, relationship, *args, **kwargs) @@ -1309,6 +1324,12 @@ class TeamAccess(BaseAccess): role_access = RoleAccess(self.user) return role_access.can_unattach(sub_obj, obj, 'member_role.parents', *args, **kwargs) + + # If the request is updating the membership, check the membership role permissions instead + if relationship in ('member_role.members', 'admin_role.members'): + rel_role = getattr(obj, relationship.split('.')[0]) + return RoleAccess(self.user).can_unattach(rel_role, sub_obj, 'members', *args, **kwargs) + return super(TeamAccess, self).can_unattach(obj, sub_obj, relationship, *args, **kwargs) @@ -2700,10 +2721,6 @@ class RoleAccess(BaseAccess): @check_superuser def can_unattach(self, obj, sub_obj, relationship, data=None, skip_sub_obj_read_check=False): - if isinstance(obj.content_object, Team): - if not settings.MANAGE_ORGANIZATION_AUTH and not self.user.is_superuser: - return False - if not skip_sub_obj_read_check and relationship in ['members', 'member_role.parents', 'parents']: # If we are unattaching a team Role, check the Team read access if relationship == 'parents': @@ -2715,18 +2732,22 @@ class RoleAccess(BaseAccess): # Being a user in the member_role or admin_role of an organization grants # administrators of that Organization the ability to edit that user. To prevent - # unwanted escalations lets ensure that the Organization administartor has the abilty + # unwanted escalations let's ensure that the Organization administrator has the ability # to admin the user being added to the role. - if (isinstance(obj.content_object, Organization) and - obj.role_field in (Organization.member_role.field.parent_role + ['member_role'])): + if isinstance(obj.content_object, Organization) and obj.role_field in ['admin_role', 'member_role']: if not isinstance(sub_obj, User): logger.error('Unexpected attempt to associate {} with organization role.'.format(sub_obj)) return False + if not settings.MANAGE_ORGANIZATION_AUTH and not self.user.is_superuser: + return False if not UserAccess(self.user).can_admin(sub_obj, None, allow_orphans=True): return False - if isinstance(obj.content_object, ResourceMixin) and \ - self.user in obj.content_object.admin_role: + if isinstance(obj.content_object, Team) and obj.role_field in ['admin_role', 'member_role']: + if not settings.MANAGE_ORGANIZATION_AUTH and not self.user.is_superuser: + return False + + if isinstance(obj.content_object, ResourceMixin) and self.user in obj.content_object.admin_role: return True return False diff --git a/awx/main/tests/functional/test_rbac_organization.py b/awx/main/tests/functional/test_rbac_organization.py index c3c78cf09e..ddb0692ea3 100644 --- a/awx/main/tests/functional/test_rbac_organization.py +++ b/awx/main/tests/functional/test_rbac_organization.py @@ -36,3 +36,15 @@ def test_organization_access_user(cl, organization, user): org = access.get_queryset()[0] assert len(org.admin_role.members.all()) == 0 assert len(org.member_role.members.all()) == 1 + + +@pytest.mark.django_db +@pytest.mark.parametrize('ext_auth', [True, False]) +def test_org_resource_role(ext_auth, organization, rando, org_admin): + with mock.patch('awx.main.access.settings') as settings_mock: + settings_mock.MANAGE_ORGANIZATION_AUTH = ext_auth + access = OrganizationAccess(org_admin) + + assert access.can_attach(organization, rando, 'member_role.members') == ext_auth + organization.member_role.members.add(rando) + assert access.can_unattach(organization, rando, 'member_role.members') == ext_auth diff --git a/awx/main/tests/functional/test_rbac_role.py b/awx/main/tests/functional/test_rbac_role.py index 29598b6753..838a410d58 100644 --- a/awx/main/tests/functional/test_rbac_role.py +++ b/awx/main/tests/functional/test_rbac_role.py @@ -3,7 +3,9 @@ import pytest from awx.main.access import ( RoleAccess, UserAccess, - TeamAccess) + OrganizationAccess, + TeamAccess, +) from awx.main.models import Role, Organization @@ -15,24 +17,21 @@ def test_team_access_attach(rando, team, inventory): # team has read_role for the inventory team.member_role.children.add(inventory.read_role) - access = TeamAccess(rando) + team_access = TeamAccess(rando) + role_access = RoleAccess(rando) data = {'id': inventory.admin_role.pk} - assert not access.can_attach(team, inventory.admin_role, 'member_role.children', data, False) + assert not team_access.can_attach(team, inventory.admin_role, 'member_role.children', data, False) + assert not role_access.can_attach(inventory.admin_role, team, 'member_role.parents', data, False) @pytest.mark.django_db def test_user_access_attach(rando, inventory): inventory.read_role.members.add(rando) - access = UserAccess(rando) + user_access = UserAccess(rando) + role_access = RoleAccess(rando) data = {'id': inventory.admin_role.pk} - assert not access.can_attach(rando, inventory.admin_role, 'roles', data, False) - - -@pytest.mark.django_db -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) + assert not user_access.can_attach(rando, inventory.admin_role, 'roles', data, False) + assert not role_access.can_attach(inventory.admin_role, rando, 'members', data, False) @pytest.mark.django_db @@ -66,8 +65,11 @@ def test_org_user_role_attach(user, organization, inventory): organization.admin_role.members.add(admin) role_access = RoleAccess(admin) + org_access = OrganizationAccess(admin) assert not role_access.can_attach(organization.member_role, nonmember, 'members', None) assert not role_access.can_attach(organization.admin_role, nonmember, 'members', None) + assert not org_access.can_attach(organization, nonmember, 'member_role.members', None) + assert not org_access.can_attach(organization, nonmember, 'admin_role.members', None) # Permissions when adding users/teams to org special-purpose roles @@ -81,9 +83,15 @@ def test_user_org_object_roles(organization, org_admin, org_member): assert RoleAccess(org_admin).can_attach( organization.notification_admin_role, org_member, 'members', None ) + assert OrganizationAccess(org_admin).can_attach( + organization, org_member, 'notification_admin_role.members', None + ) assert not RoleAccess(org_member).can_attach( organization.notification_admin_role, org_member, 'members', None ) + assert not OrganizationAccess(org_member).can_attach( + organization, org_member, 'notification_admin_role.members', None + ) @pytest.mark.django_db @@ -118,8 +126,11 @@ def test_org_superuser_role_attach(admin_user, org_admin, organization): organization.member_role.members.add(admin_user) role_access = RoleAccess(org_admin) + org_access = OrganizationAccess(org_admin) assert not role_access.can_attach(organization.member_role, admin_user, 'members', None) assert not role_access.can_attach(organization.admin_role, admin_user, 'members', None) + assert not org_access.can_attach(organization, admin_user, 'member_role.members', None) + assert not org_access.can_attach(organization, admin_user, 'admin_role.members', None) user_access = UserAccess(org_admin) assert not user_access.can_change(admin_user, {'last_name': 'Witzel'}) @@ -160,12 +171,17 @@ def test_need_all_orgs_to_admin_user(user): assert not user_access.can_change(org12_member, {'last_name': 'Witzel'}) role_access = RoleAccess(org1_admin) + org_access = OrganizationAccess(org1_admin) assert not role_access.can_attach(org1.admin_role, org12_member, 'members', None) assert not role_access.can_attach(org1.member_role, org12_member, 'members', None) + assert not org_access.can_attach(org1, org12_member, 'admin_role.members') + assert not org_access.can_attach(org1, org12_member, 'member_role.members') org2.admin_role.members.add(org1_admin) assert role_access.can_attach(org1.admin_role, org12_member, 'members', None) assert role_access.can_attach(org1.member_role, org12_member, 'members', None) + assert org_access.can_attach(org1, org12_member, 'admin_role.members') + assert org_access.can_attach(org1, org12_member, 'member_role.members') # Orphaned user can be added to member role, only in special cases @@ -178,7 +194,9 @@ def test_orphaned_user_allowed(org_admin, rando, organization): *orphaned means user is not a member of any organization ''' role_access = RoleAccess(org_admin) + org_access = OrganizationAccess(org_admin) assert role_access.can_attach(organization.member_role, rando, 'members', None) + assert org_access.can_attach(organization, rando, 'member_role.members', None) # Cannot edit the user directly without adding to org first user_access = UserAccess(org_admin) assert not user_access.can_change(rando, {'last_name': 'Witzel'}) diff --git a/awx/main/tests/functional/test_rbac_team.py b/awx/main/tests/functional/test_rbac_team.py index 80a825edd5..a18a69a94b 100644 --- a/awx/main/tests/functional/test_rbac_team.py +++ b/awx/main/tests/functional/test_rbac_team.py @@ -21,7 +21,21 @@ def test_team_attach_unattach(team, user): u2 = user('non-member', False) access = TeamAccess(u2) assert not access.can_attach(team, team.member_role, 'member_role.children', None) - assert not access.can_unattach(team, team.member_role, 'member_role.chidlren') + assert not access.can_unattach(team, team.member_role, 'member_role.children') + + +@pytest.mark.django_db +@pytest.mark.parametrize('ext_auth', [True, False]) +def test_team_org_resource_role(ext_auth, team, user, rando): + with mock.patch('awx.main.access.settings') as settings_mock: + settings_mock.MANAGE_ORGANIZATION_AUTH = ext_auth + u = user('member', False) + team.organization.admin_role.members.add(u) + access = TeamAccess(u) + + assert access.can_attach(team, rando, 'member_role.members') == ext_auth + team.member_role.members.add(rando) + assert access.can_unattach(team, rando, 'member_role.members') == ext_auth @pytest.mark.django_db diff --git a/awx/main/tests/functional/test_rbac_user.py b/awx/main/tests/functional/test_rbac_user.py index 96cb5524d0..403767749f 100644 --- a/awx/main/tests/functional/test_rbac_user.py +++ b/awx/main/tests/functional/test_rbac_user.py @@ -77,19 +77,27 @@ def test_manage_org_auth_setting(ext_auth, superuser, expect, organization, rand assert [ # use via /api/v2/users/N/roles/ UserAccess(u).can_attach(rando, organization.admin_role, 'roles'), + UserAccess(u).can_attach(rando, organization.member_role, 'roles'), UserAccess(u).can_attach(rando, team.admin_role, 'roles'), + UserAccess(u).can_attach(rando, team.member_role, 'roles'), # use via /api/v2/roles/N/users/ RoleAccess(u).can_attach(organization.admin_role, rando, 'members'), - RoleAccess(u).can_attach(team.admin_role, rando, 'members') - ] == [expect for i in range(4)] + RoleAccess(u).can_attach(organization.member_role, rando, 'members'), + RoleAccess(u).can_attach(team.admin_role, rando, 'members'), + RoleAccess(u).can_attach(team.member_role, rando, 'members'), + ] == [expect for i in range(8)] assert [ # use via /api/v2/users/N/roles/ UserAccess(u).can_unattach(rando, organization.admin_role, 'roles'), + UserAccess(u).can_unattach(rando, organization.member_role, 'roles'), UserAccess(u).can_unattach(rando, team.admin_role, 'roles'), + UserAccess(u).can_unattach(rando, team.member_role, 'roles'), # use via /api/v2/roles/N/users/ RoleAccess(u).can_unattach(organization.admin_role, rando, 'members'), - RoleAccess(u).can_unattach(team.admin_role, rando, 'members') - ] == [expect for i in range(4)] + RoleAccess(u).can_unattach(organization.member_role, rando, 'members'), + RoleAccess(u).can_unattach(team.admin_role, rando, 'members'), + RoleAccess(u).can_unattach(team.member_role, rando, 'members'), + ] == [expect for i in range(8)] @pytest.mark.django_db