Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ public interface IOpenApiSecurityScheme : IOpenApiDescribedElement, IOpenApiRead
/// </summary>
public Uri? OpenIdConnectUrl { get; }

/// <summary>
/// URL to the OAuth2 Authorization Server Metadata document (RFC 8414).
/// Note: This field is supported in OpenAPI 3.2.0+ only.
/// </summary>
public Uri? OAuth2MetadataUrl { get; }

/// <summary>
/// Specifies that a security scheme is deprecated and SHOULD be transitioned out of usage.
/// Note: This field is supported in OpenAPI 3.2.0+. For earlier versions, it will be serialized as x-oai-deprecated extension.
Expand Down
5 changes: 5 additions & 0 deletions src/Microsoft.OpenApi/Models/OpenApiConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,11 @@ public static class OpenApiConstants
/// </summary>
public const string Flows = "flows";

/// <summary>
/// Field: Oauth2MetadataUrl
/// </summary>
public const string OAuth2MetadataUrl = "oauth2MetadataUrl";

/// <summary>
/// Field: OpenIdConnectUrl
/// </summary>
Expand Down
13 changes: 13 additions & 0 deletions src/Microsoft.OpenApi/Models/OpenApiSecurityScheme.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ public class OpenApiSecurityScheme : IOpenApiExtensible, IOpenApiSecurityScheme
/// <inheritdoc/>
public Uri? OpenIdConnectUrl { get; set; }

/// <inheritdoc/>
public Uri? OAuth2MetadataUrl { get; set; }

/// <inheritdoc/>
public bool Deprecated { get; set; }

Expand All @@ -60,6 +63,7 @@ internal OpenApiSecurityScheme(IOpenApiSecurityScheme securityScheme)
BearerFormat = securityScheme.BearerFormat ?? BearerFormat;
Flows = securityScheme.Flows != null ? new(securityScheme.Flows) : null;
OpenIdConnectUrl = securityScheme.OpenIdConnectUrl != null ? new Uri(securityScheme.OpenIdConnectUrl.OriginalString, UriKind.RelativeOrAbsolute) : null;
OAuth2MetadataUrl = securityScheme.OAuth2MetadataUrl != null ? new Uri(securityScheme.OAuth2MetadataUrl.OriginalString, UriKind.RelativeOrAbsolute) : null;
Deprecated = securityScheme.Deprecated;
Extensions = securityScheme.Extensions != null ? new Dictionary<string, IOpenApiExtension>(securityScheme.Extensions) : null;
}
Expand Down Expand Up @@ -118,7 +122,16 @@ private void SerializeInternal(IOpenApiWriter writer, OpenApiSpecVersion version
break;
case SecuritySchemeType.OAuth2:
// This property apply to oauth2 type only.
// oauth2MetadataUrl
// flows
if (version >= OpenApiSpecVersion.OpenApi3_2)
{
writer.WriteProperty(OpenApiConstants.OAuth2MetadataUrl, OAuth2MetadataUrl?.ToString());
}
else
{
writer.WriteProperty("x-oauth2-metadata-url", OAuth2MetadataUrl?.ToString());
}
writer.WriteOptionalObject(OpenApiConstants.Flows, Flows, callback);
break;
case SecuritySchemeType.OpenIdConnect:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ public string? Description
/// <inheritdoc/>
public Uri? OpenIdConnectUrl { get => Target?.OpenIdConnectUrl; }

/// <inheritdoc/>
public Uri? OAuth2MetadataUrl { get => Target?.OAuth2MetadataUrl; }

/// <inheritdoc/>
public IDictionary<string, IOpenApiExtension>? Extensions { get => Target?.Extensions; }

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license.

using System;
Expand Down Expand Up @@ -68,6 +68,16 @@ internal static partial class OpenApiV32Deserializer
}
}
},
{
"oauth2MetadataUrl", (o, n, _) =>
{
var metadataUrl = n.GetScalarValue();
if (metadataUrl != null)
{
o.OAuth2MetadataUrl = new(metadataUrl, UriKind.RelativeOrAbsolute);
}
}
},
{
"flows", (o, n, t) =>
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,36 @@ public async Task ParseOAuth2SecuritySchemeShouldSucceed()
}, securityScheme);
}

[Fact]
public async Task ParseOAuth2SecuritySchemeWithMetadataUrlShouldSucceed()
{
// Act
var securityScheme = await OpenApiModelFactory.LoadAsync<OpenApiSecurityScheme>(
Path.Combine(SampleFolderPath, "oauth2SecuritySchemeWithMetadataUrl.yaml"),
OpenApiSpecVersion.OpenApi3_2,
new(),
SettingsFixture.ReaderSettings);

// Assert
Assert.Equivalent(
new OpenApiSecurityScheme
{
Type = SecuritySchemeType.OAuth2,
OAuth2MetadataUrl = new Uri("https://idp.example.com/.well-known/oauth-authorization-server"),
Flows = new OpenApiOAuthFlows
{
ClientCredentials = new OpenApiOAuthFlow
{
TokenUrl = new Uri("https://idp.example.com/oauth/token"),
Scopes = new System.Collections.Generic.Dictionary<string, string>
{
["scope:one"] = "Scope one"
}
}
}
}, securityScheme);
}

[Fact]
public async Task ParseOpenIdConnectSecuritySchemeShouldSucceed()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.2.0.md#securitySchemeObject
type: oauth2
oauth2MetadataUrl: https://idp.example.com/.well-known/oauth-authorization-server
flows:
clientCredentials:
tokenUrl: https://idp.example.com/oauth/token
scopes:
scope:one: Scope one
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,24 @@ public class OpenApiSecuritySchemeTests
}
};

private static OpenApiSecurityScheme OAuth2MetadataSecurityScheme => new()
{
Description = "description1",
Type = SecuritySchemeType.OAuth2,
OAuth2MetadataUrl = new("https://idp.example.com/.well-known/oauth-authorization-server"),
Flows = new()
{
ClientCredentials = new()
{
TokenUrl = new("https://idp.example.com/oauth/token"),
Scopes = new Dictionary<string, string>
{
["scope:one"] = "Scope one"
}
}
}
};

private static OpenApiSecurityScheme OpenIdConnectSecurityScheme => new()
{
Description = "description1",
Expand Down Expand Up @@ -257,6 +275,62 @@ public async Task SerializeOAuthSingleFlowSecuritySchemeAsV3JsonWorks()
Assert.Equal(expected, actual);
}

[Fact]
public async Task SerializeOAuthSecuritySchemeWithMetadataUrlAsV32JsonWorks()
{
// Arrange
var expected =
"""
{
"type": "oauth2",
"description": "description1",
"oauth2MetadataUrl": "https://idp.example.com/.well-known/oauth-authorization-server",
"flows": {
"clientCredentials": {
"tokenUrl": "https://idp.example.com/oauth/token",
"scopes": {
"scope:one": "Scope one"
}
}
}
}
""";

// Act
var actual = await OAuth2MetadataSecurityScheme.SerializeAsJsonAsync(OpenApiSpecVersion.OpenApi3_2);

// Assert
Assert.True(JsonNode.DeepEquals(JsonNode.Parse(expected), JsonNode.Parse(actual)));
}

[Fact]
public async Task SerializeOAuthSecuritySchemeWithMetadataUrlAsV31JsonOmitsMetadataUrl()
{
// Arrange
var expected =
"""
{
"type": "oauth2",
"description": "description1",
"x-oauth2-metadata-url": "https://idp.example.com/.well-known/oauth-authorization-server",
"flows": {
"clientCredentials": {
"tokenUrl": "https://idp.example.com/oauth/token",
"scopes": {
"scope:one": "Scope one"
}
}
}
}
""";

// Act
var actual = await OAuth2MetadataSecurityScheme.SerializeAsJsonAsync(OpenApiSpecVersion.OpenApi3_1);

// Assert
Assert.True(JsonNode.DeepEquals(JsonNode.Parse(expected), JsonNode.Parse(actual)));
}

[Fact]
public async Task SerializeOAuthMultipleFlowSecuritySchemeAsV3JsonWorks()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ namespace Microsoft.OpenApi
Microsoft.OpenApi.OpenApiOAuthFlows? Flows { get; }
Microsoft.OpenApi.ParameterLocation? In { get; }
string? Name { get; }
System.Uri? OAuth2MetadataUrl { get; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is technically source breaking. Even though we recommend not to take a dependency on those interfaces. This still poses a problem with the transitive dependency aspect of things. I.E. if a dependency like aspnetcore.openapi or swashbuckle is accessing other properties in the DOM of type IOpenApiSecurityScheme and have not re-compiled after we ship that change, they'll get exceptions like MethodNotFound/ or that the interface is not implemented by the types that actually implement it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One way we could try to mitigate this is through Default Interface Implementations, but they are only supported for net core 3.0 onwards. And we currently ship netstandard2.0 and net8.0. We'd need to figure out something for netstandard2.0

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And even if we added the default implementation, it'd still remain binary breaking.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This effectively leaves us with the following choices:

  • major rev, and the cost associated with supporting v3.
  • major rev, and not support v3, only a valid approach as long as net11 does not have support for v3. And we should ensure we don't have any other breaking changes that need to be slated.
  • decline this PR, and have an incomplete DOM.
  • revert the interface change, but keep the rest. That degrades the experience quite a bit forcing type assertions/casts to access the property. And we'd clean this up in a future major version (OpenAPI 3.3?)
  • introduce an additional interface to facilitate the type assertions. (variation of the above)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CC @martincostello in case you have a better suggestion here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Introduce this via a new interface instead?

Thinking out loud, you could add an IOAuth2MetadataProvider interface:

public interface IOAuth2MetadataProvider
{
    Uri? OAuth2MetadataUrl { get; }
}

Then implement that interface on whatever concrete types need it, and require users to type cast to test for it for implementations that care about it:

if (thing is IOAuth2MetadataProvider oauth2Metadata)
{
    // whatever
}

As already noted, adding it to other interfaces in their inheritance chain is source breaking and DIMs are problematic for TFMs that are too old to support it (i.e. for .NET Framework via netstandard2.0).

You could potentially avoid that by making the new interface conditionally compiled for #if NET and use DIMs, but that would make the API surface non-symmetrical for different TFMs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the additional information.

Yes that was the last option I was listing even though I didn't explicitly document it. Let's go with that then.

@mdaneri, would you mind introducing a new interface please? Additionally please:

  • leave a super explicit doc comment this is a "temporary" interface.
  • leave a todo comment to remove and collapse the interface with the existing one on the next major version.
  • update the doc comment of the existing interface to point to this one. Example with type assertions would be nice as well. So people are not surprised by this pattern, know that's mostly because we (Microsoft) messed up, and know where to find the missing property.

System.Uri? OpenIdConnectUrl { get; }
string? Scheme { get; }
Microsoft.OpenApi.SecuritySchemeType? Type { get; }
Expand Down Expand Up @@ -538,6 +539,7 @@ namespace Microsoft.OpenApi
public const string Null = "null";
public const string Nullable = "nullable";
public const string NullableExtension = "x-nullable";
public const string OAuth2MetadataUrl = "oauth2MetadataUrl";
public const string OneOf = "oneOf";
public const string OpenApi = "openapi";
public const string OpenIdConnectUrl = "openIdConnectUrl";
Expand Down Expand Up @@ -1381,6 +1383,7 @@ namespace Microsoft.OpenApi
public Microsoft.OpenApi.OpenApiOAuthFlows? Flows { get; set; }
public Microsoft.OpenApi.ParameterLocation? In { get; set; }
public string? Name { get; set; }
public System.Uri? OAuth2MetadataUrl { get; set; }
public System.Uri? OpenIdConnectUrl { get; set; }
public string? Scheme { get; set; }
public Microsoft.OpenApi.SecuritySchemeType? Type { get; set; }
Expand All @@ -1400,6 +1403,7 @@ namespace Microsoft.OpenApi
public Microsoft.OpenApi.OpenApiOAuthFlows? Flows { get; }
public Microsoft.OpenApi.ParameterLocation? In { get; }
public string? Name { get; }
public System.Uri? OAuth2MetadataUrl { get; }
public System.Uri? OpenIdConnectUrl { get; }
public string? Scheme { get; }
public Microsoft.OpenApi.SecuritySchemeType? Type { get; }
Expand Down
Loading