From e48b218bb130efd6f69aaf2bc5e0b8237bdc4937 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Wed, 13 May 2026 13:48:55 -0600 Subject: [PATCH 1/3] Apply object-level permission check to finding duplicate API actions `FindingViewSet.reset_finding_duplicate_status` and `FindingViewSet.set_finding_as_original` were never calling `self.get_object()`, so DRF never invoked `UserHasFindingRelatedObjectPermission.has_object_permission`. The `has_permission` method on that class always returns `True`, so the per-finding check was effectively skipped. Sibling actions like `close`, `verify`, and `remove_tags` already call `self.get_object()` at the top. Adds `self.get_object()` at the top of both action bodies and regression tests in `unittests/test_rest_framework.py` (`FindingActionAuthzTest`). Co-Authored-By: Claude Opus 4.7 (1M context) --- dojo/api_v2/views.py | 2 + unittests/test_rest_framework.py | 63 ++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/dojo/api_v2/views.py b/dojo/api_v2/views.py index bd61d76ee2a..27444b33fc7 100644 --- a/dojo/api_v2/views.py +++ b/dojo/api_v2/views.py @@ -1546,6 +1546,7 @@ def get_duplicate_cluster(self, request, pk): ) @action(detail=True, methods=["post"], url_path=r"duplicate/reset", permission_classes=(IsAuthenticated, permissions.UserHasFindingRelatedObjectPermission)) def reset_finding_duplicate_status(self, request, pk): + self.get_object() checked_duplicate_id = reset_finding_duplicate_status_internal( request.user, pk, ) @@ -1566,6 +1567,7 @@ def reset_finding_duplicate_status(self, request, pk): detail=True, methods=["post"], url_path=r"original/(?P\d+)", permission_classes=(IsAuthenticated, permissions.UserHasFindingRelatedObjectPermission), ) def set_finding_as_original(self, request, pk, new_fid): + self.get_object() success = set_finding_as_original_internal(request.user, pk, new_fid) if not success: return Response(status=status.HTTP_400_BAD_REQUEST) diff --git a/unittests/test_rest_framework.py b/unittests/test_rest_framework.py index 43483a13e3d..edff2fd6590 100644 --- a/unittests/test_rest_framework.py +++ b/unittests/test_rest_framework.py @@ -1849,6 +1849,69 @@ def test_post_without_finding_returns_4xx(self): self.assertLess(response.status_code, 500) +class FindingActionAuthzTest(DojoAPITestCase): + + fixtures = ["dojo_testdata.json"] + + def _client_for(self, username): + user = User.objects.get(username=username) + token = Token.objects.get(user=user) + client = APIClient() + client.credentials(HTTP_AUTHORIZATION="Token " + token.key) + return client + + def test_admin_can_reset_finding_duplicate_status(self): + client = self._client_for("admin") + # Mark finding 2 as a duplicate of finding 3 first, then reset. + set_response = client.post("/api/v2/findings/2/original/3/") + self.assertEqual(set_response.status_code, status.HTTP_204_NO_CONTENT, set_response.content[:500]) + reset_response = client.post("/api/v2/findings/2/duplicate/reset/") + self.assertEqual(reset_response.status_code, status.HTTP_204_NO_CONTENT, reset_response.content[:500]) + refreshed = Finding.objects.get(pk=2) + self.assertFalse(refreshed.duplicate) + self.assertIsNone(refreshed.duplicate_finding) + + def test_admin_can_set_finding_as_original(self): + client = self._client_for("admin") + response = client.post("/api/v2/findings/2/original/3/") + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT, response.content[:500]) + refreshed = Finding.objects.get(pk=2) + self.assertTrue(refreshed.duplicate) + self.assertEqual(refreshed.duplicate_finding_id, 3) + + def test_unrelated_user_cannot_reset_finding_duplicate_status(self): + client = self._client_for("user2") + # Sanity: finding 7 is not visible to this user. + self.assertEqual(client.get("/api/v2/findings/7/").status_code, 404) + + before = Finding.objects.get(pk=7) + before_duplicate = before.duplicate + before_duplicate_finding_id = before.duplicate_finding_id + + response = client.post("/api/v2/findings/7/duplicate/reset/") + self.assertIn(response.status_code, (403, 404), response.content[:500]) + + after = Finding.objects.get(pk=7) + self.assertEqual(after.duplicate, before_duplicate) + self.assertEqual(after.duplicate_finding_id, before_duplicate_finding_id) + + def test_unrelated_user_cannot_set_finding_as_original(self): + client = self._client_for("user2") + # Sanity: finding 7 is not visible to this user. + self.assertEqual(client.get("/api/v2/findings/7/").status_code, 404) + + before = Finding.objects.get(pk=7) + before_duplicate = before.duplicate + before_duplicate_finding_id = before.duplicate_finding_id + + response = client.post("/api/v2/findings/7/original/2/") + self.assertIn(response.status_code, (403, 404), response.content[:500]) + + after = Finding.objects.get(pk=7) + self.assertEqual(after.duplicate, before_duplicate) + self.assertEqual(after.duplicate_finding_id, before_duplicate_finding_id) + + @versioned_fixtures class FilesTest(DojoAPITestCase): fixtures = ["dojo_testdata.json"] From a2104c618dfda22dd777899d7cf9ed41f76756a9 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Wed, 13 May 2026 14:58:06 -0600 Subject: [PATCH 2/3] Update test_finding_reset_duplicate_reader to expect 403 The existing assertion documented the prior bypass behavior (Reader reaching the internal helper and getting 400). With the object-level permission check now running on these actions, a Reader is denied upfront with 403. Co-Authored-By: Claude Opus 4.7 (1M context) --- unittests/test_permissions_audit.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/unittests/test_permissions_audit.py b/unittests/test_permissions_audit.py index 9d4ca0a746e..7e4ead33580 100644 --- a/unittests/test_permissions_audit.py +++ b/unittests/test_permissions_audit.py @@ -1571,18 +1571,16 @@ def test_finding_metadata_reader_allowed(self): self.assertEqual(response.status_code, 200, response.content) # ── Finding: duplicate status actions ────────────────────────────── - # NOTE: reset_finding_duplicate_status and set_finding_as_original - # bypass self.get_object(), so DRF object-level permission checks - # (has_object_permission) never fire. The internal helpers do their - # own permission checks. These tests verify current behaviour. + # reset_finding_duplicate_status and set_finding_as_original call + # self.get_object() so DRF's object-level permission check runs via + # UserHasFindingRelatedObjectPermission (POST -> Finding_Edit). def test_finding_reset_duplicate_reader(self): - """View bypasses get_object() — internal helper checks permissions.""" + """Reader lacks Finding_Edit — POST must be denied before the helper runs.""" client = self._client_for_user(self.reader_user) url = reverse("finding-reset-finding-duplicate-status", args=(self.finding.id,)) response = client.post(url, format="json") - # Returns 400 (not a duplicate) — internal helper runs before perm check - self.assertEqual(response.status_code, 400, response.content) + self.assertEqual(response.status_code, 403, response.content) def test_finding_reset_duplicate_writer(self): client = self._client_for_user(self.writer_user) From 98928ae5e52ee3db66b6d8b0e9023748a23bdd14 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Wed, 13 May 2026 15:21:15 -0600 Subject: [PATCH 3/3] Use versioned fixtures in FindingActionAuthzTest The sibling `RequestResponsePairsAuthzTest` already uses `@versioned_fixtures` so the suite picks up `dojo_testdata_locations.json` when V3_FEATURE_LOCATIONS is enabled. Matching that decorator avoids the Endpoint-deprecation fixture-load error in the V3 CI variant. Co-Authored-By: Claude Opus 4.7 (1M context) --- unittests/test_rest_framework.py | 1 + 1 file changed, 1 insertion(+) diff --git a/unittests/test_rest_framework.py b/unittests/test_rest_framework.py index edff2fd6590..0c15703e646 100644 --- a/unittests/test_rest_framework.py +++ b/unittests/test_rest_framework.py @@ -1849,6 +1849,7 @@ def test_post_without_finding_returns_4xx(self): self.assertLess(response.status_code, 500) +@versioned_fixtures class FindingActionAuthzTest(DojoAPITestCase): fixtures = ["dojo_testdata.json"]