Skip to content

Conversation

@bgaidioz
Copy link
Contributor

@bgaidioz bgaidioz commented Oct 6, 2025

Description

Each tool invocation fetches the user context to pass as a parameter. This currently triggers a call to the AS on every request. In this changeset, a five-minute cache is added to minimize remote AS calls.

The changeset also adds the logic to trigger an external token refresh when the external access token expires. This implies a schema change to the OAuth DB. This doesn't implement a schema migration.

We finally set the MCP access token duration to 365 days while the possibility of refreshing isn't provided by the mcp lib.

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📝 Documentation update
  • 🔧 Refactoring (no functional changes, no api changes)
  • ⚡ Performance improvement
  • 🧪 Test improvement
  • 🔒 Security fix

Testing

  • Tests pass locally with uv run pytest
  • Linting passes with uv run ruff check .
  • Code formatting passes with uv run black --check .
  • Type checking passes with uv run mypy .
  • Added tests for new functionality (if applicable)
  • Updated documentation (if applicable)

Security Considerations

  • This change does not introduce security vulnerabilities
  • Sensitive data handling reviewed (if applicable)
  • Policy enforcement implications considered (if applicable)

Breaking Changes

No.

Additional Notes

When calling within cache duration, the cache is used:

INFO:mcp.server.lowlevel.server:Processing request of type CallToolRequest
...
...
...
DEBUG:mxcp.sdk.auth.middleware:External token mapping found
DEBUG:mxcp.sdk.auth.middleware:🎯 Cache HIT - using cached user context for token mcp_336188be92afbb05...
DEBUG:mxcp.sdk.auth.middleware:Successfully retrieved user context for ben (provider: keycloak)
...
...
...
INFO:mxcp.server.interfaces.server.mcp:Authenticated user: ben (provider: keycloak)

When the cache is expired, the value is ignored and a call is issued to the external AS:

INFO:mcp.server.lowlevel.server:Processing request of type CallToolRequest
...
...
...
DEBUG:mxcp.sdk.auth.middleware:⏰ Cache EXPIRED - removed entry for token mcp_f03722757d9e2f2e...
DEBUG:mxcp.sdk.auth.middleware:🔄 Cache MISS - calling KeycloakOAuthHandler.get_user_context() - Provider API call #6610
...
...
...
INFO:httpx:HTTP Request: GET http://localhost:8080/realms/demo/protocol/openid-connect/userinfo "HTTP/1.1 200 OK"
...
...
...
DEBUG:mxcp.sdk.auth.middleware:💾 Cache STORE - cached user context for token mcp_f03722757d9e2f2e... (TTL: 300s)
DEBUG:mxcp.sdk.auth.middleware:Successfully retrieved user context for ben (provider: keycloak)
DEBUG:mxcp.sdk.auth.middleware:Executing _body for authenticated user (provider: keycloak)
INFO:mxcp.server.interfaces.server.mcp:Calling tool get_user_info with: {}
INFO:mxcp.server.interfaces.server.mcp:Authenticated user: ben (provider: keycloak)

Here's the log when calling a tool after more than five minutes (cache expiration) and the keycloak access token has expired in the meantime.

  1. The cached entry is discarded because it has expired.
  2. The call to keycloak to read again the user context fails because the external (keycloak) token has expired.
  3. The keycloak access token is refreshed using the AS refresh token.
  4. The call to read the user context succeeds.
  5. The MCP tool is called with a recent user context.
INFO:mcp.server.lowlevel.server:Processing request of type CallToolRequest
...
...
...
DEBUG:mxcp.sdk.auth.middleware:⏰ Cache EXPIRED - removed entry for token mcp_336188be92afbb05...
DEBUG:mxcp.sdk.auth.middleware:🔄 Cache MISS - calling KeycloakOAuthHandler.get_user_context() - Provider API call #8377
...
...
...
INFO:httpx:HTTP Request: GET http://localhost:8080/realms/demo/protocol/openid-connect/userinfo "HTTP/1.1 401 Unauthorized"
...
...
...
ERROR:mxcp.sdk.auth.providers.keycloak:Failed to get user info: 401
...
...
...
INFO:mxcp.sdk.auth.middleware:🔄 Access token expired, attempting refresh...
...
...
...
INFO:httpx:HTTP Request: POST http://localhost:8080/realms/demo/protocol/openid-connect/token "HTTP/1.1 200 OK"
...
...
...
INFO:mxcp.sdk.auth.base:Successfully refreshed external token for MCP token: mcp_336188... (new expires_at: 1759761783.1377778)
INFO:mxcp.sdk.auth.middleware:🔄 Successfully refreshed external token: eyJhbGciOiJSUzI1NiIs...
INFO:mxcp.sdk.auth.middleware:✅ Token refresh successful, retrying user context
...
...
...
INFO:httpx:HTTP Request: GET http://localhost:8080/realms/demo/protocol/openid-connect/userinfo "HTTP/1.1 200 OK"
...
...
...
DEBUG:mxcp.sdk.auth.middleware:💾 Cache STORE - cached user context for token mcp_336188be92afbb05... (TTL: 300s)
DEBUG:mxcp.sdk.auth.middleware:Successfully retrieved user context for ben (provider: keycloak)
DEBUG:mxcp.sdk.auth.middleware:Executing _body for authenticated user (provider: keycloak)
INFO:mxcp.server.interfaces.server.mcp:Calling tool get_user_info with: {}
INFO:mxcp.server.interfaces.server.mcp:Authenticated user: ben (provider: keycloak)

Note

Adds cached user context with configurable TTL, background OAuth cleanup, external token refresh with persisted refresh tokens, extends MXCP token TTL to 1 year, and updates config, docs, and tests.

  • Auth Server / Core:
    • Add external token refresh flow refresh_external_token() with persisted refresh_token; map and update external tokens on refresh.
    • Introduce background cleanup loop for expired auth codes and orphaned token/refresh mappings; configurable via auth.cleanup_interval (default 300s).
    • Extend MXCP access token lifetime to 1 year (DEFAULT_ACCESS_TOKEN_TTL).
    • Improve locking and logging; clean up mappings on code expiration.
  • Middleware (src/mxcp/sdk/auth/middleware.py):
    • Implement in‑memory user context cache with TTL (cache_ttl, default 300s), periodic cleanup, and concurrency-safe refresh.
    • Auto-refresh expired external tokens on 401 and retry get_user_context, caching the result.
  • Providers:
    • Add refresh support methods to keycloak, google, atlassian, salesforce; set refresh_token on ExternalUserInfo. GitHub remains non-refresh.
    • Default Keycloak scope includes offline_access.
  • Persistence (src/mxcp/sdk/auth/persistence.py):
    • Store refresh_token in access_tokens; add column and runtime migration; load/store updated.
  • Types / Config:
    • Extend AuthConfig with cache_ttl and cleanup_interval; add refresh_token to ExternalUserInfo.
    • Wire cache_ttl into server middleware (src/mxcp/server/interfaces/server/mcp.py).
    • Update JSON schema (mxcp-config-schema-1.json) to include auth.cache_ttl and auth.cleanup_interval.
  • Docs (docs/guides/authentication.md):
    • Document User Context Caching and OAuth Cleanup configuration and considerations.
  • Examples: Simplify examples/keycloak/config.yml and remove inline client.
  • Tests: Add middleware caching tests (tests/sdk/auth/test_middleware.py).

Written by Cursor Bugbot for commit 6d820e4. This will update automatically on new commits. Configure here.

cursor[bot]

This comment was marked as outdated.

@bgaidioz bgaidioz force-pushed the feature/oauth-user-context-caching branch from f474c13 to 31d822c Compare October 6, 2025 13:42
@bgaidioz bgaidioz force-pushed the feature/oauth-refresh-tokens branch from 7ff502b to a282c2b Compare October 6, 2025 14:23
@bgaidioz bgaidioz changed the base branch from feature/oauth-user-context-caching to main October 6, 2025 14:23
@bgaidioz bgaidioz changed the title Refresh external tokens Add OAuth user context caching + Refresh external tokens Oct 6, 2025
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@bgaidioz bgaidioz force-pushed the feature/oauth-refresh-tokens branch from 9bf103d to 4e9b05a Compare October 13, 2025 09:28
cursor[bot]

This comment was marked as outdated.

@bgaidioz bgaidioz force-pushed the feature/oauth-refresh-tokens branch from 4e9b05a to 644def6 Compare October 13, 2025 09:51
- 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.
@bgaidioz bgaidioz force-pushed the feature/oauth-refresh-tokens branch from 644def6 to bed1395 Compare October 17, 2025 13:53
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.


except Exception as e:
logger.error(f"Error refreshing external token: {e}")
return None
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: OAuth Locking During Network Calls

The refresh_external_token method holds self._lock during an external network call to refresh the access token. This blocks all other OAuth operations for the duration of the network request, potentially causing performance degradation and timeouts.

Fix in Cursor Fix in Web


user_context = await self.oauth_handler.get_user_context(external_token)
# Try to get user context from cache first
cached_user_context = await self._get_cached_user_context(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Race Condition in OAuth User Context Retrieval

Race condition: The code checks if cached_user_context is None and then calls self.oauth_handler.get_user_context() without holding any lock. Multiple concurrent requests for the same token could all see cached_user_context is None and make duplicate API calls to the OAuth provider, defeating the purpose of the cache. The cache lock (self._cache_lock) is only held when reading/writing the cache, not during the actual API call, so multiple requests can race through this section.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants