Skip to content

Commit 226190b

Browse files
committed
fix(server/auth): fall back to Basic header for client_id under client_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.
1 parent 6affe5c commit 226190b

2 files changed

Lines changed: 87 additions & 1 deletion

File tree

‎src/mcp/server/auth/middleware/client_auth.py‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,16 @@ async def authenticate_request(self, request: Request) -> OAuthClientInformation
5353
"""
5454
form_data = await request.form()
5555
client_id = form_data.get("client_id")
56+
auth_header = request.headers.get("Authorization", "")
57+
if not client_id and auth_header.startswith("Basic "):
58+
# RFC 6749 §2.3.1 lets a client_secret_basic client carry its id
59+
# only in the Basic header, so fall back to it before giving up.
60+
try:
61+
decoded = base64.b64decode(auth_header[6:]).decode("utf-8")
62+
if ":" in decoded:
63+
client_id = unquote(decoded.split(":", 1)[0]) or None
64+
except (ValueError, UnicodeDecodeError, binascii.Error):
65+
client_id = None
5666
if not client_id:
5767
raise AuthenticationError("Missing client_id")
5868

@@ -61,7 +71,6 @@ async def authenticate_request(self, request: Request) -> OAuthClientInformation
6171
raise AuthenticationError("Invalid client_id") # pragma: no cover
6272

6373
request_client_secret: str | None = None
64-
auth_header = request.headers.get("Authorization", "")
6574

6675
if client.token_endpoint_auth_method == "client_secret_basic":
6776
if not auth_header.startswith("Basic "):
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
"""Direct tests for ClientAuthenticator's Basic-header client_id fallback."""
2+
3+
import base64
4+
5+
import pytest
6+
from starlette.requests import Request
7+
8+
from mcp.server.auth.middleware.client_auth import (
9+
AuthenticationError,
10+
ClientAuthenticator,
11+
)
12+
from mcp.server.auth.provider import OAuthAuthorizationServerProvider
13+
from mcp.shared.auth import OAuthClientInformationFull
14+
15+
16+
class _StubProvider(OAuthAuthorizationServerProvider[None, None, None]): # type: ignore[type-arg]
17+
def __init__(self, client: OAuthClientInformationFull | None):
18+
self._client = client
19+
20+
async def get_client(self, client_id: str) -> OAuthClientInformationFull | None:
21+
if self._client and self._client.client_id == client_id:
22+
return self._client
23+
return None # pragma: no cover
24+
25+
async def register_client(self, client_info: OAuthClientInformationFull): ... # pragma: no cover
26+
async def authorize(self, client: OAuthClientInformationFull, params): ... # pragma: no cover
27+
28+
29+
def _client() -> OAuthClientInformationFull:
30+
return OAuthClientInformationFull(
31+
client_id="header-only-client",
32+
client_secret="s3cret",
33+
token_endpoint_auth_method="client_secret_basic",
34+
redirect_uris=["https://client.example.com/callback"],
35+
grant_types=["authorization_code"],
36+
)
37+
38+
39+
def _request(body: bytes, auth_header: str | None) -> Request:
40+
async def receive() -> dict:
41+
return {"type": "http.request", "body": body, "more_body": False}
42+
43+
headers = [(b"content-type", b"application/x-www-form-urlencoded")]
44+
if auth_header is not None:
45+
headers.append((b"authorization", auth_header.encode()))
46+
return Request(
47+
{
48+
"type": "http",
49+
"method": "POST",
50+
"path": "/token",
51+
"headers": headers,
52+
},
53+
receive=receive,
54+
)
55+
56+
57+
@pytest.mark.anyio
58+
async def test_basic_client_id_falls_back_to_authorization_header() -> None:
59+
"""A client_secret_basic request with no body client_id authenticates via the header."""
60+
stored = _client()
61+
auth = ClientAuthenticator(_StubProvider(stored))
62+
creds = base64.b64encode(f"{stored.client_id}:{stored.client_secret}".encode()).decode()
63+
req = _request(b"grant_type=authorization_code&code=abc", f"Basic {creds}")
64+
65+
result = await auth.authenticate_request(req)
66+
67+
assert result.client_id == stored.client_id
68+
69+
70+
@pytest.mark.anyio
71+
async def test_no_credentials_anywhere_still_rejected() -> None:
72+
"""With neither body nor header credentials, the original Missing client_id stands."""
73+
auth = ClientAuthenticator(_StubProvider(_client()))
74+
req = _request(b"grant_type=authorization_code&code=abc", None)
75+
76+
with pytest.raises(AuthenticationError, match="Missing client_id"):
77+
await auth.authenticate_request(req)

0 commit comments

Comments
 (0)