[Closed — superseded] Draft: local ACL / data-tenancy functional tests for MS1

Status: Closed, superseded by the current tests_sonadoracl_* suite

What this MR set out to do

Opened in May 2024 as an early attempt at the "Local Permissions" test suite tracked by the MS1 milestone (Sonador Security Extensions: Access Control). It staged NIH-CXR test data and drafted functional tests covering:

• Server-level upload and query permissions
• Global (server-wide policy) view access at the series, study, and patient scope
• The milestone's three explicit Local Permissions checklist items:
  1. User data tenancy control — admin grants upload, test user uploads, admin creates a local Orthanc grant for the uploaded resource, verify access
  2. Inverse — revoke the grant, verify access is denied
  3. Grant to a second user — user 1 uploads, a second limited user is granted local access, access is verified, the grant is removed, access is denied again

Why it wasn't merged

The branch (bharp/acl_tests) sat as a draft for roughly two years while the ACL model itself was substantially redesigned — new local (Orthanc-resource-level) ACL payload shapes, a descend_only scope semantics, an acl permission of its own (sonador#73 (closed)), and DICOMweb-mirrored management routes were all added after this was drafted. By the time it was revisited it was 19 commits behind master and exercised policy shapes/endpoints that no longer match what's shipped — rebasing it would have amounted to a rewrite rather than a merge.

Closeout review (2026-07)

As part of closing out #4 (closed), I re-reviewed this draft against the test suite that has since been built out (tests/tests_sonadoracl_*.py) to confirm there's no coverage gap left behind by not merging it. Every scenario here has a current equivalent, generally with materially more depth (query/modify/remove/acl are now first-class per-resource-level test cases, which this draft hadn't yet reached):

This draft	Current equivalent
Server upload permission	tests/tests_sonadoracl_global.py::testacl_server_upload_data
Server query permission	tests/tests_sonadoracl_global.py::testacl_server_query_data, plus discrete series/study/patient allow-deny cycles in tests/tests_sonadoracl_query.py
Global series/study/patient view	tests/tests_sonadoracl_global.py::testacl_sonador_{series,study,patient}_access_permission
Local series/patient view	tests/tests_sonadoracl_local_view.py::testacl_local_series_view_data_tenancy, testacl_local_patient_view_{user,group}_grant
Local study view (propagation to child series)	tests/tests_sonadoracl_manage.py::testacl_local_scoped_study
Local Permissions Test 1 (upload, admin grants local Orthanc access, verify authorized)	tests_sonadoracl_local_view.py::testacl_local_series_view_data_tenancy (steps 1–5: upload-only does not grant view; local series ACL grant does)
Local Permissions Test 2 (inverse — revoke, verify denied)	same test, steps 6–7 (local ACL revoked → 403)
Local Permissions Test 3 (grant to a second user, then revoke)	Each constituent mechanic is covered, if not chained in one literal test: granting a local view ACL to a distinct beneficiary user/group is exercised repeatedly in tests_sonadoracl_manage.py's creator/beneficiary pattern (TESTUSER01 grants TESTUSER02/TESTGROUP02 access, verified); revoking a local grant and confirming the resulting 403 is exercised in tests_sonadoracl_local_view.py and tests_sonadoracl_modify_remove.py's local-ACL tests.

Net result: no functionality hole. The milestone's "Local Permissions" checklist is fully covered by the current suite. Closing rather than merging since a rebase here would be a rewrite against an already-superseded design, and the underlying goal has already been reached through a different implementation path.

Thanks for the groundwork here, Brandon — it correctly mapped out the test surface this area needed, even though the implementation needed to catch up to where the ACL model eventually landed.

Related

• #4 (closed) — DICOMweb endpoint audit and functional-test closeout this review was performed as part of.
• sonador#73 (closed) — the acl permission and local-grant redesign that superseded this branch's approach.