DICOMweb: enforce per-operation permissions (can_query / can_retrieve / can_store) #380

The DICOMweb API defines three per-operation permissions in adit/dicom_web/models.py (DicomWebSettings.Meta.permissions, added in migration 0003_add_dicom_web_permissions):

• can_query
• can_retrieve
• can_store

None of them are enforced. adit/dicom_web/views.py carries three # TODO: respect permission can_* comments above QueryAPIView, RetrieveAPIView and StoreImagesAPIView. Access is gated only by server access (DicomNodeGroupAccess.source / .destination), so any user whose group can reach a server can query, retrieve and store on it, regardless of which of the three permissions that group holds.

Why this can't just be switched on

Django does not assign a newly created permission to existing groups. No group currently holds these permissions, so adding the checks on their own would return 403 to every current DICOMweb user and integration. These are token-authenticated scripts and pipelines, so the breakage would be silent — callers would simply start failing.

The migration below therefore has to land before enforcement, in the same release or an earlier one.

Action items

• Add a data migration in adit/dicom_web/migrations/ that grants the permissions to every group that already has DICOMweb access, mapping existing access to the equivalent permission so that current behaviour is preserved exactly:

  • group has a DicomNodeGroupAccess row with source=True → grant can_query and can_retrieve
  • group has a DicomNodeGroupAccess row with destination=True → grant can_store

• Enforce the permissions in adit/dicom_web/views.py, replacing the three TODOs — return 403 when the user's groups lack the permission for that operation (QueryAPIView → can_query, RetrieveAPIView → can_retrieve, StoreImagesAPIView → can_store).

  The permission check must run after the server-access lookup, not before. WebDicomAPIView._get_dicom_server currently raises NotFound (404) when the user's groups have no access to the server. If the permission check runs first, those requests return 403 instead and the five tests in TestNoServerAccessRejected break. Required behaviour: no server access → 404 (unchanged); server access but missing permission → 403.

• Update the existing positive tests in adit/dicom_web/tests/test_authorization.py::TestGrantedAccessPassesAuthLayer (4 tests). They currently grant server access only — the file grants no dicom_web permissions anywhere — and assert the response is not 401/403/404. Once enforcement is on they will receive 403 and fail, so they must also grant the relevant permission (see adit/dicom_web/utils/testing_helpers.py::create_dicom_web_group).

  The acceptance tests (tests/acceptance/test_qidors.py, test_stowrs.py, test_wadors.py) already use that helper and need no change.

• Un-xfail the three tests in adit/dicom_web/tests/test_authorization.py::TestPerOperationPermissionsNotEnforced. They are strict=True, so they will fail as XPASS the moment enforcement works — the markers must be removed in the same PR.

• Announce to API consumers before the enforcing release ships, so anyone running their own group setup knows the permissions now matter.

What this actually buys us

After the migration, every group holds exactly the permissions matching its current access, so nothing is restricted on day one — that is the point, it is what makes the rollout non-breaking.

The security benefit arrives afterwards, when an admin deliberately revokes a permission from a group in Django admin — for example removing can_store from a read-only research group so it can query and retrieve but no longer write to a PACS. That is per-deployment configuration, not a code change, and is out of scope for this issue.