Digest Authentication Middleware for aiohttp by TimMenninger · Pull Request #10725 · aio-libs/aiohttp

TimMenninger · 2025-04-15T23:32:32Z

What do these changes do?

Implementation Details

This middleware implements RFC 7616.

The implementation fully complies with the RFC specifications, including:

Support for all standard hash algorithms (MD5, SHA-256, SHA-512) and their session variants
Proper nonce counting and opaque value handling as required by the RFC
Support for both "auth" and "auth-int" quality of protection (qop) methods
Complete implementation of the authentication challenge/response flow

Changes

This PR adds:

A new DigestAuthMiddleware class that handles HTTP Digest Authentication
Support for multiple hashing algorithms (MD5, SHA-256, SHA-512)
Implementation of both "auth" and "auth-int" quality of protection (qop)
Automatic handling of authentication challenges and retries
Proper nonce counting and opaque value handling
Support for modern hashing algorithms as specified in RFC 7616

Are there changes in behavior for the user?

There are no changes to default behavior. It will continue to use BasicAuth, and existing uses of BasicAuth should be intact.

Is it a substantial burden for the maintainers to support this?

no

Related issue number

replaces and closes #2213 closes #4939

Checklist

I think the code is well written
Unit tests for the changes exist
Documentation reflects the changes
If you provide code modification, please add yourself to CONTRIBUTORS.txt
- The format is <Name> <Surname>.
- Please keep alphabetical order, the file is sorted by names.
Add a new news fragment into the CHANGES/ folder
- name it <issue_or_pr_num>.<type>.rst (e.g. 588.bugfix.rst)
- if you don't have an issue number, change it to the pull request
  number after creating the PR
  - .bugfix: A bug fix for something the maintainers deemed an
    improper undesired behavior that got corrected to match
    pre-agreed expectations.
  - .feature: A new behavior, public APIs. That sort of stuff.
  - .deprecation: A declaration of future API removals and breaking
    changes in behavior.
  - .breaking: When something public is removed in a breaking way.
    Could be deprecated in an earlier release.
  - .doc: Notable updates to the documentation structure or build
    process.
  - .packaging: Notes for downstreams about unobvious side effects
    and tooling. Changes in the test invocation considerations and
    runtime assumptions.
  - .contrib: Stuff that affects the contributor experience. e.g.
    Running tests, building the docs, setting up the development
    environment.
  - .misc: Changes that are hard to assign to any of the above
    categories.
- Make sure to use full sentences with correct case and punctuation,
  for example:
```
Fixed issue with non-ascii contents in doctest text files
-- by :user:`contributor-gh-handle`.
```
  Use the past tense or the present tense a non-imperative mood,
  referring to what's changed compared to the last released version
  of this project.

This is the initial work by jf from aio-libs#2213, which I rebased onto the tip of master. It is fully brought back to life in subsequent commits.

This completes the implementation of the digest helper, implementing for additional hashing algorithms sha256 and sha512, as well as auth-int qop. It implements tests and documents the features. The DigestAuth feature was shimmed into the existing API so it could be used in place of BasicAuth in the auth field of ClientSession and ClientRequest.

bdraco · 2025-04-15T23:42:03Z

We had talked about making a more generic client middleware, but we never moved that forward. I'm inclined to accept this since we would very much like to have digest auth support in 3.x, and move to using client middleware for auth in 4.x since it would be a breaking change anyways.

codspeed-hq · 2025-04-15T23:45:16Z

CodSpeed Performance Report

Merging #10725 will improve performances by 10.6%

_{Comparing TimMenninger:digest-auth-helper (e650960) with master (918c76d)}

Summary

⚡ 1 improvements
✅ 59 untouched benchmarks

Benchmarks breakdown

	Benchmark	`BASE`	`HEAD`	Change
⚡	`test_ten_streamed_responses_iter_any[pyloop]`	23.8 ms	21.5 ms	+10.6%

TimMenninger · 2025-04-15T23:46:00Z

We had talked about making a more generic client middleware, but we never moved that forward. I'm inclined to accept this since we would very much like to have digest auth support in 3.x, and move to using client middleware for auth in 4.x since it would be a breaking change anyways.

I agree with that - I didn't see any other good way of doing this without breaking backward compatibility. Almost requested punting this one to 4.x but it ended up not being horrible for a first attempt. @feus4177 did most of the heavy lifting anyway.

Dreamsorcerer · 2025-04-15T23:52:40Z

We had talked about making a more generic client middleware, but we never moved that forward. I'm inclined to accept this since we would very much like to have digest auth support in 3.x, and move to using client middleware for auth in 4.x since it would be a breaking change anyways.

I was hoping to work on that reasonably soon. This PR would add several extra hooks throughout the code that we'd need to remember to remove again. It'd also be confusing to users if we add this and then deprecate it only a release or 2 later.

My current understanding is that the middlewares would not be a breaking change and they could be backported. Then BasicAuth would become deprecated in favour of the middlewares.

bdraco · 2025-04-15T23:56:38Z

We had talked about making a more generic client middleware, but we never moved that forward. I'm inclined to accept this since we would very much like to have digest auth support in 3.x, and move to using client middleware for auth in 4.x since it would be a breaking change anyways.

I was hoping to work on that reasonably soon. This PR would add several extra hooks throughout the code that we'd need to remember to remove again. It'd also be confusing to users if we add this and then deprecate it only a release or 2 later.

My current understanding is that the middlewares would not be a breaking change and they could be backported. Then BasicAuth would become deprecated in favour of the middlewares.

I'd be much happier with client middleware vs this implementation. Seems like that is closer than I expected so I'd probably stop working on this PR but keep it warm in case we run into any snags with client middleware that would prevent it from being available in 3.12.

bdraco · 2025-04-15T23:57:20Z

For clarity: Once we have the client middleware we could add a digest auth client middleware (probably much of this could be reused)

TimMenninger · 2025-04-16T00:02:45Z

I'd be much happier with client middleware vs this implementation. Seems like that is closer than I expected so I'd probably stop working on this PR but keep it warm in case we run into any snags with client middleware that would prevent it from being available in 3.12.

Okay I am going to put this down for now. I really only picked it up in an effort to move 3.12 along. Feel free to ping me if/when we're ready to pick back up.

bdraco · 2025-05-20T14:26:40Z

We could write a bit better tests if we had server side support for digest auth, but thats likely something we don't need to implement for aiohttp

bdraco · 2025-05-20T14:27:06Z

Thanks @TimMenninger for getting this moving again.

I reworked it to use the new client middleware.

patchback · 2025-05-20T14:41:24Z

Backport to 3.12: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.12/ab76c5a6ed2dcb133a10e6c0bf55b737d49654b9/pr-10725

Backported as #10894

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

Co-authored-by: jf <[email protected]> Co-authored-by: J. Nick Koston <[email protected]> Co-authored-by: J. Nick Koston <[email protected]> Co-authored-by: Sam Bull <[email protected]> (cherry picked from commit ab76c5a)

…for aiohttp (#10894) Co-authored-by: Tim Menninger <[email protected]> Co-authored-by: jf <[email protected]> Co-authored-by: J. Nick Koston <[email protected]> Co-authored-by: J. Nick Koston <[email protected]> Co-authored-by: Sam Bull <[email protected]> closes #2213 closes #4939

psf-chronographer Bot added the bot:chronographer:provided There is a change note present in this PR label Apr 15, 2025

feus4177 and others added 3 commits April 15, 2025 16:33

Added a digest authentication helper (1/2)

3cd9feb

This is the initial work by jf from aio-libs#2213, which I rebased onto the tip of master. It is fully brought back to life in subsequent commits.

FIXUP: revert proxy_auth back to BasicAuth

cd59ebd

github-advanced-security AI found potential problems Apr 15, 2025

View reviewed changes

Comment thread aiohttp/helpers.py Fixed

FIXUP: added CHANGES file

22825c1

TimMenninger force-pushed the digest-auth-helper branch from 9927632 to 22825c1 Compare April 15, 2025 23:34

bdraco added the backport-3.12 label Apr 15, 2025

FIXUP: spelling

e6f53a9

bdraco reviewed Apr 15, 2025

View reviewed changes