Skip to content

Record history of requests made during authentication - #718

Merged
florimondmanca merged 2 commits into
masterfrom
auth-history
Jan 3, 2020
Merged

florimondmanca merged 2 commits into
masterfrom
auth-history

Conversation

@florimondmanca

Copy link
Copy Markdown
Contributor

Fixes #669

Based on @gdhameeja's work in #686 🌟🙇

The updated tests contain:

  • Asserts on response.history in the digest auth case.
  • An isolated test case using an imaginary auth scheme, for in-depth verification of what gets pushed to the history.

@florimondmanca
florimondmanca requested a review from a team January 3, 2020 20:45
Comment thread httpx/client.py
@florimondmanca

florimondmanca commented Jan 3, 2020 •

Copy link
Copy Markdown
Contributor Author

As a follow-up, I think we'll also want to update the wording in Redirection and History, as it currently describes the history as "a list of all any (sic) redirect responses that were followed". Technically that's true, but now the history would also list intermediate auth responses.

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

This is fanastic, yup!
Great work, team. ✨

Comment thread httpx/client.py
raise exc from None
else:
response.history = list(history)
await response.aread()

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.

Yup, good call.

It's possible that at a later date we might want to ensure that streaming responses don't load an intermediate response bodies (in redirect or auth), but that's only a "might", and would be a seperate issue if we did choose to tackle it.

@florimondmanca

florimondmanca commented Jan 3, 2020 •

Copy link
Copy Markdown
Contributor Author

Merging, thanks for taking the time to look into this @gdhameeja! 💯 Added you as a co-author to bc6163c. :-)

@florimondmanca
florimondmanca merged commit bc6163c into master Jan 3, 2020
@florimondmanca
florimondmanca deleted the auth-history branch January 3, 2020 21:59
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.

Authentication flows should preserve history.

2 participants