Skip to content

Anchor location finding reference authorization to the finding's own product#14871

Open
Maffooch wants to merge 2 commits into
bugfixfrom
fix/location-finding-reference-authz
Open

Anchor location finding reference authorization to the finding's own product#14871
Maffooch wants to merge 2 commits into
bugfixfrom
fix/location-finding-reference-authz

Conversation

@Maffooch
Copy link
Copy Markdown
Contributor

Summary

  • 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 (Product_Member, Product_Group, Product_Type_Member, Product_Type_Group subqueries) to anchor on the finding's actual product: finding__test__engagement__product_id / finding__test__engagement__product__prod_type_id. Each LocationFindingReference row now resolves authorization against the single product that actually owns the finding. Renamed the annotation aliases to finding__… 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.
  • Added unittests/test_location_finding_reference_authz.py that creates two products sharing a Location, gives Alice/Bob Reader on one product each, and verifies each user only sees their product's LocationFindingReference rows when querying the helper. Confirmed the tests fail without the queries.py change.

Performance notes

The new OuterRef path traverses three FK joins (FindingTestEngagement → product) instead of the old two (LocationLocationProductReference → product). Every column involved is a normal FK and indexed by Django by default. The old path fanned out across Location.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 the Exists subquery. For typical workloads where a location belongs to one or two products the cost is roughly equivalent.

Test plan

  • `python manage.py test unittests.test_location_finding_reference_authz`
  • `python manage.py test unittests.test_location_models`
  • `python manage.py check`
  • `ruff check . --fix`

…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>
@Maffooch Maffooch requested a review from mtesauro as a code owner May 13, 2026 20:48
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>
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.

2 participants