Refactor engagement permissions: introduce BaseRelatedObjectPermission and update related views

Author: Maffooch

dojo/api_v2/permissions.py

@@ -278,40 +278,55 @@ def has_object_permission(self, request, view, obj):
         )
 
 
-class UserHasEngagementPermission(permissions.BasePermission):
-    # Permission checks for related objects (like notes or metadata) can be moved
-    # into a seperate class, when the legacy authorization will be removed.
-    path_engagement_post = re.compile(r"^/api/v2/engagements/$")
-    path_engagement = re.compile(r"^/api/v2/engagements/\d+/$")
+class BaseRelatedObjectPermission(permissions.BasePermission):
+
+    """
+    An "abstract" base class for related object permissions (like notes, metadata, etc.)
+    that only need object permissions, not general permissions. This class will serve as
+    the base class for other more aptly named permission classes.
+    """
+
+    permission_map = {
+        "get_permission": None,
+        "put_permission": None,
+        "delete_permission": None,
+        "post_permission": None,
+    }
 
     def has_permission(self, request, view):
-        if UserHasEngagementPermission.path_engagement_post.match(
-            request.path,
-        ) or UserHasEngagementPermission.path_engagement.match(request.path):
-            return check_post_permission(
-                request, Product, "product", Permissions.Engagement_Add,
-            )
         # related object only need object permission
         return True
 
     def has_object_permission(self, request, view, obj):
-        if UserHasEngagementPermission.path_engagement_post.match(
-            request.path,
-        ) or UserHasEngagementPermission.path_engagement.match(request.path):
-            return check_object_permission(
-                request,
-                obj,
-                Permissions.Engagement_View,
-                Permissions.Engagement_Edit,
-                Permissions.Engagement_Delete,
+        return check_object_permission(
+            request,
+            obj,
+            **self.permission_map,
+        )
+
+
+class UserHasEngagementRelatedObjectPermission(BaseRelatedObjectPermission):
+    permission_map = {
+        "get_permission": Permissions.Engagement_View,
+        "put_permission": Permissions.Engagement_Edit,
+        "delete_permission": Permissions.Engagement_Edit,
+        "post_permission": Permissions.Engagement_Edit,
+    }
+
+
+class UserHasEngagementPermission(permissions.BasePermission):
+    def has_permission(self, request, view):
+        return check_post_permission(
+                request, Product, "product", Permissions.Engagement_Add,
             )
+
+    def has_object_permission(self, request, view, obj):
         return check_object_permission(
             request,
             obj,
             Permissions.Engagement_View,
             Permissions.Engagement_Edit,
-            Permissions.Engagement_Edit,
-            Permissions.Engagement_Edit,
+            Permissions.Engagement_Delete,
         )
 
 

dojo/api_v2/views.py

@@ -503,7 +503,7 @@ def generate_report(self, request, pk=None):
         request=serializers.AddNewNoteOptionSerializer,
         responses={status.HTTP_201_CREATED: serializers.NoteSerializer},
     )
-    @action(detail=True, methods=["get", "post"])
+    @action(detail=True, methods=["get", "post"], permission_classes=[IsAuthenticated, permissions.UserHasEngagementRelatedObjectPermission])
     def notes(self, request, pk=None):
         engagement = self.get_object()
         if request.method == "POST":
@@ -567,7 +567,7 @@ def notes(self, request, pk=None):
         responses={status.HTTP_201_CREATED: serializers.FileSerializer},
     )
     @action(
-        detail=True, methods=["get", "post"], parser_classes=(MultiPartParser,),
+        detail=True, methods=["get", "post"], parser_classes=(MultiPartParser,), permission_classes=[IsAuthenticated, permissions.UserHasEngagementRelatedObjectPermission],
     )
     def files(self, request, pk=None):
         engagement = self.get_object()
@@ -603,7 +603,7 @@ def files(self, request, pk=None):
             status.HTTP_201_CREATED: serializers.EngagementCheckListSerializer,
         },
     )
-    @action(detail=True, methods=["get", "post"])
+    @action(detail=True, methods=["get", "post"], permission_classes=[IsAuthenticated, permissions.UserHasEngagementRelatedObjectPermission])
     def complete_checklist(self, request, pk=None):
         engagement = self.get_object()
         check_lists = Check_List.objects.filter(engagement=engagement)
@@ -650,6 +650,7 @@ def complete_checklist(self, request, pk=None):
         detail=True,
         methods=["get"],
         url_path=r"files/download/(?P<file_id>\d+)",
+        permission_classes=[IsAuthenticated, permissions.UserHasEngagementRelatedObjectPermission],
     )
     def download_file(self, request, file_id, pk=None):
         engagement = self.get_object()