Skip to content

Use a dedicated permission class for BurpRawRequestResponseViewSet#14838

Merged
Maffooch merged 2 commits intoDefectDojo:bugfixfrom
Maffooch:fix/burp-request-response-permission
May 8, 2026
Merged

Use a dedicated permission class for BurpRawRequestResponseViewSet#14838
Maffooch merged 2 commits intoDefectDojo:bugfixfrom
Maffooch:fix/burp-request-response-permission

Conversation

@Maffooch
Copy link
Copy Markdown
Contributor

@Maffooch Maffooch commented May 8, 2026

What

The top-level /api/v2/request_response_pairs/ viewset previously reused UserHasFindingRelatedObjectPermission, a BaseRelatedObjectPermission subclass whose has_permission returns True. That class is shaped for @action(detail=True, …) endpoints where DRF resolves the parent Finding from the URL and has_object_permission runs against it. On a top-level POST there is no parent object resolved yet, so the create flow only ran has_object_permission against the not-yet-saved row and skipped any check on the client-supplied finding foreign key.

This PR introduces UserHasBurpRawRequestResponsePermission, mirroring the pattern already used by UserHasFindingPermission, UserHasProductPermission, and the other parent-keyed viewsets:

  • has_permission calls check_post_permission(request, Finding, "finding", Permissions.Finding_Edit) on POST, which fetches the referenced finding and rejects with 403/404 unless the user has Finding_Edit. Missing finding is rejected with ParseError.
  • has_object_permission dereferences obj.finding for retrieve/update/delete so list/detail/PUT/PATCH/DELETE behavior is unchanged.

The other 11 callers of UserHasFindingRelatedObjectPermission are all @action(detail=True, …) decorators where the URL-resolved finding makes the existing class behave correctly, so the base class is left as-is.

Test plan

Added unittests.test_rest_framework.RequestResponsePairsAuthzTest covering:

  1. Positive control — admin POST returns 201 and the row count increases.
  2. A user with no product/product_type/global membership receives 404 for GET /api/v2/findings/7/ and a 4xx for POST /api/v2/request_response_pairs/ with that finding ID; the row count is unchanged.
  3. POST without finding returns 4xx (the ParseError path in check_post_permission).
docker compose -f docker-compose.yml -f docker-compose.override.unit_tests.yml run --rm \
  -e DJANGO_SETTINGS_MODULE=dojo.settings.settings \
  --entrypoint '' uwsgi \
  python3 manage.py test \
  unittests.test_rest_framework.RequestResponsePairsAuthzTest -v2 --keepdb

🤖 Generated with Claude Code

The top-level /api/v2/request_response_pairs/ viewset reused
UserHasFindingRelatedObjectPermission, which is shaped for
@action(detail=True) endpoints where DRF resolves the parent finding
from the URL. On a top-level POST there is no parent object resolved
yet, so the create flow only ran has_object_permission against the
not-yet-saved row and effectively skipped any check on the
client-supplied "finding" foreign key.

Introduce UserHasBurpRawRequestResponsePermission, which validates
the parent finding against Finding_Edit on POST via
check_post_permission, mirroring the pattern already used by
UserHasFindingPermission, UserHasProductPermission, and the other
parent-keyed viewsets. has_object_permission dereferences obj.finding
for retrieve/update/delete so list/detail/PUT/PATCH/DELETE behavior
is unchanged.

Add regression coverage in unittests/test_rest_framework.py asserting
the positive control still works, that an authenticated user without
membership cannot create a pair on a hidden finding, and that POSTs
missing the finding key are rejected.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Maffooch Maffooch requested a review from mtesauro as a code owner May 8, 2026 02:34
@Maffooch Maffooch added this to the 2.59.0 milestone May 8, 2026
The dojo_testdata.json fixture contains Endpoint rows, which raise
NotImplementedError in Endpoint.__init__ when V3_FEATURE_LOCATIONS is
enabled. Mirror the surrounding API test classes by applying the
@versioned_fixtures decorator so the locations-aware fixture is loaded
on the V3 matrix leg.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@mtesauro mtesauro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

@Maffooch Maffooch merged commit 04c3d89 into DefectDojo:bugfix May 8, 2026
157 of 158 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants