Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Commit 45f6794

Browse files
mfahadahmedmikeproeng37
mfahadahmed
authored and
mikeproeng37
committed
feature(API): Adds input validation in all API methods and validates empty user Id. (optimizely#170)
1 parent 249e64f commit 45f6794

File tree

4 files changed

+58
-36
lines changed

4 files changed

+58
-36
lines changed

‎packages/optimizely-sdk/lib/core/project_config/index.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -338,10 +338,6 @@ module.exports = {
338338
* @throws If the user id is not valid
339339
*/
340340
setInForcedVariationMap: function(projectConfig, userId, experimentId, variationId, logger) {
341-
if (!userId) {
342-
throw new Error(sprintf(ERROR_MESSAGES.INVALID_USER_ID, MODULE_NAME));
343-
}
344-
345341
if (projectConfig.forcedVariationMap.hasOwnProperty(userId)) {
346342
projectConfig.forcedVariationMap[userId][experimentId] = variationId;
347343
} else {

‎packages/optimizely-sdk/lib/core/project_config/index.tests.js

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -726,22 +726,6 @@ describe('lib/core/project_config', function() {
726726
assert.strictEqual(variation2, 'controlLaunched');
727727
});
728728

729-
it('should return false for a null userId', function() {
730-
var testData = testDatafile.getTestProjectConfig();
731-
var configObj = projectConfig.createProjectConfig(testData);
732-
733-
var didSetVariation = projectConfig.setForcedVariation(configObj, 'testExperiment', null, 'control', createdLogger);
734-
assert.strictEqual(didSetVariation, false);
735-
});
736-
737-
it('should return false for an undefined userId', function() {
738-
var testData = testDatafile.getTestProjectConfig();
739-
var configObj = projectConfig.createProjectConfig(testData);
740-
741-
var didSetVariation = projectConfig.setForcedVariation(configObj, 'testExperiment', undefined, 'control', createdLogger);
742-
assert.strictEqual(didSetVariation, false);
743-
});
744-
745729
it('should return false for an empty variation key', function() {
746730
var testData = testDatafile.getTestProjectConfig();
747731
var configObj = projectConfig.createProjectConfig(testData);

‎packages/optimizely-sdk/lib/optimizely/index.js

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,10 @@ Optimizely.prototype.getVariation = function(experimentKey, userId, attributes)
333333
* @return boolean A boolean value that indicates if the set completed successfully.
334334
*/
335335
Optimizely.prototype.setForcedVariation = function(experimentKey, userId, variationKey) {
336+
if (!this.__validateInputs({ experiment_key: experimentKey, user_id: userId })) {
337+
return false;
338+
}
339+
336340
try {
337341
return projectConfig.setForcedVariation(this.configObj, experimentKey, userId, variationKey, this.logger);
338342
} catch (ex) {
@@ -349,6 +353,10 @@ Optimizely.prototype.setForcedVariation = function(experimentKey, userId, variat
349353
* @return {string|null} The forced variation key.
350354
*/
351355
Optimizely.prototype.getForcedVariation = function(experimentKey, userId) {
356+
if (!this.__validateInputs({ experiment_key: experimentKey, user_id: userId })) {
357+
return null;
358+
}
359+
352360
try {
353361
return projectConfig.getForcedVariation(this.configObj, experimentKey, userId, this.logger);
354362
} catch (ex) {
@@ -359,7 +367,7 @@ Optimizely.prototype.getForcedVariation = function(experimentKey, userId) {
359367
};
360368

361369
/**
362-
* Validates user ID and attributes parameters
370+
* Validate string inputs, user attributes and event tags.
363371
* @param {string} stringInputs Map of string keys and associated values
364372
* @param {Object} userAttributes Optional parameter for user's attributes
365373
* @param {Object} eventTags Optional parameter for event tags
@@ -368,6 +376,16 @@ Optimizely.prototype.getForcedVariation = function(experimentKey, userId) {
368376
*/
369377
Optimizely.prototype.__validateInputs = function(stringInputs, userAttributes, eventTags) {
370378
try {
379+
// Null, undefined or non-string user Id is invalid.
380+
if (stringInputs.hasOwnProperty('user_id')) {
381+
var userId = stringInputs.user_id;
382+
if (typeof userId !== 'string' || userId === null || userId === 'undefined') {
383+
throw new Error(sprintf(ERROR_MESSAGES.INVALID_INPUT_FORMAT, MODULE_NAME, 'user_id'));
384+
}
385+
386+
delete stringInputs.user_id;
387+
}
388+
371389
var inputKeys = Object.keys(stringInputs);
372390
for (var index = 0; index < inputKeys.length; index++) {
373391
var key = inputKeys[index];
@@ -536,6 +554,10 @@ Optimizely.prototype.getEnabledFeatures = function (userId, attributes) {
536554
return enabledFeatures;
537555
}
538556

557+
if (!this.__validateInputs({ user_id: userId })) {
558+
return enabledFeatures;
559+
}
560+
539561
fns.forOwn(this.configObj.featureKeyMap, function (feature) {
540562
if (this.isFeatureEnabled(feature.key, userId, attributes)) {
541563
enabledFeatures.push(feature.key);

‎packages/optimizely-sdk/lib/optimizely/index.tests.js

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1762,31 +1762,31 @@ describe('lib/optimizely', function() {
17621762
assert.strictEqual(forcedVariation, null);
17631763

17641764
var logMessage = createdLogger.log.args[0][1];
1765-
assert.strictEqual(logMessage, sprintf(LOG_MESSAGES.USER_HAS_NO_FORCED_VARIATION, 'PROJECT_CONFIG', 'user1'));
1765+
assert.strictEqual(logMessage, sprintf(ERROR_MESSAGES.INVALID_INPUT_FORMAT, 'OPTIMIZELY', 'experiment_key'));
17661766
});
17671767

17681768
it('should return null with an undefined experimentKey', function() {
17691769
var forcedVariation = optlyInstance.getForcedVariation(undefined, 'user1');
17701770
assert.strictEqual(forcedVariation, null);
17711771

17721772
var logMessage = createdLogger.log.args[0][1];
1773-
assert.strictEqual(logMessage, sprintf(LOG_MESSAGES.USER_HAS_NO_FORCED_VARIATION, 'PROJECT_CONFIG', 'user1'));
1773+
assert.strictEqual(logMessage, sprintf(ERROR_MESSAGES.INVALID_INPUT_FORMAT, 'OPTIMIZELY', 'experiment_key'));
17741774
});
17751775

17761776
it('should return null with a null userId', function() {
17771777
var forcedVariation = optlyInstance.getForcedVariation('testExperiment', null);
17781778
assert.strictEqual(forcedVariation, null);
17791779

17801780
var logMessage = createdLogger.log.args[0][1];
1781-
assert.strictEqual(logMessage, sprintf(LOG_MESSAGES.USER_HAS_NO_FORCED_VARIATION, 'PROJECT_CONFIG', null));
1781+
assert.strictEqual(logMessage, sprintf(ERROR_MESSAGES.INVALID_INPUT_FORMAT, 'OPTIMIZELY', 'user_id'));
17821782
});
17831783

17841784
it('should return null with an undefined userId', function() {
17851785
var forcedVariation = optlyInstance.getForcedVariation('testExperiment', undefined);
17861786
assert.strictEqual(forcedVariation, null);
17871787

17881788
var logMessage = createdLogger.log.args[0][1];
1789-
assert.strictEqual(logMessage, sprintf(LOG_MESSAGES.USER_HAS_NO_FORCED_VARIATION, 'PROJECT_CONFIG', undefined));
1789+
assert.strictEqual(logMessage, sprintf(ERROR_MESSAGES.INVALID_INPUT_FORMAT, 'OPTIMIZELY', 'user_id'));
17901790
});
17911791
});
17921792

@@ -1888,31 +1888,44 @@ describe('lib/optimizely', function() {
18881888
assert.strictEqual(didSetVariation, false);
18891889

18901890
var setVariationLogMessage = createdLogger.log.args[0][1];
1891-
assert.strictEqual(setVariationLogMessage, 'PROJECT_CONFIG: Experiment key null is not in datafile.');
1891+
assert.strictEqual(setVariationLogMessage, sprintf(ERROR_MESSAGES.INVALID_INPUT_FORMAT, 'OPTIMIZELY', 'experiment_key'));
18921892
});
18931893

18941894
it('should return false for an undefined experimentKey', function() {
18951895
var didSetVariation = optlyInstance.setForcedVariation(undefined, 'user1', 'control');
18961896
assert.strictEqual(didSetVariation, false);
18971897

18981898
var setVariationLogMessage = createdLogger.log.args[0][1];
1899-
assert.strictEqual(setVariationLogMessage, 'PROJECT_CONFIG: Experiment key undefined is not in datafile.');
1899+
assert.strictEqual(setVariationLogMessage, sprintf(ERROR_MESSAGES.INVALID_INPUT_FORMAT, 'OPTIMIZELY', 'experiment_key'));
1900+
});
1901+
1902+
it('should return false for an empty experimentKey', function() {
1903+
var didSetVariation = optlyInstance.setForcedVariation('', 'user1', 'control');
1904+
assert.strictEqual(didSetVariation, false);
1905+
1906+
var setVariationLogMessage = createdLogger.log.args[0][1];
1907+
assert.strictEqual(setVariationLogMessage, sprintf(ERROR_MESSAGES.INVALID_INPUT_FORMAT, 'OPTIMIZELY', 'experiment_key'));
19001908
});
19011909

19021910
it('should return false for a null userId', function() {
19031911
var didSetVariation = optlyInstance.setForcedVariation('testExperiment', null, 'control');
19041912
assert.strictEqual(didSetVariation, false);
19051913

19061914
var setVariationLogMessage = createdLogger.log.args[0][1];
1907-
assert.strictEqual(setVariationLogMessage, 'PROJECT_CONFIG: Provided user ID is in an invalid format.');
1915+
assert.strictEqual(setVariationLogMessage, sprintf(ERROR_MESSAGES.INVALID_INPUT_FORMAT, 'OPTIMIZELY', 'user_id'));
19081916
});
19091917

19101918
it('should return false for an undefined userId', function() {
19111919
var didSetVariation = optlyInstance.setForcedVariation('testExperiment', undefined, 'control');
19121920
assert.strictEqual(didSetVariation, false);
19131921

19141922
var setVariationLogMessage = createdLogger.log.args[0][1];
1915-
assert.strictEqual(setVariationLogMessage, 'PROJECT_CONFIG: Provided user ID is in an invalid format.');
1923+
assert.strictEqual(setVariationLogMessage, sprintf(ERROR_MESSAGES.INVALID_INPUT_FORMAT, 'OPTIMIZELY', 'user_id'));
1924+
});
1925+
1926+
it('should return true for an empty userId', function() {
1927+
var didSetVariation = optlyInstance.setForcedVariation('testExperiment', '', 'control');
1928+
assert.strictEqual(didSetVariation, true);
19161929
});
19171930

19181931
it('should return false for a null variationKey', function() {
@@ -1949,6 +1962,7 @@ describe('lib/optimizely', function() {
19491962
describe('__validateInputs', function() {
19501963
it('should return true if user ID and attributes are valid', function() {
19511964
assert.isTrue(optlyInstance.__validateInputs({user_id: 'testUser'}));
1965+
assert.isTrue(optlyInstance.__validateInputs({user_id: ''}));
19521966
assert.isTrue(optlyInstance.__validateInputs({user_id: 'testUser'}, {browser_type: 'firefox'}));
19531967
sinon.assert.notCalled(createdLogger.log);
19541968
});
@@ -1957,11 +1971,17 @@ describe('lib/optimizely', function() {
19571971
var falseUserIdInput = optlyInstance.__validateInputs({user_id: []});
19581972
assert.isFalse(falseUserIdInput);
19591973

1960-
sinon.assert.calledOnce(errorHandler.handleError);
1974+
falseUserIdInput = optlyInstance.__validateInputs({user_id: null});
1975+
assert.isFalse(falseUserIdInput);
1976+
1977+
falseUserIdInput = optlyInstance.__validateInputs({user_id: 3.14});
1978+
assert.isFalse(falseUserIdInput);
1979+
1980+
sinon.assert.calledThrice(errorHandler.handleError);
19611981
var errorMessage = errorHandler.handleError.lastCall.args[0].message;
19621982
assert.strictEqual(errorMessage, sprintf(ERROR_MESSAGES.INVALID_INPUT_FORMAT, 'OPTIMIZELY', 'user_id'));
19631983

1964-
sinon.assert.calledOnce(createdLogger.log);
1984+
sinon.assert.calledThrice(createdLogger.log);
19651985
var logMessage = createdLogger.log.args[0][1];
19661986
assert.strictEqual(logMessage, sprintf(ERROR_MESSAGES.INVALID_INPUT_FORMAT, 'OPTIMIZELY', 'user_id'));
19671987
});
@@ -2698,7 +2718,7 @@ describe('lib/optimizely', function() {
26982718
var result = optlyInstance.isFeatureEnabled(null, null, attributes);
26992719
assert.strictEqual(result, false);
27002720
sinon.assert.notCalled(eventDispatcher.dispatchEvent);
2701-
sinon.assert.calledWithExactly(createdLogger.log, LOG_LEVEL.ERROR, 'OPTIMIZELY: Provided feature_key is in an invalid format.');
2721+
sinon.assert.calledWithExactly(createdLogger.log, LOG_LEVEL.ERROR, 'OPTIMIZELY: Provided user_id is in an invalid format.');
27022722
});
27032723

27042724
it('returns false when feature key is undefined', function() {
@@ -2725,7 +2745,7 @@ describe('lib/optimizely', function() {
27252745
var result = optlyInstance.isFeatureEnabled();
27262746
assert.strictEqual(result, false);
27272747
sinon.assert.notCalled(eventDispatcher.dispatchEvent);
2728-
sinon.assert.calledWithExactly(createdLogger.log, LOG_LEVEL.ERROR, 'OPTIMIZELY: Provided feature_key is in an invalid format.');
2748+
sinon.assert.calledWithExactly(createdLogger.log, LOG_LEVEL.ERROR, 'OPTIMIZELY: Provided user_id is in an invalid format.');
27292749
});
27302750

27312751
it('returns false when user id is an object', function() {
@@ -2749,10 +2769,10 @@ describe('lib/optimizely', function() {
27492769
sinon.assert.calledWithExactly(createdLogger.log, LOG_LEVEL.ERROR, 'OPTIMIZELY: Provided feature_key is in an invalid format.');
27502770
});
27512771

2752-
it('returns false when user id is an empty string', function() {
2772+
it('returns true when user id is an empty string', function() {
27532773
var result = optlyInstance.isFeatureEnabled('test_feature_for_experiment', '', attributes);
2754-
assert.strictEqual(result, false);
2755-
sinon.assert.notCalled(eventDispatcher.dispatchEvent);
2774+
assert.strictEqual(result, true);
2775+
sinon.assert.calledOnce(eventDispatcher.dispatchEvent);
27562776
});
27572777

27582778
it('returns false when feature key is an empty string', function() {

0 commit comments

Comments
 (0)