Add support for authorization flow with PKCE

[mukiblejlok]

Hello @singingwolfboy,

I know there is already issue #293 with a discussion about supporting PKCE flow. Since it's true that PKCE is mostly used with mobile apps, there are some OAuth2 servers (for example the one that we use in our company) that require clients to use PKCE flow. As the link says:

PKCE was originally designed to protect the authorization code flow in mobile apps, but its ability to prevent authorization code injection makes it useful for every type of OAuth client, even web apps that use a client secret.

As far as I'm aware there is no Open Source package for Flask that supports it.

I took some initial effort to implement the basic support for it in this PR. Being aware I'm far away from done (tests and docs updates are not there yet), I wanted to start the conversation and ask if you see the point in adding the PKCE support to Flask-Dance in the form I have proposed.

If so, I would be happy to continue my work on that pull request, especially since it would be my first bigger Open Source contribution. Something that I wanted to start doing for a while.

[codecov[bot]]

Codecov Report

Merging #406 (549dfa3) into main (d299c15) will increase coverage by 0.11%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #406      +/-   ##
==========================================
+ Coverage   93.41%   93.53%   +0.11%     
==========================================
  Files          36       36              
  Lines        1048     1067      +19     
==========================================
+ Hits          979      998      +19     
  Misses         69       69              

Impacted Files	Coverage Δ
flask_dance/consumer/base.py	90.90% <100.00%> (ø)
flask_dance/consumer/oauth2.py	93.54% <100.00%> (+1.16%)	:arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[singingwolfboy]

Thank you for doing this, and I'm sorry it took me a few days to review this!

I like your initiative here, but I think it would be better to implement this in a different way. OAuthlib has supported PKCE since version 3.0, according to their changelog. Rather than implementing these functions in a flask_dance/consumer/pkce.py file, can we use the right function arguments to make OAuthlib do this verification?

Check the docs for prepare_request_uri -- looks like we can just pass code_challenge and code_challenge_method in the kwargs to make this work. I think we can do something similar with the prepare_request_body function -- pass the code_verifier in the kwargs. Can you see how to do that? I don't think that requests-oauthlib has any special handling for PKCE, but I don't think it will get in the way, either!

[mukiblejlok]

Thank you for the very detailed feedback. I agree, it would be much better to use the features of OAuthlib instead of writing your own as I did. I was not aware of the PKCE support in OAuthlib 3.

As for passing the PCKE parameters to the prepare_request_uri and prepare_request_body, I believe that is what I am doing anyway, but not explicitly. authorization_url method from request_oautlib passes kwargs to _client.prepare_request_uri and fetch_token passes kwargs to _client.prepare_request_body.

Still, the core challenge in PKCE flow is to generate a code_verifier and code_challenge pair at the login stage, then save the code_verifier challenge in the context of the current OAuth Session (by storing it with the state param) and then to use the code_verifier at the authorization stage. In my opinion that would have to be implemented in the login and authorized methods of OAuth2ConsumerBlueprint. But if you have any other ideas, I'd be happy to do it your way.

[mukiblejlok]

I have made all the requested changes.

Additionally, I have noticed that the PKCE implementation for clients in OAuthlib was introduced in version 3.2.0 (not 3.0.0 as you have mentioned) OAuthlib Changelog. That is why I have used this version in pyproject.toml

Also, I have improved a test_login_url_with_pkce, so it tests if the values of code_verifier and code_challenge are correct.

[singingwolfboy]

Thanks! And congratulations on your first big open source contribution! 🎉

[mukiblejlok]

I am happy I could add a small contribution to your great package. Due to your support, it was a very pleasant experience. I will try to get involved more in the future.