From 6f075e4d1ca15070bd1e27f444fe7c2892870d08 Mon Sep 17 00:00:00 2001 From: Fredrik Strand Oseberg Date: Fri, 4 Mar 2022 17:29:42 +0100 Subject: [PATCH] Feat/new constraint operators (#1397) * feat: add migration for currentTime context field * feat: add tests for number validator * feat: add validation fields for constraint * feat: add validation for semver, date and legalvalues * fix: import paths * fix: only allow specified operators * fix: add operator test * fix: reset db * fix: remove unused import * fix: set semver as dependency --- package.json | 3 +- src/lib/routes/admin-api/project/features.ts | 15 ++- src/lib/schema/constraint-value-types.test.ts | 77 +++++++++++++++ src/lib/schema/constraint-value-types.ts | 7 ++ src/lib/schema/feature-schema.test.ts | 19 +++- src/lib/schema/feature-schema.ts | 6 +- src/lib/services/feature-toggle-service.ts | 74 +++++++++++++- src/lib/types/model.ts | 5 +- src/lib/util/constants.ts | 47 +++++++++ .../util/validators/constraint-types.test.ts | 97 +++++++++++++++++++ src/lib/util/validators/constraint-types.ts | 49 ++++++++++ ...24111626-add-current-time-context-field.js | 19 ++++ 12 files changed, 412 insertions(+), 6 deletions(-) create mode 100644 src/lib/schema/constraint-value-types.test.ts create mode 100644 src/lib/schema/constraint-value-types.ts create mode 100644 src/lib/util/validators/constraint-types.test.ts create mode 100644 src/lib/util/validators/constraint-types.ts create mode 100644 src/migrations/20220224111626-add-current-time-context-field.js diff --git a/package.json b/package.json index 38d6cc334a..c7a81eb252 100644 --- a/package.json +++ b/package.json @@ -113,7 +113,8 @@ "stoppable": "^1.1.0", "type-is": "^1.6.18", "unleash-frontend": "4.8.0", - "uuid": "^8.3.2" + "uuid": "^8.3.2", + "semver": "^7.3.5" }, "devDependencies": { "@babel/core": "7.17.5", diff --git a/src/lib/routes/admin-api/project/features.ts b/src/lib/routes/admin-api/project/features.ts index a677a1c01e..14f0390b7c 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, @@ -84,7 +85,6 @@ export default class ProjectFeaturesController extends Controller { this.toggleEnvironmentOff, UPDATE_FEATURE_ENVIRONMENT, ); - // activation strategies this.get(`${PATH_STRATEGIES}`, this.getStrategies); this.post( @@ -92,6 +92,7 @@ export default class ProjectFeaturesController extends Controller { this.addStrategy, CREATE_FEATURE_STRATEGY, ); + this.get(`${PATH_STRATEGY}`, this.getStrategy); this.put( `${PATH_STRATEGY}`, @@ -108,6 +109,11 @@ export default class ProjectFeaturesController extends Controller { this.deleteStrategy, DELETE_FEATURE_STRATEGY, ); + this.post( + `${PATH_FEATURE}/constraint/validate`, + this.validateConstraint, + NONE, + ); // feature toggles this.get(PATH, this.getFeatures); @@ -337,6 +343,13 @@ export default class ProjectFeaturesController extends Controller { res.status(200).json(updatedStrategy); } + async validateConstraint(req: Request, res: Response): Promise { + const constraint: IConstraint = { ...req.body }; + + await this.featureService.validateConstraint(constraint); + res.status(204).send(); + } + async getStrategy( req: IAuthRequest, res: Response, diff --git a/src/lib/schema/constraint-value-types.test.ts b/src/lib/schema/constraint-value-types.test.ts new file mode 100644 index 0000000000..906c25f698 --- /dev/null +++ b/src/lib/schema/constraint-value-types.test.ts @@ -0,0 +1,77 @@ +import { + constraintDateTypeSchema, + constraintNumberTypeSchema, + constraintStringTypeSchema, +} from './constraint-value-types'; + +/* Number type */ +test('should require number', async () => { + try { + await constraintNumberTypeSchema.validateAsync('test'); + } catch (error) { + expect(error.details[0].message).toEqual('"value" must be a number'); + } +}); + +test('should allow strings that can be parsed to a number', async () => { + await constraintNumberTypeSchema.validateAsync('5'); +}); + +test('should allow floating point numbers', async () => { + await constraintNumberTypeSchema.validateAsync(5.72); +}); + +test('should allow numbers', async () => { + await constraintNumberTypeSchema.validateAsync(5); +}); + +test('should allow negative numbers', async () => { + await constraintNumberTypeSchema.validateAsync(-5); +}); + +/* String types */ +test('should require a list of strings', async () => { + expect.assertions(1); + try { + await constraintStringTypeSchema.validateAsync(['test', 1]); + } catch (error) { + expect(error.details[0].message).toEqual('"[1]" must be a string'); + } +}); + +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 new file mode 100644 index 0000000000..5861395169 --- /dev/null +++ b/src/lib/schema/constraint-value-types.ts @@ -0,0 +1,7 @@ +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/schema/feature-schema.test.ts b/src/lib/schema/feature-schema.test.ts index 236fdb047e..700eb5bbc5 100644 --- a/src/lib/schema/feature-schema.test.ts +++ b/src/lib/schema/feature-schema.test.ts @@ -1,4 +1,4 @@ -import { featureSchema, querySchema } from './feature-schema'; +import { constraintSchema, featureSchema, querySchema } from './feature-schema'; test('should require URL firendly name', () => { const toggle = { @@ -272,3 +272,20 @@ test('Filter queries should reject project names that are not alphanum', () => { '"project[0]" must be URL friendly', ); }); + +test('constraint schema should only allow specified operators', async () => { + const invalidConstraint = { + contextName: 'semver', + operator: 'INVALID_OPERATOR', + value: 123123213123, + }; + expect.assertions(1); + + try { + await constraintSchema.validateAsync(invalidConstraint); + } catch (error) { + expect(error.message).toBe( + '"operator" must be one of [NOT_IN, IN, STR_ENDS_WITH, STR_STARTS_WITH, STR_CONTAINS, NUM_EQ, NUM_GT, NUM_GTE, NUM_LT, NUM_LTE, DATE_AFTER, DATE_BEFORE, SEMVER_EQ, SEMVER_GT, SEMVER_LT]', + ); + } +}); diff --git a/src/lib/schema/feature-schema.ts b/src/lib/schema/feature-schema.ts index 395d2c8e97..93bd22f152 100644 --- a/src/lib/schema/feature-schema.ts +++ b/src/lib/schema/feature-schema.ts @@ -1,4 +1,5 @@ import joi from 'joi'; +import { ALL_OPERATORS } from '../util/constants'; import { nameType } from '../routes/util'; export const nameSchema = joi @@ -8,8 +9,11 @@ export const nameSchema = joi export const constraintSchema = joi.object().keys({ contextName: joi.string(), - operator: joi.string(), + operator: joi.string().valid(...ALL_OPERATORS), values: joi.array().items(joi.string().min(1).max(100)).min(1).optional(), + value: joi.optional(), + caseInsensitive: joi.boolean().optional(), + inverted: joi.boolean().optional(), }); export const strategiesSchema = joi.object().keys({ diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts index d237c87c62..3445a7629a 100644 --- a/src/lib/services/feature-toggle-service.ts +++ b/src/lib/services/feature-toggle-service.ts @@ -6,6 +6,7 @@ import NameExistsError from '../error/name-exists-error'; import InvalidOperationError from '../error/invalid-operation-error'; import { FOREIGN_KEY_VIOLATION } from '../error/db-error'; import { + constraintSchema, featureMetadataSchema, nameSchema, variantsArraySchema, @@ -39,6 +40,7 @@ import { FeatureToggleDTO, FeatureToggleLegacy, FeatureToggleWithEnvironment, + IConstraint, IEnvironmentDetail, IFeatureEnvironmentInfo, IFeatureOverview, @@ -50,9 +52,23 @@ import { } from '../types/model'; import { IFeatureEnvironmentStore } from '../types/stores/feature-environment-store'; import { IFeatureToggleClientStore } from '../types/stores/feature-toggle-client-store'; -import { DEFAULT_ENV } from '../util/constants'; +import { + DATE_OPERATORS, + DEFAULT_ENV, + NUM_OPERATORS, + SEMVER_OPERATORS, + STRING_OPERATORS, +} from '../util/constants'; import { applyPatch, deepClone, Operation } from 'fast-json-patch'; import { OperationDeniedError } from '../error/operation-denied-error'; +import { + validateDate, + validateLegalValues, + validateNumber, + validateSemver, + validateString, +} from '../util/validators/constraint-types'; +import { IContextFieldStore } from 'lib/types/stores/context-field-store'; interface IFeatureContext { featureName: string; @@ -63,6 +79,10 @@ interface IFeatureStrategyContext extends IFeatureContext { environment: string; } +const oneOf = (values: string[], match: string) => { + return values.some((value) => value === match); +}; + class FeatureToggleService { private logger: Logger; @@ -80,6 +100,8 @@ class FeatureToggleService { private eventStore: IEventStore; + private contextFieldStore: IContextFieldStore; + constructor( { featureStrategiesStore, @@ -89,6 +111,7 @@ class FeatureToggleService { eventStore, featureTagStore, featureEnvironmentStore, + contextFieldStore, }: Pick< IUnleashStores, | 'featureStrategiesStore' @@ -98,6 +121,7 @@ class FeatureToggleService { | 'eventStore' | 'featureTagStore' | 'featureEnvironmentStore' + | 'contextFieldStore' >, { getLogger }: Pick, ) { @@ -109,6 +133,7 @@ class FeatureToggleService { this.projectStore = projectStore; this.eventStore = eventStore; this.featureEnvironmentStore = featureEnvironmentStore; + this.contextFieldStore = contextFieldStore; } async validateFeatureContext({ @@ -140,6 +165,53 @@ 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)) { + await validateNumber(constraint.value); + } + + if (oneOf(STRING_OPERATORS, operator)) { + await validateString(constraint.values); + } + + 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)) { + 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( project: string, featureName: string, diff --git a/src/lib/types/model.ts b/src/lib/types/model.ts index 57641040a1..301b0df1c0 100644 --- a/src/lib/types/model.ts +++ b/src/lib/types/model.ts @@ -6,7 +6,10 @@ import { IUser } from './user'; export interface IConstraint { contextName: string; operator: string; - values: string[]; + values?: string[]; + value?: string; + inverted?: boolean; + caseInsensitive?: boolean; } export enum WeightType { VARIABLE = 'variable', diff --git a/src/lib/util/constants.ts b/src/lib/util/constants.ts index 45b0ab23b8..6b95bc9e76 100644 --- a/src/lib/util/constants.ts +++ b/src/lib/util/constants.ts @@ -5,3 +5,50 @@ export const ENVIRONMENT_PERMISSION_TYPE = 'environment'; export const PROJECT_PERMISSION_TYPE = 'project'; export const CUSTOM_ROLE_TYPE = 'custom'; + +/* CONTEXT FIELD OPERATORS */ + +export const NOT_IN = 'NOT_IN'; +export const IN = 'IN'; +export const STR_ENDS_WITH = 'STR_ENDS_WITH'; +export const STR_STARTS_WITH = 'STR_STARTS_WITH'; +export const STR_CONTAINS = 'STR_CONTAINS'; +export const NUM_EQ = 'NUM_EQ'; +export const NUM_GT = 'NUM_GT'; +export const NUM_GTE = 'NUM_GTE'; +export const NUM_LT = 'NUM_LT'; +export const NUM_LTE = 'NUM_LTE'; +export const DATE_AFTER = 'DATE_AFTER'; +export const DATE_BEFORE = 'DATE_BEFORE'; +export const SEMVER_EQ = 'SEMVER_EQ'; +export const SEMVER_GT = 'SEMVER_GT'; +export const SEMVER_LT = 'SEMVER_LT'; + +export const ALL_OPERATORS = [ + NOT_IN, + IN, + STR_ENDS_WITH, + STR_STARTS_WITH, + STR_CONTAINS, + NUM_EQ, + NUM_GT, + NUM_GTE, + NUM_LT, + NUM_LTE, + DATE_AFTER, + DATE_BEFORE, + SEMVER_EQ, + SEMVER_GT, + SEMVER_LT, +]; + +export const STRING_OPERATORS = [ + STR_ENDS_WITH, + STR_STARTS_WITH, + STR_CONTAINS, + IN, + NOT_IN, +]; +export const NUM_OPERATORS = [NUM_EQ, NUM_GT, NUM_GTE, NUM_LT, NUM_LTE]; +export const DATE_OPERATORS = [DATE_AFTER, DATE_BEFORE]; +export const SEMVER_OPERATORS = [SEMVER_EQ, SEMVER_GT, SEMVER_LT]; 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 new file mode 100644 index 0000000000..6cbf56e704 --- /dev/null +++ b/src/lib/util/validators/constraint-types.ts @@ -0,0 +1,49 @@ +import semver from 'semver'; + +import { + constraintDateTypeSchema, + constraintNumberTypeSchema, + constraintStringTypeSchema, +} from '../../schema/constraint-value-types'; +import BadDataError from '../../error/bad-data-error'; + +export const validateNumber = async (value: unknown): Promise => { + await constraintNumberTypeSchema.validateAsync(value); +}; + +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`, + ); + } +}; diff --git a/src/migrations/20220224111626-add-current-time-context-field.js b/src/migrations/20220224111626-add-current-time-context-field.js new file mode 100644 index 0000000000..73aca8aae2 --- /dev/null +++ b/src/migrations/20220224111626-add-current-time-context-field.js @@ -0,0 +1,19 @@ +'use strict'; + +exports.up = function (db, cb) { + db.runSql( + ` + INSERT INTO context_fields(name, description, sort_order) VALUES('currentTime', 'Allows you to constrain on date values', 3); + `, + cb, + ); +}; + +exports.down = function (db, cb) { + db.runSql( + ` + DELETE FROM context_fields WHERE name = 'currentTime'; + `, + cb, + ); +};