From 8ad0b3f78750786f075ee20024e5fa2e6d8b8260 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 17 Apr 2019 17:45:20 -0400 Subject: [PATCH] Check the permissions for adding users to orgs/teams in the other direction --- awx/main/access.py | 37 ++++++++++++++----- .../functional/test_rbac_organization.py | 12 ++++++ awx/main/tests/functional/test_rbac_team.py | 16 +++++++- 3 files changed, 55 insertions(+), 10 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 511f4fdca7..6d615e8677 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -645,25 +645,22 @@ class UserAccess(BaseAccess): return False def can_attach(self, obj, sub_obj, relationship, *args, **kwargs): - # 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) - if not settings.MANAGE_ORGANIZATION_AUTH and not self.user.is_superuser: - return False - - 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): + # 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) - if not settings.MANAGE_ORGANIZATION_AUTH and not self.user.is_superuser: - return False - - 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): @@ -774,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 @@ -781,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) @@ -1301,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) @@ -1311,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) 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_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