Merge pull request #670 from jbernal0019/master

Fix #500.

Author: jbernal0019

chris_backend/plugininstances/permissions.py

@@ -2,20 +2,26 @@
 from rest_framework import permissions
 
 
-class IsOwnerOrChrisOrAuthenticatedReadOnlyOrPublicReadOnly(permissions.BasePermission):
+class IsOwnerOrChrisOrHasFeedPermissionReadOnlyOrPublicFeedReadOnly(
+    permissions.BasePermission):
     """
-    Custom permission to only allow owners of an object or superuser
-    'chris' to modify/edit it. Read only is allowed to authenticated users and to
-    unauthenticated users if the related feed is public.
+    Custom permission to only allow owners of an object or superuser 'chris' to
+    modify/edit it (e.g. a plugin instance).
+    Read-only access is allowed to users that have been granted the object's feed
+    access permission or to any user if the object's feed is public.
     """
 
     def has_object_permission(self, request, view, obj):
         user = request.user
+
+        if hasattr(obj, 'plugin_inst'):
+            obj = obj.plugin_inst
+
         if user.username == 'chris' or user == obj.owner:
             return True
 
-        return request.method in permissions.SAFE_METHODS and (user.is_authenticated or
-                                                               obj.feed.public)
+        return request.method in permissions.SAFE_METHODS and (
+                obj.feed.public or obj.feed.has_user_permission(user))
 
 
 class IsNotDeleteFSPluginInstance(permissions.BasePermission):
@@ -26,28 +32,3 @@ class IsNotDeleteFSPluginInstance(permissions.BasePermission):
 
     def has_object_permission(self, request, view, obj):
         return request.method != 'DELETE' or obj.plugin.meta.type != 'fs'
-
-
-class IsAuthenticatedReadOnlyOrPublicReadOnly(permissions.BasePermission):
-    """
-    Custom permission to allow read only access to authenticated users and to
-    unauthenticated users if the related feed is public.
-    """
-
-    def has_object_permission(self, request, view, obj):
-        return request.method in permissions.SAFE_METHODS and (
-                request.user.is_authenticated or obj.feed.public)
-
-
-class IsOwnerOrReadOnly(permissions.BasePermission):
-    """
-    Custom permission to only allow owners of an object modify/edit it.
-    Read only is allowed to other users.
-    """
-
-    def has_object_permission(self, request, view, obj):
-        if request.method in permissions.SAFE_METHODS:
-            return True
-
-        # Write permissions are only allowed to the owner and superuser 'chris'.
-        return request.user == obj.owner

chris_backend/plugininstances/tests/test_views.py

@@ -860,27 +860,36 @@ def test_plugin_instance_delete_failure_fs_plugin(self):
 
 class PluginInstanceListQuerySearchViewTests(ViewTests):
     """
-    Test the plugininstance-list-query-search view.
+    Test the allplugininstance-list-query-search view.
     """
 
     def setUp(self):
         super(PluginInstanceListQuerySearchViewTests, self).setUp()
 
         user = User.objects.get(username=self.username)
 
-        # create two plugin instances
+        # create three plugin instances
         plugin = Plugin.objects.get(meta__name="pacspull")
+        PluginInstance.objects.get_or_create(
+            plugin=plugin, owner=user, compute_resource=plugin.compute_resources.all()[0])
+
         (inst, tf) = PluginInstance.objects.get_or_create(
             plugin=plugin, owner=user, compute_resource=plugin.compute_resources.all()[0])
 
+        inst.status = 'finishedSuccessfully'  # set second instance's status
+        inst.save()
+
         plugin = Plugin.objects.get(meta__name="mri_convert")
         (inst, tf) = PluginInstance.objects.get_or_create(
             plugin=plugin, owner=user, previous=inst,
             compute_resource=plugin.compute_resources.all()[0])
-        # set second instance's status
-        inst.status = 'finishedSuccessfully'
+
+        inst.status = 'finishedSuccessfully'  # set third instance's status
         inst.save()
 
+        inst.feed.public = True  # second and third instances now belongs to a public feed
+        inst.feed.save()
+
         self.list_url = reverse("allplugininstance-list-query-search") + '?status=created'
 
     def test_plugin_instance_query_search_list_success(self):
@@ -890,9 +899,17 @@ def test_plugin_instance_query_search_list_success(self):
         self.assertContains(response, 'created')
         self.assertNotContains(response,'finishedSuccessfully')
 
-    def test_plugin_instance_query_search_list_failure_unauthenticated(self):
+    def test_plugin_instance_query_search_list_success_but_empty_unauthenticated(self):
         response = self.client.get(self.list_url)
-        self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(response.data['results'], [])
+
+    def test_plugin_instance_query_search_list_success_unauthenticated(self):
+        list_url = reverse("allplugininstance-list-query-search") + '?status=finishedSuccessfully'
+        response = self.client.get(list_url)
+        # response should only contain the instances that match the query
+        self.assertContains(response, 'finishedSuccessfully')
+        self.assertNotContains(response, 'created')
 
 
 class PluginInstanceDescendantListViewTests(ViewTests):

chris_backend/plugininstances/views.py

@@ -1,4 +1,5 @@
 
+from django.db.models import Q
 from rest_framework import generics
 from rest_framework import permissions
 from rest_framework.reverse import reverse
@@ -14,8 +15,7 @@
 from .serializers import PARAMETER_SERIALIZERS
 from .serializers import GenericParameterSerializer, PluginInstanceSplitSerializer
 from .serializers import PluginInstanceSerializer
-from .permissions import (IsOwnerOrChrisOrAuthenticatedReadOnlyOrPublicReadOnly,
-                          IsOwnerOrReadOnly, IsAuthenticatedReadOnlyOrPublicReadOnly,
+from .permissions import (IsOwnerOrChrisOrHasFeedPermissionReadOnlyOrPublicFeedReadOnly,
                           IsNotDeleteFSPluginInstance)
 from .tasks import run_plugin_instance, cancel_plugin_instance
 from .utils import run_if_ready
@@ -92,13 +92,15 @@ def list(self, request, *args, **kwargs):
         queryset = self.get_plugin_instances_queryset()
         response = services.get_list_response(self, queryset)
         plugin = self.get_object()
+
         # append document-level link relations
         links = {'plugin': reverse('plugin-detail', request=request,
                                    kwargs={"pk": plugin.id}),
                  'compute_resources': reverse('plugin-computeresource-list',
                                               request=request, kwargs={"pk": plugin.id})
                  }
         response = services.append_collection_links(response, links)
+
         # append write template
         param_names = plugin.get_plugin_parameter_names()
         template_data = {'title': '', 'compute_resource_name': '', 'previous_id': '',
@@ -112,8 +114,9 @@ def get_plugin_instances_queryset(self):
         """
         Custom method to get the actual plugin instances' queryset.
         """
+        user = self.request.user
         plugin = self.get_object()
-        return self.filter_queryset(plugin.instances.all())
+        return plugin.instances.filter(owner=user)
 
 
 @extend_schema_view(
@@ -125,8 +128,6 @@ class AllPluginInstanceList(generics.ListAPIView):
     """
     http_method_names = ['get']
     serializer_class = PluginInstanceSerializer
-    queryset = PluginInstance.objects.all()
-    permission_classes = (permissions.IsAuthenticated,)
 
     def list(self, request, *args, **kwargs):
         """
@@ -140,17 +141,55 @@ def list(self, request, *args, **kwargs):
         links = {'plugins': reverse('plugin-list', request=request)}
         return services.append_collection_links(response, links)
 
+    def get_queryset(self):
+        """
+        Overriden to return a custom queryset that is only comprised by the plugin
+        instances in public feeds if the user is not authenticated. For the superuser
+        'chris' all plugin instances are retuned. For other users all the plugin instances
+        in public feeds or owned by the user or belonging to feeds that have been shared
+        with the user are returned.
+        """
+        user = self.request.user
+
+        if not user.is_authenticated:
+            return PluginInstance.objects.filter(feed__public=True)
+
+        if user.username == 'chris':
+            return PluginInstance.objects.all()
+
+        lookup = Q(feed__public=True) | Q(feed__owner=user) | Q(
+            feed__shared_users=user) | Q(feed__shared_groups__in=user.groups.all())
+        return PluginInstance.objects.filter(lookup)
+
 
 class AllPluginInstanceListQuerySearch(generics.ListAPIView):
     """
     A view for the collection of plugin instances resulting from a query search.
     """
     http_method_names = ['get']
     serializer_class = PluginInstanceSerializer
-    queryset = PluginInstance.objects.all()
-    permission_classes = (permissions.IsAuthenticated,)
     filterset_class = PluginInstanceFilter
 
+    def get_queryset(self):
+        """
+        Overriden to return a custom queryset that is only comprised by the plugin
+        instances in public feeds if the user is not authenticated. For the superuser
+        'chris' all plugin instances are retuned. For other users all the plugin instances
+        in public feeds or owned by the user or belonging to feeds that have been shared
+        with the user are returned.
+        """
+        user = self.request.user
+
+        if not user.is_authenticated:
+            return PluginInstance.objects.filter(feed__public=True)
+
+        if user.username == 'chris':
+            return PluginInstance.objects.all()
+
+        lookup = Q(feed__public=True) | Q(feed__owner=user) | Q(
+            feed__shared_users=user) | Q(feed__shared_groups__in=user.groups.all())
+        return PluginInstance.objects.filter(lookup)
+
 
 class PluginInstanceDetail(generics.RetrieveUpdateDestroyAPIView):
     """
@@ -159,7 +198,7 @@ class PluginInstanceDetail(generics.RetrieveUpdateDestroyAPIView):
     http_method_names = ['get', 'put', 'delete']
     serializer_class = PluginInstanceSerializer
     queryset = PluginInstance.objects.all()
-    permission_classes = (IsOwnerOrChrisOrAuthenticatedReadOnlyOrPublicReadOnly,
+    permission_classes = (IsOwnerOrChrisOrHasFeedPermissionReadOnlyOrPublicFeedReadOnly,
                           IsNotDeleteFSPluginInstance,)
 
     def retrieve(self, request, *args, **kwargs):
@@ -218,7 +257,7 @@ class PluginInstanceDescendantList(generics.ListAPIView):
     http_method_names = ['get']
     serializer_class = PluginInstanceSerializer
     queryset = PluginInstance.objects.all()
-    permission_classes = (IsAuthenticatedReadOnlyOrPublicReadOnly,)
+    permission_classes = (IsOwnerOrChrisOrHasFeedPermissionReadOnlyOrPublicFeedReadOnly,)
 
     def list(self, request, *args, **kwargs):
         """
@@ -242,7 +281,7 @@ class PluginInstanceSplitList(generics.ListCreateAPIView):
     http_method_names = ['get', 'post']
     serializer_class = PluginInstanceSplitSerializer
     queryset = PluginInstance.objects.all()
-    permission_classes = (permissions.IsAuthenticated, IsOwnerOrReadOnly,)
+    permission_classes = (IsOwnerOrChrisOrHasFeedPermissionReadOnlyOrPublicFeedReadOnly,)
 
     def perform_create(self, serializer):
         """
@@ -317,7 +356,7 @@ class PluginInstanceSplitDetail(generics.RetrieveAPIView):
     http_method_names = ['get']
     queryset = PluginInstanceSplit.objects.all()
     serializer_class = PluginInstanceSplitSerializer
-    permission_classes = (permissions.IsAuthenticated,)
+    permission_classes = (IsOwnerOrChrisOrHasFeedPermissionReadOnlyOrPublicFeedReadOnly,)
 
 
 class PluginInstanceParameterList(generics.ListAPIView):
@@ -327,7 +366,7 @@ class PluginInstanceParameterList(generics.ListAPIView):
     http_method_names = ['get']
     serializer_class = GenericParameterSerializer
     queryset = PluginInstance.objects.all()
-    permission_classes = (IsAuthenticatedReadOnlyOrPublicReadOnly,)
+    permission_classes = (IsOwnerOrChrisOrHasFeedPermissionReadOnlyOrPublicFeedReadOnly,)
 
     def list(self, request, *args, **kwargs):
         """
@@ -352,7 +391,7 @@ class StrParameterDetail(generics.RetrieveAPIView):
     http_method_names = ['get']
     serializer_class = PARAMETER_SERIALIZERS['string']
     queryset = StrParameter.objects.all()
-    permission_classes = (IsAuthenticatedReadOnlyOrPublicReadOnly,)
+    permission_classes = (IsOwnerOrChrisOrHasFeedPermissionReadOnlyOrPublicFeedReadOnly,)
 
 
 class IntParameterDetail(generics.RetrieveAPIView):
@@ -362,7 +401,7 @@ class IntParameterDetail(generics.RetrieveAPIView):
     http_method_names = ['get']
     serializer_class = PARAMETER_SERIALIZERS['integer']
     queryset = IntParameter.objects.all()
-    permission_classes = (IsAuthenticatedReadOnlyOrPublicReadOnly,)
+    permission_classes = (IsOwnerOrChrisOrHasFeedPermissionReadOnlyOrPublicFeedReadOnly,)
 
 
 class FloatParameterDetail(generics.RetrieveAPIView):
@@ -372,7 +411,7 @@ class FloatParameterDetail(generics.RetrieveAPIView):
     http_method_names = ['get']
     serializer_class = PARAMETER_SERIALIZERS['float']
     queryset = FloatParameter.objects.all()
-    permission_classes = (IsAuthenticatedReadOnlyOrPublicReadOnly,)
+    permission_classes = (IsOwnerOrChrisOrHasFeedPermissionReadOnlyOrPublicFeedReadOnly,)
 
 
 class BoolParameterDetail(generics.RetrieveAPIView):
@@ -382,7 +421,7 @@ class BoolParameterDetail(generics.RetrieveAPIView):
     http_method_names = ['get']
     serializer_class = PARAMETER_SERIALIZERS['boolean']
     queryset = BoolParameter.objects.all()
-    permission_classes = (IsAuthenticatedReadOnlyOrPublicReadOnly,)
+    permission_classes = (IsOwnerOrChrisOrHasFeedPermissionReadOnlyOrPublicFeedReadOnly,)
 
 
 class PathParameterDetail(generics.RetrieveAPIView):
@@ -392,7 +431,7 @@ class PathParameterDetail(generics.RetrieveAPIView):
     http_method_names = ['get']
     serializer_class = PARAMETER_SERIALIZERS['path']
     queryset = PathParameter.objects.all()
-    permission_classes = (IsAuthenticatedReadOnlyOrPublicReadOnly,)
+    permission_classes = (IsOwnerOrChrisOrHasFeedPermissionReadOnlyOrPublicFeedReadOnly,)
 
 
 class UnextpathParameterDetail(generics.RetrieveAPIView):
@@ -402,4 +441,4 @@ class UnextpathParameterDetail(generics.RetrieveAPIView):
     http_method_names = ['get']
     serializer_class = PARAMETER_SERIALIZERS['unextpath']
     queryset = UnextpathParameter.objects.all()
-    permission_classes = (IsAuthenticatedReadOnlyOrPublicReadOnly,)
+    permission_classes = (IsOwnerOrChrisOrHasFeedPermissionReadOnlyOrPublicFeedReadOnly,)