mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-11 22:51:28 +00:00
fix(mcp): re-request the sealed scope on a bridge refresh when the client omits it
The refresh envelope seals the upstream scope as the scope to re-request (RefreshCredential), but _prepare_bridge_refresh dropped it, unwrapping only the refresh token, and the exchange added scope to the upstream request only from the client's HTTP form. A DCR/MCP client typically omits scope on refresh, so the sealed scope was never sent and a stricter upstream could narrow or drop the renewed token's scope Thread the sealed scope through _BridgeRefreshReady.upstream_scope and fall back to it when the client sends none; a client-supplied scope still wins, which RFC 6749 section 6 bounds to the original grant. The regression test drives a refresh where the client omits scope and asserts the upstream POST carries the sealed scope, mutation-checked against both the drop and the fallback
This commit is contained in:
parent
1f1628d85c
commit
2eb37d7948
2 changed files with 41 additions and 9 deletions
|
|
@ -1175,13 +1175,17 @@ async def _prepare_bridge_mint(
|
|||
|
||||
@dataclass(frozen=True, slots=True)
|
||||
class _BridgeRefreshReady:
|
||||
"""A validated refresh request: the identity+keys to mint the renewed pair under, and the upstream
|
||||
refresh token (unwrapped from the client's refresh envelope) to exchange with the upstream IdP. The
|
||||
upstream refresh token is a ``SecretStr`` like every other credential in this layer, so a repr or a
|
||||
traceback that captures this value never exposes the raw upstream refresh token in plaintext."""
|
||||
"""A validated refresh request: the identity+keys to mint the renewed pair under, the upstream refresh
|
||||
token (unwrapped from the client's refresh envelope) to exchange with the upstream IdP, and the scope
|
||||
sealed alongside it at mint. The upstream refresh token is a ``SecretStr`` like every other credential
|
||||
in this layer, so a repr or a traceback that captures this value never exposes the raw upstream refresh
|
||||
token in plaintext. ``upstream_scope`` carries the originally-granted scope so the renewal re-requests
|
||||
it when the client (a DCR/MCP client that typically omits scope on refresh) sends none, keeping the
|
||||
renewed token's scope stable against an upstream that would otherwise narrow or drop it."""
|
||||
|
||||
ready: "_BridgeMintReady"
|
||||
upstream_refresh_token: SecretStr
|
||||
upstream_scope: str | None = None
|
||||
|
||||
|
||||
def _refresh_key_failure_to_mint_error(failure: _KeyResolutionFailure) -> _BridgeMintError:
|
||||
|
|
@ -1234,6 +1238,7 @@ async def _prepare_bridge_refresh(
|
|||
return _BridgeRefreshReady(
|
||||
ready=_BridgeMintReady(identity=opened.identity, keys=keys),
|
||||
upstream_refresh_token=opened.refresh.refresh_token,
|
||||
upstream_scope=opened.refresh.scope,
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -1370,6 +1375,7 @@ async def exchange_token_with_server(
|
|||
bridge_identity: _BridgeAuthorizationCode | None = None
|
||||
bridge_mint_ready: _BridgeMintReady | None = None
|
||||
bridge_upstream_refresh: SecretStr | None = None
|
||||
bridge_upstream_scope: str | None = None
|
||||
is_bridge = mcp_server.is_oauth_delegate and mcp_server.is_dcr_bridge
|
||||
|
||||
if grant_type == "refresh_token":
|
||||
|
|
@ -1382,6 +1388,7 @@ async def exchange_token_with_server(
|
|||
return _bridge_mint_error_response(prepared_refresh)
|
||||
bridge_mint_ready = prepared_refresh.ready
|
||||
bridge_upstream_refresh = prepared_refresh.upstream_refresh_token
|
||||
bridge_upstream_scope = prepared_refresh.upstream_scope
|
||||
# A bridge server sends the unwrapped upstream refresh token recovered from the client's refresh
|
||||
# envelope above; every other server sends the client's own refresh token verbatim.
|
||||
upstream_refresh_token = (
|
||||
|
|
@ -1397,8 +1404,9 @@ async def exchange_token_with_server(
|
|||
"refresh_token": upstream_refresh_token,
|
||||
**client_auth.body,
|
||||
}
|
||||
if scope:
|
||||
token_data["scope"] = scope
|
||||
effective_scope = scope or bridge_upstream_scope
|
||||
if effective_scope:
|
||||
token_data["scope"] = effective_scope
|
||||
else:
|
||||
if not code:
|
||||
raise HTTPException(
|
||||
|
|
|
|||
|
|
@ -4745,10 +4745,12 @@ async def test_bridge_refresh_grant_with_non_envelope_is_invalid_grant_before_up
|
|||
|
||||
|
||||
def _mint_test_refresh_envelope(
|
||||
server_id="bridge_srv", key_hash="hashed-litellm-key-77", upstream_refresh="UPSTREAM-REFRESH", identity=None
|
||||
server_id="bridge_srv", key_hash="hashed-litellm-key-77", upstream_refresh="UPSTREAM-REFRESH", identity=None,
|
||||
scope=None,
|
||||
):
|
||||
"""Mint a refresh envelope the way the producer does, for driving the refresh_token grant in tests.
|
||||
Defaults to a key_hash subject; pass ``identity`` to seal a specific subject (e.g. a user_id)."""
|
||||
Defaults to a key_hash subject; pass ``identity`` to seal a specific subject (e.g. a user_id), and
|
||||
``scope`` to seal the scope to re-request on refresh."""
|
||||
from datetime import datetime, timezone
|
||||
|
||||
from pydantic import SecretStr
|
||||
|
|
@ -4766,7 +4768,8 @@ def _mint_test_refresh_envelope(
|
|||
keys = envelope_keys_from_master_key(_BRIDGE_MASTER_KEY)
|
||||
identity = identity if identity is not None else key_hash_identity(server_id=server_id, key_hash=key_hash)
|
||||
sealed = build_bridge_refresh_token_response(
|
||||
identity, RefreshCredential(refresh_token=SecretStr(upstream_refresh)), keys, datetime.now(timezone.utc)
|
||||
identity, RefreshCredential(refresh_token=SecretStr(upstream_refresh), scope=scope), keys,
|
||||
datetime.now(timezone.utc),
|
||||
)
|
||||
assert isinstance(sealed, SealedEnvelope)
|
||||
return sealed.token.get_secret_value()
|
||||
|
|
@ -4987,6 +4990,27 @@ async def test_bridge_refresh_grant_renews_a_user_subject_envelope():
|
|||
assert captured["client"].post.call_args.kwargs["data"]["refresh_token"] == "UP-REFRESH-USER"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bridge_refresh_re_requests_the_sealed_scope_when_client_omits_it():
|
||||
"""A DCR/MCP client omits scope on the refresh request, so the gateway must re-request the scope sealed
|
||||
at mint; dropping it lets a stricter upstream narrow the renewed token. The upstream POST must carry
|
||||
the sealed scope even though the client sent none. Regression for the dropped sealed refresh scope."""
|
||||
from litellm.types.mcp import MCPAuth
|
||||
|
||||
server = _bridge_server(auth_type=MCPAuth.oauth_delegate)
|
||||
refresh_env = _mint_test_refresh_envelope(
|
||||
server_id=server.server_id, upstream_refresh="UP-REFRESH", scope="read:tools write:tools"
|
||||
)
|
||||
captured: dict = {}
|
||||
response = await _refresh_for_bridge_server(
|
||||
server, refresh_env, {"access_token": "NEW-ACCESS", "token_type": "Bearer", "expires_in": 3600}, None,
|
||||
fake_client_out=captured,
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert captured["client"].post.call_args.kwargs["data"]["scope"] == "read:tools write:tools"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bridge_refresh_grant_with_deactivated_user_is_invalid_grant_before_upstream():
|
||||
"""A user_id-subject refresh envelope whose user has since been deactivated (SCIM offboarding, or
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue