Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/mcp/client/auth/oauth2.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -262,8 +262,11 @@ def prepare_token_auth(
credentials = f"{encoded_id}:{encoded_secret}"
encoded_credentials = base64.b64encode(credentials.encode()).decode()
headers["Authorization"] = f"Basic {encoded_credentials}"
# Don't include client_secret in body for basic auth
data = {k: v for k, v in data.items() if k != "client_secret"}
# RFC 6749 section 2.3: with HTTP Basic auth the client credentials
# must not also appear in the body. Some strict token endpoints
# reject a request that carries both the Authorization header and
# client_id in the body as two auth methods at once.
data = {k: v for k, v in data.items() if k not in ("client_secret", "client_id")}
elif auth_method == "client_secret_post" and self.client_info.client_secret:
# Include client_id and client_secret in request body (RFC 6749 §2.3.1)
data["client_id"] = self.client_info.client_id
Expand Down
17 changes: 14 additions & 3 deletions src/mcp/server/auth/handlers/token.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -24,7 +24,10 @@ class AuthorizationCodeRequest(BaseModel):
grant_type: Literal["authorization_code"]
code: str = Field(..., description="The authorization code")
redirect_uri: AnyUrl | None = Field(None, description="Must be the same as redirect URI provided in /authorize")
client_id: str
# Optional: a client_secret_basic client authenticates via the Authorization header and
# isn't required to repeat client_id in the body (RFC 6749 section 2.3). handle() backfills
# this from the already-authenticated client when the body omits it.
client_id: str | None = None
# we use the client_secret param, per https://datatracker.ietf.org/doc/html/rfc6749#section-2.3.1
client_secret: str | None = None
# See https://datatracker.ietf.org/doc/html/rfc7636#section-4.5
Expand All@@ -38,7 +41,8 @@ class RefreshTokenRequest(BaseModel):
grant_type: Literal["refresh_token"]
refresh_token: str = Field(..., description="The refresh token")
scope: str | None = Field(None, description="Optional scope parameter")
client_id: str
# Optional, see AuthorizationCodeRequest.client_id above.
client_id: str | None = None
# we use the client_secret param, per https://datatracker.ietf.org/doc/html/rfc6749#section-2.3.1
client_secret: str | None = None
# RFC 8707 resource indicator
Expand All@@ -52,7 +56,8 @@ class JwtBearerRequest(BaseModel):
# See https://datatracker.ietf.org/doc/html/rfc7523#section-2.1
assertion: str = Field(..., description="The ID-JAG (a signed JWT) being presented as the grant")
scope: str | None = Field(None, description="Optional scope parameter")
client_id: str
# Optional, see AuthorizationCodeRequest.client_id above.
client_id: str | None = None
# we use the client_secret param, per https://datatracker.ietf.org/doc/html/rfc6749#section-2.3.1
client_secret: str | None = None
# RFC 8707 resource indicator
Expand DownExpand Up@@ -121,6 +126,12 @@ async def handle(self, request: Request):
form_data = await request.form()
# TODO(Marcelo): Can someone check if this `dict()` wrapper is necessary?
token_request = token_request_adapter.validate_python(dict(form_data))
# A client_secret_basic client authenticated via the Authorization header and may
# have omitted client_id from the body; client_info is already the verified identity
# (authenticate_request cross-checks any body-supplied client_id against it), so
# backfill from there rather than requiring the body to repeat it.
if token_request.client_id is None:
token_request.client_id = client_info.client_id
except ValidationError as validation_error:
return self.response(
TokenErrorResponse(
Expand Down
48 changes: 28 additions & 20 deletions src/mcp/server/auth/middleware/client_auth.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -52,37 +52,45 @@ async def authenticate_request(self, request: Request) -> OAuthClientInformation
AuthenticationError: If authentication fails
"""
form_data = await request.form()
client_id = form_data.get("client_id")
if not client_id:
raise AuthenticationError("Missing client_id")

client = await self.provider.get_client(str(client_id))
if not client:
raise AuthenticationError("Invalid client_id") # pragma: no cover

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

if client.token_endpoint_auth_method == "client_secret_basic":
if not auth_header.startswith("Basic "):
raise AuthenticationError("Missing or invalid Basic authentication in Authorization header")

# A Basic header carries client_id itself, so a client using client_secret_basic
# is not required to also repeat it in the body (RFC 6749 section 2.3 treats the
# header and the body as alternatives, and strict token endpoints reject both at
# once). Decode it up front so the lookup below can fall back to it.
basic_client_id: str | None = None
basic_client_secret: str | None = None
if auth_header.startswith("Basic "):
try:
encoded_credentials = auth_header[6:] # Remove "Basic " prefix
decoded = base64.b64decode(encoded_credentials).decode("utf-8")
if ":" not in decoded:
raise ValueError("Invalid Basic auth format")
basic_client_id, request_client_secret = decoded.split(":", 1)

raw_client_id, raw_client_secret = decoded.split(":", 1)
# URL-decode both parts per RFC 6749 Section 2.3.1
basic_client_id = unquote(basic_client_id)
request_client_secret = unquote(request_client_secret)

if basic_client_id != client_id:
raise AuthenticationError("Client ID mismatch in Basic auth")
basic_client_id = unquote(raw_client_id)
basic_client_secret = unquote(raw_client_secret)
except (ValueError, UnicodeDecodeError, binascii.Error):
raise AuthenticationError("Invalid Basic authentication header")

form_client_id = form_data.get("client_id")
client_id = str(form_client_id) if form_client_id else basic_client_id
if not client_id:
raise AuthenticationError("Missing client_id")

client = await self.provider.get_client(client_id)
if not client:
raise AuthenticationError("Invalid client_id") # pragma: no cover

request_client_secret: str | None = None

if client.token_endpoint_auth_method == "client_secret_basic":
if basic_client_id is None:
raise AuthenticationError("Missing or invalid Basic authentication in Authorization header")
if form_client_id and str(form_client_id) != basic_client_id:
raise AuthenticationError("Client ID mismatch in Basic auth")
request_client_secret = basic_client_secret

elif client.token_endpoint_auth_method == "client_secret_post":
raw_form_data = form_data.get("client_secret")
# form_data.get() can return an UploadFile or None, so we need to check if it's a string
Expand Down
7 changes: 4 additions & 3 deletions tests/client/test_auth.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -676,10 +676,10 @@ async def test_basic_auth_token_exchange(self, oauth_provider: OAuthClientProvid
assert unquote(client_id) == client_id_raw
assert unquote(client_secret) == client_secret_raw

# client_secret should NOT be in body for basic auth
# Neither credential belongs in the body for basic auth (RFC 6749 2.3)
content = request.content.decode()
assert "client_secret=" not in content
assert "client_id=test%40client" in content # client_id still in body
assert "client_id=" not in content

@pytest.mark.anyio
async def test_basic_auth_refresh_token(self, oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken):
Expand DownExpand Up@@ -712,9 +712,10 @@ async def test_basic_auth_refresh_token(self, oauth_provider: OAuthClientProvide
decoded = base64.b64decode(encoded_creds).decode()
assert decoded == f"{client_id}:{client_secret}"

# client_secret should NOT be in body
# Neither credential belongs in the body for basic auth (RFC 6749 2.3)
content = request.content.decode()
assert "client_secret=" not in content
assert "client_id=" not in content

@pytest.mark.anyio
async def test_none_auth_method(self, oauth_provider: OAuthClientProvider):
Expand Down
Loading