From 26e7e5a15093bd8a824df3bbbeafdd74b1cca4ab Mon Sep 17 00:00:00 2001 From: Paul Rijneveld Date: Sun, 13 Apr 2025 23:35:51 +0200 Subject: [PATCH 1/3] fix!: handle deserializing and writing empty security requirements #1426 make distinction between empty security requirements and no security requirements on an operation. empty security requirements are read as an empty list, no security requirements are read as null for OpenAPI v2/v3/v3.1. This is a breaking change, previously both cases were read as an empty list. also includes a change to OpenApiOperation.SerializeInternal so it can serialize these two cases separately. this required a new method OpenApiWriterExtensions.WriteOptionalOrEmptyCollection. includes unit tests, change to PublicApi.approved.txt to include the new method, and I removed a couple of unused usings and a typo in test name `SerializeDocWithSecuritySchemeWithInlineReferencesWorks`. --- .../Models/OpenApiOperation.cs | 4 +- .../Reader/V2/OpenApiOperationDeserializer.cs | 7 +- .../Reader/V3/OpenApiOperationDeserializer.cs | 9 ++- .../V31/OpenApiOperationDeserializer.cs | 8 ++- .../Writers/OpenApiWriterExtensions.cs | 20 ++++++ .../Microsoft.OpenApi.Tests.csproj | 8 +++ .../Models/OpenApiDocumentTests.cs | 69 ++++++++++++++++++- .../docWithEmptyOperationSecurity.yaml | 20 ++++++ .../Samples/docWithoutOperationSecurity.yaml | 19 +++++ .../PublicApi/PublicApi.approved.txt | 1 + 10 files changed, 155 insertions(+), 10 deletions(-) create mode 100644 test/Microsoft.OpenApi.Tests/Models/Samples/docWithEmptyOperationSecurity.yaml create mode 100644 test/Microsoft.OpenApi.Tests/Models/Samples/docWithoutOperationSecurity.yaml diff --git a/src/Microsoft.OpenApi/Models/OpenApiOperation.cs b/src/Microsoft.OpenApi/Models/OpenApiOperation.cs index 592d9cb58..3fed29698 100644 --- a/src/Microsoft.OpenApi/Models/OpenApiOperation.cs +++ b/src/Microsoft.OpenApi/Models/OpenApiOperation.cs @@ -213,8 +213,8 @@ private void SerializeInternal(IOpenApiWriter writer, OpenApiSpecVersion version writer.WriteProperty(OpenApiConstants.Deprecated, Deprecated, false); // security - writer.WriteOptionalCollection(OpenApiConstants.Security, Security, callback); - + writer.WriteOptionalOrEmptyCollection(OpenApiConstants.Security, Security, callback); + // servers writer.WriteOptionalCollection(OpenApiConstants.Servers, Servers, callback); diff --git a/src/Microsoft.OpenApi/Reader/V2/OpenApiOperationDeserializer.cs b/src/Microsoft.OpenApi/Reader/V2/OpenApiOperationDeserializer.cs index 4e53273e8..7366ce976 100644 --- a/src/Microsoft.OpenApi/Reader/V2/OpenApiOperationDeserializer.cs +++ b/src/Microsoft.OpenApi/Reader/V2/OpenApiOperationDeserializer.cs @@ -10,7 +10,7 @@ using Microsoft.OpenApi.Models.References; using Microsoft.OpenApi.Models.Interfaces; using System; -using Microsoft.OpenApi.Interfaces; +using System.Text.Json.Nodes; namespace Microsoft.OpenApi.Reader.V2 { @@ -96,7 +96,10 @@ internal static partial class OpenApiV2Deserializer }, { "security", - (o, n, t) => o.Security = n.CreateList(LoadSecurityRequirement, t) + (o, n, t) => { if (n.JsonNode is JsonArray) + { + o.Security = new List(n.CreateList(LoadSecurityRequirement, t)); + } } }, }; diff --git a/src/Microsoft.OpenApi/Reader/V3/OpenApiOperationDeserializer.cs b/src/Microsoft.OpenApi/Reader/V3/OpenApiOperationDeserializer.cs index eb971da7c..8a0e19336 100644 --- a/src/Microsoft.OpenApi/Reader/V3/OpenApiOperationDeserializer.cs +++ b/src/Microsoft.OpenApi/Reader/V3/OpenApiOperationDeserializer.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Text.Json.Nodes; using Microsoft.OpenApi.Extensions; using Microsoft.OpenApi.Models; using Microsoft.OpenApi.Models.References; @@ -83,7 +84,13 @@ internal static partial class OpenApiV3Deserializer }, { "security", - (o, n, t) => o.Security = n.CreateList(LoadSecurityRequirement, t) + (o, n, t) => + { + if (n.JsonNode is JsonArray) + { + o.Security = n.CreateList(LoadSecurityRequirement, t); + } + } }, { "servers", diff --git a/src/Microsoft.OpenApi/Reader/V31/OpenApiOperationDeserializer.cs b/src/Microsoft.OpenApi/Reader/V31/OpenApiOperationDeserializer.cs index abf35545b..e14d34ffc 100644 --- a/src/Microsoft.OpenApi/Reader/V31/OpenApiOperationDeserializer.cs +++ b/src/Microsoft.OpenApi/Reader/V31/OpenApiOperationDeserializer.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Text.Json.Nodes; using Microsoft.OpenApi.Extensions; using Microsoft.OpenApi.Models; using Microsoft.OpenApi.Models.References; @@ -96,8 +97,11 @@ internal static partial class OpenApiV31Deserializer }, { "security", (o, n, t) => - { - o.Security = n.CreateList(LoadSecurityRequirement, t); + { + if (n.JsonNode is JsonArray) + { + o.Security = new List(n.CreateList(LoadSecurityRequirement, t)); + } } }, { diff --git a/src/Microsoft.OpenApi/Writers/OpenApiWriterExtensions.cs b/src/Microsoft.OpenApi/Writers/OpenApiWriterExtensions.cs index 31f60d0fd..c799171c5 100644 --- a/src/Microsoft.OpenApi/Writers/OpenApiWriterExtensions.cs +++ b/src/Microsoft.OpenApi/Writers/OpenApiWriterExtensions.cs @@ -217,6 +217,26 @@ public static void WriteOptionalCollection( writer.WriteCollectionInternal(name, elements, action); } } + + /// + /// Write the optional or empty Open API object/element collection. + /// + /// The Open API element type. + /// The Open API writer. + /// The property name. + /// The collection values. + /// The collection element writer action. + public static void WriteOptionalOrEmptyCollection( + this IOpenApiWriter writer, + string name, + IEnumerable? elements, + Action action) + { + if (elements != null) + { + writer.WriteCollectionInternal(name, elements, action); + } + } /// /// Write the required Open API object/element collection. diff --git a/test/Microsoft.OpenApi.Tests/Microsoft.OpenApi.Tests.csproj b/test/Microsoft.OpenApi.Tests/Microsoft.OpenApi.Tests.csproj index cf5736be0..f9c244d2e 100644 --- a/test/Microsoft.OpenApi.Tests/Microsoft.OpenApi.Tests.csproj +++ b/test/Microsoft.OpenApi.Tests/Microsoft.OpenApi.Tests.csproj @@ -56,5 +56,13 @@ + + + PreserveNewest + + + + PreserveNewest + \ No newline at end of file diff --git a/test/Microsoft.OpenApi.Tests/Models/OpenApiDocumentTests.cs b/test/Microsoft.OpenApi.Tests/Models/OpenApiDocumentTests.cs index 2774680a7..6052e4898 100644 --- a/test/Microsoft.OpenApi.Tests/Models/OpenApiDocumentTests.cs +++ b/test/Microsoft.OpenApi.Tests/Models/OpenApiDocumentTests.cs @@ -9,12 +9,10 @@ using System.Threading.Tasks; using Microsoft.OpenApi.Any; using Microsoft.OpenApi.Extensions; -using Microsoft.OpenApi.Interfaces; using Microsoft.OpenApi.Models; using Microsoft.OpenApi.Models.Interfaces; using Microsoft.OpenApi.Models.References; using Microsoft.OpenApi.Writers; -using Microsoft.VisualBasic; using VerifyXunit; using Xunit; @@ -2179,7 +2177,7 @@ public void SerializeAsThrowsIfVersionIsNotSupported() } [Fact] - public async Task SerializeDocWithSecuritySchemeWithInlineRefererencesWorks() + public async Task SerializeDocWithSecuritySchemeWithInlineReferencesWorks() { var expected = @"openapi: 3.0.4 info: @@ -2220,5 +2218,70 @@ public async Task SerializeDocWithSecuritySchemeWithInlineRefererencesWorks() var actual = stringWriter.ToString(); Assert.Equal(expected.MakeLineBreaksEnvironmentNeutral(), actual.MakeLineBreaksEnvironmentNeutral()); } + + [Fact] + public async Task SerializeDocWithoutOperationSecurityWorks() + { + var expected = """ + openapi: 3.0.4 + info: + title: Repair Service + version: 1.0.0 + servers: + - url: https://pluginrentu.azurewebsites.net/api + paths: + /repairs: + get: + summary: List all repairs + description: Returns a list of repairs with their details and images + operationId: listRepairs + responses: + '200': + description: A list of repairs + content: + application/json: + schema: + type: object + """; + + var doc = (await OpenApiDocument.LoadAsync("Models/Samples/docWithoutOperationSecurity.yaml", SettingsFixture.ReaderSettings)).Document; + var stringWriter = new StringWriter(); + doc!.SerializeAsV3(new OpenApiYamlWriter(stringWriter, new OpenApiWriterSettings { InlineLocalReferences = true })); + var actual = stringWriter.ToString(); + Assert.Equal(expected.MakeLineBreaksEnvironmentNeutral(), actual.MakeLineBreaksEnvironmentNeutral()); + } + + [Fact] + public async Task SerializeDocWithEmptyOperationSecurityWorks() + { + var expected = """ + openapi: 3.0.4 + info: + title: Repair Service + version: 1.0.0 + servers: + - url: https://pluginrentu.azurewebsites.net/api + paths: + /repairs: + get: + summary: List all repairs + description: Returns a list of repairs with their details and images + operationId: listRepairs + responses: + '200': + description: A list of repairs + content: + application/json: + schema: + type: object + security: [ ] + """; + + var doc = (await OpenApiDocument.LoadAsync("Models/Samples/docWithEmptyOperationSecurity.yaml", SettingsFixture.ReaderSettings)).Document; + var stringWriter = new StringWriter(); + doc!.SerializeAsV3(new OpenApiYamlWriter(stringWriter, new OpenApiWriterSettings { InlineLocalReferences = true })); + var actual = stringWriter.ToString(); + Assert.Equal(expected.MakeLineBreaksEnvironmentNeutral(), actual.MakeLineBreaksEnvironmentNeutral()); + } } } diff --git a/test/Microsoft.OpenApi.Tests/Models/Samples/docWithEmptyOperationSecurity.yaml b/test/Microsoft.OpenApi.Tests/Models/Samples/docWithEmptyOperationSecurity.yaml new file mode 100644 index 000000000..1f397fedf --- /dev/null +++ b/test/Microsoft.OpenApi.Tests/Models/Samples/docWithEmptyOperationSecurity.yaml @@ -0,0 +1,20 @@ +openapi: 3.0.0 +info: + title: Repair Service + version: 1.0.0 +servers: + - url: https://pluginrentu.azurewebsites.net/api +paths: + /repairs: + get: + operationId: listRepairs + summary: List all repairs + description: Returns a list of repairs with their details and images + responses: + '200': + description: A list of repairs + content: + application/json: + schema: + type: object + security: [] \ No newline at end of file diff --git a/test/Microsoft.OpenApi.Tests/Models/Samples/docWithoutOperationSecurity.yaml b/test/Microsoft.OpenApi.Tests/Models/Samples/docWithoutOperationSecurity.yaml new file mode 100644 index 000000000..edaf2f1f7 --- /dev/null +++ b/test/Microsoft.OpenApi.Tests/Models/Samples/docWithoutOperationSecurity.yaml @@ -0,0 +1,19 @@ +openapi: 3.0.0 +info: + title: Repair Service + version: 1.0.0 +servers: + - url: https://pluginrentu.azurewebsites.net/api +paths: + /repairs: + get: + operationId: listRepairs + summary: List all repairs + description: Returns a list of repairs with their details and images + responses: + '200': + description: A list of repairs + content: + application/json: + schema: + type: object \ No newline at end of file diff --git a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt index 2ec1fb15b..4a8efd957 100644 --- a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt +++ b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt @@ -1975,6 +1975,7 @@ namespace Microsoft.OpenApi.Writers public static void WriteOptionalMap(this Microsoft.OpenApi.Writers.IOpenApiWriter writer, string name, System.Collections.Generic.Dictionary? elements, System.Action action) where T : Microsoft.OpenApi.Interfaces.IOpenApiElement { } public static void WriteOptionalObject(this Microsoft.OpenApi.Writers.IOpenApiWriter writer, string name, T? value, System.Action action) { } + public static void WriteOptionalOrEmptyCollection(this Microsoft.OpenApi.Writers.IOpenApiWriter writer, string name, System.Collections.Generic.IEnumerable? elements, System.Action action) { } public static void WriteProperty(this Microsoft.OpenApi.Writers.IOpenApiWriter writer, string name, string? value) { } public static void WriteProperty(this Microsoft.OpenApi.Writers.IOpenApiWriter writer, string name, bool value, bool defaultValue = false) { } public static void WriteProperty(this Microsoft.OpenApi.Writers.IOpenApiWriter writer, string name, bool? value, bool defaultValue = false) { } From 30f2fd16a889e197203f416f0a1943770b8f7a71 Mon Sep 17 00:00:00 2001 From: Paul Rijneveld Date: Fri, 18 Apr 2025 09:30:22 +0200 Subject: [PATCH 2/3] Review: make security property tests more explicit --- test/Microsoft.OpenApi.Tests/Models/OpenApiDocumentTests.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/Microsoft.OpenApi.Tests/Models/OpenApiDocumentTests.cs b/test/Microsoft.OpenApi.Tests/Models/OpenApiDocumentTests.cs index 6052e4898..d5a45489c 100644 --- a/test/Microsoft.OpenApi.Tests/Models/OpenApiDocumentTests.cs +++ b/test/Microsoft.OpenApi.Tests/Models/OpenApiDocumentTests.cs @@ -2249,6 +2249,8 @@ public async Task SerializeDocWithoutOperationSecurityWorks() doc!.SerializeAsV3(new OpenApiYamlWriter(stringWriter, new OpenApiWriterSettings { InlineLocalReferences = true })); var actual = stringWriter.ToString(); Assert.Equal(expected.MakeLineBreaksEnvironmentNeutral(), actual.MakeLineBreaksEnvironmentNeutral()); + var actualOperation = doc.Paths["/repairs"]!.Operations![HttpMethod.Get]; + Assert.Null(actualOperation.Security); } [Fact] @@ -2282,6 +2284,9 @@ public async Task SerializeDocWithEmptyOperationSecurityWorks() doc!.SerializeAsV3(new OpenApiYamlWriter(stringWriter, new OpenApiWriterSettings { InlineLocalReferences = true })); var actual = stringWriter.ToString(); Assert.Equal(expected.MakeLineBreaksEnvironmentNeutral(), actual.MakeLineBreaksEnvironmentNeutral()); + var actualOperation = doc.Paths["/repairs"]!.Operations![HttpMethod.Get]; + Assert.NotNull(actualOperation.Security); + Assert.Empty(actualOperation.Security); } } } From 305c3cab5b18ece47f50547315de745e15397876 Mon Sep 17 00:00:00 2001 From: Paul Rijneveld Date: Fri, 18 Apr 2025 09:49:40 +0200 Subject: [PATCH 3/3] Review: remove unneeded list creation from list --- src/Microsoft.OpenApi/Reader/V2/OpenApiOperationDeserializer.cs | 2 +- .../Reader/V31/OpenApiOperationDeserializer.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.OpenApi/Reader/V2/OpenApiOperationDeserializer.cs b/src/Microsoft.OpenApi/Reader/V2/OpenApiOperationDeserializer.cs index 7366ce976..b7bd18564 100644 --- a/src/Microsoft.OpenApi/Reader/V2/OpenApiOperationDeserializer.cs +++ b/src/Microsoft.OpenApi/Reader/V2/OpenApiOperationDeserializer.cs @@ -98,7 +98,7 @@ internal static partial class OpenApiV2Deserializer "security", (o, n, t) => { if (n.JsonNode is JsonArray) { - o.Security = new List(n.CreateList(LoadSecurityRequirement, t)); + o.Security = n.CreateList(LoadSecurityRequirement, t); } } }, }; diff --git a/src/Microsoft.OpenApi/Reader/V31/OpenApiOperationDeserializer.cs b/src/Microsoft.OpenApi/Reader/V31/OpenApiOperationDeserializer.cs index e14d34ffc..d04e91750 100644 --- a/src/Microsoft.OpenApi/Reader/V31/OpenApiOperationDeserializer.cs +++ b/src/Microsoft.OpenApi/Reader/V31/OpenApiOperationDeserializer.cs @@ -100,7 +100,7 @@ internal static partial class OpenApiV31Deserializer { if (n.JsonNode is JsonArray) { - o.Security = new List(n.CreateList(LoadSecurityRequirement, t)); + o.Security = n.CreateList(LoadSecurityRequirement, t); } } },