From 607ef61c36f8641b9dc0908a5b773fbc4fdae4e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20Ol=C3=A1h?= Date: Sun, 20 Nov 2022 17:15:34 +0000 Subject: [PATCH 1/2] feat: enable `forbidUnknownValues` by default --- README.md | 8 +++++++- src/validation/ValidationExecutor.ts | 5 ++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 2706f4134..3a290b034 100644 --- a/README.md +++ b/README.md @@ -140,7 +140,9 @@ export interface ValidatorOptions { } ``` -> It's highly advised to set `forbidUnknownValues: true` as it will prevent unknown objects from passing validation. +> **IMPORTANT** +> The `forbidUnknownValues` value is set to `true` by default and **it is highly advised to keep the default**. +> Setting it to `false` will result unknown objects passing the validation! ## Validation errors @@ -524,6 +526,10 @@ for you, even if skipMissingProperties is set to true. For such cases you should In different situations you may want to use different validation schemas of the same object. In such cases you can use validation groups. +> **IMPORTANT** +> Calling a validation with a group combination that would not result in a validation (eg: non existent group name) +> will result in a unknown value error. When validating with groups the provided group combination should match at least one decorator. + ```typescript import { validate, Min, Length } from 'class-validator'; diff --git a/src/validation/ValidationExecutor.ts b/src/validation/ValidationExecutor.ts index 844bd20af..2d9fb4054 100644 --- a/src/validation/ValidationExecutor.ts +++ b/src/validation/ValidationExecutor.ts @@ -52,6 +52,9 @@ export class ValidationExecutor { const groups = this.validatorOptions ? this.validatorOptions.groups : undefined; const strictGroups = (this.validatorOptions && this.validatorOptions.strictGroups) || false; const always = (this.validatorOptions && this.validatorOptions.always) || false; + /** Forbid unknown values are turned on by default and any other value than false will enable it. */ + const forbidUnknownValues = + this.validatorOptions?.forbidUnknownValues === undefined || this.validatorOptions.forbidUnknownValues !== false; const targetMetadatas = this.metadataStorage.getTargetValidationMetadatas( object.constructor, @@ -62,7 +65,7 @@ export class ValidationExecutor { ); const groupedMetadatas = this.metadataStorage.groupByPropertyName(targetMetadatas); - if (this.validatorOptions && this.validatorOptions.forbidUnknownValues && !targetMetadatas.length) { + if (this.validatorOptions && forbidUnknownValues && !targetMetadatas.length) { const validationError = new ValidationError(); if ( From 0e84a27e5677c4bf04d401d49d574cc0d182c49e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20Ol=C3=A1h?= Date: Sun, 20 Nov 2022 17:16:22 +0000 Subject: [PATCH 2/2] test: update group tests to pass validation with `forbidUnknownValues` enabled --- test/functional/validation-options.spec.ts | 28 +++++++++++++------- test/functional/whitelist-validation.spec.ts | 28 ++++++++++++++------ 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/test/functional/validation-options.spec.ts b/test/functional/validation-options.spec.ts index d36366847..99db80aa7 100644 --- a/test/functional/validation-options.spec.ts +++ b/test/functional/validation-options.spec.ts @@ -9,6 +9,7 @@ import { ValidatorConstraint, IsOptional, IsNotEmpty, + Allow, } from '../../src/decorator/decorators'; import { Validator } from '../../src/validation/Validator'; import { @@ -937,36 +938,45 @@ describe('groups', () => { }); describe('strictGroups', function () { - class MyClass { - @Contains('hello', { - groups: ['A'], - }) + class MyPayload { + /** + * Since forbidUnknownValues defaults to true, we must add a property to + * register the class in the metadata storage. Otherwise the unknown value check + * would take priority (first check) and exit without running the grouping logic. + * + * To solve this we register this property with always: true, so at least a single + * metadata is returned for each validation preventing the unknown value was passed error. + */ + @IsOptional({ always: true }) + propertyToRegisterClass: string; + + @Contains('hello', { groups: ['A'] }) title: string; } - const model1 = new MyClass(); + const instance = new MyPayload(); it('should ignore decorators with groups if validating without groups', function () { - return validator.validate(model1, { strictGroups: true }).then(errors => { + return validator.validate(instance, { strictGroups: true }).then(errors => { expect(errors).toHaveLength(0); }); }); it('should ignore decorators with groups if validating with empty groups array', function () { - return validator.validate(model1, { strictGroups: true, groups: [] }).then(errors => { + return validator.validate(instance, { strictGroups: true, groups: [] }).then(errors => { expect(errors).toHaveLength(0); }); }); it('should include decorators with groups if validating with matching groups', function () { - return validator.validate(model1, { strictGroups: true, groups: ['A'] }).then(errors => { + return validator.validate(instance, { strictGroups: true, groups: ['A'] }).then(errors => { expect(errors).toHaveLength(1); expectTitleContains(errors[0]); }); }); it('should not include decorators with groups if validating with different groups', function () { - return validator.validate(model1, { strictGroups: true, groups: ['B'] }).then(errors => { + return validator.validate(instance, { strictGroups: true, groups: ['B'] }).then(errors => { expect(errors).toHaveLength(0); }); }); diff --git a/test/functional/whitelist-validation.spec.ts b/test/functional/whitelist-validation.spec.ts index 483bd1d83..2667bcb6a 100644 --- a/test/functional/whitelist-validation.spec.ts +++ b/test/functional/whitelist-validation.spec.ts @@ -1,4 +1,4 @@ -import { Allow, IsDefined, Min } from '../../src/decorator/decorators'; +import { Allow, IsDefined, IsOptional, Min } from '../../src/decorator/decorators'; import { Validator } from '../../src/validation/Validator'; import { ValidationTypes } from '../../src'; @@ -46,18 +46,30 @@ describe('whitelist validation', () => { }); it('should throw an error when forbidNonWhitelisted flag is set', () => { - class MyClass {} + class MyPayload { + /** + * Since forbidUnknownValues defaults to true, we must add a property to + * register the class in the metadata storage. Otherwise the unknown value check + * would take priority (first check) and exit without running the whitelist logic. + */ + @IsOptional() + propertyToRegisterClass: string; - const model: any = new MyClass(); + nonDecorated: string; - model.unallowedProperty = 'non-whitelisted'; + constructor(nonDecorated: string) { + this.nonDecorated = nonDecorated; + } + } + + const instance = new MyPayload('non-whitelisted'); - return validator.validate(model, { whitelist: true, forbidNonWhitelisted: true }).then(errors => { + return validator.validate(instance, { whitelist: true, forbidNonWhitelisted: true }).then(errors => { expect(errors.length).toEqual(1); - expect(errors[0].target).toEqual(model); - expect(errors[0].property).toEqual('unallowedProperty'); + expect(errors[0].target).toEqual(instance); + expect(errors[0].property).toEqual('nonDecorated'); expect(errors[0].constraints).toHaveProperty(ValidationTypes.WHITELIST); - expect(() => errors[0].toString()).not.toThrowError(); + expect(() => errors[0].toString()).not.toThrow(); }); }); });