Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: query string dynamic app config #3391

Merged
merged 2 commits into from
May 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 19 additions & 5 deletions extensions/default/src/ViewerLayout/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,27 @@ function ViewerLayout({
const onClickReturnButton = () => {
const { pathname } = location;
const dataSourceIdx = pathname.indexOf('/', 1);
const search =
dataSourceIdx === -1
? undefined
: `datasources=${pathname.substring(dataSourceIdx + 1)}`;
// const search =
// dataSourceIdx === -1
// ? undefined
// : `datasources=${pathname.substring(dataSourceIdx + 1)}`;

// Todo: Handle parameters in a better way.
const query = new URLSearchParams(window.location.search);
const configUrl = query.get('configUrl');

const searchQuery = new URLSearchParams();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you just add a TODO here to handle the remembering of the parameters in a better way? Really we should add the relevant parameters to the other queries, and then restore the full query string on going back, but that is NOT your problem here, it is just that I want a reminder that having the viewer page know about hte query page isn't a good idea.

if (dataSourceIdx !== -1) {
searchQuery.append('datasources', pathname.substring(dataSourceIdx + 1));
}

if (configUrl) {
searchQuery.append('configUrl', configUrl);
}

navigate({
pathname: '/',
search,
search: decodeURIComponent(searchQuery.toString()),
});
};

Expand Down
19 changes: 18 additions & 1 deletion platform/docs/docs/configuration/configurationFiles.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,24 @@ Here are a list of some options available:
if auth headers are used, a preflight request is required.
- `maxNumRequests`: The maximum number of requests to allow in parallel. It is an object with keys of `interaction`, `thumbnail`, and `prefetch`. You can specify a specific number for each type.
- `showLoadingIndicator`: (default to true), if set to false, the loading indicator will not be shown when navigating between studies.

- `dangerouslyUseDynamicConfig`: Dynamic config allows user to pass `configUrl` query string. This allows to load config without recompiling application. If the `configUrl` query string is passed, the worklist and modes will load from the referenced json rather than the default .env config. If there is no `configUrl` path provided, the default behaviour is used and there should not be any deviation from current user experience.<br/>
Points to consider while using `dangerouslyUseDynamicConfig`:<br/>
- User have to enable this feature by setting `dangerouslyUseDynamicConfig.enabled:true`. By default it is `false`.
- Regex helps to avoid easy exploit. Dafault is `/.*/`. Setup your own regex to choose a specific source of configuration only.
- System administrators can return `cross-origin: same-origin` with OHIF files to disallow any loading from other origin. It will block read access to resources loaded from a different origin to avoid potential attack vector.
- Example config:
```js
dangerouslyUseDynamicConfig: {
enabled: false,
regex: /.*/
}
```
> Example 1, to allow numbers and letters in an absolute or sub-path only.<br/>
`regex: /(0-9A-Za-z.]+)(\/[0-9A-Za-z.]+)*/`<br/>
Example 2, to restricts to either hosptial.com or othersite.com.<br/>
`regex: /(https:\/\/hospital.com(\/[0-9A-Za-z.]+)*)|(https:\/\/othersite.com(\/[0-9A-Za-z.]+)*)/` <br/>
Example usage:<br/>
`http://localhost:3000/?configUrl=http://localhost:3000/config/example.json`<br/>



Expand Down
10 changes: 10 additions & 0 deletions platform/viewer/public/config/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ window.config = {
},
// filterQueryParam: false,
defaultDataSourceName: 'dicomweb',
/* Dynamic config allows user to pass "configUrl" query string this allows to load config without recompiling application. The regex will ensure valid configuration source */
// dangerouslyUseDynamicConfig: {
// enabled: true,
// // regex will ensure valid configuration source and default is /.*/ which matches any character. To use this, setup your own regex to choose a specific source of configuration only.
// // Example 1, to allow numbers and letters in an absolute or sub-path only.
// // regex: /(0-9A-Za-z.]+)(\/[0-9A-Za-z.]+)*/
// // Example 2, to restricts to either hosptial.com or othersite.com.
// // regex: /(https:\/\/hospital.com(\/[0-9A-Za-z.]+)*)|(https:\/\/othersite.com(\/[0-9A-Za-z.]+)*)/
// regex: /.*/,
// },
dataSources: [
{
friendlyName: 'dcmjs DICOMWeb Server',
Expand Down
34 changes: 21 additions & 13 deletions platform/viewer/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,28 @@ import {
modes as defaultModes,
extensions as defaultExtensions,
} from './pluginImports';
import loadDynamicConfig from './loadDynamicConfig';

/**
* Combine our appConfiguration with installed extensions and modes.
* In the future appConfiguration may contain modes added at runtime.
* */
const appProps = {
config: window ? window.config : {},
defaultExtensions,
defaultModes,
};
loadDynamicConfig(window.config).then(config_json => {
// Reset Dynamic config if defined
if (config_json !== null) {
window.config = config_json;
}

/**
* Combine our appConfiguration with installed extensions and modes.
* In the future appConfiguration may contain modes added at runtime.
* */
const appProps = {
config: window ? window.config : {},
defaultExtensions,
defaultModes,
};

/** Create App */
const app = React.createElement(App, appProps, null);
/** Render */
ReactDOM.render(app, document.getElementById('root'));
/** Create App */
const app = React.createElement(App, appProps, null);
/** Render */
ReactDOM.render(app, document.getElementById('root'));
});

export { history };
23 changes: 23 additions & 0 deletions platform/viewer/src/loadDynamicConfig.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
export default async config => {
const useDynamicConfig = config.dangerouslyUseDynamicConfig;

// Check if dangerouslyUseDynamicConfig enabled
if (useDynamicConfig?.enabled) {
// If enabled then get configUrl query-string
let query = new URLSearchParams(window.location.search);
let configUrl = query.get('configUrl');

if (configUrl) {
// validate regex
const regex = useDynamicConfig.regex;

if (configUrl.match(regex)) {
const response = await fetch(configUrl);
return response.json();
} else {
return null;
}
}
}
return null;
};
1 change: 1 addition & 0 deletions platform/viewer/src/routes/DataSourceWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ function _getQueryFilterValues(query, queryLimit) {
// Offset...
offset:
Math.floor((pageNumber * resultsPerPage) / queryLimit) * (queryLimit - 1),
config: query.get('configUrl'),
};

// patientName: good
Expand Down
11 changes: 9 additions & 2 deletions platform/viewer/src/routes/WorkList/WorkList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -345,12 +345,17 @@ function WorkList({
// mode.routeName
// mode.routes[x].path
// Don't specify default data source, and it should just be picked up... (this may not currently be the case)
// How do we know which params to pass? Today, it's just StudyInstanceUIDs
// How do we know which params to pass? Today, it's just StudyInstanceUIDs and configUrl if exists
const query = new URLSearchParams();
if (filterValues.configUrl) {
query.append('configUrl', filterValues.configUrl);
}
query.append('StudyInstanceUIDs', studyInstanceUid);
return (
<Link
key={i}
to={`${dataPath ? '../../' : ''}${mode.routeName}${dataPath ||
''}?StudyInstanceUIDs=${studyInstanceUid}`}
''}?${query.toString()}`}
// to={`${mode.routeName}/dicomweb?StudyInstanceUIDs=${studyInstanceUid}`}
>
<Button
Expand Down Expand Up @@ -531,6 +536,7 @@ const defaultFilterValues = {
pageNumber: 1,
resultsPerPage: 25,
datasources: '',
configUrl: null,
};

function _tryParseInt(str, defaultValue) {
Expand Down Expand Up @@ -561,6 +567,7 @@ function _getQueryFilterValues(params) {
pageNumber: _tryParseInt(params.get('pagenumber'), undefined),
resultsPerPage: _tryParseInt(params.get('resultsperpage'), undefined),
datasources: params.get('datasources'),
configUrl: params.get('configurl'),
};

// Delete null/undefined keys
Expand Down