From 3a53479bb703deda72ec7220dc1d4091cad1f23d Mon Sep 17 00:00:00 2001 From: Erik Ziegler Date: Fri, 2 Jul 2021 22:54:26 +0200 Subject: [PATCH] fix: OpenID Connect and Local route rendering --- .../core/src/extensions/ExtensionManager.js | 28 ++----------------- .../UserAuthenticationService.js | 4 +-- .../UserAuthenticationProvider.js | 14 ++++++++++ platform/viewer/src/App.jsx | 2 -- platform/viewer/src/routes/Local/Local.jsx | 16 ++++------- platform/viewer/src/routes/Mode/Mode.jsx | 12 ++------ .../viewer/src/routes/WorkList/WorkList.jsx | 10 ++++--- platform/viewer/src/routes/buildModeRoutes.js | 8 ++---- .../viewer/src/utils/OpenIdConnectRoutes.jsx | 23 +++++++++++---- 9 files changed, 50 insertions(+), 67 deletions(-) diff --git a/platform/core/src/extensions/ExtensionManager.js b/platform/core/src/extensions/ExtensionManager.js index ba4e10d41..439f4f64b 100644 --- a/platform/core/src/extensions/ExtensionManager.js +++ b/platform/core/src/extensions/ExtensionManager.js @@ -186,6 +186,7 @@ export default class ExtensionManager { case MODULE_TYPES.SOP_CLASS_HANDLER: case MODULE_TYPES.CONTEXT: case MODULE_TYPES.LAYOUT_TEMPLATE: + case MODULE_TYPES.HANGING_PROTOCOL: // Default for most extension points, // Just adds each entry ready for consumption by mode. extensionModule.forEach(element => { @@ -194,13 +195,6 @@ export default class ExtensionManager { ] = element; }); break; - case MODULE_TYPES.HANGING_PROTOCOL: - extensionModule.forEach(element => { - this.modulesMap[ - `${extensionId}.${moduleType}.${element.name}` - ] = element; - }); - break; default: throw new Error(`Module type invalid: ${moduleType}`); } @@ -306,27 +300,9 @@ export default class ExtensionManager { } _initHangingProtocolModule(extensionModule, extensionId) { - extensionModule.forEach(element => { - const namespace = `${extensionId}.${MODULE_TYPES.HANGING_PROTOCOL}.${element.name}`; - - dataSources.forEach(dataSource => { - if (dataSource.namespace === namespace) { - const dataSourceInstance = element.createDataSource( - dataSource.configuration - ); - - if (this.dataSourceMap[dataSource.sourceName]) { - this.dataSourceMap[dataSource.sourceName].push(dataSourceInstance); - } else { - this.dataSourceMap[dataSource.sourceName] = [dataSourceInstance]; - } - } - }); - }); - extensionModule.forEach(element => { this.modulesMap[ - `${extensionId}.${MODULE_TYPES.DATA_SOURCE}.${element.name}` + `${extensionId}.${MODULE_TYPES.HANGING_PROTOCOL}.${element.name}` ] = element; }); } diff --git a/platform/core/src/services/UserAuthenticationService/UserAuthenticationService.js b/platform/core/src/services/UserAuthenticationService/UserAuthenticationService.js index 777c0466d..23647aca8 100644 --- a/platform/core/src/services/UserAuthenticationService/UserAuthenticationService.js +++ b/platform/core/src/services/UserAuthenticationService/UserAuthenticationService.js @@ -37,9 +37,7 @@ function _getUser() { } function _getAuthorizationHeader() { - const user = serviceImplementation._getUser(); - - return serviceImplementation._getAuthorizationHeader(user); + return serviceImplementation._getAuthorizationHeader(); } function _handleUnauthenticated() { diff --git a/platform/ui/src/contextProviders/UserAuthenticationProvider.js b/platform/ui/src/contextProviders/UserAuthenticationProvider.js index fd7c062f5..667f2d20e 100644 --- a/platform/ui/src/contextProviders/UserAuthenticationProvider.js +++ b/platform/ui/src/contextProviders/UserAuthenticationProvider.js @@ -88,6 +88,7 @@ export function UserAuthenticationProvider({ children, service }) { * * @returns void */ + // TODO: should this be a useEffect or not? useEffect(() => { if (service) { service.setServiceImplementation({ @@ -100,6 +101,19 @@ export function UserAuthenticationProvider({ children, service }) { } }, [getState, service, setUser, getUser, reset, set]); + // TODO: This may not be correct, but I think we need to set the implementation for the service + // immediately when this runs, since otherwise the authentication redirects will fail. + // (useEffect only runs after the child components - in this case, routing logic - has failed) + if (service) { + service.setServiceImplementation({ + getState, + setUser, + getUser, + reset, + set, + }); + } + const api = { getState, setUser, diff --git a/platform/viewer/src/App.jsx b/platform/viewer/src/App.jsx index 6516228ac..9b9470306 100644 --- a/platform/viewer/src/App.jsx +++ b/platform/viewer/src/App.jsx @@ -79,8 +79,6 @@ function App({ config, defaultExtensions }) { let authRoutes = null; if (oidc) { - UserAuthenticationService.set({ enabled: true }); - authRoutes = ( { ) } -function Local(props) { - const { history } = props +function Local() { + const navigate = useNavigate(); const dropzoneRef = useRef() // Initializing the dicom local dataSource @@ -61,7 +61,7 @@ function Local(props) { const onDrop = async (acceptedFiles) => { const studies = await filesToStudies(acceptedFiles, dataSource) // Todo: navigate to work list and let user select a mode - history.push(`/viewer/dicomlocal?StudyInstanceUIDs=${studies[0]}`) + navigate(`/viewer/dicomlocal?StudyInstanceUIDs=${studies[0]}`) } // Set body style @@ -74,7 +74,7 @@ function Local(props) { return ( - {({ getRootProps, getInputProps }) => ( + {({ getRootProps }) => (
@@ -107,10 +107,4 @@ function Local(props) { ) } -Local.propTypes = { - history: PropTypes.shape({ - push: PropTypes.func, - }).isRequired, -}; - export default Local diff --git a/platform/viewer/src/routes/Mode/Mode.jsx b/platform/viewer/src/routes/Mode/Mode.jsx index 5be08f173..2dc4ff9b3 100644 --- a/platform/viewer/src/routes/Mode/Mode.jsx +++ b/platform/viewer/src/routes/Mode/Mode.jsx @@ -1,5 +1,5 @@ import React, { useEffect, useState, useRef } from 'react'; -import { useParams } from 'react-router'; +import { useParams, useLocation } from 'react-router'; import PropTypes from 'prop-types'; // TODO: DicomMetadataStore should be injected? import { DicomMetadataStore } from '@ohif/core'; @@ -54,7 +54,6 @@ async function defaultRouteInit({ } export default function ModeRoute({ - location, mode, dataSourceName, extensionManager, @@ -62,6 +61,7 @@ export default function ModeRoute({ hotkeysManager, }) { // Parse route params/querystring + const location = useLocation(); const query = useQuery(); const params = useParams(); @@ -287,14 +287,6 @@ export default function ModeRoute({ } ModeRoute.propTypes = { - // Ref: https://reacttraining.com/react-router/web/api/location - location: PropTypes.shape({ - key: PropTypes.string, - pathname: PropTypes.string.isRequired, - search: PropTypes.string.isRequired, - hash: PropTypes.string.isRequired, - //state: PropTypes.object.isRequired, - }), mode: PropTypes.object.isRequired, dataSourceName: PropTypes.string, extensionManager: PropTypes.object, diff --git a/platform/viewer/src/routes/WorkList/WorkList.jsx b/platform/viewer/src/routes/WorkList/WorkList.jsx index 0b9bf1b53..d2dd7dec2 100644 --- a/platform/viewer/src/routes/WorkList/WorkList.jsx +++ b/platform/viewer/src/routes/WorkList/WorkList.jsx @@ -175,12 +175,14 @@ function WorkList({ } }); + const search = qs.stringify(queryString, { + skipNull: true, + skipEmptyString: true, + }); + navigate({ pathname: '/', - search: `?${qs.stringify(queryString, { - skipNull: true, - skipEmptyString: true, - })}`, + search: search ? `?${search}` : undefined, }); // eslint-disable-next-line react-hooks/exhaustive-deps }, [debouncedFilterValues]); diff --git a/platform/viewer/src/routes/buildModeRoutes.js b/platform/viewer/src/routes/buildModeRoutes.js index daef8e565..5f30466d6 100644 --- a/platform/viewer/src/routes/buildModeRoutes.js +++ b/platform/viewer/src/routes/buildModeRoutes.js @@ -50,9 +50,8 @@ export default function buildModeRoutes({ const path = `/${mode.id}/${dataSourceName}`; // TODO move up. - const component = ({ location }) => ( + const children = () => ( ( + const children = () => ( { + + const getAuthorizationHeader = () => { + const user = UserAuthenticationService.getUser(); + return { Authorization: `Bearer ${user.access_token}` }; @@ -115,10 +119,14 @@ function OpenIdConnectRoutes({ const navigate = useNavigate(); - UserAuthenticationService.setServiceImplementation({ - getAuthorizationHeader, - handleUnauthenticated - }); + useEffect(() => { + UserAuthenticationService.set({ enabled: true }); + + UserAuthenticationService.setServiceImplementation({ + getAuthorizationHeader, + handleUnauthenticated + }); + }, []) const oidcAuthority = oidc[0].authority; @@ -166,7 +174,10 @@ function OpenIdConnectRoutes({ UserAuthenticationService.setUser(user); - navigate(`${pathname}?${search}`); + navigate({ + pathname, + search + }) }}/>} />