Fix reader file upload (#15133)

Author: samiat4911

dojo/authorization/api_permissions.py

@@ -16,6 +16,7 @@
     user_has_permission,
     user_is_superuser_or_global_owner,
 )
+from dojo.authorization.roles_permissions import Permissions
 from dojo.importers.auto_create_context import AutoCreateContextManager
 from dojo.location.models import Location
 from dojo.models import (
@@ -401,6 +402,15 @@ class UserHasEngagementRelatedObjectPermission(BaseRelatedObjectPermission):
     }
 
 
+class UserHasEngagementFilePermission(BaseRelatedObjectPermission):
+    permission_map = {
+        "get_permission": Permissions.Product_Tracking_Files_View,
+        "put_permission": Permissions.Product_Tracking_Files_Edit,
+        "delete_permission": Permissions.Product_Tracking_Files_Delete,
+        "post_permission": Permissions.Product_Tracking_Files_Add,
+    }
+
+
 class UserHasEngagementNotePermission(BaseRelatedObjectPermission):
     permission_map = {
         "get_permission": "view",
@@ -462,6 +472,15 @@ class UserHasFindingRelatedObjectPermission(BaseRelatedObjectPermission):
     }
 
 
+class UserHasFindingFilePermission(BaseRelatedObjectPermission):
+    permission_map = {
+        "get_permission": Permissions.Product_Tracking_Files_View,
+        "put_permission": Permissions.Product_Tracking_Files_Edit,
+        "delete_permission": Permissions.Product_Tracking_Files_Delete,
+        "post_permission": Permissions.Product_Tracking_Files_Add,
+    }
+
+
 class UserHasFindingNotePermission(BaseRelatedObjectPermission):
     permission_map = {
         "get_permission": "view",
@@ -778,6 +797,15 @@ class UserHasTestRelatedObjectPermission(BaseRelatedObjectPermission):
     }
 
 
+class UserHasTestFilePermission(BaseRelatedObjectPermission):
+    permission_map = {
+        "get_permission": Permissions.Product_Tracking_Files_View,
+        "put_permission": Permissions.Product_Tracking_Files_Edit,
+        "delete_permission": Permissions.Product_Tracking_Files_Delete,
+        "post_permission": Permissions.Product_Tracking_Files_Add,
+    }
+
+
 class UserHasTestNotePermission(BaseRelatedObjectPermission):
     permission_map = {
         "get_permission": "view",

dojo/engagement/api/views.py

@@ -219,7 +219,7 @@ def notes(self, request, pk=None):
         responses={status.HTTP_201_CREATED: api_v2_serializers.FileSerializer},
     )
     @action(
-        detail=True, methods=["get", "post"], parser_classes=(MultiPartParser,), permission_classes=[IsAuthenticated, permissions.UserHasEngagementRelatedObjectPermission],
+        detail=True, methods=["get", "post"], parser_classes=(MultiPartParser,), permission_classes=[IsAuthenticated, permissions.UserHasEngagementFilePermission],
     )
     def files(self, request, pk=None):
         engagement = self.get_object()

dojo/finding/api/views.py

@@ -480,7 +480,7 @@ def notes(self, request, pk=None):
         responses={status.HTTP_201_CREATED: api_v2_serializers.FileSerializer},
     )
     @action(
-        detail=True, methods=["get", "post"], parser_classes=(MultiPartParser,), permission_classes=(IsAuthenticated, permissions.UserHasFindingRelatedObjectPermission),
+        detail=True, methods=["get", "post"], parser_classes=(MultiPartParser,), permission_classes=(IsAuthenticated, permissions.UserHasFindingFilePermission),
     )
     def files(self, request, pk=None):
         finding = self.get_object()
@@ -518,7 +518,7 @@ def files(self, request, pk=None):
     @action(
         detail=True,
         methods=["get"],
-        url_path=r"files/download/(?P<file_id>\d+)", permission_classes=(IsAuthenticated, permissions.UserHasFindingRelatedObjectPermission),
+        url_path=r"files/download/(?P<file_id>\d+)", permission_classes=(IsAuthenticated, permissions.UserHasFindingFilePermission),
     )
     def download_file(self, request, file_id, pk=None):
         finding = self.get_object()

dojo/forms.py

@@ -572,6 +572,7 @@ def clean(self):
 
 
 ManageFileFormSet = modelformset_factory(FileUpload, extra=3, max_num=10, fields=["title", "file"], can_delete=True, formset=BaseManageFileFormSet)
+AddOnlyManageFileFormSet = modelformset_factory(FileUpload, extra=3, max_num=10, fields=["title", "file"], can_delete=False, formset=BaseManageFileFormSet)
 
 
 # Risk acceptance forms live in dojo/risk_acceptance/ui/forms.py. Re-exported here for

dojo/test/api/views.py

@@ -194,7 +194,7 @@ def notes(self, request, pk=None):
         responses={status.HTTP_201_CREATED: api_v2_serializers.FileSerializer},
     )
     @action(
-        detail=True, methods=["get", "post"], parser_classes=(MultiPartParser,), permission_classes=(IsAuthenticated, permissions.UserHasTestRelatedObjectPermission),
+        detail=True, methods=["get", "post"], parser_classes=(MultiPartParser,), permission_classes=(IsAuthenticated, permissions.UserHasTestFilePermission),
     )
     def files(self, request, pk=None):
         test = self.get_object()
@@ -233,7 +233,7 @@ def files(self, request, pk=None):
         detail=True,
         methods=["get"],
         url_path=r"files/download/(?P<file_id>\d+)",
-        permission_classes=(IsAuthenticated, permissions.UserHasTestRelatedObjectPermission),
+        permission_classes=(IsAuthenticated, permissions.UserHasTestFilePermission),
     )
     def download_file(self, request, file_id, pk=None):
         test = self.get_object()

dojo/views.py

@@ -10,8 +10,9 @@
 from django.shortcuts import get_object_or_404, render
 from django.urls import reverse
 
-from dojo.authorization.authorization import user_has_permission_or_403
-from dojo.forms import ManageFileFormSet
+from dojo.authorization.authorization import user_has_permission, user_has_permission_or_403
+from dojo.authorization.roles_permissions import Permissions
+from dojo.forms import AddOnlyManageFileFormSet, ManageFileFormSet
 from dojo.models import (
     Engagement,
     FileUpload,
@@ -42,34 +43,39 @@ def custom_bad_request_view(request, exception=None):
 def manage_files(request, oid, obj_type):
     if obj_type == "Engagement":
         obj = get_object_or_404(Engagement, pk=oid)
-        user_has_permission_or_403(request.user, obj, "edit")
         obj_vars = ("view_engagement", "engagement_set")
     elif obj_type == "Test":
         obj = get_object_or_404(Test, pk=oid)
-        user_has_permission_or_403(request.user, obj, "edit")
         obj_vars = ("view_test", "test_set")
     elif obj_type == "Finding":
         obj = get_object_or_404(Finding, pk=oid)
-        user_has_permission_or_403(request.user, obj, "edit")
         obj_vars = ("view_finding", "finding_set")
     else:
         raise Http404
 
-    files_formset = ManageFileFormSet(queryset=obj.files.all())
+    has_file_add_permission = user_has_permission(request.user, obj, Permissions.Product_Tracking_Files_Add)
+    has_file_edit_permission = user_has_permission(request.user, obj, Permissions.Product_Tracking_Files_Edit)
+    if not (has_file_add_permission or has_file_edit_permission):
+        raise PermissionDenied
+
+    formset_class = ManageFileFormSet if has_file_edit_permission else AddOnlyManageFileFormSet
+    files_queryset = obj.files.all() if has_file_edit_permission else FileUpload.objects.none()
+    files_formset = formset_class(queryset=files_queryset)
     error = False
 
     if request.method == "POST":
-        files_formset = ManageFileFormSet(
-            request.POST, request.FILES, queryset=obj.files.all())
+        files_formset = formset_class(
+            request.POST, request.FILES, queryset=files_queryset)
         if files_formset.is_valid():
             # remove all from database and disk
 
             files_formset.save()
 
-            for o in files_formset.deleted_objects:
-                logger.debug("removing file: %s", o.file.name)
-                with suppress(FileNotFoundError):
-                    (Path(settings.MEDIA_ROOT) / o.file.name).unlink()
+            for o in getattr(files_formset, "deleted_objects", []):
+                if has_file_edit_permission:
+                    logger.debug("removing file: %s", o.file.name)
+                    with suppress(FileNotFoundError):
+                        (Path(settings.MEDIA_ROOT) / o.file.name).unlink()
 
             for o in files_formset.new_objects:
                 logger.debug("adding file: %s", o.file.name)

unittests/test_permissions_audit.py

@@ -1406,6 +1406,9 @@ def _client_for_user(self, user):
         client.credentials(HTTP_AUTHORIZATION="Token " + token.key)
         return client
 
+    def _authorize_reader_user(self):
+        self.product.authorized_users.add(self.reader_user)
+
     def setUp(self):
         super().setUp()
         # Legacy auth collapses Reader/Writer/Maintainer/Owner into
@@ -1493,11 +1496,13 @@ def test_engagement_notes_post_writer_allowed(self):
 
     # ── Engagement: files ──────────────────────────────────────────────
 
-    def test_engagement_files_post_reader_denied(self):
+    def test_engagement_files_post_member_allowed(self):
+        self._authorize_reader_user()
         client = self._client_for_user(self.reader_user)
         url = reverse("engagement-files", args=(self.engagement.id,))
-        response = client.post(url, data={}, format="json")
-        self.assertEqual(response.status_code, 403, response.content)
+        test_file = SimpleUploadedFile("reader-proof.txt", b"engagement file content", content_type="text/plain")
+        response = client.post(url, data={"title": "reader proof", "file": test_file}, format="multipart")
+        self.assertEqual(response.status_code, 201, response.content)
 
     def test_engagement_files_post_writer_allowed(self):
         client = self._client_for_user(self.writer_user)
@@ -1587,11 +1592,13 @@ def test_finding_notes_post_writer_allowed(self):
 
     # ── Finding: files ─────────────────────────────────────────────────
 
-    def test_finding_files_post_reader_denied(self):
+    def test_finding_files_post_member_allowed(self):
+        self._authorize_reader_user()
         client = self._client_for_user(self.reader_user)
         url = reverse("finding-files", args=(self.finding.id,))
-        response = client.post(url, data={}, format="json")
-        self.assertEqual(response.status_code, 403, response.content)
+        test_file = SimpleUploadedFile("reader-evidence.txt", b"finding file content", content_type="text/plain")
+        response = client.post(url, data={"title": "reader evidence", "file": test_file}, format="multipart")
+        self.assertEqual(response.status_code, 201, response.content)
 
     def test_finding_files_post_writer_allowed(self):
         client = self._client_for_user(self.writer_user)
@@ -1600,6 +1607,26 @@ def test_finding_files_post_writer_allowed(self):
         response = client.post(url, data={"title": "test evidence", "file": test_file}, format="multipart")
         self.assertEqual(response.status_code, 201, response.content)
 
+    def test_manage_files_member_can_upload_to_finding(self):
+        self._authorize_reader_user()
+        client = Client()
+        client.login(username="relobjperm_reader", password="testTEST1234!@#$")  # noqa: S106
+        test_file = SimpleUploadedFile("reader-ui-evidence.txt", b"finding file content", content_type="text/plain")
+        response = client.post(
+            reverse("manage_files", args=(self.finding.id, "Finding")),
+            data={
+                "form-TOTAL_FORMS": "3",
+                "form-INITIAL_FORMS": "0",
+                "form-MIN_NUM_FORMS": "0",
+                "form-MAX_NUM_FORMS": "10",
+                "form-0-title": "reader ui evidence",
+                "form-0-file": test_file,
+            },
+        )
+
+        self.assertEqual(response.status_code, 302, response.content)
+        self.assertTrue(self.finding.files.filter(title="reader ui evidence").exists())
+
     # ── Finding: remove_note (NotePermission — PATCH uses Edit) ────────
 
     def test_finding_remove_note_reader_denied(self):
@@ -1691,11 +1718,13 @@ def test_test_notes_post_writer_allowed(self):
 
     # ── Test: files ────────────────────────────────────────────────────
 
-    def test_test_files_post_reader_denied(self):
+    def test_test_files_post_member_allowed(self):
+        self._authorize_reader_user()
         client = self._client_for_user(self.reader_user)
         url = reverse("test-files", args=(self.test.id,))
-        response = client.post(url, data={}, format="json")
-        self.assertEqual(response.status_code, 403, response.content)
+        test_file = SimpleUploadedFile("reader-results.txt", b"test file content", content_type="text/plain")
+        response = client.post(url, data={"title": "reader results", "file": test_file}, format="multipart")
+        self.assertEqual(response.status_code, 201, response.content)
 
     def test_test_files_post_writer_allowed(self):
         client = self._client_for_user(self.writer_user)