Skip to content

Commit 5a3422e

Browse files
fix(client): don't send client_id in token body under client_secret_basic
RFC 6749 section 2.3 requires that with HTTP Basic auth, client credentials must not also appear in the request body. prepare_token_auth stripped client_secret from the body for the client_secret_basic branch but left client_id in, so strict token endpoints (Keycloak, Okta in strict mode, and the RFC 6749 compliance test suite) reject the request as presenting two authentication methods at once. Fixes #3138 Two existing tests asserted the old behavior explicitly, updated both to assert the corrected one, and added the same check to the refresh token test for symmetry. Signed-off-by: manjunathbhaskar <manjunathbhaskar854@gmail.com>
1 parent 00a7014 commit 5a3422e

2 files changed

Lines changed: 12 additions & 3 deletions

File tree

src/mcp/client/auth/oauth2.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,11 @@ def prepare_token_auth(
207207
credentials = f"{encoded_id}:{encoded_secret}"
208208
encoded_credentials = base64.b64encode(credentials.encode()).decode()
209209
headers["Authorization"] = f"Basic {encoded_credentials}"
210-
# Don't include client_secret in body for basic auth
211-
data = {k: v for k, v in data.items() if k != "client_secret"}
210+
# RFC 6749 section 2.3: with HTTP Basic auth, client credentials must not also
211+
# appear in the request body. Strict token endpoints reject a request that
212+
# presents both the Authorization header and client_id in the body as two
213+
# authentication methods at once.
214+
data = {k: v for k, v in data.items() if k not in ("client_secret", "client_id")}
212215
elif auth_method == "client_secret_post" and self.client_info.client_id and self.client_info.client_secret:
213216
# Include client_id and client_secret in request body (RFC 6749 §2.3.1)
214217
data["client_id"] = self.client_info.client_id

tests/client/test_auth.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -680,7 +680,10 @@ async def test_basic_auth_token_exchange(self, oauth_provider: OAuthClientProvid
680680
# client_secret should NOT be in body for basic auth
681681
content = request.content.decode()
682682
assert "client_secret=" not in content
683-
assert "client_id=test%40client" in content # client_id still in body
683+
# RFC 6749 section 2.3: with Basic auth, client_id must not appear in the body
684+
# either, it is already presented via the Authorization header, and strict token
685+
# endpoints reject a request that presents credentials both ways.
686+
assert "client_id=" not in content
684687

685688
@pytest.mark.anyio
686689
async def test_basic_auth_refresh_token(self, oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken):
@@ -716,6 +719,9 @@ async def test_basic_auth_refresh_token(self, oauth_provider: OAuthClientProvide
716719
# client_secret should NOT be in body
717720
content = request.content.decode()
718721
assert "client_secret=" not in content
722+
# ...and neither should client_id, for the same RFC 6749 section 2.3 reason as
723+
# token exchange.
724+
assert "client_id=" not in content
719725

720726
@pytest.mark.anyio
721727
async def test_none_auth_method(self, oauth_provider: OAuthClientProvider):

0 commit comments

Comments
 (0)