TMSS-2305: process review comments

SAS/TMSS/backend/src/tmss/tmssapp/viewsets/lofar_viewset.py

@@ -83,6 +83,44 @@ class LOFARViewSet(viewsets.ModelViewSet):
     #                        'project_ref_override_model': models.<name_of_model_the_path_to_project_attribute_refers_to>}}
     extra_action_permission_specs = {}
 
+    @staticmethod
+    def _user_has_permission_for_any_project(user, method, permission_name):
+                permitted_project_roles = get_project_roles_with_permission(permission_name, method)
+                user_project_roles = set([project_role['role'] for project_role in get_project_roles_for_user(user)])
+                for role in permissions.ProjectRole.objects.filter(value__in=list(user_project_roles)).all():
+                    if role in permitted_project_roles:
+                        return True
+                return False
+
+    def _get_permitted_extra_actions(self, request):
+        # determined the extra actions a user is allowed to perform (these may have different permissions than the base
+        # model, so we list which ones are allowed next to the allowed REST methods. This should be fine as actions only
+        # support either POST or GET so there is no ambiguity.)
+
+        permission_name = self.serializer_class.Meta.model.__name__.lower()
+        allowed_extra_actions = []
+        extra_actions = self.get_extra_actions()
+        actual_method = request.method  # keep the actually requested method so that we can set it when we are done
+        actual_action = self.action  # keep the actually requested action so that we can set it when we are done
+        for extra_action in extra_actions:
+            action = extra_action.__name__
+            for method in extra_action.mapping:
+                self.action = action  # pretend to do something else and check if we are allowed to do that
+                request.method = method.upper()  # pretend to do something else and check if we are allowed to do that
+                if TMSSPermissions().has_permission(request=request, view=self):
+                    if action not in allowed_extra_actions:
+                        allowed_extra_actions.append(action)
+                # show extra action as permitted if the user is allowed to perform it in case the  correct project is
+                # referenced (which then only works if the user posts data that links the object to the right project).
+                if actual_action == 'list' and action not in allowed_extra_actions:
+                    action_permission_name = '%s-%s' % (permission_name, action)
+                    if self._user_has_permission_for_any_project(request.user, method.upper(), action_permission_name):
+                            allowed_extra_actions.append(action)
+        self.action = actual_action
+        request.method = actual_method
+
+        return allowed_extra_actions
+
     def _get_permitted_methods(self, request, pk=None):
         # Django returns an "Allow" header that reflects what methods the model supports in principle, but not what
         # the current user is actually has permission to perform. We use the "Access-Control-Allow-Methods" header
@@ -108,49 +146,20 @@ class LOFARViewSet(viewsets.ModelViewSet):
 
         permission_name = self.serializer_class.Meta.model.__name__.lower()
 
-        # check allowed extra actions (these may have different permissions than the base model, so we list which ones
-        # are allowed next to the allowed REST methods. This should be fine as actions only support either POST or GET
-        # so there is no ambiguity.)
-        extra_actions = self.get_extra_actions()
-        actual_action = self.action  # keep the actually requested action so that we can set it when we are done
-        for extra_action in extra_actions:
-            action = extra_action.__name__
-            for method in extra_action.mapping:
-                self.action = action  # pretend to do something else and check if we are allowed to do that
-                request.method = method.upper()  # pretend to do something else and check if we are allowed to do that
-                if TMSSPermissions().has_permission(request=request, view=self):
-                    if action not in allowed_methods:
-                        allowed_methods.append(action)
-                # add extra action permission if project permission allows creation if correct project is referenced
-                # (see below for reasoning)
-                if actual_action == 'list' and action not in allowed_methods:
-                    action_permission_name = '%s-%s' % (permission_name, action)
-                    permitted_project_roles = get_project_roles_with_permission(action_permission_name, method.upper())
-                    user_project_roles = set(
-                        [project_role['role'] for project_role in get_project_roles_for_user(request.user)])
-                    for role in permissions.ProjectRole.objects.filter(value__in=list(user_project_roles)).all():
-                        if role in permitted_project_roles:
-                            allowed_methods.append(action)
-        self.action = actual_action
-        request.method = actual_method
-
         # add POST permission if project permission allows creation if correct project is referenced
         if self.action == 'list' and 'POST' not in allowed_methods:
             # has_permission on list actions only returns true if there is a system permission that allows POSTing
             # generally. Hence we need to check if there is the theoretical possibility of POSTing an object based
             # on project permissions (which then only works if the user posts data that links the object to the
             # right project).
-            permitted_project_roles = get_project_roles_with_permission(permission_name, 'POST')
-            user_project_roles = set([project_role['role'] for project_role in get_project_roles_for_user(request.user)])
-            for role in permissions.ProjectRole.objects.filter(value__in=list(user_project_roles)).all():
-                if role in permitted_project_roles:
-                    allowed_methods.append('POST')
-                    # optimization: we grant POST/PUT/PATCH/DELETE permissions en block, so we only need to check one
-                    # of them and skip the expensive actual permission check for the rest. Note: this only affects the
-                    # header and the permission will still be properly checked when the user performs one of these.
-                    allowed_methods += ['PUT', 'PATCH', 'DELETE']
-                    break
-
+            if self._user_has_permission_for_any_project(request.user, 'POST', permission_name):
+                allowed_methods.append('POST')
+                # optimization: we grant POST/PUT/PATCH/DELETE permissions en block, so we only need to check one
+                # of them and skip the expensive actual permission check for the rest. Note: this only affects the
+                # header and the permission will still be properly checked when the user performs one of these.
+                allowed_methods += ['PUT', 'PATCH', 'DELETE']
+
+        allowed_methods += self._get_permitted_extra_actions(request)
         return allowed_methods
 
     @swagger_auto_schema(responses={403: 'forbidden'})

SAS/TMSS/backend/src/tmss/tmssapp/viewsets/permissions.py

@@ -15,6 +15,7 @@ from lofar.common import isProductionEnvironment, isDevelopmentEnvironment
 from django.db.models import Q
 from django.contrib.auth import get_user_model
 User = get_user_model()
+from django.shortcuts import get_object_or_404
 
 #
 # Permissions
@@ -186,7 +187,6 @@ class IsProjectMember(drf_permissions.DjangoObjectPermissions):
                                     extra_action_specs.get('project_ref_override_pk_url_param') in request.query_params:
                     # some extra actions do not carry all info in their POSTed data, but include a url parameter
                     # reference to the related project. Use that to resolve the project here.
-                    from django.shortcuts import get_object_or_404
                     obj = get_object_or_404(extra_action_specs.get('project_ref_override_model'),
                                             pk=request.query_params[extra_action_specs.get('project_ref_override_pk_url_param')])
                     continue