Emit 'acl_traverse' ancestor-traversal signal for local ACL policy builder

Sonador Clauderequested to merge
claude/acl-traverse into master

Summary

Companion MR to oak-tree/medical-imaging/sonador!91 (enforces the acl permission for resource ACL policy-management requests, per sonador#73 (closed)). Both MRs must deploy together — see Deployment.

Background

The auth plugin's route parser now tags the internal ACL policy-management route's exploded patient → study → series hierarchy with action="acl" (mirroring action="comment"/"worklist" — see companion oak-tree/medical-imaging/orthanc-authorization MR). The ancestor (empty-resource) levels of that hierarchy therefore resolve via ResourceAuthorization.resource_perm()'s new acl branch (sonador!91), which consults an acl_traverse signal instead of the ancestor's own acl permission.

This is necessary because acl propagates downward only — a series-only grant contributes nothing to its parent study (confirmed correct and unchanged in this MR; only the traversal signal is new). Without acl_traverse, a series-scoped acl grant would be denied at the parent study/patient ancestor level, rejecting the whole hierarchy-exploded request even though the leaf (series) itself is legitimately authorized.

This exactly mirrors the existing modify_traverse/remove_traverse pattern established in !53 (merged) / sonador!88.

Changes

sonador_orthanc/auth/web.py:

  • New ACL_PERM_ACL_TRAVERSE = 'acl_traverse' constant, alongside ACL_PERM_MODIFY_TRAVERSE/ACL_PERM_REMOVE_TRAVERSE.
  • _fetch_patient_policy (patient ancestor): emits acl_traverse=True when the patient's own acl, or a descendant study/series acl grant, is set.
  • _fetch_resource_policy study branch (study ancestor): emits acl_traverse=True when the study's own (merged) acl, or a descendant series acl grant, is set.
  • _fetch_resource_policy series branch (leaf): emits acl_traverse=True mirroring the resolved acl grant (irrelevant on the leaf itself, which enforces the real acl directly, but kept for consistency with modify/remove).
  • Both response-serialization points (pick() in _fetch_resource_policy, and the user/group merge in resource_policy) now include ACL_PERM_ACL_TRAVERSE alongside the existing traverse keys, so the signal survives to the Sonador consumer.

Emitted only when True (same rule as modify/remove traverse): an explicit False would survive pick() and make an otherwise-empty no-grant policy non-empty, which the Sonador consumer treats as authoritative local policy instead of deferring to the global policy.

Test evidence

Initial static trace against the drafted acceptance suite (sonador-ftests!13 (merged), tests/tests_sonadoracl_manage.py) claimed no code changes were required. A subsequent live run against the deployed stack found this was wrong. 9/11 tests passed but testacl_local_scoped_study and testacl_local_scoped_series both failed with 403, while the equivalent global-scoped tests and the patient-level local test passed.

Root cause: in _fetch_patient_policy, the acl_traverse block was indented one level too deep — nested inside the body of the remove_traverse if statement instead of being a sibling condition. It only ran when remove_traverse's own condition (requiring a modify/remove grant somewhere in the hierarchy) was also true, which it never is for an acl-only local grant. This left the patient ancestor's acl_traverse unset for exactly the local study/series-scoped acl grants the two failing tests exercise — patient-level local grants need no ancestor traversal (pass trivially), and global-scoped tests don't depend on this builder-computed signal at all, which is why only those two cases were affected. _fetch_resource_policy's study and series branches were already correctly indented as siblings and did not have this bug.

Fixed with a whitespace-only dedent (commit 917bb511). Verified via AST parse and a minimal diff (8 insertions/8 deletions, no logic change). Awaiting a live re-run of testacl_local_scoped_study/testacl_local_scoped_series to confirm.

Deployment

Deploy with:

  • oak-tree/medical-imaging/sonador!91 — the consumer of this signal (ResourceAuthorization.acl_traverse). Without it, this MR alone has no effect (the key is emitted but never read).
  • orthanc-authorization!2 (merged) — without it, the plugin never tags the internal route's ancestor calls with action="acl", so they never reach sonador!91's acl branch to consult this signal at all (they'd fall through to modify_traverse/remove_traverse instead, denying acl-only ancestors that lack modify/remove).

Reload Orthanc to pick up sonador_orthanc. Verify a system/acl/resource response for a resource with a descendant acl grant now includes acl_traverse: true.

Related issues

Edited by Sonador Claude

Merge request reports