Repository: lukeautry/tsoa (ts)

Issue:
required Nested Properties are not validated for existence
<!--- Provide a general summary of the issue in the Title above -->

## Sorting

* **I'm submitting a ...**
  - [x] bug report
  - [ ] feature request
  - [ ] support request

* I confirm that I
  - [x] used the [search](https://github.com/lukeautry/tsoa/search?type=Issues) to make sure that a similar issue hasn't already been submit

## Expected Behavior
Types containing nested properties which are required should be validated to exist.

<!--- If this is related to a TypeScript issue, then include the way that you expected to be able to write the code  -->
```ts
export interface UserCreateRequest {
    firstName: string;
    lastName: string;
    email: string;
    address: {
        country: string;
        state: string;
    };
    phoneNumber?:  string;
}
```

## Current Behavior
Validation is bypassed on fields which are not included in the input.  For example, this is allowed through as a valid request:
```
{
	"firstName": "fanta",
	"lastName": "pop",
	"email": "blah@blah.com",
	"address": {}
}
```

## Possible Solution
It looks like the issue is here: https://github.com/lukeautry/tsoa/blob/282905a260f35ab743f5cdf20b71373f6090f070/src/routeGeneration/templateHelpers.ts#L131-L141

The incoming request object is being looped over to find keys to validate but if the field is missing, there is nothing checking that.

I think the fix is to do something similar to what is being done here:
https://github.com/lukeautry/tsoa/blob/282905a260f35ab743f5cdf20b71373f6090f070/src/routeGeneration/templateHelpers.ts#L626-L633 in which all the propertes in the data model are looped over and then checked for required or not.

## Context (Environment)
Version of the library: 3.0.1
Version of NodeJS:  12.12.0
* Confirm you were using yarn not npm: [x]

## Breaking change?
I think technically this is a breaking change because certain api calls which are now being allowed would now fail validation.

Pull request:
Check required nested properties for existence
### All Submissions:

* [x] Have you followed the guidelines in our [Contributing](https://github.com/lukeautry/tsoa/tree/master/docs/CONTRIBUTING.md) document?
* [x] Have you checked to ensure there aren't other open [Pull Requests](https://github.com/lukeautry/tsoa/pulls) for the same update/change?
* [x] Have you written unit tests?
* [x] Have you written unit tests that cover the negative cases (i.e.: if bad data is submitted, does the library respond properly)?
* [x] This PR is associated with an existing issue?

**Closing issues**

Closes #574

### If this is a new feature submission:

* [x] Has the issue had a maintainer respond to the issue and clarify that the feature is something that aligns with the [goals](https://github.com/lukeautry/tsoa#goal) and [philosophy](https://github.com/lukeautry/tsoa#philosophy) of the project?

Patch:
diff --git a/src/routeGeneration/templateHelpers.ts b/src/routeGeneration/templateHelpers.ts
index f53c4f40..ba29820c 100644
--- a/src/routeGeneration/templateHelpers.ts
+++ b/src/routeGeneration/templateHelpers.ts
@@ -128,18 +128,17 @@ export class ValidationService {
       }
     }
 
-    Object.keys(value).forEach(key => {
-      if (!nestedProperties[key]) {
-        if (additionalProperties && additionalProperties !== true) {
-          return this.ValidateParam(additionalProperties, value[key], key, fieldErrors, parent + name + '.', swaggerConfig);
-        } else {
-          return key;
-        }
+    Object.keys(nestedProperties).forEach(key => {
+      const validatedProp = this.ValidateParam(nestedProperties[key], value[key], key, fieldErrors, parent + name + '.', swaggerConfig);
+      if (validatedProp) {
+        value[key] = validatedProp;
       }
-
-      return this.ValidateParam(nestedProperties[key], value[key], key, fieldErrors, parent + name + '.', swaggerConfig);
     });
 
+    if (Object.keys(fieldErrors).length > 0) {
+      return;
+    }
+
     return value;
   }
 


Test patch:
diff --git a/tests/integration/dynamic-controllers-express-server.spec.ts b/tests/integration/dynamic-controllers-express-server.spec.ts
index 47fcc52d..9ea6ab13 100644
--- a/tests/integration/dynamic-controllers-express-server.spec.ts
+++ b/tests/integration/dynamic-controllers-express-server.spec.ts
@@ -752,7 +752,7 @@ describe('Express Server', () => {
           expect(body.fields['body.typeAliases.fourtyTwo'].message).to.equal('min 42');
           expect(body.fields['body.typeAliases.unionAlias'].message).to.contain('Could not match the union against any of the items');
           expect(body.fields['body.typeAliases.intersectionAlias'].message).to.equal(
-            `Could not match the intersection against every type. Issues: [{"body.typeAliases.value1":{"message":"'value1' is required"}}]`,
+            `Could not match the intersection against every type. Issues: [{"body.typeAliases.intersectionAlias.value1":{"message":"'value1' is required"}},{"body.typeAliases.value1":{"message":"'value1' is required"}}]`,
           );
           expect(body.fields['body.typeAliases.nOLAlias'].message).to.equal('invalid object');
           expect(body.fields['body.typeAliases.genericAlias'].message).to.equal('invalid string value');
diff --git a/tests/integration/express-server.spec.ts b/tests/integration/express-server.spec.ts
index 70565d9f..fdd9ac04 100644
--- a/tests/integration/express-server.spec.ts
+++ b/tests/integration/express-server.spec.ts
@@ -147,6 +147,8 @@ describe('Express Server', () => {
         ...model.objLiteral,
         extra: 123,
         nested: {
+          bool: true,
+          allNestedOptional: {},
           anotherExtra: 123,
         },
       },
@@ -154,7 +156,16 @@ describe('Express Server', () => {
 
     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(resModel).to.deep.equal({
+        ...model,
+        objLiteral: {
+          ...model.objLiteral,
+          nested: {
+            bool: true,
+            allNestedOptional: {},
+          },
+        },
+      });
       expect(res.status).to.eq(200);
     });
   });
@@ -780,7 +791,7 @@ describe('Express Server', () => {
           expect(body.fields['body.typeAliases.fourtyTwo'].message).to.equal('min 42');
           expect(body.fields['body.typeAliases.unionAlias'].message).to.contain('Could not match the union against any of the items');
           expect(body.fields['body.typeAliases.intersectionAlias'].message).to.equal(
-            `Could not match the intersection against every type. Issues: [{"body.typeAliases.value1":{"message":"'value1' is required"}}]`,
+            `Could not match the intersection against every type. Issues: [{"body.typeAliases.intersectionAlias.value1":{"message":"'value1' is required"}},{"body.typeAliases.value1":{"message":"'value1' is required"}}]`,
           );
           expect(body.fields['body.typeAliases.nOLAlias'].message).to.equal('invalid object');
           expect(body.fields['body.typeAliases.genericAlias'].message).to.equal('invalid string value');
diff --git a/tests/integration/hapi-server.spec.ts b/tests/integration/hapi-server.spec.ts
index 3a862081..1224b44a 100644
--- a/tests/integration/hapi-server.spec.ts
+++ b/tests/integration/hapi-server.spec.ts
@@ -726,7 +726,7 @@ describe('Hapi Server', () => {
           expect(body.fields['body.typeAliases.fourtyTwo'].message).to.equal('min 42');
           expect(body.fields['body.typeAliases.unionAlias'].message).to.contain('Could not match the union against any of the items');
           expect(body.fields['body.typeAliases.intersectionAlias'].message).to.equal(
-            `Could not match the intersection against every type. Issues: [{"body.typeAliases.value1":{"message":"'value1' is required"}}]`,
+            `Could not match the intersection against every type. Issues: [{"body.typeAliases.intersectionAlias.value1":{"message":"'value1' is required"}},{"body.typeAliases.value1":{"message":"'value1' is required"}}]`,
           );
           expect(body.fields['body.typeAliases.nOLAlias'].message).to.equal('invalid object');
           expect(body.fields['body.typeAliases.genericAlias'].message).to.equal('invalid string value');
diff --git a/tests/integration/koa-server.spec.ts b/tests/integration/koa-server.spec.ts
index 8c70fdfa..91f31896 100644
--- a/tests/integration/koa-server.spec.ts
+++ b/tests/integration/koa-server.spec.ts
@@ -704,7 +704,7 @@ describe('Koa Server', () => {
           expect(body.fields['body.typeAliases.fourtyTwo'].message).to.equal('min 42');
           expect(body.fields['body.typeAliases.unionAlias'].message).to.contain('Could not match the union against any of the items');
           expect(body.fields['body.typeAliases.intersectionAlias'].message).to.equal(
-            `Could not match the intersection against every type. Issues: [{"body.typeAliases.value1":{"message":"'value1' is required"}}]`,
+            `Could not match the intersection against every type. Issues: [{"body.typeAliases.intersectionAlias.value1":{"message":"'value1' is required"}},{"body.typeAliases.value1":{"message":"'value1' is required"}}]`,
           );
           expect(body.fields['body.typeAliases.nOLAlias'].message).to.equal('invalid object');
           expect(body.fields['body.typeAliases.genericAlias'].message).to.equal('invalid string value');
diff --git a/tests/integration/openapi3-express.spec.ts b/tests/integration/openapi3-express.spec.ts
index 89cbdd17..73740eb6 100644
--- a/tests/integration/openapi3-express.spec.ts
+++ b/tests/integration/openapi3-express.spec.ts
@@ -188,7 +188,7 @@ describe('OpenAPI3 Express Server', () => {
       intersectionAlias2: { value1: 'value1', value2: 'value2', value4: 'test' },
       unionIntersectionAlias1: { value1: 'one', value2: 'two', value3: 'three' },
       unionIntersectionAlias2: { value1: 'one' },
-      unionIntersectionAlias3: { value1: 'one', value2: 'two' },
+      unionIntersectionAlias3: { value1: 'one', value2: 'two', value3: 'three' },
       unionIntersectionAlias4: { value2: 2, value4: 'four' },
       unionAlias: {},
       nOLAlias: true,
@@ -297,7 +297,7 @@ describe('OpenAPI3 Express Server', () => {
         expect(body.fields['body.typeAliases.fourtyTwo'].message).to.equal('min 42');
         expect(body.fields['body.typeAliases.unionAlias'].message).to.contain('Could not match the union against any of the items');
         expect(body.fields['body.typeAliases.intersectionAlias'].message).to.equal(
-          `Could not match the intersection against every type. Issues: [{"body.typeAliases.value1":{"message":"'value1' is required"}}]`,
+          `Could not match the intersection against every type. Issues: [{"body.typeAliases.intersectionAlias.value1":{"message":"'value1' is required"}},{"body.typeAliases.value1":{"message":"'value1' is required"}}]`,
         );
         expect(body.fields['body.typeAliases.nOLAlias'].message).to.equal('invalid object');
         expect(body.fields['body.typeAliases.genericAlias'].message).to.equal('invalid string value');
@@ -314,7 +314,7 @@ describe('OpenAPI3 Express Server', () => {
           'Could not match intersection against any of the possible combinations: [["value1","value3"],["value1","value4"],["value2","value3"],["value2","value4"]]',
         );
         expect(body.fields['body.typeAliases.unionIntersectionAlias2'].message).to.equal(
-          'Could not match intersection against any of the possible combinations: [["value1","value3"],["value1","value4"],["value2","value3"],["value2","value4"]]',
+          `Could not match the intersection against every type. Issues: [{"body.typeAliases.unionIntersectionAlias2":{"message":"Could not match the union against any of the items. Issues: [{\\"body.typeAliases.value3\\":{\\"message\\":\\"'value3' is required\\"}},{\\"body.typeAliases.unionIntersectionAlias2.value4\\":{\\"message\\":\\"'value4' is required\\"}}]","value":{"value1":"one"}}}]`,
         );
         expect(body.fields['body.typeAliases.unionIntersectionAlias3'].message).to.equal(
           'Could not match intersection against any of the possible combinations: [["value1","value3"],["value1","value4"],["value2","value3"],["value2","value4"]]',
diff --git a/tests/unit/templating/templateHelpers.spec.ts b/tests/unit/templating/templateHelpers.spec.ts
index 3b81ecec..7fafeca0 100644
--- a/tests/unit/templating/templateHelpers.spec.ts
+++ b/tests/unit/templating/templateHelpers.spec.ts
@@ -266,22 +266,7 @@ it('should throw if the data has additionalProperties (on a nested Object) if no
       value: { removed: '123' },
     },
   });
-  expect(result).to.eql({
-    name: '',
-    // extra
-    extra: 123,
-    nested: {
-      bool: true,
-      allNestedOptional: {
-        // extra
-        removed: '123',
-      },
-      additionals: {
-        one: { value1: 'one' },
-        two: { value1: 'two' },
-      },
-    },
-  });
+  expect(result).to.eql(undefined);
 });
 
 it('should not throw if the data has additionalProperties (on a intersection) if noImplicitAdditionalProperties is set to silently-remove-extras', () => {
@@ -583,3 +568,48 @@ it('should not throw if the data has additionalProperties (on a nested Object) i
     },
   });
 });
+
+it('should throw if properties on nOl are missing', () => {
+  const schema: { [name: string]: TsoaRoute.PropertySchema } = {
+    country: { dataType: 'string', required: true },
+    street: {
+      dataType: 'nestedObjectLiteral',
+      nestedProperties: {
+        streetName: { dataType: 'string', required: true },
+      },
+      required: true,
+      additionalProperties: true,
+    },
+  };
+
+  const v = new ValidationService({});
+
+  const errors = {};
+  const minimalSwaggerConfig: SwaggerConfigRelatedToRoutes = {
+    noImplicitAdditionalProperties: 'silently-remove-extras',
+  };
+
+  v.validateNestedObjectLiteral('nested', {}, errors, minimalSwaggerConfig, schema, true, 'Model.');
+
+  expect(Object.keys(errors).length).to.equal(2);
+
+  expect(errors).to.deep.eq({
+    'Model.nested.country': { message: "'country' is required", value: undefined },
+    'Model.nested.street': { message: "'street' is required", value: undefined },
+  });
+
+  const nestedErrors = {};
+
+  v.validateNestedObjectLiteral('nested', { street: {} }, nestedErrors, minimalSwaggerConfig, schema, true, 'Model.');
+
+  expect(nestedErrors).to.deep.eq({
+    'Model.nested.country': {
+      message: "'country' is required",
+      value: undefined,
+    },
+    'Model.nested.street.streetName': {
+      message: "'streetName' is required",
+      value: undefined,
+    },
+  });
+});