-
Notifications
You must be signed in to change notification settings - Fork 285
Fixed security requirement resolution #226
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
Changes from all commits
16ec3b1
1475af6
8957d1e
dab4ddb
777cc97
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 |
|---|---|---|
|
|
@@ -44,6 +44,7 @@ public void Walk(OpenApiDocument doc) | |
| Walk(OpenApiConstants.Servers, () => Walk(doc.Servers)); | ||
| Walk(OpenApiConstants.Paths, () => Walk(doc.Paths)); | ||
| Walk(OpenApiConstants.Components, () => Walk(doc.Components)); | ||
| Walk(OpenApiConstants.Security, () => Walk(doc.SecurityRequirements)); | ||
| Walk(OpenApiConstants.ExternalDocs, () => Walk(doc.ExternalDocs)); | ||
| Walk(OpenApiConstants.Tags, () => Walk(doc.Tags)); | ||
| Walk(doc as IOpenApiExtensible); | ||
|
|
@@ -471,10 +472,32 @@ internal void Walk(OpenApiOperation operation) | |
| Walk(OpenApiConstants.Responses, () => Walk(operation.Responses)); | ||
| Walk(OpenApiConstants.Callbacks, () => Walk(operation.Callbacks)); | ||
| Walk(OpenApiConstants.Tags, () => Walk(operation.Tags)); | ||
|
|
||
| Walk(OpenApiConstants.Security, () => Walk(operation.Security)); | ||
|
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. If we miss add the process code, it should not be failed. So, I can't understand why? Is there any default setting that we can fix it?
Member
Author
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. I'm not sure I follow. If we are missing Walk() methods then there may be references that remain unresolved or objects not validated. Technically that's not an error as Unresolved references is a valid state. They could be resolved manually later. Yes, there has been an unfortunate number of cases of missing resolved references. In theory these should have been picked up by regular test cases as unresolved references have null/default property values.. Not quite sure why more were not. |
||
| Walk(operation as IOpenApiExtensible); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Visits list of <see cref="OpenApiSecurityRequirement"/> | ||
| /// </summary> | ||
| internal void Walk(IList<OpenApiSecurityRequirement> securityRequirements) | ||
| { | ||
| if (securityRequirements == null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| _visitor.Visit(securityRequirements); | ||
|
|
||
| if (securityRequirements != null) | ||
| { | ||
| for (int i = 0; i < securityRequirements.Count; i++) | ||
| { | ||
| Walk(i.ToString(), () => Walk(securityRequirements[i])); | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
| /// <summary> | ||
| /// Visits list of <see cref="OpenApiParameter"/> | ||
| /// </summary> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT license. | ||
|
|
||
| using System.IO; | ||
| using System.Linq; | ||
| using Xunit; | ||
|
|
||
| namespace Microsoft.OpenApi.Readers.Tests.V3Tests | ||
| { | ||
| public class OpenApiOperationTests | ||
| { | ||
| private const string SampleFolderPath = "V3Tests/Samples/OpenApiOperation/"; | ||
|
|
||
| [Fact] | ||
| public void OperationWithSecurityRequirementShouldReferenceSecurityScheme() | ||
| { | ||
| using (var stream = Resources.GetStream(Path.Combine(SampleFolderPath, "securedOperation.yaml"))) | ||
| { | ||
| var openApiDoc = new OpenApiStreamReader().Read(stream, out var diagnostic); | ||
|
|
||
| var securityRequirement = openApiDoc.Paths["/"].Operations[Models.OperationType.Get].Security.First(); | ||
|
|
||
| Assert.Same(securityRequirement.Keys.First(), openApiDoc.Components.SecuritySchemes.First().Value); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| openapi: 3.0.0 | ||
| info: | ||
| title: Example of Security Requirement referencing a security scheme | ||
| version: 1.0.0 | ||
| paths: {} | ||
| security: | ||
| - basicAuth: [] | ||
| components: | ||
| securitySchemes: | ||
| basicAuth: | ||
| type: http | ||
| scheme: basic |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| openapi: 3.0.0 | ||
| info: | ||
| title: Example of Security Requirement referencing a security scheme | ||
| version: 1.0.0 | ||
| paths: | ||
| '/': | ||
| get: | ||
| security: | ||
| - basicAuth: [] | ||
| responses: | ||
| '200': | ||
| description: OK | ||
| components: | ||
| securitySchemes: | ||
| basicAuth: | ||
| type: http | ||
| scheme: basic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the comments should be "SecurityRequirement" not "SecuritySchemes" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually no. SecurityRequirement is a dictionary whose keys are SecuritySchemes. It is those keys that are being replaced if they are unresolved references.