From 53694da2164809573a2f96eb8b7d870454bae39c Mon Sep 17 00:00:00 2001 From: Luis Miguel Santos <127312735+luissantosHCIT@users.noreply.github.com> Date: Thu, 16 Oct 2025 20:01:34 -0400 Subject: [PATCH] [Ready] fix 5288 removal of accept header options in retrieve metadata (#5293) * chore: Minor adjustment to generateAcceptHeader function signature to reflect expected return type. Signed-off-by: Luis M. Santos * fix: Added switch to skip the Accept header generation when requesting metadata. WADO metadata request is more likely to return JSON and some VNAs do not like the extra options in the Accept header. Also, passing an empty array is not sufficient because somewhere we still include a comma that breaks the Accept header. It's better to omit it for the metadata request only. Signed-off-by: Luis M. Santos * chore: Added new HeadersInterface interface type docstrings. Signed-off-by: Luis M. Santos * chore: Refactored getAuthorizationHeader() function signature so it is typed checked. Of course, I upgraded the module to a TypeScript module. Moved the request header interfaces into its own TypeScript module (RequestHeaders.ts) in the core types. Signed-off-by: Luis M. Santos * chore: Refactored the user logic to include a TypeScript interface. As a result, upgraded the source file to TypeScript. Removed the User interface from RequestHeaders module and moved them to the user module. Signed-off-by: Luis M. Santos * chore: Updated function signatures, user import, and confirmed unit tests are passing. Signed-off-by: Luis M. Santos * chore: Minor stylistic adjustment to getAuthorizationHeader.test.ts Signed-off-by: Luis M. Santos * chore: Added missing comments. Signed-off-by: Luis M. Santos * chore adjusted generateWadoHeader parameter's name and added comment about the expected default header. Signed-off-by: Luis M. Santos * Just tweaking the interface a bit to be more consistent. * fix authorization header signature change --------- Signed-off-by: Luis M. Santos Co-authored-by: Bill Wallace Co-authored-by: Alireza --- .../default/src/DicomWebDataSource/index.ts | 59 ++++++++++++++----- ...test.js => getAuthorizationHeader.test.ts} | 15 ++--- ...ionHeader.js => getAuthorizationHeader.ts} | 23 +++++--- platform/core/src/DICOMWeb/index.js | 2 +- platform/core/src/index.ts | 2 +- platform/core/src/types/RequestHeaders.ts | 35 +++++++++++ platform/core/src/user.js | 13 ---- platform/core/src/user.ts | 31 ++++++++++ .../core/src/utils/generateAcceptHeader.ts | 2 +- 9 files changed, 135 insertions(+), 47 deletions(-) rename platform/core/src/DICOMWeb/{getAuthorizationHeader.test.js => getAuthorizationHeader.test.ts} (77%) rename platform/core/src/DICOMWeb/{getAuthorizationHeader.js => getAuthorizationHeader.ts} (56%) create mode 100644 platform/core/src/types/RequestHeaders.ts delete mode 100644 platform/core/src/user.js create mode 100644 platform/core/src/user.ts diff --git a/extensions/default/src/DicomWebDataSource/index.ts b/extensions/default/src/DicomWebDataSource/index.ts index afc781b21..5fb351767 100644 --- a/extensions/default/src/DicomWebDataSource/index.ts +++ b/extensions/default/src/DicomWebDataSource/index.ts @@ -16,6 +16,7 @@ import { retrieveStudyMetadata, deleteStudyMetadataPromise } from './retrieveStu import StaticWadoClient from './utils/StaticWadoClient'; import getDirectURL from '../utils/getDirectURL'; import { fixBulkDataURI } from './utils/fixBulkDataURI'; +import {HeadersInterface} from '@ohif/core/src/types/RequestHeaders'; const { DicomMetaDictionary, DicomDict } = dcmjs.data; @@ -97,6 +98,20 @@ export type BulkDataURIConfig = { relativeResolution?: 'studies' | 'series'; }; +/** + * The header options are the options passed into the generateWadoHeader + * command. This takes an extensible set of attributes to allow future enhancements. + */ +export interface HeaderOptions { + includeTransferSyntax?: boolean; +} + +/** + * Metadata and some other requests don't permit the transfer syntax to be included, + * so pass in the excludeTransferSyntax parameter. + */ +export const excludeTransferSyntax: HeaderOptions = { includeTransferSyntax: false }; + /** * Creates a DICOM Web API based on the provided configuration. * @@ -128,7 +143,7 @@ function createDicomWebApi(dicomWebConfig: DicomWebConfig, servicesManager) { dicomWebConfigCopy = JSON.parse(JSON.stringify(dicomWebConfig)); getAuthorizationHeader = () => { - const xhrRequestHeaders = {}; + const xhrRequestHeaders: HeadersInterface = {}; const authHeaders = userAuthenticationService.getAuthorizationHeader(); if (authHeaders && authHeaders.Authorization) { xhrRequestHeaders.Authorization = authHeaders.Authorization; @@ -136,19 +151,33 @@ function createDicomWebApi(dicomWebConfig: DicomWebConfig, servicesManager) { return xhrRequestHeaders; }; - generateWadoHeader = () => { + /** + * Generates the wado header for requesting resources from DICOMweb. + * These are classified into those that are dependent on the transfer syntax + * and those that aren't, as defined by the include transfer syntax attribute. + */ + generateWadoHeader = (options: HeaderOptions): HeadersInterface => { const authorizationHeader = getAuthorizationHeader(); - //Generate accept header depending on config params - const formattedAcceptHeader = utils.generateAcceptHeader( - dicomWebConfig.acceptHeader, - dicomWebConfig.requestTransferSyntaxUID, - dicomWebConfig.omitQuotationForMultipartRequest - ); - - return { - ...authorizationHeader, - Accept: formattedAcceptHeader, - }; + if (options?.includeTransferSyntax!==false) { + //Generate accept header depending on config params + const formattedAcceptHeader = utils.generateAcceptHeader( + dicomWebConfig.acceptHeader, + dicomWebConfig.requestTransferSyntaxUID, + dicomWebConfig.omitQuotationForMultipartRequest + ); + return { + ...authorizationHeader, + Accept: formattedAcceptHeader, + }; + } else { + // The base header will be included in the request. We simply skip customization options around + // transfer syntaxes and whether the request is multipart. In other words, a request in + // which the server expects Accept: application/dicom+json will still include that in the + // header. + return { + ...authorizationHeader + }; + } }; qidoConfig = { @@ -410,7 +439,7 @@ function createDicomWebApi(dicomWebConfig: DicomWebConfig, servicesManager) { madeInClient ) => { const enableStudyLazyLoad = false; - wadoDicomWebClient.headers = generateWadoHeader(); + wadoDicomWebClient.headers = generateWadoHeader(excludeTransferSyntax); // data is all SOPInstanceUIDs const data = await retrieveStudyMetadata( wadoDicomWebClient, @@ -484,7 +513,7 @@ function createDicomWebApi(dicomWebConfig: DicomWebConfig, servicesManager) { returnPromises = false ) => { const enableStudyLazyLoad = true; - wadoDicomWebClient.headers = generateWadoHeader(); + wadoDicomWebClient.headers = generateWadoHeader(excludeTransferSyntax); // Get Series const { preLoadData: seriesSummaryMetadata, promises: seriesPromises } = await retrieveStudyMetadata( diff --git a/platform/core/src/DICOMWeb/getAuthorizationHeader.test.js b/platform/core/src/DICOMWeb/getAuthorizationHeader.test.ts similarity index 77% rename from platform/core/src/DICOMWeb/getAuthorizationHeader.test.js rename to platform/core/src/DICOMWeb/getAuthorizationHeader.test.ts index 9bf0b4636..2ac4c334a 100644 --- a/platform/core/src/DICOMWeb/getAuthorizationHeader.test.js +++ b/platform/core/src/DICOMWeb/getAuthorizationHeader.test.ts @@ -1,7 +1,8 @@ import getAuthorizationHeader from './getAuthorizationHeader'; import user from './../user'; +import { HeadersInterface, RequestOptions } from '../types/RequestHeaders'; -jest.mock('./../user.js'); +jest.mock('../user'); describe('getAuthorizationHeader', () => { it('should return a HTTP Basic Auth when server contains requestOptions.auth', () => { @@ -15,7 +16,7 @@ describe('getAuthorizationHeader', () => { Authorization: `Basic ${btoa(validServer.requestOptions.auth)}`, }; - const authentication = getAuthorizationHeader(validServer); + const authentication: HeadersInterface = getAuthorizationHeader(validServer); expect(authentication).toEqual(expectedAuthorizationHeader); }); @@ -31,13 +32,13 @@ describe('getAuthorizationHeader', () => { Authorization: `Basic ${btoa(validServerWithoutPassword.requestOptions.auth)}`, }; - const authentication = getAuthorizationHeader(validServerWithoutPassword); + const authentication: HeadersInterface = getAuthorizationHeader(validServerWithoutPassword); expect(authentication).toEqual(expectedAuthorizationHeader); }); it('should return a HTTP Basic Auth when server contains requestOptions.auth custom function', () => { - const validServerCustomAuth = { + const validServerCustomAuth: RequestOptions = { requestOptions: { auth: options => `Basic ${options.token}`, token: 'ZHVtbXlfdXNlcjpkdW1teV9wYXNzd29yZA==', @@ -48,13 +49,13 @@ describe('getAuthorizationHeader', () => { Authorization: `Basic ${validServerCustomAuth.requestOptions.token}`, }; - const authentication = getAuthorizationHeader(validServerCustomAuth); + const authentication: HeadersInterface = getAuthorizationHeader(validServerCustomAuth); expect(authentication).toEqual(expectedAuthorizationHeader); }); it('should return an empty object when there is no either server.requestOptions.auth or accessToken', () => { - const authentication = getAuthorizationHeader({}); + const authentication: HeadersInterface = getAuthorizationHeader({}); expect(authentication).toEqual({}); }); @@ -62,7 +63,7 @@ describe('getAuthorizationHeader', () => { it('should return an Authorization with accessToken when server is not defined and there is an accessToken', () => { user.getAccessToken.mockImplementationOnce(() => 'MOCKED_TOKEN'); - const authentication = getAuthorizationHeader({}, user); + const authentication: HeadersInterface = getAuthorizationHeader({}, user); const expectedHeaderBasedOnUserAccessToken = { Authorization: 'Bearer MOCKED_TOKEN', }; diff --git a/platform/core/src/DICOMWeb/getAuthorizationHeader.js b/platform/core/src/DICOMWeb/getAuthorizationHeader.ts similarity index 56% rename from platform/core/src/DICOMWeb/getAuthorizationHeader.js rename to platform/core/src/DICOMWeb/getAuthorizationHeader.ts index 4d996bb97..b7736bb5e 100644 --- a/platform/core/src/DICOMWeb/getAuthorizationHeader.js +++ b/platform/core/src/DICOMWeb/getAuthorizationHeader.ts @@ -1,23 +1,29 @@ import 'isomorphic-base64'; -import user from '../user'; +import {UserAccountInterface} from '../user'; +import { HeadersInterface, RequestOptions } from '../types/RequestHeaders'; /** * Returns the Authorization header as part of an Object. * * @export * @param {Object} [server={}] - * @param {Object} [server.requestOptions] - * @param {string|function} [server.requestOptions.auth] + * @param {Object} [requestOptions] + * @param {string|function} [requestOptions.auth] + * @param {Object} [user] + * @param {function} [user.getAccessToken] * @returns {Object} { Authorization } */ -export default function getAuthorizationHeader({ requestOptions } = {}, user) { - const headers = {}; +export default function getAuthorizationHeader( + {requestOptions}: RequestOptions = {}, + user: UserAccountInterface = {}): HeadersInterface +{ + const headers: HeadersInterface = {}; // Check for OHIF.user since this can also be run on the server const accessToken = user && user.getAccessToken && user.getAccessToken(); // Auth for a specific server - if (requestOptions && requestOptions.auth) { + if (requestOptions?.auth) { if (typeof requestOptions.auth === 'function') { // Custom Auth Header headers.Authorization = requestOptions.auth(requestOptions); @@ -25,9 +31,8 @@ export default function getAuthorizationHeader({ requestOptions } = {}, user) { // HTTP Basic Auth (user:password) headers.Authorization = `Basic ${btoa(requestOptions.auth)}`; } - } - // Auth for the user's default - else if (accessToken) { + } else if (accessToken) { + // Auth for the user's default headers.Authorization = `Bearer ${accessToken}`; } diff --git a/platform/core/src/DICOMWeb/index.js b/platform/core/src/DICOMWeb/index.js index 0775d9274..f8d7bf23b 100644 --- a/platform/core/src/DICOMWeb/index.js +++ b/platform/core/src/DICOMWeb/index.js @@ -1,5 +1,5 @@ import getAttribute from './getAttribute.js'; -import getAuthorizationHeader from './getAuthorizationHeader.js'; +import getAuthorizationHeader from './getAuthorizationHeader'; import getModalities from './getModalities.js'; import getName from './getName.js'; import getNumber from './getNumber.js'; diff --git a/platform/core/src/index.ts b/platform/core/src/index.ts index 134f471d2..91c80df56 100644 --- a/platform/core/src/index.ts +++ b/platform/core/src/index.ts @@ -9,7 +9,7 @@ import errorHandler from './errorHandler.js'; import log from './log.js'; import object from './object.js'; import string from './string.js'; -import user from './user.js'; +import user from './user'; import utils from './utils'; import defaults from './defaults'; import * as Types from './types'; diff --git a/platform/core/src/types/RequestHeaders.ts b/platform/core/src/types/RequestHeaders.ts new file mode 100644 index 000000000..2ed11722b --- /dev/null +++ b/platform/core/src/types/RequestHeaders.ts @@ -0,0 +1,35 @@ +/** + * Interface to clearly present the expected fields to linters when building a request header. + */ +export interface HeadersInterface { + /** + * Request Accept options. For example, + * `['multipart/related; type=application/octet-stream; transfer-syntax=1.2.840.10008.1.2.1.99',]`. + * + * Defines to the server the formats it can use to deliver data to us. + */ + Accept?: string[]; + /** + * Request Authorization field. It can be overridden with the `requestOptions.auth` config item. + * Contains the authorization credentials or tokens necessary to authorize the request with the + * server. + */ + Authorization?: string; +} + +/** + * Interface to clearly present the expected fields to linters when passing the configuration's + * requestOptions struct. + */ +export interface RequestOptions { + requestOptions?: { + /** + * Authentication options to include. Can be a function. + */ + auth?: Function | string; + /** + * Authentication token. Satisfies the test requirement? + */ + token?: string; + } +} diff --git a/platform/core/src/user.js b/platform/core/src/user.js deleted file mode 100644 index 9e5df8d06..000000000 --- a/platform/core/src/user.js +++ /dev/null @@ -1,13 +0,0 @@ -// These should be overridden by the implementation -let user = { - userLoggedIn: () => false, - getUserId: () => null, - getName: () => null, - getAccessToken: () => null, - login: () => new Promise((resolve, reject) => reject()), - logout: () => new Promise((resolve, reject) => reject()), - getData: key => null, - setData: (key, value) => null, -}; - -export default user; diff --git a/platform/core/src/user.ts b/platform/core/src/user.ts new file mode 100644 index 000000000..f7acd7d05 --- /dev/null +++ b/platform/core/src/user.ts @@ -0,0 +1,31 @@ +/** + * Global user information, to be replaced with a specific version which + * applies the methods. + */ +export let user = { + userLoggedIn: (): boolean => false, + getUserId: () => null, + getName: () => null, + getAccessToken: () => null, + login: () => new Promise((resolve, reject) => reject()), + logout: () => new Promise((resolve, reject) => reject()), + getData: key => null, + setData: (key, value) => null, +}; + +/** + * Interface to clearly present the expected fields to linters when passing the user account + * struct. + */ +export interface UserAccountInterface { + userLoggedIn?: () => boolean; + getUserId?: () => null; + getName?: () => null; + getAccessToken?: () => null; + login?: () => Promise; + logout?: () => Promise; + getData?: (key: any) => null; + setData?: (key: any, value: any) => null; +} + +export default user; diff --git a/platform/core/src/utils/generateAcceptHeader.ts b/platform/core/src/utils/generateAcceptHeader.ts index c384411a3..c20778c4f 100644 --- a/platform/core/src/utils/generateAcceptHeader.ts +++ b/platform/core/src/utils/generateAcceptHeader.ts @@ -1,5 +1,5 @@ const generateAcceptHeader = ( - configAcceptHeader = [], + configAcceptHeader: string[] = [], requestTransferSyntaxUID = '*', //default to accept all transfer syntax omitQuotationForMultipartRequest = false ): string[] => {