Skip to content

Nested-document permission checks (ChildOfFilterParameter) are broken, and unsafe to fix without a Solr query-composition change #3737

Description

@luis100

Summary

Nested-document search (ChildOfFilterParameter, block-join queries against IndexedAIP that return child metadata elements — e.g. an individual prelDecision under a BOB AIP — as first-class results) silently bypasses permission enforcement for non-admin users. Reported by a customer via helpdesk #123903: identical queries returned correct results for admin and 0 results for every other user, regardless of actual group-based read access.

Root-caused to three distinct, verified defects, all in roda-core/roda-core/src/main/java/org/roda/core/index/utils/SolrUtils.java. A straightforward fix for the first was implemented and then reverted after it was empirically shown to make the customer's real query return 0 results for everyone, including admin — because fixing #1 in isolation triggers #3. All three need to be fixed together.

This blocks #3662 (Virtual Catalogue, Phase 2 of #3382), which will exercise ChildOfFilterParameter from production UI flows and will hit these same defects.

Defect 1 — hasNestedDocumentsFilter only inspects top-level filter parameters

// SolrUtils.java:407-413
private static <T extends IsIndexed> boolean hasNestedDocumentsFilter(Filter filter, Class<T> classToRetrieve) {
  if (!IndexedAIP.class.isAssignableFrom(classToRetrieve)) { return false; }
  return filter.getParameters().stream().anyMatch(ChildOfFilterParameter.class::isInstance);
}

It never recurses into AndFiltersParameters/OrFiltersParameters (FiltersParameters.getValues()). Every realistic nested query needs to combine ChildOfFilterParameter with additional child-level search conditions inside an AndFiltersParameters — a bare ChildOfFilterParameter with no other constraint is a degenerate case. So this detection effectively never fires for real queries, and SolrUtils.find (line ~359) falls back to the standard top-level permission fq, which filters the returned document. Since the returned document is the child (permission/ancestor/ghost fields only exist on the parent AIP), every non-admin user is silently excluded. Admin bypasses the top-level fq via a hardcoded username check (getFilterQueryPermissions, ~line 1406: !RodaConstants.ADMIN.equals(user.getName())), so admin still sees results — exactly reproducing the reported symptom.

Defect 2 — reachable ClassCastException in the same branch

// SolrUtils.java:364-365 (current code)
ChildOfFilterParameter childOfFilter = (ChildOfFilterParameter) findRequest.getFilter().getParameters().getFirst();

Unconditional cast on .getFirst() whenever hasNestedDocumentsFilter returns true via anyMatch(...). Throws if the matching ChildOfFilterParameter isn't literally the first element of the top-level parameter list. Reachable from the public /api/v2/aips/find endpoint — SecurityFilteringUtils.sanitizeFindRequest only strips unauthorized facets, never touches filter parameters.

Defect 3 — THE BLOCKER: {!child of=X} Y does not bind Y as the child qparser's argument when embedded among sibling AND/OR clauses

This is why #1 cannot be fixed by itself — a naive fix (make detection recursive, AND the permission clause into ChildOfFilterParameter.parentFilter) is unsafe. Verified empirically via Solr's debugQuery=true / response.getDebugMap().get("parsedquery"):

  • ChildOfFilterParameter as the entire top-level query (parentFilter non-null):
    q={!child of=(content_type:"BOB")} (permission clause)
    → parsedquery=ToChildBlockJoinQuery(...) — correctly performs the join. Works.

  • The same clause embedded as one AND-ed sibling (q=({!child of=(content_type:"BOB")} (permission clause) AND (content_type:"prelDecision"))):
    → parsedquery=+(+BitSetProducerQuery(...) +(permission clause) +(content_type:preldecision))
    No ToChildBlockJoinQuery at all. Solr's default parser does not bind the trailing text after {!child of=X} as that qparser's argument in this position — it degenerates {!child of=X} into a bare block-membership filter and parses the would-be argument and the sibling clause as three independent flat boolean terms, evaluated against the child document directly. numFound=0 for every user, including admin — confirmed against the raw Solr response, not just RODA's deserialization layer.

Causal chain that makes a naive fix of #1 actively harmful: in the customer's real query (their working admin request), ChildOfFilterParameter.parentFilter was null at compose time, so appendBlockJoinChildrenFilterParameter took its else branch and emitted a bare {!child of=X} with no trailing text — which, embedded among AND siblings, happens to still work (this is why their admin query succeeded). Any fix that makes hasNestedDocumentsFilter recursive and injects the permission clause into parentFilter turns that null into non-null trailing text — exactly the shape that breaks per the evidence above. Such a fix returns 0 results for every user, including admin, for the customer's exact query shape. This was caught before shipping via a throwaway integration test with a custom XSLT crosswalk producing real nested Solr documents, plus the debugQuery inspection above — not from reading the code alone.

Recommended fix for Defect 3 (design only — not yet implemented or verified)

Bind the parent-selector sub-query via Solr's inline v='...' local-param syntax instead of positional trailing text, which sidesteps the ambiguity entirely (there's no trailing text left for the outer parser to misparse — the whole clause is self-contained within one set of {! } braces):

  • Current (broken when embedded): {!child of=X} Y
  • Fixed: {!child of=X v='<Y, escaped>'}

Scoped changes, both in SolrUtils.java:

  • appendBlockJoinChildrenFilterParameter (~line 1093): when parentFilter != null, build the someParents sub-query into its own string, escape it (backslash-escape backslashes, then single quotes, per Solr local-params string escaping), and emit {!child of=X v='<escaped>'} instead of {!child of=X} <someParents>.
  • appendBlockJoinFilterParameter (~line 1077, ParentWhichFilterParameter / {!parent which=...}): same code shape, same latent bug. No in-tree caller currently combines it with sibling conditions (RepresentationInformationService uses it as a sole top-level filter), but Nested Documents UI — Phase 1: Advanced AIP Search with Nested Filter Groups #3661 (Advanced AIP Search: Nested Filter Groups) will need to, so it should get the same defensive fix now.
  • No broader signature refactor needed — someParents/someChildren are already locally-scoped StringBuilders in these two methods; this only changes how they're spliced into the output.

Then, with #3 addressed:

  • Fix Mavenize the project #1: make hasNestedDocumentsFilter recurse into FiltersParameters.getValues().
  • Fix Make initial import to GitHub repository #2: replace the .getFirst() cast with a recursive walk that finds every ChildOfFilterParameter at any depth and ANDs buildQueryPermissions(user) into its parentFilter.
  • Verify with debugQuery/parsedquery again to confirm ToChildBlockJoinQuery appears (not the flattened BitSetProducerQuery shape) for the AND-embedded case, before trusting result counts.

Defect 4 — no admin bypass in the nested-doc permission-injection path

buildQueryPermissions (used to build the permission clause injected into parentFilter) has no admin bypass, unlike getFilterQueryPermissions (used for the top-level fq, which skips entirely for RodaConstants.ADMIN). This is a real inconsistency, but it can't be validated or safely shipped until Defect 3 is fixed, since exercising it needs a query shape that's currently broken at the Solr level regardless of the permission logic.

Clarifying note: the customer's admin query worked because their AIP's aip.json explicitly granted permissions.users.READ: ["admin"], confirmed in the helpdesk thread — not because of a system-wide admin bypass in this code path. Don't assume admin universally bypasses nested-doc permission checks today; it doesn't, until Defect 4 is fixed.

Verification approach for whoever picks this up

A throwaway integration test was built (not committed) to validate this: a custom XSLT crosswalk (config/crosswalks/ingest/bob.xslt) producing real nested Solr documents via SolrXMLLoader's labeled-child-doc parsing (<field name="X"><doc>...</doc></field> → SolrInputDocument.addField(name, childDoc), confirmed to survive indexDescriptiveMetadataFields's merge onto the parent AIP doc), and a fixture AIP granting READ to a specific group only. The debugQuery=true / response.getDebugMap().get("parsedquery") technique is the fastest way to confirm whether a given query shape produces a real ToChildBlockJoinQuery versus silently degenerating — recommend using it to validate any fix before trusting totalCount assertions alone.

Scope

Backend only (roda-core), no UI changes. Should land before or alongside #3662.


🤖 Filed with Claude Code while investigating helpdesk #123903.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions