-
Notifications
You must be signed in to change notification settings - Fork 83
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
Implement code verifier (PKCE) #42
Conversation
@jay0lee This library currently supports Python 2.7, 3.4, 3.5, 3.6, and 3.7. Dropping 3.4 support is probably OK immediately, but 2.7 needs to stick around until at least EOY, and 3.5 until September 2020 (when its EOL happens). |
OK, will see about getting it working on 2.7. I also failed to update all the tests :-) |
with authorization_url_path as authorization_url_spy: | ||
url, _ = instance.authorization_url() | ||
|
||
_, kwargs = authorization_url_spy.call_args_list[0] |
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.
The mock has an assert_called_once_with
helper for this, e.g.:
authorization_url_spy..assert_called_once_with(code_challenge_method='S256')
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.
thanks, I can do this for checking code_challenge_method
but the other values are randomized and I'm just doing size and character type validations but I think I need to unpack the arguments to do those regex comparisons.
Definitely open to suggestions here. Thanks.
Tests are now passing, any further steps I need to take here? |
@jay0lee Thanks for your efforts! |
This PR adds support for PKCE code verification as documented at:
https://developers.google.com/identity/protocols/OAuth2InstalledApp#step1-code-verifier
note that I'm currently using secrets module which is Python 3.6+. I couldn't find anything that documented minimum Python version requirements for this project but if it is something older I'm happy to try and make the PR backwards compatible with it.