From 254b70f04e6daca40898fdc8046ba02c09a094b3 Mon Sep 17 00:00:00 2001 From: Maggie Kimani Date: Thu, 21 Jul 2022 17:14:08 +0300 Subject: [PATCH 1/9] Add logic to check whether the properties of anyOf/oneOf/allOf match the property name in the dicriminator --- .../Validations/Rules/OpenApiSchemaRules.cs | 43 +++++++++++++++++-- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs index 1639e6248..aadaac4ec 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs @@ -72,13 +72,48 @@ public static class OpenApiSchemaRules { if (!schema.Required.Contains(schema.Discriminator?.PropertyName)) { - context.CreateError(nameof(ValidateSchemaDiscriminator), - string.Format(SRResource.Validation_SchemaRequiredFieldListMustContainThePropertySpecifiedInTheDiscriminator, - schema.Reference.Id, schema.Discriminator.PropertyName)); + // check schema.OneOf, schema.AnyOf or schema.AllOf + if(schema.OneOf.Count != 0) + { + ValidateDiscriminatorAgainstChildSchema(schema.OneOf, schema, context); + } + else if (schema.AnyOf.Count != 0) + { + ValidateDiscriminatorAgainstChildSchema(schema.AnyOf, schema, context); + } + else if (schema.AllOf.Count != 0) + { + ValidateDiscriminatorAgainstChildSchema(schema.AllOf, schema, context); + } + else + { + context.CreateError(nameof(ValidateSchemaDiscriminator), + string.Format(SRResource.Validation_SchemaRequiredFieldListMustContainThePropertySpecifiedInTheDiscriminator, + schema.Reference.Id, schema.Discriminator.PropertyName)); + } } - } + } context.Exit(); }); + + /// + /// Validates the property name in the discriminator against the ones present in the children schema + /// + /// The derived schema. + /// The parent schema. + /// A validation context. + public static void ValidateDiscriminatorAgainstChildSchema(IList childSchema, OpenApiSchema schema, IValidationContext context) + { + foreach (var schemaItem in childSchema) + { + if (!schemaItem.Properties.Keys.Contains(schema.Discriminator?.PropertyName)) + { + context.CreateError(nameof(ValidateSchemaDiscriminator), + string.Format(SRResource.Validation_SchemaRequiredFieldListMustContainThePropertySpecifiedInTheDiscriminator, + schema.Reference.Id, schema.Discriminator.PropertyName)); + } + } + } } } From 96cfc45bbb7a7c66afbe6d30b672b636fbec4c09 Mon Sep 17 00:00:00 2001 From: Maggie Kimani Date: Thu, 21 Jul 2022 17:14:28 +0300 Subject: [PATCH 2/9] Add test case --- .../OpenApiSchemaValidationTests.cs | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/test/Microsoft.OpenApi.Tests/Validations/OpenApiSchemaValidationTests.cs b/test/Microsoft.OpenApi.Tests/Validations/OpenApiSchemaValidationTests.cs index d239e15a1..04acf7737 100644 --- a/test/Microsoft.OpenApi.Tests/Validations/OpenApiSchemaValidationTests.cs +++ b/test/Microsoft.OpenApi.Tests/Validations/OpenApiSchemaValidationTests.cs @@ -268,5 +268,60 @@ public void ValidateSchemaRequiredFieldListMustContainThePropertySpecifiedInTheD "schema1", "property1")) }); } + + [Fact] + public void ValidateOneOfSchemaPropertyNameContainsPropertySpecifiedInTheDiscriminator() + { + // Arrange + var components = new OpenApiComponents + { + Schemas = + { + { + "Person", + new OpenApiSchema + { + Type = "array", + Discriminator = new OpenApiDiscriminator + { + PropertyName = "type" + }, + OneOf = new List + { + new OpenApiSchema + { + Properties = + { + { + "type", + new OpenApiSchema + { + Type = "array" + } + } + }, + Reference = new OpenApiReference + { + Type = ReferenceType.Schema, + Id = "Person" + } + } + }, + Reference = new OpenApiReference { Id = "Person" } + } + } + } + }; + + // Act + var validator = new OpenApiValidator(ValidationRuleSet.GetDefaultRuleSet()); + var walker = new OpenApiWalker(validator); + walker.Walk(components); + + var errors = validator.Errors; + + //Assert + errors.Should().BeEmpty(); + } } } From 2b34d2dad6c7fcdb657437a5c320a0c655cdcd9e Mon Sep 17 00:00:00 2001 From: Maggie Kimani Date: Thu, 21 Jul 2022 17:22:09 +0300 Subject: [PATCH 3/9] Update public Api --- test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt index e9d78acb2..dc42f5485 100755 --- a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt +++ b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt @@ -1283,6 +1283,7 @@ namespace Microsoft.OpenApi.Validations.Rules { public static Microsoft.OpenApi.Validations.ValidationRule SchemaMismatchedDataType { get; } public static Microsoft.OpenApi.Validations.ValidationRule ValidateSchemaDiscriminator { get; } + public static void ValidateDiscriminatorAgainstChildSchema(System.Collections.Generic.IList childSchema, Microsoft.OpenApi.Models.OpenApiSchema schema, Microsoft.OpenApi.Validations.IValidationContext context) { } } [Microsoft.OpenApi.Validations.Rules.OpenApiRule] public static class OpenApiServerRules From 6af10c23e2c68f5f14c7c9d3dcb49400fd0cbb43 Mon Sep 17 00:00:00 2001 From: Maggie Kimani Date: Thu, 21 Jul 2022 17:27:13 +0300 Subject: [PATCH 4/9] Remove whitespace --- src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs index aadaac4ec..0b72ebc52 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs @@ -92,8 +92,8 @@ public static class OpenApiSchemaRules schema.Reference.Id, schema.Discriminator.PropertyName)); } } - } - + } + context.Exit(); }); From 5fd60398c3347ec814f9f65b38fbc164055601fe Mon Sep 17 00:00:00 2001 From: Maggie Kimani Date: Tue, 2 Aug 2022 12:13:07 +0300 Subject: [PATCH 5/9] Update the validation process to be recursive --- .../Validations/Rules/OpenApiSchemaRules.cs | 87 ++++++++++++------- 1 file changed, 56 insertions(+), 31 deletions(-) diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs index 0b72ebc52..e6316588c 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. -using Microsoft.OpenApi.Any; using Microsoft.OpenApi.Models; using Microsoft.OpenApi.Properties; using System.Collections.Generic; @@ -11,7 +10,6 @@ namespace Microsoft.OpenApi.Validations.Rules /// /// The validation rules for . /// - [OpenApiRule] public static class OpenApiSchemaRules { @@ -70,50 +68,77 @@ public static class OpenApiSchemaRules if (schema.Reference != null && schema.Discriminator != null) { - if (!schema.Required.Contains(schema.Discriminator?.PropertyName)) + var discriminator = schema.Discriminator?.PropertyName; + var schemaReferenceId = schema.Reference.Id; + + if (!ValidateChildSchemaAgainstDiscriminator(schema, discriminator, schemaReferenceId, context)) { - // check schema.OneOf, schema.AnyOf or schema.AllOf - if(schema.OneOf.Count != 0) - { - ValidateDiscriminatorAgainstChildSchema(schema.OneOf, schema, context); - } - else if (schema.AnyOf.Count != 0) - { - ValidateDiscriminatorAgainstChildSchema(schema.AnyOf, schema, context); - } - else if (schema.AllOf.Count != 0) - { - ValidateDiscriminatorAgainstChildSchema(schema.AllOf, schema, context); - } - else - { - context.CreateError(nameof(ValidateSchemaDiscriminator), - string.Format(SRResource.Validation_SchemaRequiredFieldListMustContainThePropertySpecifiedInTheDiscriminator, - schema.Reference.Id, schema.Discriminator.PropertyName)); - } + context.CreateError(nameof(ValidateSchemaDiscriminator), + string.Format(SRResource.Validation_SchemaRequiredFieldListMustContainThePropertySpecifiedInTheDiscriminator, + schemaReferenceId, discriminator)); } } - + context.Exit(); }); /// /// Validates the property name in the discriminator against the ones present in the children schema /// - /// The derived schema. /// The parent schema. + /// Adds support for polymorphism. The discriminator is an object name that is used to differentiate + /// between other schemas which may satisfy the payload description. + /// /// A validation context. - public static void ValidateDiscriminatorAgainstChildSchema(IList childSchema, OpenApiSchema schema, IValidationContext context) + public static bool ValidateChildSchemaAgainstDiscriminator(OpenApiSchema schema, string discriminator, string schemaReferenceId, IValidationContext context) { - foreach (var schemaItem in childSchema) + bool containsDiscriminator = false; + + if (!schema.Required.Contains(discriminator)) { - if (!schemaItem.Properties.Keys.Contains(schema.Discriminator?.PropertyName)) + // recursively check nested schema.OneOf, schema.AnyOf or schema.AllOf and their required fields for the discriminator + if (schema.OneOf.Count != 0) + { + return TraverseSchemaElements(discriminator, schema.OneOf, schemaReferenceId, context, containsDiscriminator); + } + if (schema.AnyOf.Count != 0) { - context.CreateError(nameof(ValidateSchemaDiscriminator), - string.Format(SRResource.Validation_SchemaRequiredFieldListMustContainThePropertySpecifiedInTheDiscriminator, - schema.Reference.Id, schema.Discriminator.PropertyName)); + return TraverseSchemaElements(discriminator, schema.AnyOf, schemaReferenceId, context, containsDiscriminator); } - } + if (schema.AllOf.Count != 0) + { + return TraverseSchemaElements(discriminator, schema.AllOf, schemaReferenceId, context, containsDiscriminator); + } + } + + return containsDiscriminator; + } + + /// + /// Traverses the schema elements and checks whether the schema contains the discriminator. + /// + /// Adds support for polymorphism. The discriminator is an object name that is used to differentiate + /// between other schemas which may satisfy the payload description. + /// The child schema. + /// The schema reference Id. + /// A validation context. + /// Tracks whether the discriminator is present. + /// + public static bool TraverseSchemaElements(string discriminator, IList childSchema, string schemaReferenceId, IValidationContext context, bool containsDiscriminator) + { + foreach (var childItem in childSchema) + { + if (!childItem.Properties.ContainsKey(discriminator) && !childItem.Required.Contains(discriminator)) + { + return ValidateChildSchemaAgainstDiscriminator(childItem, discriminator, schemaReferenceId, context); + } + else + { + return containsDiscriminator = true; + } + } + + return containsDiscriminator; } } } From 2707493e77b3dcac79e1531bd46fe550e2f36d9b Mon Sep 17 00:00:00 2001 From: Maggie Kimani Date: Tue, 2 Aug 2022 15:39:51 +0300 Subject: [PATCH 6/9] Code cleanup --- .../Validations/Rules/OpenApiSchemaRules.cs | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs index e6316588c..3b274232e 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs @@ -68,14 +68,13 @@ public static class OpenApiSchemaRules if (schema.Reference != null && schema.Discriminator != null) { - var discriminator = schema.Discriminator?.PropertyName; - var schemaReferenceId = schema.Reference.Id; + var discriminatorName = schema.Discriminator?.PropertyName; - if (!ValidateChildSchemaAgainstDiscriminator(schema, discriminator, schemaReferenceId, context)) + if (!ValidateChildSchemaAgainstDiscriminator(schema, discriminatorName)) { context.CreateError(nameof(ValidateSchemaDiscriminator), string.Format(SRResource.Validation_SchemaRequiredFieldListMustContainThePropertySpecifiedInTheDiscriminator, - schemaReferenceId, discriminator)); + schema.Reference.Id, discriminatorName)); } } @@ -86,30 +85,32 @@ public static class OpenApiSchemaRules /// Validates the property name in the discriminator against the ones present in the children schema /// /// The parent schema. - /// Adds support for polymorphism. The discriminator is an object name that is used to differentiate + /// Adds support for polymorphism. The discriminator is an object name that is used to differentiate /// between other schemas which may satisfy the payload description. - /// - /// A validation context. - public static bool ValidateChildSchemaAgainstDiscriminator(OpenApiSchema schema, string discriminator, string schemaReferenceId, IValidationContext context) + public static bool ValidateChildSchemaAgainstDiscriminator(OpenApiSchema schema, string discriminatorName) { bool containsDiscriminator = false; - if (!schema.Required.Contains(discriminator)) + if (!schema.Required?.Contains(discriminatorName) ?? false) { // recursively check nested schema.OneOf, schema.AnyOf or schema.AllOf and their required fields for the discriminator if (schema.OneOf.Count != 0) { - return TraverseSchemaElements(discriminator, schema.OneOf, schemaReferenceId, context, containsDiscriminator); + return TraverseSchemaElements(discriminatorName, schema.OneOf, ref containsDiscriminator); } if (schema.AnyOf.Count != 0) { - return TraverseSchemaElements(discriminator, schema.AnyOf, schemaReferenceId, context, containsDiscriminator); + return TraverseSchemaElements(discriminatorName, schema.AnyOf, ref containsDiscriminator); } if (schema.AllOf.Count != 0) { - return TraverseSchemaElements(discriminator, schema.AllOf, schemaReferenceId, context, containsDiscriminator); + return TraverseSchemaElements(discriminatorName, schema.AllOf, ref containsDiscriminator); } } + else + { + return true; + } return containsDiscriminator; } @@ -117,20 +118,19 @@ public static bool ValidateChildSchemaAgainstDiscriminator(OpenApiSchema schema, /// /// Traverses the schema elements and checks whether the schema contains the discriminator. /// - /// Adds support for polymorphism. The discriminator is an object name that is used to differentiate + /// Adds support for polymorphism. The discriminator is an object name that is used to differentiate /// between other schemas which may satisfy the payload description. /// The child schema. - /// The schema reference Id. - /// A validation context. /// Tracks whether the discriminator is present. /// - public static bool TraverseSchemaElements(string discriminator, IList childSchema, string schemaReferenceId, IValidationContext context, bool containsDiscriminator) + public static bool TraverseSchemaElements(string discriminatorName, IList childSchema, ref bool containsDiscriminator) { foreach (var childItem in childSchema) { - if (!childItem.Properties.ContainsKey(discriminator) && !childItem.Required.Contains(discriminator)) + if ((!childItem.Properties?.ContainsKey(discriminatorName) ?? false) && + (!childItem.Required?.Contains(discriminatorName) ?? false)) { - return ValidateChildSchemaAgainstDiscriminator(childItem, discriminator, schemaReferenceId, context); + return ValidateChildSchemaAgainstDiscriminator(childItem, discriminatorName); } else { From 13fa9e82cd09007f416f2ae117f58594bfca6864 Mon Sep 17 00:00:00 2001 From: Maggie Kimani Date: Tue, 2 Aug 2022 15:44:13 +0300 Subject: [PATCH 7/9] Update public API interface --- test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt index dc42f5485..87300f76d 100755 --- a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt +++ b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt @@ -1283,7 +1283,8 @@ namespace Microsoft.OpenApi.Validations.Rules { public static Microsoft.OpenApi.Validations.ValidationRule SchemaMismatchedDataType { get; } public static Microsoft.OpenApi.Validations.ValidationRule ValidateSchemaDiscriminator { get; } - public static void ValidateDiscriminatorAgainstChildSchema(System.Collections.Generic.IList childSchema, Microsoft.OpenApi.Models.OpenApiSchema schema, Microsoft.OpenApi.Validations.IValidationContext context) { } + public static bool TraverseSchemaElements(string discriminatorName, System.Collections.Generic.IList childSchema, ref bool containsDiscriminator) { } + public static bool ValidateChildSchemaAgainstDiscriminator(Microsoft.OpenApi.Models.OpenApiSchema schema, string discriminatorName) { } } [Microsoft.OpenApi.Validations.Rules.OpenApiRule] public static class OpenApiServerRules From 3682ceaea538e6026c9e81edb78fa52c6c88bc3e Mon Sep 17 00:00:00 2001 From: Maggie Kimani Date: Thu, 4 Aug 2022 15:18:13 +0300 Subject: [PATCH 8/9] Clean up code --- .../Validations/Rules/OpenApiSchemaRules.cs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs index 3b274232e..a8ed2e93c 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs @@ -89,22 +89,20 @@ public static class OpenApiSchemaRules /// between other schemas which may satisfy the payload description. public static bool ValidateChildSchemaAgainstDiscriminator(OpenApiSchema schema, string discriminatorName) { - bool containsDiscriminator = false; - if (!schema.Required?.Contains(discriminatorName) ?? false) { // recursively check nested schema.OneOf, schema.AnyOf or schema.AllOf and their required fields for the discriminator if (schema.OneOf.Count != 0) { - return TraverseSchemaElements(discriminatorName, schema.OneOf, ref containsDiscriminator); + return TraverseSchemaElements(discriminatorName, schema.OneOf); } if (schema.AnyOf.Count != 0) { - return TraverseSchemaElements(discriminatorName, schema.AnyOf, ref containsDiscriminator); + return TraverseSchemaElements(discriminatorName, schema.AnyOf); } if (schema.AllOf.Count != 0) { - return TraverseSchemaElements(discriminatorName, schema.AllOf, ref containsDiscriminator); + return TraverseSchemaElements(discriminatorName, schema.AllOf); } } else @@ -112,7 +110,7 @@ public static bool ValidateChildSchemaAgainstDiscriminator(OpenApiSchema schema, return true; } - return containsDiscriminator; + return false; } /// @@ -121,9 +119,8 @@ public static bool ValidateChildSchemaAgainstDiscriminator(OpenApiSchema schema, /// Adds support for polymorphism. The discriminator is an object name that is used to differentiate /// between other schemas which may satisfy the payload description. /// The child schema. - /// Tracks whether the discriminator is present. /// - public static bool TraverseSchemaElements(string discriminatorName, IList childSchema, ref bool containsDiscriminator) + public static bool TraverseSchemaElements(string discriminatorName, IList childSchema) { foreach (var childItem in childSchema) { @@ -134,11 +131,11 @@ public static bool TraverseSchemaElements(string discriminatorName, IList Date: Thu, 4 Aug 2022 15:36:40 +0300 Subject: [PATCH 9/9] Remove param --- test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt index 87300f76d..c8930e9fb 100755 --- a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt +++ b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt @@ -1283,7 +1283,7 @@ namespace Microsoft.OpenApi.Validations.Rules { public static Microsoft.OpenApi.Validations.ValidationRule SchemaMismatchedDataType { get; } public static Microsoft.OpenApi.Validations.ValidationRule ValidateSchemaDiscriminator { get; } - public static bool TraverseSchemaElements(string discriminatorName, System.Collections.Generic.IList childSchema, ref bool containsDiscriminator) { } + public static bool TraverseSchemaElements(string discriminatorName, System.Collections.Generic.IList childSchema) { } public static bool ValidateChildSchemaAgainstDiscriminator(Microsoft.OpenApi.Models.OpenApiSchema schema, string discriminatorName) { } } [Microsoft.OpenApi.Validations.Rules.OpenApiRule]