fix(configuration): Harden dynamic datasource URL trust boundaries and credential handling. 3.12 (#5978)
This commit is contained in:
parent
bab20143fd
commit
838f51957e
2
bun.lock
2
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",
|
||||
},
|
||||
},
|
||||
|
||||
@ -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);
|
||||
}
|
||||
|
||||
@ -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 = [];
|
||||
|
||||
|
||||
115
extensions/default/src/utils/secureConfigFetch.js
Normal file
115
extensions/default/src/utils/secureConfigFetch.js
Normal file
@ -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 };
|
||||
163
extensions/default/src/utils/secureConfigFetch.test.js
Normal file
163
extensions/default/src/utils/secureConfigFetch.test.js
Normal file
@ -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',
|
||||
})
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
@ -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',
|
||||
// ],
|
||||
},
|
||||
},
|
||||
{
|
||||
|
||||
@ -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": {
|
||||
|
||||
@ -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`.
|
||||
|
||||
Loading…
Reference in New Issue
Block a user