Skip to content

feature(API): Adds input validation in all API methods and validates empty user Id. #170

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Nov 27, 2018
Merged

feature(API): Adds input validation in all API methods and validates empty user Id. #170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5f0e50a
Adds input validation in all API methods and validates empty user Id.
mfahadahmed Sep 28, 2018
d2e58f8
Merge remote-tracking branch 'origin/master' into fahad/input-validation
mfahadahmed Nov 15, 2018
e84c089
Remove user Id validation from project_config.
mfahadahmed Nov 15, 2018
376de1a
Add unit test for empty experiment key.
mfahadahmed Nov 15, 2018
8a8ac55
Addressed code review comments.
mfahadahmed Nov 16, 2018
8f6d793
Merge branch 'master' into fahad/input-validation
mikeproeng37 Nov 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions packages/optimizely-sdk/lib/core/project_config/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,10 +338,6 @@ module.exports = {
* @throws If the user id is not valid
*/
setInForcedVariationMap: function(projectConfig, userId, experimentId, variationId, logger) {
if (!userId) {
throw new Error(sprintf(ERROR_MESSAGES.INVALID_USER_ID, MODULE_NAME));
}

if (projectConfig.forcedVariationMap.hasOwnProperty(userId)) {
projectConfig.forcedVariationMap[userId][experimentId] = variationId;
} else {
Expand Down
16 changes: 0 additions & 16 deletions packages/optimizely-sdk/lib/core/project_config/index.tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,22 +726,6 @@ describe('lib/core/project_config', function() {
assert.strictEqual(variation2, 'controlLaunched');
});

it('should return false for a null userId', function() {
var testData = testDatafile.getTestProjectConfig();
var configObj = projectConfig.createProjectConfig(testData);

var didSetVariation = projectConfig.setForcedVariation(configObj, 'testExperiment', null, 'control', createdLogger);
assert.strictEqual(didSetVariation, false);
});

it('should return false for an undefined userId', function() {
var testData = testDatafile.getTestProjectConfig();
var configObj = projectConfig.createProjectConfig(testData);

var didSetVariation = projectConfig.setForcedVariation(configObj, 'testExperiment', undefined, 'control', createdLogger);
assert.strictEqual(didSetVariation, false);
});

it('should return false for an empty variation key', function() {
var testData = testDatafile.getTestProjectConfig();
var configObj = projectConfig.createProjectConfig(testData);
Expand Down
24 changes: 23 additions & 1 deletion packages/optimizely-sdk/lib/optimizely/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,10 @@ Optimizely.prototype.getVariation = function(experimentKey, userId, attributes)
* @return boolean A boolean value that indicates if the set completed successfully.
*/
Optimizely.prototype.setForcedVariation = function(experimentKey, userId, variationKey) {
if (!this.__validateInputs({ experiment_key: experimentKey, user_id: userId })) {
return false;
}

try {
return projectConfig.setForcedVariation(this.configObj, experimentKey, userId, variationKey, this.logger);
} catch (ex) {
Expand All @@ -349,6 +353,10 @@ Optimizely.prototype.setForcedVariation = function(experimentKey, userId, variat
* @return {string|null} The forced variation key.
*/
Optimizely.prototype.getForcedVariation = function(experimentKey, userId) {
if (!this.__validateInputs({ experiment_key: experimentKey, user_id: userId })) {
return null;
}

try {
return projectConfig.getForcedVariation(this.configObj, experimentKey, userId, this.logger);
} catch (ex) {
Expand All @@ -359,7 +367,7 @@ Optimizely.prototype.getForcedVariation = function(experimentKey, userId) {
};

/**
* Validates user ID and attributes parameters
* Validate string inputs, user attributes and event tags.
* @param {string} stringInputs Map of string keys and associated values
* @param {Object} userAttributes Optional parameter for user's attributes
* @param {Object} eventTags Optional parameter for event tags
Expand All @@ -368,6 +376,16 @@ Optimizely.prototype.getForcedVariation = function(experimentKey, userId) {
*/
Optimizely.prototype.__validateInputs = function(stringInputs, userAttributes, eventTags) {
try {
// Null, undefined or non-string user Id is invalid.
if (stringInputs.hasOwnProperty('user_id')) {
var userId = stringInputs.user_id;
if (typeof userId !== 'string' || userId === null || userId === 'undefined') {
throw new Error(sprintf(ERROR_MESSAGES.INVALID_INPUT_FORMAT, MODULE_NAME, 'user_id'));
}

delete stringInputs.user_id;
}

var inputKeys = Object.keys(stringInputs);
for (var index = 0; index < inputKeys.length; index++) {
var key = inputKeys[index];
Expand Down Expand Up @@ -536,6 +554,10 @@ Optimizely.prototype.getEnabledFeatures = function (userId, attributes) {
return enabledFeatures;
}

if (!this.__validateInputs({ user_id: userId })) {
return enabledFeatures;
}

fns.forOwn(this.configObj.featureKeyMap, function (feature) {
if (this.isFeatureEnabled(feature.key, userId, attributes)) {
enabledFeatures.push(feature.key);
Expand Down
50 changes: 35 additions & 15 deletions packages/optimizely-sdk/lib/optimizely/index.tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1762,31 +1762,31 @@ describe('lib/optimizely', function() {
assert.strictEqual(forcedVariation, null);

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

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

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

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

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

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

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

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

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

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

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

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

var setVariationLogMessage = createdLogger.log.args[0][1];
assert.strictEqual(setVariationLogMessage, sprintf(ERROR_MESSAGES.INVALID_INPUT_FORMAT, 'OPTIMIZELY', 'experiment_key'));
});

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

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

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

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

it('should return true for an empty userId', function() {
var didSetVariation = optlyInstance.setForcedVariation('testExperiment', '', 'control');
assert.strictEqual(didSetVariation, true);
});

it('should return false for a null variationKey', function() {
Expand Down Expand Up @@ -1949,6 +1962,7 @@ describe('lib/optimizely', function() {
describe('__validateInputs', function() {
it('should return true if user ID and attributes are valid', function() {
assert.isTrue(optlyInstance.__validateInputs({user_id: 'testUser'}));
assert.isTrue(optlyInstance.__validateInputs({user_id: ''}));
assert.isTrue(optlyInstance.__validateInputs({user_id: 'testUser'}, {browser_type: 'firefox'}));
sinon.assert.notCalled(createdLogger.log);
});
Expand All @@ -1957,11 +1971,17 @@ describe('lib/optimizely', function() {
var falseUserIdInput = optlyInstance.__validateInputs({user_id: []});
assert.isFalse(falseUserIdInput);

sinon.assert.calledOnce(errorHandler.handleError);
falseUserIdInput = optlyInstance.__validateInputs({user_id: null});
assert.isFalse(falseUserIdInput);

falseUserIdInput = optlyInstance.__validateInputs({user_id: 3.14});
assert.isFalse(falseUserIdInput);

sinon.assert.calledThrice(errorHandler.handleError);
var errorMessage = errorHandler.handleError.lastCall.args[0].message;
assert.strictEqual(errorMessage, sprintf(ERROR_MESSAGES.INVALID_INPUT_FORMAT, 'OPTIMIZELY', 'user_id'));

sinon.assert.calledOnce(createdLogger.log);
sinon.assert.calledThrice(createdLogger.log);
var logMessage = createdLogger.log.args[0][1];
assert.strictEqual(logMessage, sprintf(ERROR_MESSAGES.INVALID_INPUT_FORMAT, 'OPTIMIZELY', 'user_id'));
});
Expand Down Expand Up @@ -2698,7 +2718,7 @@ describe('lib/optimizely', function() {
var result = optlyInstance.isFeatureEnabled(null, null, attributes);
assert.strictEqual(result, false);
sinon.assert.notCalled(eventDispatcher.dispatchEvent);
sinon.assert.calledWithExactly(createdLogger.log, LOG_LEVEL.ERROR, 'OPTIMIZELY: Provided feature_key is in an invalid format.');
sinon.assert.calledWithExactly(createdLogger.log, LOG_LEVEL.ERROR, 'OPTIMIZELY: Provided user_id is in an invalid format.');
});

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

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

it('returns false when user id is an empty string', function() {
it('returns true when user id is an empty string', function() {
var result = optlyInstance.isFeatureEnabled('test_feature_for_experiment', '', attributes);
assert.strictEqual(result, false);
sinon.assert.notCalled(eventDispatcher.dispatchEvent);
assert.strictEqual(result, true);
sinon.assert.calledOnce(eventDispatcher.dispatchEvent);
});

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