Skip to content

Commit eec7f04

Browse files
committed
Close is_public-extract parity gap in source visibility builders
Reviewing the visible_extracts_for docstring surfaced a latent filter/check parity violation (same class as the group-grant drift this PR fixes): user_can's privacy recursion delegates to Extract.objects.user_can, which grants READ on is_public extracts for authenticated users, while the list-side privacy gates omitted the flag for extracts (analyses always included it). An extract-rooted row on a public extract therefore passed user_can(READ) but never surfaced in visible_to_user or service listings — a public-extract fixture would have failed the parity invariant. visible_extracts_for now includes is_public on the authenticated branch (anonymous still gets none — ExtractManager denies outright), mirroring visible_analyses_for. No practical exposure today: no user-facing flow sets Extract.is_public. Pinned by test_public_extract_source_passes_both_surfaces; changelog updated.
1 parent 3b7df6f commit eec7f04

3 files changed

Lines changed: 61 additions & 5 deletions

File tree

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
- **Relationship privacy recursion implemented (closes the Phase-C deferral from issue #1655).** `RelationshipManager.user_can` and `RelationshipManager.visible_to_user` (`opencontractserver/shared/Managers.py`) now enforce `created_by_analysis` / `created_by_extract` source permissions exactly like annotations: non-creators need the requested permission on the source Analysis/Extract in addition to the doc+corpus MIN. Previously a user with doc+corpus grants could see and write analysis-/extract-private relationships through the manager surfaces. The recursion logic is shared with `AnnotationManager` via the new module-level `_source_privacy_recursion_passes`. Parity pinned by new fixtures/tests in `opencontractserver/tests/permissioning/test_authorization_invariants.py` (`RelationshipAuthorizationInvariantsTestCase` — the old `test_no_privacy_widening_via_created_by_analysis` sentinel that documented the deferral is replaced by real privacy invariants) and `opencontractserver/tests/permissioning/test_relationship_privacy_scoping.py`.
22
- **Extracts locked down to never be anonymous-visible.** `ExtractService.get_visible_extracts` / `check_extract_permission` (`opencontractserver/extracts/services/extract_service.py`) previously exposed `is_public` extracts in public corpora to anonymous callers (drift copied from the analysis service, reachable through the un-gated `extracts` GraphQL resolver), contradicting the documented "Extract — never" anonymous contract. A new `ExtractManager` (`opencontractserver/extracts/models.py`) denies anonymous users on both `visible_to_user` and `user_can` so the rule holds at the manager layer too (e.g. agent tools listing extracts). User-visible consequence: anonymous viewers of a public corpus now see `totalExtracts: 0` in `corpusStats` (the resolver's counts are per-viewer by contract). Two legacy tests that pinned the old semantic were updated to the agreed behavior: `test_query_optimizer_methods.py::test_get_visible_extracts_anonymous_only_public` (now `..._anonymous_sees_nothing`) and `test_stats.py::test_public_corpus_stats_query` (anonymous branch). New coverage: `opencontractserver/tests/permissioning/test_extract_anonymous_lockdown.py`.
3-
- **Group-granted analysis/extract permissions now unlock private rows in list queries.** The `created_by_*` privacy gates in `AnnotationQuerySet.visible_to_user` (`opencontractserver/shared/QuerySets.py`), `AnnotationService.get_document_annotations` / `get_corpus_annotations`, and `RelationshipService.get_document_relationships` consulted only the USER object-permission tables, so a viewer whose source grant arrived via a Django group passed `user_can` (which resolves group grants by default) but never saw the rows in lists — a filter/check parity drift. All gates now route through the new shared builders in `opencontractserver/utils/source_visibility.py`, which join the group object-permission tables. Tests: `test_source_visibility_group_grants.py` + new invariant fixtures.
3+
- **Group-granted analysis/extract permissions now unlock private rows in list queries.** The `created_by_*` privacy gates in `AnnotationQuerySet.visible_to_user` (`opencontractserver/shared/QuerySets.py`), `AnnotationService.get_document_annotations` / `get_corpus_annotations`, and `RelationshipService.get_document_relationships` consulted only the USER object-permission tables, so a viewer whose source grant arrived via a Django group passed `user_can` (which resolves group grants by default) but never saw the rows in lists — a filter/check parity drift. All gates now route through the new shared builders in `opencontractserver/utils/source_visibility.py`, which join the group object-permission tables. The same parity class also covered `Extract.is_public`: the inline gates omitted the flag for extracts (while honouring it for analyses) even though `user_can`'s recursion grants READ on public sources — `visible_extracts_for` now includes `is_public` on the authenticated branch (no practical exposure today: no user-facing flow sets the flag). Tests: `test_source_visibility_group_grants.py` + new invariant fixtures incl. `test_public_extract_source_passes_both_surfaces`.
44
- **Anonymous privacy leak in corpus-wide annotation listing fixed.** `AnnotationService.get_corpus_annotations` applied the `created_by_*` privacy exclusion only to authenticated users — anonymous viewers of a public corpus could see analysis-/extract-private annotations on public documents. The exclusion now applies unconditionally (anonymous resolves to public-analyses-only / no extracts via `source_visibility`).

opencontractserver/tests/permissioning/test_authorization_invariants.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1623,6 +1623,50 @@ def test_group_granted_source_read_passes_both_surfaces(self):
16231623
"visible_to_user must honour the group-level READ grant on the source",
16241624
)
16251625

1626+
def test_public_extract_source_passes_both_surfaces(self):
1627+
"""A PUBLIC extract unlocks its extract-rooted rows for authenticated
1628+
viewers on BOTH surfaces (2026-06 audit follow-up): ``user_can``'s
1629+
privacy recursion grants READ on ``is_public`` sources via
1630+
``Extract.objects.user_can``, and the list gates previously omitted
1631+
the flag for extracts (while honouring it for analyses) — a latent
1632+
filter/check parity violation. ``visible_extracts_for`` now includes
1633+
``is_public`` on the authenticated branch; this pins the two
1634+
surfaces agreeing. (Anonymous users still see nothing —
1635+
``ExtractManager`` denies them outright.)
1636+
"""
1637+
from opencontractserver.annotations.models import Annotation
1638+
from opencontractserver.extracts.models import Extract
1639+
1640+
public_extract = Extract.objects.create(
1641+
name="Public Invariant Source Extract",
1642+
corpus=self.corpus,
1643+
fieldset=self.fieldset,
1644+
creator=self.creator,
1645+
is_public=True,
1646+
)
1647+
rooted = Annotation.objects.create(
1648+
raw_text="via_public_extract",
1649+
json={"x": 6},
1650+
page=1,
1651+
annotation_label=self.token_label,
1652+
creator=self.creator,
1653+
document=self.document,
1654+
corpus=self.corpus,
1655+
created_by_extract=public_extract,
1656+
)
1657+
# shared_reader has doc+corpus READ but no grant on the extract.
1658+
self.assertTrue(
1659+
rooted.user_can(self.shared_reader, PermissionTypes.READ),
1660+
"user_can must grant READ via the PUBLIC source extract",
1661+
)
1662+
self.assertTrue(
1663+
Annotation.objects.visible_to_user(self.shared_reader)
1664+
.filter(pk=rooted.pk)
1665+
.exists(),
1666+
"visible_to_user must include rows rooted in a PUBLIC extract "
1667+
"(parity with user_can's recursion)",
1668+
)
1669+
16261670
def test_is_public_does_not_grant_writes(self):
16271671
"""SECURITY: ``is_public=True`` grants a non-creator READ but never
16281672
UPDATE/DELETE — the write asymmetry must hold for Annotation."""

opencontractserver/utils/source_visibility.py

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,21 @@ def visible_extracts_for(user: Any) -> QuerySet:
7373
7474
Returns a lazy queryset (safe to embed as a subquery via
7575
``__in=...``): nothing for anonymous users (extracts are never
76-
anonymous-visible); own | user-granted | group-granted extracts for
77-
authenticated users. ``is_public`` is intentionally absent — extract
78-
privacy has never keyed off the flag and no user-facing flow sets it.
76+
anonymous-visible — ``ExtractManager`` denies them on both surfaces, so
77+
the flag is irrelevant on that branch); public | own | user-granted |
78+
group-granted extracts for authenticated users.
79+
80+
``is_public`` note: the original inline implementations omitted the
81+
flag for extracts (while including it for analyses). That was a latent
82+
filter/check parity violation, not a design choice — ``user_can``'s
83+
privacy recursion delegates to ``Extract.objects.user_can``, which
84+
grants READ on ``is_public`` rows for authenticated users, so an
85+
extract-rooted row on a public extract passed ``user_can(READ)`` while
86+
never appearing in lists. Including the flag here (authenticated branch
87+
only) restores parity and mirrors ``visible_analyses_for``. No
88+
user-facing flow currently sets ``Extract.is_public``, so this has no
89+
practical exposure today; pinned by
90+
``test_public_extract_source_passes_both_surfaces``.
7991
"""
8092
from django.db.models import Q
8193

@@ -88,7 +100,7 @@ def visible_extracts_for(user: Any) -> QuerySet:
88100
if user is None or getattr(user, "is_anonymous", True):
89101
return Extract.objects.none()
90102

91-
visible = Extract.objects.filter(Q(creator=user))
103+
visible = Extract.objects.filter(Q(creator=user) | Q(is_public=True))
92104

93105
user_grant_ids = ExtractUserObjectPermission.objects.filter(user=user).values_list(
94106
"content_object_id", flat=True

0 commit comments

Comments
 (0)