DICOMweb resource management endpoints: remove a study or series by DICOM UID (DELETE .../manage)
## 1. Overview Add two DICOMweb-rooted **resource management** endpoints to the Sonador Orthanc plugin so that an authorized DICOMweb client can remove an imaging resource addressed by its **DICOM UID**: ``` DELETE {dicomweb_root}/studies/{StudyInstanceUID}/manage DELETE {dicomweb_root}/series/{SeriesInstanceUID}/manage ``` Each endpoint resolves the DICOM UID to a Sonador cache resource, confirms it exists, and answers with a **redirect** to that resource's own Orthanc API URL, where the actual deletion is performed. This is the same shape as the existing archive-download endpoints in `sonador_orthanc/web/download.py`, transposed from `GET`/`archive` to `DELETE`/`manage`. Authorization is enforced outside these views, by the `orthanc-authorization` plugin consulting the Sonador ACL system, exactly as it is for every other DICOMweb route. **However**, the Sonador web application cannot currently classify a `/manage` route and will deny it for every non-superuser. Closing that gap is in scope for this issue and is specified in §6. Scoping headline: this issue delivers the **API only**. The OHIF study-list and viewer controls that call it are tracked separately under oak-tree/medical-imaging/imaging-development-env#64. ### Why a `manage` path component `DELETE` of a study or series is not part of the DICOMweb standard, so there is no standard path to implement. The endpoint is therefore an *extension* route and needs a path component that distinguishes it from the standard study/series resource paths. `manage` is deliberately generic: this issue delivers removal, but the segment is the natural home for future management operations (anonymize, modify, reindex) without a second naming decision. --- ## 2. Background and current state ### 2.1 The only delete path today is unreachable from a DICOMweb client Deletion already works, but only when addressed by **Orthanc public ID**: | View | File | Route | |---|---|---| | `SonadorStudyResourceView` | `sonador_orthanc/cache/web/study.py` | `/studies/([0-9a-fA-F]{8}\-?){5}` | | `SonadorSeriesResourceView` | `sonador_orthanc/cache/web/series.py` | `/series/([0-9a-fA-F]{8}\-?){5}` | Both are registered in `sonador_orthanc/cache/__init__.py::init()` and inherit `delete` from `SonadorResourceBaseView` (`sonador_orthanc/web/resource.py`): ```python def delete_resource(self, session, resource, *args, response=None, **kwargs): response = response or {} r = ResponseLikeObject(orthanc.RestApiDelete( local_orthanc_apiurl(posixpath.join(self.resource_base, resource.publicid), query_params=self.GET))) if r.text: response.update(r.json()) return response ``` A DICOMweb client — including the OHIF frontend — holds `StudyInstanceUID` / `SeriesInstanceUID`, not Orthanc public IDs. There is no `/dicom-web/studies/{uid}` or `/dicom-web/series/{uid}` `DELETE` route anywhere in the plugin. Verified by reading every `orthanc.RegisterRestCallback` call site: `sonador-plugin.py`, `sonador_orthanc/cache/__init__.py`, `sonador_orthanc/web/dicomweb.py` (all seven `init_*` functions), `sonador_orthanc/auth/__init__.py`, `sonador_orthanc/worklist/__init__.py`, `sonador_orthanc/web/comments.py::init_comments`. ### 2.2 The reference implementation: `download.py` `DownloadBaseView(DicomResourceMixin, CacheBaseView)` is the normative pattern to follow. It resolves the UID, fetches the imaging resource, and redirects: ```python def get(self, output, uri, request, *args, **kwargs): try: with self.sessionmaker() as session: obj = self.get_object(session, *args, **kwargs) # Redirect to study download endpoint return self.send_response('', status_code=302, headers={ 'Location': obj.pacs.orthanc_apiurl_fqdn(obj.filearchive_url, internal_dns=False) }) except ResourceDoesNotExist as e: return self.http404_resource_not_found(message=str(e)) except Exception as err: emsg = 'Unable to download archive for resource=%s uid=%s. Error:\n%s' % ( self.resource_type, self.get_resource_uid(*args, **kwargs), err) logger.error('%s\n%s' % (emsg, traceback.format_exc())) return self.send_response(json.dumps({ gcapicodes.ERROR: emsg, gcapicodes.STATUS: gcapicodes.FAIL, }, cls=SonadorJsonEncoder), status_code=500) ``` Two of its details must **not** be copied verbatim — see AR-1 (status code) and AR-5 (the 404 call convention). The UID is resolved by `DicomResourceMixin.get_resource_uid` (`sonador_orthanc/web/dicomweb.py`), which seeds the URI by picking the first path segment that is numeric-with-dots: ```python resource_uri = resource_uri or first(self.uri.split('/'), key=lambda s: s.replace('.', '').isnumeric()) ``` For `/dicom-web/studies/1.2.840.113619.2.55/manage` this yields `1.2.840.113619.2.55`; `manage` is skipped because it is not numeric. **The parser needs no change.** `get_resource` then maps the UID through `DicomIdentifiers` to a `Resource` row, raising `ResourceDoesNotExist` when absent. ### 2.3 `manage` does not collide with anything — verified Complete inventory of first path segments currently registered under the two DICOMweb prefixes: | Prefix | Registered sub-segments | |---|---| | `{dicomweb_root}/studies/(\d+(\.\d+)+)/` | `series`, `series/{uid}/metadata`, `series/{uid}/instances/{uid}`, `series/{uid}/instances/{uid}/frames/{n}`, `archive`, `comments`, `comments/{uuid}`, `worklists`, `worklists/{uuid}`, `acl/user`, `acl/user/{uuid}`, `acl/group`, `acl/group/{uuid}`, `resource-acl` | | `{dicomweb_root}/series/(\d+(\.\d+)+)/` | `archive`, `comments`, `comments/{uuid}`, `acl/user`, `acl/user/{uuid}`, `acl/group`, `acl/group/{uuid}`, `resource-acl` | No registered pattern contains `manage`. The only catch-all regex in the plugin is `{dicomweb_root}/servers/(.+)`, which is scoped under `servers/` and cannot match. The Sonador DICOMweb root is also a deliberately separate namespace from the stock DICOMweb plugin's root (`init_dcmweb_system_endpoints` warns and installs forwarding views when `SonadorDicomWebRoot != Root`), which further reduces collision risk with vendor routes. Confirming that neither Orthanc core nor the DICOMweb plugin defines a `manage` sub-resource is a verification task (§5.4, V-1) — the repo vendors neither source nor an `orthanc.json`, so it could not be settled from code. ### 2.4 Where the redirect actually lands `/studies/{orthanc-id}` and `/series/{orthanc-id}` are already claimed by the Sonador plugin (§2.1), so the redirected `DELETE` is handled by `SonadorStudyResourceView.delete` / `SonadorSeriesResourceView.delete`, which then proxy to Orthanc core via `orthanc.RestApiDelete`. Net behavior is the same as hitting Orthanc directly, with two consequences: * Success is always **HTTP 200** (`send_response` with no `status_code` calls `AnswerBuffer`); the upstream Orthanc status is not propagated. Response body is Orthanc's JSON (`{"RemainingAncestor": {...}}` or `null`). * `SonadorResourceBaseView._execute_resource_request` carries a live format-string defect: the generic `except Exception` branch formats `emsg_404` (a single-`%s` template) with a 2-tuple, and the `emsg_500` argument it accepts is never used. Any non-404 failure raises `TypeError: not all arguments converted during string formatting` from inside the handler instead of returning the intended 500 JSON. This path is currently reachable only by an operator with an Orthanc ID; `manage` makes it reachable from the browser, so fixing it is in scope (FR-8). ### 2.5 The Sonador web application will deny these routes today The `orthanc-authorization` plugin cannot classify sub-routes outside a closed enum (`series` / `metadata` / `instances` / `rendered` / `thumbnail`), so extension routes arrive at Sonador as `level="system"` with the raw URI. `OrthancServiceAuthorizationForm.clean_auth_request()` (`apps/visionaire/auth/forms/orthanc.py` in oak-tree/medical-imaging/sonador) exists precisely to re-derive `level` and `dicom_uid` for those. It has branches for worklists, `/archive`, `/comments`, `/acl/{user,group}`, `resource-acl`, distortion filters, and `/_dicom-web/...` — **and none for `/manage`.** Trace for an unclassified `DELETE`: 1. `PacsImagingServerGroupAuthorization.user_has_perm()` accepts `level == ORTHANC_SYSTEM` and fetches local ACLs. 2. `ResourceAuthorization.resource_perm()` with `level == 'system'` has only two system branches (a `GET`-on-`/dicom-web/studies` view branch and two comments branches). A `DELETE` matches none, returning `None`. 3. The global-policy fall-through calls `resource_perm` with `level='system'` again — `None` again. 4. Result: `granted: False`, even for a user holding a global wildcard `remove` grant. Once `level` is corrected to `study`/`series` and `dicom_uid` is populated, the **existing** evaluator handles the decision correctly with no further change: ```python elif level in orthanc_api.ORTHANC_IMAGING_RESOURCES and method.lower() == gapicodes.HTTP_DELETE.lower(): if not resource: return self.remove_traverse if resource and orthanc_api.ORTHANC_COMMENTS in resource: return self.comment_edit return self.remove ``` `ORTHANC_IMAGING_RESOURCES = {'patient', 'study', 'series', 'instance'}`, so `remove` is already evaluated at both study and series granularity, and the ACL models already carry it (`UserSeriesAuth` / `GroupSeriesAuth` in `sonador_orthanc/db/auth.py`; `PacsImagingServerGroupAuthorization.remove` in the web app). **No new permission is introduced by this work.** ### 2.6 Authentication across the redirect `UserContextMixin.get_user_creds` (`sonador_orthanc/web/secure_user.py`) accepts three credential carriers: the `Authorization` header, a `?token=` query parameter, or a `token` header. The existing download flow survives its redirect because the redirect is **same-origin**: the request goes to `https://{orthanc-fqdn}{dicomweb_root}/studies/{uid}/archive` and `Location` resolves to `https://{orthanc-fqdn}/studies/{orthanc-id}/archive`. `fetch()` and `requests` both strip `Authorization` only when a redirect changes origin, so a same-origin redirect preserves it. That holds only while the public `OrthancServerScheme` / `OrthancServerHostname` / `OrthancServerPort` registered in Sonador exactly match the origin the client used. Confirming this per environment is verification task V-3. --- ## 3. Functional requirements **FR-1 — Study removal endpoint.** `DELETE {dicomweb_root}/studies/{StudyInstanceUID}/manage` resolves the UID to a `CacheStudy` resource and answers with a redirect whose `Location` is that study's Orthanc API URL on the public FQDN. **FR-2 — Series removal endpoint.** `DELETE {dicomweb_root}/series/{SeriesInstanceUID}/manage` does the same for a `CacheSeries` resource. **FR-3 — The redirect preserves the request method.** The response status is **307**, not 302 (AR-1). A client following the redirect must issue a second `DELETE`. **FR-4 — Unknown UID answers 404.** A UID with no `DicomIdentifiers` mapping produces `{Error, Status: "fail"}` with status 404, matching the download views' `ResourceDoesNotExist` branch. **FR-5 — Server errors answer 500.** Any other exception is logged with a traceback and answered as `{Error, Status: "fail"}` with status 500, matching `DownloadBaseView`. **FR-6 — Only DELETE and OPTIONS are accepted.** `GET`, `HEAD`, `POST`, `PUT`, and `PATCH` on `.../manage` return 405 via `http_method_not_allowed`. Specifically, `GET .../manage` must **not** fall through to an inherited archive-download handler (AR-3). **FR-7 — `OPTIONS` advertises the allowed methods.** The inherited `OrthancBaseView.options` returns 204 with `Allow: DELETE, OPTIONS`. No new code, but it must be correct — a cross-origin `DELETE` always triggers a preflight. **FR-8 — The redirect target's error path returns valid JSON.** Fix `SonadorResourceBaseView._execute_resource_request` so its generic exception branch formats `emsg_500` with the correct argument count (§2.4). A failed delete must produce a 500 JSON body, not a `TypeError` inside the handler. **FR-9 — The Sonador web application classifies `/manage` and authorizes on `remove`.** A user holding a `remove` grant on the resource (or an ancestor, per the existing inheritance rules) is granted; a user without one is denied with 403. See §6. **FR-10 — Deletion semantics are Orthanc's, unchanged.** Removal is a hard delete that cascades to child series and instances; the existing Lua/change-callback pipeline (`lua/SonadorEvents.lua` -> `sonador_orthanc/web/events.py` -> `sonador_orthanc/tasks/maintenance/cache.py::remove_cache_resource`) prunes the Sonador cache row, its tags, comments, worklist items, and **its user and group ACL grants**. This issue introduces no new cascade behavior and must not alter the existing one. --- ## 4. Architectural requirements **AR-1 — Use 307, not 302. This corrects the originating design note.** RFC 7231 §6.4.3 permits a user agent to rewrite the method to `GET` on a 302, and browsers do so inconsistently for non-`GET` methods. A `DELETE` rewritten to `GET` would land on `SonadorStudyResourceView.get` and return **200 with study JSON, having deleted nothing** — a silent no-op that reads as success, the worst available failure mode for a destructive operation. The repository already establishes 307 as the convention for exactly this reason; `RedirectView` in `orthanc-sonador-common` `web.py`: ```python class RedirectView(OrthancBaseView): # 307 is used to preserve method and body forward_status_code = 307 ``` `DicomWebRedirectView` (used for `{dicomweb_root}/servers/(.+)` with `allow_delete = True`) already inherits it. There is no body to preserve on a `DELETE`, so 307 costs nothing. Expose the value as a class attribute (`redirect_status_code = 307`) so it remains settable through `as_view` — see AR-6. **AR-2 — New module `sonador_orthanc/web/manage.py`, not `sonador_orthanc/web/ext/`.** The `ext/` hierarchy is built entirely around a Sonador database model plus a pydantic form plus a JSON serializer; `ObjectViewMixin.init_object_mixin` raises `ConfigurationError` when `orthanc_objectjson` is not callable. The management endpoints have no model, no form, and no persistence — they resolve a UID and redirect, which is what `download.py` does. Place `manage.py` beside `download.py` and mirror its structure. **AR-3 — Do not subclass `DownloadBaseView`.** It defines `get`, and `dispatch` resolves handlers with `getattr(self, self.method.lower(), ...)`, so `GET .../manage` would return a redirect to the *archive*. Build a sibling base directly on `DicomResourceMixin, CacheBaseView`. A view with no `get` also skips the `head = get` aliasing in `OrthancBaseView.setup`, so `_allowed_methods()` correctly reports `['DELETE', 'OPTIONS']` (FR-6, FR-7). **AR-4 — The redirect target is `resource_url`, not `filearchive_url`.** In `sonador-client` `imaging/orthanc/base.py`, `ImagingStudy.resource_url` and `ImagingSeriesCoreResource.resource_url` both return `posixpath.join(self.fetch_endpoint, self.pk)`, yielding `studies/{orthanc-id}` and `series/{orthanc-id}`. This is the same property the client's own `delete()` uses. The expression to emit: ```python 'Location': obj.pacs.orthanc_apiurl_fqdn(obj.resource_url, internal_dns=False) ``` `orthanc_apiurl_fqdn` is defined on `OrthancCloudInternalImagingServer` in `sonador_orthanc/manager.py` and resolves to the **public** scheme/host/port registered in Sonador when `internal_dns=False`. **AR-5 — Call `http404_resource_not_found(response={...})`, not `(message=...)`.** `http404_resource_not_found` reads `message` via `kwargs.get(...)` without popping it, then forwards `**kwargs` to `send_response`, which has no `message` parameter. The `message=str(e)` form used in `download.py` therefore raises `TypeError: send_response() got an unexpected keyword argument 'message'` — a latent defect on the existing download 404 path. Use the `response={gcapicodes.ERROR: str(e)}` form that every other view in the codebase uses. (Fixing the download view's copy of this is optional and out of scope; do not silently inherit it.) **AR-6 — Any configurable attribute must exist as a class attribute.** `OrthancBaseView.as_view(**initkwargs)` rejects a kwarg unless `hasattr(cls, key)` and rejects any kwarg named after an HTTP method. `sonador_manager` and `sessionmaker` work only because `CacheBaseView` declares them as `None`; `redirect_status_code` must be declared the same way. **AR-7 — No permission logic in the views.** Authorization stays where it is: the `orthanc-authorization` plugin intercepts the request and consults Sonador. The views verify existence only, exactly as `DownloadBaseView` does. Do not add an ACL mixin, a decorator, or a permission helper call. **AR-8 — `manage` is a generic management namespace.** Name the module, views, and registration function for *management*, not for deletion (`ManageBaseView`, `StudyDICOMManageView`, `SeriesDICOMManageView`, `init_manage_endpoints`), so a later `PUT`/`POST` management operation extends the same views rather than needing a new route. --- ## 5. Implementation specification ### 5.1 Route table | Method | Path | View | Success | Errors | |---|---|---|---|---| | `DELETE` | `{dicomweb_root}/studies/(\d+(\.\d+)+)/manage` | `StudyDICOMManageView` | 307 + `Location: {public}/studies/{orthanc-id}` | 404 unknown UID, 500 otherwise, 403 from the auth plugin | | `DELETE` | `{dicomweb_root}/series/(\d+(\.\d+)+)/manage` | `SeriesDICOMManageView` | 307 + `Location: {public}/series/{orthanc-id}` | same | | `OPTIONS` | both | inherited | 204 + `Allow: DELETE, OPTIONS` | — | | any other verb | both | inherited | 405 | — | `dicomweb_root = DicomWeb.SonadorDicomWebRoot or DicomWeb.Root`, normally `/dicom-web/`. ### 5.2 `sonador_orthanc/web/manage.py` (new) Mirror `download.py`. Skeleton, to be completed against the reference file: ```python class ManageBaseView(DicomResourceMixin, CacheBaseView): ''' Resource management views which proxy DICOM-UID addressed management operations to the Orthanc internal API. Authorization is enforced by the orthanc-authorization plugin against the Sonador ACL system; these views verify resource existence only (AR-7). ''' # 307 preserves the request method across the redirect. A 302 permits the # user agent to rewrite DELETE to GET (AR-1). redirect_status_code = 307 def setup(self, output, uri, request, *args, **kwargs): super().setup(output, uri, request, *args, **kwargs) self.init_resource_mixin(*args, **kwargs) @abc.abstractmethod def get_object(self, session, *args, **kwargs): ''' Retrieve the imaging resource instance to be managed. ''' def delete(self, output, uri, request, *args, **kwargs): try: with self.sessionmaker() as session: obj = self.get_object(session, *args, **kwargs) return self.send_response('', status_code=self.redirect_status_code, headers={ 'Location': obj.pacs.orthanc_apiurl_fqdn(obj.resource_url, internal_dns=False) }) except ResourceDoesNotExist as e: return self.http404_resource_not_found(response={gcapicodes.ERROR: str(e)}) except Exception as err: emsg = 'Unable to remove resource=%s uid=%s. Error:\n%s' % ( self.resource_type, self.get_resource_uid(*args, **kwargs), err) logger.error('%s\n%s' % (emsg, traceback.format_exc())) return self.send_response(json.dumps({ gcapicodes.ERROR: emsg, gcapicodes.STATUS: gcapicodes.FAIL, }, cls=SonadorJsonEncoder), status_code=500) class StudyDICOMManageView(ManageBaseView): resource_type = CacheStudy.type resource_code = CacheStudy.code def get_object(self, session, *args, **kwargs): r = self.get_resource(session, *args, **kwargs) return self.sonador_manager.get_internal_imageserver().get_study(r.publicid) class SeriesDICOMManageView(ManageBaseView): resource_type = CacheSeries.type resource_code = CacheSeries.code def get_object(self, session, *args, **kwargs): r = self.get_resource(session, *args, **kwargs) return self.sonador_manager.get_internal_imageserver().get_series(r.publicid) ``` Imports follow `download.py`: `client.apisettings as gcapicodes`, `client.errors.ResourceDoesNotExist`, `sonador.serialization.SonadorJsonEncoder`, `..db.cache.{CacheStudy, CacheSeries}`, `..cache.web.base.CacheBaseView`, `.dicomweb.DicomResourceMixin`. ### 5.3 `sonador_orthanc/web/dicomweb.py` Add `init_manage_endpoints(orthanc_conf, sonador_manager, OrthancSession)`, modelled directly on `init_download_endpoints`. Import the views **inside** the function, as `init_download_endpoints` and `init_ext_endpoints` do, to avoid a circular import. Resolve `dicomweb_root` the same way and raise `ConfigurationError` when it is absent. Log each registration with `orthanc.LogWarning`, matching the surrounding style. ```python orthanc.RegisterRestCallback(posixpath.join(dicomweb_root, r'studies/(\d+(\.\d+)+)/manage'), StudyDICOMManageView.as_view(sonador_manager=sonador_manager, sessionmaker=OrthancSession)) orthanc.RegisterRestCallback(posixpath.join(dicomweb_root, r'series/(\d+(\.\d+)+)/manage'), SeriesDICOMManageView.as_view(sonador_manager=sonador_manager, sessionmaker=OrthancSession)) ``` ### 5.4 `sonador-plugin.py` Call `sonador_dicomweb.init_manage_endpoints(...)` from `orthanc_cache_onstart`, immediately after the existing `init_download_endpoints(...)` call. ### 5.5 `sonador_orthanc/web/resource.py` Fix `_execute_resource_request` (FR-8): the generic exception branch must format `emsg_500` with the arguments it actually receives. Confirm both the 404 and non-404 branches return well-formed JSON, and add a regression test. ### 5.6 Verification tasks for the implementer These could not be settled from the repositories and must be confirmed before the endpoint is considered done. Record the answers in this issue. * **V-1 — `manage` is free upstream.** Confirm neither Orthanc 26.6.1 core nor the DICOMweb plugin defines a `manage` sub-resource on `studies`/`series`, and confirm against the deployed `orthanc.json` (which lives in oak-tree/medical-imaging/imaging-development-env, not here). Core's known study sub-resources are `anonymize, archive, attachments, instances, instances-tags, labels, media, merge, metadata, modify, module, patient, reconstruct, series, shared-tags, split, statistics`; the DICOMweb plugin's are `series, instances, metadata, rendered, bulk`. Neither list contains `manage`, but this is knowledge-based rather than verified against the deployed build. * **V-2 — CORS allows `DELETE`.** A cross-origin `DELETE` always triggers an `OPTIONS` preflight, and the preflight cannot carry credentials. `Access-Control-Allow-Origin/Methods/Headers` are set nowhere in this repository — the layer responsible is either Orthanc's `HttpHeaders` config or the ingress. Determine where, confirm `DELETE` is in the allowed-methods list (it is not used cross-origin anywhere today, so it may well be absent), and confirm the preflight bypasses the `orthanc-authorization` plugin. * **V-3 — The redirect stays same-origin.** Confirm, per environment, that the public `OrthancServerScheme`/`OrthancServerHostname`/`OrthancServerPort` registered in Sonador match the origin the OHIF client uses. If they diverge the redirect becomes cross-origin, the `Authorization` header is dropped by `fetch()`, and the follow-up request 401s. If divergence is possible, the `?token=` credential carrier (`UserContextMixin.get_user_creds`) sidesteps the problem entirely and should be preferred for this endpoint. * **V-4 — `orthanc-id` back-fill.** `_clean_dcmweb_resource_level()` in the web app sets only `level` and `dicom_uid`, while `user_has_resource_access()` matches global policy grants against `orthanc_id`. Confirm whether the auth plugin supplies `orthanc-id` for a system-classified access. If it does not, a **scoped** global policy (`study={orthanc-id}`) cannot authorize the delete and only wildcard global policies or Orthanc-side local ACLs can. This is pre-existing behavior shared with `/archive` and DICOMweb `/comments`, but it is directly load-bearing for a "grant remove on one study" workflow. * **V-5 — Hierarchy explosion.** Confirm the auth plugin sends a single resource entry for `/manage` rather than exploding the patient -> study -> series hierarchy. If it explodes, verify `remove_traverse` resolves correctly for ancestors. ### 5.7 Tests There is no test suite in this repository and `.gitlab-ci.yml` has no test stage. Endpoint tests live in oak-tree/medical-imaging/sonador-client under `ftests/`, with fixtures in `test/`. Add `ftests/tests_manage.py` (a new module rather than extending `tests_download.py`, because these flows are destructive and need their own teardown discipline), following the conventions in `ftests/tests_download.py`: * Setup via `setupTestAuth(testuser_config=TESTUSER01, testgroup_name=TESTGROUP01)`, `iserver.admin_create_acl(testgroup, {'resource': '*', 'duration': 1})`, `getLimitedImageServer(...)`, and `stageImageArchiveSeries(...)`. `cleanupImageUpload` already tolerates a resource the test deleted. * Resource-level grants via `test_sx.create_group_acl(testgroup, {'View': ..., 'Remove': True, ...})` — Orthanc-cased keys. * Assertions: * `test_dcmweb_manage_delete_series_acl_ltd` — with `Remove: True`, assert the **un-followed** response to catch a method rewrite: ```python r = requests.delete(url, allow_redirects=False, ...) self.assertEqual(r.status_code, 307) self.assertTrue(r.headers['Location'].endswith('/series/%s' % test_sx.pk)) ``` then a second call with `allow_redirects=True` and confirm the resource is gone. * `test_dcmweb_manage_delete_study_acl_ltd` — same for a study; confirm child series and instances are gone. * `test_dcmweb_manage_delete_series_acl_denied` — with `Remove: False`, assert 403. * `test_dcmweb_manage_delete_study_acl_revoked` — grant, confirm allowed, revoke, confirm 403 (mirrors `test_dcmweb_download_study_acl_revoked`). * `test_dcmweb_manage_unknown_uid` — assert 404 with a JSON `Error` body. * `test_dcmweb_manage_method_not_allowed` — assert `GET .../manage` returns 405 and does **not** redirect to the archive (AR-3). --- ## 6. Companion changes in the Sonador web application These live in **oak-tree/medical-imaging/sonador**, not this repository. They are in scope for this issue because without them the endpoints return 403 for every non-superuser (§2.5) and the work cannot be validated. ### 6.1 `lib/orthancapi/apisettings.py` Add route constants mirroring the existing `ORTHANC_DICOMWEB_ACL_MANAGEMENT_REGEX` / `ORTHANC_ACL_MANAGEMENT_PATH_REGEX` pair: * a path guard, e.g. `ORTHANC_RESOURCE_MANAGE = 'manage'` and `ORTHANC_MANAGEMENT_PATH_REGEX = re.compile(r'/manage/?$')` * a capture regex, e.g. ```python ORTHANC_DICOMWEB_MANAGE_REGEX = re.compile(r'%s/%s/%s/manage' % ( ORTHANC_DICOMWEB, ORTHANC_DICOMWEB_RESOURCE_TYPE_REGEX_STR, ORTHANC_DICOMWEB_RESOURCE_REGEX_STR)) ``` The existing `ORTHANC_LOCALAUTH_RESOURCES_PLURAL` mapping already normalizes `studies` -> `study`, and `series` matches directly, so one regex covers both endpoints. ### 6.2 `apps/visionaire/auth/forms/orthanc.py` Add a branch to `OrthancServiceAuthorizationForm.clean_auth_request()`: ```python elif orthanc_api.ORTHANC_DICOMWEB in _resource \ and orthanc_api.ORTHANC_MANAGEMENT_PATH_REGEX.search(_resource): _dcmweb_manage = orthanc_api.ORTHANC_DICOMWEB_MANAGE_REGEX.match(_resource) cleaned_data = self._clean_dcmweb_resource_level(cleaned_data, _dcmweb_manage) ``` **Placement matters:** it must not shadow the existing `/comments`, `/archive`, `/acl/`, or `resource-acl` branches. Once `level` becomes `study`/`series` and `dicom_uid` is populated, `ResourceAuthorization.resource_perm()` handles the decision at both granularities with **no change to `lib/orthancapi/auth/acl.py`**. ### 6.3 Stale ACL scope resilience (recommended, not blocking) `PacsImagingServerGroupAuthorization.resource` stores Orthanc IDs as a text policy (`study={id} series={id}`). Nothing prunes them when a resource is deleted, and `resource_authscope()` calls `orthanc_resource_info()` for **every** UID in the policy while building the authorization scope. A deleted UID makes that a 404 against Orthanc, and the call is not wrapped in a `try`. If the underlying helper raises on 404, deleting a study can break authorization for *unrelated* resources sharing the same group policy. Wrap `orthanc_resource_info()` in `apps/visionaire/auth/models/auth.py` so a 404 degrades to "not in scope" rather than propagating. Optionally prune deleted UIDs from the stored policy. The failure mode of the underlying `server_controloperation_get` on 404 lives in the `lib/microservices` submodule and was not readable — confirm it before deciding how much of this is required. ### 6.4 Authorization response cache (decision, not necessarily a change) `OrthancServiceAuthorizationView.cache_set_authorization_response()` caches `granted` decisions for up to `AUTH_CREDENTIALS_CACHE_MAX_AGE` (default 180s), and `OrthancResourceAclIntrospectionView` caches per `(level, orthanc_id)`. After a delete these entries are stale. Risk is low — Orthanc 404s a deleted resource regardless — but record an explicit decision rather than leaving it implicit. ### 6.5 Explicitly not required * **No new permission.** `remove` already exists at patient/study/series granularity in both the model and the evaluator. * **No change to the PACS listing API.** `GET /visionaire/api/pacs?output-type=ohif` already emits `perms.remove` (lowercase) via `PacsImagingServer.server_perms()`. Note its semantics for the consuming frontend: it is `True` only for a superuser or for a group policy with `remove=True` **and** `resource == '*'`. A user holding only a scoped grant gets `perms.remove == False`, so a UI that gates solely on the server-level flag will hide the affordance from scoped-grant users. The per-resource `resource-acl` response already returns `Remove` and is the correct gate for a per-study/per-series control. This is a frontend design point, tracked under oak-tree/medical-imaging/imaging-development-env#64, not a defect here. * **No cache/index sync work.** The Sonador web application holds no index of studies or series; resource metadata is fetched live from Orthanc on every authorization decision. There is no webhook, queue consumer, or periodic sync to update, and none is needed. --- ## 7. Acceptance criteria - [ ] `DELETE {dicomweb_root}/studies/{StudyInstanceUID}/manage` returns **307** with `Location` set to `{public-orthanc-fqdn}/studies/{orthanc-id}`, and following the redirect removes the study. - [ ] `DELETE {dicomweb_root}/series/{SeriesInstanceUID}/manage` returns **307** with `Location` set to `{public-orthanc-fqdn}/series/{orthanc-id}`, and following the redirect removes the series. - [ ] An un-followed `DELETE` returns 307, not 302 — asserted explicitly in a test, so a regression to 302 fails the build. - [ ] `GET .../manage` returns **405** and does not redirect to the archive endpoint. - [ ] `POST`, `PUT`, and `PATCH` on `.../manage` return 405; `OPTIONS` returns 204 with `Allow: DELETE, OPTIONS`. - [ ] A `DELETE` for a UID with no `DicomIdentifiers` mapping returns 404 with a JSON `Error` body — and does **not** raise `TypeError` from inside `http404_resource_not_found` (AR-5). - [ ] A user holding `Remove: True` on the resource (granted at series, study, or patient level, or globally) is authorized; the delete succeeds end to end. - [ ] A user holding `View: True` but `Remove: False` receives **403** from the authorization plugin. - [ ] Revoking a `Remove` grant mid-session causes a subsequent `DELETE` to return 403. - [ ] Deleting a study removes its series and instances; deleting a series leaves the parent study intact when other series remain, and prunes the study when it was the last one. - [ ] Deleting a resource removes its Sonador cache row, private tags, datetime tags, comments, worklist items, and ACL grants, via the existing change-callback pipeline — confirmed by inspecting the database after a test delete. - [ ] A failure during the redirected delete returns 500 with a well-formed JSON body, not a `TypeError` (FR-8). - [ ] The existing archive-download endpoints are unaffected: `GET .../archive` still returns its redirect and downloads successfully for both studies and series. - [ ] `manage` collides with no Orthanc core or DICOMweb plugin route in the deployed build (V-1), and the answer is recorded in this issue. - [ ] CORS permits a cross-origin `DELETE` preflight from the OHIF origin (V-2), and the answer is recorded in this issue. - [ ] The plugin starts cleanly with the new endpoints registered, and `orthanc.LogWarning` reports both registrations at boot. --- ## 8. Out of scope * **Any OHIF frontend work.** The study-list and viewer controls that call these endpoints are tracked under oak-tree/medical-imaging/imaging-development-env#64. * **Changes to the `orthanc-authorization` plugin.** The accept/deny dispatch already works for extension routes; only the Sonador-side classification is missing (§6.2). * **A new `remove` permission or any change to the ACL model.** `remove` already exists at every granularity this feature needs. * **Audit logging of the delete.** No audit hook exists anywhere today — not in the plugin, not in the web app, not in the Kafka producers (which are wired only to `STABLE_*` and stored-instance callbacks). HIPAA audit logging is tracked as open, unimplemented work under oak-tree/medical-imaging/imaging-development-env#78. A destructive operation is the natural first consumer, and this endpoint is the natural insertion point (`SonadorResourceBaseView.delete_resource`, or a `ChangeType.DELETED` Kafka producer alongside `init_export_resource_data`) — but building it is that issue's scope, not this one. **If a compliance requirement makes an audit trail a precondition for shipping resource removal, that is a scoping decision to make before this work starts.** * **Instance-level removal.** Only study and series endpoints are specified. Instances have no ACL model of their own and inherit their series' policy; a `/manage` route for instances would follow the same pattern but is not requested. * **Patient-level removal.** `SonadorPatientResourceView` exists and supports `DELETE` by Orthanc ID, but there is no DICOMweb patient resource to hang a `/manage` route from. * **Soft delete, undo, or a recycle bin.** Removal is Orthanc's hard delete. * **Bulk or multi-resource removal.** One resource per request. * **Propagating the upstream Orthanc status code** through `SonadorResourceBaseView` (§2.4). Worth doing; not required here. --- ## 9. References * Origin: the scoping decision recorded in oak-tree/medical-imaging/imaging-development-env#64 * Consuming frontend work: oak-tree/medical-imaging/imaging-development-env#64, oak-tree/medical-imaging/ohif-viewers#64 * Archive export (the pattern this follows): oak-tree/medical-imaging/ohif-viewers#36, oak-tree/medical-imaging/ohif-viewers#52 * HIPAA audit logging (open, unimplemented): oak-tree/medical-imaging/imaging-development-env#78 * Reference files, this repository: * `sonador_orthanc/web/download.py` — the reference implementation * `sonador_orthanc/web/dicomweb.py` — `DicomResourceMixin`, `init_download_endpoints`, `init_ext_endpoints` * `sonador_orthanc/cache/web/base.py` — `CacheBaseView`, `ResourceBaseMixin` * `sonador_orthanc/web/resource.py` — `SonadorResourceBaseView.delete` / `delete_resource`, `_execute_resource_request` * `sonador_orthanc/cache/web/study.py`, `sonador_orthanc/cache/web/series.py` — the redirect targets * `sonador_orthanc/cache/__init__.py` — registration of the Orthanc-ID resource overrides * `sonador_orthanc/manager.py` — `orthanc_apiurl_fqdn`, `introspect_resource_perms` * `sonador_orthanc/web/secure_user.py` — `UserContextMixin.get_user_creds`, credential carriers * `sonador_orthanc/db/auth.py` — `UserSeriesAuth` / `GroupSeriesAuth` * `sonador_orthanc/auth/web.py` — resource policy resolution, `remove_traverse` * `lua/SonadorEvents.lua`, `sonador_orthanc/web/events.py`, `sonador_orthanc/tasks/maintenance/cache.py` — delete propagation * `sonador-plugin.py` — `orthanc_cache_onstart` * Reference files, oak-tree/medical-imaging/orthanc-sonador-common: * `web.py` — `OrthancBaseView` (`as_view`, `dispatch`, `send_response`, `options`, `http404_resource_not_found`), `RedirectView` (the 307 convention) * Reference files, oak-tree/medical-imaging/sonador-client: * `servers/auth.py` — permission constants, `ACL_PERM_ORTHANC_MAPPINGS` * `imaging/orthanc/base.py` — `resource_url`, `filearchive_url`, `delete()` * `servers/__init__.py` — `orthanc_apiurl` * `ftests/tests_download.py`, `test/base.py`, `test/acl.py` — test conventions and fixtures * Reference files, oak-tree/medical-imaging/sonador: * `lib/orthancapi/apisettings.py` — permission and route constants * `lib/orthancapi/auth/acl.py` — `ResourceAuthorization.resource_perm` * `apps/visionaire/auth/forms/orthanc.py` — `OrthancServiceAuthorizationForm.clean_auth_request` * `apps/visionaire/auth/views/service/orthanc_auth.py` — the accept/deny endpoint * `apps/visionaire/auth/models/auth.py` — `PacsImagingServerGroupAuthorization`, `resource_authscope` * `apps/visionaire/models/servers.py` — `PacsImagingServer.server_perms`
issue