-
Notifications
You must be signed in to change notification settings - Fork 282
feat(securityscheme): add oauth2MetadataUrl support (OpenAPI 3.2) #2706
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 |
|---|---|---|
|
|
@@ -288,6 +288,7 @@ namespace Microsoft.OpenApi | |
| Microsoft.OpenApi.OpenApiOAuthFlows? Flows { get; } | ||
| Microsoft.OpenApi.ParameterLocation? In { get; } | ||
| string? Name { get; } | ||
| System.Uri? OAuth2MetadataUrl { get; } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This effectively leaves us with the following choices:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CC @martincostello in case you have a better suggestion here
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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 You could potentially avoid that by making the new interface conditionally compiled for
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
|
||
| System.Uri? OpenIdConnectUrl { get; } | ||
| string? Scheme { get; } | ||
| Microsoft.OpenApi.SecuritySchemeType? Type { get; } | ||
|
|
@@ -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"; | ||
|
|
@@ -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; } | ||
|
|
@@ -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; } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.