diff --git a/package.json b/package.json index 38d6cc334a..64aa272535 100644 --- a/package.json +++ b/package.json @@ -152,6 +152,7 @@ "lint-staged": "12.3.4", "prettier": "2.5.1", "proxyquire": "2.1.3", + "semver": "^7.3.5", "source-map-support": "0.5.21", "superagent": "7.1.1", "supertest": "6.2.2", diff --git a/src/lib/routes/admin-api/project/features.ts b/src/lib/routes/admin-api/project/features.ts index afcdf4d2a7..0a25d4f31d 100644 --- a/src/lib/routes/admin-api/project/features.ts +++ b/src/lib/routes/admin-api/project/features.ts @@ -10,6 +10,7 @@ import { CREATE_FEATURE_STRATEGY, DELETE_FEATURE, DELETE_FEATURE_STRATEGY, + NONE, UPDATE_FEATURE, UPDATE_FEATURE_ENVIRONMENT, UPDATE_FEATURE_STRATEGY, @@ -92,6 +93,11 @@ export default class ProjectFeaturesController extends Controller { this.addStrategy, CREATE_FEATURE_STRATEGY, ); + this.post( + `${PATH_STRATEGIES}/validate-constraint`, + this.validateConstraint, + NONE, + ); this.get(`${PATH_STRATEGY}`, this.getStrategy); this.put( `${PATH_STRATEGY}`, diff --git a/src/lib/schema/constraint-value-types.test.ts b/src/lib/schema/constraint-value-types.test.ts index 7da57bd5d9..906c25f698 100644 --- a/src/lib/schema/constraint-value-types.test.ts +++ b/src/lib/schema/constraint-value-types.test.ts @@ -1,4 +1,5 @@ import { + constraintDateTypeSchema, constraintNumberTypeSchema, constraintStringTypeSchema, } from './constraint-value-types'; @@ -30,6 +31,7 @@ test('should allow negative numbers', async () => { /* String types */ test('should require a list of strings', async () => { + expect.assertions(1); try { await constraintStringTypeSchema.validateAsync(['test', 1]); } catch (error) { @@ -38,9 +40,38 @@ test('should require a list of strings', async () => { }); test('should succeed with a list of strings', async () => { + expect.assertions(0); await constraintStringTypeSchema.validateAsync([ 'test', 'another-test', 'supervalue', ]); }); + +/* Date type */ + +test('should fail an invalid date', async () => { + expect.assertions(1); + + const invalidDate = 'Tuesday the awesome day'; + try { + await constraintDateTypeSchema.validateAsync(invalidDate); + } catch (error) { + expect(error.details[0].message).toEqual( + '"value" must be a valid date', + ); + } +}); + +test('Should pass a valid date', async () => { + expect.assertions(0); + + const invalidDate = '2022-01-29T13:00:00.000Z'; + try { + await constraintDateTypeSchema.validateAsync(invalidDate); + } catch (error) { + expect(error.details[0].message).toEqual( + '"value" must be a valid date', + ); + } +}); diff --git a/src/lib/schema/constraint-value-types.ts b/src/lib/schema/constraint-value-types.ts index 506c78f587..5861395169 100644 --- a/src/lib/schema/constraint-value-types.ts +++ b/src/lib/schema/constraint-value-types.ts @@ -3,3 +3,5 @@ import joi from 'joi'; export const constraintNumberTypeSchema = joi.number(); export const constraintStringTypeSchema = joi.array().items(joi.string()); + +export const constraintDateTypeSchema = joi.date(); diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts index 0c52278f4a..62a9d94ef7 100644 --- a/src/lib/services/feature-toggle-service.ts +++ b/src/lib/services/feature-toggle-service.ts @@ -61,7 +61,14 @@ import { } from '../util/constants'; import { applyPatch, deepClone, Operation } from 'fast-json-patch'; import { OperationDeniedError } from '../error/operation-denied-error'; -import { validateNumber } from 'lib/util/validators/constraint-types'; +import { + validateDate, + validateLegalValues, + validateNumber, + validateSemver, + validateString, +} from 'lib/util/validators/constraint-types'; +import { IContextFieldStore } from 'lib/types/stores/context-field-store'; interface IFeatureContext { featureName: string; @@ -93,6 +100,8 @@ class FeatureToggleService { private eventStore: IEventStore; + private contextFieldStore: IContextFieldStore; + constructor( { featureStrategiesStore, @@ -102,6 +111,7 @@ class FeatureToggleService { eventStore, featureTagStore, featureEnvironmentStore, + contextFieldStore, }: Pick< IUnleashStores, | 'featureStrategiesStore' @@ -111,6 +121,7 @@ class FeatureToggleService { | 'eventStore' | 'featureTagStore' | 'featureEnvironmentStore' + | 'contextFieldStore' >, { getLogger }: Pick, ) { @@ -122,6 +133,7 @@ class FeatureToggleService { this.projectStore = projectStore; this.eventStore = eventStore; this.featureEnvironmentStore = featureEnvironmentStore; + this.contextFieldStore = contextFieldStore; } async validateFeatureContext({ @@ -156,30 +168,48 @@ class FeatureToggleService { async validateConstraint(constraint: IConstraint): Promise { const { operator } = constraint; await constraintSchema.validateAsync(constraint); + const contextDefinition = await this.contextFieldStore.get( + constraint.contextName, + ); if (oneOf(NUM_OPERATORS, operator)) { - // Validate number value await validateNumber(constraint.value); - // 1. Retrieve context defintion based on constraint contextName - // 2. Check if value is a valid number / value type - // 3. Check the value against predefined legalValues if specified on the - // context definition } if (oneOf(STRING_OPERATORS, operator)) { - // validate string values array await validateString(constraint.values); } - // if (oneOf(SEMVER_OPERATORS, operator)) { - // // validate semver - // validateSemver(constraint.value) - // } + if (oneOf(SEMVER_OPERATORS, operator)) { + // Semver library is not asynchronous, so we do not + // need to await here. + validateSemver(constraint.value); + } - // if (oneOf(DATE_OPERATORS, operator)) { - // // validate dates - // validateDate(constraint.value); - // } + if (oneOf(DATE_OPERATORS, operator)) { + validateDate(constraint.value); + } + + if ( + oneOf( + [...DATE_OPERATORS, ...SEMVER_OPERATORS, ...NUM_OPERATORS], + operator, + ) + ) { + if (contextDefinition?.legalValues?.length > 0) { + validateLegalValues( + contextDefinition.legalValues, + constraint.value, + ); + } + } else { + if (contextDefinition?.legalValues?.length > 0) { + validateLegalValues( + contextDefinition.legalValues, + constraint.values, + ); + } + } } async patchFeature( diff --git a/src/lib/util/validators/constraint-types.test.ts b/src/lib/util/validators/constraint-types.test.ts new file mode 100644 index 0000000000..74945e27b2 --- /dev/null +++ b/src/lib/util/validators/constraint-types.test.ts @@ -0,0 +1,97 @@ +import { validateSemver, validateLegalValues } from './constraint-types'; + +test('semver validation should throw with bad format', () => { + const badSemver = 'a.b.c'; + expect.assertions(1); + + try { + validateSemver(badSemver); + } catch (e) { + expect(e.message).toBe( + `the provided value is not a valid semver format. The value provided was: ${badSemver}`, + ); + } +}); + +test('semver valdiation should pass with correct format', () => { + const validSemver = '1.2.3'; + expect.assertions(0); + + try { + validateSemver(validSemver); + } catch (e) { + expect(e.message).toBe( + `the provided value is not a valid semver format. The value provided was: ${validSemver}`, + ); + } +}); + +test('semver validation should fail partial semver', () => { + const partial = '1.2'; + expect.assertions(1); + + try { + validateSemver(partial); + } catch (e) { + expect(e.message).toBe( + `the provided value is not a valid semver format. The value provided was: ${partial}`, + ); + } +}); + +/* Legal values tests */ +test('should fail validation if value does not exist in single legal value', () => { + const legalValues = ['100', '200', '300']; + const value = '500'; + expect.assertions(1); + + try { + validateLegalValues(legalValues, value); + } catch (error) { + expect(error.message).toBe( + `${value} is not specified as a legal value on this context field`, + ); + } +}); + +test('should pass validation if value exists in single legal value', () => { + const legalValues = ['100', '200', '300']; + const value = '100'; + expect.assertions(0); + + try { + validateLegalValues(legalValues, value); + } catch (error) { + expect(error.message).toBe( + `${value} is not specified as a legal value on this context field`, + ); + } +}); + +test('should fail validation if one of the values does not exist in multiple legal values', () => { + const legalValues = ['100', '200', '300']; + const values = ['500', '100']; + expect.assertions(1); + + try { + validateLegalValues(legalValues, values); + } catch (error) { + expect(error.message).toBe( + `input values are not specified as a legal value on this context field`, + ); + } +}); + +test('should pass validation if all of the values exists in legal values', () => { + const legalValues = ['100', '200', '300']; + const values = ['200', '100']; + expect.assertions(0); + + try { + validateLegalValues(legalValues, values); + } catch (error) { + expect(error.message).toBe( + `input values are not specified as a legal value on this context field`, + ); + } +}); diff --git a/src/lib/util/validators/constraint-types.ts b/src/lib/util/validators/constraint-types.ts index 0d024715cc..6cbf56e704 100644 --- a/src/lib/util/validators/constraint-types.ts +++ b/src/lib/util/validators/constraint-types.ts @@ -1,7 +1,11 @@ +import semver from 'semver'; + import { + constraintDateTypeSchema, constraintNumberTypeSchema, constraintStringTypeSchema, -} from 'lib/schema/constraint-value-types'; +} from '../../schema/constraint-value-types'; +import BadDataError from '../../error/bad-data-error'; export const validateNumber = async (value: unknown): Promise => { await constraintNumberTypeSchema.validateAsync(value); @@ -10,3 +14,36 @@ export const validateNumber = async (value: unknown): Promise => { export const validateString = async (value: unknown): Promise => { await constraintStringTypeSchema.validateAsync(value); }; + +export const validateSemver = (value: unknown): void => { + const result = semver.valid(value); + + if (result) return; + throw new BadDataError( + `the provided value is not a valid semver format. The value provided was: ${value}`, + ); +}; + +export const validateDate = async (value: unknown): Promise => { + await constraintDateTypeSchema.validateAsync(value); +}; + +export const validateLegalValues = ( + legalValues: string[], + match: string[] | string, +): void => { + if (Array.isArray(match)) { + // Compare arrays to arrays + const valid = match.every((value) => legalValues.includes(value)); + if (!valid) + throw new BadDataError( + `input values are not specified as a legal value on this context field`, + ); + } else { + const valid = legalValues.includes(match); + if (!valid) + throw new BadDataError( + `${match} is not specified as a legal value on this context field`, + ); + } +};