From a676c29794e4f2a6f165cd1d08ad920c754dde20 Mon Sep 17 00:00:00 2001 From: Tin Chi Lo Date: Thu, 25 Jun 2026 19:46:16 -0700 Subject: [PATCH] feat(mcp): gateway-managed OAuth apps for Slack MCP and manifest provisioning Slack's hosted MCP server does not support Dynamic Client Registration, so the gateway must present a registered Slack app's client_id. When a Slack MCP server had no client_id, register_client_with_server returned the server's database id as the OAuth client_id, which Slack rejects with "Invalid client_id parameter" Resolve client_id and client_secret from a new general_settings.mcp_managed_oauth_apps map keyed by authorization-server host, so one operator-registered app backs every user with no per-server config and no end-user input. Stop fabricating the server id; raise a clear error when neither a client_id nor a registration_url is available Add a manifest-assisted provisioning endpoint that creates the Slack app through apps.manifest.create and stores its client_id plus an encrypted client_secret as the managed app for slack.com Enable token rotation for the managed Slack app. The manifest opts into token_rotation_enabled, and the per-user refresh path now resolves the managed app's client_id and client_secret the same way the authorize/token handshake does, so a managed app that carries no per-server client_id refreshes with the managed credentials instead of omitting them and being rejected. Slack rotates user tokens at oauth.v2.access, a different endpoint than the oauth.v2.user.access used for the initial code exchange, so the refresh targets that endpoint for Slack hosts while every other provider keeps its single token endpoint --- litellm/proxy/_experimental/mcp_server/db.py | 17 +- .../mcp_server/discoverable_endpoints.py | 123 +++++++-- .../mcp_server/slack_app_provisioning.py | 175 +++++++++++++ .../mcp_management_endpoints.py | 111 ++++++++- .../mcp_server/test_discoverable_endpoints.py | 2 +- .../mcp_server/test_db_credentials.py | 63 +++++ .../mcp_server/test_discoverable_endpoints.py | 235 +++++++++++++++++- .../mcp_server/test_slack_app_provisioning.py | 129 ++++++++++ .../test_mcp_management_endpoints.py | 146 ++++++++++- 9 files changed, 974 insertions(+), 27 deletions(-) create mode 100644 litellm/proxy/_experimental/mcp_server/slack_app_provisioning.py create mode 100644 tests/test_litellm/proxy/_experimental/mcp_server/test_slack_app_provisioning.py diff --git a/litellm/proxy/_experimental/mcp_server/db.py b/litellm/proxy/_experimental/mcp_server/db.py index 2dd046ceada..42d0e6f24a9 100644 --- a/litellm/proxy/_experimental/mcp_server/db.py +++ b/litellm/proxy/_experimental/mcp_server/db.py @@ -1013,8 +1013,6 @@ async def refresh_user_oauth_token( refresh_token: Optional[str] = cred.get("refresh_token") token_url: Optional[str] = getattr(server, "token_url", None) server_id: str = getattr(server, "server_id", "") - client_id: Optional[str] = getattr(server, "client_id", None) - client_secret: Optional[str] = getattr(server, "client_secret", None) if not refresh_token: verbose_proxy_logger.debug( @@ -1030,6 +1028,19 @@ async def refresh_user_oauth_token( ) return None + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( # noqa: PLC0415 + _effective_oauth_credentials, + ) + from litellm.proxy._experimental.mcp_server.slack_app_provisioning import ( # noqa: PLC0415 + slack_token_refresh_url, + ) + + # Resolve client_id/secret the same way the authorize/token handshake does so + # gateway-managed apps (which carry no per-server client_id) refresh with the + # managed app's credentials instead of omitting them and being rejected. + client_id, client_secret = _effective_oauth_credentials(server) + refresh_url = slack_token_refresh_url(token_url) or token_url + token_data: Dict[str, str] = { "grant_type": "refresh_token", "refresh_token": refresh_token, @@ -1042,7 +1053,7 @@ async def refresh_user_oauth_token( try: async_client = get_async_httpx_client(llm_provider=httpxSpecialProvider.Oauth2Check) response = await async_client.post( - token_url, + refresh_url, headers={"Accept": "application/json"}, data=token_data, ) diff --git a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py index 7a8df83f9f9..4fe32fec620 100644 --- a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py @@ -2,6 +2,7 @@ import asyncio import html as _html import json import time +from dataclasses import dataclass from typing import Any, Dict, Optional, Tuple from urllib.parse import parse_qsl, urlencode, urlparse, urlunparse @@ -26,6 +27,10 @@ from litellm.proxy.common_utils.encrypt_decrypt_utils import ( ) from litellm.proxy.common_utils.http_parsing_utils import _read_request_body from litellm.proxy.utils import get_server_root_path +from litellm.secret_managers.main import ( + get_secret_str, + normalize_nonempty_secret_str, +) from litellm.types.mcp import MCPAuth from litellm.types.mcp_server.mcp_server_manager import MCPServer @@ -323,6 +328,88 @@ async def _store_per_user_token_server_side( ) +@dataclass(frozen=True) +class ManagedOAuthApp: + client_id: str + client_secret: Optional[str] = None + + +def _resolve_config_secret(raw: object) -> Optional[str]: + if not isinstance(raw, str): + return None + if raw.startswith("os.environ/"): + return normalize_nonempty_secret_str(get_secret_str(raw)) + # A provisioned app stores client_secret encrypted; decrypt_value_helper + # returns the original value unchanged for plain operator-typed config. + decrypted = decrypt_value_helper( + value=raw, + key="mcp_managed_oauth_apps", + exception_type="debug", + return_original_value=True, + ) + return normalize_nonempty_secret_str(decrypted) + + +def _managed_oauth_app(authorization_url: Optional[str]) -> Optional[ManagedOAuthApp]: + """Resolve a gateway-owned OAuth app for an MCP server by its authorization + host, from ``general_settings.mcp_managed_oauth_apps``. + + Lets one operator-registered app back every user of a provider whose + authorization server has no Dynamic Client Registration (e.g. Slack), so no + per-server client_id and no end-user input is required. + """ + if not authorization_url: + return None + from litellm.proxy.proxy_server import general_settings + + registry: object = general_settings.get("mcp_managed_oauth_apps") + if not isinstance(registry, dict): + return None + # hostname (not netloc) so a key like "slack.com" still matches an + # authorization_url that carries an explicit port or userinfo + host = urlparse(authorization_url).hostname + if host is None: + return None + entry: object = registry.get(host) + if not isinstance(entry, dict): + return None + client_id = _resolve_config_secret(entry.get("client_id")) + if client_id is None: + return None + return ManagedOAuthApp( + client_id=client_id, + client_secret=_resolve_config_secret(entry.get("client_secret")), + ) + + +def _effective_client_id(mcp_server: MCPServer) -> Optional[str]: + if mcp_server.client_id: + return mcp_server.client_id + managed = _managed_oauth_app(mcp_server.authorization_url) + return managed.client_id if managed else None + + +def _effective_client_secret(mcp_server: MCPServer) -> Optional[str]: + if mcp_server.client_secret: + return mcp_server.client_secret + managed = _managed_oauth_app(mcp_server.authorization_url) + return managed.client_secret if managed else None + + +def _effective_oauth_credentials( + mcp_server: MCPServer, +) -> tuple[Optional[str], Optional[str]]: + """Resolve client_id and client_secret with a single managed-app lookup, + so the stored secret is decrypted at most once per call.""" + if mcp_server.client_id and mcp_server.client_secret: + return mcp_server.client_id, mcp_server.client_secret + managed = _managed_oauth_app(mcp_server.authorization_url) + return ( + mcp_server.client_id or (managed.client_id if managed else None), + mcp_server.client_secret or (managed.client_secret if managed else None), + ) + + async def authorize_with_server( request: Request, mcp_server: MCPServer, @@ -356,7 +443,7 @@ async def authorize_with_server( ) params = { - "client_id": mcp_server.client_id if mcp_server.client_id else client_id, + "client_id": _effective_client_id(mcp_server) or client_id, "redirect_uri": f"{request_base_url}/callback", "state": encoded_state, "response_type": response_type or "code", @@ -396,8 +483,9 @@ async def exchange_token_with_server( if mcp_server.token_url is None: raise HTTPException(status_code=400, detail="MCP server token url is not set") - resolved_client_id = mcp_server.client_id if mcp_server.client_id else client_id - resolved_client_secret = mcp_server.client_secret if mcp_server.client_secret else client_secret + effective_client_id, effective_client_secret = _effective_oauth_credentials(mcp_server) + resolved_client_id = effective_client_id or client_id + resolved_client_secret = effective_client_secret or client_secret if grant_type == "refresh_token": if not refresh_token: @@ -508,23 +596,30 @@ async def register_client_with_server( grant_types: Optional[list], response_types: Optional[list], token_endpoint_auth_method: Optional[str], - fallback_client_id: Optional[str] = None, ): request_base_url = get_request_base_url(request) - dummy_return = { - "client_id": fallback_client_id or mcp_server.server_name, - "client_secret": "dummy", - "redirect_uris": [f"{request_base_url}/callback"], - } - if mcp_server.client_id and mcp_server.client_secret: - return dummy_return + effective_client_id = _effective_client_id(mcp_server) + if effective_client_id: + return { + "client_id": effective_client_id, + "client_secret": "dummy", + "redirect_uris": [f"{request_base_url}/callback"], + } if mcp_server.authorization_url is None: raise HTTPException(status_code=400, detail="MCP server authorization url is not set") if mcp_server.registration_url is None: - return dummy_return + raise HTTPException( + status_code=400, + detail={ + "error": "This MCP server's authorization server does not support " + "Dynamic Client Registration. Set a client_id (and client_secret) on " + "the server, or register a gateway-managed OAuth app for its " + "authorization host under general_settings.mcp_managed_oauth_apps" + }, + ) register_data = { "client_name": client_name, @@ -586,7 +681,7 @@ async def authorize( # Use server's stored client_id when caller doesn't supply one. # Raise a clear error instead of passing an empty string — an empty # client_id would silently produce a broken authorization URL. - resolved_client_id: str = mcp_server.client_id or client_id or "" + resolved_client_id: str = _effective_client_id(mcp_server) or client_id or "" if not resolved_client_id: raise HTTPException( status_code=400, @@ -1227,7 +1322,6 @@ async def register_client(request: Request, mcp_server_name: Optional[str] = Non grant_types=data.get("grant_types", []), response_types=data.get("response_types", []), token_endpoint_auth_method=data.get("token_endpoint_auth_method", ""), - fallback_client_id=resolved.server_name or resolved.name, ) return dummy_return @@ -1241,5 +1335,4 @@ async def register_client(request: Request, mcp_server_name: Optional[str] = Non grant_types=data.get("grant_types", []), response_types=data.get("response_types", []), token_endpoint_auth_method=data.get("token_endpoint_auth_method", ""), - fallback_client_id=mcp_server_name, ) diff --git a/litellm/proxy/_experimental/mcp_server/slack_app_provisioning.py b/litellm/proxy/_experimental/mcp_server/slack_app_provisioning.py new file mode 100644 index 00000000000..795086e65e6 --- /dev/null +++ b/litellm/proxy/_experimental/mcp_server/slack_app_provisioning.py @@ -0,0 +1,175 @@ +"""Manifest-assisted provisioning for the hosted Slack MCP server. + +Slack does not support Dynamic Client Registration, so a gateway needs a +registered Slack app's ``client_id``/``client_secret`` to run the user-token +OAuth flow against ``mcp.slack.com``. This module creates that app for the +operator from a manifest via Slack's ``apps.manifest.create`` API, so they never +hand-build a manifest or copy credentials. The operator still completes the +governance steps Slack requires by hand (enable the MCP toggle, then publish or +make the app internal with admin approval); those cannot be automated. +""" + +import json +from dataclasses import dataclass +from typing import Optional, cast +from urllib.parse import urlparse + +from fastapi import HTTPException + +from litellm.llms.custom_httpx.http_handler import ( + get_async_httpx_client, + httpxSpecialProvider, +) + +SLACK_DEFAULT_API_BASE = "https://slack.com/api" + +# Slack rotates user tokens at oauth.v2.access, a different endpoint than the +# oauth.v2.user.access used for the initial code exchange, so a refresh cannot +# reuse the server's token_url the way an RFC 6749 provider does. +SLACK_MCP_TOKEN_REFRESH_URL = "https://slack.com/api/oauth.v2.access" + + +def slack_token_refresh_url(token_url: Optional[str]) -> Optional[str]: + """Return the endpoint that refreshes a Slack user token, else ``token_url``. + + Only Slack hosts are remapped; every other provider keeps the single token + endpoint used for both the authorization_code and refresh_token grants. + """ + if token_url is None: + return None + host = urlparse(token_url).hostname + if host == "slack.com" or (host is not None and host.endswith(".slack.com")): + return SLACK_MCP_TOKEN_REFRESH_URL + return token_url + + +# User scopes the hosted Slack MCP server requests. Kept in sync with the +# scopes Slack's MCP authorize flow asks for. +SLACK_MCP_USER_SCOPES: list[str] = [ + "search:read.public", + "search:read.private", + "search:read.mpim", + "search:read.im", + "search:read.files", + "search:read.users", + "chat:write", + "channels:history", + "groups:history", + "im:history", + "mpim:history", + "channels:read", + "groups:read", + "mpim:read", + "channels:write", + "groups:write", + "im:write", + "mpim:write", + "canvases:read", + "canvases:write", + "users:read", + "users:read.email", + "reactions:read", + "reactions:write", + "emoji:read", + "files:read", +] + + +@dataclass(frozen=True) +class SlackProvisionedApp: + app_id: str + client_id: str + client_secret: str + + +def _string_field(mapping: dict[str, object], key: str) -> str: + value = mapping.get(key) + if isinstance(value, str) and value.strip(): + return value + raise HTTPException( + status_code=502, + detail=f"Slack apps.manifest.create response omitted {key}", + ) + + +def build_slack_mcp_manifest( + callback_url: str, + app_name: str, + user_scopes: Optional[list[str]] = None, +) -> dict[str, object]: + """Build a Slack app manifest for the hosted MCP server. + + ``callback_url`` is this gateway's OAuth callback so the registered redirect + URL always matches the deployment's own domain. ``user_scopes`` overrides the + default MCP scope set so operators can track Slack's evolving scope surface + without a code change. + """ + return { + "display_information": { + "name": app_name, + "description": "Connects this gateway's users to the Slack MCP server", + }, + "oauth_config": { + "redirect_urls": [callback_url], + "scopes": {"user": user_scopes or SLACK_MCP_USER_SCOPES}, + }, + "settings": { + "org_deploy_enabled": False, + "socket_mode_enabled": False, + "token_rotation_enabled": True, + }, + } + + +async def provision_slack_app( + app_config_token: str, + manifest: dict[str, object], + slack_api_base: str = SLACK_DEFAULT_API_BASE, +) -> SlackProvisionedApp: + """Create a Slack app from ``manifest`` via ``apps.manifest.create``. + + ``app_config_token`` is a Slack app-configuration token the operator + generates once; it is used for this single call and never stored. + """ + async_client = get_async_httpx_client(llm_provider=httpxSpecialProvider.Oauth2Register) + response = await async_client.post( + f"{slack_api_base.rstrip('/')}/apps.manifest.create", + headers={ + "Authorization": f"Bearer {app_config_token}", + "Content-Type": "application/json", + }, + json={"manifest": json.dumps(manifest)}, + ) + if response is None: + raise HTTPException( + status_code=502, + detail="Slack apps.manifest.create returned no response", + ) + + raw: object = response.json() + if not isinstance(raw, dict): + raise HTTPException( + status_code=502, + detail="Slack apps.manifest.create returned a malformed response", + ) + payload = cast(dict[str, object], raw) + if payload.get("ok") is not True: + error = payload.get("error") + detail = error if isinstance(error, str) else "unknown_error" + raise HTTPException( + status_code=502, + detail=f"Slack apps.manifest.create failed: {detail}", + ) + + credentials = payload.get("credentials") + if not isinstance(credentials, dict): + raise HTTPException( + status_code=502, + detail="Slack apps.manifest.create response omitted app credentials", + ) + + return SlackProvisionedApp( + app_id=_string_field(payload, "app_id"), + client_id=_string_field(cast(dict[str, object], credentials), "client_id"), + client_secret=_string_field(cast(dict[str, object], credentials), "client_secret"), + ) diff --git a/litellm/proxy/management_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index ab9d04a4eb4..5912cea7e3c 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -132,6 +132,7 @@ if MCP_AVAILABLE: update_mcp_server, ) from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + _effective_client_id, authorize_with_server, exchange_token_with_server, get_request_base_url, @@ -1623,8 +1624,9 @@ if MCP_AVAILABLE: scope: Optional[str] = None, ): mcp_server = await _get_cached_temporary_mcp_server_or_404(server_id, user_api_key_dict, request=request) - # Use the server's stored client_id when the caller doesn't supply one - resolved_client_id = mcp_server.client_id or client_id or "" + # Use the server's stored or gateway-managed client_id when the caller + # doesn't supply one + resolved_client_id = _effective_client_id(mcp_server) or client_id or "" if not resolved_client_id: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, @@ -1667,7 +1669,7 @@ if MCP_AVAILABLE: scope: Optional[str] = Form(None), ): mcp_server = await _get_cached_temporary_mcp_server_or_404(server_id, user_api_key_dict, request=request) - resolved_client_id = mcp_server.client_id or client_id or "" + resolved_client_id = _effective_client_id(mcp_server) or client_id or "" if not resolved_client_id: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, @@ -1713,9 +1715,110 @@ if MCP_AVAILABLE: grant_types=data.get("grant_types", []), response_types=data.get("response_types", []), token_endpoint_auth_method=data.get("token_endpoint_auth_method", ""), - fallback_client_id=server_id, ) + @router.post( + "/server/oauth/slack/provision", + include_in_schema=False, + dependencies=[Depends(user_api_key_auth)], + ) + async def mcp_provision_slack_app( + request: Request, + user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth), + ): + """Create a Slack app for the hosted Slack MCP server from a manifest and + store its credentials as the gateway-managed OAuth app for slack.com. + + The operator supplies a one-time Slack app-configuration token; the + gateway builds the manifest (this deployment's callback + MCP user + scopes), creates the app, and persists client_id plus an encrypted + client_secret. Enabling MCP and publishing/approving the app remain + manual Slack steps. + """ + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + get_request_base_url, + ) + from litellm.proxy._experimental.mcp_server.slack_app_provisioning import ( + build_slack_mcp_manifest, + provision_slack_app, + ) + from litellm.proxy.proxy_server import general_settings, proxy_config + + if user_api_key_dict.user_role != LitellmUserRoles.PROXY_ADMIN: + raise HTTPException( + status_code=403, + detail={"error": "Only proxy admins can provision a Slack MCP app"}, + ) + + body = await _read_request_body(request=request) + app_config_token = body.get("app_config_token") + if not isinstance(app_config_token, str) or not app_config_token.strip(): + raise HTTPException(status_code=400, detail={"error": "app_config_token is required"}) + requested_name = body.get("app_name") + app_name = ( + requested_name if isinstance(requested_name, str) and requested_name.strip() else "LiteLLM MCP Connector" + ) + requested_scopes = body.get("user_scopes") + user_scopes = ( + [scope for scope in requested_scopes if isinstance(scope, str)] + if isinstance(requested_scopes, list) + else None + ) + force = body.get("force") is True + + config = await proxy_config.get_config() + config_general = config.get("general_settings") + if not isinstance(config_general, dict): + config_general = {} + config["general_settings"] = config_general + managed_apps = config_general.get("mcp_managed_oauth_apps") + if not isinstance(managed_apps, dict): + managed_apps = {} + config_general["mcp_managed_oauth_apps"] = managed_apps + + # Refuse to replace a working app (which would mint a new Slack app and + # invalidate every active user session) unless the caller opts in + if "slack.com" in managed_apps and not force: + raise HTTPException( + status_code=409, + detail={ + "error": "A managed Slack OAuth app is already configured. " + "Re-provisioning creates a new Slack app and invalidates " + "existing user sessions; pass force=true to replace it." + }, + ) + + callback_url = f"{get_request_base_url(request)}/callback" + provisioned = await provision_slack_app( + app_config_token=app_config_token.strip(), + manifest=build_slack_mcp_manifest(callback_url=callback_url, app_name=app_name, user_scopes=user_scopes), + ) + + stored_app = { + "client_id": provisioned.client_id, + "client_secret": encrypt_value_helper(provisioned.client_secret), + } + managed_apps["slack.com"] = stored_app + await proxy_config.save_config(new_config=config) + + # Reflect into the running process so the app works without a restart + runtime_apps = general_settings.get("mcp_managed_oauth_apps") + if not isinstance(runtime_apps, dict): + runtime_apps = {} + general_settings["mcp_managed_oauth_apps"] = runtime_apps + runtime_apps["slack.com"] = stored_app + + return { + "app_id": provisioned.app_id, + "client_id": provisioned.client_id, + "redirect_url": callback_url, + "next_steps": [ + "In Slack app settings, enable the Slack MCP Server toggle", + "Publish the app to the directory, or make it internal and have a workspace admin approve it", + "Users can then click Authorize and Allow to connect", + ], + } + @router.delete( "/server/{server_id}", description="Allows deleting mcp serves in the db", diff --git a/tests/litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py b/tests/litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py index 2a8768df722..65bc6962c17 100644 --- a/tests/litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py +++ b/tests/litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py @@ -339,7 +339,7 @@ async def test_register_client_returns_existing_server_credentials(): global_mcp_server_manager.registry.clear() assert result == { - "client_id": "stored_server", + "client_id": "existing-client", "client_secret": "dummy", "redirect_uris": ["https://proxy.litellm.example/callback"], } diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_db_credentials.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_db_credentials.py index c230cfd6cd0..4b59159ae36 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_db_credentials.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_db_credentials.py @@ -643,3 +643,66 @@ async def test_rotate_user_env_vars_skips_undecryptable_rows(): assert prisma.db.litellm_mcpuserenvvars.update.call_count == 1 where = prisma.db.litellm_mcpuserenvvars.update.call_args.kwargs["where"] assert where["user_id_server_id"]["server_id"] == "srv-ok" + + +@pytest.mark.asyncio +async def test_refresh_resolves_managed_client_id_and_slack_rotation_endpoint( + monkeypatch, +): + # A token minted via a gateway-managed Slack app carries no per-server + # client_id. The refresh must resolve the managed app's client_id/secret (not + # omit them, which the provider rejects) and POST to Slack's rotation endpoint + # oauth.v2.access, not the oauth.v2.user.access used for the initial exchange. + import types + + import litellm.proxy._experimental.mcp_server.db as db_mod + + server = types.SimpleNamespace( + token_url="https://slack.com/api/oauth.v2.user.access", + server_id="srv-1", + client_id=None, + client_secret=None, + authorization_url="https://slack.com/oauth/v2_user/authorize", + ) + + mock_response = MagicMock() + mock_response.json.return_value = { + "access_token": "xoxp-new", + "refresh_token": "xoxe-1-new", + "expires_in": 43200, + } + mock_response.raise_for_status = MagicMock() + mock_client = MagicMock() + mock_client.post = AsyncMock(return_value=mock_response) + + monkeypatch.setattr(db_mod, "get_async_httpx_client", lambda **_: mock_client) + monkeypatch.setattr(db_mod, "store_user_oauth_credential", AsyncMock()) + monkeypatch.setattr( + db_mod, + "get_user_oauth_credential", + AsyncMock(return_value={"access_token": "xoxp-new"}), + ) + monkeypatch.setattr( + "litellm.proxy.proxy_server.general_settings", + { + "mcp_managed_oauth_apps": { + "slack.com": {"client_id": "111.222", "client_secret": "shh"} + } + }, + raising=False, + ) + + result = await db_mod.refresh_user_oauth_token( + prisma_client=MagicMock(), + user_id="alice", + server=server, + cred={"refresh_token": "xoxe-1-old"}, + ) + + assert result == {"access_token": "xoxp-new"} + call = mock_client.post.call_args + assert call.args[0] == "https://slack.com/api/oauth.v2.access" + assert call.kwargs["data"]["client_id"] == "111.222" + assert call.kwargs["data"]["client_secret"] == "shh" + assert call.kwargs["data"]["grant_type"] == "refresh_token" + assert call.kwargs["data"]["refresh_token"] == "xoxe-1-old" diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py index 6fd935e3364..17656c68aa2 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py @@ -435,12 +435,241 @@ async def test_register_client_returns_existing_server_credentials(): global_mcp_server_manager.registry.clear() assert result == { - "client_id": "stored_server", + "client_id": "existing-client", "client_secret": "dummy", "redirect_uris": ["https://proxy.litellm.example/callback"], } +def test_resolve_config_secret_handles_plain_and_encrypted(monkeypatch): + """A managed-app secret may be operator-typed plaintext or stored encrypted + by provisioning; both must resolve to the same usable value.""" + try: + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + _resolve_config_secret, + ) + from litellm.proxy.common_utils.encrypt_decrypt_utils import ( + encrypt_value_helper, + ) + except ImportError: + pytest.skip("MCP discoverable endpoints not available") + + monkeypatch.setenv("LITELLM_SALT_KEY", "sk-salt-1234567890") + + assert _resolve_config_secret("1601185624273.8899143856786") == ( + "1601185624273.8899143856786" + ) + assert _resolve_config_secret(None) is None + assert _resolve_config_secret(" ") is None + + encrypted = encrypt_value_helper("real-secret") + assert encrypted != "real-secret" + assert _resolve_config_secret(encrypted) == "real-secret" + + +def test_managed_oauth_app_matches_host_ignoring_port(): + """authorization_url with an explicit port must still match a bare-host key.""" + try: + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + _managed_oauth_app, + ) + except ImportError: + pytest.skip("MCP discoverable endpoints not available") + + with patch.dict( + "litellm.proxy.proxy_server.general_settings", + {"mcp_managed_oauth_apps": {"slack.com": {"client_id": "123.456"}}}, + clear=False, + ): + app = _managed_oauth_app("https://slack.com:443/oauth/v2_user/authorize") + + assert app is not None + assert app.client_id == "123.456" + + +@pytest.mark.asyncio +async def test_register_client_without_dcr_and_no_client_id_does_not_leak_server_id(): + """Regression: a Slack-style OAuth2 server (no DCR registration_url and no + stored client_id) must not have its internal server_id returned as the OAuth + client_id. Slack rejects such a value with "Invalid client_id parameter".""" + try: + from fastapi import HTTPException, Request + + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + register_client_with_server, + ) + from litellm.proxy._types import MCPTransport + from litellm.types.mcp import MCPAuth + from litellm.types.mcp_server.mcp_server_manager import MCPServer + except ImportError: + pytest.skip("MCP discoverable endpoints not available") + + server_id = "eb8070ac-efe6-4470-9b1b-9646fe1b39f8" + slack_server = MCPServer( + server_id=server_id, + name="slack", + server_name="slack", + alias="slack", + transport=MCPTransport.http, + auth_type=MCPAuth.oauth2, + client_id=None, + client_secret=None, + authorization_url="https://slack.com/oauth/v2_user/authorize", + token_url="https://slack.com/api/oauth.v2.user.access", + registration_url=None, + ) + + mock_request = MagicMock(spec=Request) + mock_request.base_url = "https://gateway.example/" + mock_request.headers = {} + + with pytest.raises(HTTPException) as exc_info: + await register_client_with_server( + request=mock_request, + mcp_server=slack_server, + client_name="", + grant_types=[], + response_types=[], + token_endpoint_auth_method="", + ) + + assert exc_info.value.status_code == 400 + assert server_id not in str(exc_info.value.detail) + + +@pytest.mark.asyncio +async def test_register_client_uses_managed_oauth_app(): + """A server with no stored client_id inherits the gateway-managed OAuth app + registered for its authorization host, instead of failing or leaking an id.""" + try: + from fastapi import Request + + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + register_client_with_server, + ) + from litellm.proxy._types import MCPTransport + from litellm.types.mcp import MCPAuth + from litellm.types.mcp_server.mcp_server_manager import MCPServer + except ImportError: + pytest.skip("MCP discoverable endpoints not available") + + slack_server = MCPServer( + server_id="eb8070ac-efe6-4470-9b1b-9646fe1b39f8", + name="slack", + server_name="slack", + alias="slack", + transport=MCPTransport.http, + auth_type=MCPAuth.oauth2, + client_id=None, + client_secret=None, + authorization_url="https://slack.com/oauth/v2_user/authorize", + token_url="https://slack.com/api/oauth.v2.user.access", + registration_url=None, + ) + + mock_request = MagicMock(spec=Request) + mock_request.base_url = "https://gateway.example/" + mock_request.headers = {} + + with patch.dict( + "litellm.proxy.proxy_server.general_settings", + { + "mcp_managed_oauth_apps": { + "slack.com": { + "client_id": "1601185624273.8899143856786", + "client_secret": "managed-secret", + } + } + }, + clear=False, + ): + result = await register_client_with_server( + request=mock_request, + mcp_server=slack_server, + client_name="", + grant_types=[], + response_types=[], + token_endpoint_auth_method="", + ) + + assert result == { + "client_id": "1601185624273.8899143856786", + "client_secret": "dummy", + "redirect_uris": ["https://gateway.example/callback"], + } + + +@pytest.mark.asyncio +async def test_authorize_uses_managed_oauth_app_client_id_not_server_id(): + """End-to-end regression: the authorize redirect must carry the managed + Slack client_id, never the internal server_id that Slack rejects.""" + try: + from fastapi import Request + + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + authorize, + ) + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + global_mcp_server_manager, + ) + from litellm.proxy._types import MCPTransport + from litellm.types.mcp import MCPAuth + from litellm.types.mcp_server.mcp_server_manager import MCPServer + except ImportError: + pytest.skip("MCP discoverable endpoints not available") + + global_mcp_server_manager.registry.clear() + server_id = "eb8070ac-efe6-4470-9b1b-9646fe1b39f8" + slack_server = MCPServer( + server_id=server_id, + name="slack", + server_name="slack", + alias="slack", + transport=MCPTransport.http, + auth_type=MCPAuth.oauth2, + client_id=None, + client_secret=None, + authorization_url="https://slack.com/oauth/v2_user/authorize", + token_url="https://slack.com/api/oauth.v2.user.access", + scopes=["search:read.public", "chat:write"], + ) + global_mcp_server_manager.registry[slack_server.server_id] = slack_server + + mock_request = MagicMock(spec=Request) + mock_request.base_url = "https://gateway.example/" + mock_request.headers = {} + + try: + with ( + patch( + "litellm.proxy._experimental.mcp_server.discoverable_endpoints.encrypt_value_helper", + return_value="mocked_encrypted_state", + ), + patch.dict( + "litellm.proxy.proxy_server.general_settings", + { + "mcp_managed_oauth_apps": { + "slack.com": {"client_id": "1601185624273.8899143856786"} + } + }, + clear=False, + ), + ): + response = await authorize( + request=mock_request, + client_id=server_id, + mcp_server_name="slack", + redirect_uri="http://127.0.0.1:60108/callback", + state="test_state", + ) + finally: + global_mcp_server_manager.registry.clear() + + location = response.headers["location"] + assert "client_id=1601185624273.8899143856786" in location + assert server_id not in location + + @pytest.mark.asyncio async def test_register_client_remote_registration_success(): try: @@ -1885,8 +2114,8 @@ async def test_register_root_resolves_single_oauth2_server(): ): result = await register_client(request=mock_request, mcp_server_name=None) - # Should resolve to the single server and return its name as client_id - assert result["client_id"] == "test_oauth" + # Should resolve to the single server and return its real client_id + assert result["client_id"] == "test_client_id" assert "redirect_uris" in result finally: global_mcp_server_manager.registry.clear() diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_slack_app_provisioning.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_slack_app_provisioning.py new file mode 100644 index 00000000000..1fec7a58cdd --- /dev/null +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_slack_app_provisioning.py @@ -0,0 +1,129 @@ +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + + +def test_build_slack_mcp_manifest_uses_callback_and_user_scopes(): + from litellm.proxy._experimental.mcp_server.slack_app_provisioning import ( + SLACK_MCP_USER_SCOPES, + build_slack_mcp_manifest, + ) + + manifest = build_slack_mcp_manifest( + callback_url="https://gw.acme.com/callback", app_name="Acme Connector" + ) + + assert manifest["display_information"]["name"] == "Acme Connector" + assert manifest["oauth_config"]["redirect_urls"] == ["https://gw.acme.com/callback"] + # MCP is a user-token flow: scopes must be declared under "user", not "bot" + assert manifest["oauth_config"]["scopes"]["user"] == SLACK_MCP_USER_SCOPES + assert "bot" not in manifest["oauth_config"]["scopes"] + assert "search:read.public" in manifest["oauth_config"]["scopes"]["user"] + + +def test_build_slack_mcp_manifest_allows_scope_override(): + from litellm.proxy._experimental.mcp_server.slack_app_provisioning import ( + build_slack_mcp_manifest, + ) + + manifest = build_slack_mcp_manifest( + callback_url="https://gw.acme.com/callback", + app_name="Acme", + user_scopes=["chat:write", "channels:read"], + ) + assert manifest["oauth_config"]["scopes"]["user"] == ["chat:write", "channels:read"] + + +def test_build_slack_mcp_manifest_enables_token_rotation(): + from litellm.proxy._experimental.mcp_server.slack_app_provisioning import ( + build_slack_mcp_manifest, + ) + + manifest = build_slack_mcp_manifest( + callback_url="https://gw.acme.com/callback", app_name="Acme" + ) + assert manifest["settings"]["token_rotation_enabled"] is True + + +def test_slack_token_refresh_url_remaps_only_slack_hosts(): + from litellm.proxy._experimental.mcp_server.slack_app_provisioning import ( + SLACK_MCP_TOKEN_REFRESH_URL, + slack_token_refresh_url, + ) + + # Slack refreshes at oauth.v2.access, not the oauth.v2.user.access exchange URL + assert ( + slack_token_refresh_url("https://slack.com/api/oauth.v2.user.access") + == SLACK_MCP_TOKEN_REFRESH_URL + ) + # every other provider keeps its single token endpoint for refresh + assert ( + slack_token_refresh_url("https://auth.example.com/oauth/token") + == "https://auth.example.com/oauth/token" + ) + assert slack_token_refresh_url(None) is None + + +@pytest.mark.asyncio +async def test_provision_slack_app_returns_credentials(): + from litellm.proxy._experimental.mcp_server.slack_app_provisioning import ( + provision_slack_app, + ) + + mock_response = MagicMock() + mock_response.json.return_value = { + "ok": True, + "app_id": "A0123", + "credentials": { + "client_id": "1601185624273.8899143856786", + "client_secret": "real-secret", + "signing_secret": "sign", + }, + } + mock_client = MagicMock() + mock_client.post = AsyncMock(return_value=mock_response) + + with patch( + "litellm.proxy._experimental.mcp_server.slack_app_provisioning.get_async_httpx_client", + return_value=mock_client, + ): + result = await provision_slack_app( + app_config_token="xoxe.xoxp-token", + manifest={"display_information": {"name": "x"}}, + ) + + assert result.app_id == "A0123" + assert result.client_id == "1601185624273.8899143856786" + assert result.client_secret == "real-secret" + + call = mock_client.post.call_args + assert call.args[0] == "https://slack.com/api/apps.manifest.create" + assert call.kwargs["headers"]["Authorization"] == "Bearer xoxe.xoxp-token" + assert "manifest" in call.kwargs["json"] + + +@pytest.mark.asyncio +async def test_provision_slack_app_raises_on_slack_error(): + from fastapi import HTTPException + + from litellm.proxy._experimental.mcp_server.slack_app_provisioning import ( + provision_slack_app, + ) + + mock_response = MagicMock() + mock_response.json.return_value = {"ok": False, "error": "invalid_auth"} + mock_client = MagicMock() + mock_client.post = AsyncMock(return_value=mock_response) + + with patch( + "litellm.proxy._experimental.mcp_server.slack_app_provisioning.get_async_httpx_client", + return_value=mock_client, + ): + with pytest.raises(HTTPException) as exc_info: + await provision_slack_app( + app_config_token="bad-token", + manifest={"display_information": {"name": "x"}}, + ) + + assert exc_info.value.status_code == 502 + assert "invalid_auth" in str(exc_info.value.detail) diff --git a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py index f40904e234d..b8d8f9b9ae4 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py @@ -2263,7 +2263,6 @@ class TestTemporaryMCPSessionEndpoints: grant_types=["authorization_code"], response_types=["code"], token_endpoint_auth_method="client_secret_basic", - fallback_client_id="server-1", ) @pytest.mark.asyncio @@ -5034,3 +5033,148 @@ class TestPerUserCredentialConfigServerResolution: _, _, _, updates, _ = merge_mock.await_args.args assert updates == {"CORP_USERNAME": "alice"} assert result.server_id == self.CONFIG_SERVER_ID + + +@pytest.mark.asyncio +async def test_provision_slack_app_stores_encrypted_managed_app(monkeypatch): + """The provision endpoint derives the callback from the deployed gateway URL, + stores the client_id and an encrypted client_secret under + general_settings.mcp_managed_oauth_apps['slack.com'], and reflects it into + the running process.""" + from litellm.proxy._experimental.mcp_server.slack_app_provisioning import ( + SlackProvisionedApp, + ) + from litellm.proxy.common_utils.encrypt_decrypt_utils import decrypt_value_helper + + monkeypatch.setenv("LITELLM_SALT_KEY", "sk-salt-1234567890") + + req = MagicMock() + req.base_url = "https://gateway.acme.com/" + req.headers = {} + + saved: dict = {} + + async def fake_get_config(): + return {} + + async def fake_save_config(new_config): + saved["config"] = new_config + + runtime_general_settings: dict = {} + fake_proxy_server = types.SimpleNamespace( + general_settings=runtime_general_settings, + master_key="sk-1234", + proxy_config=types.SimpleNamespace( + get_config=fake_get_config, save_config=fake_save_config + ), + ) + + provisioned = SlackProvisionedApp( + app_id="A1", client_id="123.456", client_secret="top-secret" + ) + + with ( + patch.dict(sys.modules, {"litellm.proxy.proxy_server": fake_proxy_server}), + patch( + "litellm.proxy._experimental.mcp_server.slack_app_provisioning.provision_slack_app", + new=AsyncMock(return_value=provisioned), + ), + patch.object( + mgmt_endpoints, + "_read_request_body", + new=AsyncMock( + return_value={"app_config_token": "xoxe-tok", "app_name": "Acme"} + ), + ), + ): + result = await mgmt_endpoints.mcp_provision_slack_app( + request=req, user_api_key_dict=generate_mock_user_api_key_auth() + ) + + assert result["app_id"] == "A1" + assert result["client_id"] == "123.456" + assert result["redirect_url"] == "https://gateway.acme.com/callback" + + stored = saved["config"]["general_settings"]["mcp_managed_oauth_apps"]["slack.com"] + assert stored["client_id"] == "123.456" + assert stored["client_secret"] != "top-secret" + assert ( + decrypt_value_helper( + value=stored["client_secret"], key="x", return_original_value=True + ) + == "top-secret" + ) + assert ( + runtime_general_settings["mcp_managed_oauth_apps"]["slack.com"]["client_id"] + == "123.456" + ) + + +@pytest.mark.asyncio +async def test_provision_slack_app_requires_admin(): + """Non-admins cannot provision a Slack MCP app.""" + req = MagicMock() + req.base_url = "https://gateway.acme.com/" + req.headers = {} + + with patch.object( + mgmt_endpoints, + "_read_request_body", + new=AsyncMock(return_value={"app_config_token": "x"}), + ): + with pytest.raises(HTTPException) as exc_info: + await mgmt_endpoints.mcp_provision_slack_app( + request=req, + user_api_key_dict=generate_mock_user_api_key_auth( + user_role=LitellmUserRoles.INTERNAL_USER + ), + ) + + assert exc_info.value.status_code == 403 + + +@pytest.mark.asyncio +async def test_provision_slack_app_refuses_overwrite_without_force(): + """Re-provisioning over an existing managed app must 409 without force, and + must not create a throwaway Slack app.""" + req = MagicMock() + req.base_url = "https://gateway.acme.com/" + req.headers = {} + + async def fake_get_config(): + return { + "general_settings": { + "mcp_managed_oauth_apps": {"slack.com": {"client_id": "existing"}} + } + } + + save_mock = AsyncMock() + fake_proxy_server = types.SimpleNamespace( + general_settings={}, + master_key="sk-1234", + proxy_config=types.SimpleNamespace( + get_config=fake_get_config, save_config=save_mock + ), + ) + provision_mock = AsyncMock() + + with ( + patch.dict(sys.modules, {"litellm.proxy.proxy_server": fake_proxy_server}), + patch( + "litellm.proxy._experimental.mcp_server.slack_app_provisioning.provision_slack_app", + new=provision_mock, + ), + patch.object( + mgmt_endpoints, + "_read_request_body", + new=AsyncMock(return_value={"app_config_token": "tok"}), + ), + ): + with pytest.raises(HTTPException) as exc_info: + await mgmt_endpoints.mcp_provision_slack_app( + request=req, user_api_key_dict=generate_mock_user_api_key_auth() + ) + + assert exc_info.value.status_code == 409 + provision_mock.assert_not_awaited() + save_mock.assert_not_awaited()