Skip to content

Commit

Permalink
merge: enable forbidUnknownValues by default (#1798)
Browse files Browse the repository at this point in the history
  • Loading branch information
NoNameProvided committed Nov 20, 2022
2 parents 23071f6 + 0e84a27 commit 54a2cdf
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 19 deletions.
8 changes: 7 additions & 1 deletion README.md
Expand Up @@ -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

Expand Down Expand Up @@ -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';

Expand Down
5 changes: 4 additions & 1 deletion src/validation/ValidationExecutor.ts
Expand Up @@ -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,
Expand All @@ -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 (
Expand Down
28 changes: 19 additions & 9 deletions test/functional/validation-options.spec.ts
Expand Up @@ -9,6 +9,7 @@ import {
ValidatorConstraint,
IsOptional,
IsNotEmpty,
Allow,
} from '../../src/decorator/decorators';
import { Validator } from '../../src/validation/Validator';
import {
Expand Down Expand Up @@ -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);
});
});
Expand Down
28 changes: 20 additions & 8 deletions 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';

Expand Down Expand Up @@ -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();
});
});
});

0 comments on commit 54a2cdf

Please sign in to comment.