-
Notifications
You must be signed in to change notification settings - Fork 9.1k
Security definition documentation updates #818
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
Conversation
Corrected RFC reference. Clarified that URLs can be relative. Fixed basic auth example.
@darrelmiller looks good to me! |
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.
Looks good but I have some minor formatting/phrasing suggestions.
<a name="securitySchemeScheme"></a>scheme | `string` | `http` | **Required.** The name of the HTTP Authorization scheme to be used in the Authorization header as per RFC 7234. | ||
<a name="securitySchemeBearerFormat"></a>bearerFormat | `string` | `http` (`"bearer"`) | A hint to the client to identify how the bearer token should be formatted. | ||
<a name="securitySchemeOpenIdConnectUrl"></a>openIdConnectUrl | `string` | `openIdConnect` | **Required.** OpenId Connect URL to discover OAuth2 configuration values. | ||
<a name="securitySchemeScheme"></a>scheme | `string` | `http` | **Required.** The name of the HTTP Authorization scheme to be used in the Authorization header as per [RFC 7235](https://tools.ietf.org/html/rfc7235). |
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.
or link to the section:
the [Authorization header of RFC 7235](https://tools.ietf.org/html/rfc7235#section-4.2)
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.
Done
<a name="securitySchemeFlow"></a>flow | `string` | `oauth2` | **Required.** The flow used by the OAuth2 security scheme. Valid values are `"implicit"`, `"password"`, `"application"` or `"accessCode"`. | ||
<a name="securitySchemeAuthorizationUrl"></a>authorizationUrl | `string` | `oauth2` (`"implicit"`, `"accessCode"`) | **Required.** The authorization URL to be used for this flow. This SHOULD be in the form of a URL. | ||
<a name="securitySchemeTokenUrl"></a>tokenUrl | `string` | `oauth2` (`"password"`, `"application"`, `"accessCode"`) | **Required.** The token URL to be used for this flow. This SHOULD be in the form of a URL. | ||
<a name="securitySchemeAuthorizationUrl"></a>authorizationUrl | `string` | `oauth2` (`"implicit"`, `"accessCode"`) | **Required.** The authorization URL to be used for this flow. This SHOULD be in the form of a URL. Url may be relative to location of OpenAPI document. |
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.
Reads a little awkward an uses Url as a work, not an acronym. I suggest either
"The authorizationUrl
..." or "The URL
..."
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.
Removed the bit about relative URLs. Going to try and handle it globally.
<a name="securitySchemeAuthorizationUrl"></a>authorizationUrl | `string` | `oauth2` (`"implicit"`, `"accessCode"`) | **Required.** The authorization URL to be used for this flow. This SHOULD be in the form of a URL. | ||
<a name="securitySchemeTokenUrl"></a>tokenUrl | `string` | `oauth2` (`"password"`, `"application"`, `"accessCode"`) | **Required.** The token URL to be used for this flow. This SHOULD be in the form of a URL. | ||
<a name="securitySchemeAuthorizationUrl"></a>authorizationUrl | `string` | `oauth2` (`"implicit"`, `"accessCode"`) | **Required.** The authorization URL to be used for this flow. This SHOULD be in the form of a URL. Url may be relative to location of OpenAPI document. | ||
<a name="securitySchemeTokenUrl"></a>tokenUrl | `string` | `oauth2` (`"password"`, `"application"`, `"accessCode"`) | **Required.** The token URL to be used for this flow. This SHOULD be in the form of a URL. Url may be relative to location of OpenAPI document. |
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.
As above, I suggest either "The authorizationUrl
..." or "The URL
..."
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.
Removed.
@darrelmiller LGTM (XL) |
<a name="securitySchemeScheme"></a>scheme | `string` | `http` | **Required.** The name of the HTTP Authorization scheme to be used in the Authorization header as per [RFC 7235](https://tools.ietf.org/html/rfc7235). | ||
<a name="securitySchemeBearerFormat"></a>bearerFormat | `string` | `http` (`"bearer"`) | A hint to the client to identify how the bearer token is formatted. Bearer tokens are usually generated by an authorization server, so this information is primarily for documentation purposes. | ||
<a name="securitySchemeFlow"></a>flow | [OAuth Flows Object](#oauthFlowsObject) | `oauth2` | **Required.** An object containing configuration information for the flow types supported. | ||
<a name="securitySchemeOpenIdConnectUrl"></a>openIdConnectUrl | `string` | `openIdConnect` | **Required.** OpenId Connect URL to discover OAuth2 configuration values. Url may be relative to location of OpenAPI document. |
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.
Minor typo: Url
needs all-caps.
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.
Fixed via DELETION :-)
TDC voted for merge, @darrelmiller will fix conflicts and merge when ready. |
@darrelmiller - see if you can fix the conflicts and check @earth2marsh's comment before the call, and we could even merge it live! |
…dm/securityreview # Conflicts: # versions/3.0.md
URLs will change from SHOULD to MUST and then merged 🎉 |
Security definition documentation updates
No description provided.