Anchor location finding reference authorization to the finding's own product#14871
Open
Maffooch wants to merge 2 commits into
Open
Anchor location finding reference authorization to the finding's own product#14871Maffooch wants to merge 2 commits into
Maffooch wants to merge 2 commits into
Conversation
…product `get_authorized_location_finding_reference` in `dojo/location/queries.py` was building its membership `OuterRef` paths against `location__products__product_id` / `location__products__product__prod_type_id`. Because a `Location` can be associated with more than one product via `LocationProductReference`, this allowed a user with access to any product that shared the location to read references for findings belonging to other products on the same location. Switched the four `OuterRef` paths to anchor on the finding's actual product (`finding__test__engagement__product[_…]`), so each row resolves authorization against the single product that owns the finding. Renamed the annotation aliases to `finding__…` to match the new path. `V3EndpointStatusCompatibleViewSet.get_queryset` uses the same helper and picks up the change automatically. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The bug being fixed is in the V3-locations code path, so the test must run with V3_FEATURE_LOCATIONS=True. In that mode the legacy `dojo_testdata.json` fixture fails to load because the Endpoint model is deprecated. `@versioned_fixtures` swaps to `dojo_testdata_locations.json` automatically so the suite passes in both V2 and V3 CI variants. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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
get_authorized_location_finding_referenceindojo/location/queries.pywas building its membershipOuterRefpaths againstlocation__products__product_id/location__products__product__prod_type_id. Because aLocationcan be associated with more than one product viaLocationProductReference, this allowed a user with access to any product that shared the location to read references for findings belonging to other products on the same location.OuterRefpaths (Product_Member,Product_Group,Product_Type_Member,Product_Type_Groupsubqueries) to anchor on the finding's actual product:finding__test__engagement__product_id/finding__test__engagement__product__prod_type_id. EachLocationFindingReferencerow now resolves authorization against the single product that actually owns the finding. Renamed the annotation aliases tofinding__…to reflect the new path.V3EndpointStatusCompatibleViewSet.get_queryset(dojo/location/api/endpoint_compat.py) calls the same helper, so it picks up the change automatically — no separate edit needed there.unittests/test_location_finding_reference_authz.pythat creates two products sharing a Location, gives Alice/Bob Reader on one product each, and verifies each user only sees their product'sLocationFindingReferencerows when querying the helper. Confirmed the tests fail without the queries.py change.Performance notes
The new
OuterRefpath traverses three FK joins (Finding→Test→Engagement→ product) instead of the old two (Location→LocationProductReference→ product). Every column involved is a normal FK and indexed by Django by default. The old path fanned out acrossLocation.products(one-to-many); the new path is many-to-one and yields a single product per row, so for locations shared across multiple products the new path is actually cheaper in theExistssubquery. For typical workloads where a location belongs to one or two products the cost is roughly equivalent.Test plan