-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
Add Oauth1 to Security Scheme Object #1080
Conversation
This adds in the security definitions for Oauth1. It follows for from the discussions in OAI#61. It is similar to the way RAML handles it's description of Oauth1 as well.
Add Oauth1 to Security Scheme Object
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.
Nits
versions/3.0.md
Outdated
@@ -3369,19 +3369,24 @@ animals: | |||
#### <a name="securitySchemeObject"></a>Security Scheme Object | |||
|
|||
Allows the definition of a security scheme that can be used by the operations. | |||
Supported schemes are HTTP authentication, an API key (either as a header or as a query parameter) and OAuth2's common flows (implicit, password, application and access code). | |||
Supported schemes are HTTP authentication, an API key (either as a header or as a query parameter), OAuth2's common flows (implicit, password, application and access code) and Oauth1. |
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.
Capitalise as OAuth1 ?
versions/3.0.md
Outdated
<a name="securitySchemeAuthorizationUrl"></a>authorizationUrl | `string` | `oauth1` | **Required.** Resource Owner Authorization URL to send the user to the server to authorize the request. This MUST be in the form of a URL. | ||
<a name="securitySchemeTokenUrl"></a>tokenUrl | `string` | `oauth1` | **Required.** Token Credentials URL to obtain a set of token credentials from the server. This MUST be in the form of a URL. | ||
<a name="securitySchemeRequestUrl"></a>requestUrl | `string` | `oauth1` | **Required.** Temporary Credentials URL to obtain a set of temporary credentials from the server. This MUST be in the form of a URL. | ||
<a name="signatureMethod"></a>requestUrl | [`string`] | `oauth1` | A list of supported signatures used for authorization. Valid values are `"HMAC-SHA1"`, `"RSA-SHA1"`, or `"PLAINTEXT"`. Default vaule is `"HMAC-SHA1"` |
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.
Cut and paste of requestUrl
should be signatureMethod
. list of supported signatures
-> list of supported signature methods
.
versions/3.0.md
Outdated
@@ -3432,6 +3437,26 @@ type: http | |||
scheme: bearer | |||
bearerFormat: JWT | |||
``` | |||
###### Oauth1 Sample |
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.
Capitalise as OAuth1 ?
Thanks for this PR @starfishmod Are there one or two publicly accessible APIs that you could point to in the comments that currently use OAuth1 so folks could give this some consideration with a real-world example? |
|
I have my own project which adds a swagger generator to WordPress v2 API's - however they provide a OAuth1a plugin for Auth as a lot of sites are http only and Oauth2 is no good if there is no https (AFAICT) https://www.discogs.com/developers/#page:authentication,header:authentication-oauth-flow I'm sure there are lots more, but these are the ones I'm immediately aware of. |
TDC: Agree we want to add support for OAuth1. Should we reference:
@MikeRalphson - Would you be willing to do some research on how to reference OAuth1 in the spec and which version(s) of OAuth1 we can claim the spec supports @raymondfeng, @webron, and @earth2marsh have offered to review when this is ready. |
Is this more for release notes than the spec itself? The spec doesn't seem to make an equivalent reference of any standard or levels of support for OAuth2 (RFC6749/6750) - or is that only because there is no confusion re: oAuth2 versions ? Happy to add links for both if that's what's wanted. |
Thanks @MikeRalphson. IMHO, adding links for both OAuth1 and OAuth2 would be good. Would you please consider doing one PR for OAuth1 (just another commit on this one?) and a separate PR for OAuth2? Thanks again! |
@starfishmod would you prefer for me to add suggested text for OAuth1 version support as review comments, or to add me as a collaborator on your fork so I can add a commit to your PR? |
Feel free to collaborate on my fork :-) I assume that will be more
effective. Thanks for this :-)
…On May 9, 2017 9:03 AM, "Mike Ralphson" ***@***.***> wrote:
@starfishmod <https://github.com/starfishmod> would you prefer for me to
add suggested text for OAuth1 version support as review comments, or to add
me as a collaborator on your fork so I can add a commit to your PR?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1080 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAs6k8GRWBGcUR9wNwV2IZ8lkkwtnK94ks5r359KgaJpZM4NOp9M>
.
|
Other providers: |
Thanks to @raymondfeng who also pointed out passport.js's list of providers, which identifies some various services that use OAuth 1.0a with the |
it is excellent to see oauth 1 being added. I would suggest that some of the fields currently marked as required should not be required. per the oauth 1.0a spec:
based on this, I think that the fields |
Created PR#2 on @starfishmod 's fork adding the link to RFC5849.
|
@notEthan the idea behind being able to describe the security schemes (as well as all other aspects in the spec) is not just to document what you have, but also to be able to automate processes around the API. If you end up creating something that's customized, where do you see the value in being able to define that it's OAuth1? |
well, saying that the API's security is oauth1 is valuable in itself for the client to know. if credentials are exchanged in another method than the one suggested by the spec, it still knows that this is an API where it should use those credentials. while the URL fields are certainly valuable to describe if supported, marking them as required fields when they are part of the spec that that is explicitly described as optional does not seem like the right choice, to me. |
another oauth 1 thing that I think might be valuable to describe would be the OAuth request body hash, specified: https://tools.ietf.org/id/draft-eaton-oauth-bodyhash-00.html APIs that I encounter have differing levels of support for this specification. something like:
|
on the other hand, I can see that including a draft that expired long ago without becoming RFC may not be considered to have much value to most. |
@notEthan If all you need to do advertise that the API is using OAuth1 then just add this security scheme. {
"type": "http",
"scheme": "oauth"
} |
After much debate in multiple @OAI/tdc meetings we have decided not to merge these changes in OAS V3.0.0. Using the new The prevailing feeling of the TDC is that there is sufficient variability in current OAuth1 implementations that attempting to provide explicit support for describing those mechanisms would burden tooling with the challenging job of writing client code that can work with any OAuth1 implementation. We remain open to reconsidering this for future versions. |
@darrelmiller this is disappointing, however could I ask you to expand on this more as I'm not party to the discussions in meetings you mentioned. When you say "variability" in OAuth1a implementations can you please provide an example of what you mean especially in relation to the API's listed previously in this thread. Considering the only variability described is the signature method I am somewhat puzzled by this. Having connected to several Oauth1a providers I have not experienced this variability, however I would like more information and that way I can propose any additions to the spec. You say that this can be handled under the http security scheme, however I strongly disagree with this. Currently in Oauth2 security we provide several settings to handle Authentication endpoints as well as flows etc. This is done because to make it useful we need to know HOW to return the security keys to build the auth header. Oauth1a is similar that unless there are ways to figure out how to get the keys it makes it cumbersome to do so. Speaking of tooling - your statement of tooling support confuses me. Is OAI required to work with certain tools - or does OAI tell the tools what they should support? I also believe that if the tooling is able to support OAuth2 then there should be no logical reason that it cannot add OAuth1a - especially given that libraries tend to abstract complexities of both OAuth types. And lastly why close this issue? If you felt that this couldn't make it into 3.0 why not just postpone this to 3.xx? If you felt you didn't want these variables in the securityScheme Object then maybe they could be moved to OAuth1a "flow" - similar to OAuth2? At this point I also implore you to please provide clear directions of what is required to get OAuth1a into the spec. |
@starfishmod A good example of something that is a concern, is from one of the comments a couple of weeks ago. Here #1080 (comment) I agree that indicating an API is using OAuth1 is not the same as having tooling generate the correct authorization header. I didn't mean to suggest it was. The OpenAPI specification defines a set of functionality. Past experience has shown that tooling that only partially supports the specification is a source of frustration to users. Due to this we want to encourage tooling to fully support the specification. Every feature we add to the OAS is raising the bar for tooling. If we consider the example I referenced earlier, this now means that tooling needs to handle all these scenarios where implementations decided to pick and choose which parts of the OAuth1 spec they were going to use. What is the experience when a user tries to get a OAuth1 token and it fails? Is it bad configuration in the OpenAPI definition? Is it a bug in the tooling? Is the service using a variant of OAuth1 that isn't supported by the tooling? How is a user supposed to figure that out. I have no doubt that tooling can be written that handles some reasonable percent of scenarios using some set of OAuth1 configuration parameters. The question is how much frustration and support issues are we generating for those unsupported scenarios? You asked for clear directions on what is required to get OAuth1 into the spec. I speak only for myself here, but I would hope to see examples of the configuration values for the major OAuth1 protected sites and sample code that generates tokens based on just on the configuration values without special cased code per site. I realize that is a big request, but it is what we are asking of tooling developers if we include OAuth1 as part of the specification. |
Agree with @starfishmod that the original issue (#61) should be re-opened to track the request for this feature for future versions of the spec. |
@MikeRalphson @starfishmod Re-opened issue for tracking in future releases. |
@darrelmiller it has taken me some time to respond. That being said, in the OAuth1a spec (5.2. Consumer Request Parameters) you can send the auth fields as either HTTP Authorization Header or as GET/POST variables. I found that with a lot of (cheap) Wordpress hosting that the Authorization header is stripped out for "security reasons". However the use of POST oauth_* fields works fine. I'm suggesting the addition of a non-required field called authorizationMethod which is an array of header, post and or get. With the default being all 3. Examples of various integrations - where the current proposed schema works https://github.com/starfishmod/WPAPI-SwaggerGenerator https://www.discogs.com/developers/#page:authentication,header:authentication-oauth-flow http://api.eventful.com/docs/faq https://dev.evernote.com/doc/articles/authentication.php https://www.flickr.com/services/api/auth.oauth.html https://developers.upwork.com/?lang=python#authentication_oauth-10 http://www.scoop.it/dev/api/1/intro https://www.tumblr.com/docs/en/api/v2#auth https://oauth.withings.com/api https://developer.xero.com/documentation/api/api-overview https://us.etrade.com/ctnt/dev-portal/getContent?contentUri=V0_Documentation-DeveloperGuides-Authorization https://dev.twitter.com/oauth/overview/authorizing-requests Examples where the proposed schema does not work In reference to the comment about wanting to keep the URL field as non-required when he is using xAuth so maybe that is the place to use the option in the http scheme.
http://api.thenounproject.com/getting_started.html#authentication https://api.slideroom.com/
https://www.etsy.com/developers/documentation/getting_started/oauth |
I'm having the same requirement The old component has no unit test and very long running end-to-end test Currently Working on migration
Could we have hope to get official support? It's more than custom fields and getting support from implementation is easier when a feature is standard ;-) |
You also have Magento which is Oauth 1 based, even in its recent version |
This adds in the security definitions for Oauth1. It follows for from the discussions in #61. It is similar to the way RAML handles it's description of Oauth1 as well.