mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-09 03:18:44 +00:00
fix(mcp): don't auto-detect M2M OAuth from field presence (#23187)
* fix(mcp): require explicit opt-in for OAuth2 M2M client_credentials flow Auto-detecting M2M from client_id+secret+token_url presence broke existing interactive OAuth setups (e.g. GitHub Enterprise). Add oauth2_flow field and default has_client_credentials to False — M2M must be explicitly opted into with oauth2_flow: client_credentials. * test(mcp): add regression tests for oauth2_flow M2M opt-in behavior
This commit is contained in:
parent
26febf11a7
commit
9543d785b5
3 changed files with 106 additions and 4 deletions
|
|
@ -318,6 +318,7 @@ class MCPServerManager:
|
|||
# oauth specific fields
|
||||
client_id=server_config.get("client_id", None),
|
||||
client_secret=server_config.get("client_secret", None),
|
||||
oauth2_flow=server_config.get("oauth2_flow", None),
|
||||
scopes=resolved_scopes,
|
||||
authorization_url=resolved_authorization_url,
|
||||
token_url=resolved_token_url,
|
||||
|
|
@ -634,6 +635,7 @@ class MCPServerManager:
|
|||
client_id=client_id_value or getattr(mcp_server, "client_id", None),
|
||||
client_secret=client_secret_value
|
||||
or getattr(mcp_server, "client_secret", None),
|
||||
oauth2_flow=getattr(mcp_server, "oauth2_flow", None),
|
||||
scopes=resolved_scopes,
|
||||
authorization_url=mcp_server.authorization_url
|
||||
or getattr(mcp_oauth_metadata, "authorization_url", None),
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
from datetime import datetime
|
||||
from typing import Any, Dict, List, Optional
|
||||
from typing import Any, Dict, List, Literal, Optional
|
||||
|
||||
from pydantic import BaseModel, ConfigDict
|
||||
|
||||
|
|
@ -60,12 +60,22 @@ class MCPServer(BaseModel):
|
|||
byok_api_key_help_url: Optional[str] = None
|
||||
created_at: Optional[datetime] = None
|
||||
updated_at: Optional[datetime] = None
|
||||
# OAuth2 flow type. Defaults to None (interactive / authorization_code).
|
||||
# Set to "client_credentials" to enable M2M token fetching.
|
||||
oauth2_flow: Optional[Literal["client_credentials", "authorization_code"]] = None
|
||||
model_config = ConfigDict(arbitrary_types_allowed=True)
|
||||
|
||||
@property
|
||||
def has_client_credentials(self) -> bool:
|
||||
"""True if this server has OAuth2 client_credentials config (client_id, client_secret, token_url)."""
|
||||
return bool(self.client_id and self.client_secret and self.token_url)
|
||||
"""True if this server should use the OAuth2 client_credentials (M2M) flow.
|
||||
|
||||
M2M flow must be opted into explicitly via ``oauth2_flow: client_credentials``.
|
||||
Having client_id / client_secret / token_url present is NOT sufficient —
|
||||
those fields are also used for interactive (authorization_code) OAuth,
|
||||
e.g. GitHub Enterprise. Auto-detecting M2M from field presence was a
|
||||
breaking regression introduced with the M2M feature.
|
||||
"""
|
||||
return self.oauth2_flow == "client_credentials"
|
||||
|
||||
@property
|
||||
def needs_user_oauth_token(self) -> bool:
|
||||
|
|
|
|||
|
|
@ -1195,7 +1195,7 @@ class TestMCPServerManager:
|
|||
@pytest.mark.asyncio
|
||||
async def test_requires_per_user_auth_property_oauth2_with_client_creds(self):
|
||||
"""Test that requires_per_user_auth returns False for OAuth2 with client credentials"""
|
||||
# OAuth2 with client credentials
|
||||
# M2M must be opted in explicitly with oauth2_flow="client_credentials"
|
||||
server = MCPServer(
|
||||
server_id="oauth-server",
|
||||
name="oauth-server",
|
||||
|
|
@ -1205,6 +1205,7 @@ class TestMCPServerManager:
|
|||
client_id="client-id",
|
||||
client_secret="client-secret",
|
||||
token_url="http://oauth-server.com/token",
|
||||
oauth2_flow="client_credentials",
|
||||
)
|
||||
assert server.requires_per_user_auth is False
|
||||
assert server.has_client_credentials is True
|
||||
|
|
@ -2393,5 +2394,94 @@ class TestMCPServerTimestamps:
|
|||
assert rebuilt_table.updated_at == updated
|
||||
|
||||
|
||||
class TestHasClientCredentialsOAuth2Flow:
|
||||
"""
|
||||
Regression tests for the M2M auto-detection bug.
|
||||
|
||||
Before the fix, has_client_credentials returned True whenever
|
||||
client_id + client_secret + token_url were all set, even for
|
||||
interactive OAuth setups (e.g. GitHub Enterprise). This silently
|
||||
dropped user tokens and fetched M2M tokens instead.
|
||||
|
||||
The fix: M2M must be opted in explicitly via oauth2_flow="client_credentials".
|
||||
"""
|
||||
|
||||
def _make_server(self, **kwargs) -> MCPServer:
|
||||
return MCPServer(
|
||||
server_id="test-server",
|
||||
name="test-server",
|
||||
transport=MCPTransport.http,
|
||||
auth_type=MCPAuth.oauth2,
|
||||
url="https://github.example.com/mcp",
|
||||
**kwargs,
|
||||
)
|
||||
|
||||
def test_all_three_fields_set_without_oauth2_flow_is_not_m2m(self):
|
||||
"""
|
||||
GitHub Enterprise regression: client_id + client_secret + token_url
|
||||
should NOT trigger M2M flow unless oauth2_flow is explicitly set.
|
||||
"""
|
||||
server = self._make_server(
|
||||
client_id="gh-client-id",
|
||||
client_secret="gh-client-secret",
|
||||
token_url="https://github.example.com/login/oauth/access_token",
|
||||
)
|
||||
assert server.has_client_credentials is False
|
||||
|
||||
def test_explicit_client_credentials_flow_enables_m2m(self):
|
||||
"""oauth2_flow='client_credentials' opts in to M2M."""
|
||||
server = self._make_server(
|
||||
client_id="svc-client-id",
|
||||
client_secret="svc-client-secret",
|
||||
token_url="https://idp.example.com/token",
|
||||
oauth2_flow="client_credentials",
|
||||
)
|
||||
assert server.has_client_credentials is True
|
||||
|
||||
def test_explicit_authorization_code_flow_disables_m2m(self):
|
||||
"""oauth2_flow='authorization_code' always returns False."""
|
||||
server = self._make_server(
|
||||
client_id="gh-client-id",
|
||||
client_secret="gh-client-secret",
|
||||
token_url="https://github.example.com/login/oauth/access_token",
|
||||
oauth2_flow="authorization_code",
|
||||
)
|
||||
assert server.has_client_credentials is False
|
||||
|
||||
def test_no_fields_no_flow_is_not_m2m(self):
|
||||
"""No credentials configured — not M2M."""
|
||||
server = self._make_server()
|
||||
assert server.has_client_credentials is False
|
||||
|
||||
def test_partial_fields_without_flow_is_not_m2m(self):
|
||||
"""Partial credential fields without explicit flow — not M2M."""
|
||||
server = self._make_server(
|
||||
client_id="only-client-id",
|
||||
)
|
||||
assert server.has_client_credentials is False
|
||||
|
||||
def test_needs_user_oauth_token_true_without_explicit_m2m(self):
|
||||
"""
|
||||
Without oauth2_flow='client_credentials', an oauth2 server with
|
||||
client fields set still needs a user OAuth token (interactive flow).
|
||||
"""
|
||||
server = self._make_server(
|
||||
client_id="gh-client-id",
|
||||
client_secret="gh-client-secret",
|
||||
token_url="https://github.example.com/login/oauth/access_token",
|
||||
)
|
||||
assert server.needs_user_oauth_token is True
|
||||
|
||||
def test_needs_user_oauth_token_false_with_explicit_m2m(self):
|
||||
"""With oauth2_flow='client_credentials', no per-user token needed."""
|
||||
server = self._make_server(
|
||||
client_id="svc-client-id",
|
||||
client_secret="svc-client-secret",
|
||||
token_url="https://idp.example.com/token",
|
||||
oauth2_flow="client_credentials",
|
||||
)
|
||||
assert server.needs_user_oauth_token is False
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
pytest.main([__file__])
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue