OpenConceptLab/ocl_online#224 | Access-control fixes for content in repos the requester can't view

[paynejd]

Refs OpenConceptLab/ocl_online#224 (partial: the items still open are listed on that ticket).

To finish (reviewer): the steps after merging live in the checklist on OpenConceptLab/ocl_online#224:

• deploy to staging, then prod;
• merge OpenConceptLab/ocl_online#224 | Org ids unique regardless of case; concept /parents/ and /children/ ocl-docs#70 once this is live;
• one post-deploy cleanup and two checks;
• then close the ticket by hand.

Items this PR leaves open are tracked in OpenConceptLab/ocl_online#482 to OpenConceptLab/ocl_online#488.

Follow-up to #906 and #910: more places where a response depended on content in repos the requester can't view. Each commit makes one fix and adds a regression test that fails on master and passes here.

Fixes

Concepts and search

• Concept lookup (concepts/lookup/) checks view access, and its page cache holds only public sources. This restores e1f3138, which was reverted in 7bfb6c9.
• $match filters on exact-case owner and source names (owner_text.keyword, source_text.keyword). A target_repo is now searched by the stored names of the repos that were checked, so a version key or a lowercase owner_type works too.
• /parents/ and /children/ list only concepts the requester can view, the same rule includeParentConcepts/includeChildConcepts already use. They aren't searchable now: a search there listed the repo's concepts instead.
• Search in the requester's own scope (their user, an org they belong to, or all their orgs on /user/orgs/...) leaves out the private content of an owner whose name differs only in case. Concepts are filtered on the exact-case owner. Mapping, source, collection and repo indexes have no exact-case owner field, so when such an owner exists those searches show only public content.
• New org names are unique regardless of case, as on GitHub, through the API and bulk import alike.
• List cache: repo version concept lists with includeParentConcepts/includeChildConcepts aren't cached, since their content depends on the requester.
• collection_versions (concept and mapping version detail, versions/?includeCollectionVersions=true) and the /collection-versions/ lists show only collection versions the requester can view.

Mappings, cascade and imports

• Linking stored targets: when a concept or source is created, mappings that already pointed at it are linked only if the mapping's last saver can view it, the same rule as on save. This also fixes the re-index of from-side mappings, which filtered on to_source.
• $cascade and $clone check view access on the repo they run over, as well as on the root concept's repo.
• omitIfExistsIn ($cascade, and a collection reference's omit_if_exists_in) is ignored when the requester, or the reference's creator, can't view the repo.
• A collection's mappings for a concept (includeMappings in collection scope) match the concept and collection URLs by exact prefix, not by case-insensitive substring.
• Bulk updates: a concept update reads its previous version's parents only when a new parent isn't viewable. Before, it did so on every update, at 2 queries per concept.

FHIR

• CodeSystem $lookup / $validate-code: a code system the requester can't view is handled exactly like one that doesn't exist, with or without a code. A public code system that shares a canonical URL with a private one is no longer hidden by it.
• ValueSet $validate-code reads only the value sets the request names: the url value set and the route's collection. Other query params can't pick another owner or collection. A display with a code that isn't in the value set now returns false instead of a 500.

GraphQL

• Scoped queries: a source the requester can't view returns the same error as a missing source.
• Global queries (conceptIds or text), their mappings and the ES totalCount use the REST visibility rules. Staff are unaffected.

Users, pins, follows and events

• lastLoginSince/lastLoginBefore apply only for staff.
• extras.* filters match only the named extras field.
• User detail: subscribed_orgs lists only orgs the requester can view, and pins shows only for the user themselves and staff.
• Pins: reads and updates cover only pins whose org and resource the requester can view. The owner can still delete any of their pins.
• Following lists (/following/ and includeFollowing) show only follows of objects the requester can view. That includes the user themselves once they lose access.
• Following feeds show a followed user's private events to staff followers only. Org members still get them through scopes=orgs.
• Followed/Unfollowed events of objects the public can't view are private.
• Events: an event's objects are serialized live, so they're shown only to requesters who can view them now. Otherwise they're null, for example for a repo made private after the event, and the description is just the event type. Event lists can't be exported as CSV.
• $cascade entries list the collection versions their requester can view.

CSV export

• ?csv=true builds each export from the view's own queryset, so its access checks apply, and stores it under the requester. The response shape ({"url": ...}) is unchanged.

Behavior changes

• Non-staff lastLoginSince/lastLoginBefore params are ignored.
• extras.x= values containing : or \ are now matched literally.
• Every CSV export is built fresh: one DB query (at most 1,000 rows) and one upload per request.
• ValueSet $validate-code ignores org, user, collection and source query params. A version param only narrows the route's own collection.
• /parents/ and /children/ ignore q.
• Members of an org whose name matches another org's apart from letter case see only public mappings and repos when they search their own org; the same applies to a user whose username matches another's apart from case, searching their own repos. Production has 10 such org name groups and 16 username groups. Concept search is unaffected.
• Creating an org whose name differs from an existing one only in case returns 400 from the API and FAILED from bulk import.
• ?csv=true on event lists is ignored.
• Requesters who can't view a repo stop seeing it in the places above. Owners, org members and staff see the same as before.

Migration

• events/0011_private_follow_events (data only, reverse is a no-op) marks existing Followed/Unfollowed events of non-public orgs, repos and repo versions as private. On production it changes 2 rows and runs in seconds.

After deploy

• Clear the previously cached CSV exports (the files at the top level of the reader downloads folder). See step 4 of the checklist on OpenConceptLab/ocl_online#224.
• Merge OpenConceptLab/ocl_online#224 | Org ids unique regardless of case; concept /parents/ and /children/ ocl-docs#70, which documents the org id rule and the concept /parents/ and /children/ endpoints.

Verification

• Every fix commit carries a regression test that was run against the code before the fix (it fails there) and with the fix (it passes).
• Full suite, as CI runs it (manage.py test --parallel=1 --keepdb, PYTHONHASHSEED=2, fresh ES indexes): 2,586 tests, OK.
• pylint -j0 core/: 10.00/10.
• Prod EXPLAIN (no ANALYZE) of the new mapping-link query: index scans on to_concept_code, to_source_url and the membership table.
• The owner_text.keyword and source_text.keyword subfields exist in the production concepts index mapping.
• Review: three Codex defect passes, a simplification review and an adversarial review, then Codex checks of the later fix commits (clean). Details are on OpenConceptLab/ocl_online#224.

🤖 Generated with Claude Code

[snyaggarwal]

Reviewed commit fb7f85494233c4c958a35e72439f1c2526333319.

No actionable defects found in this review. I checked the changed access-control paths, including exact-case search scoping, FHIR repository selection, GraphQL visibility, deferred mapping links, hierarchy and collection membership, CSV exports, pins/follows/events, and the data migration.

Verification in an isolated checkout inside the existing API container:

• GraphQL, CodeSystem, ValueSet, pins, and events suites: 104 of 105 tests passed initially. test_fetch_concepts_global_search failed because the shared Elasticsearch index returned IDs outside the test database while the fixture concepts were absent from the results. A temporary diagnostic explicitly indexed/refreshed those fixture concepts, and the same assertion then passed. This is a local verification caveat, not a confirmed application defect.
• 53 tests passed covering public-read access, cross-repository access, search resolution, CSV export access, events, and the refreshed GraphQL diagnostic.
• Targeted pylint on the shared filtering/serialization code and affected GraphQL, concept, ValueSet, and event paths: 10.00/10.

I did not rerun the full project suite. No application files were changed.

[snyaggarwal]

Approved but requires merge conflicts to be handled

[snyaggarwal]

Second review after conflict resolution, at 2f184ea857b4339dc4c9cf42632d05817d5d859a (merging master b016e253 into the previously reviewed fb7f8549).

No actionable defects found in the merge resolutions.

I inspected Git's reconstructed conflict-resolution diff and compared the result against both parents, focusing on the interaction between master's batched list serialization and this PR's collection-version visibility checks:

• SourceChildPageSerializer forwards the requester from both normal request context and cascade's user context.
• Batched collection-version loading applies the visibility criteria through the correct expansion__ relation before populating each row's cached values.
• Concept and mapping serializers retain both the collection-version visibility mixin and master's list/source-version serialization behavior.
• The existing collection-version access regression covers anonymous users, outsiders, members, staff, version lists/details, and cascade; I checked that those paths now reach the merged implementation.

Verification: targeted pylint on all four resolution-related files passed at 10.00/10 in a temporary OCL image container; all 50 Python files changed against the current PR base parsed successfully; git diff --check passed and the resolved files contain no conflict markers.

Limitation: I did not rerun Django/integration tests for this commit. The OCL API, PostgreSQL, Redis, and Elasticsearch containers are stopped. The prior review's runtime results apply to the earlier commit, not this merged one. No application files were changed during this review.