mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-07 08:26:10 +00:00
Merge pull request #33346 from BerriAI/litellm_mcp_token_storage_ttl_cap
fix(mcp): cap per-user OAuth token cache TTL at the token's own lifetime
This commit is contained in:
commit
1bb69a17fc
5 changed files with 79 additions and 13 deletions
|
|
@ -175,16 +175,18 @@ mcp_oauth2_token_cache = MCPOAuth2TokenCache()
|
|||
def _compute_per_user_token_ttl(server: "MCPServer", expires_in: Optional[int]) -> int:
|
||||
"""Compute Redis TTL for a per-user token.
|
||||
|
||||
Uses server.token_storage_ttl_seconds when configured; otherwise derives
|
||||
TTL from expires_in minus the expiry buffer; falls back to the default TTL.
|
||||
Uses server.token_storage_ttl_seconds when configured, capped at the token's
|
||||
remaining lifetime (expires_in minus the expiry buffer) so a cached entry never
|
||||
outlives the token itself; otherwise derives TTL from expires_in minus the
|
||||
expiry buffer; falls back to the default TTL.
|
||||
"""
|
||||
lifetime_bound = expires_in - MCP_PER_USER_TOKEN_EXPIRY_BUFFER_SECONDS if expires_in is not None else None
|
||||
if server.token_storage_ttl_seconds is not None:
|
||||
return max(server.token_storage_ttl_seconds, 1)
|
||||
if expires_in is not None:
|
||||
return max(
|
||||
expires_in - MCP_PER_USER_TOKEN_EXPIRY_BUFFER_SECONDS,
|
||||
1,
|
||||
)
|
||||
if lifetime_bound is None:
|
||||
return max(server.token_storage_ttl_seconds, 1)
|
||||
return max(min(server.token_storage_ttl_seconds, lifetime_bound), 1)
|
||||
if lifetime_bound is not None:
|
||||
return max(lifetime_bound, 1)
|
||||
return MCP_PER_USER_TOKEN_DEFAULT_TTL
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -122,9 +122,10 @@ class MCPServer(BaseModel):
|
|||
# response (supports dot-notation for nested fields, e.g. "team.enterprise_id").
|
||||
# Tokens that fail validation are rejected before storage.
|
||||
token_validation: Optional[Dict[str, Any]] = None
|
||||
# Optional TTL override (seconds) for the Redis per-user token cache.
|
||||
# Defaults to the token's expires_in minus the expiry buffer, or
|
||||
# MCP_PER_USER_TOKEN_DEFAULT_TTL when expires_in is absent.
|
||||
# Optional TTL override (seconds) for the Redis per-user token cache, capped
|
||||
# at the token's expires_in minus the expiry buffer so a cached entry never
|
||||
# outlives the token. Defaults to the token's expires_in minus the expiry
|
||||
# buffer, or MCP_PER_USER_TOKEN_DEFAULT_TTL when expires_in is absent.
|
||||
token_storage_ttl_seconds: Optional[int] = None
|
||||
timeout: Optional[float] = None
|
||||
# Max concurrent outbound tool calls to this server; excess calls queue.
|
||||
|
|
|
|||
|
|
@ -227,3 +227,66 @@ async def test_client_credentials_uses_client_secret_basic_when_configured():
|
|||
assert "client_secret" not in kwargs["data"]
|
||||
assert "client_id" not in kwargs["data"]
|
||||
assert kwargs["data"]["grant_type"] == "client_credentials"
|
||||
|
||||
|
||||
def test_storage_ttl_capped_at_token_lifetime():
|
||||
"""A token_storage_ttl_seconds longer than the token's own lifetime must be capped at
|
||||
expires_in minus the expiry buffer. Before the cap, the configured TTL won outright and the
|
||||
Redis fast path (which never re-checks expires_at) kept serving the dead token until eviction,
|
||||
while the stored refresh_token sat unused because refresh only runs on the DB read-through."""
|
||||
from litellm.constants import MCP_PER_USER_TOKEN_EXPIRY_BUFFER_SECONDS
|
||||
from litellm.proxy._experimental.mcp_server.oauth2_token_cache import (
|
||||
_compute_per_user_token_ttl,
|
||||
)
|
||||
|
||||
server = _server(oauth2_flow=None, token_storage_ttl_seconds=604800)
|
||||
assert _compute_per_user_token_ttl(server, expires_in=86400) == 86400 - MCP_PER_USER_TOKEN_EXPIRY_BUFFER_SECONDS
|
||||
|
||||
|
||||
def test_storage_ttl_shorter_than_token_lifetime_wins():
|
||||
"""A configured TTL below the token lifetime is the operative value: the knob's purpose is to
|
||||
force earlier DB re-checks (staleness backstop), so the shorter side must win the min()."""
|
||||
from litellm.proxy._experimental.mcp_server.oauth2_token_cache import (
|
||||
_compute_per_user_token_ttl,
|
||||
)
|
||||
|
||||
server = _server(oauth2_flow=None, token_storage_ttl_seconds=3600)
|
||||
assert _compute_per_user_token_ttl(server, expires_in=86400) == 3600
|
||||
|
||||
|
||||
def test_storage_ttl_verbatim_when_token_lifetime_unknown():
|
||||
"""With no expires_in from the upstream there is nothing to cap against, so the configured
|
||||
TTL applies as-is (matching the pre-cap behavior for lifetime-less tokens)."""
|
||||
from litellm.proxy._experimental.mcp_server.oauth2_token_cache import (
|
||||
_compute_per_user_token_ttl,
|
||||
)
|
||||
|
||||
server = _server(oauth2_flow=None, token_storage_ttl_seconds=604800)
|
||||
assert _compute_per_user_token_ttl(server, expires_in=None) == 604800
|
||||
|
||||
|
||||
def test_storage_ttl_floors_at_one_second_for_nearly_dead_token():
|
||||
"""A token already inside the expiry buffer yields the 1-second floor, not zero or a negative
|
||||
TTL, mirroring the floor the default (unconfigured) path has always had."""
|
||||
from litellm.proxy._experimental.mcp_server.oauth2_token_cache import (
|
||||
_compute_per_user_token_ttl,
|
||||
)
|
||||
|
||||
server = _server(oauth2_flow=None, token_storage_ttl_seconds=3600)
|
||||
assert _compute_per_user_token_ttl(server, expires_in=30) == 1
|
||||
|
||||
|
||||
def test_default_ttl_paths_unchanged_without_storage_ttl():
|
||||
"""With token_storage_ttl_seconds unset the TTL still derives from expires_in minus the
|
||||
buffer, and falls back to MCP_PER_USER_TOKEN_DEFAULT_TTL when expires_in is absent."""
|
||||
from litellm.constants import (
|
||||
MCP_PER_USER_TOKEN_DEFAULT_TTL,
|
||||
MCP_PER_USER_TOKEN_EXPIRY_BUFFER_SECONDS,
|
||||
)
|
||||
from litellm.proxy._experimental.mcp_server.oauth2_token_cache import (
|
||||
_compute_per_user_token_ttl,
|
||||
)
|
||||
|
||||
server = _server(oauth2_flow=None)
|
||||
assert _compute_per_user_token_ttl(server, expires_in=86400) == 86400 - MCP_PER_USER_TOKEN_EXPIRY_BUFFER_SECONDS
|
||||
assert _compute_per_user_token_ttl(server, expires_in=None) == MCP_PER_USER_TOKEN_DEFAULT_TTL
|
||||
|
|
|
|||
|
|
@ -228,7 +228,7 @@ const OAuthFormFields: React.FC<OAuthFormFieldsProps> = ({
|
|||
label={
|
||||
<FieldLabel
|
||||
label="Token Storage TTL (seconds, optional)"
|
||||
tooltip="How long to cache each user's OAuth access token in Redis before evicting it (regardless of the token's own expires_in). Leave blank to derive the TTL from the token's expires_in, or fall back to the 12-hour default."
|
||||
tooltip="How long to cache each user's OAuth access token in Redis before evicting it (never longer than the token's own expires_in). Leave blank to derive the TTL from the token's expires_in, or fall back to the 12-hour default."
|
||||
/>
|
||||
}
|
||||
name="token_storage_ttl_seconds"
|
||||
|
|
|
|||
|
|
@ -1386,7 +1386,7 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
|||
label={
|
||||
<span className="text-sm font-medium text-gray-700 flex items-center">
|
||||
Token Storage TTL (seconds, optional)
|
||||
<Tooltip title="How long to cache each user's OAuth access token in Redis before evicting it (regardless of the token's own expires_in). Leave blank to derive the TTL from the token's expires_in, or fall back to the 12-hour default.">
|
||||
<Tooltip title="How long to cache each user's OAuth access token in Redis before evicting it (never longer than the token's own expires_in). Leave blank to derive the TTL from the token's expires_in, or fall back to the 12-hour default.">
|
||||
<InfoCircleOutlined className="ml-2 text-blue-400 hover:text-blue-600 cursor-help" />
|
||||
</Tooltip>
|
||||
</span>
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue