Repository navigation
Support non-ASCII passwords in basic_auth - #1782
Closed
Bountybette wants to merge 1 commit into
Closed
Bountybette wants to merge 1 commit into
Bountybette wants to merge 1 commit into
Conversation
hmac.compare_digest raises TypeError on str containing non-ASCII characters, so a correct non-ASCII password was answered with HTTP 500 instead of authenticating. Compare UTF-8 bytes instead. Also reject credentials that aren't valid UTF-8 with HTTP 401 instead of crashing on UnicodeDecodeError.
Member
|
Checklist? Second time you ignore it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
Problem
With
basic_auth(credentials=("bob", "pässword")), bob can never log in: the server answers HTTP 500 and logs "process_request failed" withTypeError: comparing strings with non-ASCII characters is not supported, becausehmac.compare_digestonly accepts ASCIIstr. The same 500 happens for any client that sends a non-ASCII password for a known username, and for credentials that aren't valid UTF-8, whereparse_authorization_basicletsUnicodeDecodeErrorescape. RFC 7617 test vectors with123£already parse fine, so only the comparison breaks.Solution
Compare UTF-8 encoded bytes in the hard coded
check_credentialsof the asyncio, sync and legacy implementations (trio reuses the asyncio one), and turnUnicodeDecodeErrorintoInvalidHeaderValueso invalid credentials get a 401. Restricting passwords to ASCII was the alternative, but RFC 7617 specifies UTF-8 andbuild_authorization_basicalready sends UTF-8.