Merge pull request #336 from splitio/enhanced_sdk_headers

[Enhanced SDK headers] Implementation

Author: EmilianoSanchez

CHANGES.txt

@@ -1,4 +1,5 @@
-1.17.0 (September XX, 2024)
+1.17.0 (September 6, 2024)
+ - Added `sync.requestOptions.getHeaderOverrides` configuration option to enhance SDK HTTP request Headers for Authorization Frameworks.
  - Added `isTimedout` and `lastUpdate` properties to IStatusInterface to keep track of the timestamp of the last SDK event, used on React and Redux SDKs.
  - Updated some transitive dependencies for vulnerability fixes.
 

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@splitsoftware/splitio-commons",
-  "version": "1.16.1",
+  "version": "1.17.0",
   "description": "Split JavaScript SDK common components",
   "main": "cjs/index.js",
   "module": "esm/index.js",

package.json

@@ -0,0 +1,50 @@
+import { ISettings } from '../../types';
+import { decorateHeaders } from '../decorateHeaders';
+
+const HEADERS = {
+  Authorization: 'Bearer SDK-KEY',
+  SplitSDKVersion: 'JS' // Overriding is forbidden
+};
+
+describe('decorateHeaders', () => {
+
+  test('should not decorate headers if getHeaderOverrides is not provided', () => {
+    const headers = { ...HEADERS };
+    const settings = { sync: {} };
+
+    expect(decorateHeaders(settings as unknown as ISettings, headers)).toEqual(HEADERS);
+  });
+
+  test('should decorate headers with header overrides, ignoring forbidden headers', () => {
+    const headers = { ...HEADERS };
+    const settings = {
+      sync: {
+        requestOptions: {
+          getHeaderOverrides: (context: { headers: Record<string, string> }) => {
+            context.headers['Authorization'] = 'ignored';
+            return { 'Authorization': 'updated', 'OTHER_HEADER': 'other_value', 'SplitSdkVersion': 'FORBIDDEN', 'splitsdkversion': 'FORBIDDEN TOO' };
+          }
+        }
+      }
+    };
+
+    expect(decorateHeaders(settings as unknown as ISettings, headers)).toEqual({ 'Authorization': 'updated', 'OTHER_HEADER': 'other_value', 'SplitSDKVersion': 'JS' });
+  });
+
+  test('should handle errors when decorating headers', () => {
+    const headers = { ...HEADERS };
+    const settings = {
+      sync: {
+        requestOptions: {
+          getHeaderOverrides: () => {
+            throw new Error('Unexpected error');
+          }
+        }
+      },
+      log: { error: jest.fn() }
+    };
+
+    expect(decorateHeaders(settings as unknown as ISettings, headers)).toEqual(HEADERS);
+    expect(settings.log.error).toHaveBeenCalledWith('Problem adding custom headers to request decorator: Error: Unexpected error');
+  });
+});

src/services/__tests__/decorateHeaders.spec.ts

@@ -0,0 +1,33 @@
+import { objectAssign } from '../utils/lang/objectAssign';
+import { _Set } from '../utils/lang/sets';
+import { ISettings } from '../types';
+
+const FORBIDDEN_HEADERS = new _Set([
+  'splitsdkclientkey',
+  'splitsdkversion',
+  'splitsdkmachineip',
+  'splitsdkmachinename',
+  'splitsdkimpressionsmode',
+  'host',
+  'referrer',
+  'content-type',
+  'content-length',
+  'content-encoding',
+  'accept',
+  'keep-alive',
+  'x-fastly-debug'
+]);
+
+export function decorateHeaders(settings: ISettings, headers: Record<string, string>) {
+  if (settings.sync.requestOptions?.getHeaderOverrides) {
+    try {
+      const headerOverrides = settings.sync.requestOptions.getHeaderOverrides({ headers: objectAssign({}, headers) });
+      Object.keys(headerOverrides)
+        .filter(key => !FORBIDDEN_HEADERS.has(key.toLowerCase()))
+        .forEach(key => headers[key] = headerOverrides[key]);
+    } catch (e) {
+      settings.log.error('Problem adding custom headers to request decorator: ' + e);
+    }
+  }
+  return headers;
+}

src/services/decorateHeaders.ts

@@ -3,6 +3,7 @@ import { objectAssign } from '../utils/lang/objectAssign';
 import { ERROR_HTTP, ERROR_CLIENT_CANNOT_GET_READY } from '../logger/constants';
 import { ISettings } from '../types';
 import { IPlatform } from '../sdkFactory/types';
+import { decorateHeaders } from './decorateHeaders';
 
 const messageNoFetch = 'Global fetch API is not available.';
 
@@ -21,20 +22,20 @@ export function splitHttpClientFactory(settings: ISettings, { getOptions, getFet
   // if fetch is not available, log Error
   if (!fetch) log.error(ERROR_CLIENT_CANNOT_GET_READY, [messageNoFetch]);
 
-  const headers: Record<string, string> = {
+  const commonHeaders: Record<string, string> = {
     'Accept': 'application/json',
     'Content-Type': 'application/json',
     'Authorization': `Bearer ${authorizationKey}`,
     'SplitSDKVersion': version
   };
 
-  if (ip) headers['SplitSDKMachineIP'] = ip;
-  if (hostname) headers['SplitSDKMachineName'] = hostname;
+  if (ip) commonHeaders['SplitSDKMachineIP'] = ip;
+  if (hostname) commonHeaders['SplitSDKMachineName'] = hostname;
 
   return function httpClient(url: string, reqOpts: IRequestOptions = {}, latencyTracker: (error?: NetworkError) => void = () => { }, logErrorsAsInfo: boolean = false): Promise<IResponse> {
 
     const request = objectAssign({
-      headers: reqOpts.headers ? objectAssign({}, headers, reqOpts.headers) : headers,
+      headers: decorateHeaders(settings, objectAssign({}, commonHeaders, reqOpts.headers || {})),
       method: reqOpts.method || 'GET',
       body: reqOpts.body
     }, options);

src/services/splitHttpClient.ts

@@ -1,7 +1,7 @@
 // @ts-nocheck
 import EventSourceMock from '../../../../__tests__/testUtils/eventSourceMock';
 import { authDataSample, channelsQueryParamSample } from '../../__tests__/dataMocks';
-import { fullSettings as settings } from '../../../../utils/settingsValidation/__tests__/settings.mocks';
+import { fullSettings as settings, fullSettingsServerSide as settingsServerSide } from '../../../../utils/settingsValidation/__tests__/settings.mocks';
 import { url } from '../../../../utils/settingsValidation/url';
 
 import { SSEClient } from '../index';
@@ -18,12 +18,12 @@ const EXPECTED_HEADERS = {
 
 test('SSClient / instance creation throws error if EventSource is not provided', () => {
   expect(() => { new SSEClient(settings); }).toThrow(Error);
-  expect(() => { new SSEClient(settings, false, {}); }).toThrow(Error);
-  expect(() => { new SSEClient(settings, false, { getEventSource: () => undefined }); }).toThrow(Error);
+  expect(() => { new SSEClient(settings, {}); }).toThrow(Error);
+  expect(() => { new SSEClient(settings, { getEventSource: () => undefined }); }).toThrow(Error);
 });
 
 test('SSClient / instance creation success if EventSource is provided', () => {
-  const instance = new SSEClient(settings, false, { getEventSource: () => EventSourceMock });
+  const instance = new SSEClient(settings, { getEventSource: () => EventSourceMock });
   expect(instance.eventSource).toBe(EventSourceMock);
 });
 
@@ -36,7 +36,7 @@ test('SSClient / setEventHandler, open and close methods', () => {
   };
 
   // instance SSEClient
-  const instance = new SSEClient(settings, false, { getEventSource: () => EventSourceMock });
+  const instance = new SSEClient(settings, { getEventSource: () => EventSourceMock });
   instance.setEventHandler(handler);
 
   // open connection
@@ -80,9 +80,9 @@ test('SSClient / setEventHandler, open and close methods', () => {
 
 });
 
-test('SSClient / open method: URL with metadata query params', () => {
+test('SSClient / open method on client-side: metadata as query params', () => {
 
-  const instance = new SSEClient(settings, false, { getEventSource: () => EventSourceMock });
+  const instance = new SSEClient(settings, { getEventSource: () => EventSourceMock });
   instance.open(authDataSample);
 
   const EXPECTED_BROWSER_URL = EXPECTED_URL + `&SplitSDKVersion=${settings.version}&SplitSDKClientKey=${EXPECTED_HEADERS.SplitSDKClientKey}`;
@@ -91,16 +91,25 @@ test('SSClient / open method: URL with metadata query params', () => {
   expect(instance.connection.__eventSourceInitDict).toEqual({}); // No headers are passed for streaming connection
 });
 
-test('SSClient / open method: URL and metadata headers with IP and Hostname', () => {
+test('SSClient / open method on server-side: metadata as headers', () => {
+
+  const instance = new SSEClient(settingsServerSide, { getEventSource: () => EventSourceMock });
+  instance.open(authDataSample);
+
+  expect(instance.connection.url).toBe(EXPECTED_URL); // URL is properly set for streaming connection
+  expect(instance.connection.__eventSourceInitDict).toEqual({ headers: EXPECTED_HEADERS }); // Headers are properly set for streaming connection
+});
+
+test('SSClient / open method on server-side: metadata with IP and Hostname as headers', () => {
 
   const settingsWithRuntime = {
-    ...settings,
+    ...settingsServerSide,
     runtime: {
       ip: 'some ip',
       hostname: 'some hostname'
     }
   };
-  const instance = new SSEClient(settingsWithRuntime, true, { getEventSource: () => EventSourceMock });
+  const instance = new SSEClient(settingsWithRuntime, { getEventSource: () => EventSourceMock });
   instance.open(authDataSample);
 
   expect(instance.connection.url).toBe(EXPECTED_URL); // URL is properly set for streaming connection
@@ -113,25 +122,44 @@ test('SSClient / open method: URL and metadata headers with IP and Hostname', ()
   }); // Headers are properly set for streaming connection
 });
 
-test('SSClient / open method: URL and metadata headers without IP and Hostname', () => {
+test('SSClient / open method on server-side: metadata as headers and options', () => {
+  const platform = { getEventSource: jest.fn(() => EventSourceMock), getOptions: jest.fn(() => ({ withCredentials: true })) };
 
-  const instance = new SSEClient(settings, true, { getEventSource: () => EventSourceMock });
+  const instance = new SSEClient(settingsServerSide, platform);
   instance.open(authDataSample);
 
   expect(instance.connection.url).toBe(EXPECTED_URL); // URL is properly set for streaming connection
-  expect(instance.connection.__eventSourceInitDict).toEqual({ headers: EXPECTED_HEADERS }); // Headers are properly set for streaming connection
-});
+  expect(instance.connection.__eventSourceInitDict).toEqual({ headers: EXPECTED_HEADERS, withCredentials: true }); // Headers and options are properly set for streaming connection
 
-test('SSClient / open method: URL, metadata headers and options', () => {
-  const platform = { getEventSource: jest.fn(() => EventSourceMock), getOptions: jest.fn(() => ({ withCredentials: true })) };
+  // Assert that getEventSource and getOptions were called once with settings
+  expect(platform.getEventSource.mock.calls).toEqual([[settingsServerSide]]);
+  expect(platform.getOptions.mock.calls).toEqual([[settingsServerSide]]);
+});
 
-  const instance = new SSEClient(settings, true, platform);
+test('SSClient / open method with getHeaderOverrides: custom headers', () => {
+  const settingsWithGetHeaderOverrides = {
+    ...settings,
+    sync: {
+      requestOptions: {
+        getHeaderOverrides: (context) => {
+          expect(context).toEqual({ headers: EXPECTED_HEADERS });
+          context.headers['otherheader'] = 'customvalue';
+          return {
+            SplitSDKClientKey: '4321', // will not be overridden
+            CustomHeader: 'custom-value'
+          };
+        }
+      }
+    }
+  };
+  const instance = new SSEClient(settingsWithGetHeaderOverrides, { getEventSource: () => EventSourceMock });
   instance.open(authDataSample);
 
   expect(instance.connection.url).toBe(EXPECTED_URL); // URL is properly set for streaming connection
-  expect(instance.connection.__eventSourceInitDict).toEqual({ headers: EXPECTED_HEADERS, withCredentials: true }); // Headers and options are properly set for streaming connection
-
-  // Assert that getEventSource and getOptions were called once with settings
-  expect(platform.getEventSource.mock.calls).toEqual([[settings]]);
-  expect(platform.getOptions.mock.calls).toEqual([[settings]]);
+  expect(instance.connection.__eventSourceInitDict).toEqual({
+    headers: {
+      ...EXPECTED_HEADERS,
+      CustomHeader: 'custom-value'
+    }
+  }); // Headers are properly set for streaming connection
 });

src/sync/streaming/SSEClient/__tests__/index.spec.ts

@@ -1,4 +1,5 @@
 import { IPlatform } from '../../../sdkFactory/types';
+import { decorateHeaders } from '../../../services/decorateHeaders';
 import { IEventSourceConstructor } from '../../../services/types';
 import { ISettings } from '../../../types';
 import { isString } from '../../../utils/lang';
@@ -36,29 +37,23 @@ function buildSSEHeaders(settings: ISettings) {
 export class SSEClient implements ISSEClient {
   // Instance properties:
   eventSource?: IEventSourceConstructor;
-  streamingUrl: string;
   connection?: InstanceType<IEventSourceConstructor>;
   handler?: ISseEventHandler;
-  useHeaders?: boolean;
   headers: Record<string, string>;
   options?: object;
 
   /**
    * SSEClient constructor.
    *
    * @param settings Validated settings.
-   * @param useHeaders True to send metadata as headers or false to send as query params. If `true`, the provided EventSource must support headers.
    * @param platform object containing environment-specific dependencies
    * @throws 'EventSource API is not available.' if EventSource is not available.
    */
-  constructor(settings: ISettings, useHeaders: boolean, { getEventSource, getOptions }: IPlatform) {
+  constructor(private settings: ISettings, { getEventSource, getOptions }: IPlatform) {
     this.eventSource = getEventSource && getEventSource(settings);
     // if eventSource is not available, throw an exception
     if (!this.eventSource) throw new Error('EventSource API is not available.');
 
-    this.streamingUrl = settings.urls.streaming + '/sse';
-    // @TODO get `useHeaders` flag from `getEventSource`, to use EventSource headers on client-side SDKs when possible.
-    this.useHeaders = useHeaders;
     this.headers = buildSSEHeaders(settings);
     this.options = getOptions && getOptions(settings);
   }
@@ -82,14 +77,16 @@ export class SSEClient implements ISSEClient {
         return encodeURIComponent(params + channel);
       }
     ).join(',');
-    const url = `${this.streamingUrl}?channels=${channelsQueryParam}&accessToken=${authToken.token}&v=${ABLY_API_VERSION}&heartbeats=true`; // same results using `&heartbeats=false`
+    const url = `${this.settings.urls.streaming}/sse?channels=${channelsQueryParam}&accessToken=${authToken.token}&v=${ABLY_API_VERSION}&heartbeats=true`; // same results using `&heartbeats=false`
+    // use headers in server-side or if getHeaderOverrides is defined
+    const useHeaders = !this.settings.core.key || this.settings.sync.requestOptions?.getHeaderOverrides;
 
     this.connection = new this.eventSource!(
       // For client-side SDKs, SplitSDKClientKey and SplitSDKClientKey metadata is passed as query params,
       // because native EventSource implementations for browser doesn't support headers.
-      this.useHeaders ? url : url + `&SplitSDKVersion=${this.headers.SplitSDKVersion}&SplitSDKClientKey=${this.headers.SplitSDKClientKey}`,
+      useHeaders ? url : url + `&SplitSDKVersion=${this.headers.SplitSDKVersion}&SplitSDKClientKey=${this.headers.SplitSDKClientKey}`,
       // For server-side SDKs, metadata is passed via headers. EventSource must support headers, like 'eventsource' package for Node.
-      objectAssign(this.useHeaders ? { headers: this.headers } : {}, this.options)
+      objectAssign(useHeaders ? { headers: decorateHeaders(this.settings, this.headers) } : {}, this.options)
     );
 
     if (this.handler) { // no need to check if SSEClient is used only by PushManager

src/sync/streaming/SSEClient/index.ts

@@ -42,7 +42,7 @@ export function pushManagerFactory(
   let sseClient: ISSEClient;
   try {
     // `useHeaders` false for client-side, even if the platform EventSource supports headers (e.g., React Native).
-    sseClient = new SSEClient(settings, userKey ? false : true, platform);
+    sseClient = new SSEClient(settings, platform);
   } catch (e) {
     log.warn(STREAMING_FALLBACK, [e]);
     return;

src/sync/streaming/pushManager.ts

@@ -119,7 +119,10 @@ export interface ISettings {
     __splitFiltersValidation: ISplitFiltersValidation,
     localhostMode?: SplitIO.LocalhostFactory,
     enabled: boolean,
-    flagSpecVersion: string
+    flagSpecVersion: string,
+    requestOptions?: {
+      getHeaderOverrides?: (context: { headers: Record<string, string> }) => Record<string, string>
+    }
   },
   readonly runtime: {
     ip: string | false
@@ -218,7 +221,10 @@ interface ISharedSettings {
      * Enables synchronization.
      * @property {boolean} enabled
      */
-    enabled: boolean
+    enabled?: boolean,
+    requestOptions?: {
+      getHeaderOverrides?: (context: { headers: Record<string, string> }) => Record<string, string>
+    },
   }
 }
 /**