Merge pull request #4578 from AlanCoding/access_list_perm

Use ParentMixin for access_list permissions check

awx/api/generics.py

@@ -559,14 +559,12 @@ class DestroyAPIView(GenericAPIView, generics.DestroyAPIView):
     pass
 
 
-class ResourceAccessList(ListAPIView):
+class ResourceAccessList(ParentMixin, ListAPIView):
 
     serializer_class = ResourceAccessListElementSerializer
 
     def get_queryset(self):
-        self.object_id = self.kwargs['pk']
-        resource_model = getattr(self, 'resource_model')
-        obj = get_object_or_404(resource_model, pk=self.object_id)
+        obj = self.get_parent_object()
 
         content_type = ContentType.objects.get_for_model(obj)
         roles = set(Role.objects.filter(content_type=content_type, object_id=obj.id))

awx/api/serializers.py

@@ -1586,8 +1586,7 @@ class ResourceAccessListElementSerializer(UserSerializer):
         the resource.
         '''
         ret = super(ResourceAccessListElementSerializer, self).to_representation(user)
-        object_id = self.context['view'].object_id
-        obj = self.context['view'].resource_model.objects.get(pk=object_id)
+        obj = self.context['view'].get_parent_object()
         if self.context['view'].request is not None:
             requesting_user = self.context['view'].request.user
         else:

awx/api/views.py

@@ -872,7 +872,7 @@ class OrganizationNotificationTemplatesSuccessList(SubListCreateAttachDetachAPIV
 class OrganizationAccessList(ResourceAccessList):
 
     model = User # needs to be User for AccessLists's
-    resource_model = Organization
+    parent_model = Organization
     new_in_300 = True
 
 
@@ -1007,7 +1007,7 @@ class TeamActivityStreamList(ActivityStreamEnforcementMixin, SubListAPIView):
 class TeamAccessList(ResourceAccessList):
 
     model = User # needs to be User for AccessLists's
-    resource_model = Team
+    parent_model = Team
     new_in_300 = True
 
 
@@ -1201,7 +1201,7 @@ class ProjectUpdateNotificationsList(SubListAPIView):
 class ProjectAccessList(ResourceAccessList):
 
     model = User # needs to be User for AccessLists's
-    resource_model = Project
+    parent_model = Project
     new_in_300 = True
 
 
@@ -1415,7 +1415,7 @@ class UserDetail(RetrieveUpdateDestroyAPIView):
 class UserAccessList(ResourceAccessList):
 
     model = User # needs to be User for AccessLists's
-    resource_model = User
+    parent_model = User
     new_in_300 = True
 
 
@@ -1522,7 +1522,7 @@ class CredentialActivityStreamList(ActivityStreamEnforcementMixin, SubListAPIVie
 class CredentialAccessList(ResourceAccessList):
 
     model = User # needs to be User for AccessLists's
-    resource_model = Credential
+    parent_model = Credential
     new_in_300 = True
 
 
@@ -1616,7 +1616,7 @@ class InventoryActivityStreamList(ActivityStreamEnforcementMixin, SubListAPIView
 class InventoryAccessList(ResourceAccessList):
 
     model = User # needs to be User for AccessLists's
-    resource_model = Inventory
+    parent_model = Inventory
     new_in_300 = True
 
 
@@ -2690,7 +2690,7 @@ class JobTemplateJobsList(SubListCreateAPIView):
 class JobTemplateAccessList(ResourceAccessList):
 
     model = User # needs to be User for AccessLists's
-    resource_model = JobTemplate
+    parent_model = JobTemplate
     new_in_300 = True
 
 
@@ -3036,7 +3036,7 @@ class WorkflowJobTemplateNotificationTemplatesSuccessList(WorkflowsEnforcementMi
 class WorkflowJobTemplateAccessList(WorkflowsEnforcementMixin, ResourceAccessList):
 
     model = User # needs to be User for AccessLists's
-    resource_model = WorkflowJobTemplate
+    parent_model = WorkflowJobTemplate
     new_in_310 = True
 
 

awx/main/tests/unit/api/test_generics.py

@@ -6,9 +6,16 @@ import mock
 # DRF
 from rest_framework import status
 from rest_framework.response import Response
+from rest_framework.exceptions import PermissionDenied
 
 # AWX
-from awx.api.generics import ParentMixin, SubListCreateAttachDetachAPIView, DeleteLastUnattachLabelMixin
+from awx.api.generics import (
+    ParentMixin,
+    SubListCreateAttachDetachAPIView,
+    DeleteLastUnattachLabelMixin,
+    ResourceAccessList
+)
+from awx.main.models import Organization
 
 
 @pytest.fixture
@@ -29,6 +36,11 @@ def mock_response_new(mocker):
     return m
 
 
+@pytest.fixture
+def mock_organization():
+    return Organization(pk=4, name="Unsaved Org")
+
+
 @pytest.fixture
 def parent_relationship_factory(mocker):
     def rf(serializer_class, relationship_name, relationship_value=mocker.Mock()):
@@ -178,3 +190,37 @@ class TestParentMixin:
 
         get_object_or_404.assert_called_with(parent_mixin.parent_model, **parent_mixin.kwargs)
         assert get_object_or_404.return_value == return_value
+
+
+class TestResourceAccessList:
+
+    def mock_request(self):
+        return mock.MagicMock(
+            user=mock.MagicMock(
+                is_anonymous=mock.MagicMock(return_value=False),
+                is_superuser=False
+            ), method='GET')
+
+
+    def mock_view(self):
+        view = ResourceAccessList()
+        view.parent_model = Organization
+        view.kwargs = {'pk': 4}
+        return view
+
+
+    def test_parent_access_check_failed(self, mocker, mock_organization):
+        with mocker.patch('awx.api.permissions.get_object_or_400', return_value=mock_organization):
+            mock_access = mocker.MagicMock(__name__='for logger', return_value=False)
+            with mocker.patch('awx.main.access.BaseAccess.can_read', mock_access):
+                with pytest.raises(PermissionDenied):
+                    self.mock_view().check_permissions(self.mock_request())
+                mock_access.assert_called_once_with(mock_organization)
+
+
+    def test_parent_access_check_worked(self, mocker, mock_organization):
+        with mocker.patch('awx.api.permissions.get_object_or_400', return_value=mock_organization):
+            mock_access = mocker.MagicMock(__name__='for logger', return_value=True)
+            with mocker.patch('awx.main.access.BaseAccess.can_read', mock_access):
+                self.mock_view().check_permissions(self.mock_request())
+                mock_access.assert_called_once_with(mock_organization)