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

create_s256_code_challenge shouldn't accept invalid code verifiers #165

Closed
flokli opened this issue Nov 12, 2019 · 2 comments
Closed

create_s256_code_challenge shouldn't accept invalid code verifiers #165

flokli opened this issue Nov 12, 2019 · 2 comments
Assignees
Labels
bug

Comments

@flokli
Copy link
Contributor

@flokli flokli commented Nov 12, 2019

Describe the bug

RFC7636 describes the code challenge using the following ABNF:

code-challenge = 43*128unreserved
unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
ALPHA = %x41-5A / %x61-7A
DIGIT = %x30-39

This translates to the following regex:

[a-zA-Z0-9\-\.\_\~]{43,128}

authlib currently doesn't check if code challenges match the regex (but it should), and even docs and tests use invalid challenges.

IMHO, create_s256_code_challenge should verify the code_verifier and raise an error if it doesn't match the regex - I don't assume it's a considerable performance penalty.

We might also want to provide a create_code_verifier convenience method, and use this consistently throughout docs and tests.

@flokli flokli added the bug label Nov 12, 2019
@flokli flokli mentioned this issue Nov 12, 2019
3 of 8 tasks complete
lepture added a commit that referenced this issue Nov 12, 2019
lepture added a commit that referenced this issue Nov 12, 2019
@lepture

This comment has been minimized.

Copy link
Owner

@lepture lepture commented Nov 12, 2019

Thanks for your report. We need to check code_verifier for none method too, so I put the validation in validate_code_verifier.

@flokli

This comment has been minimized.

Copy link
Contributor Author

@flokli flokli commented Nov 13, 2019

Thanks!

@flokli flokli closed this Nov 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.