fix(mcp): keep grant_type authoritative over extra_token_params (#4860)

* fix(mcp): keep grant_type authoritative over extra_token_params

_fetch_token built the token request body as
{"grant_type": oauth.grant_type, **oauth.extra_token_params}, so an
operator-supplied extra_token_params that happened to contain
"grant_type" silently overwrote the value sent to the token endpoint
while the branch logic below still keyed off oauth.grant_type — the
sent grant_type and the chosen auth flow would disagree, and the
provider would almost certainly reject the request.

Spread extra_token_params first and set grant_type (and the other
reserved fields, which were already set after the spread) afterward, so
operator-supplied params can populate arbitrary extra fields but never
override the reserved ones the flow depends on.

* test(mcp): cover extra OAuth token parameters

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
Baldwinzc 2026-08-22 17:01:38 +08:00 committed by GitHub
parent 5ffc2d3e27
commit 15802c37fb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 59 additions and 4 deletions

View File

@ -118,10 +118,13 @@ class OAuthTokenManager:
async def _fetch_token(self, oauth: McpOAuthConfig) -> _OAuthToken:
import httpx # pyright: ignore[reportMissingImports]
data: dict[str, str] = {
"grant_type": oauth.grant_type,
**oauth.extra_token_params,
}
# extra_token_params is spread first so the reserved fields below
# (grant_type, scope, audience, client_id, ...) cannot be silently
# overridden by an operator-supplied key — otherwise the branch logic
# below (which keys off oauth.grant_type) and the value actually sent
# to the token endpoint would disagree.
data: dict[str, str] = dict(oauth.extra_token_params)
data["grant_type"] = oauth.grant_type
if oauth.scope:
data["scope"] = oauth.scope

View File

@ -86,6 +86,58 @@ def test_oauth_token_manager_fetches_and_caches_token(monkeypatch):
assert post_calls[0]["data"]["grant_type"] == "client_credentials"
def test_oauth_extra_token_params_cannot_override_grant_type(monkeypatch):
post_calls: list[dict[str, Any]] = []
def _client_factory(*args, **kwargs):
return _MockAsyncClient(
payload={
"access_token": "token-123",
"token_type": "Bearer",
"expires_in": 3600,
},
post_calls=post_calls,
**kwargs,
)
monkeypatch.setattr("httpx.AsyncClient", _client_factory)
config = ExtensionsConfig.model_validate(
{
"mcpServers": {
"secure-http": {
"enabled": True,
"type": "http",
"url": "https://api.example.com/mcp",
"oauth": {
"enabled": True,
"token_url": "https://auth.example.com/oauth/token",
"grant_type": "client_credentials",
"client_id": "client-id",
"client_secret": "client-secret",
# A careless copy-paste from another OAuth config.
"extra_token_params": {
"grant_type": "password",
"resource": "https://api.example.com",
},
},
}
}
}
)
manager = OAuthTokenManager.from_extensions_config(config)
asyncio.run(manager.get_authorization_header("secure-http"))
# The reserved grant_type must win over the operator-supplied param so
# the value sent to the token endpoint matches the branch logic that
# picked client_credentials below. Other extension params still pass
# through unchanged.
assert post_calls[0]["data"]["grant_type"] == "client_credentials"
assert post_calls[0]["data"]["resource"] == "https://api.example.com"
def test_build_oauth_interceptor_injects_authorization_header(monkeypatch):
post_calls: list[dict[str, Any]] = []