Skip to content
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

DigestAuth as middleware - No nonce count #332

Merged

Conversation

yeraydiazdiaz
Copy link
Contributor

@yeraydiazdiaz yeraydiazdiaz commented Sep 10, 2019

Split from #305

This PR introduces a DigestAuth middleware that can be used as an auth parameter to Client and the request methods.

This works for the synchronous Client situations, for example:

import httpx

URL = "https://httpbin.org/digest-auth/auth/yeray/{password}/{algorithm}"

ALGORITHMS = [
    "MD5",
    "MD5-SESS",
    "SHA",
    "SHA-SESS",
    "SHA-256",
    "SHA-256-SESS",
    "SHA-512",
    "SHA-512-SESS",
]


def main():
    password = "foo"
    with httpx.Client(auth=httpx.DigestAuth("yeray", password)) as client:
        for algorithm in ALGORITHMS:
            response = client.get(URL.format(password=password, algorithm=algorithm))
            print("FAILED!" if response.status_code != 200 else "", algorithm, response)

        print("\nThis should fail...")
        print(client.get(URL.format(password="bar", algorithm=ALGORITHMS[0])))


if __name__ == "__main__":
    main()

Prints:

 MD5 <Response [200 OK]>
 MD5-SESS <Response [200 OK]>
 SHA <Response [200 OK]>
 SHA-SESS <Response [200 OK]>
 SHA-256 <Response [200 OK]>
 SHA-256-SESS <Response [200 OK]>
 SHA-512 <Response [200 OK]>
 SHA-512-SESS <Response [200 OK]>

This should fail...
<Response [401 Unauthorized]>

But fails on concurrent AsyncClient such as:

iimport asyncio

import httpx

URL = "https://httpbin.org/digest-auth/auth/yeray/{password}/{algorithm}"

ALGORITHMS = [
    "MD5",
    "MD5-SESS",
    "SHA",
    "SHA-SESS",
    "SHA-256",
    "SHA-256-SESS",
    "SHA-512",
    "SHA-512-SESS",
]


async def main():
    password = "foo"
    client = httpx.AsyncClient(auth=httpx.DigestAuth("yeray", password))
    fs = [
        client.get(URL.format(password=password, algorithm=algorithm))
        for algorithm in ALGORITHMS
    ]
    responses = await asyncio.gather(*fs)
    for algorithm, response in zip(ALGORITHMS, responses):
        print("FAILED!" if response.status_code != 200 else "", algorithm, response)

    print("\nThis should fail...")
    print(await client.get(URL.format(password="bar", algorithm=ALGORITHMS[0])))


if __name__ == "__main__":
    asyncio.run(main())

Printing inconsistent results with only some of the requests succeeding:

 MD5 <Response [200 OK]>
FAILED! MD5-SESS <Response [401 Unauthorized]>
FAILED! SHA <Response [401 Unauthorized]>
FAILED! SHA-SESS <Response [401 Unauthorized]>
FAILED! SHA-256 <Response [401 Unauthorized]>
 SHA-256-SESS <Response [200 OK]>
FAILED! SHA-512 <Response [401 Unauthorized]>
 SHA-512-SESS <Response [200 OK]>

This should fail...
<Response [401 Unauthorized]>

There's already a fair bit of code in the PR so I opted not to include the discussed solution in the previous PR (having a per request instance of the middleware) until I get feedback.

Copy link
Contributor

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

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

This looks good to me, I left some comments inline. :)

httpx/middleware/digest_auth.py Outdated Show resolved Hide resolved
response = client.get(url, auth=auth)

assert response.status_code == 200
assert response.json() == {"auth": None}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a test case where only 1 of the 2 conditions are met like 200+WWW-Authenticate or 401 without the header?

tests/client/test_auth.py Outdated Show resolved Hide resolved
tests/client/test_auth.py Outdated Show resolved Hide resolved
assert digest_data["algorithm"] == "SHA-256"


def test_digest_auth_qop_including_spaces_and_auth_returns_auth():
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we parametrize this with: (current input), "auth-int,auth", and "unknown, auth"?

httpx/middleware/digest_auth.py Outdated Show resolved Hide resolved
Copy link
Member

@florimondmanca florimondmanca left a comment

Choose a reason for hiding this comment

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

Looks great!

@sethmlarson sethmlarson merged commit fd3b69d into encode:master Sep 10, 2019
@sethmlarson
Copy link
Contributor

Thanks @yeraydiazdiaz :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants