From e044b996e5c44cc97876f3bbf93852bff005875b Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Mon, 2 Jul 2018 11:31:07 -0400 Subject: [PATCH] allow adding teams to org object roles --- awx/api/views.py | 8 ++-- awx/main/access.py | 26 ++++++++----- .../0042_v330_org_member_role_deparent.py | 29 ++++++++++++++ awx/main/models/organization.py | 11 +++--- awx/main/tests/functional/api/test_role.py | 25 ++++++++++++ awx/main/tests/functional/conftest.py | 2 +- awx/main/tests/functional/test_rbac_role.py | 38 ++++++++++++++++++- awx/main/tests/functional/test_rbac_team.py | 24 ++++++++++++ awx/main/tests/unit/api/test_roles.py | 38 ------------------- 9 files changed, 143 insertions(+), 58 deletions(-) create mode 100644 awx/main/migrations/0042_v330_org_member_role_deparent.py delete mode 100644 awx/main/tests/unit/api/test_roles.py diff --git a/awx/api/views.py b/awx/api/views.py index 2dd0d5467d..07f597dec7 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -1204,8 +1204,8 @@ class TeamRolesList(SubListAttachDetachAPIView): role = get_object_or_400(Role, pk=sub_id) org_content_type = ContentType.objects.get_for_model(Organization) - if role.content_type == org_content_type: - data = dict(msg=_("You cannot assign an Organization role as a child role for a Team.")) + if role.content_type == org_content_type and role.role_field in ['member_role', 'admin_role']: + data = dict(msg=_("You cannot assign an Organization participation role as a child role for a Team.")) return Response(data, status=status.HTTP_400_BAD_REQUEST) if role.is_singleton(): @@ -5071,8 +5071,8 @@ class RoleTeamsList(SubListAttachDetachAPIView): role = Role.objects.get(pk=self.kwargs['pk']) organization_content_type = ContentType.objects.get_for_model(Organization) - if role.content_type == organization_content_type: - data = dict(msg=_("You cannot assign an Organization role as a child role for a Team.")) + if role.content_type == organization_content_type and role.role_field in ['member_role', 'admin_role']: + data = dict(msg=_("You cannot assign an Organization participation role as a child role for a Team.")) return Response(data, status=status.HTTP_400_BAD_REQUEST) credential_content_type = ContentType.objects.get_for_model(Credential) diff --git a/awx/main/access.py b/awx/main/access.py index 93f94489a6..51fd904fa5 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -5,6 +5,7 @@ import os import sys import logging +import six # Django from django.conf import settings @@ -519,20 +520,24 @@ class UserAccess(BaseAccess): return False return bool(self.user == obj or self.can_admin(obj, data)) - def user_membership_roles(self, u): - return Role.objects.filter( - content_type=ContentType.objects.get_for_model(Organization), - role_field__in=Organization.member_role.field.parent_role + ['member_role'], - members=u - ) + @staticmethod + def user_organizations(u): + ''' + Returns all organizations that count `u` as a member + ''' + return Organization.accessible_objects(u, 'member_role') def is_all_org_admin(self, u): - return not self.user_membership_roles(u).exclude( - ancestors__in=self.user.roles.filter(role_field='admin_role') + ''' + returns True if `u` is member of any organization that is + not also an organization that `self.user` admins + ''' + return not self.user_organizations(u).exclude( + pk__in=Organization.accessible_pk_qs(self.user, 'admin_role') ).exists() def user_is_orphaned(self, u): - return not self.user_membership_roles(u).exists() + return not self.user_organizations(u).exists() @check_superuser def can_admin(self, obj, data, allow_orphans=False, check_setting=True): @@ -2550,6 +2555,9 @@ class RoleAccess(BaseAccess): # 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 not isinstance(sub_obj, User): + logger.error(six.text_type('Unexpected attempt to associate {} with organization role.').format(sub_obj)) + return False if not UserAccess(self.user).can_admin(sub_obj, None, allow_orphans=True): return False diff --git a/awx/main/migrations/0042_v330_org_member_role_deparent.py b/awx/main/migrations/0042_v330_org_member_role_deparent.py new file mode 100644 index 0000000000..2ae100053d --- /dev/null +++ b/awx/main/migrations/0042_v330_org_member_role_deparent.py @@ -0,0 +1,29 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.11 on 2018-07-02 13:47 +from __future__ import unicode_literals + +import awx.main.fields +from django.db import migrations +import django.db.models.deletion +from awx.main.migrations._rbac import rebuild_role_hierarchy + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0041_v330_update_oauth_refreshtoken'), + ] + + operations = [ + migrations.AlterField( + model_name='organization', + name='member_role', + field=awx.main.fields.ImplicitRoleField(editable=False, null=b'True', on_delete=django.db.models.deletion.CASCADE, parent_role=[b'admin_role'], related_name='+', to='main.Role'), + ), + migrations.AlterField( + model_name='organization', + name='read_role', + field=awx.main.fields.ImplicitRoleField(editable=False, null=b'True', on_delete=django.db.models.deletion.CASCADE, parent_role=[b'member_role', b'auditor_role', b'execute_role', b'project_admin_role', b'inventory_admin_role', b'workflow_admin_role', b'notification_admin_role', b'credential_admin_role', b'job_template_admin_role'], related_name='+', to='main.Role'), + ), + migrations.RunPython(rebuild_role_hierarchy), + ] diff --git a/awx/main/models/organization.py b/awx/main/models/organization.py index 047678a1fc..d1373442b2 100644 --- a/awx/main/models/organization.py +++ b/awx/main/models/organization.py @@ -67,13 +67,14 @@ class Organization(CommonModel, NotificationFieldsModel, ResourceMixin, CustomVi parent_role='singleton:' + ROLE_SINGLETON_SYSTEM_AUDITOR, ) member_role = ImplicitRoleField( - parent_role=['admin_role', 'execute_role', 'project_admin_role', - 'inventory_admin_role', 'workflow_admin_role', - 'notification_admin_role', 'credential_admin_role', - 'job_template_admin_role',] + parent_role=['admin_role'] ) read_role = ImplicitRoleField( - parent_role=['member_role', 'auditor_role'], + parent_role=['member_role', 'auditor_role', + 'execute_role', 'project_admin_role', + 'inventory_admin_role', 'workflow_admin_role', + 'notification_admin_role', 'credential_admin_role', + 'job_template_admin_role',], ) diff --git a/awx/main/tests/functional/api/test_role.py b/awx/main/tests/functional/api/test_role.py index af673509e7..d398263c37 100644 --- a/awx/main/tests/functional/api/test_role.py +++ b/awx/main/tests/functional/api/test_role.py @@ -12,3 +12,28 @@ def test_admin_visible_to_orphaned_users(get, alice): names.add(item['name']) assert 'System Auditor' in names assert 'System Administrator' in names + + +@pytest.mark.django_db +@pytest.mark.parametrize('role,code', [ + ('member_role', 400), + ('admin_role', 400), + ('inventory_admin_role', 204) +]) +@pytest.mark.parametrize('reversed', [ + True, False +]) +def test_org_object_role_assigned_to_team(post, team, organization, org_admin, role, code, reversed): + if reversed: + url = reverse('api:role_teams_list', kwargs={'pk': getattr(organization, role).id}) + sub_id = team.id + else: + url = reverse('api:team_roles_list', kwargs={'pk': team.id}) + sub_id = getattr(organization, role).id + + post( + url=url, + data={'id': sub_id}, + user=org_admin, + expect=code + ) diff --git a/awx/main/tests/functional/conftest.py b/awx/main/tests/functional/conftest.py index 56b8819355..de25bf1a7f 100644 --- a/awx/main/tests/functional/conftest.py +++ b/awx/main/tests/functional/conftest.py @@ -684,7 +684,7 @@ def job_template_labels(organization, job_template): @pytest.fixture def workflow_job_template(organization): - wjt = WorkflowJobTemplate(name='test-workflow_job_template') + wjt = WorkflowJobTemplate(name='test-workflow_job_template', organization=organization) wjt.save() return wjt diff --git a/awx/main/tests/functional/test_rbac_role.py b/awx/main/tests/functional/test_rbac_role.py index 7cbea31f8a..29598b6753 100644 --- a/awx/main/tests/functional/test_rbac_role.py +++ b/awx/main/tests/functional/test_rbac_role.py @@ -67,10 +67,46 @@ def test_org_user_role_attach(user, organization, inventory): role_access = RoleAccess(admin) assert not role_access.can_attach(organization.member_role, nonmember, 'members', None) - assert not role_access.can_attach(organization.notification_admin_role, nonmember, 'members', None) assert not role_access.can_attach(organization.admin_role, nonmember, 'members', None) +# Permissions when adding users/teams to org special-purpose roles +@pytest.mark.django_db +def test_user_org_object_roles(organization, org_admin, org_member): + ''' + Unlike admin & member roles, the special-purpose organization roles do not + confer any permissions related to user management, + Normal rules about role delegation should apply, only admin to org needed. + ''' + assert RoleAccess(org_admin).can_attach( + organization.notification_admin_role, org_member, 'members', None + ) + assert not RoleAccess(org_member).can_attach( + organization.notification_admin_role, org_member, 'members', None + ) + + +@pytest.mark.django_db +def test_team_org_object_roles(organization, team, org_admin, org_member): + ''' + the special-purpose organization roles are not ancestors of any + team roles, and can be delegated en masse through teams, + following normal admin rules + ''' + assert RoleAccess(org_admin).can_attach( + organization.notification_admin_role, team, 'member_role.parents', {'id': 68} + ) + # Obviously team admin isn't enough to assign organization roles to the team + team.admin_role.members.add(org_member) + assert not RoleAccess(org_member).can_attach( + organization.notification_admin_role, team, 'member_role.parents', {'id': 68} + ) + # Cannot make a team member of an org + assert not RoleAccess(org_admin).can_attach( + organization.member_role, team, 'member_role.parents', {'id': 68} + ) + + # Singleton user editing restrictions @pytest.mark.django_db def test_org_superuser_role_attach(admin_user, org_admin, organization): diff --git a/awx/main/tests/functional/test_rbac_team.py b/awx/main/tests/functional/test_rbac_team.py index bb75c4f0cc..0ea0851adc 100644 --- a/awx/main/tests/functional/test_rbac_team.py +++ b/awx/main/tests/functional/test_rbac_team.py @@ -106,6 +106,30 @@ def test_team_admin_member_access(team, user, project): assert len(Project.accessible_objects(u, 'use_role')) == 1 +@pytest.mark.django_db +def test_team_member_org_role_access_project(team, rando, project, organization): + team.member_role.members.add(rando) + assert rando not in project.read_role + team.member_role.children.add(organization.project_admin_role) + assert rando in project.admin_role + + +@pytest.mark.django_db +def test_team_member_org_role_access_workflow(team, rando, workflow_job_template, organization): + team.member_role.members.add(rando) + assert rando not in workflow_job_template.read_role + team.member_role.children.add(organization.workflow_admin_role) + assert rando in workflow_job_template.admin_role + + +@pytest.mark.django_db +def test_team_member_org_role_access_inventory(team, rando, inventory, organization): + team.member_role.members.add(rando) + assert rando not in inventory.read_role + team.member_role.children.add(organization.inventory_admin_role) + assert rando in inventory.admin_role + + @pytest.mark.django_db def test_org_admin_team_access(organization, team, user, project): u = user('team_admin', False) diff --git a/awx/main/tests/unit/api/test_roles.py b/awx/main/tests/unit/api/test_roles.py deleted file mode 100644 index a51dbb9f58..0000000000 --- a/awx/main/tests/unit/api/test_roles.py +++ /dev/null @@ -1,38 +0,0 @@ -import mock - -from rest_framework.test import APIRequestFactory -from rest_framework.test import force_authenticate - -from django.contrib.contenttypes.models import ContentType - -from awx.api.views import ( - TeamRolesList, -) - -from awx.main.models import ( - User, - Role, -) - - -def test_team_roles_list_post_org_roles(): - with mock.patch('awx.api.views.get_object_or_400') as role_get, \ - mock.patch('awx.api.views.ContentType.objects.get_for_model') as ct_get: - - role_mock = mock.MagicMock(spec=Role) - content_type_mock = mock.MagicMock(spec=ContentType) - role_mock.content_type = content_type_mock - role_get.return_value = role_mock - ct_get.return_value = content_type_mock - - factory = APIRequestFactory() - view = TeamRolesList.as_view() - - request = factory.post("/team/1/roles", {'id':1}, format="json") - force_authenticate(request, User(username="root", is_superuser=True)) - - response = view(request) - response.render() - - assert response.status_code == 400 - assert 'cannot assign' in response.content