diff --git a/package-lock.json b/package-lock.json index 0a014819..ebe428e0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.2.1-rc.8", + "version": "1.2.1-rc.11", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -4984,9 +4984,9 @@ } }, "minimist": { - "version": "1.2.5", - "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.5.tgz", - "integrity": "sha512-FM9nNUYrRBAELZQT3xeZQ7fmMOBg6nWNmJKTcgsJeaLstP/UODVpGsr5OhXhhXg6f+qtJ8uiZ+PUxkDWcgIXLw==", + "version": "1.2.6", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.6.tgz", + "integrity": "sha512-Jsjnk4bw3YJqYzbdyBiNsPWHPfO++UGG749Cxs6peCu5Xg4nrena6OVxOYxrQTqww0Jmwt+Ref8rggumkTLz9Q==", "dev": true }, "ms": { diff --git a/package.json b/package.json index 63b45dc7..8338e84a 100644 --- a/package.json +++ b/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", diff --git a/src/logger/constants.ts b/src/logger/constants.ts index 68259646..10b45254 100644 --- a/src/logger/constants.ts +++ b/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; diff --git a/src/logger/messages/info.ts b/src/logger/messages/info.ts index 9f4a4254..96703540 100644 --- a/src/logger/messages/info.ts +++ b/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.'], diff --git a/src/sdkClient/__tests__/sdkClientMethodCS.spec.ts b/src/sdkClient/__tests__/sdkClientMethodCS.spec.ts index b5b6b21e..7eabeb7c 100644 --- a/src/sdkClient/__tests__/sdkClientMethodCS.spec.ts +++ b/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); diff --git a/src/sdkClient/sdkClientMethodCS.ts b/src/sdkClient/sdkClientMethodCS.ts index 2abd7a20..32f6f3fb 100644 --- a/src/sdkClient/sdkClientMethodCS.ts +++ b/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); diff --git a/src/sdkClient/sdkClientMethodCSWithTT.ts b/src/sdkClient/sdkClientMethodCSWithTT.ts index 7c5427e2..22a49ac6 100644 --- a/src/sdkClient/sdkClientMethodCSWithTT.ts +++ b/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); diff --git a/src/sdkFactory/userConsentProps.ts b/src/sdkFactory/userConsentProps.ts index da715c33..b275406c 100644 --- a/src/sdkFactory/userConsentProps.ts +++ b/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]); } diff --git a/src/utils/settingsValidation/__tests__/index.spec.ts b/src/utils/settingsValidation/__tests__/index.spec.ts index 50b960a9..3f799970 100644 --- a/src/utils/settingsValidation/__tests__/index.spec.ts +++ b/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', () => { diff --git a/src/utils/settingsValidation/index.ts b/src/utils/settingsValidation/index.ts index d7a31c10..5bb1125b 100644 --- a/src/utils/settingsValidation/index.ts +++ b/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 diff --git a/src/utils/settingsValidation/types.ts b/src/utils/settingsValidation/types.ts index 40cd155c..2322d042 100644 --- a/src/utils/settingsValidation/types.ts +++ b/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 & { 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'],