Merge pull request #3730 from jbradberry/manage-org-resource-roles

Adjust the access logic for settings.MANAGE_ORGANIZATION_AUTH = False

Reviewed-by: https://github.com/softwarefactory-project-zuul[bot]
This commit is contained in:
softwarefactory-project-zuul[bot] 2019-04-22 16:44:05 +00:00 committed by GitHub
commit 805a0c7a9a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 108 additions and 35 deletions

View File

@ -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

View File

@ -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

View File

@ -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'})

View File

@ -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

View File

@ -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