From 56615e91f0f207949f1af1a13def109377dd479c Mon Sep 17 00:00:00 2001 From: olav Date: Wed, 4 May 2022 15:16:18 +0200 Subject: [PATCH] fix: validate the type and length of parameter values (#1559) * refactor: coerce primitive types in OpenAPI requests * refactor: avoid broken array args to serializeDates * refactor: avoid some spec refs to improve generated types * refactor: remove debug logging * refactor: fix IExpressOpenApi interface name prefix * refactor: ensure that parameter values are strings * refactor: test that parameter values are coerced to strings --- src/lib/db/feature-strategy-store.ts | 11 +- src/lib/openapi/index.ts | 19 +- src/lib/openapi/spec/constraint-schema.ts | 11 +- .../openapi/spec/create-strategy-request.ts | 12 + .../openapi/spec/create-strategy-schema.ts | 25 + src/lib/openapi/spec/feature-schema.ts | 10 +- src/lib/openapi/spec/features-schema.ts | 7 +- src/lib/openapi/spec/override-schema.ts | 2 +- src/lib/openapi/spec/parameters-schema.ts | 13 + src/lib/openapi/spec/strategy-response.ts | 12 + src/lib/openapi/spec/strategy-schema.ts | 12 +- src/lib/openapi/spec/variant-schema.ts | 9 +- src/lib/routes/admin-api/archive.ts | 12 +- src/lib/routes/admin-api/feature.ts | 6 +- src/lib/routes/admin-api/project/features.ts | 67 ++- src/lib/services/feature-toggle-service.ts | 11 +- src/lib/services/openapi-service.ts | 5 +- src/lib/types/model.ts | 4 +- src/lib/types/openapi.d.ts | 8 +- src/lib/types/saved.ts | 7 + src/lib/util/rewriteHTML.test.ts | 3 +- src/lib/util/serialize-dates.ts | 6 +- .../api/admin/project/features.e2e.test.ts | 72 ++- .../__snapshots__/openapi.e2e.test.ts.snap | 500 +++++++++++++++++- .../feature-toggle-service-v2.e2e.test.ts | 6 +- 25 files changed, 742 insertions(+), 108 deletions(-) create mode 100644 src/lib/openapi/spec/create-strategy-request.ts create mode 100644 src/lib/openapi/spec/create-strategy-schema.ts create mode 100644 src/lib/openapi/spec/parameters-schema.ts create mode 100644 src/lib/openapi/spec/strategy-response.ts create mode 100644 src/lib/types/saved.ts diff --git a/src/lib/db/feature-strategy-store.ts b/src/lib/db/feature-strategy-store.ts index 69210d30d9..8a3c77335b 100644 --- a/src/lib/db/feature-strategy-store.ts +++ b/src/lib/db/feature-strategy-store.ts @@ -53,6 +53,15 @@ interface IFeatureStrategiesTable { created_at?: Date; } +function ensureStringValues(data: object): { [key: string]: string } { + const stringEntries = Object.entries(data).map(([key, value]) => [ + key, + String(value), + ]); + + return Object.fromEntries(stringEntries); +} + function mapRow(row: IFeatureStrategiesTable): IFeatureStrategy { return { id: row.id, @@ -60,7 +69,7 @@ function mapRow(row: IFeatureStrategiesTable): IFeatureStrategy { projectId: row.project_name, environment: row.environment, strategyName: row.strategy_name, - parameters: row.parameters, + parameters: ensureStringValues(row.parameters), constraints: (row.constraints as unknown as IConstraint[]) || [], createdAt: row.created_at, sortOrder: row.sort_order, diff --git a/src/lib/openapi/index.ts b/src/lib/openapi/index.ts index f3fa160390..d5d98ca5d4 100644 --- a/src/lib/openapi/index.ts +++ b/src/lib/openapi/index.ts @@ -1,13 +1,14 @@ import { OpenAPIV3 } from 'openapi-types'; -import { featuresSchema } from './spec/features-schema'; +import { constraintSchema } from './spec/constraint-schema'; +import { createFeatureSchema } from './spec/create-feature-schema'; +import { createStrategySchema } from './spec/create-strategy-schema'; import { featureSchema } from './spec/feature-schema'; +import { featuresSchema } from './spec/features-schema'; +import { overrideSchema } from './spec/override-schema'; +import { parametersSchema } from './spec/parameters-schema'; import { strategySchema } from './spec/strategy-schema'; import { variantSchema } from './spec/variant-schema'; -import { overrideSchema } from './spec/override-schema'; -import { createFeatureSchema } from './spec/create-feature-schema'; -import { constraintSchema } from './spec/constraint-schema'; -// Create the base OpenAPI schema, with everything except paths. export const createOpenApiSchema = ( serverUrl?: string, ): Omit => { @@ -32,13 +33,15 @@ export const createOpenApiSchema = ( }, }, schemas: { + constraintSchema, createFeatureSchema, - featuresSchema, + createStrategySchema, featureSchema, + featuresSchema, + overrideSchema, + parametersSchema, strategySchema, variantSchema, - overrideSchema, - constraintSchema, }, }, }; diff --git a/src/lib/openapi/spec/constraint-schema.ts b/src/lib/openapi/spec/constraint-schema.ts index 079da5ab01..c62e9d854d 100644 --- a/src/lib/openapi/spec/constraint-schema.ts +++ b/src/lib/openapi/spec/constraint-schema.ts @@ -1,6 +1,6 @@ import { createSchemaObject, CreateSchemaType } from '../types'; -export const schema = { +const schema = { type: 'object', additionalProperties: false, required: ['contextName', 'operator'], @@ -11,12 +11,21 @@ export const schema = { operator: { type: 'string', }, + caseInsensitive: { + type: 'boolean', + }, + inverted: { + type: 'boolean', + }, values: { type: 'array', items: { type: 'string', }, }, + value: { + type: 'string', + }, }, } as const; diff --git a/src/lib/openapi/spec/create-strategy-request.ts b/src/lib/openapi/spec/create-strategy-request.ts new file mode 100644 index 0000000000..e350bc4e1e --- /dev/null +++ b/src/lib/openapi/spec/create-strategy-request.ts @@ -0,0 +1,12 @@ +import { OpenAPIV3 } from 'openapi-types'; + +export const createStrategyRequest: OpenAPIV3.RequestBodyObject = { + required: true, + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/createStrategySchema', + }, + }, + }, +}; diff --git a/src/lib/openapi/spec/create-strategy-schema.ts b/src/lib/openapi/spec/create-strategy-schema.ts new file mode 100644 index 0000000000..821b731ce3 --- /dev/null +++ b/src/lib/openapi/spec/create-strategy-schema.ts @@ -0,0 +1,25 @@ +import { createSchemaObject, CreateSchemaType } from '../types'; +import { parametersSchema } from './parameters-schema'; +import { constraintSchema } from './constraint-schema'; + +const schema = { + type: 'object', + additionalProperties: false, + properties: { + name: { + type: 'string', + }, + sortOrder: { + type: 'number', + }, + constraints: { + type: 'array', + items: constraintSchema, + }, + parameters: parametersSchema, + }, +} as const; + +export type CreateStrategySchema = CreateSchemaType; + +export const createStrategySchema = createSchemaObject(schema); diff --git a/src/lib/openapi/spec/feature-schema.ts b/src/lib/openapi/spec/feature-schema.ts index 263283069c..148d4238ed 100644 --- a/src/lib/openapi/spec/feature-schema.ts +++ b/src/lib/openapi/spec/feature-schema.ts @@ -1,4 +1,6 @@ import { createSchemaObject, CreateSchemaType } from '../types'; +import { strategySchema } from './strategy-schema'; +import { variantSchema } from './variant-schema'; const schema = { type: 'object', @@ -38,14 +40,10 @@ const schema = { }, strategies: { type: 'array', - items: { - $ref: '#/components/schemas/strategySchema', - }, + items: strategySchema, }, variants: { - items: { - $ref: '#/components/schemas/variantSchema', - }, + items: variantSchema, type: 'array', }, }, diff --git a/src/lib/openapi/spec/features-schema.ts b/src/lib/openapi/spec/features-schema.ts index 0e8cde902c..f9164996bd 100644 --- a/src/lib/openapi/spec/features-schema.ts +++ b/src/lib/openapi/spec/features-schema.ts @@ -1,6 +1,7 @@ import { createSchemaObject, CreateSchemaType } from '../types'; +import { featureSchema } from './feature-schema'; -export const schema = { +const schema = { type: 'object', additionalProperties: false, required: ['version', 'features'], @@ -10,9 +11,7 @@ export const schema = { }, features: { type: 'array', - items: { - $ref: '#/components/schemas/featureSchema', - }, + items: featureSchema, }, }, } as const; diff --git a/src/lib/openapi/spec/override-schema.ts b/src/lib/openapi/spec/override-schema.ts index ecda013fb0..9e02e7a880 100644 --- a/src/lib/openapi/spec/override-schema.ts +++ b/src/lib/openapi/spec/override-schema.ts @@ -1,6 +1,6 @@ import { createSchemaObject, CreateSchemaType } from '../types'; -export const schema = { +const schema = { type: 'object', additionalProperties: false, required: ['contextName', 'values'], diff --git a/src/lib/openapi/spec/parameters-schema.ts b/src/lib/openapi/spec/parameters-schema.ts new file mode 100644 index 0000000000..f72eda1353 --- /dev/null +++ b/src/lib/openapi/spec/parameters-schema.ts @@ -0,0 +1,13 @@ +import { createSchemaObject, CreateSchemaType } from '../types'; + +const schema = { + type: 'object', + additionalProperties: { + type: 'string', + maxLength: 100, + }, +} as const; + +export type ParametersSchema = CreateSchemaType; + +export const parametersSchema = createSchemaObject(schema); diff --git a/src/lib/openapi/spec/strategy-response.ts b/src/lib/openapi/spec/strategy-response.ts new file mode 100644 index 0000000000..b00a572752 --- /dev/null +++ b/src/lib/openapi/spec/strategy-response.ts @@ -0,0 +1,12 @@ +import { OpenAPIV3 } from 'openapi-types'; + +export const strategyResponse: OpenAPIV3.ResponseObject = { + description: 'strategyResponse', + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/strategySchema', + }, + }, + }, +}; diff --git a/src/lib/openapi/spec/strategy-schema.ts b/src/lib/openapi/spec/strategy-schema.ts index 90f7df6e26..aa4ce190ce 100644 --- a/src/lib/openapi/spec/strategy-schema.ts +++ b/src/lib/openapi/spec/strategy-schema.ts @@ -1,6 +1,8 @@ import { createSchemaObject, CreateSchemaType } from '../types'; +import { constraintSchema } from './constraint-schema'; +import { parametersSchema } from './parameters-schema'; -export const schema = { +const schema = { type: 'object', additionalProperties: false, required: ['id', 'name', 'constraints', 'parameters'], @@ -13,13 +15,9 @@ export const schema = { }, constraints: { type: 'array', - items: { - $ref: '#/components/schemas/constraintSchema', - }, - }, - parameters: { - type: 'object', + items: constraintSchema, }, + parameters: parametersSchema, }, } as const; diff --git a/src/lib/openapi/spec/variant-schema.ts b/src/lib/openapi/spec/variant-schema.ts index dd528f689a..a5366f6b94 100644 --- a/src/lib/openapi/spec/variant-schema.ts +++ b/src/lib/openapi/spec/variant-schema.ts @@ -1,9 +1,10 @@ import { createSchemaObject, CreateSchemaType } from '../types'; +import { overrideSchema } from './override-schema'; -export const schema = { +const schema = { type: 'object', additionalProperties: false, - required: ['name', 'weight', 'weightType', 'stickiness', 'overrides'], + required: ['name', 'weight', 'weightType', 'stickiness'], properties: { name: { type: 'string', @@ -22,9 +23,7 @@ export const schema = { }, overrides: { type: 'array', - items: { - $ref: '#/components/schemas/overrideSchema', - }, + items: overrideSchema, }, }, } as const; diff --git a/src/lib/routes/admin-api/archive.ts b/src/lib/routes/admin-api/archive.ts index 01c0a230d6..c32c308612 100644 --- a/src/lib/routes/admin-api/archive.ts +++ b/src/lib/routes/admin-api/archive.ts @@ -11,6 +11,7 @@ import FeatureToggleService from '../../services/feature-toggle-service'; import { IAuthRequest } from '../unleash-types'; import { featuresResponse } from '../../openapi/spec/features-response'; import { FeaturesSchema } from '../../openapi/spec/features-schema'; +import { serializeDates } from '../../util/serialize-dates'; export default class ArchiveController extends Controller { private readonly logger: Logger; @@ -71,7 +72,11 @@ export default class ArchiveController extends Controller { const features = await this.featureService.getMetadataForAllFeatures( true, ); - res.json({ version: 2, features }); + + res.json({ + version: 2, + features: features.map(serializeDates), + }); } async getArchivedFeaturesByProjectId( @@ -84,7 +89,10 @@ export default class ArchiveController extends Controller { true, projectId, ); - res.json({ version: 2, features }); + res.json({ + version: 2, + features: features.map(serializeDates), + }); } async deleteFeature( diff --git a/src/lib/routes/admin-api/feature.ts b/src/lib/routes/admin-api/feature.ts index f1c07fa00e..8c3860b5b3 100644 --- a/src/lib/routes/admin-api/feature.ts +++ b/src/lib/routes/admin-api/feature.ts @@ -20,6 +20,7 @@ import { IAuthRequest } from '../unleash-types'; import { DEFAULT_ENV } from '../../util/constants'; import { featuresResponse } from '../../openapi/spec/features-response'; import { FeaturesSchema } from '../../openapi/spec/features-schema'; +import { serializeDates } from '../../util/serialize-dates'; const version = 1; @@ -120,7 +121,10 @@ class FeatureController extends Controller { const query = await this.prepQuery(req.query); const features = await this.service.getFeatureToggles(query); - res.json({ version, features }); + res.json({ + version, + features: features.map(serializeDates), + }); } async getToggle( diff --git a/src/lib/routes/admin-api/project/features.ts b/src/lib/routes/admin-api/project/features.ts index ef1b1fd7ea..c706722efc 100644 --- a/src/lib/routes/admin-api/project/features.ts +++ b/src/lib/routes/admin-api/project/features.ts @@ -14,11 +14,7 @@ import { UPDATE_FEATURE_ENVIRONMENT, UPDATE_FEATURE_STRATEGY, } from '../../../types/permissions'; -import { - FeatureToggleDTO, - IConstraint, - IStrategyConfig, -} from '../../../types/model'; +import { FeatureToggleDTO, IStrategyConfig } from '../../../types/model'; import { extractUsername } from '../../../util/extract-user'; import { IAuthRequest } from '../../unleash-types'; import { createFeatureRequest } from '../../../openapi/spec/create-feature-request'; @@ -26,6 +22,10 @@ import { featureResponse } from '../../../openapi/spec/feature-response'; import { CreateFeatureSchema } from '../../../openapi/spec/create-feature-schema'; import { FeatureSchema } from '../../../openapi/spec/feature-schema'; import { serializeDates } from '../../../util/serialize-dates'; +import { createStrategyRequest } from '../../../openapi/spec/create-strategy-request'; +import { CreateStrategySchema } from '../../../openapi/spec/create-strategy-schema'; +import { strategyResponse } from '../../../openapi/spec/strategy-response'; +import { StrategySchema } from '../../../openapi/spec/strategy-schema'; import { featuresResponse } from '../../../openapi/spec/features-response'; interface FeatureStrategyParams { @@ -47,12 +47,6 @@ interface StrategyIdParams extends FeatureStrategyParams { strategyId: string; } -interface StrategyUpdateBody { - name?: string; - constraints?: IConstraint[]; - parameters?: object; -} - const PATH = '/:projectId/features'; const PATH_FEATURE = `${PATH}/:featureName`; const PATH_FEATURE_CLONE = `${PATH_FEATURE}/clone`; @@ -79,11 +73,13 @@ export default class ProjectFeaturesController extends Controller { this.logger = config.getLogger('/admin-api/project/features.ts'); this.get(`${PATH_ENV}`, this.getEnvironment); + this.post( `${PATH_ENV}/on`, this.toggleEnvironmentOn, UPDATE_FEATURE_ENVIRONMENT, ); + this.post( `${PATH_ENV}/off`, this.toggleEnvironmentOff, @@ -91,22 +87,43 @@ export default class ProjectFeaturesController extends Controller { ); this.get(`${PATH_STRATEGIES}`, this.getStrategies); - this.post( - `${PATH_STRATEGIES}`, - this.addStrategy, - CREATE_FEATURE_STRATEGY, - ); + + this.route({ + method: 'post', + path: PATH_STRATEGIES, + handler: this.addStrategy, + permission: CREATE_FEATURE_STRATEGY, + middleware: [ + openApiService.validPath({ + tags: ['admin'], + requestBody: createStrategyRequest, + responses: { 200: strategyResponse }, + }), + ], + }); + this.get(`${PATH_STRATEGY}`, this.getStrategy); - this.put( - `${PATH_STRATEGY}`, - this.updateStrategy, - UPDATE_FEATURE_STRATEGY, - ); + + this.route({ + method: 'put', + path: PATH_STRATEGY, + handler: this.updateStrategy, + permission: UPDATE_FEATURE_STRATEGY, + middleware: [ + openApiService.validPath({ + tags: ['admin'], + requestBody: createStrategyRequest, + responses: { 200: strategyResponse }, + }), + ], + }); + this.patch( `${PATH_STRATEGY}`, this.patchStrategy, UPDATE_FEATURE_STRATEGY, ); + this.delete( `${PATH_STRATEGY}`, this.deleteStrategy, @@ -318,8 +335,8 @@ export default class ProjectFeaturesController extends Controller { } async addStrategy( - req: IAuthRequest, - res: Response, + req: IAuthRequest, + res: Response, ): Promise { const { projectId, featureName, environment } = req.params; const userName = extractUsername(req); @@ -346,8 +363,8 @@ export default class ProjectFeaturesController extends Controller { } async updateStrategy( - req: IAuthRequest, - res: Response, + req: IAuthRequest, + res: Response, ): Promise { const { strategyId, environment, projectId, featureName } = req.params; const userName = extractUsername(req); diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts index d8fe322f1f..583c99ad88 100644 --- a/src/lib/services/feature-toggle-service.ts +++ b/src/lib/services/feature-toggle-service.ts @@ -69,6 +69,7 @@ import { validateString, } from '../util/validators/constraint-types'; import { IContextFieldStore } from 'lib/types/stores/context-field-store'; +import { Saved, Unsaved } from '../types/saved'; interface IFeatureContext { featureName: string; @@ -268,7 +269,7 @@ class FeatureToggleService { featureStrategyToPublic( featureStrategy: IFeatureStrategy, - ): IStrategyConfig { + ): Saved { return { id: featureStrategy.id, name: featureStrategy.strategyName, @@ -278,10 +279,10 @@ class FeatureToggleService { } async createStrategy( - strategyConfig: Omit, + strategyConfig: Unsaved, context: IFeatureStrategyContext, createdBy: string, - ): Promise { + ): Promise> { const { featureName, projectId, environment } = context; await this.validateFeatureContext(context); @@ -342,7 +343,7 @@ class FeatureToggleService { updates: Partial, context: IFeatureStrategyContext, userName: string, - ): Promise { + ): Promise> { const { projectId, environment, featureName } = context; const existingStrategy = await this.featureStrategiesStore.get(id); this.validateFeatureStrategyContext(existingStrategy, context); @@ -392,7 +393,7 @@ class FeatureToggleService { this.validateFeatureStrategyContext(existingStrategy, context); if (existingStrategy.id === id) { - existingStrategy.parameters[name] = value; + existingStrategy.parameters[name] = String(value); const strategy = await this.featureStrategiesStore.updateStrategy( id, existingStrategy, diff --git a/src/lib/services/openapi-service.ts b/src/lib/services/openapi-service.ts index 5febf5bb85..eeb010df0a 100644 --- a/src/lib/services/openapi-service.ts +++ b/src/lib/services/openapi-service.ts @@ -1,4 +1,4 @@ -import openapi, { ExpressOpenApi } from '@unleash/express-openapi'; +import openapi, { IExpressOpenApi } from '@unleash/express-openapi'; import { Express, RequestHandler } from 'express'; import { OpenAPIV3 } from 'openapi-types'; import { IUnleashConfig } from '../types/option'; @@ -8,13 +8,14 @@ import { AdminApiOperation, ClientApiOperation } from '../openapi/types'; export class OpenApiService { private readonly config: IUnleashConfig; - private readonly api: ExpressOpenApi; + private readonly api: IExpressOpenApi; constructor(config: IUnleashConfig) { this.config = config; this.api = openapi( this.docsPath(), createOpenApiSchema(config.server?.unleashUrl), + { coerce: true }, ); } diff --git a/src/lib/types/model.ts b/src/lib/types/model.ts index 42f26d97a5..f06a6c6489 100644 --- a/src/lib/types/model.ts +++ b/src/lib/types/model.ts @@ -19,7 +19,7 @@ export interface IStrategyConfig { id?: string; name: string; constraints: IConstraint[]; - parameters: object; + parameters: { [key: string]: string }; sortOrder?: number; } export interface IFeatureStrategy { @@ -28,7 +28,7 @@ export interface IFeatureStrategy { projectId: string; environment: string; strategyName: string; - parameters: object; + parameters: { [key: string]: string }; sortOrder?: number; constraints: IConstraint[]; createdAt?: Date; diff --git a/src/lib/types/openapi.d.ts b/src/lib/types/openapi.d.ts index c1ad42c396..e4ebb04a0f 100644 --- a/src/lib/types/openapi.d.ts +++ b/src/lib/types/openapi.d.ts @@ -3,11 +3,15 @@ declare module '@unleash/express-openapi' { import { RequestHandler } from 'express'; import { OpenAPIV3 } from 'openapi-types'; - export interface ExpressOpenApi extends RequestHandler { + export interface IExpressOpenApi extends RequestHandler { validPath: (operation: OpenAPIV3.OperationObject) => RequestHandler; schema: (name: string, schema: OpenAPIV3.SchemaObject) => void; swaggerui: RequestHandler; } - export default function openapi(docsPath: string, any): ExpressOpenApi; + export default function openapi( + docsPath: string, + document: Omit, + options?: { coerce: boolean }, + ): IExpressOpenApi; } diff --git a/src/lib/types/saved.ts b/src/lib/types/saved.ts new file mode 100644 index 0000000000..04dd2bb5a7 --- /dev/null +++ b/src/lib/types/saved.ts @@ -0,0 +1,7 @@ +// Add an id field to an object. +export type Saved = T & { + id: Id; +}; + +// Remove an id field from an object. +export type Unsaved = Omit; diff --git a/src/lib/util/rewriteHTML.test.ts b/src/lib/util/rewriteHTML.test.ts index fefde5836c..85db0cb21d 100644 --- a/src/lib/util/rewriteHTML.test.ts +++ b/src/lib/util/rewriteHTML.test.ts @@ -14,7 +14,7 @@ const input = ` + /> { test('rewriteHTML sets favicon path to root', () => { const result = rewriteHTML(input, ''); - console.log(result); expect(result.includes('')).toBe( true, ); diff --git a/src/lib/util/serialize-dates.ts b/src/lib/util/serialize-dates.ts index b8ba3623cb..ee0755b50f 100644 --- a/src/lib/util/serialize-dates.ts +++ b/src/lib/util/serialize-dates.ts @@ -2,9 +2,13 @@ type SerializedDates = { [P in keyof T]: T[P] extends Date ? string : T[P]; }; +// Disallow array arguments for serializeDates. +// Use `array.map(serializeDates)` instead. +type NotArray = Exclude; + // Serialize top-level date values to strings. export const serializeDates = ( - obj: T, + obj: NotArray, ): SerializedDates => { const entries = Object.entries(obj).map(([k, v]) => { if (v instanceof Date) { diff --git a/src/test/e2e/api/admin/project/features.e2e.test.ts b/src/test/e2e/api/admin/project/features.e2e.test.ts index 95f56b810d..11825a28df 100644 --- a/src/test/e2e/api/admin/project/features.e2e.test.ts +++ b/src/test/e2e/api/admin/project/features.e2e.test.ts @@ -16,6 +16,7 @@ import IncompatibleProjectError from '../../../../../lib/error/incompatible-proj import { IVariant, WeightType } from '../../../../../lib/types/model'; import { v4 as uuidv4 } from 'uuid'; import supertest from 'supertest'; +import { randomId } from '../../../../../lib/util/random-id'; let app: IUnleashTest; let db: ITestDb; @@ -704,12 +705,7 @@ test('Can add strategy to feature toggle to a "some-env-2"', async () => { .post( `/api/admin/projects/default/features/${featureName}/environments/${envName}/strategies`, ) - .send({ - name: 'default', - parameters: { - userId: 'string', - }, - }) + .send({ name: 'default', parameters: { userId: 'string' } }) .expect(200); await app.request .get(`/api/admin/projects/default/features/${featureName}`) @@ -722,7 +718,6 @@ test('Can add strategy to feature toggle to a "some-env-2"', async () => { test('Can update strategy on feature toggle', async () => { const envName = 'default'; const featureName = 'feature.strategy.update.strat'; - const projectPath = '/api/admin/projects/default'; const featurePath = `${projectPath}/features/${featureName}`; @@ -735,23 +730,13 @@ test('Can update strategy on feature toggle', async () => { // add strategy const { body: strategy } = await app.request .post(`${featurePath}/environments/${envName}/strategies`) - .send({ - name: 'default', - parameters: { - userIds: '', - }, - }) + .send({ name: 'default', parameters: { userIds: '' } }) .expect(200); // update strategy await app.request .put(`${featurePath}/environments/${envName}/strategies/${strategy.id}`) - .send({ - name: 'default', - parameters: { - userIds: '1234', - }, - }) + .send({ name: 'default', parameters: { userIds: 1234 } }) .expect(200); const { body } = await app.request.get(`${featurePath}`); @@ -764,6 +749,47 @@ test('Can update strategy on feature toggle', async () => { }); }); +test('should coerce all strategy parameter values to strings', async () => { + const envName = 'default'; + const featureName = randomId(); + const projectPath = '/api/admin/projects/default'; + const featurePath = `${projectPath}/features/${featureName}`; + + await app.request + .post(`${projectPath}/features`) + .send({ name: featureName }) + .expect(201); + + await app.request + .post(`${featurePath}/environments/${envName}/strategies`) + .send({ name: 'default', parameters: { foo: 1234 } }) + .expect(200); + + const { body } = await app.request.get(`${featurePath}`); + const defaultEnv = body.environments[0]; + expect(defaultEnv.strategies).toHaveLength(1); + expect(defaultEnv.strategies[0].parameters).toStrictEqual({ + foo: '1234', + }); +}); + +test('should limit the length of parameter values', async () => { + const envName = 'default'; + const featureName = randomId(); + const projectPath = '/api/admin/projects/default'; + const featurePath = `${projectPath}/features/${featureName}`; + + await app.request + .post(`${projectPath}/features`) + .send({ name: featureName }) + .expect(201); + + await app.request + .post(`${featurePath}/environments/${envName}/strategies`) + .send({ name: 'default', parameters: { foo: 'x'.repeat(101) } }) + .expect(400); +}); + test('Can NOT delete strategy with wrong projectId', async () => { const envName = 'default'; const featureName = 'feature.strategy.delete.strat.error'; @@ -1110,7 +1136,7 @@ test('Can patch a strategy based on id', async () => { ) .expect(200) .expect((res) => { - expect(res.body.parameters.rollout).toBe(42); + expect(res.body.parameters.rollout).toBe('42'); }); }); @@ -1209,7 +1235,7 @@ test('Deleting a strategy should include name of feature strategy was deleted fr .post( `/api/admin/projects/default/features/${featureName}/environments/${environment}/strategies`, ) - .send({ name: 'default', constraints: [], properties: {} }) + .send({ name: 'default', constraints: [] }) .expect(200) .expect((res) => { strategyId = res.body.id; @@ -1257,7 +1283,7 @@ test('Enabling environment creates a FEATURE_ENVIRONMENT_ENABLED event', async ( .post( `/api/admin/projects/default/features/${featureName}/environments/${environment}/strategies`, ) - .send({ name: 'default', constraints: [], properties: {} }) + .send({ name: 'default', constraints: [] }) .expect(200); await app.request @@ -1299,7 +1325,7 @@ test('Disabling environment creates a FEATURE_ENVIRONMENT_DISABLED event', async .post( `/api/admin/projects/default/features/${featureName}/environments/${environment}/strategies`, ) - .send({ name: 'default', constraints: [], properties: {} }) + .send({ name: 'default', constraints: [] }) .expect(200); await app.request diff --git a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap index ca6d46425c..bac82f8d88 100644 --- a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap +++ b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap @@ -54,12 +54,21 @@ Object { "constraintSchema": Object { "additionalProperties": false, "properties": Object { + "caseInsensitive": Object { + "type": "boolean", + }, "contextName": Object { "type": "string", }, + "inverted": Object { + "type": "boolean", + }, "operator": Object { "type": "string", }, + "value": Object { + "type": "string", + }, "values": Object { "items": Object { "type": "string", @@ -93,6 +102,59 @@ Object { ], "type": "object", }, + "createStrategySchema": Object { + "additionalProperties": false, + "properties": Object { + "constraints": Object { + "items": Object { + "additionalProperties": false, + "properties": Object { + "caseInsensitive": Object { + "type": "boolean", + }, + "contextName": Object { + "type": "string", + }, + "inverted": Object { + "type": "boolean", + }, + "operator": Object { + "type": "string", + }, + "value": Object { + "type": "string", + }, + "values": Object { + "items": Object { + "type": "string", + }, + "type": "array", + }, + }, + "required": Array [ + "contextName", + "operator", + ], + "type": "object", + }, + "type": "array", + }, + "name": Object { + "type": "string", + }, + "parameters": Object { + "additionalProperties": Object { + "maxLength": 100, + "type": "string", + }, + "type": "object", + }, + "sortOrder": Object { + "type": "number", + }, + }, + "type": "object", + }, "featureSchema": Object { "additionalProperties": false, "properties": Object { @@ -126,7 +188,63 @@ Object { }, "strategies": Object { "items": Object { - "$ref": "#/components/schemas/strategySchema", + "additionalProperties": false, + "properties": Object { + "constraints": Object { + "items": Object { + "additionalProperties": false, + "properties": Object { + "caseInsensitive": Object { + "type": "boolean", + }, + "contextName": Object { + "type": "string", + }, + "inverted": Object { + "type": "boolean", + }, + "operator": Object { + "type": "string", + }, + "value": Object { + "type": "string", + }, + "values": Object { + "items": Object { + "type": "string", + }, + "type": "array", + }, + }, + "required": Array [ + "contextName", + "operator", + ], + "type": "object", + }, + "type": "array", + }, + "id": Object { + "type": "string", + }, + "name": Object { + "type": "string", + }, + "parameters": Object { + "additionalProperties": Object { + "maxLength": 100, + "type": "string", + }, + "type": "object", + }, + }, + "required": Array [ + "id", + "name", + "constraints", + "parameters", + ], + "type": "object", }, "type": "array", }, @@ -135,7 +253,53 @@ Object { }, "variants": Object { "items": Object { - "$ref": "#/components/schemas/variantSchema", + "additionalProperties": false, + "properties": Object { + "name": Object { + "type": "string", + }, + "overrides": Object { + "items": Object { + "additionalProperties": false, + "properties": Object { + "contextName": Object { + "type": "string", + }, + "values": Object { + "items": Object { + "type": "string", + }, + "type": "array", + }, + }, + "required": Array [ + "contextName", + "values", + ], + "type": "object", + }, + "type": "array", + }, + "payload": Object { + "type": "object", + }, + "stickiness": Object { + "type": "string", + }, + "weight": Object { + "type": "number", + }, + "weightType": Object { + "type": "string", + }, + }, + "required": Array [ + "name", + "weight", + "weightType", + "stickiness", + ], + "type": "object", }, "type": "array", }, @@ -151,7 +315,159 @@ Object { "properties": Object { "features": Object { "items": Object { - "$ref": "#/components/schemas/featureSchema", + "additionalProperties": false, + "properties": Object { + "createdAt": Object { + "format": "date", + "nullable": true, + "type": "string", + }, + "description": Object { + "type": "string", + }, + "enabled": Object { + "type": "boolean", + }, + "impressionData": Object { + "type": "boolean", + }, + "lastSeenAt": Object { + "format": "date", + "nullable": true, + "type": "string", + }, + "name": Object { + "type": "string", + }, + "project": Object { + "type": "string", + }, + "stale": Object { + "type": "boolean", + }, + "strategies": Object { + "items": Object { + "additionalProperties": false, + "properties": Object { + "constraints": Object { + "items": Object { + "additionalProperties": false, + "properties": Object { + "caseInsensitive": Object { + "type": "boolean", + }, + "contextName": Object { + "type": "string", + }, + "inverted": Object { + "type": "boolean", + }, + "operator": Object { + "type": "string", + }, + "value": Object { + "type": "string", + }, + "values": Object { + "items": Object { + "type": "string", + }, + "type": "array", + }, + }, + "required": Array [ + "contextName", + "operator", + ], + "type": "object", + }, + "type": "array", + }, + "id": Object { + "type": "string", + }, + "name": Object { + "type": "string", + }, + "parameters": Object { + "additionalProperties": Object { + "maxLength": 100, + "type": "string", + }, + "type": "object", + }, + }, + "required": Array [ + "id", + "name", + "constraints", + "parameters", + ], + "type": "object", + }, + "type": "array", + }, + "type": Object { + "type": "string", + }, + "variants": Object { + "items": Object { + "additionalProperties": false, + "properties": Object { + "name": Object { + "type": "string", + }, + "overrides": Object { + "items": Object { + "additionalProperties": false, + "properties": Object { + "contextName": Object { + "type": "string", + }, + "values": Object { + "items": Object { + "type": "string", + }, + "type": "array", + }, + }, + "required": Array [ + "contextName", + "values", + ], + "type": "object", + }, + "type": "array", + }, + "payload": Object { + "type": "object", + }, + "stickiness": Object { + "type": "string", + }, + "weight": Object { + "type": "number", + }, + "weightType": Object { + "type": "string", + }, + }, + "required": Array [ + "name", + "weight", + "weightType", + "stickiness", + ], + "type": "object", + }, + "type": "array", + }, + }, + "required": Array [ + "name", + "project", + ], + "type": "object", }, "type": "array", }, @@ -184,12 +500,47 @@ Object { ], "type": "object", }, + "parametersSchema": Object { + "additionalProperties": Object { + "maxLength": 100, + "type": "string", + }, + "type": "object", + }, "strategySchema": Object { "additionalProperties": false, "properties": Object { "constraints": Object { "items": Object { - "$ref": "#/components/schemas/constraintSchema", + "additionalProperties": false, + "properties": Object { + "caseInsensitive": Object { + "type": "boolean", + }, + "contextName": Object { + "type": "string", + }, + "inverted": Object { + "type": "boolean", + }, + "operator": Object { + "type": "string", + }, + "value": Object { + "type": "string", + }, + "values": Object { + "items": Object { + "type": "string", + }, + "type": "array", + }, + }, + "required": Array [ + "contextName", + "operator", + ], + "type": "object", }, "type": "array", }, @@ -200,6 +551,10 @@ Object { "type": "string", }, "parameters": Object { + "additionalProperties": Object { + "maxLength": 100, + "type": "string", + }, "type": "object", }, }, @@ -219,7 +574,23 @@ Object { }, "overrides": Object { "items": Object { - "$ref": "#/components/schemas/overrideSchema", + "additionalProperties": false, + "properties": Object { + "contextName": Object { + "type": "string", + }, + "values": Object { + "items": Object { + "type": "string", + }, + "type": "array", + }, + }, + "required": Array [ + "contextName", + "values", + ], + "type": "object", }, "type": "array", }, @@ -241,7 +612,6 @@ Object { "weight", "weightType", "stickiness", - "overrides", ], "type": "object", }, @@ -432,6 +802,124 @@ Object { ], }, }, + "/api/admin/projects/{projectId}/features/{featureName}/environments/{environment}/strategies": Object { + "post": Object { + "parameters": Array [ + Object { + "in": "path", + "name": "projectId", + "required": true, + "schema": Object { + "type": "string", + }, + }, + Object { + "in": "path", + "name": "featureName", + "required": true, + "schema": Object { + "type": "string", + }, + }, + Object { + "in": "path", + "name": "environment", + "required": true, + "schema": Object { + "type": "string", + }, + }, + ], + "requestBody": Object { + "content": Object { + "application/json": Object { + "schema": Object { + "$ref": "#/components/schemas/createStrategySchema", + }, + }, + }, + "required": true, + }, + "responses": Object { + "200": Object { + "content": Object { + "application/json": Object { + "schema": Object { + "$ref": "#/components/schemas/strategySchema", + }, + }, + }, + "description": "strategyResponse", + }, + }, + "tags": Array [ + "admin", + ], + }, + }, + "/api/admin/projects/{projectId}/features/{featureName}/environments/{environment}/strategies/{strategyId}": Object { + "put": Object { + "parameters": Array [ + Object { + "in": "path", + "name": "projectId", + "required": true, + "schema": Object { + "type": "string", + }, + }, + Object { + "in": "path", + "name": "featureName", + "required": true, + "schema": Object { + "type": "string", + }, + }, + Object { + "in": "path", + "name": "environment", + "required": true, + "schema": Object { + "type": "string", + }, + }, + Object { + "in": "path", + "name": "strategyId", + "required": true, + "schema": Object { + "type": "string", + }, + }, + ], + "requestBody": Object { + "content": Object { + "application/json": Object { + "schema": Object { + "$ref": "#/components/schemas/createStrategySchema", + }, + }, + }, + "required": true, + }, + "responses": Object { + "200": Object { + "content": Object { + "application/json": Object { + "schema": Object { + "$ref": "#/components/schemas/strategySchema", + }, + }, + }, + "description": "strategyResponse", + }, + }, + "tags": Array [ + "admin", + ], + }, + }, }, "security": Array [ Object { diff --git a/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts b/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts index a7b7fd065f..67025897f5 100644 --- a/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts +++ b/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts @@ -75,14 +75,12 @@ test('Should be able to update existing strategy configuration', async () => { expect(createdConfig.name).toEqual('default'); const updatedConfig = await service.updateStrategy( createdConfig.id, - { - parameters: { b2b: true }, - }, + { parameters: { b2b: 'true' } }, { projectId, featureName, environment: DEFAULT_ENV }, username, ); expect(createdConfig.id).toEqual(updatedConfig.id); - expect(updatedConfig.parameters).toEqual({ b2b: true }); + expect(updatedConfig.parameters).toEqual({ b2b: 'true' }); }); test('Should be able to get strategy by id', async () => {