Colin-b / httpx_auth

Authentication classes to be used with httpx
MIT License
117 stars 26 forks source link

Bug fix for issue 14 #15

Closed martinka closed 4 years ago

martinka commented 4 years ago

The actual issue appears to be related to creation of canonical URL paths when the path in the request was empty. The fix changes the canonical path form . to / in the case of an empty path. This also results in the expected signatures to change, for a number of the test cases. This also adds a unit test for the problem to avoid future regressions.

martinka commented 4 years ago

Not sure whats going on, with the test coverage... locally mine looks ok `---------- coverage: platform darwin, python 3.7.3-final-0 ----------- Name Stmts Miss Cover Missing

httpx_auth/init.py 5 0 100% httpx_auth/authentication.py 318 0 100% httpx_auth/aws.py 122 0 100% httpx_auth/errors.py 51 0 100% httpx_auth/oauth2_authentication_responses_server.py 114 0 100% httpx_auth/oauth2_tokens.py 108 0 100% httpx_auth/testing.py 79 0 100% httpx_auth/version.py 1 0 100%

TOTAL 798 0 100% `. I don't have a linux machine handy, although I can spin up a VM if I have to. can we get the CI to print the term-missing report so I know which line its complaining about?

Colin-b commented 4 years ago

Yes feel free to edit the travis.yml file to include this

martinka commented 4 years ago

I think we are good to go at this point. Let me know if you need anything else for this PR. Because httpx now always includes the host header I removed the check that set it if it was missing.

Colin-b commented 4 years ago

Seems good to me, except the test case, I wonder when this case happen

martinka commented 4 years ago

Colin Sure, it happens any time you havecc V a URL that ends with the host. I have a bunch of examples. I can add a test. Mike

On Aug 29, 2020, at 5:44 PM, Colin Bounouar notifications@github.com wrote:

 @Colin-b commented on this pull request.

In tests/test_aws4auth.py:

 )

assert headers["x-amz-date"] == "20181011T150505Z" + + +def test_amz_cano_path_empty_path():

  • auth = httpx_auth.AWS4Auth(
  • access_id="access_id", Can you create a test case like the other so that we have a real example of an empty path ? I don't see how it can occur

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

martinka commented 4 years ago

I added a test for a get with no path portion of the URL. It turns out that many of the other tests were actually covering this case, the reason they were passing is the signature I included in the tests was calculated using the same code that had the error. That is why the PR has all the signature changes. Anyway hope this helps.

Colin-b commented 4 years ago

Thanks a lot, I will remove the unit test then and issue a new release :-)