fix(configuration): Harden dynamic datasource URL trust boundaries and credential handling. (#5963)
* fix(configuration): Harden dynamic datasource URL trust boundaries and credential handling. * Remove testing configuration. * Update policies for runtime ?url=... datasource loading. * PR feedback.
This commit is contained in:
1 parent
51e6b35dbb
commit
eede569a88
6 files changed
+334
-13
No files matched your search
@@ -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 = [];
|
||||
|
||||
|
||||
@@ -0,0 +1,118 @@
|
||||
// @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();
|
||||
|
||||
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) {
|
||||
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,
|
||||
};
|
||||
}
|
||||
|
||||
async function fetchConfigJson(normalizedPolicy) {
|
||||
const { normalizedUrl, isAuthenticated } = normalizedPolicy;
|
||||
const response = isAuthenticated
|
||||
? await fetch(normalizedUrl)
|
||||
: await fetch(normalizedUrl, {
|
||||
method: 'GET',
|
||||
mode: 'cors',
|
||||
credentials: 'omit',
|
||||
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,
|
||||
};
|
||||
@@ -0,0 +1,113 @@
|
||||
// @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);
|
||||
});
|
||||
|
||||
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);
|
||||
});
|
||||
|
||||
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('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 in unauthenticated environments', async () => {
|
||||
global.fetch.mockResolvedValue({
|
||||
status: 200,
|
||||
ok: true,
|
||||
json: async () => ({ ok: true }),
|
||||
});
|
||||
|
||||
await fetchConfigJson({
|
||||
normalizedUrl: 'https://example.com/config.json',
|
||||
isAuthenticated: false,
|
||||
});
|
||||
|
||||
expect(global.fetch).toHaveBeenCalledWith(
|
||||
'https://example.com/config.json',
|
||||
expect.objectContaining({
|
||||
method: 'GET',
|
||||
mode: 'cors',
|
||||
credentials: 'omit',
|
||||
redirect: 'error',
|
||||
referrerPolicy: 'no-referrer',
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it('uses simple fetch 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,
|
||||
});
|
||||
|
||||
expect(global.fetch).toHaveBeenCalledWith('https://trusted.example.com/config.json');
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user