From da71a8960e79c534cf951deec603fe3e155a2aa7 Mon Sep 17 00:00:00 2001 From: WoH Date: Sat, 17 Aug 2019 23:11:56 +0200 Subject: [PATCH] Validation --- src/metadataGeneration/tsoa.ts | 7 +- src/metadataGeneration/typeResolver.ts | 33 ++++++-- src/routeGeneration/routeGenerator.ts | 11 +++ src/routeGeneration/templateHelpers.ts | 76 +++++++++++++++++++ src/routeGeneration/tsoa-route.ts | 2 + src/swagger/specGenerator.ts | 2 +- src/utils/validatorUtils.ts | 2 +- tests/fixtures/testModel.ts | 5 +- tests/integration/express-server.spec.ts | 20 +++++ .../koa-server-no-additional-allowed.spec.ts | 40 ++++++++++ .../definitionsGeneration/definitions.spec.ts | 37 ++++----- tests/unit/swagger/schemaDetails3.spec.ts | 39 +++++----- 12 files changed, 226 insertions(+), 48 deletions(-) diff --git a/src/metadataGeneration/tsoa.ts b/src/metadataGeneration/tsoa.ts index 03e9d8a8d..470e6bd7a 100644 --- a/src/metadataGeneration/tsoa.ts +++ b/src/metadataGeneration/tsoa.ts @@ -68,11 +68,11 @@ export namespace Tsoa { validators: Validators; } - export type TypeStringLiteral = 'string' | 'boolean' | 'double' | 'float' | 'integer' | 'long' | 'enum' | 'array' | 'datetime' | 'date' | 'binary' | 'buffer' | 'byte' | 'void' | 'object' | 'any' | 'refEnum' | 'refObject' | 'objectLiteral'; + export type TypeStringLiteral = 'string' | 'boolean' | 'double' | 'float' | 'integer' | 'long' | 'enum' | 'array' | 'datetime' | 'date' | 'binary' | 'buffer' | 'byte' | 'void' | 'object' | 'any' | 'refEnum' | 'refObject' | 'nestedObjectLiteral'; export type RefTypeLiteral = 'refObject' | 'refEnum'; - export type PrimitiveTypeLiteral = Exclude; + export type PrimitiveTypeLiteral = Exclude; export interface Type { dataType: TypeStringLiteral; @@ -88,8 +88,9 @@ export namespace Tsoa { } export interface ObjectLiteralType extends Type { - dataType: 'objectLiteral'; + dataType: 'nestedObjectLiteral'; properties: Property[]; + additionalProperties?: Type; } export interface ArrayType extends Type { diff --git a/src/metadataGeneration/typeResolver.ts b/src/metadataGeneration/typeResolver.ts index e2d9fdfb4..2e43fca88 100644 --- a/src/metadataGeneration/typeResolver.ts +++ b/src/metadataGeneration/typeResolver.ts @@ -19,7 +19,8 @@ const inProgressTypes: { [typeName: string]: boolean } = {}; type UsableDeclaration = ts.InterfaceDeclaration | ts.ClassDeclaration - | ts.TypeAliasDeclaration; + | ts.TypeAliasDeclaration + | ts.PropertySignature; export class TypeResolver { constructor( @@ -67,7 +68,8 @@ export class TypeResolver { } if (this.typeNode.kind === ts.SyntaxKind.TypeLiteral) { - const properties = (this.typeNode as ts.TypeLiteralNode) + const typeLiteralNode = this.typeNode as ts.TypeLiteralNode; + const properties = typeLiteralNode .members .filter(member => member.kind === ts.SyntaxKind.PropertySignature) .reduce((res, propertySignature: ts.PropertySignature) => { @@ -75,15 +77,36 @@ export class TypeResolver { return [ { + default: getJSDocComment(propertySignature, 'default'), + description: this.getNodeDescription(propertySignature), + format: this.getNodeFormat(propertySignature), name: (propertySignature.name as ts.Identifier).text, required: !propertySignature.questionToken, type, + validators: getPropertyValidators(propertySignature), } as Tsoa.Property, ...res, ]; }, []); - return { dataType: 'objectLiteral', properties } as Tsoa.ObjectLiteralType; + const indexMember = typeLiteralNode.members.find((member) => member.kind === ts.SyntaxKind.IndexSignature); + let additionalType: Tsoa.Type | undefined; + + if (indexMember) { + const indexSignatureDeclaration = indexMember as ts.IndexSignatureDeclaration; + const indexType = new TypeResolver(indexSignatureDeclaration.parameters[0].type as ts.TypeNode, this.current).resolve(); + if (indexType.dataType !== 'string') { + throw new GenerateMetadataError(`Only string indexers are supported.`); + } + + additionalType = new TypeResolver(indexSignatureDeclaration.type as ts.TypeNode, this.current).resolve(); + } + + return { + additionalProperties: indexMember && additionalType, + dataType: 'nestedObjectLiteral', + properties, + } as Tsoa.ObjectLiteralType; } if (this.typeNode.kind === ts.SyntaxKind.ObjectKeyword) { @@ -445,7 +468,7 @@ export class TypeResolver { const modelTypeDeclaration = node as UsableDeclaration; return (modelTypeDeclaration.name as ts.Identifier).text === typeName; - }) as UsableDeclaration[]; + }) as Array>; if (!modelTypes.length) { throw new GenerateMetadataError(`No matching model found for referenced type ${typeName}. If ${typeName} comes from a dependency, please create an interface in your own code that has the same structure. Tsoa can not utilize interfaces from external dependencies. Read more at https://github.com/lukeautry/tsoa/blob/master/ExternalInterfacesExplanation.MD`); @@ -646,7 +669,7 @@ export class TypeResolver { return undefined; } - private getModelInheritedProperties(modelTypeDeclaration: UsableDeclaration): Tsoa.Property[] { + private getModelInheritedProperties(modelTypeDeclaration: Exclude): Tsoa.Property[] { const properties = [] as Tsoa.Property[]; if (modelTypeDeclaration.kind === ts.SyntaxKind.TypeAliasDeclaration) { return []; diff --git a/src/routeGeneration/routeGenerator.ts b/src/routeGeneration/routeGenerator.ts index d5fae7468..561203aec 100644 --- a/src/routeGeneration/routeGenerator.ts +++ b/src/routeGeneration/routeGenerator.ts @@ -218,6 +218,17 @@ export class RouteGenerator { if (type.dataType === 'enum') { schema.enums = (type as Tsoa.EnumerateType).enums; } + + if (type.dataType === 'nestedObjectLiteral') { + const objLiteral = type as Tsoa.ObjectLiteralType; + + schema.nestedProperties = objLiteral.properties.reduce((acc, prop) => { + return {...acc, [prop.name]: this.buildPropertySchema(prop)}; + }, {}); + + schema.additionalProperties = objLiteral.additionalProperties && this.buildProperty(objLiteral.additionalProperties); + } + return schema; } } diff --git a/src/routeGeneration/templateHelpers.ts b/src/routeGeneration/templateHelpers.ts index 0a88deffb..8f8402078 100644 --- a/src/routeGeneration/templateHelpers.ts +++ b/src/routeGeneration/templateHelpers.ts @@ -59,6 +59,8 @@ export class ValidationService { return this.validateBuffer(name, value); case 'any': return value; + case 'nestedObjectLiteral': + return this.validateNestedObjectLiteral(name, value, fieldErrors, minimalSwaggerConfig, property.nestedProperties, property.additionalProperties, parent); default: if (property.ref) { return this.validateModel({ name, value, refName: property.ref, fieldErrors, parent, minimalSwaggerConfig }); @@ -67,6 +69,80 @@ export class ValidationService { } } + public validateNestedObjectLiteral( + name: string, + value: any, + fieldErrors: FieldErrors, + minimalSwaggerConfig: SwaggerConfigRelatedToRoutes, + nestedProperties: { [name: string]: TsoaRoute.PropertySchema} | undefined, + additionalProperties: TsoaRoute.PropertySchema | boolean | undefined, + parent: string, + ) { + if (!nestedProperties) { + return; + } + + const propHandling = minimalSwaggerConfig.noImplicitAdditionalProperties; + if (propHandling) { + const excessProps = this.getExcessPropertiesFor({properties: nestedProperties, additionalProperties}, Object.keys(value)); + if (excessProps.length > 0) { + if (propHandling) { + excessProps.forEach(excessProp => { + delete value[excessProp]; + }); + if (propHandling === 'throw-on-extras') { + fieldErrors[parent + name] = { + message: `"${excessProps}" is an excess property and therefore is not allowed`, + value: excessProps, + }; + } + } + } + } + + Object.keys(value).forEach(key => { + if (!nestedProperties[key]) { + if (additionalProperties && additionalProperties !== true) { + return this.ValidateParam( + additionalProperties, + value[key], + key, + fieldErrors, + parent + name + '.', + minimalSwaggerConfig, + ); + } else { + return key; + } + } + + return this.ValidateParam( + nestedProperties[key], + value[key], + key, + fieldErrors, + parent + name + '.', + minimalSwaggerConfig, + ); + }); + + return value; + } + + private getExcessPropertiesFor(modelDefinition: TsoaRoute.ModelSchema, properties: string[]): string[] { + if (!modelDefinition || !modelDefinition.properties) { + return properties; + } + + const modelProperties = new Set(Object.keys(modelDefinition.properties)); + + if (modelDefinition.additionalProperties) { + return []; + } else { + return [...properties].filter(property => !modelProperties.has(property)); + } + } + public validateInt(name: string, value: any, fieldErrors: FieldErrors, validators?: IntegerValidator, parent = '') { if (!validator.isInt(String(value))) { let message = `invalid integer number`; diff --git a/src/routeGeneration/tsoa-route.ts b/src/routeGeneration/tsoa-route.ts index 8125d09c1..b49ef3e64 100644 --- a/src/routeGeneration/tsoa-route.ts +++ b/src/routeGeneration/tsoa-route.ts @@ -29,6 +29,8 @@ export namespace TsoaRoute { enums?: string[]; validators?: ValidatorSchema; default?: any; + additionalProperties?: boolean | PropertySchema; + nestedProperties?: {[name: string]: PropertySchema}; } export interface ParameterSchema extends PropertySchema { diff --git a/src/swagger/specGenerator.ts b/src/swagger/specGenerator.ts index d7a8ff2bb..503e4daea 100644 --- a/src/swagger/specGenerator.ts +++ b/src/swagger/specGenerator.ts @@ -76,7 +76,7 @@ export class SpecGenerator { return this.getSwaggerTypeForArrayType(type as Tsoa.ArrayType); } else if (type.dataType === 'enum') { return this.getSwaggerTypeForEnumType(type as Tsoa.EnumerateType); - } else if (type.dataType === 'objectLiteral') { + } else if (type.dataType === 'nestedObjectLiteral') { return this.getSwaggerTypeForObjectLiteral(type as Tsoa.ObjectLiteralType); } else { return assertNever(type.dataType); diff --git a/src/utils/validatorUtils.ts b/src/utils/validatorUtils.ts index 81decf72f..08ddcd9f3 100644 --- a/src/utils/validatorUtils.ts +++ b/src/utils/validatorUtils.ts @@ -95,7 +95,7 @@ export function getParameterValidators(parameter: ts.ParameterDeclaration, param }, {} as Tsoa.Validators); } -export function getPropertyValidators(property: ts.PropertyDeclaration): Tsoa.Validators | undefined { +export function getPropertyValidators(property: ts.PropertyDeclaration | ts.PropertySignature): Tsoa.Validators | undefined { const tags = getJSDocTags(property, (tag) => { return getParameterTagSupport().some(value => value === tag.tagName.text); }); diff --git a/tests/fixtures/testModel.ts b/tests/fixtures/testModel.ts index 76a3848e1..0a379c134 100644 --- a/tests/fixtures/testModel.ts +++ b/tests/fixtures/testModel.ts @@ -70,10 +70,13 @@ export interface TestModel extends Model { nested?: { bool: boolean, optional?: number, - allOptional: { + allNestedOptional: { one?: string, two?: string, }, + additionals?: { + [name: string]: TypeAliasModel1, + }, }, }; } diff --git a/tests/integration/express-server.spec.ts b/tests/integration/express-server.spec.ts index 757ad78a3..786c41b1e 100644 --- a/tests/integration/express-server.spec.ts +++ b/tests/integration/express-server.spec.ts @@ -111,6 +111,26 @@ describe('Express Server', () => { }); }); + it('removes additional properties', () => { + const model = getFakeModel(); + const data = { + ...model, + objLiteral: { + ...model.objLiteral, + extra: 123, + nested: { + anotherExtra: 123, + }, + }, + }; + + return verifyPostRequest(basePath + '/PostTest', data, (err: any, res: any) => { + const resModel = res.body as TestModel; + expect(resModel).to.deep.equal({...model, objLiteral: { ...model.objLiteral, nested: {}}}); + expect(res.status).to.eq(200); + }); + }); + it('correctly returns status code', () => { const data = getFakeModel(); const path = basePath + '/PostTest/WithDifferentReturnCode'; diff --git a/tests/integration/koa-server-no-additional-allowed.spec.ts b/tests/integration/koa-server-no-additional-allowed.spec.ts index 9f6876ab0..439993d66 100644 --- a/tests/integration/koa-server-no-additional-allowed.spec.ts +++ b/tests/integration/koa-server-no-additional-allowed.spec.ts @@ -27,6 +27,46 @@ describe('Koa Server (with noImplicitAdditionalProperties turned on)', () => { }, 400); }); + it('should call out any additionalProperties', () => { + const data = Object.assign({}, getFakeModel(), { + objLiteral: { + extra: 123, + nested: { + anotherExtra: 123, + }, + }, + }); + + return verifyPostRequest(basePath + '/PostTest', data, (err: any, res: any) => { + const body = JSON.parse(err.text); + expect(body.fields['model.objLiteral'].message).to.eql('"extra" is an excess property and therefore is not allowed'); + expect(body.fields['model.objLiteral.nested'].message).to.eql('"anotherExtra" is an excess property and therefore is not allowed'); + }, 400); + }); + + it('should respect additional props', () => { + const fakeModel = getFakeModel(); + const data = { + ...fakeModel, + objLiteral: { + name: 'hello', + nested: { + additionals: { + one: { value1: ''}, + }, + allNestedOptional: {}, + bool: true, + }, + }, + } as TestModel; + + return verifyPostRequest(basePath + '/PostTest', data, (err: any, res: any) => { + expect(err).to.equal(false); + const model = res.body as TestModel; + expect(model).to.deep.equal(data); + }); + }); + it('should be okay if there are no additionalProperties', () => { const data = getFakeModel(); diff --git a/tests/unit/swagger/definitionsGeneration/definitions.spec.ts b/tests/unit/swagger/definitionsGeneration/definitions.spec.ts index e3f90878c..92f836933 100644 --- a/tests/unit/swagger/definitionsGeneration/definitions.spec.ts +++ b/tests/unit/swagger/definitionsGeneration/definitions.spec.ts @@ -319,28 +319,29 @@ describe('Definition generation', () => { expect(propertySchema['x-nullable']).to.eq(true, `for property ${propertyName}[x-nullable]`); }, objLiteral: (propertyName, propertySchema) => { - expect(propertySchema).to.deep.include({ - properties: { - name: { - type: 'string', - }, - nested: { - properties: { - allOptional: { + expect(propertySchema).to.deep.equal({ + default: undefined, + description: undefined, + format: undefined, + properties: { + name: { type: 'string' }, + nested: { + properties: { + additionals: { properties: {}, type: 'object' }, + allNestedOptional: { properties: { one: { type: 'string' }, two: { type: 'string' } }, type: 'object', - }, - bool: { type: 'boolean' }, - optional: { format: 'double', type: 'number'}, }, - required: ['allOptional', 'bool'], - type: 'object', + bool: { type: 'boolean' }, + optional: { type: 'number', format: 'double' }, }, - }, - required: ['name'], - type: 'object', - }); - }, + required: [ 'allNestedOptional', 'bool' ], + type: 'object' }, + }, + required: [ 'name' ], + type: 'object', + }); + }, }; Object.keys(assertionsPerProperty).forEach(aPropertyName => { diff --git a/tests/unit/swagger/schemaDetails3.spec.ts b/tests/unit/swagger/schemaDetails3.spec.ts index 9d8499f24..bc29ee52b 100644 --- a/tests/unit/swagger/schemaDetails3.spec.ts +++ b/tests/unit/swagger/schemaDetails3.spec.ts @@ -192,26 +192,27 @@ describe('Definition generation for OpenAPI 3.0.0', () => { expect(propertySchema).to.not.haveOwnProperty('additionalProperties', `for property ${propertyName}`); }, objLiteral: (propertyName, propertySchema) => { - expect(propertySchema).to.deep.include({ - properties: { - name: { - type: 'string', + expect(propertySchema).to.deep.equal({ + default: undefined, + description: undefined, + format: undefined, + properties: { + name: { type: 'string' }, + nested: { + properties: { + additionals: { properties: {}, type: 'object' }, + allNestedOptional: { + properties: { one: { type: 'string' }, two: { type: 'string' } }, + type: 'object', + }, + bool: { type: 'boolean' }, + optional: { type: 'number', format: 'double' }, + }, + required: [ 'allNestedOptional', 'bool' ], + type: 'object' }, }, - nested: { - properties: { - allOptional: { - properties: { one: { type: 'string' }, two: { type: 'string' } }, - type: 'object', - }, - bool: { type: 'boolean' }, - optional: { format: 'double', type: 'number'}, - }, - required: ['allOptional', 'bool'], - type: 'object', - }, - }, - required: ['name'], - type: 'object', + required: [ 'name' ], + type: 'object', }); }, object: (propertyName, propertySchema) => {