Apply object-level permission check to finding duplicate API actions#14866
Open
Maffooch wants to merge 3 commits into
Open
Apply object-level permission check to finding duplicate API actions#14866Maffooch wants to merge 3 commits into
Maffooch wants to merge 3 commits into
Conversation
`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>
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>
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>
Maffooch
requested review from
Jino-T,
blakeaowens,
dogboat and
valentijnscholten
valentijnscholten
approved these changes
May 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FindingViewSet.reset_finding_duplicate_status(POST/api/v2/findings/{id}/duplicate/reset/) andFindingViewSet.set_finding_as_original(POST/api/v2/findings/{id}/original/{new_fid}/) never calledself.get_object(). Because the related-object permission classUserHasFindingRelatedObjectPermissiononly enforces the per-finding check inhas_object_permission(itshas_permissionalways returnsTrue), DRF skipped the check entirely on these two actions. Sibling actions likeclose,verify, andremove_tagsalready callself.get_object()at the top.self.get_object()at the top of both action bodies so the existing object-level permission flow runs. No other behavior changes.unittests/test_rest_framework.py(FindingActionAuthzTest) covering admin happy paths and a non-member user on both endpoints, including a state-preservation check on the unauthorized attempt.Test plan
python manage.py test unittests.test_rest_framework.FindingActionAuthzTestpython manage.py test unittests.test_rest_framework.FindingsTest.test_duplicate(existing happy path still passes)python manage.py checkruff check . --fix