add unattach field to user and team roles list

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_dict['user_capabilities'] = {'unattach': requesting_user.can_access(
-                Role, 'unattach', role, user, 'members', data={}, skip_sub_obj_read_check=False)}
+                    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_dict['user_capabilities'] = {'unattach': requesting_user.can_access(
-                    Role, 'unattach', role, team_role, 'parents', data={}, skip_sub_obj_read_check=False)}
+                        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
 

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'

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
 

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):
+    Test that the access_list serializer shows the exact output of the RoleAccess.can_attach
-        "Mocking this requires extra work because of the logging statement"
+     - looks at /api/v1/inventories/N/access_list/
-        mock_method = mocker.MagicMock()
+     - test for types: direct, indirect, and team access
-        mock_method.return_value = 'foobar'
+    """
-        mock_method.__name__ = 'bars'
+
-        return mock_method
+    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):
+    def test_access_list_direct_access_capability(
-        """Test that the access_list serializer shows the exact output of the
+            self, inventory, rando, get, mocker, mock_access_method):
-        RoleAccess.can_attach method in the direct_access list"""
         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):
+    def test_access_list_indirect_access_capability(
-        """Test the display of unattach access for a singleton permission"""
+            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):
+    def test_access_list_team_direct_access_capability(
-        """Test the display of unattach access for team-based permissions
+            self, inventory, team, team_member, get, mocker, mock_access_method):
-        this happens in a difference place in the serializer code from the user permission"""
         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):
+def test_team_roles_unattach(mocker, team, team_member, inventory, mock_access_method, get):
-    pass
+    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):
+def test_user_roles_unattach(mocker, organization, alice, bob, mock_access_method, get):
-    pass
+    # 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