Fixes for Orthanc cloud plugin to facilitate complete ACL coverage as part of 0.4 release series
Summary
Server-side (Orthanc cloud plugin) ACL fixes that complete local-policy coverage for the 0.4 release and align the plugin's authorization behavior with the functional test suite. All changes are in the local-ACL policy builder (SonadorResourceAuthorizationView, sonador_orthanc/auth/web.py) — the view that answers the system/acl/resource introspection requests Sonador issues during authorization.
This is the Orthanc-side half of a coordinated pair; the Sonador-side consumer changes are in sonador!88. Both must be deployed together (see Deployment).
Background
The Orthanc Advanced Authorization plugin explodes a sub-resource request (e.g. POST /series/<id>/modify, POST /studies/<id>/worklists, GET /series/<id>/comments) into the full patient -> study -> series hierarchy and requires every level to be granted with the same method. With IncludeResourceUri enabled, only the leaf carries a non-empty URI; the ancestor levels arrive with an empty resource. Local grants do not, by design, propagate up the hierarchy, so the ancestor checks for a leaf operation would be denied and reject the whole request.
Changes
1. Comment grants imply ancestor view (_acl_effective_view)
A comment_view / comment_edit grant on a resource necessarily implies the ability to view (and therefore traverse to) it. The new _acl_effective_view() helper derives the ancestor-traversal view as view OR comment_view OR comment_edit, and is wired into the patient policy and the study branch of the resource policy. Without this, a local comment grant that was not also paired with an explicit view was denied at the ancestor level (the ancestor found no view, fell back to the empty global policy, and the all-levels-must-pass rule rejected the request).
2. Ancestor-traversal signals for modify/remove (modify_traverse / remove_traverse)
New policy keys, emitted on patient/study/series policies, set True only when a descendant modify/remove grant — or the existing worklist exemption, via the patient's own modify — justifies traversing the ancestor. The consumer (sonador!88) uses these for the empty-resource ancestor branch instead of a blanket view. The leaf still enforces the real modify/remove, so a descendant-only grant cannot modify/remove its ancestor directly (no privilege escalation). This replaces an earlier over-broad return self.view shortcut that also matched worklist-creation ancestors, which let a user without Modify create worklist items.
Why (test impact)
- Restores denial of worklist creation for users lacking Modify (fixes a regression introduced by the prior over-broad ancestor shortcut).
- Keeps leaf modify/remove working, via genuine and scoped ancestor traversal rather than a blanket view grant.
- Allows local comment grants to authorize the full request hierarchy (comment functional tests).
Deployment
Reload Orthanc to pick up sonador_orthanc. Deploy with sonador!88 — if the consumer (acl.py) is live but this builder is not yet emitting modify_traverse/remove_traverse, local ancestor modify/remove will regress. Verify a system/acl/resource response now includes comment_view/comment_edit and modify_traverse/remove_traverse.
A schema migration accompanies the comment permissions — alembic revision a7c4e1f2b9d3 adds comment_edit / comment_view columns to sonador_auth_user_study and sonador_auth_group_study.
Test evidence
These fixes are exercised by the ACL functional suite:
-
sonador-ftests!8 (merged) — ACL functional test matrix;
tests_sonadoracl_worklist_security.pyguards the worklist/modify-traversal regression andtests_sonadoracl_local_comments.pycovers the comment-implies-view path. - oak-tree/medical-imaging/sonador-client!71 — IO-client test hardening the matrix depends on.
Related issues
- sonador#53 — Sonador ACL and IDP improvements (umbrella tracking issue)
- sonador#55 (closed) — Sonador ACL MS1: Test Coverage Tracking (canonical inventory / functional-suite alignment)
- sonador#64 — Sonador ACL: Test Matrix and Coverage Assessment
-
sonador#75 (closed) — Add comment action parsing to enforce
comment_edit/comment_view - sonador#52 (closed) — Create a "comments" permission within Sonador
- Companion MR: sonador!88