From 7f0856a98aef41aee5c514bb48c3c8a990fa0c05 Mon Sep 17 00:00:00 2001 From: Patrick Avery Date: Wed, 25 Oct 2023 11:02:52 -0500 Subject: [PATCH] Add token authentication option for DICOMweb When the user is creating a DICOMweb assetstore, if they select the "token" authentication type, an input box will appear where the user must enter the token. This token is saved in MongoDB, and is used for making all requests for this DICOMweb assetstore. That includes requests for importing the data (which may only be done by an admin), and requests for viewing the data (which may be done by anyone with access to the folder). If an admin wishes to restrict users from viewing DICOMweb assets that were imported using this token, the admin must restrict access to the imported items via girder's folder access controls. Signed-off-by: Patrick Avery --- .../large_image_source_dicom/__init__.py | 4 +- .../assetstore/__init__.py | 5 ++ .../assetstore/dicomweb_assetstore_adapter.py | 59 +++++++++++++------ .../assetstore/rest.py | 1 - .../large_image_source_dicom/girder_source.py | 5 +- .../templates/dicomwebAssetstoreCreate.pug | 2 +- .../dicomwebAssetstoreEditFields.pug | 2 +- .../templates/dicomwebAssetstoreMixins.pug | 57 ++++++++++++++---- .../web_client/views/AuthOptions.js | 13 ++++ .../web_client/views/EditAssetstoreWidget.js | 12 +++- .../web_client/views/NewAssetstoreWidget.js | 15 +++-- .../web_client_specs/dicomWebSpec.js | 18 +++++- 12 files changed, 149 insertions(+), 44 deletions(-) create mode 100644 sources/dicom/large_image_source_dicom/web_client/views/AuthOptions.js diff --git a/sources/dicom/large_image_source_dicom/__init__.py b/sources/dicom/large_image_source_dicom/__init__.py index ede687db5..bce31c015 100644 --- a/sources/dicom/large_image_source_dicom/__init__.py +++ b/sources/dicom/large_image_source_dicom/__init__.py @@ -170,14 +170,14 @@ def _open_wsi_dicomweb(self, info): # These are optional keys qido_prefix = info.get('qido_prefix') wado_prefix = info.get('wado_prefix') - auth = info.get('auth') + session = info.get('session') # Create the client client = wsidicom.WsiDicomWebClient( url, qido_prefix=qido_prefix, wado_prefix=wado_prefix, - auth=auth, + auth=session, ) # Identify the transfer syntax diff --git a/sources/dicom/large_image_source_dicom/assetstore/__init__.py b/sources/dicom/large_image_source_dicom/assetstore/__init__.py index 68482ba17..2a548f672 100644 --- a/sources/dicom/large_image_source_dicom/assetstore/__init__.py +++ b/sources/dicom/large_image_source_dicom/assetstore/__init__.py @@ -32,6 +32,7 @@ def createAssetstore(event): 'qido_prefix': params.get('qido_prefix'), 'wado_prefix': params.get('wado_prefix'), 'auth_type': params.get('auth_type'), + 'auth_token': params.get('auth_token'), }, })) event.preventDefault() @@ -53,6 +54,7 @@ def updateAssetstore(event): 'qido_prefix': params.get('qido_prefix'), 'wado_prefix': params.get('wado_prefix'), 'auth_type': params.get('auth_type'), + 'auth_token': params.get('auth_token'), } @@ -78,6 +80,9 @@ def load(info): required=False) .param('auth_type', 'The authentication type required for the server, if needed (for DICOMweb)', + required=False) + .param('auth_token', + 'Token for authentication if needed (for DICOMweb)', required=False)) info['apiRoot'].dicomweb_assetstore = DICOMwebAssetstoreResource() diff --git a/sources/dicom/large_image_source_dicom/assetstore/dicomweb_assetstore_adapter.py b/sources/dicom/large_image_source_dicom/assetstore/dicomweb_assetstore_adapter.py index 62e705135..135eff6e0 100644 --- a/sources/dicom/large_image_source_dicom/assetstore/dicomweb_assetstore_adapter.py +++ b/sources/dicom/large_image_source_dicom/assetstore/dicomweb_assetstore_adapter.py @@ -1,3 +1,4 @@ +import requests from requests.exceptions import HTTPError from girder.exceptions import ValidationException @@ -45,10 +46,13 @@ def validateInfo(doc): if isinstance(info.get(field), str) and not info[field].strip(): info[field] = None - # Now, if there is no authentication, verify that we can connect to the server. - # If there is authentication, we may need to prompt the user for their - # username and password sometime before here. - if info['auth_type'] is None: + if info['auth_type'] == 'token' and not info.get('auth_token'): + msg = 'A token must be provided if the auth type is "token"' + raise ValidationException(msg) + + # Verify that we can connect to the server, if the authentication type + # allows it. + if info['auth_type'] in (None, 'token'): study_instance_uid_tag = dicom_key_to_tag('StudyInstanceUID') series_instance_uid_tag = dicom_key_to_tag('SeriesInstanceUID') @@ -61,7 +65,8 @@ def validateInfo(doc): fields=(study_instance_uid_tag, series_instance_uid_tag), ) except HTTPError as e: - raise ValidationException('Failed to validate DICOMweb server settings: ' + str(e)) + msg = f'Failed to validate DICOMweb server settings: {e}' + raise ValidationException(msg) # If we found a series, then test the wado prefix as well if series: @@ -76,10 +81,15 @@ def validateInfo(doc): series_instance_uid=series_uid, ) except HTTPError as e: - raise ValidationException('Failed to validate DICOMweb WADO prefix: ' + str(e)) + msg = f'Failed to validate DICOMweb WADO prefix: {e}' + raise ValidationException(msg) return doc + @property + def assetstore_meta(self): + return self.assetstore[DICOMWEB_META_KEY] + def initUpload(self, upload): msg = 'DICOMweb assetstores are import only.' raise NotImplementedError(msg) @@ -122,9 +132,6 @@ def importData(self, parent, parentType, params, progress, user, **kwargs): :search_filters: (optional) a dictionary of additional search filters to use with dicomweb_client's `search_for_series()` function. - :auth: (optional) if the DICOMweb server requires authentication, - this should be an authentication handler derived from - requests.auth.AuthBase. :type params: dict :param progress: Object on which to record progress if possible. @@ -142,9 +149,9 @@ def importData(self, parent, parentType, params, progress, user, **kwargs): limit = params.get('limit') search_filters = params.get('search_filters', {}) - meta = self.assetstore[DICOMWEB_META_KEY] + meta = self.assetstore_meta - client = _create_dicomweb_client(meta, auth=params.get('auth')) + client = _create_dicomweb_client(meta) study_uid_key = dicom_key_to_tag('StudyInstanceUID') series_uid_key = dicom_key_to_tag('SeriesInstanceUID') @@ -201,19 +208,37 @@ def importData(self, parent, parentType, params, progress, user, **kwargs): file['imported'] = True File().save(file) - # FIXME: should we return a list of items (like this), or should - # we return files? items.append(item) return items + @property + def auth_session(self): + return _create_auth_session(self.assetstore_meta) + + +def _create_auth_session(meta): + auth_type = meta.get('auth_type') + if auth_type is None: + return None + + if auth_type == 'token': + return _create_token_auth_session(meta['auth_token']) + + msg = f'Unhandled auth type: {auth_type}' + raise NotImplementedError(msg) + + +def _create_token_auth_session(token): + s = requests.Session() + s.headers.update({'Authorization': f'Bearer {token}'}) + return s + -def _create_dicomweb_client(meta, auth=None): +def _create_dicomweb_client(meta): from dicomweb_client.api import DICOMwebClient - from dicomweb_client.session_utils import create_session_from_auth - # Create the authentication session - session = create_session_from_auth(auth) + session = _create_auth_session(meta) # Make the DICOMwebClient return DICOMwebClient( diff --git a/sources/dicom/large_image_source_dicom/assetstore/rest.py b/sources/dicom/large_image_source_dicom/assetstore/rest.py index 7994a212e..a6adbd5ce 100644 --- a/sources/dicom/large_image_source_dicom/assetstore/rest.py +++ b/sources/dicom/large_image_source_dicom/assetstore/rest.py @@ -65,7 +65,6 @@ def _importData(self, assetstore, params): { 'limit': limit, 'search_filters': search_filters, - 'auth': None, }, ctx, user, diff --git a/sources/dicom/large_image_source_dicom/girder_source.py b/sources/dicom/large_image_source_dicom/girder_source.py index 9bcf8071a..e32fd1f3a 100644 --- a/sources/dicom/large_image_source_dicom/girder_source.py +++ b/sources/dicom/large_image_source_dicom/girder_source.py @@ -4,6 +4,7 @@ from girder.models.file import File from girder.models.folder import Folder from girder.models.item import Item +from girder.utility import assetstore_utilities from . import DICOMFileTileSource from .assetstore import DICOMWEB_META_KEY @@ -61,6 +62,8 @@ def _getDICOMwebLargeImagePath(self, assetstore): file = Item().childFiles(self.item, limit=1)[0] file_meta = file['dicomweb_meta'] + adapter = assetstore_utilities.getAssetstoreAdapter(assetstore) + return { 'url': meta['url'], 'study_uid': file_meta['study_uid'], @@ -68,5 +71,5 @@ def _getDICOMwebLargeImagePath(self, assetstore): # The following are optional 'qido_prefix': meta.get('qido_prefix'), 'wado_prefix': meta.get('wado_prefix'), - 'auth': meta.get('auth'), + 'session': adapter.auth_session, } diff --git a/sources/dicom/large_image_source_dicom/web_client/templates/dicomwebAssetstoreCreate.pug b/sources/dicom/large_image_source_dicom/web_client/templates/dicomwebAssetstoreCreate.pug index 313c46b34..d1fae4d2f 100644 --- a/sources/dicom/large_image_source_dicom/web_client/templates/dicomwebAssetstoreCreate.pug +++ b/sources/dicom/large_image_source_dicom/web_client/templates/dicomwebAssetstoreCreate.pug @@ -20,7 +20,7 @@ include dicomwebAssetstoreMixins input#g-new-dwas-name.input-sm.form-control( type="text", placeholder="Name") - +g-dwas-parameters + +g-dwas-parameters("new") p#g-new-dwas-error.g-validation-failed-message input.g-new-assetstore-submit.btn.btn-sm.btn-primary( type="submit", value="Create") diff --git a/sources/dicom/large_image_source_dicom/web_client/templates/dicomwebAssetstoreEditFields.pug b/sources/dicom/large_image_source_dicom/web_client/templates/dicomwebAssetstoreEditFields.pug index 2071aea4f..e7fb8d8d3 100644 --- a/sources/dicom/large_image_source_dicom/web_client/templates/dicomwebAssetstoreEditFields.pug +++ b/sources/dicom/large_image_source_dicom/web_client/templates/dicomwebAssetstoreEditFields.pug @@ -1,3 +1,3 @@ include dicomwebAssetstoreMixins -+g-dwas-parameters ++g-dwas-parameters("edit") diff --git a/sources/dicom/large_image_source_dicom/web_client/templates/dicomwebAssetstoreMixins.pug b/sources/dicom/large_image_source_dicom/web_client/templates/dicomwebAssetstoreMixins.pug index adeb1b8c8..961beeae9 100644 --- a/sources/dicom/large_image_source_dicom/web_client/templates/dicomwebAssetstoreMixins.pug +++ b/sources/dicom/large_image_source_dicom/web_client/templates/dicomwebAssetstoreMixins.pug @@ -1,19 +1,52 @@ -mixin g-dwas-parameters +mixin g-dwas-parameters(label_key) + - const key = label_key; + + //- We need to make sure the html elements all have unique ids when this + //- mixin is reused in different places, so that we can locate the correct + //- html elements in the script. + - const url_id = `g-${key}-dwas-url`; + - const qido_id = `g-${key}-dwas-qido-prefix`; + - const wado_id = `g-${key}-dwas-wado-prefix`; + - const auth_type_id = `g-${key}-dwas-auth-type`; + - const auth_type_container_id = `g-${key}-dwas-auth-type-container`; + - const auth_token_id = `g-${key}-dwas-auth-token`; + - const auth_token_container_id = `g-${key}-dwas-auth-token-container`; + .form-group - label.control-label(for="g-edit-dwas-url") DICOMweb server URL - input#g-edit-dwas-url.input-sm.form-control( + label.control-label(for=url_id) DICOMweb server URL + input.input-sm.form-control( + id=url_id, type="text", placeholder="URL") - label.control-label(for="g-edit-dwas-qido-prefix") DICOMweb QIDO prefix (optional) - input#g-edit-dwas-qido-prefix.input-sm.form-control( + label.control-label(for=qido_id) DICOMweb QIDO prefix (optional) + input.input-sm.form-control( + id=qido_id, type="text", placeholder="QIDO prefix") - label.control-label(for="g-edit-dwas-wado-prefix") DICOMweb WADO prefix (optional) - input#g-edit-dwas-wado-prefix.input-sm.form-control( + label.control-label(for=wado_id) DICOMweb WADO prefix (optional) + input.input-sm.form-control( + id=wado_id, type="text", placeholder="WADO prefix") - //- COMMENTED OUT UNTIL WE ADD AUTHENTICATION - label.control-label(for="g-edit-dwas-auth-type") DICOMweb authentication type (optional) - input#g-edit-dwas-auth-type.input-sm.form-control( - type="text", - placeholder="Authentication type") + label.control-label(for=auth_type_id) DICOMweb authentication type (optional) + - const auth_type = (assetstore && assetstore.attributes.dicomweb_meta.auth_type) || null; + - const updateFuncName = `${key}UpdateVisibilities`; + script. + var #{updateFuncName} = function () { + const isToken = document.getElementById('#{auth_type_id}').value === 'token'; + const display = isToken ? 'block' : 'none'; + document.getElementById('#{auth_token_container_id}').style.display = display; + }; + div(id=auth_type_container_id) + select.form-control( + id=auth_type_id, + onchange=updateFuncName + '()') + each auth_option in authOptions + option(value=auth_option.value, selected=(auth_type === auth_option.value)) #{auth_option.label} + - const display = auth_type === 'token' ? 'block': 'none'; + div(id=auth_token_container_id, style='display: ' + display + ';') + label.control-label(for=auth_token_id) DICOMweb authentication token + input.input-sm.form-control( + id=auth_token_id, + type="text", + placeholder="Token") diff --git a/sources/dicom/large_image_source_dicom/web_client/views/AuthOptions.js b/sources/dicom/large_image_source_dicom/web_client/views/AuthOptions.js new file mode 100644 index 000000000..f1d6fec1e --- /dev/null +++ b/sources/dicom/large_image_source_dicom/web_client/views/AuthOptions.js @@ -0,0 +1,13 @@ +const authOptions = [ + { + // HTML can't accept null, but it can accept an empty string + value: '', + label: 'None' + }, + { + value: 'token', + label: 'Token' + } +]; + +export default authOptions; diff --git a/sources/dicom/large_image_source_dicom/web_client/views/EditAssetstoreWidget.js b/sources/dicom/large_image_source_dicom/web_client/views/EditAssetstoreWidget.js index 058f9274c..90240bff6 100644 --- a/sources/dicom/large_image_source_dicom/web_client/views/EditAssetstoreWidget.js +++ b/sources/dicom/large_image_source_dicom/web_client/views/EditAssetstoreWidget.js @@ -4,6 +4,8 @@ import { wrap } from '@girder/core/utilities/PluginUtils'; import AssetstoreEditFieldsTemplate from '../templates/dicomwebAssetstoreEditFields.pug'; +import authOptions from './AuthOptions'; + /** * Adds DICOMweb-specific fields to the edit dialog. */ @@ -13,7 +15,8 @@ wrap(EditAssetstoreWidget, 'render', function (render) { if (this.model.get('type') === AssetstoreType.DICOMWEB) { this.$('.g-assetstore-form-fields').append( AssetstoreEditFieldsTemplate({ - assetstore: this.model + assetstore: this.model, + authOptions }) ); } @@ -26,7 +29,8 @@ EditAssetstoreWidget.prototype.fieldsMap[AssetstoreType.DICOMWEB] = { url: this.$('#g-edit-dwas-url').val(), qido_prefix: this.$('#g-edit-dwas-qido-prefix').val(), wado_prefix: this.$('#g-edit-dwas-wado-prefix').val(), - auth_type: this.$('#g-edit-dwas-auth-type').val() + auth_type: this.$('#g-edit-dwas-auth-type').val(), + auth_token: this.$('#g-edit-dwas-auth-token').val() }; }, set: function () { @@ -34,6 +38,8 @@ EditAssetstoreWidget.prototype.fieldsMap[AssetstoreType.DICOMWEB] = { this.$('#g-edit-dwas-url').val(dwInfo.url); this.$('#g-edit-dwas-qido-prefix').val(dwInfo.qido_prefix); this.$('#g-edit-dwas-wado-prefix').val(dwInfo.wado_prefix); - this.$('#g-edit-dwas-auth-type').val(dwInfo.auth_type); + // HTML can't accept null, so set it to an empty string + this.$('#g-edit-dwas-auth-type').val(dwInfo.auth_type || ''); + this.$('#g-edit-dwas-auth-token').val(dwInfo.auth_token); } }; diff --git a/sources/dicom/large_image_source_dicom/web_client/views/NewAssetstoreWidget.js b/sources/dicom/large_image_source_dicom/web_client/views/NewAssetstoreWidget.js index a3fb0a2e3..f8414187f 100644 --- a/sources/dicom/large_image_source_dicom/web_client/views/NewAssetstoreWidget.js +++ b/sources/dicom/large_image_source_dicom/web_client/views/NewAssetstoreWidget.js @@ -4,13 +4,17 @@ import { wrap } from '@girder/core/utilities/PluginUtils'; import AssetstoreCreateTemplate from '../templates/dicomwebAssetstoreCreate.pug'; +import authOptions from './AuthOptions'; + /** * Add UI for creating new DICOMweb assetstore. */ wrap(NewAssetstoreWidget, 'render', function (render) { render.call(this); - this.$('#g-assetstore-accordion').append(AssetstoreCreateTemplate()); + this.$('#g-assetstore-accordion').append(AssetstoreCreateTemplate({ + authOptions + })); return this; }); @@ -18,9 +22,10 @@ NewAssetstoreWidget.prototype.events['submit #g-new-dwas-form'] = function (e) { this.createAssetstore(e, this.$('#g-new-dwas-error'), { type: AssetstoreType.DICOMWEB, name: this.$('#g-new-dwas-name').val(), - url: this.$('#g-edit-dwas-url').val(), - qido_prefix: this.$('#g-edit-dwas-qido-prefix').val(), - wado_prefix: this.$('#g-edit-dwas-wado-prefix').val(), - auth_type: this.$('#g-edit-dwas-auth-type').val() + url: this.$('#g-new-dwas-url').val(), + qido_prefix: this.$('#g-new-dwas-qido-prefix').val(), + wado_prefix: this.$('#g-new-dwas-wado-prefix').val(), + auth_type: this.$('#g-new-dwas-auth-type').val(), + auth_token: this.$('#g-new-dwas-auth-token').val() }); }; diff --git a/sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js b/sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js index 3895c067d..c1442f8ef 100644 --- a/sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js +++ b/sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js @@ -40,7 +40,23 @@ describe('DICOMWeb assetstore', function () { runs(function () { // Create the DICOMweb assetstore $('#g-new-dwas-name').val('DICOMweb'); - $('#g-edit-dwas-url').val('https://idc-external-006.uc.r.appspot.com/dcm4chee-arc/aets/DCM4CHEE/rs'); + $('#g-new-dwas-url').val('https://idc-external-006.uc.r.appspot.com/dcm4chee-arc/aets/DCM4CHEE/rs'); + + // Test error for setting auth type to "token" with no token + $('#g-new-dwas-auth-type').val('token'); + $('#g-new-dwas-form input.btn-primary').click(); + }); + + waitsFor(function () { + const msg = 'A token must be provided if the auth type is "token"'; + return $('#g-new-dwas-error').html() === msg; + }, 'No token provided check'); + + runs(function () { + // Change the auth type back to None + $('#g-new-dwas-auth-type').val(null); + + // This should work now $('#g-new-dwas-form input.btn-primary').click(); });