Validate and sanitize key and trafficType in settings

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@splitsoftware/splitio-commons",
-  "version": "1.2.1-rc.8",
+  "version": "1.2.1-rc.11",
   "description": "Split Javascript SDK common components",
   "main": "cjs/index.js",
   "module": "esm/index.js",

src/logger/constants.ts

@@ -70,6 +70,7 @@ export const EVENTS_TRACKER_SUCCESS = 120;
 export const IMPRESSIONS_TRACKER_SUCCESS = 121;
 export const USER_CONSENT_UPDATED = 122;
 export const USER_CONSENT_NOT_UPDATED = 123;
+export const USER_CONSENT_INITIAL = 124;
 
 export const ENGINE_VALUE_INVALID = 200;
 export const ENGINE_VALUE_NO_ATTRIBUTES = 201;

src/logger/messages/info.ts

@@ -16,6 +16,7 @@ export const codesInfo: [number, string][] = codesWarn.concat([
   [c.IMPRESSIONS_TRACKER_SUCCESS, c.LOG_PREFIX_IMPRESSIONS_TRACKER + 'Successfully stored %s impression(s).'],
   [c.USER_CONSENT_UPDATED, 'setUserConsent: consent status changed from %s to %s.'],
   [c.USER_CONSENT_NOT_UPDATED, 'setUserConsent: call had no effect because it was the current consent status (%s).'],
+  [c.USER_CONSENT_INITIAL, 'Starting the SDK with %s user consent. No data will be sent.'],
 
   // synchronizer
   [c.POLLING_SMART_PAUSING, c.LOG_PREFIX_SYNC_POLLING + 'Turning segments data polling %s.'],

src/sdkClient/__tests__/sdkClientMethodCS.spec.ts

@@ -2,10 +2,6 @@ import { sdkClientMethodCSFactory as sdkClientMethodCSWithTTFactory } from '../s
 import { sdkClientMethodCSFactory } from '../sdkClientMethodCS';
 import { assertClientApi } from './testUtils';
 
-/** Mocks */
-import * as clientCS from '../clientCS';
-const clientCSDecoratorSpy = jest.spyOn(clientCS, 'clientCSDecorator');
-
 import { settingsWithKey, settingsWithKeyAndTT, settingsWithKeyObject } from '../../utils/settingsValidation/__tests__/settings.mocks';
 
 const partialStorages: { destroy: jest.Mock }[] = [];
@@ -220,31 +216,6 @@ describe('sdkClientMethodCSFactory', () => {
     if (!ignoresTT) expect(() => sdkClientMethod('valid-key', ['invalid-TT'])).toThrow('Shared Client needs a valid traffic type or no traffic type at all.');
   });
 
-  test.each(testTargets)('invalid key/TT binds a false key/TT in the default client', (sdkClientMethodCSFactory, ignoresTT) => {
-    const paramsWithInvalidKeyAndTT = {
-      ...params,
-      settings: {
-        ...params.settings,
-        core: {
-          key: true, // invalid key
-          trafficType: '' // invalid TT
-        }
-      }
-    };
-
-    (clientCSDecoratorSpy as jest.Mock).mockClear();
-    // @ts-expect-error
-    const sdkClientMethod = sdkClientMethodCSFactory(paramsWithInvalidKeyAndTT);
-
-    // calling the function should return a client instance
-    const client = sdkClientMethod();
-    assertClientApi(client, params.sdkReadinessManager.sdkStatus);
-
-    // but with false as binded key and TT
-    if (ignoresTT) expect(clientCSDecoratorSpy).toHaveBeenCalledWith(expect.anything(), expect.anything(), false);
-    else expect(clientCSDecoratorSpy).toHaveBeenCalledWith(expect.anything(), expect.anything(), false, false);
-  });
-
   test.each(testTargets)('attributes binding - main client', (sdkClientMethodCSFactory) => {
     // @ts-expect-error
     const sdkClientMethod = sdkClientMethodCSFactory(params);

src/sdkClient/sdkClientMethodCS.ts

@@ -23,15 +23,10 @@ const method = 'Client instantiation';
 export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key?: SplitIO.SplitKey) => SplitIO.ICsClient {
   const { storage, syncManager, sdkReadinessManager, settings: { core: { key }, startup: { readyTimeout }, log } } = params;
 
-  // Keeping similar behaviour as in the isomorphic JS SDK: if settings key is invalid,
-  // `false` value is used as binded key of the default client, but trafficType is ignored
-  // @TODO handle as a non-recoverable error
-  const validKey = validateKey(log, key, method);
-
   const mainClientInstance = clientCSDecorator(
     log,
     sdkClientFactory(params) as SplitIO.IClient, // @ts-ignore
-    validKey
+    key
   );
 
   const parsedDefaultKey = keyParser(key);

src/sdkClient/sdkClientMethodCSWithTT.ts

@@ -25,20 +25,11 @@ const method = 'Client instantiation';
 export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key?: SplitIO.SplitKey, trafficType?: string) => SplitIO.ICsClient {
   const { storage, syncManager, sdkReadinessManager, settings: { core: { key, trafficType }, startup: { readyTimeout }, log } } = params;
 
-  // Keeping the behaviour as in the isomorphic JS SDK: if settings key or TT are invalid,
-  // `false` value is used as binded key/TT of the default client, which leads to several issues.
-  // @TODO update when supporting non-recoverable errors
-  const validKey = validateKey(log, key, method);
-  let validTrafficType;
-  if (trafficType !== undefined) {
-    validTrafficType = validateTrafficType(log, trafficType, method);
-  }
-
   const mainClientInstance = clientCSDecorator(
     log,
     sdkClientFactory(params) as SplitIO.IClient, // @ts-ignore
-    validKey,
-    validTrafficType
+    key,
+    trafficType
   );
 
   const parsedDefaultKey = keyParser(key);

src/sdkFactory/userConsentProps.ts

@@ -1,6 +1,7 @@
-import { ERROR_NOT_BOOLEAN, USER_CONSENT_UPDATED, USER_CONSENT_NOT_UPDATED } from '../logger/constants';
+import { ERROR_NOT_BOOLEAN, USER_CONSENT_UPDATED, USER_CONSENT_NOT_UPDATED, USER_CONSENT_INITIAL } from '../logger/constants';
 import { ISyncManager } from '../sync/types';
 import { ISettings } from '../types';
+import { isConsentGranted } from '../utils/consent';
 import { CONSENT_GRANTED, CONSENT_DECLINED } from '../utils/constants';
 import { isBoolean } from '../utils/lang';
 
@@ -9,6 +10,8 @@ export function userConsentProps(settings: ISettings, syncManager?: ISyncManager
 
   const log = settings.log;
 
+  if (!isConsentGranted(settings)) log.info(USER_CONSENT_INITIAL, [settings.userConsent]);
+
   return {
     setUserConsent(consent: unknown) {
       // validate input param
@@ -19,13 +22,12 @@ export function userConsentProps(settings: ISettings, syncManager?: ISyncManager
 
       const newConsentStatus = consent ? CONSENT_GRANTED : CONSENT_DECLINED;
 
-      if (settings.userConsent !== newConsentStatus) { // @ts-ignore, modify readonly prop
+      if (settings.userConsent !== newConsentStatus) {
+        log.info(USER_CONSENT_UPDATED, [settings.userConsent, newConsentStatus]); // @ts-ignore, modify readonly prop
         settings.userConsent = newConsentStatus;
 
         if (consent) syncManager?.submitter?.start(); // resumes submitters if transitioning to GRANTED
         else syncManager?.submitter?.stop(); // pauses submitters if transitioning to DECLINED
-
-        log.info(USER_CONSENT_UPDATED, [settings.userConsent, newConsentStatus]);
       } else {
         log.info(USER_CONSENT_NOT_UPDATED, [newConsentStatus]);
       }

src/utils/settingsValidation/__tests__/index.spec.ts

@@ -189,6 +189,56 @@ describe('settingsValidation', () => {
     expect(settings.integrations).toBe(integrationsValidatorResult);
     expect(integrationsValidatorMock).toBeCalledWith(settings);
   });
+
+  test('validates and sanitizes key and traffic type in client-side', () => {
+    const clientSideValidationParams = { ...minimalSettingsParams, acceptKey: true, acceptTT: true };
+
+    const samples = [{
+      key: '  valid-key  ', settingsKey: 'valid-key', // key string is trimmed
+      trafficType: 'VALID-TT', settingsTrafficType: 'valid-tt', // TT is converted to lowercase
+    }, {
+      key: undefined, settingsKey: false, // undefined key is not valid in client-side
+      trafficType: undefined, settingsTrafficType: undefined,
+    }, {
+      key: null, settingsKey: false,
+      trafficType: null, settingsTrafficType: false,
+    }, {
+      key: true, settingsKey: false,
+      trafficType: true, settingsTrafficType: false,
+    }, {
+      key: 1.5, settingsKey: '1.5', // finite number as key is parsed
+      trafficType: 100, settingsTrafficType: false,
+    }, {
+      key: { matchingKey: 100, bucketingKey: ' BUCK ' }, settingsKey: { matchingKey: '100', bucketingKey: 'BUCK' },
+      trafficType: {}, settingsTrafficType: false,
+    }];
+
+    samples.forEach(({ key, trafficType, settingsKey, settingsTrafficType }) => {
+      const settings = settingsValidation({
+        core: {
+          authorizationKey: 'dummy token',
+          key,
+          trafficType
+        }
+      }, clientSideValidationParams);
+
+      expect(settings.core.key).toEqual(settingsKey);
+      expect(settings.core.trafficType).toEqual(settingsTrafficType);
+    });
+  });
+
+  test('validates and sanitizes key, while traffic type is ignored', () => {
+    const settings = settingsValidation({
+      core: {
+        authorizationKey: 'dummy token',
+        key: true,
+        trafficType: true
+      }
+    }, { ...minimalSettingsParams, acceptKey: true });
+
+    expect(settings.core.key).toEqual(false); // key is validated
+    expect(settings.core.trafficType).toEqual(true); // traffic type is ignored
+  });
 });
 
 test('SETTINGS / urls should be correctly assigned', () => {

src/utils/settingsValidation/index.ts

@@ -5,6 +5,8 @@ import { STANDALONE_MODE, OPTIMIZED, LOCALHOST_MODE } from '../constants';
 import { validImpressionsMode } from './impressionsMode';
 import { ISettingsValidationParams } from './types';
 import { ISettings } from '../../types';
+import { validateKey } from '../inputValidation/key';
+import { validateTrafficType } from '../inputValidation/trafficType';
 
 const base = {
   // Define which kind of object you want to retrieve from SplitFactory
@@ -129,9 +131,25 @@ export function settingsValidation(config: unknown, validationParams: ISettingsV
   // @ts-ignore, modify readonly prop
   if (storage) withDefaults.storage = storage(withDefaults);
 
-  // Although `key` is mandatory according to TS declaration files, it can be omitted in LOCALHOST mode. In that case, the value `localhost_key` is used.
-  if (withDefaults.mode === LOCALHOST_MODE && withDefaults.core.key === undefined) {
-    withDefaults.core.key = 'localhost_key';
+  // Validate key and TT (for client-side)
+  if (validationParams.acceptKey) {
+    const maybeKey = withDefaults.core.key;
+    // Although `key` is required in client-side, it can be omitted in LOCALHOST mode. In that case, the value `localhost_key` is used.
+    if (withDefaults.mode === LOCALHOST_MODE && maybeKey === undefined) {
+      withDefaults.core.key = 'localhost_key';
+    } else {
+      // Keeping same behaviour than JS SDK: if settings key or TT are invalid,
+      // `false` value is used as binded key/TT of the default client, which leads to some issues.
+      // @ts-ignore, @TODO handle invalid keys as a non-recoverable error?
+      withDefaults.core.key = validateKey(log, maybeKey, 'Client instantiation');
+    }
+
+    if (validationParams.acceptTT) {
+      const maybeTT = withDefaults.core.trafficType;
+      if (maybeTT !== undefined) { // @ts-ignore
+        withDefaults.core.trafficType = validateTrafficType(log, maybeTT, 'Client instantiation');
+      }
+    }
   }
 
   // Current ip/hostname information

src/utils/settingsValidation/types.ts

@@ -10,7 +10,11 @@ export interface ISettingsValidationParams {
    * Version and startup properties are required, because they are not defined in the base settings.
    */
   defaults: Partial<ISettings> & { version: string } & { startup: ISettings['startup'] },
-  /** Function to define runtime values (`settings.runtime`) */
+  /** If true, validates core.key */
+  acceptKey?: boolean,
+  /** If true, validates core.trafficType */
+  acceptTT?: boolean,
+  /** Define runtime values (`settings.runtime`) */
   runtime: (settings: ISettings) => ISettings['runtime'],
   /** Storage validator (`settings.storage`) */
   storage?: (settings: ISettings) => ISettings['storage'],