diff --git a/bun.lock b/bun.lock index 507f39b59..6985712f3 100644 --- a/bun.lock +++ b/bun.lock @@ -824,7 +824,7 @@ "@cornerstonejs/core": "4.15.29", "@cornerstonejs/dicom-image-loader": "4.15.29", "@ohif/ui": "3.12.0", - "cornerstone-math": "0.1.9", + "cornerstone-math": "0.1.10", "dicom-parser": "1.8.21", }, }, diff --git a/extensions/default/src/DicomJSONDataSource/index.js b/extensions/default/src/DicomJSONDataSource/index.js index 18949f6e6..ef47e9bb8 100644 --- a/extensions/default/src/DicomJSONDataSource/index.js +++ b/extensions/default/src/DicomJSONDataSource/index.js @@ -4,6 +4,7 @@ import qs from 'query-string'; import getImageId from '../DicomWebDataSource/utils/getImageId'; import getDirectURL from '../utils/getDirectURL'; +import { resolveConfigFetchPolicy, fetchConfigJson } from '../utils/secureConfigFetch'; const metadataProvider = OHIF.classes.MetadataProvider; @@ -59,13 +60,18 @@ const findStudies = (key, value) => { return studies; }; -function createDicomJSONApi(dicomJsonConfig) { +function createDicomJSONApi(dicomJsonConfig, servicesManager) { + const { userAuthenticationService } = servicesManager.services; const implementation = { initialize: async ({ query, url }) => { if (!url) { url = query.get('url'); } - let metaData = getMetaDataByURL(url); + const evaluatedUrl = resolveConfigFetchPolicy(url, { + allowedOrigins: dicomJsonConfig.dangerouslyAllowedOriginsForAuthenticatedEnvironments, + userAuthenticationService, + }); + let metaData = getMetaDataByURL(evaluatedUrl.normalizedUrl); // if we have already cached the data from this specific url // We are only handling one StudyInstanceUID to run; however, @@ -76,8 +82,7 @@ function createDicomJSONApi(dicomJsonConfig) { }); } - const response = await fetch(url); - const data = await response.json(); + const data = await fetchConfigJson(evaluatedUrl); let StudyInstanceUID; let SeriesInstanceUID; @@ -105,11 +110,11 @@ function createDicomJSONApi(dicomJsonConfig) { }); _store.urls.push({ - url, + url: evaluatedUrl.normalizedUrl, studies: [...data.studies], }); _store.studyInstanceUIDMap.set( - url, + evaluatedUrl.normalizedUrl, data.studies.map(study => study.StudyInstanceUID) ); }, @@ -252,6 +257,8 @@ function createDicomJSONApi(dicomJsonConfig) { console.warn(' DICOMJson store dicom not implemented'); }, }, + reject: {}, + deleteStudyMetadataPromise: () => {}, getImageIdsForDisplaySet(displaySet) { const images = displaySet.images; const imageIds = []; @@ -297,8 +304,21 @@ function createDicomJSONApi(dicomJsonConfig) { }, getStudyInstanceUIDs: ({ params, query }) => { const url = query.get('url'); - return _store.studyInstanceUIDMap.get(url); + if (!url) { + return; + } + + try { + const evaluatedUrl = resolveConfigFetchPolicy(url, { + allowedOrigins: dicomJsonConfig.dangerouslyAllowedOriginsForAuthenticatedEnvironments, + userAuthenticationService, + }); + return _store.studyInstanceUIDMap.get(evaluatedUrl.normalizedUrl); + } catch { + return; + } }, + getConfig: () => dicomJsonConfig, }; return IWebApiDataSource.create(implementation); } diff --git a/extensions/default/src/DicomWebProxyDataSource/index.ts b/extensions/default/src/DicomWebProxyDataSource/index.ts index 8c83899aa..28bb70d19 100644 --- a/extensions/default/src/DicomWebProxyDataSource/index.ts +++ b/extensions/default/src/DicomWebProxyDataSource/index.ts @@ -1,5 +1,6 @@ import { IWebApiDataSource } from '@ohif/core'; import { createDicomWebApi } from '../DicomWebDataSource/index'; +import { resolveConfigFetchPolicy, fetchConfigJson } from '../utils/secureConfigFetch'; /** * This datasource is initialized with a url that returns a JSON object with a @@ -11,6 +12,7 @@ import { createDicomWebApi } from '../DicomWebDataSource/index'; */ function createDicomWebProxyApi(dicomWebProxyConfig, servicesManager: AppTypes.ServicesManager) { const { name } = dicomWebProxyConfig; + const { userAuthenticationService } = servicesManager.services; let dicomWebDelegate = undefined; const implementation = { @@ -20,12 +22,14 @@ function createDicomWebProxyApi(dicomWebProxyConfig, servicesManager: AppTypes.S if (!url) { throw new Error(`No url for '${name}'`); } else { - const response = await fetch(url); - const data = await response.json(); + const evaluatedUrl = resolveConfigFetchPolicy(url, { + allowedOrigins: dicomWebProxyConfig.dangerouslyAllowedOriginsForAuthenticatedEnvironments, + userAuthenticationService, + }); + const data = await fetchConfigJson(evaluatedUrl); if (!data.servers?.dicomWeb?.[0]) { throw new Error('Invalid configuration returned by url'); } - dicomWebDelegate = createDicomWebApi( data.servers.dicomWeb[0].configuration || data.servers.dicomWeb[0], servicesManager @@ -54,9 +58,16 @@ function createDicomWebProxyApi(dicomWebProxyConfig, servicesManager: AppTypes.S store: { dicom: (...args) => dicomWebDelegate.store.dicom(...args), }, - deleteStudyMetadataPromise: (...args) => dicomWebDelegate.deleteStudyMetadataPromise(...args), - getImageIdsForDisplaySet: (...args) => dicomWebDelegate.getImageIdsForDisplaySet(...args), - getImageIdsForInstance: (...args) => dicomWebDelegate.getImageIdsForInstance(...args), + reject: { + series: (...args) => dicomWebDelegate?.reject?.series?.(...args), + }, + deleteStudyMetadataPromise: (...args) => dicomWebDelegate?.deleteStudyMetadataPromise?.(...args), + getImageIdsForDisplaySet: (...args) => dicomWebDelegate?.getImageIdsForDisplaySet?.(...args), + getImageIdsForInstance: (...args) => dicomWebDelegate?.getImageIdsForInstance?.(...args), + getConfig: (...args) => + dicomWebDelegate?.getConfig?.(...args) ?? { + dicomUploadEnabled: false, + }, getStudyInstanceUIDs({ params, query }) { let studyInstanceUIDs = []; diff --git a/extensions/default/src/utils/secureConfigFetch.js b/extensions/default/src/utils/secureConfigFetch.js new file mode 100644 index 000000000..63fbc5429 --- /dev/null +++ b/extensions/default/src/utils/secureConfigFetch.js @@ -0,0 +1,115 @@ +// @ts-nocheck + +function normalizeAllowedOrigins(allowedOrigins = []) { + if (!Array.isArray(allowedOrigins)) { + return []; + } + + const configuredOrigins = allowedOrigins + .filter(origin => typeof origin === 'string') + .map(origin => origin.trim()) + .filter(Boolean); + + return configuredOrigins + .map(origin => { + try { + const parsedOrigin = new URL(origin); + if (!['http:', 'https:'].includes(parsedOrigin.protocol)) { + console.error( + `[secureConfigFetch] Ignoring misconfigured allowed origin "${origin}". ` + + 'Entries must use http:// or https://.' + ); + return null; + } + if ( + parsedOrigin.username || + parsedOrigin.password || + parsedOrigin.pathname !== '/' || + parsedOrigin.search || + parsedOrigin.hash + ) { + console.error( + `[secureConfigFetch] Ignoring misconfigured allowed origin "${origin}". ` + + 'Entries must be bare origins only (scheme + host + optional port), with no username/password, path, query, or hash.' + ); + return null; + } + return parsedOrigin.origin; + } catch { + console.error( + `[secureConfigFetch] Ignoring misconfigured allowed origin "${origin}". Entry is not a valid URL.` + ); + return null; + } + }) + .filter(Boolean); +} + +function resolveConfigUrl(rawUrl) { + if (!rawUrl || typeof rawUrl !== 'string') { + throw new Error('Missing required "url" query parameter'); + } + + try { + return new URL(rawUrl, window.location.href); + } catch { + throw new Error('Invalid URL in "url" query parameter'); + } +} + +function resolveConfigFetchPolicy(rawUrl, policy = {}) { + const { allowedOrigins = [], userAuthenticationService } = policy; + const parsedUrl = resolveConfigUrl(rawUrl); + const protocol = parsedUrl.protocol.toLowerCase(); + const isSameOrigin = parsedUrl.origin === window.location.origin; + + if (!['http:', 'https:'].includes(protocol)) { + throw new Error('Only HTTP(S) URLs are allowed for dynamic datasource configuration'); + } + + if (parsedUrl.hash) { + throw new Error('URL fragments are not allowed for dynamic datasource configuration'); + } + + if (parsedUrl.username || parsedUrl.password) { + throw new Error('URL userinfo is not allowed for dynamic datasource configuration'); + } + + const isAuthenticated = Boolean( + userAuthenticationService?.getAuthorizationHeader?.()?.Authorization + ); + + if (isAuthenticated && !isSameOrigin) { + const normalizedAllowedOrigins = normalizeAllowedOrigins(allowedOrigins); + if (!normalizedAllowedOrigins.length || !normalizedAllowedOrigins.includes(parsedUrl.origin)) { + throw new Error( + `Blocked remote configuration origin "${parsedUrl.origin}" in authenticated environment` + ); + } + } + + return { + parsedUrl, + normalizedUrl: parsedUrl.toString(), + isAuthenticated, + isSameOrigin, + }; +} + +async function fetchConfigJson(normalizedPolicy) { + const { normalizedUrl } = normalizedPolicy; + const response = await fetch(normalizedUrl, { + method: 'GET', + mode: 'cors', + credentials: 'same-origin', + redirect: 'error', + referrerPolicy: 'no-referrer', + }); + + if (!response.ok) { + throw new Error(`Failed to fetch dynamic datasource configuration (${response.status})`); + } + + return response.json(); +} +export { resolveConfigFetchPolicy, fetchConfigJson }; diff --git a/extensions/default/src/utils/secureConfigFetch.test.js b/extensions/default/src/utils/secureConfigFetch.test.js new file mode 100644 index 000000000..bf71b6d50 --- /dev/null +++ b/extensions/default/src/utils/secureConfigFetch.test.js @@ -0,0 +1,163 @@ +// @ts-nocheck +import { resolveConfigFetchPolicy, fetchConfigJson } from './secureConfigFetch'; + +describe('secureConfigFetch', () => { + describe('resolveConfigFetchPolicy', () => { + it('allows arbitrary origin in unauthenticated environments', () => { + const result = resolveConfigFetchPolicy('https://untrusted.example.com/config.json', { + userAuthenticationService: { + getAuthorizationHeader: () => ({}), + }, + }); + + expect(result.normalizedUrl).toBe('https://untrusted.example.com/config.json'); + expect(result.isAuthenticated).toBe(false); + expect(result.isSameOrigin).toBe(false); + }); + + it('blocks non-allowlisted origins in authenticated environments', () => { + expect(() => + resolveConfigFetchPolicy('https://untrusted.example.com/config.json', { + allowedOrigins: ['https://trusted.example.com'], + userAuthenticationService: { + getAuthorizationHeader: () => ({ Authorization: 'Bearer token123' }), + }, + }) + ).toThrow('Blocked remote configuration origin'); + }); + + it('allows allowlisted origin in authenticated environments', () => { + const result = resolveConfigFetchPolicy('http://localhost:5000/config.json', { + allowedOrigins: ['http://localhost:5000', 'https://trusted.example.com'], + userAuthenticationService: { + getAuthorizationHeader: () => ({ Authorization: 'Bearer token123' }), + }, + }); + + expect(result.normalizedUrl).toBe('http://localhost:5000/config.json'); + expect(result.isAuthenticated).toBe(true); + expect(result.isSameOrigin).toBe(false); + }); + + it('blocks authenticated fetch when allowlist is missing', () => { + expect(() => + resolveConfigFetchPolicy('https://noTrustList.example.com/config.json', { + userAuthenticationService: { + getAuthorizationHeader: () => ({ Authorization: 'Bearer token123' }), + }, + }) + ).toThrow('Blocked remote configuration origin'); + }); + + it('allows same-origin in authenticated environments without allowlist', () => { + const result = resolveConfigFetchPolicy('/protected/config.json', { + userAuthenticationService: { + getAuthorizationHeader: () => ({ Authorization: 'Bearer token123' }), + }, + }); + + expect(result.normalizedUrl).toBe(`${window.location.origin}/protected/config.json`); + expect(result.isAuthenticated).toBe(true); + expect(result.isSameOrigin).toBe(true); + }); + + it('rejects embedded userinfo in config URLs', () => { + expect(() => + resolveConfigFetchPolicy('https://user:pass@trusted.example.com/config.json', { + allowedOrigins: ['https://trusted.example.com'], + userAuthenticationService: { + getAuthorizationHeader: () => ({ Authorization: 'Bearer token123' }), + }, + }) + ).toThrow('URL userinfo is not allowed for dynamic datasource configuration'); + }); + }); + + describe('fetchConfigJson', () => { + const originalFetch = global.fetch; + + beforeEach(() => { + global.fetch = jest.fn(); + }); + + afterEach(() => { + jest.restoreAllMocks(); + global.fetch = originalFetch; + }); + + it('uses hardened fetch options for unauthenticated cross-origin requests', async () => { + global.fetch.mockResolvedValue({ + status: 200, + ok: true, + json: async () => ({ ok: true }), + }); + + await fetchConfigJson({ + normalizedUrl: 'https://example.com/config.json', + isAuthenticated: false, + isSameOrigin: false, + }); + + expect(global.fetch).toHaveBeenCalledWith( + 'https://example.com/config.json', + expect.objectContaining({ + method: 'GET', + mode: 'cors', + credentials: 'same-origin', + redirect: 'error', + referrerPolicy: 'no-referrer', + }) + ); + }); + + it('uses hardened fetch options for unauthenticated same-origin requests', async () => { + global.fetch.mockResolvedValue({ + status: 200, + ok: true, + json: async () => ({ ok: true }), + }); + + await fetchConfigJson({ + normalizedUrl: `${window.location.origin}/protected/config.json`, + isAuthenticated: false, + isSameOrigin: true, + }); + + expect(global.fetch).toHaveBeenCalledWith( + `${window.location.origin}/protected/config.json`, + expect.objectContaining({ + method: 'GET', + mode: 'cors', + credentials: 'same-origin', + redirect: 'error', + referrerPolicy: 'no-referrer', + }) + ); + }); + + it('uses hardened fetch options in authenticated environments', async () => { + global.fetch.mockResolvedValue({ + status: 200, + ok: true, + json: async () => ({ ok: true }), + }); + + await fetchConfigJson({ + normalizedUrl: 'https://trusted.example.com/config.json', + isAuthenticated: true, + isSameOrigin: false, + }); + + expect(global.fetch).toHaveBeenCalledWith( + 'https://trusted.example.com/config.json', + expect.objectContaining({ + method: 'GET', + mode: 'cors', + credentials: 'same-origin', + redirect: 'error', + referrerPolicy: 'no-referrer', + }) + ); + }); + }); +}); diff --git a/platform/app/public/config/default.js b/platform/app/public/config/default.js index f29a70acb..955b60d86 100644 --- a/platform/app/public/config/default.js +++ b/platform/app/public/config/default.js @@ -251,6 +251,12 @@ window.config = { configuration: { friendlyName: 'dicomweb delegating proxy', name: 'dicomwebproxy', + // Security controls for runtime ?url=... datasource loading: + // In authenticated environments, runtime ?url origins must be allowlisted: + // dangerouslyAllowedOriginsForAuthenticatedEnvironments: [ + // 'https://config.example.com', + // 'http://localhost:5000', + // ], }, }, { @@ -259,6 +265,12 @@ window.config = { configuration: { friendlyName: 'dicom json', name: 'json', + // Security controls for runtime ?url=... datasource loading: + // In authenticated environments, runtime ?url origins must be allowlisted: + // dangerouslyAllowedOriginsForAuthenticatedEnvironments: [ + // 'https://config.example.com', + // 'http://localhost:5000', + // ], }, }, { diff --git a/platform/core/package.json b/platform/core/package.json index d1094b0cc..e72995a28 100644 --- a/platform/core/package.json +++ b/platform/core/package.json @@ -40,7 +40,7 @@ "@cornerstonejs/core": "4.15.29", "@cornerstonejs/dicom-image-loader": "4.15.29", "@ohif/ui": "3.12.0", - "cornerstone-math": "0.1.9", + "cornerstone-math": "0.1.10", "dicom-parser": "1.8.21" }, "dependencies": { diff --git a/platform/docs/docs/deployment/authorization.md b/platform/docs/docs/deployment/authorization.md index 76abd915b..ae19fcd83 100644 --- a/platform/docs/docs/deployment/authorization.md +++ b/platform/docs/docs/deployment/authorization.md @@ -66,6 +66,56 @@ http://localhost:3000/viewer?StudyInstanceUIDs=1.2.3.4.5.6.6.7&token=e123125jsdf +## Securing dynamic datasource URLs +When using `dicomwebproxy` or `dicomjson` data sources with a runtime `?url=...` query parameter, +configure explicit trust boundaries to prevent credential exfiltration. + +Use these datasource configuration options: + +- `dangerouslyAllowedOriginsForAuthenticatedEnvironments`: Origin allowlist used only when the viewer is running in an authenticated environment. Entries may use `http://` or `https://` and can include localhost origins. + +Allowed entry format for `dangerouslyAllowedOriginsForAuthenticatedEnvironments`: + +- Must be a bare origin only: `scheme://host[:port]` +- `http://` and `https://` are both allowed +- Localhost is allowed when explicitly listed (for example, `http://localhost:5000`) +- Must not include username/password, path, query string, or hash +- Invalid entries are ignored and logged as misconfigured + +Policy summary: + +- In unauthenticated environments, any HTTP(S) `?url=` origin is allowed. +- In authenticated environments, same-origin `?url=` values are allowed by default. +- In authenticated environments, cross-origin `?url=` values must be present in `dangerouslyAllowedOriginsForAuthenticatedEnvironments`, otherwise loading fails closed. +- In unauthenticated environments, cross-origin config URLs are fetched with: + - `method: 'GET'` + - `mode: 'cors'` + - `credentials: 'omit'` + - `redirect: 'error'` + - `referrerPolicy: 'no-referrer'` +- In unauthenticated environments, same-origin config URLs use a plain `fetch()` call (browser default `credentials: 'same-origin'`). +- Same-origin config URLs are fetched using simple fetch behavior (so same-origin session/cookie auth is preserved). +- In authenticated environments, allowlisted cross-origin config URLs are fetched using simple fetch behavior. +- Returned datasource configuration payloads are consumed as-is (no additional URL/config scrubbing). + +Example: + +```js +dataSources: [ + { + namespace: '@ohif/extension-default.dataSourcesModule.dicomwebproxy', + sourceName: 'dicomwebproxy', + configuration: { + name: 'dicomwebproxy', + dangerouslyAllowedOriginsForAuthenticatedEnvironments: [ + 'https://config.example.com', + 'http://localhost:5000', + ], + }, + }, +] +``` + ## Implicit Flow vs Authorization Code Flow The Viewer supports both the Implicit Flow and the Authorization Code Flow. The Implicit Flow is the default currently, as it is easier to set up and use. However, you can opt for better security by using the Authorization Code Flow. To do so, add `useAuthorizationCodeFlow` to the configuration and change the `response_type` from `id_token token` to `code`.