Skip to content

reuse the digest auth state to avoid unnecessary requests - #2463

Merged
lovelydinosaur merged 9 commits into
encode:masterfrom
rettier:reuse-digest-challenge
Nov 29, 2022
Merged

lovelydinosaur merged 9 commits into
encode:masterfrom
rettier:reuse-digest-challenge

Conversation

@rettier

@rettier rettier commented Nov 25, 2022

Copy link
Copy Markdown
Contributor

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).

Comment thread tests/client/test_auth.py
Comment thread 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")

@lovelydinosaur lovelydinosaur Nov 25, 2022 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
auth = DigestAuth(username="tomchristie", password="password123")
auth = DigestAuth(username="user", password="password123")

Comment thread 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")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
auth = DigestAuth(username="tomchristie", password="password123")
auth = DigestAuth(username="user", password="password123")

Comment thread tests/client/test_auth.py
Comment thread tests/test_auth.py

@lovelydinosaur lovelydinosaur left a comment

Copy link
Copy Markdown
Member

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.

@rettier

rettier commented Nov 27, 2022

Copy link
Copy Markdown
Contributor Author

I have refactored some code in view of your suggestions.
Since you removed your username from the test-data i have also removed it from the existing basic auth test cases.

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?

@lovelydinosaur

Copy link
Copy Markdown
Member

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

@lovelydinosaur
lovelydinosaur merged commit 8327e13 into encode:master Nov 29, 2022
@lovelydinosaur

Copy link
Copy Markdown
Member

Good work, thanks!

@lovelydinosaur lovelydinosaur mentioned this pull request Jan 2, 2023
samclearman pushed a commit to titanmsp/httpx that referenced this pull request Apr 26, 2025
* reuse the digest auth challenge to avoid sending twice as many requests

* fix for digest testcase

* ran testing/linting scripts

* codereview changes, removed tomchristie username from all authentication tests

Co-authored-by: Philipp Reitter <p.reitter@accessio.at>
Co-authored-by: Tom Christie <tom@tomchristie.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants