Skip to content

Commit

Permalink
fix: failing unit tests for security scheme references in security re…
Browse files Browse the repository at this point in the history 
…quirements

Signed-off-by: Vincent Biret <vibiret@microsoft.com>
  • Loading branch information
@baywet
baywet committed Jan 29, 2025
1 parent 837f000 commit d2e4111
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 79 deletions.
76 changes: 23 additions & 53 deletions src/Microsoft.OpenApi/Models/OpenApiSecurityRequirement.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.OpenApi.Interfaces;
using Microsoft.OpenApi.Models.Interfaces;
using Microsoft.OpenApi.Models.References;
Expand All @@ -18,7 +19,7 @@ namespace Microsoft.OpenApi.Models
/// then the value is a list of scope names required for the execution.
/// For other security scheme types, the array MUST be empty.
/// </summary>
public class OpenApiSecurityRequirement : Dictionary<IOpenApiSecurityScheme, IList<string>>,
public class OpenApiSecurityRequirement : Dictionary<OpenApiSecuritySchemeReference, IList<string>>,
IOpenApiSerializable
{
/// <summary>
Expand All @@ -36,40 +37,42 @@ public OpenApiSecurityRequirement()
/// </summary>
public void SerializeAsV31(IOpenApiWriter writer)
{
SerializeInternal(writer, (writer, element) => element.SerializeAsV31(writer));
SerializeInternal(writer, (w, s) => w.WritePropertyName(s.Reference.ReferenceV3));
}

/// <summary>
/// Serialize <see cref="OpenApiSecurityRequirement"/> to Open Api v3.0
/// </summary>
public void SerializeAsV3(IOpenApiWriter writer)
{
SerializeInternal(writer, (writer, element) => element.SerializeAsV3(writer));
SerializeInternal(writer, (w, s) => w.WritePropertyName(s.Reference.ReferenceV3));
}

/// <summary>
/// Serialize <see cref="OpenApiSecurityRequirement"/>
/// </summary>
private void SerializeInternal(IOpenApiWriter writer, Action<IOpenApiWriter, IOpenApiSerializable> callback)
private void SerializeInternal(IOpenApiWriter writer, Action<IOpenApiWriter, OpenApiSecuritySchemeReference> callback)
{
Utils.CheckArgumentNull(writer);;
Utils.CheckArgumentNull(writer);

// Reaching this point means the reference to a specific OpenApiSecurityScheme fails.
// We are not able to serialize this SecurityScheme/Scopes key value pair since we do not know what
// string to output.
var validPairs = this.Where(static p => p.Key?.Target is not null).ToArray();

if (validPairs.Length == 0)
{
return;
}

writer.WriteStartObject();

foreach (var securitySchemeAndScopesValuePair in this)
foreach (var securitySchemeAndScopesValuePair in validPairs)
{
var securityScheme = securitySchemeAndScopesValuePair.Key;
var scopes = securitySchemeAndScopesValuePair.Value;

if (securityScheme is not OpenApiSecuritySchemeReference schemeReference || schemeReference.Reference is null)
{
// Reaching this point means the reference to a specific OpenApiSecurityScheme fails.
// We are not able to serialize this SecurityScheme/Scopes key value pair since we do not know what
// string to output.
continue;
}

writer.WritePropertyName(schemeReference.Reference.ReferenceV3);
callback(writer, securityScheme);

writer.WriteStartArray();

Expand All @@ -89,48 +92,19 @@ private void SerializeInternal(IOpenApiWriter writer, Action<IOpenApiWriter, IOp
/// </summary>
public void SerializeAsV2(IOpenApiWriter writer)
{
Utils.CheckArgumentNull(writer);;

writer.WriteStartObject();

foreach (var securitySchemeAndScopesValuePair in this)
{
var securityScheme = securitySchemeAndScopesValuePair.Key;
var scopes = securitySchemeAndScopesValuePair.Value;

if (securityScheme is not OpenApiSecuritySchemeReference schemeReference || schemeReference.Reference is null)
{
// Reaching this point means the reference to a specific OpenApiSecurityScheme fails.
// We are not able to serialize this SecurityScheme/Scopes key value pair since we do not know what
// string to output.
continue;
}

securityScheme.SerializeAsV2(writer);

writer.WriteStartArray();

foreach (var scope in scopes)
{
writer.WriteValue(scope);
}

writer.WriteEndArray();
}

writer.WriteEndObject();
SerializeInternal(writer, (w, s) => s.SerializeAsV2(w));
}

/// <summary>
/// Comparer for OpenApiSecurityScheme that only considers the Id in the Reference
/// (i.e. the string that will actually be displayed in the written document)
/// </summary>
private sealed class OpenApiSecuritySchemeReferenceEqualityComparer : IEqualityComparer<IOpenApiSecurityScheme>
private sealed class OpenApiSecuritySchemeReferenceEqualityComparer : IEqualityComparer<OpenApiSecuritySchemeReference>
{
/// <summary>
/// Determines whether the specified objects are equal.
/// </summary>
public bool Equals(IOpenApiSecurityScheme x, IOpenApiSecurityScheme y)
public bool Equals(OpenApiSecuritySchemeReference x, OpenApiSecuritySchemeReference y)
{
if (x == null && y == null)
{
Expand All @@ -148,17 +122,13 @@ public bool Equals(IOpenApiSecurityScheme x, IOpenApiSecurityScheme y)
/// <summary>
/// Returns a hash code for the specified object.
/// </summary>
public int GetHashCode(IOpenApiSecurityScheme obj)
public int GetHashCode(OpenApiSecuritySchemeReference obj)
{
if (obj is null)
{
return 0;
}
else if (obj is OpenApiSecuritySchemeReference reference)
{
return string.IsNullOrEmpty(reference?.Reference?.Id) ? 0 : reference.Reference.Id.GetHashCode();
}
return obj.GetHashCode();
return string.IsNullOrEmpty(obj?.Reference?.Id) ? 0 : obj.Reference.Id.GetHashCode();
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.OpenApi/Models/References/BaseOpenApiReferenceHolder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public T Target
{
get
{
_target ??= Reference.HostDocument.ResolveReferenceTo<T>(Reference);
_target ??= Reference.HostDocument?.ResolveReferenceTo<T>(Reference);
return _target;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.OpenApi/Reader/V2/OpenApiSecurityRequirementDeserializer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public static OpenApiSecurityRequirement LoadSecurityRequirement(ParseNode node,
return securityRequirement;
}

private static IOpenApiSecurityScheme LoadSecuritySchemeByReference(
private static OpenApiSecuritySchemeReference LoadSecuritySchemeByReference(
OpenApiDocument openApiDocument,
string schemeName)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.OpenApi/Reader/V3/OpenApiSecurityRequirementDeserializer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public static OpenApiSecurityRequirement LoadSecurityRequirement(ParseNode node,
return securityRequirement;
}

private static IOpenApiSecurityScheme LoadSecuritySchemeByReference(
private static OpenApiSecuritySchemeReference LoadSecuritySchemeByReference(
OpenApiDocument openApiDocument,
string schemeName)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.OpenApi/Reader/V31/OpenApiSecurityRequirementDeserializer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public static OpenApiSecurityRequirement LoadSecurityRequirement(ParseNode node,
return securityRequirement;
}

private static IOpenApiSecurityScheme LoadSecuritySchemeByReference(string schemeName, OpenApiDocument hostDocument)
private static OpenApiSecuritySchemeReference LoadSecuritySchemeByReference(string schemeName, OpenApiDocument hostDocument)
{
return new OpenApiSecuritySchemeReference(schemeName, hostDocument);
}
Expand Down
8 changes: 4 additions & 4 deletions test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiDocumentTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -889,8 +889,8 @@ public async Task ParseModifiedPetStoreDocumentWithTagAndSecurityShouldSucceed()
{
new OpenApiSecurityRequirement
{
[securityScheme1] = new List<string>(),
[securityScheme2] = new List<string>
[new OpenApiSecuritySchemeReference(securityScheme1, "securitySchemeName1")] = new List<string>(),
[new OpenApiSecuritySchemeReference(securityScheme2, "securitySchemeName2")] = new List<string>
{
"scope1",
"scope2"
Expand Down Expand Up @@ -1035,8 +1035,8 @@ public async Task ParseModifiedPetStoreDocumentWithTagAndSecurityShouldSucceed()
{
new OpenApiSecurityRequirement
{
[securityScheme1] = new List<string>(),
[securityScheme2] = new List<string>
[new OpenApiSecuritySchemeReference(securityScheme1, "securitySchemeName1")] = new List<string>(),
[new OpenApiSecuritySchemeReference(securityScheme2, "securitySchemeName2")] = new List<string>
{
"scope1",
"scope2",
Expand Down
4 changes: 2 additions & 2 deletions test/Microsoft.OpenApi.Tests/Models/OpenApiOperationTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,8 @@ public class OpenApiOperationTests
{
new()
{
[new OpenApiSecuritySchemeReference("securitySchemeId1", hostDocument: null)] = new List<string>(),
[new OpenApiSecuritySchemeReference("securitySchemeId2", hostDocument: null)] = new List<string>
[new OpenApiSecuritySchemeReference(new OpenApiSecurityScheme(), "securitySchemeId1")] = new List<string>(),
[new OpenApiSecuritySchemeReference(new OpenApiSecurityScheme(), "securitySchemeId2")] = new List<string>
{
"scopeName1",
"scopeName2"
Expand Down
28 changes: 12 additions & 16 deletions test/Microsoft.OpenApi.Tests/Models/OpenApiSecurityRequirementTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,49 +25,45 @@ public class OpenApiSecurityRequirementTests
new()
{
[
new OpenApiSecuritySchemeReference("scheme1", hostDocument: null)
new OpenApiSecuritySchemeReference(new OpenApiSecurityScheme(), "scheme1")
] = new List<string>
{
"scope1",
"scope2",
"scope3",
},
[
new OpenApiSecuritySchemeReference("scheme2", hostDocument: null)
new OpenApiSecuritySchemeReference(new OpenApiSecurityScheme(), "scheme2")
] = new List<string>
{
"scope4",
"scope5",
},
[
new OpenApiSecuritySchemeReference("scheme3", hostDocument: null)
new OpenApiSecuritySchemeReference(new OpenApiSecurityScheme(), "scheme3")
] = new List<string>()
};

public static OpenApiSecurityRequirement SecurityRequirementWithUnreferencedSecurityScheme =
new()
{
[
new OpenApiSecuritySchemeReference("scheme1", hostDocument: null)
new OpenApiSecuritySchemeReference(new OpenApiSecurityScheme(), "scheme1")
] = new List<string>
{
"scope1",
"scope2",
"scope3",
},
[
new OpenApiSecurityScheme()
{
// This security scheme is unreferenced, so this key value pair cannot be serialized.
Name = "brokenUnreferencedScheme"
}
new OpenApiSecuritySchemeReference("brokenUnreferencedScheme", hostDocument: null)
] = new List<string>
{
"scope4",
"scope5",
},
[
new OpenApiSecuritySchemeReference("scheme3", hostDocument: null)
new OpenApiSecuritySchemeReference(new OpenApiSecurityScheme(), "scheme3")
] = new List<string>()
};

Expand Down Expand Up @@ -246,13 +242,13 @@ public void SchemesShouldConsiderOnlyReferenceIdForEquality()
};

// Act
securityRequirement.Add(securityScheme1, new List<string>());
securityRequirement.Add(securityScheme2, new List<string> { "scope1", "scope2" });
securityRequirement.Add(new OpenApiSecuritySchemeReference(securityScheme1, "securityScheme1"), new List<string>());
securityRequirement.Add(new OpenApiSecuritySchemeReference(securityScheme2, "securityScheme2"), new List<string> { "scope1", "scope2" });

var addSecurityScheme1Duplicate = () =>
securityRequirement.Add(securityScheme1Duplicate, new List<string>());
securityRequirement.Add(new OpenApiSecuritySchemeReference(securityScheme1Duplicate, "securityScheme1"), new List<string>());
var addSecurityScheme1WithDifferentProperties = () =>
securityRequirement.Add(securityScheme1WithDifferentProperties, new List<string>());
securityRequirement.Add(new OpenApiSecuritySchemeReference(securityScheme1WithDifferentProperties, "securityScheme1"), new List<string>());

// Assert
// Only the first two should be added successfully since the latter two are duplicates of securityScheme1.
Expand All @@ -267,8 +263,8 @@ public void SchemesShouldConsiderOnlyReferenceIdForEquality()
{
// This should work with any security scheme object
// as long as Reference.Id os securityScheme1
[securityScheme1WithDifferentProperties] = new List<string>(),
[securityScheme2] = new List<string> { "scope1", "scope2" },
[new OpenApiSecuritySchemeReference(securityScheme1WithDifferentProperties, "securityScheme1")] = new List<string>(),
[new OpenApiSecuritySchemeReference(securityScheme2, "securityScheme2")] = new List<string> { "scope1", "scope2" },
});
}
}
Expand Down

0 comments on commit d2e4111

Please sign in to comment.