Conversation
…t_secret_basic ClientAuthenticator read client_id only from the token form body and rejected the request before even looking up the client, even when the caller authenticated properly via HTTP Basic. RFC 6749 section 2.3.1 lets a client_secret_basic client carry its id solely in the Authorization header. When the body has no client_id but a Basic header is present, decode the header id first and use it as the lookup key. Secret verification below is unchanged: the Basic secret is still compared against the stored one, and a body-supplied id that disagrees with the header id still fails as before. Adds direct authenticator tests: header-only credentials authenticate, and a request with credentials nowhere still fails with Missing client_id.
|
This PR has been closed automatically. This repo only keeps pull requests open when they come from a maintainer, or from a contributor a maintainer has assigned to the linked issue, and you aren't currently assigned to #3545. If a maintainer assigns you to #3545, this PR reopens on its own and there's nothing more you need to do here. Assignment is a maintainer call based on capacity; comments that only ask to be assigned don't factor in. What does help is engaging on the issue itself by confirming the repro, explaining why it matters for your use case, or describing the approach you'd take. You're welcome to keep pushing commits here (just avoid force-pushing, since GitHub can't reopen a rewritten branch), but that on its own won't get the PR reviewed or the issue assigned, and realistically most auto-closed PRs stay closed. There's no need to open a new PR either way. CONTRIBUTING.md has the full reasoning, but in short:
Maintainers: reopen, remove |
Server-side ClientAuthenticator read client_id only from the token form body and threw Missing client_id before even looking the client up — even when the caller authenticated correctly via HTTP Basic. RFC 6749 section 2.3.1 lets a client_secret_basic client carry its credentials solely in the Authorization header, so spec-compliant clients get rejected.
When the body carries no client_id but a Basic header is present, the lookup key now falls back to the decoded header id. Everything downstream is untouched: the Basic secret is still verified against the stored one, a body id disagreeing with the header id still fails the mismatch check, and requests with credentials nowhere still fail with Missing client_id.
This also unblocks the client-side direction from #3536: once compliant clients stop duplicating client_id into the body, this server code accepts them.
Tests: new tests/server/auth/middleware/test_client_auth.py covers the header-only path and the no-credentials path directly against the authenticator; the full auth integration module (42 tests) still passes, and ruff check/format are clean on both touched files.
Fixes #3545.