mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-26 06:03:19 +00:00
fix(mcp)!: backport the OAuth Delegate fields and gateway-owned DCR bridge client to rc/1.107.0 (#46941, #46940) (#47153)
* feat(ui): backport the OAuth Delegate endpoint and scope fields to rc/1.107.0 (#46941) Backport of #46941 to rc/1.107.0. Cherry-picked from3637dd711fCo-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(mcp)!: backport the gateway-owned DCR client for bridge servers to rc/1.107.0 (#46940) Backport of #46940 to rc/1.107.0. Cherry-picked from7049a5ff4dCo-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
Yassin Kortam
GitHub
parent
bfd9594d8e
commit
cda17c8e81
12 changed files with 644 additions and 29 deletions
|
|
@ -862,12 +862,17 @@ def _raise_unless_oauth2_discovery_server(
|
|||
|
||||
def _dcr_bridge_relays_client_registration(mcp_server: MCPServer) -> bool:
|
||||
"""True when a DCR-bridge server relays client registration to the upstream authorization
|
||||
server instead of short-circuiting to an admin-configured OAuth client. In the relay arm the
|
||||
server instead of short-circuiting to the server's own OAuth client. In the relay arm the
|
||||
upstream holds each client's own registration, so the authorize and token relays pass the
|
||||
client's ``client_id`` and ``redirect_uri`` through verbatim and the authorization code
|
||||
returns directly to the client's redirect URI without transiting the gateway. Gateway-side
|
||||
redirect trust and the ``/callback`` state relay therefore only apply to the short-circuit
|
||||
arm, where the upstream only knows the gateway's own callback."""
|
||||
arm, where the upstream only knows the gateway's own callback.
|
||||
|
||||
Authorize and token decide per request from the server's current client: once the gateway
|
||||
holds its shared client (registered at ``/register`` under ``PROXY_BASE_URL``, or entered by an
|
||||
admin) every caller runs the gateway-callback arm, while a server that still has none keeps
|
||||
relaying, so clients registered before the shared client existed keep their own binding."""
|
||||
return bool(
|
||||
mcp_server.is_dcr_bridge
|
||||
and mcp_server.effective_registration_url
|
||||
|
|
@ -880,6 +885,27 @@ def _dcr_bridge_relays_client_registration(mcp_server: MCPServer) -> bool:
|
|||
)
|
||||
|
||||
|
||||
def _register_relays_bridge_client(mcp_server: MCPServer) -> bool:
|
||||
"""``/register`` relays the caller's own registration only when the gateway cannot keep a shared
|
||||
client: with ``PROXY_BASE_URL`` set it registers one client bound to its own ``/callback`` and
|
||||
keeps it as the server's client, so an upstream that restricts redirect domains (Atlassian)
|
||||
never sees a laptop loopback redirect. Without a database the shared client would live only in
|
||||
this process, so a database-free gateway keeps relaying as well."""
|
||||
return _dcr_bridge_relays_client_registration(mcp_server) and (
|
||||
configured_proxy_base_url() is None or not _shared_client_store_available()
|
||||
)
|
||||
|
||||
|
||||
def _shared_client_store_available() -> bool:
|
||||
from litellm.proxy.utils import get_prisma_client_or_throw # noqa: PLC0415 # avoids circular import
|
||||
|
||||
try:
|
||||
get_prisma_client_or_throw("Database not connected. Cannot keep the shared MCP OAuth client.")
|
||||
except HTTPException:
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
def _require_s256_pkce(
|
||||
code_challenge: str | None,
|
||||
code_challenge_method: str | None,
|
||||
|
|
@ -1028,6 +1054,17 @@ async def _managed_authorization_scope(
|
|||
return selected_scope, scope_user_id
|
||||
|
||||
|
||||
async def _hydrate_bridge_shared_client(mcp_server: MCPServer, request: Request) -> None:
|
||||
"""Whichever worker served ``/register`` persisted the bridge's shared gateway client; a worker
|
||||
whose catalog snapshot predates that write still holds no ``client_id``, so the gateway-callback
|
||||
arms load the persisted client before presenting one upstream. Callers that carry their own
|
||||
client (a gateway-minted ephemeral client sealed into the state or the code) skip this, since the
|
||||
upstream bound their code to that client."""
|
||||
if not mcp_server.is_dcr_bridge or mcp_server.client_id:
|
||||
return
|
||||
await _reuse_persisted_dcr_client_if_available(mcp_server, f"{get_request_base_url(request)}/callback")
|
||||
|
||||
|
||||
async def authorize_with_server(
|
||||
request: Request,
|
||||
mcp_server: MCPServer,
|
||||
|
|
@ -1069,6 +1106,8 @@ async def authorize_with_server(
|
|||
# calling this for its enforcement side effect, then falls through to the gateway
|
||||
# /callback flow below, which reads the original code_challenge names.
|
||||
bridge_challenge, bridge_method = _require_s256_pkce(code_challenge, code_challenge_method)
|
||||
if ephemeral_dcr_client is None:
|
||||
await _hydrate_bridge_shared_client(resolved_server, request)
|
||||
# A gateway-minted ephemeral client is registered against {base}/callback, so its
|
||||
# flow must run the short-circuit arm; the relay arm is only for clients that
|
||||
# registered themselves through the front door and hold their own redirect binding.
|
||||
|
|
@ -1233,6 +1272,7 @@ async def exchange_token_with_server(
|
|||
refresh_token: str | None = None,
|
||||
scope: str | None = None,
|
||||
client_token_endpoint_auth_method: MCPTokenEndpointAuthMethod | None = None,
|
||||
sealed_client: bool = False,
|
||||
):
|
||||
raise_if_not_oauth2(mcp_server)
|
||||
if grant_type not in ("authorization_code", "refresh_token"):
|
||||
|
|
@ -1282,11 +1322,14 @@ async def exchange_token_with_server(
|
|||
# recovered from a sealed code) must authenticate the way its own registration was granted,
|
||||
# not the way the server row is configured; callers that carry no method keep the row's method
|
||||
# as before.
|
||||
resolved_client_id: Final = resolved_server.client_id if resolved_server.client_id else client_id
|
||||
resolved_client_secret: Final = resolved_server.client_secret if resolved_server.client_id else client_secret
|
||||
if not sealed_client:
|
||||
await _hydrate_bridge_shared_client(resolved_server, request)
|
||||
server_client_wins: Final = bool(resolved_server.client_id) and not sealed_client
|
||||
resolved_client_id: Final = resolved_server.client_id if server_client_wins else client_id
|
||||
resolved_client_secret: Final = resolved_server.client_secret if server_client_wins else client_secret
|
||||
resolved_auth_method: Final = (
|
||||
resolved_server.token_endpoint_auth_method
|
||||
if resolved_server.client_id
|
||||
if server_client_wins
|
||||
else (client_token_endpoint_auth_method or resolved_server.token_endpoint_auth_method)
|
||||
)
|
||||
try:
|
||||
|
|
@ -1830,10 +1873,13 @@ async def _persist_dcr_client_registration(
|
|||
empty and otherwise adopts whatever a concurrent registration persisted first.
|
||||
|
||||
The client-forwarded token modes (``true_passthrough`` / ``oauth_delegate``) are skipped
|
||||
unconditionally: the caller holds the upstream token and the gateway must hold no OAuth
|
||||
client identity for these servers. Persisting here would stamp ``oauth2_flow`` and a
|
||||
``client_id`` onto a server whose mode promises the gateway stores nothing, making a
|
||||
fresh pass-through server read as gateway-authorized.
|
||||
unless the server is a DCR bridge: the caller holds the upstream token and the gateway holds
|
||||
no OAuth client identity for these servers, so persisting would stamp ``oauth2_flow`` and a
|
||||
``client_id`` onto a server whose mode promises the gateway stores nothing. A DCR bridge is
|
||||
the exception by design: its gateway-hosted sign-in registers one shared client bound to the
|
||||
gateway ``/callback`` (the same app an admin may enter by hand), and that client must survive
|
||||
for every later caller's authorize and token exchange. ``oauth2_flow`` is still never stamped
|
||||
on a client-forwarded row.
|
||||
|
||||
``redirect_uris`` records what the client is bound to so a later origin change can be
|
||||
detected as a positive mismatch and trigger re-registration instead of stranding the
|
||||
|
|
@ -1842,7 +1888,7 @@ async def _persist_dcr_client_registration(
|
|||
``update_mcp_server`` merges credential blobs: a re-registered public client must not
|
||||
inherit the previous client's secret or auth method.
|
||||
"""
|
||||
if mcp_server.is_client_forwarded_token:
|
||||
if mcp_server.is_client_forwarded_token and not mcp_server.is_dcr_bridge:
|
||||
return "skipped"
|
||||
|
||||
try:
|
||||
|
|
@ -1910,20 +1956,23 @@ async def _persist_dcr_client_registration(
|
|||
):
|
||||
await _refresh_persisted_dcr_server(stored)
|
||||
return "reused"
|
||||
stamped_flow: Final[Literal["authorization_code"] | None] = (
|
||||
None if mcp_server.is_client_forwarded_token else "authorization_code"
|
||||
)
|
||||
updated_row: Final = await update_mcp_server(
|
||||
prisma_client=prisma_client,
|
||||
data=(
|
||||
UpdateMCPServerRequest(
|
||||
server_id=mcp_server.server_id,
|
||||
credentials=credentials,
|
||||
oauth2_flow="authorization_code",
|
||||
oauth2_flow=stamped_flow,
|
||||
token_url=mcp_server.token_url,
|
||||
)
|
||||
if mcp_server.token_url
|
||||
else UpdateMCPServerRequest(
|
||||
server_id=mcp_server.server_id,
|
||||
credentials=credentials,
|
||||
oauth2_flow="authorization_code",
|
||||
oauth2_flow=stamped_flow,
|
||||
)
|
||||
),
|
||||
touched_by="mcp_oauth_dcr",
|
||||
|
|
@ -2211,7 +2260,7 @@ async def register_client_with_server(
|
|||
)
|
||||
return dummy_return
|
||||
|
||||
bridge_relay: Final = _dcr_bridge_relays_client_registration(resolved_server)
|
||||
bridge_relay: Final = _register_relays_bridge_client(resolved_server)
|
||||
if bridge_relay and not client_redirect_uris:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
|
|
|
|||
|
|
@ -2517,7 +2517,9 @@ if MCP_AVAILABLE:
|
|||
resolved_redirect_uri: Final = f"{get_request_base_url(request)}/callback" if sealed_code else redirect_uri
|
||||
caller_client_id: Final = sealed_code.client_id if sealed_code else client_id
|
||||
caller_client_secret: Final = sealed_code.client_secret if sealed_code else client_secret
|
||||
resolved_client_id: Final = mcp_server.client_id or caller_client_id or ""
|
||||
resolved_client_id: Final = (
|
||||
caller_client_id if sealed_code else (mcp_server.client_id or caller_client_id or "")
|
||||
)
|
||||
if not resolved_client_id:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
|
|
@ -2541,6 +2543,7 @@ if MCP_AVAILABLE:
|
|||
refresh_token=refresh_token,
|
||||
scope=scope,
|
||||
client_token_endpoint_auth_method=sealed_code.token_endpoint_auth_method if sealed_code else None,
|
||||
sealed_client=sealed_code is not None,
|
||||
)
|
||||
|
||||
@router.post(
|
||||
|
|
|
|||
|
|
@ -5,11 +5,12 @@ import json
|
|||
import time
|
||||
from base64 import urlsafe_b64encode
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from typing import TYPE_CHECKING, Final
|
||||
from typing import TYPE_CHECKING, Final, Literal
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from fastapi import HTTPException, Request
|
||||
from fastapi import HTTPException, Request, Response
|
||||
from typing_extensions import ReadOnly, TypedDict
|
||||
|
||||
from litellm.proxy._types import LiteLLM_MCPServerTable
|
||||
from litellm.types.mcp import MCPAuth
|
||||
|
|
@ -26,6 +27,14 @@ if TYPE_CHECKING:
|
|||
from litellm.types.mcp_server.mcp_server_manager import MCPServer
|
||||
|
||||
|
||||
class _BridgeRegistrationPayload(TypedDict, total=False):
|
||||
client_name: ReadOnly[str]
|
||||
grant_types: ReadOnly[list[str]]
|
||||
response_types: ReadOnly[list[str]]
|
||||
token_endpoint_auth_method: ReadOnly[str]
|
||||
redirect_uris: ReadOnly[list[str]]
|
||||
|
||||
|
||||
def _stored_grant(access_token="access-token", refresh_token=None, expires_in_seconds=None, expires_at=None):
|
||||
credential = {"type": "oauth2", "access_token": access_token}
|
||||
if refresh_token is not None:
|
||||
|
|
@ -5849,7 +5858,12 @@ def test_oauth_authorization_server_404_for_non_bridge_client_forwarded_server()
|
|||
assert exc.value.status_code == 404
|
||||
|
||||
|
||||
async def _bridge_register_response(server, request_payload, replace_stale_client=False):
|
||||
async def _bridge_register_response(
|
||||
server: "MCPServer",
|
||||
request_payload: _BridgeRegistrationPayload,
|
||||
replace_stale_client: bool = False,
|
||||
database_connected: bool = True,
|
||||
) -> tuple[Response, MagicMock, AsyncMock]:
|
||||
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
|
||||
register_client_with_server,
|
||||
)
|
||||
|
|
@ -5879,6 +5893,10 @@ async def _bridge_register_response(server, request_payload, replace_stale_clien
|
|||
new_callable=AsyncMock,
|
||||
return_value=False,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy.utils.get_prisma_client_or_throw",
|
||||
**({"return_value": MagicMock()} if database_connected else {"side_effect": HTTPException(status_code=500)}),
|
||||
),
|
||||
):
|
||||
response = await register_client_with_server(
|
||||
request=_bridge_mock_request(),
|
||||
|
|
@ -6033,6 +6051,309 @@ async def test_register_bridge_relay_never_persists():
|
|||
mock_persist.assert_not_called()
|
||||
|
||||
|
||||
_LAPTOP_LOOPBACK_REDIRECT: Final = "http://127.0.0.1:33418/"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("auth_type_value", ["oauth_delegate", "true_passthrough"])
|
||||
async def test_register_bridge_with_proxy_base_url_registers_gateway_client_not_laptop_loopback(
|
||||
monkeypatch: pytest.MonkeyPatch, auth_type_value: Literal["oauth_delegate", "true_passthrough"]
|
||||
) -> None:
|
||||
"""A laptop MCP client (VS Code) registers with its own loopback redirect. With PROXY_BASE_URL set
|
||||
the gateway must not relay that registration upstream: it registers its one shared client bound
|
||||
to the gateway /callback, keeps it, and hands the caller a client bound to the caller's loopback.
|
||||
Relaying the loopback is the customer failure on upstreams that restrict redirect domains."""
|
||||
import json
|
||||
|
||||
from litellm.types.mcp import MCPAuth
|
||||
|
||||
monkeypatch.setenv("PROXY_BASE_URL", "https://litellm.example.com")
|
||||
response, mock_async_client, mock_persist = await _bridge_register_response(
|
||||
_bridge_server(auth_type=MCPAuth(auth_type_value)),
|
||||
{"client_name": "Visual Studio Code", "redirect_uris": [_LAPTOP_LOOPBACK_REDIRECT]},
|
||||
)
|
||||
|
||||
posted: Final = mock_async_client.post.call_args.kwargs["json"]
|
||||
assert posted["redirect_uris"] == ["https://litellm.example.com/callback"], posted
|
||||
assert posted["client_name"] == "LiteLLM Proxy", posted
|
||||
mock_persist.assert_awaited_once()
|
||||
|
||||
payload: Final = json.loads(response.body.decode("utf-8"))
|
||||
assert payload["redirect_uris"] == [_LAPTOP_LOOPBACK_REDIRECT], payload
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_register_bridge_without_database_still_relays_client_registration(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""A database-free gateway cannot keep the shared client past this process, so even with
|
||||
PROXY_BASE_URL set it relays the caller's own registration instead of stranding the next authorize."""
|
||||
monkeypatch.setenv("PROXY_BASE_URL", "https://litellm.example.com")
|
||||
_, mock_async_client, mock_persist = await _bridge_register_response(
|
||||
_bridge_server(),
|
||||
{"client_name": "Visual Studio Code", "redirect_uris": [_LAPTOP_LOOPBACK_REDIRECT]},
|
||||
database_connected=False,
|
||||
)
|
||||
|
||||
assert mock_async_client.post.call_args.kwargs["json"]["redirect_uris"] == [_LAPTOP_LOOPBACK_REDIRECT]
|
||||
mock_persist.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_register_bridge_without_proxy_base_url_still_relays_client_registration(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Without PROXY_BASE_URL the gateway cannot keep a shared client, so the relay arm remains the
|
||||
fallback and the caller's own redirect goes upstream as before."""
|
||||
monkeypatch.delenv("PROXY_BASE_URL", raising=False)
|
||||
_, mock_async_client, mock_persist = await _bridge_register_response(
|
||||
_bridge_server(),
|
||||
{"client_name": "Visual Studio Code", "redirect_uris": [_LAPTOP_LOOPBACK_REDIRECT]},
|
||||
)
|
||||
|
||||
assert mock_async_client.post.call_args.kwargs["json"]["redirect_uris"] == [_LAPTOP_LOOPBACK_REDIRECT]
|
||||
mock_persist.assert_not_called()
|
||||
|
||||
|
||||
def _persisted_shared_gateway_client(
|
||||
server: "MCPServer", client_id: str = "shared-gateway-client"
|
||||
) -> LiteLLM_MCPServerTable:
|
||||
request: Final = _bridge_mock_request()
|
||||
credentials: Final = {
|
||||
"client_id": client_id,
|
||||
"client_secret": None,
|
||||
"token_endpoint_auth_method": None,
|
||||
"redirect_uris": [f"{request.base_url.rstrip('/')}/callback"],
|
||||
"dcr_issuer": server.issuer,
|
||||
"dcr_server_url": server.url,
|
||||
}
|
||||
return LiteLLM_MCPServerTable.model_validate({**server.model_dump(exclude_none=True), "credentials": credentials})
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
("shared_client_persisted", "expected_redirect_uri", "expected_client_id"),
|
||||
[
|
||||
(True, "https://litellm.example.com/callback", "shared-gateway-client"),
|
||||
(False, _LAPTOP_LOOPBACK_REDIRECT, "dcr-client-123"),
|
||||
],
|
||||
)
|
||||
async def test_authorize_clientless_bridge_loads_shared_client_before_choosing_the_arm(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
shared_client_persisted: bool,
|
||||
expected_redirect_uri: str,
|
||||
expected_client_id: str,
|
||||
) -> None:
|
||||
"""This worker's catalog snapshot has no client_id. When another worker already persisted the shared
|
||||
client, authorize must load it and send it with the gateway /callback upstream, never the caller's
|
||||
own client_id or loopback. When no shared client exists yet (a client registered before the gateway
|
||||
kept one), the caller keeps its own upstream binding through the relay."""
|
||||
from urllib.parse import parse_qs, urlparse
|
||||
|
||||
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import authorize_with_server
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import global_mcp_server_manager
|
||||
|
||||
monkeypatch.setenv("PROXY_BASE_URL", "https://litellm.example.com")
|
||||
monkeypatch.setenv("LITELLM_SALT_KEY", "test-registration-binding")
|
||||
server: Final = _bridge_server(url="https://mcp.example.com/mcp")
|
||||
persisted: Final = _persisted_shared_gateway_client(server) if shared_client_persisted else None
|
||||
prisma_client: Final = MagicMock()
|
||||
mock_get_mcp_server: Final = AsyncMock(return_value=persisted)
|
||||
mock_update_server: Final = AsyncMock()
|
||||
with (
|
||||
patch(
|
||||
"litellm.proxy.utils.get_prisma_client_or_throw",
|
||||
return_value=prisma_client,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.db.get_mcp_server",
|
||||
new=mock_get_mcp_server,
|
||||
),
|
||||
patch.object(
|
||||
global_mcp_server_manager,
|
||||
"update_server",
|
||||
new=mock_update_server,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.discoverable_endpoints.encrypt_value_helper",
|
||||
return_value="mocked_encrypted_state",
|
||||
),
|
||||
):
|
||||
response: Final = await authorize_with_server(
|
||||
request=_bridge_mock_request(),
|
||||
mcp_server=server,
|
||||
client_id="dcr-client-123",
|
||||
redirect_uri=_LAPTOP_LOOPBACK_REDIRECT,
|
||||
state="s",
|
||||
code_challenge="chal",
|
||||
code_challenge_method="S256",
|
||||
)
|
||||
|
||||
location: Final = response.headers["location"]
|
||||
assert location.startswith("https://provider.com/oauth/authorize"), location
|
||||
query: Final = parse_qs(urlparse(location).query)
|
||||
assert query["redirect_uri"] == [expected_redirect_uri], query
|
||||
assert query["client_id"] == [expected_client_id], query
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
("shared_client_persisted", "expected_redirect_uri", "expected_client_id"),
|
||||
[
|
||||
(True, "https://litellm.example.com/callback", "shared-gateway-client"),
|
||||
(False, _LAPTOP_LOOPBACK_REDIRECT, "dcr-client-123"),
|
||||
],
|
||||
)
|
||||
async def test_token_clientless_bridge_loads_shared_client_before_choosing_the_arm(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
shared_client_persisted: bool,
|
||||
expected_redirect_uri: str,
|
||||
expected_client_id: str,
|
||||
) -> None:
|
||||
"""The token leg mirrors authorize: with the shared client persisted it redeems the code with that
|
||||
client and the gateway /callback, and with none yet it relays the caller's own client and redirect."""
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import global_mcp_server_manager
|
||||
|
||||
monkeypatch.setenv("PROXY_BASE_URL", "https://litellm.example.com")
|
||||
monkeypatch.setenv("LITELLM_SALT_KEY", "test-registration-binding")
|
||||
server: Final = _bridge_server(url="https://mcp.example.com/mcp")
|
||||
persisted: Final = _persisted_shared_gateway_client(server) if shared_client_persisted else None
|
||||
prisma_client: Final = MagicMock()
|
||||
mock_get_mcp_server: Final = AsyncMock(return_value=persisted)
|
||||
mock_update_server: Final = AsyncMock()
|
||||
with (
|
||||
patch("litellm.proxy.utils.get_prisma_client_or_throw", return_value=prisma_client),
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.db.get_mcp_server",
|
||||
new=mock_get_mcp_server,
|
||||
),
|
||||
patch.object(
|
||||
global_mcp_server_manager,
|
||||
"update_server",
|
||||
new=mock_update_server,
|
||||
),
|
||||
):
|
||||
data: Final = await _bridge_token_post_data(server, redirect_uri=_LAPTOP_LOOPBACK_REDIRECT)
|
||||
|
||||
assert data["redirect_uri"] == expected_redirect_uri, data
|
||||
assert data["client_id"] == expected_client_id, data
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
("dcr_bridge", "expected"),
|
||||
[(True, "persisted"), (False, "skipped")],
|
||||
)
|
||||
async def test_persist_dcr_client_for_client_forwarded_server_only_when_it_is_a_bridge(
|
||||
dcr_bridge: bool,
|
||||
expected: Literal["persisted", "skipped"],
|
||||
) -> None:
|
||||
"""A DCR bridge keeps the shared gateway client on the server row (without stamping oauth2_flow on
|
||||
a client-forwarded row), while a client-forwarded server without the bridge still stores nothing."""
|
||||
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
|
||||
_persist_dcr_client_registration,
|
||||
)
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
|
||||
global_mcp_server_manager,
|
||||
)
|
||||
from litellm.types.mcp import MCPAuth
|
||||
|
||||
server: Final = _bridge_server(auth_type=MCPAuth.oauth_delegate, dcr_bridge=dcr_bridge)
|
||||
prisma: Final = MagicMock()
|
||||
prisma.db.litellm_mcpservertable.find_unique = AsyncMock(return_value=None)
|
||||
mock_update: Final = AsyncMock(return_value=MagicMock())
|
||||
with (
|
||||
patch("litellm.proxy.utils.get_prisma_client_or_throw", return_value=prisma),
|
||||
patch("litellm.proxy._experimental.mcp_server.db.update_mcp_server", new=mock_update),
|
||||
patch.object(global_mcp_server_manager, "update_server", new=AsyncMock()),
|
||||
patch.object(global_mcp_server_manager, "is_config_declared_server", return_value=False),
|
||||
):
|
||||
result: Final = await _persist_dcr_client_registration(
|
||||
server,
|
||||
{"client_id": "shared-gateway-client", "token_endpoint_auth_method": "none"},
|
||||
"https://litellm.example.com/callback",
|
||||
replace_stale_client=False,
|
||||
)
|
||||
|
||||
assert result == expected
|
||||
if expected == "skipped":
|
||||
mock_update.assert_not_awaited()
|
||||
return
|
||||
written: Final = mock_update.await_args.kwargs["data"]
|
||||
assert written.oauth2_flow is None, written
|
||||
assert written.credentials["client_id"] == "shared-gateway-client", written
|
||||
assert server.client_id == "shared-gateway-client"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("server_has_shared_client", [False, True])
|
||||
async def test_token_sealed_ephemeral_client_is_not_replaced_by_persisted_gateway_client(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
server_has_shared_client: bool,
|
||||
) -> None:
|
||||
"""The token leg must use the sealed ephemeral client even if this worker's catalog has a shared client."""
|
||||
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import exchange_token_with_server
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import global_mcp_server_manager
|
||||
|
||||
monkeypatch.setenv("PROXY_BASE_URL", "https://litellm.example.com")
|
||||
monkeypatch.setenv("LITELLM_SALT_KEY", "test-registration-binding")
|
||||
server: Final = _bridge_server(
|
||||
url="https://mcp.example.com/mcp",
|
||||
client_id="shared-gateway-client" if server_has_shared_client else None,
|
||||
client_secret="shared-secret" if server_has_shared_client else None,
|
||||
token_endpoint_auth_method="client_secret_basic" if server_has_shared_client else None,
|
||||
)
|
||||
fake_http_response: Final = MagicMock()
|
||||
fake_http_response.json.return_value = {"access_token": "tok", "token_type": "Bearer"}
|
||||
fake_http_response.raise_for_status = MagicMock()
|
||||
fake_http_client: Final = MagicMock()
|
||||
fake_http_client.post = AsyncMock(return_value=fake_http_response)
|
||||
prisma_client: Final = MagicMock()
|
||||
mock_get_mcp_server: Final = AsyncMock(
|
||||
return_value=_persisted_shared_gateway_client(server, client_id="persisted-gateway-client")
|
||||
)
|
||||
mock_update_server: Final = AsyncMock()
|
||||
with (
|
||||
patch(
|
||||
"litellm.proxy.utils.get_prisma_client_or_throw",
|
||||
return_value=prisma_client,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.db.get_mcp_server",
|
||||
new=mock_get_mcp_server,
|
||||
),
|
||||
patch.object(
|
||||
global_mcp_server_manager,
|
||||
"update_server",
|
||||
new=mock_update_server,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.discoverable_endpoints.get_async_httpx_client",
|
||||
return_value=fake_http_client,
|
||||
),
|
||||
):
|
||||
await exchange_token_with_server(
|
||||
request=_bridge_mock_request(),
|
||||
mcp_server=server,
|
||||
grant_type="authorization_code",
|
||||
code="upstream-code",
|
||||
redirect_uri="https://litellm.example.com/callback",
|
||||
client_id="ephemeral-client",
|
||||
client_secret="ephemeral-secret" if server_has_shared_client else None,
|
||||
code_verifier="verifier",
|
||||
client_token_endpoint_auth_method="client_secret_post" if server_has_shared_client else None,
|
||||
sealed_client=True,
|
||||
)
|
||||
|
||||
data: Final = fake_http_client.post.call_args.kwargs["data"]
|
||||
assert data["client_id"] == "ephemeral-client", data
|
||||
assert data["redirect_uri"] == "https://litellm.example.com/callback", data
|
||||
if server_has_shared_client:
|
||||
headers: Final = fake_http_client.post.call_args.kwargs["headers"]
|
||||
assert data["client_secret"] == "ephemeral-secret", data
|
||||
assert "Authorization" not in headers, headers
|
||||
|
||||
|
||||
_BRIDGE_MASTER_KEY = "sk-bridge-producer-master-key-0123456789abcdef"
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -3545,6 +3545,95 @@ class TestTemporaryMCPSessionEndpoints:
|
|||
assert exchange_mock.await_args.kwargs["redirect_uri"] == "https://litellm.example.com/callback"
|
||||
assert exchange_mock.await_args.kwargs["client_token_endpoint_auth_method"] == "client_secret_basic"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_mcp_token_prefers_sealed_client_when_server_has_shared_client(
|
||||
self: "TestTemporaryMCPSessionEndpoints",
|
||||
) -> None:
|
||||
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
|
||||
seal_passthrough_authorization_code,
|
||||
)
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import global_mcp_server_manager
|
||||
from litellm.proxy.management_endpoints.mcp_management_endpoints import (
|
||||
mcp_token,
|
||||
)
|
||||
|
||||
request: Final = MagicMock()
|
||||
request.base_url = "https://litellm.example.com/"
|
||||
request.headers = {}
|
||||
server: Final = generate_mock_mcp_server_config_record(server_id="server-1").model_copy(
|
||||
update={
|
||||
"auth_type": MCPAuth.true_passthrough,
|
||||
"client_id": "shared-gateway-client",
|
||||
"client_secret": "shared-secret",
|
||||
"token_endpoint_auth_method": "client_secret_basic",
|
||||
"token_url": "https://provider.example/oauth/token",
|
||||
}
|
||||
)
|
||||
admin_auth: Final = generate_mock_user_api_key_auth(user_role=LitellmUserRoles.PROXY_ADMIN)
|
||||
fake_http_response: Final = MagicMock()
|
||||
fake_http_response.json.return_value = {"access_token": "tok", "token_type": "Bearer"}
|
||||
fake_http_response.raise_for_status = MagicMock()
|
||||
fake_http_client: Final = MagicMock()
|
||||
fake_http_client.post = AsyncMock(return_value=fake_http_response)
|
||||
prisma_client: Final = MagicMock()
|
||||
mock_get_mcp_server: Final = AsyncMock(return_value=None)
|
||||
mock_update_server: Final = AsyncMock()
|
||||
|
||||
with (
|
||||
patch("litellm.proxy.proxy_server.master_key", "sk-lit4581-test-master-key"),
|
||||
patch(
|
||||
"litellm.proxy.management_endpoints.mcp_management_endpoints._oauth_server_operation",
|
||||
return_value=nullcontext(server),
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy.utils.get_prisma_client_or_throw",
|
||||
return_value=prisma_client,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.db.get_mcp_server",
|
||||
new=mock_get_mcp_server,
|
||||
),
|
||||
patch.object(
|
||||
global_mcp_server_manager,
|
||||
"update_server",
|
||||
new=mock_update_server,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.discoverable_endpoints.get_async_httpx_client",
|
||||
return_value=fake_http_client,
|
||||
),
|
||||
):
|
||||
sealed: Final = seal_passthrough_authorization_code(
|
||||
upstream_code="up-code",
|
||||
client_id="ephemeral-client",
|
||||
client_secret="ephemeral-secret",
|
||||
mcp_server_id="server-1",
|
||||
token_endpoint_auth_method="client_secret_post",
|
||||
)
|
||||
result: Final = await mcp_token(
|
||||
request=request,
|
||||
server_id="server-1",
|
||||
user_api_key_dict=admin_auth,
|
||||
grant_type="authorization_code",
|
||||
code=sealed,
|
||||
redirect_uri="https://example.com/callback",
|
||||
client_id=None,
|
||||
client_secret=None,
|
||||
code_verifier="verifier",
|
||||
refresh_token=None,
|
||||
scope=None,
|
||||
)
|
||||
|
||||
data: Final = fake_http_client.post.call_args.kwargs["data"]
|
||||
headers: Final = fake_http_client.post.call_args.kwargs["headers"]
|
||||
assert data["client_id"] == "ephemeral-client", data
|
||||
assert data["client_secret"] == "ephemeral-secret", data
|
||||
assert data["code"] == "up-code", data
|
||||
assert data["redirect_uri"] == "https://litellm.example.com/callback", data
|
||||
assert "Authorization" not in headers, headers
|
||||
assert result.status_code == 200
|
||||
assert result.body == b'{"access_token":"tok","token_type":"Bearer"}'
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_mcp_token_refresh_grant_never_opens_sealed_code(self):
|
||||
"""The minted client is unrecoverable outside the single authorization_code flow by
|
||||
|
|
@ -3841,6 +3930,7 @@ class TestTemporaryMCPSessionEndpoints:
|
|||
refresh_token=None,
|
||||
scope=None,
|
||||
client_token_endpoint_auth_method=None,
|
||||
sealed_client=False,
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -3895,6 +3985,7 @@ class TestTemporaryMCPSessionEndpoints:
|
|||
refresh_token="rt-123",
|
||||
scope=None,
|
||||
client_token_endpoint_auth_method=None,
|
||||
sealed_client=False,
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
|
|
@ -3,9 +3,11 @@ import { Button } from "@/components/ui/button";
|
|||
import { Checkbox } from "@/components/ui/checkbox";
|
||||
import { Label } from "@/components/ui/label";
|
||||
import { PasswordInput } from "@/components/shared/PasswordInput";
|
||||
import { Input } from "@/components/ui/input";
|
||||
import { MultiSelect } from "@/components/shared/MultiSelect";
|
||||
import DcrBridgeToggle from "./DcrBridgeToggle";
|
||||
import { MountedFormField } from "@/components/common_components/MountedFormField";
|
||||
import { textControl } from "./mcpFieldRules";
|
||||
import { tagsControl, textControl } from "./mcpFieldRules";
|
||||
import { credentialAuthClass, isClientForwardedTokenMode } from "@/components/mcp_tools/types";
|
||||
|
||||
interface PassthroughOAuthFlow {
|
||||
|
|
@ -69,6 +71,7 @@ export default function PassthroughAuthorizeSection({
|
|||
const clientSecretPlaceholder = classUnchanged
|
||||
? "Leave blank to keep the currently saved secret (if any)"
|
||||
: "Leave blank for public clients / PKCE";
|
||||
const fieldClassName = "rounded-lg border-border focus:border-info focus:ring-ring";
|
||||
const clientIdExtra = classUnchanged
|
||||
? "Set this to make everyone authorize through a specific app; required for upstreams without dynamic client registration (e.g. a pre-registered Slack app)."
|
||||
: "Switching the auth type discards the previously saved app; enter a client ID here or leave blank to use dynamic client registration.";
|
||||
|
|
@ -113,6 +116,39 @@ export default function PassthroughAuthorizeSection({
|
|||
/>
|
||||
)}
|
||||
</MountedFormField>
|
||||
<MountedFormField
|
||||
label={<span className="text-sm font-medium text-foreground">Authorization URL (optional)</span>}
|
||||
name="authorization_url"
|
||||
help="Set this when the upstream publishes no OAuth metadata, for example an internal server behind Microsoft Entra. Leave blank to discover it from the upstream."
|
||||
>
|
||||
{(control) => (
|
||||
<Input
|
||||
{...textControl(control)}
|
||||
placeholder="https://login.microsoftonline.com/<tenant-id>/oauth2/v2.0/authorize"
|
||||
className={fieldClassName}
|
||||
/>
|
||||
)}
|
||||
</MountedFormField>
|
||||
<MountedFormField
|
||||
label={<span className="text-sm font-medium text-foreground">Token URL (optional)</span>}
|
||||
name="token_url"
|
||||
help="Token endpoint of the same authorization server. Leave blank to discover it from the upstream."
|
||||
>
|
||||
{(control) => (
|
||||
<Input
|
||||
{...textControl(control)}
|
||||
placeholder="https://login.microsoftonline.com/<tenant-id>/oauth2/v2.0/token"
|
||||
className={fieldClassName}
|
||||
/>
|
||||
)}
|
||||
</MountedFormField>
|
||||
<MountedFormField
|
||||
label={<span className="text-sm font-medium text-foreground">Scopes (optional)</span>}
|
||||
name={["credentials", "scopes"]}
|
||||
help="Scopes requested from the upstream when a user authorizes. Press enter or comma between scopes."
|
||||
>
|
||||
{(control) => <MultiSelect {...tagsControl(control)} placeholder="Add scopes" className="rounded-lg" />}
|
||||
</MountedFormField>
|
||||
<DcrBridgeToggle authType={authType} initialChecked={dcrBridgeInitialChecked} />
|
||||
{isEditing && onRemoveStoredAppChange && (
|
||||
<Label className="items-start leading-normal font-normal text-foreground">
|
||||
|
|
|
|||
|
|
@ -201,6 +201,22 @@ describe("buildCreateServerPayload credentials", () => {
|
|||
},
|
||||
);
|
||||
|
||||
it.each([["true_passthrough"], ["oauth_delegate"]])(
|
||||
"carries the admin-entered authorization URL, token URL and scopes for %s",
|
||||
(authType) => {
|
||||
const entraServer = {
|
||||
auth_type: authType,
|
||||
authorization_url: "https://login.microsoftonline.com/tenant/oauth2/v2.0/authorize",
|
||||
token_url: "https://login.microsoftonline.com/tenant/oauth2/v2.0/token",
|
||||
credentials: { client_id: "org-app", scopes: ["api://mcp/.default", "offline_access", ""] },
|
||||
};
|
||||
const payload = payloadOf(build(entraServer));
|
||||
expect(payload.authorization_url).toBe("https://login.microsoftonline.com/tenant/oauth2/v2.0/authorize");
|
||||
expect(payload.token_url).toBe("https://login.microsoftonline.com/tenant/oauth2/v2.0/token");
|
||||
expect(payload.credentials).toEqual({ client_id: "org-app", scopes: ["api://mcp/.default", "offline_access"] });
|
||||
},
|
||||
);
|
||||
|
||||
it("merges the DCR-minted client into an oauth2 payload", () => {
|
||||
const payload = payloadOf(
|
||||
build(
|
||||
|
|
|
|||
|
|
@ -295,11 +295,6 @@ export const CASES: readonly DifferentialCase[] = [
|
|||
ui: {},
|
||||
},
|
||||
{ label: "credentials absent entirely", values: { ...ROOT, auth_type: "oauth2" }, ui: {} },
|
||||
{
|
||||
label: "removeStoredApp forces an explicit-null app write",
|
||||
values: { ...ROOT, auth_type: "true_passthrough", credentials: { ...CREDS } },
|
||||
ui: { removeStoredApp: true, mcpServer: { ...SERVER, auth_type: "true_passthrough" } },
|
||||
},
|
||||
{
|
||||
label: "removeStoredApp ignored outside a client-forwarded mode",
|
||||
values: { ...ROOT, auth_type: "api_key", credentials: { ...CREDS } },
|
||||
|
|
|
|||
|
|
@ -398,3 +398,61 @@ it.each([
|
|||
expect(result.payload.credentials?.client_secret).toBe(useDcr ? null : "configured-secret");
|
||||
expect(result.payload.credentials?.dcr_issuer).toBe(useDcr ? "https://new.example" : undefined);
|
||||
});
|
||||
|
||||
it.each([AUTH_TYPE.OAUTH_DELEGATE, AUTH_TYPE.TRUE_PASSTHROUGH])(
|
||||
"keeps the endpoints typed for %s when the server was saved as oauth2",
|
||||
(authType) => {
|
||||
const authorizationUrl = "https://login.microsoftonline.com/tenant/oauth2/v2.0/authorize";
|
||||
const tokenUrl = "https://login.microsoftonline.com/tenant/oauth2/v2.0/token";
|
||||
const values: EditServerFormValues = {
|
||||
auth_type: authType,
|
||||
transport: "http",
|
||||
authorization_url: authorizationUrl,
|
||||
token_url: tokenUrl,
|
||||
};
|
||||
const ui: EditServerUiState = { ...baseUi, mcpServer: { ...baseUi.mcpServer, auth_type: AUTH_TYPE.OAUTH2 } };
|
||||
const result = buildEditServerPayload(values, ui);
|
||||
expect(result.kind).toBe("ok");
|
||||
if (result.kind !== "ok") throw new Error("Expected valid MCP server payload");
|
||||
expect(result.payload.authorization_url).toBe(authorizationUrl);
|
||||
expect(result.payload.token_url).toBe(tokenUrl);
|
||||
expect(result.payload.issuer).toBeNull();
|
||||
expect(result.payload.registration_url).toBeNull();
|
||||
},
|
||||
);
|
||||
|
||||
it.each([AUTH_TYPE.OAUTH_DELEGATE, AUTH_TYPE.TRUE_PASSTHROUGH])(
|
||||
"writes an explicit null when the admin removes every scope on a %s server",
|
||||
(authType) => {
|
||||
const values: EditServerFormValues = {
|
||||
auth_type: authType,
|
||||
transport: "http",
|
||||
credentials: { client_id: "org-app", scopes: [""] },
|
||||
};
|
||||
const ui: EditServerUiState = { ...baseUi, mcpServer: { ...baseUi.mcpServer, auth_type: authType } };
|
||||
const result = buildEditServerPayload(values, ui);
|
||||
expect(result.kind).toBe("ok");
|
||||
if (result.kind !== "ok") throw new Error("Expected valid MCP server payload");
|
||||
expect(result.payload.credentials).toEqual({ client_id: "org-app", scopes: null });
|
||||
},
|
||||
);
|
||||
|
||||
it.each([
|
||||
[["read"], ["read"]],
|
||||
[[""], null],
|
||||
])("keeps the scope edit %j when the saved app is removed on a client-forwarded server", (typed, expected) => {
|
||||
const values: EditServerFormValues = {
|
||||
auth_type: AUTH_TYPE.TRUE_PASSTHROUGH,
|
||||
transport: "http",
|
||||
credentials: { client_id: "cid", client_secret: "csec", scopes: typed },
|
||||
};
|
||||
const ui: EditServerUiState = {
|
||||
...baseUi,
|
||||
removeStoredApp: true,
|
||||
mcpServer: { ...baseUi.mcpServer, auth_type: AUTH_TYPE.TRUE_PASSTHROUGH },
|
||||
};
|
||||
const result = buildEditServerPayload(values, ui);
|
||||
expect(result.kind).toBe("ok");
|
||||
if (result.kind !== "ok") throw new Error("Expected valid MCP server payload");
|
||||
expect(result.payload.credentials).toEqual({ scopes: expected, client_id: null, client_secret: null });
|
||||
});
|
||||
|
|
|
|||
|
|
@ -214,6 +214,21 @@ const buildCredentials = (credentialValues: unknown): Readonly<Record<string, un
|
|||
return Object.fromEntries(kept);
|
||||
};
|
||||
|
||||
const unmountedOauth2Endpoints = (authType: string | undefined) =>
|
||||
isClientForwardedTokenMode(authType) ? {} : { authorization_url: null, token_url: null };
|
||||
|
||||
const scopesCleared = (credentialValues: unknown): boolean => {
|
||||
if (!credentialValues || typeof credentialValues !== "object") return false;
|
||||
const scopes: unknown = (credentialValues as { scopes?: unknown }).scopes;
|
||||
return Array.isArray(scopes) && scopes.every((scope: unknown) => scope == null || scope === "");
|
||||
};
|
||||
|
||||
const withClearedScopes = (
|
||||
credentials: Readonly<Record<string, unknown>> | undefined,
|
||||
credentialValues: unknown,
|
||||
): Readonly<Record<string, unknown>> | undefined =>
|
||||
scopesCleared(credentialValues) ? { ...(credentials ?? {}), scopes: null } : credentials;
|
||||
|
||||
interface CredentialsEntryInput {
|
||||
readonly dcrClient?: McpDcrCredentials | null;
|
||||
readonly oauthFlow?: string;
|
||||
|
|
@ -239,7 +254,7 @@ const resolveCredentialsEntry = ({
|
|||
return { credentials: { ...credentials, ...dcrClient } };
|
||||
}
|
||||
if (removeStoredApp && isClientForwardedTokenMode(authType)) {
|
||||
return { credentials: { client_id: null, client_secret: null } };
|
||||
return { credentials: { ...(credentials ?? {}), client_id: null, client_secret: null } };
|
||||
}
|
||||
if (includeCredentials && credentials && Object.keys(credentials).length > 0) {
|
||||
return { credentials };
|
||||
|
|
@ -334,7 +349,7 @@ export const buildEditServerPayload = (values: EditServerFormValues, ui: EditSer
|
|||
// Client-forwarded rows persist ONLY the declared app; strip any token material lingering in the
|
||||
// form (e.g. from a prior oauth2 authorize this session) so it can never reach the row.
|
||||
const submitCredentials = isClientForwardedTokenMode(restValues.auth_type)
|
||||
? preservedAdminCredentials(credentialsPayload)
|
||||
? withClearedScopes(preservedAdminCredentials(credentialsPayload), credentialValues)
|
||||
: credentialsPayload;
|
||||
const includeCredentials = restValues.auth_type && AUTH_TYPES_REQUIRING_CREDENTIALS.includes(restValues.auth_type);
|
||||
|
||||
|
|
@ -355,7 +370,7 @@ export const buildEditServerPayload = (values: EditServerFormValues, ui: EditSer
|
|||
stdio_config: undefined,
|
||||
env_json: undefined,
|
||||
...(mcpServer.auth_type === AUTH_TYPE.OAUTH2 && restValues.auth_type !== AUTH_TYPE.OAUTH2
|
||||
? { issuer: null, authorization_url: null, token_url: null, registration_url: null }
|
||||
? { issuer: null, ...unmountedOauth2Endpoints(restValues.auth_type), registration_url: null }
|
||||
: {}),
|
||||
...(mcpServer.auth_type === AUTH_TYPE.OAUTH2_TOKEN_EXCHANGE &&
|
||||
restValues.auth_type !== AUTH_TYPE.OAUTH2_TOKEN_EXCHANGE
|
||||
|
|
|
|||
|
|
@ -264,6 +264,8 @@ const CASES: readonly Case[] = [
|
|||
...EXPECTED_BASE,
|
||||
auth_type: "true_passthrough",
|
||||
credentials: { client_id: "cid", client_secret: "csec" },
|
||||
authorization_url: undefined,
|
||||
token_url: undefined,
|
||||
},
|
||||
},
|
||||
{
|
||||
|
|
@ -273,6 +275,8 @@ const CASES: readonly Case[] = [
|
|||
...EXPECTED_BASE,
|
||||
auth_type: "oauth_delegate",
|
||||
credentials: { client_id: "cid", client_secret: "csec" },
|
||||
authorization_url: undefined,
|
||||
token_url: undefined,
|
||||
},
|
||||
},
|
||||
{
|
||||
|
|
@ -283,7 +287,13 @@ const CASES: readonly Case[] = [
|
|||
{
|
||||
label: "dcr_bridge stays true for a client-forwarded mode",
|
||||
server: { ...BASE, auth_type: "true_passthrough", dcr_bridge: true },
|
||||
expected: { ...EXPECTED_BASE, auth_type: "true_passthrough", dcr_bridge: true },
|
||||
expected: {
|
||||
...EXPECTED_BASE,
|
||||
auth_type: "true_passthrough",
|
||||
dcr_bridge: true,
|
||||
authorization_url: undefined,
|
||||
token_url: undefined,
|
||||
},
|
||||
},
|
||||
{
|
||||
label: "an existing allowed_tools list enforces the allowlist and round-trips",
|
||||
|
|
|
|||
|
|
@ -333,6 +333,14 @@ describe("preservedAdminCredentials vs preservedDeclaredAppCredentials", () => {
|
|||
expect(preservedAdminCredentials(credentials)).toEqual({ client_id: "cid", upstream_resource: "auto" });
|
||||
});
|
||||
|
||||
it("preserves admin-typed scopes and drops blank ones", () => {
|
||||
expect(preservedAdminCredentials({ client_id: "cid", scopes: ["api://mcp/.default", ""] })).toEqual({
|
||||
client_id: "cid",
|
||||
scopes: ["api://mcp/.default"],
|
||||
});
|
||||
expect(preservedAdminCredentials({ client_id: "cid", scopes: [] })).toEqual({ client_id: "cid" });
|
||||
});
|
||||
|
||||
// The two helpers answer different questions and must not be collapsed: "has the admin declared an
|
||||
// OAuth app" gates the app-may-not-match-upstream warning, so a resource-only server must read as
|
||||
// having no declared app.
|
||||
|
|
|
|||
|
|
@ -181,10 +181,23 @@ export const preservedDeclaredAppCredentials = (
|
|||
// Everything the admin typed into `credentials` and nothing minted: the declared app plus the config
|
||||
// keys. This is what must survive the invalidation reset and what a client-forwarded row may persist,
|
||||
// so dropping a key from here silently discards admin input on an unrelated edit.
|
||||
const adminScopes = (value: unknown): string[] | undefined => {
|
||||
if (!Array.isArray(value)) return undefined;
|
||||
const kept = value.filter((scope): scope is string => typeof scope === "string" && scope !== "");
|
||||
return kept.length > 0 ? kept : undefined;
|
||||
};
|
||||
|
||||
export const preservedAdminCredentials = (
|
||||
credentials: Record<string, unknown> | null | undefined,
|
||||
): Record<string, string> | undefined =>
|
||||
pickStringCredentials(credentials, [...DECLARED_APP_CREDENTIAL_KEYS, ...ADMIN_CONFIG_CREDENTIAL_KEYS]);
|
||||
): Record<string, string | string[]> | undefined => {
|
||||
const strings = pickStringCredentials(credentials, [
|
||||
...DECLARED_APP_CREDENTIAL_KEYS,
|
||||
...ADMIN_CONFIG_CREDENTIAL_KEYS,
|
||||
]);
|
||||
const scopes = adminScopes(credentials?.scopes);
|
||||
if (scopes === undefined) return strings;
|
||||
return { ...(strings ?? {}), scopes };
|
||||
};
|
||||
|
||||
// Drop minted token keys, keeping everything else (the declared app plus any non-token config).
|
||||
export const withoutMintedTokenCredentials = (
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue