Skip to content

Fix nested-document permission enforcement and query composition (helpdesk #123903) - #3738

Open
luis100 wants to merge 2 commits into
developmentfrom
fix/nested-documents-permissions-3737
Open

luis100 wants to merge 2 commits into
developmentfrom
fix/nested-documents-permissions-3737

Conversation

@luis100

@luis100 luis100 commented Sep 17, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #3737 — nested-document (ChildOfFilterParameter) search silently bypassed permission enforcement for non-admin users, and even once detection is fixed, the query composition itself was broken for the exact shape any real nested query needs.

Urgent / client-facing: this is the root cause of helpdesk #123903 — a customer's queries returned correct results for admin and 0 results for every other user with correct group-based read access. This PR is API-only and does not depend on any UI changes; it should ship independently and first.

What's fixed

  1. Permission detection never fired for realistic queries. hasNestedDocumentsFilter only checked the top-level filter parameter list, never recursing into AndFiltersParameters/OrFiltersParameters. Every real nested query combines ChildOfFilterParameter with additional child-level conditions this way, so RODA silently fell back to filtering the returned child document by fields only the parent AIP has (permissions, ancestors, ghost). Non-admin users got 0 results; admin bypassed the filter via an unrelated hardcoded check and still saw results — the exact reported symptom.

  2. Reachable ClassCastException from the public /api/v2/aips/find endpoint: the old code unconditionally cast filter.getParameters().getFirst() to ChildOfFilterParameter, which throws if the matching parameter isn't literally first.

  3. The real blocker — Solr query composition. Even with correct recursive detection, ANDing the permission clause into ChildOfFilterParameter.parentFilter as positional trailing text ({!child of=X} Y) is unsafe: Solr's parser does not reliably bind Y as the child qparser's argument when this clause is embedded among sibling AND/OR conditions — it silently degenerates into a flat boolean query instead of a real block-join, returning 0 results for every user, including admin. Verified empirically via Solr's debugQuery/parsedquery output (see issue Nested-document permission checks (ChildOfFilterParameter) are broken, and unsafe to fix without a Solr query-composition change #3737 for the before/after parsedquery strings). Fixed by binding the sub-query via Solr's inline v='...' local-param syntax instead, which is self-contained regardless of surrounding context. Applied to both appendBlockJoinChildrenFilterParameter and appendBlockJoinFilterParameter (the latter has the identical latent bug, fixed defensively).

  4. Admin bypass added to the nested-document permission-injection path (applyNestedDocumentsPermissions), mirroring the existing bypass in getFilterQueryPermissions used for the top-level permission filter. Previously admin had no special treatment here at all — the customer's own admin query only worked because their AIP explicitly granted permissions.users.READ: ["admin"].

  5. Bonus hardening: ordinary (non-nested) IndexedAIP searches now exclude nested child documents via Solr's managed _nest_path_ field, so any future nested-document producer can't leak raw child documents into normal top-level search results as flat hits.

Testing

Added NestedDocumentsPermissionsTest with a test-only XSLT crosswalk (bob.xslt) and fixture AIP (AIP_NESTED_PERMISSIONS, granting READ to a group only — admin has no explicit grant) producing real nested Solr documents, covering:

  • The exact customer query shape (ChildOfFilterParameter wrapped in AndFiltersParameters with child-level search conditions) for a group-permitted user vs. one outside the group.
  • Admin bypass on both the nested-document search path and the plain top-level search path.
  • Regression guards for query shapes that already worked before this fix (bare ChildOfFilterParameter, a simpler AND-wrapped shape).

Full suite: 243/243 passing.

Scope

Backend only (roda-core), no UI changes. #3667 (Nested Documents UI) currently carries its own, different fix for the same query-composition issue (defect 3) in SolrUtils.java, developed independently while building the Virtual Catalogue UI feature — that overlap will be reconciled by rebasing #3667 on top of this PR once merged, removing its duplicate/incompatible version of those two methods.


🤖 Generated with Claude Code

https://claude.ai/code/session_01Xn2jqXvJc99b9cd25o7saU

luis100 and others added 2 commits September 16, 2026 17:04
Testing docs incorrectly implied a manually-started docker-compose-dev
stack is required for roda-core-tests. RodaContainersLifecycleListener
already provisions ephemeral Solr Cloud/ZooKeeper/PostgreSQL/etc. via
Testcontainers per JVM run; only a reachable Docker daemon is needed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xn2jqXvJc99b9cd25o7saU
…(ChildOfFilterParameter) searches

Fixes #3737.

Three distinct bugs in SolrUtils.java's nested/block-join document search
against IndexedAIP (helpdesk #123903 — a customer got 0 results for a
non-admin user on a query where admin worked fine):

1. hasNestedDocumentsFilter only checked the top-level filter parameter
   list, never recursing into AndFiltersParameters/OrFiltersParameters.
   Every realistic nested query combines ChildOfFilterParameter with
   additional child-level conditions this way, so the permission-pushdown
   branch never triggered in practice, and RODA fell back to the standard
   top-level permission filter — which filters on fields the returned
   child document doesn't have (permissions/ancestors/ghost only exist on
   the parent AIP). Non-admin users were silently excluded; admin bypassed
   the filter via a separate, unrelated hardcoded check and still saw
   results, producing the reported role-dependent discrepancy.

2. A reachable ClassCastException: the old code unconditionally cast
   filter.getParameters().getFirst() to ChildOfFilterParameter whenever
   the (broken) top-level detection matched, which throws if the matching
   parameter isn't literally first. Reachable from the public
   /api/v2/aips/find endpoint.

3. Even once nested-query detection is made correctly recursive, ANDing
   the permission clause into ChildOfFilterParameter's parentFilter as
   positional trailing text ("{!child of=X} Y") is unsafe: Solr's query
   parser does not reliably bind Y as the child qparser's argument when
   this clause is embedded among sibling AND/OR conditions — it silently
   degenerates into a flat boolean query instead of a real block-join,
   returning 0 results for every user including admin. Verified via
   Solr's debugQuery/parsedquery output. Fixed by binding the sub-query
   via Solr's inline v='...' local-param syntax instead, which is
   self-contained regardless of surrounding context. Applied to both
   appendBlockJoinChildrenFilterParameter and appendBlockJoinFilterParameter
   (the latter has the same latent bug, defensively fixed even though no
   current caller combines it with sibling conditions).

Also added an admin bypass to the nested-document permission-injection
path (applyNestedDocumentsPermissions), mirroring the existing bypass in
getFilterQueryPermissions used for the top-level permission filter.

Additionally, ordinary (non-nested) IndexedAIP searches now exclude
nested child documents via Solr's managed _nest_path_ field
(NOT_NESTED_DOCUMENT_FILTER_QUERY), so any future nested-document
producer can't leak raw child documents into normal top-level results.

Added NestedDocumentsPermissionsTest (a test-only XSLT crosswalk +
fixture AIP producing real nested Solr documents) covering: the exact
customer query shape, admin bypass on both the nested and non-nested
search paths, and regression guards for the query shapes that already
worked before this fix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xn2jqXvJc99b9cd25o7saU
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.

1 participant