From fbcacc8021db7b2359ac9934975c1288ed32197b 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 --- .../assetstore/__init__.py | 5 ++ .../assetstore/dicomweb_assetstore_adapter.py | 59 +++++++++++++------ .../assetstore/rest.py | 1 - .../templates/dicomwebAssetstoreCreate.pug | 2 +- .../dicomwebAssetstoreEditFields.pug | 2 +- .../templates/dicomwebAssetstoreMixins.pug | 56 ++++++++++++++---- .../web_client/views/AuthOptions.js | 13 ++++ .../web_client/views/EditAssetstoreWidget.js | 12 +++- .../web_client/views/NewAssetstoreWidget.js | 15 +++-- .../web_client_specs/dicomWebSpec.js | 18 +++++- 10 files changed, 143 insertions(+), 40 deletions(-) create mode 100644 sources/dicom/large_image_source_dicom/web_client/views/AuthOptions.js 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/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..13fd882a7 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,53 @@ -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( + 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="Authentication type") + 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..fb656e8ff --- /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..dbb4a8381 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(); });