Conversation
…_basic
RFC 6749 Section 2.3 states that when using HTTP Basic authentication,
the client credentials (client_id and client_secret) must be sent in
the Authorization header only and must not appear in the request body.
Previously, prepare_token_auth() stripped client_secret from the body
but left client_id in when client_secret_basic was in use, causing
servers like Notion's MCP implementation to reject token requests due
to duplicate credentials.
Fix: extend the body filter from
k != 'client_secret'
to
k not in ('client_secret', 'client_id')
Tests: update existing basic-auth assertions to reflect correct behaviour
and add a dedicated regression test
(test_basic_auth_does_not_send_client_id_in_body).
Fixes modelcontextprotocol#3138
|
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 #3138. If a maintainer assigns you to #3138, 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 |
…_basic
RFC 6749 Section 2.3 states that when using HTTP Basic authentication, the client credentials (client_id and client_secret) must be sent in the Authorization header only and must not appear in the request body.
Previously, prepare_token_auth() stripped client_secret from the body but left client_id in when client_secret_basic was in use, causing servers like Notion's MCP implementation to reject token requests due to duplicate credentials.
Fix: extend the body filter from
k != 'client_secret'
to
k not in ('client_secret', 'client_id')
Tests: update existing basic-auth assertions to reflect correct behaviour and add a dedicated regression test
(test_basic_auth_does_not_send_client_id_in_body).
Fixes #3138
Fixes #
Motivation and Context
How Has This Been Tested?
Breaking Changes
Types of changes
Checklist
help wanted, or I'm a maintainer)Additional context