From 70b0679a0cd7468dd4ebb7b403ea33185943a940 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 17 Apr 2019 15:37:02 -0400 Subject: [PATCH] Adjust the access logic for settings.MANAGE_ORGANIZATION_AUTH = False so that changing the membership of Organizations and Teams are disallowed unless you are a superuser, but granting resource privileges is still permitted. --- awx/main/access.py | 30 +++++++++++---------- awx/main/tests/functional/test_rbac_user.py | 16 ++++++++--- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 250ce99a66..511f4fdca7 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -645,22 +645,24 @@ 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. 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) - def can_unattach(self, obj, sub_obj, relationship, *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) + + def can_unattach(self, obj, sub_obj, relationship, *args, **kwargs): 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) @@ -2700,10 +2702,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 +2713,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_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