From 6b0df43f3b8bc5378d802e8f6b8926cad95c6a06 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Thu, 8 Sep 2016 11:09:17 -0400 Subject: [PATCH] add unattach field to user and team roles list --- awx/api/serializers.py | 25 ++++-- awx/api/views.py | 4 +- awx/main/access.py | 16 +++- .../functional/api/test_rbac_displays.py | 90 ++++++++++++++----- 4 files changed, 104 insertions(+), 31 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 9bc5e3ecc9..417b44db87 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -331,10 +331,16 @@ class BaseSerializer(serializers.ModelSerializer): } if len(roles) > 0: summary_fields['object_roles'] = roles + + # Advance display of RBAC capabilities if hasattr(self, 'show_capabilities'): view = self.context.get('view', None) + parent_obj = None + if hasattr(view, 'parent_model'): + parent_obj = view.get_parent_object() if view and view.request and view.request.user: - user_capabilities = get_user_capabilities(view.request.user, obj, self.show_capabilities) + user_capabilities = get_user_capabilities( + view.request.user, obj, method_list=self.show_capabilities, parent_obj=parent_obj) if user_capabilities: summary_fields['user_capabilities'] = user_capabilities @@ -1537,6 +1543,9 @@ class RoleSerializer(BaseSerializer): return ret +class RoleSerializerWithParentAccess(RoleSerializer): + show_capabilities = ['unattach'] + class ResourceAccessListElementSerializer(UserSerializer): show_capabilities = [] # Clear fields from UserSerializer parent class @@ -1569,8 +1578,11 @@ class ResourceAccessListElementSerializer(UserSerializer): role_dict['resource_name'] = role.content_object.name role_dict['resource_type'] = role.content_type.name role_dict['related'] = reverse_gfk(role.content_object) - role_dict['user_capabilities'] = {'unattach': requesting_user.can_access( - Role, 'unattach', role, user, 'members', data={}, skip_sub_obj_read_check=False)} + role_dict['user_capabilities'] = {'unattach': requesting_user.can_access( + Role, 'unattach', role, user, 'members', data={}, skip_sub_obj_read_check=False)} + else: + # Singleton roles should not be managed from this view, as per copy/edit rework spec + role_dict['user_capabilities'] = {'unattach': False} return { 'role': role_dict, 'descendant_roles': get_roles_on_resource(obj, role)} def format_team_role_perm(team_role, permissive_role_ids): @@ -1587,8 +1599,11 @@ class ResourceAccessListElementSerializer(UserSerializer): role_dict['resource_name'] = role.content_object.name role_dict['resource_type'] = role.content_type.name role_dict['related'] = reverse_gfk(role.content_object) - role_dict['user_capabilities'] = {'unattach': requesting_user.can_access( - Role, 'unattach', role, team_role, 'parents', data={}, skip_sub_obj_read_check=False)} + role_dict['user_capabilities'] = {'unattach': requesting_user.can_access( + Role, 'unattach', role, team_role, 'parents', data={}, skip_sub_obj_read_check=False)} + else: + # Singleton roles should not be managed from this view, as per copy/edit rework spec + role_dict['user_capabilities'] = {'unattach': False} ret.append({ 'role': role_dict, 'descendant_roles': get_roles_on_resource(obj, team_role)}) return ret diff --git a/awx/api/views.py b/awx/api/views.py index dc9d411086..579566766c 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -864,7 +864,7 @@ class TeamUsersList(BaseUsersList): class TeamRolesList(SubListCreateAttachDetachAPIView): model = Role - serializer_class = RoleSerializer + serializer_class = RoleSerializerWithParentAccess metadata_class = RoleMetadata parent_model = Team relationship='member_role.children' @@ -1197,7 +1197,7 @@ class UserTeamsList(ListAPIView): class UserRolesList(SubListCreateAttachDetachAPIView): model = Role - serializer_class = RoleSerializer + serializer_class = RoleSerializerWithParentAccess metadata_class = RoleMetadata parent_model = User relationship='roles' diff --git a/awx/main/access.py b/awx/main/access.py index b46954db24..bd581893d3 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -116,7 +116,7 @@ def check_user_access(user, model_class, action, *args, **kwargs): return result return False -def get_user_capabilities(user, instance, method_list): +def get_user_capabilities(user, instance, **kwargs): ''' Returns a dictionary of capabilities the user has on the particular instance. *NOTE* This is not a direct mapping of can_* methods into this @@ -125,7 +125,7 @@ def get_user_capabilities(user, instance, method_list): actions in the interface. ''' for access_class in access_registry.get(type(instance), []): - return access_class(user).get_user_capabilities(instance, method_list) + return access_class(user).get_user_capabilities(instance, **kwargs) return None def check_superuser(func): @@ -219,11 +219,11 @@ class BaseAccess(object): elif "features" not in validation_info: raise LicenseForbids("Features not found in active license.") - def get_user_capabilities(self, obj, method_list): + def get_user_capabilities(self, obj, method_list=[], parent_obj=None): user_capabilities = {} # Custom ordering to loop through methods so we can reuse earlier calcs - for display_method in ['edit', 'delete', 'start', 'schedule', 'copy', 'adhoc']: + for display_method in ['edit', 'delete', 'start', 'schedule', 'copy', 'adhoc', 'unattach']: if display_method not in method_list: continue @@ -268,6 +268,14 @@ class BaseAccess(object): user_capabilities[display_method] = access_method(obj) elif method in ['add']: # 2 args with data user_capabilities[display_method] = access_method(data) + elif method in ['attach', 'unattach']: # parent/sub-object call + if type(parent_obj) == Team: + relationship = 'parents' + parent_obj = parent_obj.member_role + else: + relationship = 'members' + user_capabilities[display_method] = access_method( + obj, parent_obj, relationship, skip_sub_obj_read_check=True, data=data) return user_capabilities diff --git a/awx/main/tests/functional/api/test_rbac_displays.py b/awx/main/tests/functional/api/test_rbac_displays.py index aaf97b3782..c203e45b4b 100644 --- a/awx/main/tests/functional/api/test_rbac_displays.py +++ b/awx/main/tests/functional/api/test_rbac_displays.py @@ -159,54 +159,104 @@ def test_proj_jt_admin_copy_edit(jt_copy_edit, rando): assert response['summary_fields']['user_capabilities']['edit'] +@pytest.fixture +def mock_access_method(mocker): + mock_method = mocker.MagicMock() + mock_method.return_value = 'foobar' + mock_method.__name__ = 'bars' # Required for a logging statement + return mock_method + + @pytest.mark.django_db class TestAccessListCapabilities: - @pytest.fixture - def mock_access_method(self, mocker): - "Mocking this requires extra work because of the logging statement" - mock_method = mocker.MagicMock() - mock_method.return_value = 'foobar' - mock_method.__name__ = 'bars' - return mock_method + """ + Test that the access_list serializer shows the exact output of the RoleAccess.can_attach + - looks at /api/v1/inventories/N/access_list/ + - test for types: direct, indirect, and team access + """ + + extra_kwargs = dict(skip_sub_obj_read_check=False, data={}) def _assert_one_in_list(self, data, sublist='direct_access'): + "Establish that exactly 1 type of access exists so we know the entry is the right one" assert len(data['results']) == 1 assert len(data['results'][0]['summary_fields'][sublist]) == 1 - def test_access_list_direct_access_capability(self, inventory, rando, get, mocker, mock_access_method): - """Test that the access_list serializer shows the exact output of the - RoleAccess.can_attach method in the direct_access list""" + def test_access_list_direct_access_capability( + self, inventory, rando, get, mocker, mock_access_method): inventory.admin_role.members.add(rando) + with mocker.patch.object(access_registry[Role][0], 'can_unattach', mock_access_method): response = get(reverse('api:inventory_access_list', args=(inventory.id,)), rando) + + mock_access_method.assert_called_once_with(inventory.admin_role, rando, 'members', **self.extra_kwargs) self._assert_one_in_list(response.data) direct_access_list = response.data['results'][0]['summary_fields']['direct_access'] assert direct_access_list[0]['role']['user_capabilities']['unattach'] == 'foobar' - def test_access_list_indirect_access_capability(self, inventory, admin_user, get, mocker, mock_access_method): - """Test the display of unattach access for a singleton permission""" + def test_access_list_indirect_access_capability( + self, inventory, organization, org_admin, get, mocker, mock_access_method): with mocker.patch.object(access_registry[Role][0], 'can_unattach', mock_access_method): - response = get(reverse('api:inventory_access_list', args=(inventory.id,)), admin_user) + response = get(reverse('api:inventory_access_list', args=(inventory.id,)), org_admin) + + mock_access_method.assert_called_once_with(organization.admin_role, org_admin, 'members', **self.extra_kwargs) self._assert_one_in_list(response.data, sublist='indirect_access') indirect_access_list = response.data['results'][0]['summary_fields']['indirect_access'] assert indirect_access_list[0]['role']['user_capabilities']['unattach'] == 'foobar' - def test_access_list_team_direct_access_capability(self, inventory, team, team_member, get, mocker, mock_access_method): - """Test the display of unattach access for team-based permissions - this happens in a difference place in the serializer code from the user permission""" + def test_access_list_team_direct_access_capability( + self, inventory, team, team_member, get, mocker, mock_access_method): team.member_role.children.add(inventory.admin_role) + with mocker.patch.object(access_registry[Role][0], 'can_unattach', mock_access_method): response = get(reverse('api:inventory_access_list', args=(inventory.id,)), team_member) + + mock_access_method.assert_called_once_with(inventory.admin_role, team.member_role, 'parents', **self.extra_kwargs) self._assert_one_in_list(response.data) direct_access_list = response.data['results'][0]['summary_fields']['direct_access'] assert direct_access_list[0]['role']['user_capabilities']['unattach'] == 'foobar' @pytest.mark.django_db -def test_team_roles_unattach(mocker): - pass +def test_team_roles_unattach(mocker, team, team_member, inventory, mock_access_method, get): + team.member_role.children.add(inventory.admin_role) + + with mocker.patch.object(access_registry[Role][0], 'can_unattach', mock_access_method): + response = get(reverse('api:team_roles_list', args=(team.id,)), team_member) + + # Did we assess whether team_member can remove team's permission to the inventory? + mock_access_method.assert_called_once_with( + inventory.admin_role, team.member_role, 'parents', skip_sub_obj_read_check=True, data={}) + assert response.data['results'][0]['summary_fields']['user_capabilities']['unattach'] == 'foobar' @pytest.mark.django_db -def test_user_roles_unattach(mocker): - pass +def test_user_roles_unattach(mocker, organization, alice, bob, mock_access_method, get): + # Add to same organization so that alice and bob can see each other + organization.member_role.members.add(alice) + organization.member_role.members.add(bob) + + with mocker.patch.object(access_registry[Role][0], 'can_unattach', mock_access_method): + response = get(reverse('api:user_roles_list', args=(alice.id,)), bob) + + # Did we assess whether bob can remove alice's permission to the inventory? + mock_access_method.assert_called_once_with( + organization.member_role, alice, 'members', skip_sub_obj_read_check=True, data={}) + assert response.data['results'][0]['summary_fields']['user_capabilities']['unattach'] == 'foobar' + +@pytest.mark.django_db +def test_team_roles_unattach_functional(team, team_member, inventory, get): + team.member_role.children.add(inventory.admin_role) + response = get(reverse('api:team_roles_list', args=(team.id,)), team_member) + # Team member should be able to remove access to inventory, becauase + # the inventory admin_role grants that ability + assert response.data['results'][0]['summary_fields']['user_capabilities']['unattach'] == True + +@pytest.mark.django_db +def test_user_roles_unattach_functional(organization, alice, bob, get): + # Add to same organization so that alice and bob can see each other + organization.member_role.members.add(alice) + organization.member_role.members.add(bob) + response = get(reverse('api:user_roles_list', args=(alice.id,)), bob) + # Org members can not revoke the membership of other members + assert response.data['results'][0]['summary_fields']['user_capabilities']['unattach'] == False