truqu / elm-oauth2

OAuth 2.0 client-side utils in Elm
MIT License
81 stars 29 forks source link

Accomodate GitHub's token request requirements. #3

Closed billstclair closed 6 years ago

billstclair commented 6 years ago

GitHub requires an 'accept: application/json' header, or it URL-encodes the token information, instead of returning it as JSON in the body.

GitHub encodes the returned 'scope' as a comma-separated string, instead of a list of strings.

This change always sends the 'accept' header, and adds the comma-separated list of scope names as a Json.Decode.oneOf option.

I tested that it works for AuthorizationCode grant flow on GitHub, Gmail, Facebook, and Gab.

billstclair commented 6 years ago

I tested using a callback server and client example I'm developing at https://github.com/billstclair/elm-oauth-middleware. It takes a fair amount of configuration to set up the server, but I expect you already have some code to test it with yourself.

KtorZ commented 6 years ago

Hi and thanks for reaching out :)

The case of Github is a bit singular. I believe that you refer to the API v3 of GitHub and to this particular section of their documentation.

This comes in handy for creating OAuth token however, this isn't part of the OAuth2.0 protocol and is simply a custom accommodation provided by GitHub. The actual OAuth2.0 Authorization Code flow is detailed here and works "fine" with the current implementation. Note that:

Therefore, tokens can't be created client-side (browsers will perform an OPTION call prior to any request and get denied by GitHub). This is a conscious choice of them in order to enforce security and prevent developers from hard-coding their client secret client-side.

As a matter of fact, I don't think we should accommodate the library as such. I would like to strictly stick to the official RFC by providing a non-opinionated implementation.

billstclair commented 6 years ago

Actually, I am NOT referring to the API v3 of GitHub, which you linked to as this particular section. This is the first time I've looked at that page.

I followed their description of their implementation of the OAuth2.0 protocol, where the Authorization Code flow is detailed here, as you said.

If you read down to the Response section on that page, you will find:

By default, the response takes the following form:

access_token=e72e16c7e42f292c6912e7710c838347ae178b4a&token_type=bearer

You can also receive the content in different formats depending on the Accept header:

Accept: application/json {"access_token":"e72e16c7e42f292c6912e7710c838347ae178b4a", "scope":"repo,gist", "token_type":"bearer" }

Accept: application/xml

bearer repo,gist e72e16c7e42f292c6912e7710c838347ae178b4a

I am calling OAuth.AuthorizationCode.authorize from my client Elm code, passing the clientId, redirectUri, a responseType of OAuth.Code, a scope of ["user"], my application state, and a url of https://github.com/login/oauth/authorize, as documented on that page. That redirects to my redirectUri, passing the state and an authorization code, URL-encoded, as in the required part of the OAuth2.0 specifications you referenced. My redirect server in written in Elm, but runs on my web server machine under Node.js. That server exists to hide the client secret from the client code. It calls OAuth.AuthorizationCode.authenticate, passing the clientId and secret to GitHub's token server at https://github.com/login/oauth/access_token, and it receives a response, containing the authentication token, URL-encoded unless I pass the accept: application/json header, as in my pull request. If I pass that header, the JSON returned encodes the scope as a comma-separated string, NOT, as the spec demands, as a JSON array of strings.

Note that CORS never enters the picture here, since my OAuth.Authorization.authenticate call is on my server, running under Node.js, not the client, running in a web browser. If you're getting that call to work, to GitHub's token server, I'd love to see the code.

My guess is that the GitHub folks wrote their code before the OAuth2 spec was published, and decided that disrupting their installed base was not worth the effort to become standard compliant. But that's only a guess. I suppose I should ask them. Maybe I will.

I can understand if you prefer for your library to stick to the RFC, and it will not be hard for me to make my own, patched, version of OAuth.AuthorizationCode.authenticate for use on my server.

KtorZ commented 6 years ago

Hey!

You're doing interesting stuff with Elm as a web-server ^.^ I understand that you are quite aware of the overall process and how the authorization code flow works. Github indeed provides some sugary helpers on top of that (as the Accept header or different ways of creating tokens, cf previous links). I don't know why and when they've made these choices, but in my opinion, the whole point of using a well-defined protocol such as OAuth 2.0 is to avoid all the quirks and customizations of one's API in order to promote the usage of generic tools.

Thus, since the GitHub API actually implements (at least) the RFC specifications, I would rather stick to that. Up-to-you to make use of GitHub's flavored protocol :)

billstclair commented 6 years ago

That was the point of my long post. GitHub does NOT implement the OAuth 2.0 RFC specifications for the Authorization Code grant flow. Whatever. I know what to do. Thank you for your attention.

KtorZ commented 6 years ago

My bad, you're right. I am quite surprised to see such flaw in GitHub's implementation. This is definitely inconvenient and I suppose, hasn't been fix because it will break quite a lot of existing integrations.

KtorZ commented 6 years ago

Hey @billstclair!

I've try to come up with a solution with doesn't interfere with the current implementation but still give users of the library the ability to cope with implementation quirks like GitHub.

Can you have a quick look at #4 and tell me whether this would solve your issue?