Skip to content
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

Support OAuth 2.0 Authorization Code flow with PKCE #5348

Closed
poveilleux opened this issue May 13, 2019 · 8 comments · Fixed by #5361
Closed

Support OAuth 2.0 Authorization Code flow with PKCE #5348

poveilleux opened this issue May 13, 2019 · 8 comments · Fixed by #5361

Comments

@poveilleux
Copy link
Contributor

poveilleux commented May 13, 2019

Is your feature request related to a problem?

In the light of the new IETF related to OAuth 2.0 for Browser-Based Apps (see 4. Overview), an application running in a browser and using the Authorization Code flow should be using PKCE (Proof Key for Code Exchange) for increased security. It is not the case currently with swagger-ui. This article by Brock Allen also touches on the subject.

Describe the solution you'd like

Everytime the authorizationCode flow is used (these are based on the PKCE IETF linked above):

  1. A code_verifier is generated (potentially using uuid4 like they do in oidc-client-js). This value needs to stored somewhere because it needs to be sent on the Token request (looking at the code, I figured win.swaggerUIRedirectOauth2 would be the right place to store this).
  2. Create the code_challenge for the code_verifier (hashed using SHA256) and send this on the Authorize request alongside the code_challenge_method.

Describe alternatives you've considered

  • I've considered using additionalQueryStringParams to feed in the code_verifier and the code_challenge, but these values need to be generated on each authorize request.
  • I've considered using the Inject JavaScript functionality, but I feel like I would be replacing whole methods from swagger-ui which would bind me to a certain version of the code.

Additional context

I feel this is something that should be introduced into swagger-ui because of the added security it provides. Everybody would and should benefit from it.

@poveilleux
Copy link
Contributor Author

poveilleux commented May 13, 2019

I will try to submit a pull request for this feature request this week.

@poveilleux
Copy link
Contributor Author

I have created a branch locally where I have made the changes. How can I submit my solution to you?

@fuzzzerd
Copy link

If you cloned their repository and made a local branch, I think you'll have to fork it through github and then merge your original branch to that new fork. Then submit a pull request.

@poveilleux
Copy link
Contributor Author

@fuzzzerd Thank you for the tip. This is my first ever pull request on a public repository on GitHub, so I did not know that's how it works. I will do that to submit my pull request.

@SteadBytes
Copy link

SteadBytes commented Aug 11, 2019

Some housekeeping for the future: If/when this is resolved, this comment in the OAuth2 documentation should be removed: https://github.com/noirbizarre/flask-restplus/pull/585/files#diff-c4a0af43900c17a78b71064b375d87ecR898

@RobertoMachorro
Copy link

How is the code_challenge configured? I'm currently getting the following:
image

@rubentest
Copy link

wing:
image

I have the same error.
Do you solve this?

@mheguy-flo
Copy link

You have to enable pkce.
C# example:

app.UseSwagger().UseSwaggerUI(options => {
    options.OAuthClientId("api-swagger");
    options.OAuthScopes("profile", "openid", "api");
    options.OAuthUsePkce();
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants