Skip to content

Commit de61f86

Browse files
Maffoochclaude
andauthored
Apply object-level permission check to finding duplicate API actions (#14866)
* 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b50728e commit de61f86

3 files changed

Lines changed: 71 additions & 7 deletions

File tree

dojo/api_v2/views.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1546,6 +1546,7 @@ def get_duplicate_cluster(self, request, pk):
15461546
)
15471547
@action(detail=True, methods=["post"], url_path=r"duplicate/reset", permission_classes=(IsAuthenticated, permissions.UserHasFindingRelatedObjectPermission))
15481548
def reset_finding_duplicate_status(self, request, pk):
1549+
self.get_object()
15491550
checked_duplicate_id = reset_finding_duplicate_status_internal(
15501551
request.user, pk,
15511552
)
@@ -1566,6 +1567,7 @@ def reset_finding_duplicate_status(self, request, pk):
15661567
detail=True, methods=["post"], url_path=r"original/(?P<new_fid>\d+)", permission_classes=(IsAuthenticated, permissions.UserHasFindingRelatedObjectPermission),
15671568
)
15681569
def set_finding_as_original(self, request, pk, new_fid):
1570+
self.get_object()
15691571
success = set_finding_as_original_internal(request.user, pk, new_fid)
15701572
if not success:
15711573
return Response(status=status.HTTP_400_BAD_REQUEST)

unittests/test_permissions_audit.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1571,18 +1571,16 @@ def test_finding_metadata_reader_allowed(self):
15711571
self.assertEqual(response.status_code, 200, response.content)
15721572

15731573
# ── Finding: duplicate status actions ──────────────────────────────
1574-
# NOTE: reset_finding_duplicate_status and set_finding_as_original
1575-
# bypass self.get_object(), so DRF object-level permission checks
1576-
# (has_object_permission) never fire. The internal helpers do their
1577-
# own permission checks. These tests verify current behaviour.
1574+
# reset_finding_duplicate_status and set_finding_as_original call
1575+
# self.get_object() so DRF's object-level permission check runs via
1576+
# UserHasFindingRelatedObjectPermission (POST -> Finding_Edit).
15781577

15791578
def test_finding_reset_duplicate_reader(self):
1580-
"""View bypasses get_object()internal helper checks permissions."""
1579+
"""Reader lacks Finding_EditPOST must be denied before the helper runs."""
15811580
client = self._client_for_user(self.reader_user)
15821581
url = reverse("finding-reset-finding-duplicate-status", args=(self.finding.id,))
15831582
response = client.post(url, format="json")
1584-
# Returns 400 (not a duplicate) — internal helper runs before perm check
1585-
self.assertEqual(response.status_code, 400, response.content)
1583+
self.assertEqual(response.status_code, 403, response.content)
15861584

15871585
def test_finding_reset_duplicate_writer(self):
15881586
client = self._client_for_user(self.writer_user)

unittests/test_rest_framework.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1849,6 +1849,70 @@ def test_post_without_finding_returns_4xx(self):
18491849
self.assertLess(response.status_code, 500)
18501850

18511851

1852+
@versioned_fixtures
1853+
class FindingActionAuthzTest(DojoAPITestCase):
1854+
1855+
fixtures = ["dojo_testdata.json"]
1856+
1857+
def _client_for(self, username):
1858+
user = User.objects.get(username=username)
1859+
token = Token.objects.get(user=user)
1860+
client = APIClient()
1861+
client.credentials(HTTP_AUTHORIZATION="Token " + token.key)
1862+
return client
1863+
1864+
def test_admin_can_reset_finding_duplicate_status(self):
1865+
client = self._client_for("admin")
1866+
# Mark finding 2 as a duplicate of finding 3 first, then reset.
1867+
set_response = client.post("/api/v2/findings/2/original/3/")
1868+
self.assertEqual(set_response.status_code, status.HTTP_204_NO_CONTENT, set_response.content[:500])
1869+
reset_response = client.post("/api/v2/findings/2/duplicate/reset/")
1870+
self.assertEqual(reset_response.status_code, status.HTTP_204_NO_CONTENT, reset_response.content[:500])
1871+
refreshed = Finding.objects.get(pk=2)
1872+
self.assertFalse(refreshed.duplicate)
1873+
self.assertIsNone(refreshed.duplicate_finding)
1874+
1875+
def test_admin_can_set_finding_as_original(self):
1876+
client = self._client_for("admin")
1877+
response = client.post("/api/v2/findings/2/original/3/")
1878+
self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT, response.content[:500])
1879+
refreshed = Finding.objects.get(pk=2)
1880+
self.assertTrue(refreshed.duplicate)
1881+
self.assertEqual(refreshed.duplicate_finding_id, 3)
1882+
1883+
def test_unrelated_user_cannot_reset_finding_duplicate_status(self):
1884+
client = self._client_for("user2")
1885+
# Sanity: finding 7 is not visible to this user.
1886+
self.assertEqual(client.get("/api/v2/findings/7/").status_code, 404)
1887+
1888+
before = Finding.objects.get(pk=7)
1889+
before_duplicate = before.duplicate
1890+
before_duplicate_finding_id = before.duplicate_finding_id
1891+
1892+
response = client.post("/api/v2/findings/7/duplicate/reset/")
1893+
self.assertIn(response.status_code, (403, 404), response.content[:500])
1894+
1895+
after = Finding.objects.get(pk=7)
1896+
self.assertEqual(after.duplicate, before_duplicate)
1897+
self.assertEqual(after.duplicate_finding_id, before_duplicate_finding_id)
1898+
1899+
def test_unrelated_user_cannot_set_finding_as_original(self):
1900+
client = self._client_for("user2")
1901+
# Sanity: finding 7 is not visible to this user.
1902+
self.assertEqual(client.get("/api/v2/findings/7/").status_code, 404)
1903+
1904+
before = Finding.objects.get(pk=7)
1905+
before_duplicate = before.duplicate
1906+
before_duplicate_finding_id = before.duplicate_finding_id
1907+
1908+
response = client.post("/api/v2/findings/7/original/2/")
1909+
self.assertIn(response.status_code, (403, 404), response.content[:500])
1910+
1911+
after = Finding.objects.get(pk=7)
1912+
self.assertEqual(after.duplicate, before_duplicate)
1913+
self.assertEqual(after.duplicate_finding_id, before_duplicate_finding_id)
1914+
1915+
18521916
@versioned_fixtures
18531917
class FilesTest(DojoAPITestCase):
18541918
fixtures = ["dojo_testdata.json"]

0 commit comments

Comments
 (0)