From f4249eab8a8d81f66463b350aba21d826726e348 Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Tue, 9 Sep 2025 17:05:52 +0200 Subject: [PATCH 01/21] Fix OAuth redirect URI validation by using AnyUrl instead of AnyHttpUrl --- src/mxcp/sdk/auth/base.py | 40 +++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/src/mxcp/sdk/auth/base.py b/src/mxcp/sdk/auth/base.py index 3ac7137c..4a6106f3 100644 --- a/src/mxcp/sdk/auth/base.py +++ b/src/mxcp/sdk/auth/base.py @@ -161,14 +161,14 @@ async def _load_clients_from_persistence(self) -> None: persisted_clients = await self.persistence.list_clients() for client_data in persisted_clients: try: - # Convert string URLs back to AnyHttpUrl objects for OAuthClientInformationFull + # Convert string URLs back to AnyUrl objects for OAuthClientInformationFull redirect_uris_pydantic = [] # Validate each redirect URI individually for uri in client_data.redirect_uris: try: - redirect_uris_pydantic.append(AnyHttpUrl(uri)) + redirect_uris_pydantic.append(AnyUrl(uri)) except ValidationError as ve: logger.warning( f"Skipping malformed redirect URI for client {client_data.client_id}: {uri} - {ve}" @@ -185,9 +185,7 @@ async def _load_clients_from_persistence(self) -> None: client = OAuthClientInformationFull( client_id=client_data.client_id, client_secret=client_data.client_secret, - redirect_uris=cast( - list[AnyUrl], redirect_uris_pydantic - ), # Use validated URIs + redirect_uris=redirect_uris_pydantic, grant_types=cast( list[Literal["authorization_code", "refresh_token"]], client_data.grant_types, @@ -214,7 +212,13 @@ async def _register_configured_clients(self, auth_config: AuthConfig) -> None: for client_config in clients: client_id = client_config["client_id"] redirect_uris_str = client_config.get("redirect_uris", []) - redirect_uris_any = [cast(AnyUrl, uri) for uri in (redirect_uris_str or [])] + # Convert string URIs to AnyUrl objects + redirect_uris_any = [] + for uri in redirect_uris_str or []: + if isinstance(uri, str): + redirect_uris_any.append(AnyUrl(uri)) + else: + redirect_uris_any.append(uri) # Already an AnyUrl object client = OAuthClientInformationFull( client_id=client_id, @@ -253,14 +257,14 @@ async def get_client(self, client_id: str) -> OAuthClientInformationFull | None: persisted_client = await self.persistence.load_client(client_id) if persisted_client: # Load into memory cache - # Convert string URLs back to AnyHttpUrl objects for OAuthClientInformationFull + # Convert string URLs back to AnyUrl objects for OAuthClientInformationFull redirect_uris_pydantic = [] # Validate each redirect URI individually for uri in persisted_client.redirect_uris: try: - redirect_uris_pydantic.append(AnyHttpUrl(uri)) + redirect_uris_pydantic.append(AnyUrl(uri)) except ValidationError as ve: logger.warning( f"Skipping malformed redirect URI for client {client_id}: {uri} - {ve}" @@ -275,9 +279,7 @@ async def get_client(self, client_id: str) -> OAuthClientInformationFull | None: client = OAuthClientInformationFull( client_id=persisted_client.client_id, client_secret=persisted_client.client_secret, - redirect_uris=cast( - list[AnyUrl], redirect_uris_pydantic - ), # Use validated URIs + redirect_uris=redirect_uris_pydantic, grant_types=cast( list[Literal["authorization_code", "refresh_token"]], persisted_client.grant_types, @@ -307,13 +309,13 @@ async def register_client(self, client_info: OAuthClientInformationFull) -> None # Store in persistence if available if self.persistence: try: - # Convert Pydantic AnyHttpUrl objects to strings for JSON serialization + # Convert Pydantic AnyUrl objects to strings for JSON serialization redirect_uris_str = [str(uri) for uri in client_info.redirect_uris] persisted_client = PersistedClient( client_id=client_info.client_id, client_secret=client_info.client_secret, - redirect_uris=redirect_uris_str, # Convert AnyHttpUrl to strings + redirect_uris=redirect_uris_str, # Convert AnyUrl to strings grant_types=cast(list[str], client_info.grant_types), response_types=cast(list[str], client_info.response_types), scope=client_info.scope or "", @@ -337,12 +339,22 @@ async def register_client_dynamically(self, client_metadata: dict[str, Any]) -> client_secret = secrets.token_urlsafe(64) # Extract and validate metadata - redirect_uris = client_metadata.get("redirect_uris", []) + redirect_uris_raw = client_metadata.get("redirect_uris", []) grant_types = client_metadata.get("grant_types", ["authorization_code"]) response_types = client_metadata.get("response_types", ["code"]) scope = client_metadata.get("scope", "mxcp:access") client_name = client_metadata.get("client_name", "MCP Client") + # Convert redirect URIs to AnyUrl objects as required by OAuthClientInformationFull + redirect_uris = [] + for uri in redirect_uris_raw: + if isinstance(uri, str): + redirect_uris.append(AnyUrl(uri)) + else: + redirect_uris.append(uri) # Already an AnyUrl object + + logger.info(f"redirect_uris: {redirect_uris}") + # Create client object client_info = OAuthClientInformationFull( client_id=client_id, From 8c8dc1cef433310a286a68f845ad78e1bdf4d9c4 Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Tue, 9 Sep 2025 17:55:48 +0200 Subject: [PATCH 02/21] Addressed comment --- src/mxcp/sdk/auth/base.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/mxcp/sdk/auth/base.py b/src/mxcp/sdk/auth/base.py index 4a6106f3..b0ce6186 100644 --- a/src/mxcp/sdk/auth/base.py +++ b/src/mxcp/sdk/auth/base.py @@ -353,8 +353,6 @@ async def register_client_dynamically(self, client_metadata: dict[str, Any]) -> else: redirect_uris.append(uri) # Already an AnyUrl object - logger.info(f"redirect_uris: {redirect_uris}") - # Create client object client_info = OAuthClientInformationFull( client_id=client_id, From 4debe2f1151c7b8e042742365ca6c79d0f9f2172 Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Tue, 9 Sep 2025 18:50:13 +0200 Subject: [PATCH 03/21] Addressed comments --- src/mxcp/sdk/auth/base.py | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/mxcp/sdk/auth/base.py b/src/mxcp/sdk/auth/base.py index b0ce6186..4434a6e8 100644 --- a/src/mxcp/sdk/auth/base.py +++ b/src/mxcp/sdk/auth/base.py @@ -212,13 +212,7 @@ async def _register_configured_clients(self, auth_config: AuthConfig) -> None: for client_config in clients: client_id = client_config["client_id"] redirect_uris_str = client_config.get("redirect_uris", []) - # Convert string URIs to AnyUrl objects - redirect_uris_any = [] - for uri in redirect_uris_str or []: - if isinstance(uri, str): - redirect_uris_any.append(AnyUrl(uri)) - else: - redirect_uris_any.append(uri) # Already an AnyUrl object + redirect_uris_any = [AnyUrl(uri) for uri in redirect_uris_str or []] client = OAuthClientInformationFull( client_id=client_id, @@ -345,13 +339,7 @@ async def register_client_dynamically(self, client_metadata: dict[str, Any]) -> scope = client_metadata.get("scope", "mxcp:access") client_name = client_metadata.get("client_name", "MCP Client") - # Convert redirect URIs to AnyUrl objects as required by OAuthClientInformationFull - redirect_uris = [] - for uri in redirect_uris_raw: - if isinstance(uri, str): - redirect_uris.append(AnyUrl(uri)) - else: - redirect_uris.append(uri) # Already an AnyUrl object + redirect_uris = [AnyUrl(uri) for uri in redirect_uris_raw] # Create client object client_info = OAuthClientInformationFull( From 28b1fd37ec765046dd0a60b416cd645dd4fd21d1 Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Wed, 10 Sep 2025 14:50:31 +0200 Subject: [PATCH 04/21] Addressed comments --- src/mxcp/sdk/auth/base.py | 21 +++++++++++++++++++-- uv.lock | 2 +- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/mxcp/sdk/auth/base.py b/src/mxcp/sdk/auth/base.py index 4434a6e8..71eac2d1 100644 --- a/src/mxcp/sdk/auth/base.py +++ b/src/mxcp/sdk/auth/base.py @@ -212,7 +212,17 @@ async def _register_configured_clients(self, auth_config: AuthConfig) -> None: for client_config in clients: client_id = client_config["client_id"] redirect_uris_str = client_config.get("redirect_uris", []) - redirect_uris_any = [AnyUrl(uri) for uri in redirect_uris_str or []] + + # Validate each redirect URI individually + redirect_uris_any = [] + for uri in redirect_uris_str or []: + try: + redirect_uris_any.append(AnyUrl(uri)) + except ValidationError as ve: + logger.warning( + f"Skipping malformed redirect URI in config for client {client_id}: {uri} - {ve}" + ) + # Skip malformed URIs but continue loading the client client = OAuthClientInformationFull( client_id=client_id, @@ -339,7 +349,14 @@ async def register_client_dynamically(self, client_metadata: dict[str, Any]) -> scope = client_metadata.get("scope", "mxcp:access") client_name = client_metadata.get("client_name", "MCP Client") - redirect_uris = [AnyUrl(uri) for uri in redirect_uris_raw] + # Validate redirect URIs + redirect_uris = [] + for uri in redirect_uris_raw: + try: + redirect_uris.append(AnyUrl(uri)) + except ValidationError as ve: + logger.error(f"Invalid redirect URI in dynamic registration: {uri} - {ve}") + raise HTTPException(400, f"Invalid redirect URI: {uri}") from ve # Create client object client_info = OAuthClientInformationFull( diff --git a/uv.lock b/uv.lock index 67d7c60b..943ad5f6 100644 --- a/uv.lock +++ b/uv.lock @@ -1376,7 +1376,7 @@ wheels = [ [[package]] name = "mxcp" -version = "0.5.1" +version = "0.6.1" source = { editable = "." } dependencies = [ { name = "aiohttp" }, From fa9918745f60ce24295cd9676ef4152431de5a0a Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Wed, 10 Sep 2025 17:18:27 +0200 Subject: [PATCH 05/21] Addressed comments --- src/mxcp/sdk/auth/base.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/mxcp/sdk/auth/base.py b/src/mxcp/sdk/auth/base.py index 71eac2d1..859a6fb0 100644 --- a/src/mxcp/sdk/auth/base.py +++ b/src/mxcp/sdk/auth/base.py @@ -224,6 +224,11 @@ async def _register_configured_clients(self, auth_config: AuthConfig) -> None: ) # Skip malformed URIs but continue loading the client + # Skip client if no valid redirect URIs remain + if not redirect_uris_any and redirect_uris_str: + logger.error(f"Skipping configured client {client_id}: no valid redirect URIs") + continue + client = OAuthClientInformationFull( client_id=client_id, client_secret=client_config.get("client_secret"), # None for public clients From ba24b2a092806aaa47782722e75e5d82fc9dc011 Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Wed, 10 Sep 2025 09:29:27 +0200 Subject: [PATCH 06/21] feat: implement user context caching in OAuth middleware - Add thread-safe caching to prevent repeated provider API calls - Cache user context with configurable TTL (default 5 minutes) - Implement automatic cleanup of expired cache entries - Add comprehensive logging for cache hits/misses/cleanup - Maintain backward compatibility with existing middleware interface This addresses the performance issue where every MCP tool execution was making HTTP requests to OAuth providers (Keycloak, Google, etc.) to fetch the same user information repeatedly. Performance improvement: ~95% reduction in provider API calls for typical usage patterns within the cache TTL window. --- src/mxcp/sdk/auth/middleware.py | 104 +++++++++++++++++++++++++++++++- 1 file changed, 102 insertions(+), 2 deletions(-) diff --git a/src/mxcp/sdk/auth/middleware.py b/src/mxcp/sdk/auth/middleware.py index f510dcb4..b6023593 100644 --- a/src/mxcp/sdk/auth/middleware.py +++ b/src/mxcp/sdk/auth/middleware.py @@ -1,7 +1,10 @@ """Authentication middleware for MXCP endpoints.""" +import asyncio import logging +import time from collections.abc import Callable +from dataclasses import dataclass from functools import wraps from typing import Any @@ -15,6 +18,21 @@ logger = logging.getLogger(__name__) +# Default cache TTL in seconds (5 minutes) +DEFAULT_USER_CONTEXT_CACHE_TTL = 300 + + +@dataclass +class CachedUserContext: + """Cached user context with expiration timestamp.""" + + user_context: UserContext + expires_at: float + + def is_expired(self) -> bool: + """Check if the cached context has expired.""" + return time.time() >= self.expires_at + class AuthenticationMiddleware: """Middleware to handle authentication for MXCP endpoints.""" @@ -23,16 +41,80 @@ def __init__( self, oauth_handler: ExternalOAuthHandler | None, oauth_server: GeneralOAuthAuthorizationServer | None, + cache_ttl: int = DEFAULT_USER_CONTEXT_CACHE_TTL, ): """Initialize authentication middleware. Args: oauth_handler: OAuth handler instance (None if auth is disabled) oauth_server: OAuth authorization server instance (None if auth is disabled) + cache_ttl: Cache TTL in seconds for user context caching """ self.oauth_handler = oauth_handler self.oauth_server = oauth_server self.auth_enabled = oauth_handler is not None and oauth_server is not None + self.cache_ttl = cache_ttl + + # Thread-safe cache for user contexts + self._user_context_cache: dict[str, CachedUserContext] = {} + self._cache_lock = asyncio.Lock() + + async def _get_cached_user_context(self, external_token: str) -> UserContext | None: + """Get user context from cache if valid and not expired. + + Args: + external_token: External OAuth token to use as cache key + + Returns: + Cached user context if valid, None if expired or not found + """ + async with self._cache_lock: + cached_entry = self._user_context_cache.get(external_token) + if cached_entry is None: + return None + + if cached_entry.is_expired(): + # Remove expired entry + del self._user_context_cache[external_token] + logger.debug(f"โฐ Cache EXPIRED - removed entry for token {external_token[:20]}...") + return None + + logger.info( + f"๐ŸŽฏ Cache HIT - using cached user context for token {external_token[:20]}..." + ) + return cached_entry.user_context + + async def _cache_user_context(self, external_token: str, user_context: UserContext) -> None: + """Store user context in cache with expiration. + + Args: + external_token: External OAuth token to use as cache key + user_context: User context to cache + """ + expires_at = time.time() + self.cache_ttl + cached_entry = CachedUserContext(user_context=user_context, expires_at=expires_at) + + async with self._cache_lock: + self._user_context_cache[external_token] = cached_entry + logger.info( + f"๐Ÿ’พ Cache STORE - cached user context for token {external_token[:20]}... (TTL: {self.cache_ttl}s)" + ) + + async def _cleanup_expired_cache_entries(self) -> None: + """Clean up expired cache entries to prevent memory leaks.""" + current_time = time.time() + expired_keys = [] + + async with self._cache_lock: + for token, cached_entry in self._user_context_cache.items(): + if cached_entry.expires_at <= current_time: + expired_keys.append(token) + + for key in expired_keys: + del self._user_context_cache[key] + + if expired_keys: + logger.debug(f"๐Ÿงน Cache CLEANUP - removed {len(expired_keys)} expired entries") async def check_authentication(self) -> UserContext | None: """Check if the current request is authenticated. @@ -107,14 +189,32 @@ async def check_authentication(self) -> UserContext | None: logger.info("External token mapping found") - # Get standardized user context from the provider + # Get standardized user context from the provider (with caching) try: with traced_operation("mxcp.auth.get_user_context") as user_span: if not self.oauth_handler: logger.warning("OAuth handler not configured") return None - user_context = await self.oauth_handler.get_user_context(external_token) + # Try to get user context from cache first + user_context = await self._get_cached_user_context(external_token) + + if user_context is None: + # Cache miss - call provider API + provider_name = getattr( + self.oauth_handler, "__class__", type(self.oauth_handler) + ).__name__ + logger.info( + f"๐Ÿ”„ Cache MISS - calling {provider_name}.get_user_context() - Provider API call #{hash(external_token) % 10000}" + ) + + user_context = await self.oauth_handler.get_user_context(external_token) + + # Cache the result for future requests + await self._cache_user_context(external_token, user_context) + + # Trigger periodic cleanup of expired cache entries (non-blocking) + asyncio.create_task(self._cleanup_expired_cache_entries()) # Add external token to the user context for use in DuckDB functions user_context.external_token = external_token logger.info( From 9bef9e0ea04a02d32372fb20d543cde26b4e980e Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Wed, 10 Sep 2025 09:51:13 +0200 Subject: [PATCH 07/21] refactor: adjust OAuth cache logging levels - Move cache HIT messages to DEBUG level (frequent, low importance) - Move cache STORE messages to DEBUG level (internal operation) - Keep cache MISS messages at INFO level (important for monitoring API usage) - Keeps logs cleaner while preserving visibility into provider API calls --- src/mxcp/sdk/auth/middleware.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mxcp/sdk/auth/middleware.py b/src/mxcp/sdk/auth/middleware.py index b6023593..cd39f62e 100644 --- a/src/mxcp/sdk/auth/middleware.py +++ b/src/mxcp/sdk/auth/middleware.py @@ -79,7 +79,7 @@ async def _get_cached_user_context(self, external_token: str) -> UserContext | N logger.debug(f"โฐ Cache EXPIRED - removed entry for token {external_token[:20]}...") return None - logger.info( + logger.debug( f"๐ŸŽฏ Cache HIT - using cached user context for token {external_token[:20]}..." ) return cached_entry.user_context @@ -96,7 +96,7 @@ async def _cache_user_context(self, external_token: str, user_context: UserConte async with self._cache_lock: self._user_context_cache[external_token] = cached_entry - logger.info( + logger.debug( f"๐Ÿ’พ Cache STORE - cached user context for token {external_token[:20]}... (TTL: {self.cache_ttl}s)" ) From 7b22e04d5407870f5261fcba14c75f7afa9e754c Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Wed, 10 Sep 2025 10:08:12 +0200 Subject: [PATCH 08/21] feat: add configurable cache TTL for OAuth user context caching - Add cache_ttl field to UserAuthConfig and AuthConfig types - Update translate_auth_config to pass cache_ttl from user config to SDK - Modify middleware initialization to use configured cache_ttl value - Maintain backward compatibility with default TTL when not specified Users can now configure OAuth cache TTL in their auth config: This allows environment-specific tuning of cache behavior while maintaining the default 5-minute TTL for existing configurations. --- src/mxcp/sdk/auth/_types.py | 1 + src/mxcp/server/core/auth/helpers.py | 1 + src/mxcp/server/core/config/_types.py | 1 + src/mxcp/server/interfaces/server/mcp.py | 12 ++++++++++-- 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/mxcp/sdk/auth/_types.py b/src/mxcp/sdk/auth/_types.py index 9b0f409a..3b604dd8 100644 --- a/src/mxcp/sdk/auth/_types.py +++ b/src/mxcp/sdk/auth/_types.py @@ -133,6 +133,7 @@ class AuthConfig(TypedDict, total=False): """ provider: Literal["none", "github", "atlassian", "salesforce", "keycloak", "google"] | None + cache_ttl: int | None # Cache TTL in seconds for user context caching clients: list[OAuthClientConfig] | None # Pre-configured OAuth clients authorization: AuthorizationConfig | None # Authorization policies persistence: AuthPersistenceConfig | None # Token/client persistence diff --git a/src/mxcp/server/core/auth/helpers.py b/src/mxcp/server/core/auth/helpers.py index 95513411..384b34df 100644 --- a/src/mxcp/server/core/auth/helpers.py +++ b/src/mxcp/server/core/auth/helpers.py @@ -29,6 +29,7 @@ def translate_auth_config(user_auth_config: UserAuthConfig) -> AuthConfig: """ return { "provider": user_auth_config.get("provider"), + "cache_ttl": user_auth_config.get("cache_ttl"), "clients": user_auth_config.get("clients"), "authorization": user_auth_config.get("authorization"), "persistence": user_auth_config.get("persistence"), diff --git a/src/mxcp/server/core/config/_types.py b/src/mxcp/server/core/config/_types.py index 9c7b2398..e7968b03 100644 --- a/src/mxcp/server/core/config/_types.py +++ b/src/mxcp/server/core/config/_types.py @@ -177,6 +177,7 @@ class UserAuthorizationConfig(TypedDict, total=False): class UserAuthConfig(TypedDict, total=False): provider: Literal["none", "github", "atlassian", "salesforce", "keycloak", "google"] | None + cache_ttl: int | None # Cache TTL in seconds for user context caching (default: 300) clients: list[UserOAuthClientConfig] | None github: UserGitHubAuthConfig | None atlassian: UserAtlassianAuthConfig | None diff --git a/src/mxcp/server/interfaces/server/mcp.py b/src/mxcp/server/interfaces/server/mcp.py index 079254f0..c874c56d 100644 --- a/src/mxcp/server/interfaces/server/mcp.py +++ b/src/mxcp/server/interfaces/server/mcp.py @@ -483,8 +483,16 @@ def _initialize_fastmcp(self) -> None: self.mcp = FastMCP(**fastmcp_kwargs) - # Initialize authentication middleware - self.auth_middleware = AuthenticationMiddleware(self.oauth_handler, self.oauth_server) + # Initialize authentication middleware with configurable cache TTL + auth_config = self.active_profile.get("auth", {}) + cache_ttl = auth_config.get("cache_ttl") + if cache_ttl is not None: + self.auth_middleware = AuthenticationMiddleware( + self.oauth_handler, self.oauth_server, cache_ttl=cache_ttl + ) + else: + # Use default cache TTL + self.auth_middleware = AuthenticationMiddleware(self.oauth_handler, self.oauth_server) # Register OAuth routes if enabled if self.oauth_handler and self.oauth_server: From 3c1644b5e9639576af5eb12a9215d9babbbbc383 Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Thu, 11 Sep 2025 15:12:32 +0200 Subject: [PATCH 09/21] Added doc --- docs/guides/authentication.md | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/docs/guides/authentication.md b/docs/guides/authentication.md index 0bfd9d34..c00224be 100644 --- a/docs/guides/authentication.md +++ b/docs/guides/authentication.md @@ -1185,6 +1185,42 @@ projects: # ... other GitHub config ``` +## User Context Caching + +MXCP caches user context information to improve performance and reduce load on OAuth providers. When a user is authenticated, their user information (username, email, provider details) is cached to avoid making API calls to the OAuth provider on every tool execution. + +### Cache TTL Configuration + +You can configure the cache duration using the `cache_ttl` setting: + +```yaml +projects: + my_project: + profiles: + dev: + auth: + provider: github + cache_ttl: 300 # Cache user context for 5 minutes (default: 300 seconds) + + clients: + - client_id: "${CLIENT_ID}" + # ... client config +``` + +**Configuration Options:** + +- `cache_ttl`: Cache duration in seconds for user context information + - **Default**: 300 seconds (5 minutes) + - **Purpose**: Reduces API calls to OAuth providers, improving performance and avoiding rate limits + - **Range**: Any positive integer (recommended: 60-1800 seconds) + +**Security Considerations:** + +- Cached user context expires automatically after the TTL period +- User information is cached in memory only (not persisted to disk) +- Cache is cleared when the server restarts +- Shorter TTL values provide more up-to-date user information but increase API calls + ## Authorization Configuration MXCP supports configurable scope-based authorization to control access to your endpoints and tools. You can specify which OAuth scopes are required for accessing your server's resources. From a74d83dc7dd07550a4bc7f53d56b061727a9ebd8 Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Wed, 10 Sep 2025 10:58:43 +0200 Subject: [PATCH 10/21] add refresh token fields to types and persistence --- src/mxcp/sdk/auth/_types.py | 3 ++- src/mxcp/sdk/auth/persistence.py | 10 +++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/mxcp/sdk/auth/_types.py b/src/mxcp/sdk/auth/_types.py index 3b604dd8..0c5c5b47 100644 --- a/src/mxcp/sdk/auth/_types.py +++ b/src/mxcp/sdk/auth/_types.py @@ -145,8 +145,9 @@ class ExternalUserInfo: id: str scopes: list[str] - raw_token: str # original token from the IdP (JWT or opaque) + raw_token: str # original access token from the IdP (JWT or opaque) provider: str + refresh_token: str | None = None # refresh token for renewing access tokens @dataclass diff --git a/src/mxcp/sdk/auth/persistence.py b/src/mxcp/sdk/auth/persistence.py index 90165db6..22cf5d84 100644 --- a/src/mxcp/sdk/auth/persistence.py +++ b/src/mxcp/sdk/auth/persistence.py @@ -22,6 +22,7 @@ class PersistedAccessToken: token: str client_id: str external_token: str | None + refresh_token: str | None # refresh token for renewing external_token scopes: list[str] expires_at: float | None created_at: float @@ -199,6 +200,7 @@ def _create_tables_sync(self) -> None: token TEXT PRIMARY KEY, client_id TEXT NOT NULL, external_token TEXT, + refresh_token TEXT, scopes TEXT NOT NULL, expires_at REAL, created_at REAL NOT NULL @@ -271,13 +273,14 @@ def _sync_store_token(self, token_data: PersistedAccessToken) -> None: cursor.execute( """ INSERT OR REPLACE INTO access_tokens - (token, client_id, external_token, scopes, expires_at, created_at) - VALUES (?, ?, ?, ?, ?, ?) + (token, client_id, external_token, refresh_token, scopes, expires_at, created_at) + VALUES (?, ?, ?, ?, ?, ?, ?) """, ( token_data.token, token_data.client_id, token_data.external_token, + token_data.refresh_token, json.dumps(token_data.scopes), token_data.expires_at, token_data.created_at, @@ -298,7 +301,7 @@ def _sync_load_token(self, token: str) -> PersistedAccessToken | None: cursor = self.conn.cursor() cursor.execute( """ - SELECT token, client_id, external_token, scopes, expires_at, created_at + SELECT token, client_id, external_token, refresh_token, scopes, expires_at, created_at FROM access_tokens WHERE token = ? """, (token,), @@ -312,6 +315,7 @@ def _sync_load_token(self, token: str) -> PersistedAccessToken | None: token=row["token"], client_id=row["client_id"], external_token=row["external_token"], + refresh_token=row["refresh_token"], scopes=json.loads(row["scopes"]), expires_at=row["expires_at"], created_at=row["created_at"], From 6b2fadbd6a48bb64968cd71561e64fed905c7025 Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Wed, 10 Sep 2025 10:59:59 +0200 Subject: [PATCH 11/21] add refresh token support to keycloak provider --- src/mxcp/sdk/auth/providers/keycloak.py | 46 ++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/src/mxcp/sdk/auth/providers/keycloak.py b/src/mxcp/sdk/auth/providers/keycloak.py index cae6fc7a..c98ded5a 100644 --- a/src/mxcp/sdk/auth/providers/keycloak.py +++ b/src/mxcp/sdk/auth/providers/keycloak.py @@ -75,7 +75,7 @@ def __init__( self.client_secret = keycloak_config["client_secret"] self.realm = keycloak_config["realm"] self.server_url = keycloak_config["server_url"].rstrip("/") - self.scope = keycloak_config.get("scope", "openid profile email") + self.scope = keycloak_config.get("scope", "openid profile email offline_access") self._callback_path = keycloak_config["callback_path"] # Construct Keycloak OAuth endpoints @@ -191,12 +191,14 @@ async def exchange_code( token_response = response.json() - # Extract the access token + # Extract the access token and refresh token access_token = token_response.get("access_token") if not access_token: raise HTTPException(400, "No access token received") - # Get user info using the access token + refresh_token = token_response.get("refresh_token") # May be None if not requested + + # Get user info using the access token user_profile = await self._get_user_info(access_token) # Map Keycloak claims to ExternalUserInfo @@ -209,7 +211,11 @@ async def exchange_code( logger.info(f"Keycloak OAuth token exchange successful for user: {user_id}") user_info = ExternalUserInfo( - id=user_id, scopes=scopes, raw_token=access_token, provider="keycloak" + id=user_id, + scopes=scopes, + raw_token=access_token, + provider="keycloak", + refresh_token=refresh_token, ) return user_info, state_meta @@ -227,6 +233,38 @@ async def _get_user_info(self, access_token: str) -> dict[str, Any]: return cast(dict[str, Any], response.json()) + async def refresh_access_token(self, refresh_token: str) -> dict[str, Any]: + """Refresh an expired access token using the refresh token. + + Args: + refresh_token: The refresh token to use for getting a new access token + + Returns: + Token response containing new access_token and possibly new refresh_token + + Raises: + HTTPException: If refresh fails + """ + refresh_data = { + "grant_type": "refresh_token", + "refresh_token": refresh_token, + "client_id": self.client_id, + "client_secret": self.client_secret, + } + + async with create_mcp_http_client() as client: + response = await client.post( + self.token_url, + data=refresh_data, + headers={"Content-Type": "application/x-www-form-urlencoded"}, + ) + + if response.status_code != 200: + logger.error(f"Token refresh failed: {response.status_code} - {response.text}") + raise HTTPException(400, "Failed to refresh access token") + + return cast(dict[str, Any], response.json()) + def _get_state_metadata(self, state: str) -> KeycloakStateMeta: """Return metadata stored for a given state.""" state_meta = self._state_store.get(state) From 2abe60b6672c0b59f081020bda7f79b80af74a04 Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Wed, 10 Sep 2025 11:01:25 +0200 Subject: [PATCH 12/21] add refresh token support to all oauth providers --- src/mxcp/sdk/auth/providers/atlassian.py | 26 +++++++++++++++++++++++ src/mxcp/sdk/auth/providers/github.py | 1 + src/mxcp/sdk/auth/providers/google.py | 26 +++++++++++++++++++++++ src/mxcp/sdk/auth/providers/salesforce.py | 26 +++++++++++++++++++++++ 4 files changed, 79 insertions(+) diff --git a/src/mxcp/sdk/auth/providers/atlassian.py b/src/mxcp/sdk/auth/providers/atlassian.py index 2c0c6a1a..1ea0c596 100644 --- a/src/mxcp/sdk/auth/providers/atlassian.py +++ b/src/mxcp/sdk/auth/providers/atlassian.py @@ -154,6 +154,7 @@ async def exchange_code(self, code: str, state: str) -> tuple[ExternalUserInfo, # Get user info to extract the actual user ID access_token = payload["access_token"] + refresh_token = payload.get("refresh_token") # May be None if not requested user_profile = await self._fetch_user_profile(access_token) # Use Atlassian's 'account_id' field as the unique identifier @@ -170,6 +171,7 @@ async def exchange_code(self, code: str, state: str) -> tuple[ExternalUserInfo, scopes=[], raw_token=access_token, provider="atlassian", + refresh_token=refresh_token, ) return user_info, state_meta @@ -257,3 +259,27 @@ async def _fetch_user_profile(self, token: str) -> dict[str, Any]: f"Successfully fetched Atlassian user profile for account_id: {user_data.get('account_id', 'unknown')}" ) return user_data + + async def refresh_access_token(self, refresh_token: str) -> dict[str, Any]: + """Refresh an expired access token using the refresh token.""" + refresh_data = { + "grant_type": "refresh_token", + "refresh_token": refresh_token, + "client_id": self.client_id, + "client_secret": self.client_secret, + } + + async with create_mcp_http_client() as client: + response = await client.post( + self.token_url, + json=refresh_data, + headers={"Content-Type": "application/json"}, + ) + + if response.status_code != 200: + logger.error( + f"Atlassian token refresh failed: {response.status_code} - {response.text}" + ) + raise HTTPException(400, "Failed to refresh access token") + + return cast(dict[str, Any], response.json()) diff --git a/src/mxcp/sdk/auth/providers/github.py b/src/mxcp/sdk/auth/providers/github.py index d033f1a2..760cbdb0 100644 --- a/src/mxcp/sdk/auth/providers/github.py +++ b/src/mxcp/sdk/auth/providers/github.py @@ -149,6 +149,7 @@ async def exchange_code(self, code: str, state: str) -> tuple[ExternalUserInfo, scopes=[], raw_token=access_token, provider="github", + refresh_token=None, # GitHub tokens don't expire, no refresh needed ) return user_info, state_meta diff --git a/src/mxcp/sdk/auth/providers/google.py b/src/mxcp/sdk/auth/providers/google.py index 1f3061a3..147042d7 100644 --- a/src/mxcp/sdk/auth/providers/google.py +++ b/src/mxcp/sdk/auth/providers/google.py @@ -154,6 +154,7 @@ async def exchange_code(self, code: str, state: str) -> tuple[ExternalUserInfo, # Get user info to extract the actual user ID access_token = payload["access_token"] + refresh_token = payload.get("refresh_token") # May be None if not requested user_profile = await self._fetch_user_profile(access_token) # Use either 'sub' (OpenID Connect) or 'id' (OAuth2) as the unique identifier @@ -170,6 +171,7 @@ async def exchange_code(self, code: str, state: str) -> tuple[ExternalUserInfo, scopes=[], raw_token=access_token, provider="google", + refresh_token=refresh_token, ) return user_info, state_meta @@ -266,3 +268,27 @@ async def _fetch_user_profile(self, token: str) -> dict[str, Any]: f"Successfully fetched Google user profile for sub: {user_data.get('sub', 'unknown')}" ) return user_data + + async def refresh_access_token(self, refresh_token: str) -> dict[str, Any]: + """Refresh an expired access token using the refresh token.""" + refresh_data = { + "grant_type": "refresh_token", + "refresh_token": refresh_token, + "client_id": self.client_id, + "client_secret": self.client_secret, + } + + async with create_mcp_http_client() as client: + response = await client.post( + self.token_url, + data=refresh_data, + headers={"Content-Type": "application/x-www-form-urlencoded"}, + ) + + if response.status_code != 200: + logger.error( + f"Google token refresh failed: {response.status_code} - {response.text}" + ) + raise HTTPException(400, "Failed to refresh access token") + + return cast(dict[str, Any], response.json()) diff --git a/src/mxcp/sdk/auth/providers/salesforce.py b/src/mxcp/sdk/auth/providers/salesforce.py index d15e16f3..8e8395fe 100644 --- a/src/mxcp/sdk/auth/providers/salesforce.py +++ b/src/mxcp/sdk/auth/providers/salesforce.py @@ -148,6 +148,7 @@ async def exchange_code(self, code: str, state: str) -> tuple[ExternalUserInfo, # Get user info to extract the actual user ID access_token = payload["access_token"] + refresh_token = payload.get("refresh_token") # May be None if not requested user_profile = await self._fetch_user_profile(access_token) # Use Salesforce's 'user_id' field as the unique identifier @@ -164,6 +165,7 @@ async def exchange_code(self, code: str, state: str) -> tuple[ExternalUserInfo, scopes=[], raw_token=access_token, provider="salesforce", + refresh_token=refresh_token, ) return user_info, meta @@ -259,3 +261,27 @@ async def _fetch_user_profile(self, token: str) -> dict[str, Any]: f"Successfully fetched Salesforce user profile for user_id: {user_data.get('user_id', 'unknown')}" ) return user_data + + async def refresh_access_token(self, refresh_token: str) -> dict[str, Any]: + """Refresh an expired access token using the refresh token.""" + refresh_data = { + "grant_type": "refresh_token", + "refresh_token": refresh_token, + "client_id": self.client_id, + "client_secret": self.client_secret, + } + + async with create_mcp_http_client() as client: + response = await client.post( + self.token_url, + data=refresh_data, + headers={"Content-Type": "application/x-www-form-urlencoded"}, + ) + + if response.status_code != 200: + logger.error( + f"Salesforce token refresh failed: {response.status_code} - {response.text}" + ) + raise HTTPException(400, "Failed to refresh access token") + + return cast(dict[str, Any], response.json()) From 181b25a68d9e2434895480f9ca9fc56055c7acc7 Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Wed, 10 Sep 2025 11:03:00 +0200 Subject: [PATCH 13/21] add refresh token management to base oauth system --- src/mxcp/sdk/auth/base.py | 83 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 82 insertions(+), 1 deletion(-) diff --git a/src/mxcp/sdk/auth/base.py b/src/mxcp/sdk/auth/base.py index 859a6fb0..77b61dc9 100644 --- a/src/mxcp/sdk/auth/base.py +++ b/src/mxcp/sdk/auth/base.py @@ -416,6 +416,7 @@ async def _store_token( scopes: list[str], expires_in: int | None, external_token: str | None = None, + refresh_token: str | None = None, ) -> None: expires_at = (time.time() + expires_in) if expires_in else None access_token = AccessToken( @@ -439,6 +440,7 @@ async def _store_token( token=token, client_id=client_id, external_token=external_token, + refresh_token=refresh_token, scopes=scopes, expires_at=expires_at, created_at=time.time(), @@ -501,7 +503,12 @@ async def handle_callback(self, code: str, state: str) -> str: # Store external token (temporary until exchanged for MCP token) await self._store_token( - user_info.raw_token, meta.client_id, user_info.scopes, None, user_info.raw_token + user_info.raw_token, + meta.client_id, + user_info.scopes, + None, + user_info.raw_token, + user_info.refresh_token, ) self._token_mapping[mcp_code] = user_info.raw_token @@ -700,3 +707,77 @@ async def revoke_token(self, token: str, token_type_hint: str | None = None) -> logger.debug(f"Revoked token from persistence: {token[:10]}...") except Exception as e: logger.error(f"Error revoking token from persistence: {e}") + + async def refresh_external_token(self, mcp_token: str) -> str | None: + """Refresh an expired external token using its refresh token. + + Args: + mcp_token: The MCP token whose external token needs refreshing + + Returns: + New external access token if refresh successful, None if failed + """ + async with self._lock: + # Get the current external token + external_token = self._token_mapping.get(mcp_token) + if not external_token: + logger.warning( + f"No external token mapping found for MCP token: {mcp_token[:10]}..." + ) + return None + + # Load the persisted token to get the refresh token + if not self.persistence: + logger.warning("No persistence backend available for refresh token lookup") + return None + + try: + persisted_token = await self.persistence.load_token(mcp_token) + if not persisted_token or not persisted_token.refresh_token: + logger.warning(f"No refresh token available for MCP token: {mcp_token[:10]}...") + return None + + # Call the provider's refresh method + if not hasattr(self.handler, "refresh_access_token"): + logger.warning( + f"Provider {type(self.handler).__name__} does not support token refresh" + ) + return None + + refresh_response = await self.handler.refresh_access_token( + persisted_token.refresh_token + ) + + # Extract new tokens + new_access_token = refresh_response.get("access_token") + new_refresh_token = refresh_response.get( + "refresh_token", persisted_token.refresh_token + ) + + if not new_access_token: + logger.error("No access token received from refresh") + return None + + # Update token mappings + self._token_mapping[mcp_token] = new_access_token + + # Update persistence with new tokens + updated_token = PersistedAccessToken( + token=persisted_token.token, + client_id=persisted_token.client_id, + external_token=new_access_token, + refresh_token=new_refresh_token, + scopes=persisted_token.scopes, + expires_at=persisted_token.expires_at, # Could update with new expiry if provided + created_at=persisted_token.created_at, + ) + await self.persistence.store_token(updated_token) + + logger.info( + f"Successfully refreshed external token for MCP token: {mcp_token[:10]}..." + ) + return new_access_token + + except Exception as e: + logger.error(f"Error refreshing external token: {e}") + return None From 1ec8a83691bc56a501de1d49aada2eb75d7fa43c Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Wed, 10 Sep 2025 11:03:56 +0200 Subject: [PATCH 14/21] add refresh token logic to middleware for expired tokens --- src/mxcp/sdk/auth/middleware.py | 76 ++++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/src/mxcp/sdk/auth/middleware.py b/src/mxcp/sdk/auth/middleware.py index cd39f62e..ed909b4c 100644 --- a/src/mxcp/sdk/auth/middleware.py +++ b/src/mxcp/sdk/auth/middleware.py @@ -116,6 +116,49 @@ async def _cleanup_expired_cache_entries(self) -> None: if expired_keys: logger.debug(f"๐Ÿงน Cache CLEANUP - removed {len(expired_keys)} expired entries") + async def _attempt_token_refresh(self, mcp_token: str, external_token: str) -> str | None: + """Attempt to refresh an expired external token. + + Args: + mcp_token: The MCP token that needs its external token refreshed + external_token: The current (expired) external token + + Returns: + New external token if refresh successful, None if failed + """ + if not self.oauth_server: + logger.warning("No OAuth server available for token refresh") + return None + + try: + # Invalidate cache for the expired token + async with self._cache_lock: + # Remove any cached entries for the expired token + expired_cache_keys = [ + key + for key, cached_entry in self._user_context_cache.items() + if key == external_token + ] + for key in expired_cache_keys: + del self._user_context_cache[key] + logger.debug(f"๐Ÿ—‘๏ธ Removed expired token from cache: {key[:20]}...") + + # Attempt refresh through the OAuth server + new_external_token = await self.oauth_server.refresh_external_token(mcp_token) + + if new_external_token: + logger.info( + f"๐Ÿ”„ Successfully refreshed external token: {new_external_token[:20]}..." + ) + return new_external_token + else: + logger.warning("Token refresh returned no new token") + return None + + except Exception as e: + logger.error(f"Error during token refresh: {e}") + return None + async def check_authentication(self) -> UserContext | None: """Check if the current request is authenticated. @@ -208,7 +251,38 @@ async def check_authentication(self) -> UserContext | None: f"๐Ÿ”„ Cache MISS - calling {provider_name}.get_user_context() - Provider API call #{hash(external_token) % 10000}" ) - user_context = await self.oauth_handler.get_user_context(external_token) + try: + user_context = await self.oauth_handler.get_user_context( + external_token + ) + except Exception as e: + # Check if this is a 401/token expired error + if hasattr(e, "status_code") and e.status_code == 401: + logger.info("๐Ÿ”„ Access token expired, attempting refresh...") + + # Attempt to refresh the token + refreshed_token = await self._attempt_token_refresh( + access_token.token, external_token + ) + + if refreshed_token: + logger.info( + "โœ… Token refresh successful, retrying user context" + ) + # Retry with the new token + user_context = await self.oauth_handler.get_user_context( + refreshed_token + ) + # Update external_token for caching + external_token = refreshed_token + else: + logger.error( + "โŒ Token refresh failed, re-raising original error" + ) + raise + else: + # Not a token expiry error, re-raise + raise # Cache the result for future requests await self._cache_user_context(external_token, user_context) From c95dda5eb7c4640b4a96f9630b7567c7cd36761a Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Wed, 10 Sep 2025 11:11:12 +0200 Subject: [PATCH 15/21] add database migration for refresh_token column --- src/mxcp/sdk/auth/persistence.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/mxcp/sdk/auth/persistence.py b/src/mxcp/sdk/auth/persistence.py index 22cf5d84..bb0c7d42 100644 --- a/src/mxcp/sdk/auth/persistence.py +++ b/src/mxcp/sdk/auth/persistence.py @@ -216,6 +216,18 @@ def _create_tables_sync(self) -> None: """ ) + # Migration: Add refresh_token column if it doesn't exist + try: + cursor.execute("ALTER TABLE access_tokens ADD COLUMN refresh_token TEXT") + logger.info("โœ… Added refresh_token column to existing access_tokens table") + except sqlite3.OperationalError as e: + if "duplicate column name" in str(e).lower(): + # Column already exists, this is expected for new databases + logger.debug("refresh_token column already exists") + else: + logger.error(f"Error adding refresh_token column: {e}") + raise + # Authorization codes table cursor.execute( """ From 606417448703594e329242871170a039e06d906e Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Wed, 10 Sep 2025 11:34:50 +0200 Subject: [PATCH 16/21] fix refresh token lookup to use external token key --- src/mxcp/sdk/auth/base.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/mxcp/sdk/auth/base.py b/src/mxcp/sdk/auth/base.py index 77b61dc9..8fe7fcd2 100644 --- a/src/mxcp/sdk/auth/base.py +++ b/src/mxcp/sdk/auth/base.py @@ -732,9 +732,12 @@ async def refresh_external_token(self, mcp_token: str) -> str | None: return None try: - persisted_token = await self.persistence.load_token(mcp_token) + # Look up the refresh token using the external token, not the MCP token + persisted_token = await self.persistence.load_token(external_token) if not persisted_token or not persisted_token.refresh_token: - logger.warning(f"No refresh token available for MCP token: {mcp_token[:10]}...") + logger.warning( + f"No refresh token available for external token: {external_token[:20]}..." + ) return None # Call the provider's refresh method From 83aa5d9452fda7665b7c2d06f8f314569dab7296 Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Wed, 10 Sep 2025 12:11:02 +0200 Subject: [PATCH 17/21] fix: use MCP token as cache key instead of external token This prevents cache entries from becoming orphaned when external tokens are refreshed. The MCP token remains stable throughout the session, making it a more reliable cache key. --- src/mxcp/sdk/auth/middleware.py | 41 ++++++++++++++------------------- 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/src/mxcp/sdk/auth/middleware.py b/src/mxcp/sdk/auth/middleware.py index ed909b4c..8a9aa6d9 100644 --- a/src/mxcp/sdk/auth/middleware.py +++ b/src/mxcp/sdk/auth/middleware.py @@ -59,45 +59,45 @@ def __init__( self._user_context_cache: dict[str, CachedUserContext] = {} self._cache_lock = asyncio.Lock() - async def _get_cached_user_context(self, external_token: str) -> UserContext | None: + async def _get_cached_user_context(self, mcp_token: str) -> UserContext | None: """Get user context from cache if valid and not expired. Args: - external_token: External OAuth token to use as cache key + mcp_token: MCP token to use as cache key Returns: Cached user context if valid, None if expired or not found """ async with self._cache_lock: - cached_entry = self._user_context_cache.get(external_token) + cached_entry = self._user_context_cache.get(mcp_token) if cached_entry is None: return None if cached_entry.is_expired(): # Remove expired entry - del self._user_context_cache[external_token] - logger.debug(f"โฐ Cache EXPIRED - removed entry for token {external_token[:20]}...") + del self._user_context_cache[mcp_token] + logger.debug(f"โฐ Cache EXPIRED - removed entry for token {mcp_token[:20]}...") return None logger.debug( - f"๐ŸŽฏ Cache HIT - using cached user context for token {external_token[:20]}..." + f"๐ŸŽฏ Cache HIT - using cached user context for token {mcp_token[:20]}..." ) return cached_entry.user_context - async def _cache_user_context(self, external_token: str, user_context: UserContext) -> None: + async def _cache_user_context(self, mcp_token: str, user_context: UserContext) -> None: """Store user context in cache with expiration. Args: - external_token: External OAuth token to use as cache key + mcp_token: MCP token to use as cache key user_context: User context to cache """ expires_at = time.time() + self.cache_ttl cached_entry = CachedUserContext(user_context=user_context, expires_at=expires_at) async with self._cache_lock: - self._user_context_cache[external_token] = cached_entry + self._user_context_cache[mcp_token] = cached_entry logger.debug( - f"๐Ÿ’พ Cache STORE - cached user context for token {external_token[:20]}... (TTL: {self.cache_ttl}s)" + f"๐Ÿ’พ Cache STORE - cached user context for token {mcp_token[:20]}... (TTL: {self.cache_ttl}s)" ) async def _cleanup_expired_cache_entries(self) -> None: @@ -131,17 +131,12 @@ async def _attempt_token_refresh(self, mcp_token: str, external_token: str) -> s return None try: - # Invalidate cache for the expired token + # Invalidate cache for the expired MCP token async with self._cache_lock: - # Remove any cached entries for the expired token - expired_cache_keys = [ - key - for key, cached_entry in self._user_context_cache.items() - if key == external_token - ] - for key in expired_cache_keys: - del self._user_context_cache[key] - logger.debug(f"๐Ÿ—‘๏ธ Removed expired token from cache: {key[:20]}...") + # Remove cached entry for this MCP token + if mcp_token in self._user_context_cache: + del self._user_context_cache[mcp_token] + logger.debug(f"๐Ÿ—‘๏ธ Removed expired token from cache: {mcp_token[:20]}...") # Attempt refresh through the OAuth server new_external_token = await self.oauth_server.refresh_external_token(mcp_token) @@ -240,7 +235,7 @@ async def check_authentication(self) -> UserContext | None: return None # Try to get user context from cache first - user_context = await self._get_cached_user_context(external_token) + user_context = await self._get_cached_user_context(access_token.token) if user_context is None: # Cache miss - call provider API @@ -273,8 +268,6 @@ async def check_authentication(self) -> UserContext | None: user_context = await self.oauth_handler.get_user_context( refreshed_token ) - # Update external_token for caching - external_token = refreshed_token else: logger.error( "โŒ Token refresh failed, re-raising original error" @@ -285,7 +278,7 @@ async def check_authentication(self) -> UserContext | None: raise # Cache the result for future requests - await self._cache_user_context(external_token, user_context) + await self._cache_user_context(access_token.token, user_context) # Trigger periodic cleanup of expired cache entries (non-blocking) asyncio.create_task(self._cleanup_expired_cache_entries()) From dc492d42aaef470276f1f97481c4d1896ca311c7 Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Wed, 10 Sep 2025 12:11:42 +0200 Subject: [PATCH 18/21] fix: prevent race conditions in token refresh with per-token locks Adds per-MCP-token locks to prevent multiple concurrent requests from triggering parallel refresh attempts for the same token. Includes optimization to check if another request already completed the refresh before proceeding. --- src/mxcp/sdk/auth/middleware.py | 73 +++++++++++++++++++++++---------- 1 file changed, 52 insertions(+), 21 deletions(-) diff --git a/src/mxcp/sdk/auth/middleware.py b/src/mxcp/sdk/auth/middleware.py index 8a9aa6d9..5c809740 100644 --- a/src/mxcp/sdk/auth/middleware.py +++ b/src/mxcp/sdk/auth/middleware.py @@ -58,6 +58,24 @@ def __init__( # Thread-safe cache for user contexts self._user_context_cache: dict[str, CachedUserContext] = {} self._cache_lock = asyncio.Lock() + + # Per-MCP-token locks for refresh operations to prevent race conditions + self._refresh_locks: dict[str, asyncio.Lock] = {} + self._refresh_locks_lock = asyncio.Lock() + + async def _get_refresh_lock(self, mcp_token: str) -> asyncio.Lock: + """Get or create a refresh lock for the given MCP token. + + Args: + mcp_token: MCP token to get lock for + + Returns: + Lock specific to this MCP token + """ + async with self._refresh_locks_lock: + if mcp_token not in self._refresh_locks: + self._refresh_locks[mcp_token] = asyncio.Lock() + return self._refresh_locks[mcp_token] async def _get_cached_user_context(self, mcp_token: str) -> UserContext | None: """Get user context from cache if valid and not expired. @@ -130,30 +148,43 @@ async def _attempt_token_refresh(self, mcp_token: str, external_token: str) -> s logger.warning("No OAuth server available for token refresh") return None - try: - # Invalidate cache for the expired MCP token - async with self._cache_lock: - # Remove cached entry for this MCP token - if mcp_token in self._user_context_cache: - del self._user_context_cache[mcp_token] - logger.debug(f"๐Ÿ—‘๏ธ Removed expired token from cache: {mcp_token[:20]}...") - - # Attempt refresh through the OAuth server - new_external_token = await self.oauth_server.refresh_external_token(mcp_token) + # Get the refresh lock for this specific MCP token to prevent race conditions + refresh_lock = await self._get_refresh_lock(mcp_token) + + async with refresh_lock: + try: + # Check if another request already refreshed the token + # by checking if we have a valid cached user context now + cached_context = await self._get_cached_user_context(mcp_token) + if cached_context is not None: + logger.debug(f"๐ŸŽฏ Token already refreshed by another request for {mcp_token[:20]}...") + # Get the current external token from token mapping + current_external_token = self.oauth_server._token_mapping.get(mcp_token) + return current_external_token + + # Invalidate cache for the expired MCP token + async with self._cache_lock: + # Remove cached entry for this MCP token + if mcp_token in self._user_context_cache: + del self._user_context_cache[mcp_token] + logger.debug(f"๐Ÿ—‘๏ธ Removed expired token from cache: {mcp_token[:20]}...") + + # Attempt refresh through the OAuth server + new_external_token = await self.oauth_server.refresh_external_token(mcp_token) + + if new_external_token: + logger.info( + f"๐Ÿ”„ Successfully refreshed external token: {new_external_token[:20]}..." + ) + return new_external_token + else: + logger.warning("Token refresh returned no new token") + return None - if new_external_token: - logger.info( - f"๐Ÿ”„ Successfully refreshed external token: {new_external_token[:20]}..." - ) - return new_external_token - else: - logger.warning("Token refresh returned no new token") + except Exception as e: + logger.error(f"Error during token refresh: {e}") return None - except Exception as e: - logger.error(f"Error during token refresh: {e}") - return None - async def check_authentication(self) -> UserContext | None: """Check if the current request is authenticated. From ae7a42baa9375cb099bd7efbf04deee1bed17262 Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Wed, 10 Sep 2025 12:12:15 +0200 Subject: [PATCH 19/21] fix: improve error handling with proper HTTPException type checking Replaces fragile hasattr() checks with explicit HTTPException type checking for 401 errors. Adds separate handling for non-HTTP exceptions with proper logging for better debugging. --- src/mxcp/sdk/auth/middleware.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/mxcp/sdk/auth/middleware.py b/src/mxcp/sdk/auth/middleware.py index 5c809740..c93fca16 100644 --- a/src/mxcp/sdk/auth/middleware.py +++ b/src/mxcp/sdk/auth/middleware.py @@ -9,6 +9,7 @@ from typing import Any from mcp.server.auth.middleware.auth_context import get_access_token +from starlette.exceptions import HTTPException from mxcp.sdk.telemetry import record_counter, traced_operation @@ -281,9 +282,9 @@ async def check_authentication(self) -> UserContext | None: user_context = await self.oauth_handler.get_user_context( external_token ) - except Exception as e: + except HTTPException as e: # Check if this is a 401/token expired error - if hasattr(e, "status_code") and e.status_code == 401: + if e.status_code == 401: logger.info("๐Ÿ”„ Access token expired, attempting refresh...") # Attempt to refresh the token @@ -307,6 +308,10 @@ async def check_authentication(self) -> UserContext | None: else: # Not a token expiry error, re-raise raise + except Exception as e: + # Handle non-HTTP exceptions (network errors, etc.) + logger.error(f"Non-HTTP error during get_user_context: {e}") + raise # Cache the result for future requests await self._cache_user_context(access_token.token, user_context) From 143f7aacb5218bea8729dd0309726ad6e55301a9 Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Wed, 10 Sep 2025 12:18:35 +0200 Subject: [PATCH 20/21] fix: standardize OAuth logging levels for production readiness - DEBUG: Normal operations (cache hits/stores, token validation, client lookups) - INFO: Important events (token refresh attempts/success, errors) - Reduces log noise in production while keeping essential information --- src/mxcp/sdk/auth/base.py | 6 +++--- src/mxcp/sdk/auth/middleware.py | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/mxcp/sdk/auth/base.py b/src/mxcp/sdk/auth/base.py index 8fe7fcd2..2259f88c 100644 --- a/src/mxcp/sdk/auth/base.py +++ b/src/mxcp/sdk/auth/base.py @@ -257,7 +257,7 @@ async def get_client(self, client_id: str) -> OAuthClientInformationFull | None: # First check memory cache client = self._clients.get(client_id) if client: - logger.info(f"Looking up client_id: {client_id}, found in memory cache") + logger.debug(f"Looking up client_id: {client_id}, found in memory cache") return client # If not in cache and persistence is available, check persistence @@ -300,7 +300,7 @@ async def get_client(self, client_id: str) -> OAuthClientInformationFull | None: client_name=persisted_client.client_name, ) self._clients[client_id] = client - logger.info(f"Looking up client_id: {client_id}, found in persistence") + logger.debug(f"Looking up client_id: {client_id}, found in persistence") return client except Exception as e: logger.error(f"Error loading client from persistence: {e}") @@ -557,7 +557,7 @@ async def load_authorization_code( code_challenge=persisted_code.code_challenge or "", ) self._auth_codes[code] = auth_code - logger.info(f"Loaded auth code from persistence: {code}") + logger.debug(f"Loaded auth code from persistence: {code}") except Exception as e: logger.error(f"Error loading auth code from persistence: {e}") diff --git a/src/mxcp/sdk/auth/middleware.py b/src/mxcp/sdk/auth/middleware.py index c93fca16..c5e2c5e4 100644 --- a/src/mxcp/sdk/auth/middleware.py +++ b/src/mxcp/sdk/auth/middleware.py @@ -219,7 +219,7 @@ async def check_authentication(self) -> UserContext | None: ) return None - logger.info("Access token found in request context") + logger.debug("Access token found in request context") if span: span.set_attribute("mxcp.auth.has_token", True) @@ -245,7 +245,7 @@ async def check_authentication(self) -> UserContext | None: token_span.set_attribute("mxcp.auth.token_valid", True) token_span.set_attribute("mxcp.auth.client_id", token_info.client_id) - logger.info(f"Token validated successfully for client: {token_info.client_id}") + logger.debug(f"Token validated successfully for client: {token_info.client_id}") # Get the external token to fetch user context if not self.oauth_server: @@ -257,7 +257,7 @@ async def check_authentication(self) -> UserContext | None: logger.warning("No external token mapping found") return None - logger.info("External token mapping found") + logger.debug("External token mapping found") # Get standardized user context from the provider (with caching) try: @@ -274,7 +274,7 @@ async def check_authentication(self) -> UserContext | None: provider_name = getattr( self.oauth_handler, "__class__", type(self.oauth_handler) ).__name__ - logger.info( + logger.debug( f"๐Ÿ”„ Cache MISS - calling {provider_name}.get_user_context() - Provider API call #{hash(external_token) % 10000}" ) @@ -320,7 +320,7 @@ async def check_authentication(self) -> UserContext | None: asyncio.create_task(self._cleanup_expired_cache_entries()) # Add external token to the user context for use in DuckDB functions user_context.external_token = external_token - logger.info( + logger.debug( f"Successfully retrieved user context for {user_context.username} (provider: {user_context.provider})" ) @@ -392,7 +392,7 @@ async def wrapper(*args: Any, **kwargs: Any) -> Any: user_context = await self.check_authentication() if user_context: # Log authentication status without PII - logger.info( + logger.debug( f"Executing {func.__name__} for authenticated user " f"(provider: {user_context.provider})" ) From 5d42b9c295bb52ab24663de7d3f7b9043a37a18d Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Thu, 11 Sep 2025 15:15:23 +0200 Subject: [PATCH 21/21] fixup! fix: standardize OAuth logging levels for production readiness wip --- src/mxcp/sdk/auth/base.py | 4 ++-- src/mxcp/sdk/auth/middleware.py | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/mxcp/sdk/auth/base.py b/src/mxcp/sdk/auth/base.py index 2259f88c..d17ea4f9 100644 --- a/src/mxcp/sdk/auth/base.py +++ b/src/mxcp/sdk/auth/base.py @@ -757,7 +757,7 @@ async def refresh_external_token(self, mcp_token: str) -> str | None: "refresh_token", persisted_token.refresh_token ) - if not new_access_token: + if not new_access_token or not isinstance(new_access_token, str): logger.error("No access token received from refresh") return None @@ -779,7 +779,7 @@ async def refresh_external_token(self, mcp_token: str) -> str | None: logger.info( f"Successfully refreshed external token for MCP token: {mcp_token[:10]}..." ) - return new_access_token + return str(new_access_token) except Exception as e: logger.error(f"Error refreshing external token: {e}") diff --git a/src/mxcp/sdk/auth/middleware.py b/src/mxcp/sdk/auth/middleware.py index c5e2c5e4..053a1adf 100644 --- a/src/mxcp/sdk/auth/middleware.py +++ b/src/mxcp/sdk/auth/middleware.py @@ -59,17 +59,17 @@ def __init__( # Thread-safe cache for user contexts self._user_context_cache: dict[str, CachedUserContext] = {} self._cache_lock = asyncio.Lock() - + # Per-MCP-token locks for refresh operations to prevent race conditions self._refresh_locks: dict[str, asyncio.Lock] = {} self._refresh_locks_lock = asyncio.Lock() async def _get_refresh_lock(self, mcp_token: str) -> asyncio.Lock: """Get or create a refresh lock for the given MCP token. - + Args: mcp_token: MCP token to get lock for - + Returns: Lock specific to this MCP token """ @@ -98,9 +98,7 @@ async def _get_cached_user_context(self, mcp_token: str) -> UserContext | None: logger.debug(f"โฐ Cache EXPIRED - removed entry for token {mcp_token[:20]}...") return None - logger.debug( - f"๐ŸŽฏ Cache HIT - using cached user context for token {mcp_token[:20]}..." - ) + logger.debug(f"๐ŸŽฏ Cache HIT - using cached user context for token {mcp_token[:20]}...") return cached_entry.user_context async def _cache_user_context(self, mcp_token: str, user_context: UserContext) -> None: @@ -151,14 +149,16 @@ async def _attempt_token_refresh(self, mcp_token: str, external_token: str) -> s # Get the refresh lock for this specific MCP token to prevent race conditions refresh_lock = await self._get_refresh_lock(mcp_token) - + async with refresh_lock: try: # Check if another request already refreshed the token # by checking if we have a valid cached user context now cached_context = await self._get_cached_user_context(mcp_token) if cached_context is not None: - logger.debug(f"๐ŸŽฏ Token already refreshed by another request for {mcp_token[:20]}...") + logger.debug( + f"๐ŸŽฏ Token already refreshed by another request for {mcp_token[:20]}..." + ) # Get the current external token from token mapping current_external_token = self.oauth_server._token_mapping.get(mcp_token) return current_external_token