- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 876
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
reuse the digest auth state to avoid unnecessary requests #2463
Conversation
tests/client/test_auth.py
Outdated
@pytest.mark.asyncio | ||
async def test_digest_auth_reuses_challenge() -> None: | ||
url = "https://example.org/" | ||
auth = DigestAuth(username="tomchristie", password="password123") |
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.
auth = DigestAuth(username="tomchristie", password="password123") | |
auth = DigestAuth(username="user", password="password123") |
tests/client/test_auth.py
Outdated
@pytest.mark.asyncio | ||
async def test_digest_auth_resets_nonce_count_after_401() -> None: | ||
url = "https://example.org/" | ||
auth = DigestAuth(username="tomchristie", password="password123") |
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.
auth = DigestAuth(username="tomchristie", password="password123") | |
auth = DigestAuth(username="user", password="password123") |
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.
Great stuff - thanks!
A few minor suggestions here.
I have refactored some code in view of your suggestions. Regarding the failing test pipeline i assume this is not an issue with my branch, as other pipelines seem to fail with the same error message? |
Yup. See #2465 |
Good work, thanks! |
this implements digest authentication state reuse as discussed in #1467.
Contrary to the comment i am not skipping most of the authentication flow if a previous challenge exists. Otherwise a failed authentication (401) would not result in a retry with the new server challenge (
test_digest_auth_resets_nonce_count_after_401
would fail).