feat: add in-memory LRU cache for per-user MCP OAuth access tokens with targeted eviction - #5720
Conversation
|
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds a bounded per-user OAuth token cache with expiry-aware loading, single-flight fills, sanitized cached values, and scoped eviction. MCP handlers and server callbacks evict entries after credential, token, client, and virtual-key changes. ChangesOAuth token cache
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MCPHandler
participant BifrostHTTPServer
participant OAuth2Provider
participant UserTokenCache
MCPHandler->>BifrostHTTPServer: reconcile MCP credentials
BifrostHTTPServer->>OAuth2Provider: evict tokens by MCP client
OAuth2Provider->>UserTokenCache: remove matching entries
MCPHandler-->>BifrostHTTPServer: complete configuration update
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
9004cee to
2c5a041
Compare
514b8e1 to
26a84a1
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
2c5a041 to
c50dff4
Compare
26a84a1 to
abd598d
Compare
Merge activity
|
The base branch was changed.
…unblocks instead of waiting on an unrelated leader
c50dff4 to
7e5cb69
Compare

Summary
Per-user MCP OAuth access tokens were fetched from the database on every request. This PR introduces a bounded, LRU in-memory cache for those lookups, keyed by
(auth mode, identity, mcp client), so repeated requests for the same binding skip the database round-trip. Expired entries are treated as misses and fall through to the full database path, which retains ownership of all expiry and refresh decisions. The cache is kept consistent through targeted evictions wired into every write path that can invalidate a cached token: refresh, revocation, force-refresh, OAuth flow completion, credential rotation, access reconciliation, VK reload/delete, and MCP client update/delete.Changes
userTokenCacheinframework/oauth2/usertokencache.go: a thin wrapper around the existing LRU cache that owns the(mode, identity, mcp client)key scheme, registers each entry under its token row ID for targeted eviction, validates entries against their expiry on read, and deduplicates concurrent fills for the same key so a refresh burst triggers at most one upstream call.GetUserAccessTokenByModeinto a cache-aware outer function and aloadUserAccessTokenByModecache-miss path. Cache hits bypass the database entirely; misses run the full lookup including the lazy pre-flight refresh.OAuth2Provider:EvictUserToken,EvictUserTokenByID,EvictUserTokensByMCPClient,EvictUserTokensByVirtualKey,EvictUserTokensByUser, andFlushUserTokenCache. All are nil-safe.main.go: afterRefreshAccessToken(both success and permanent rejection),ForceRefreshAccessToken(pre-eviction before the refresh call),RevokeToken,CompleteOAuthFlow, andCompleteUserOAuthFlow.MCPOauthTokenCacheManagerinterface inmcpsessions.gowithEvictOauthTokenCacheByIDandEvictOauthTokenCacheByMCPClient. Implemented onBifrostHTTPServerand wired intoMCPSessionsHandler(session revoke) andMCPHandler(credential rotation, reconciliation after MCP client update).EvictOauthTokenCacheByID,EvictOauthTokenCacheByMCPClient,EvictOauthTokenCacheByVirtualKey, andFlushOauthTokenCachetoServerCallbacksand implemented them onBifrostHTTPServer, delegating to the provider's eviction methods.RemoveMCPClient,ReloadVirtualKey, andRemoveVirtualKeyinserver.goto cover token rows cascaded by database deletes.updateVirtualKeyingovernance.gosoReconcileOauthAfterVKChangeruns beforeReloadVirtualKey: the reload evicts VK-scoped tokens, and an eviction landing before the reconcile writes could be refilled from pre-reconcile rows and never dropped again.usertokencache_test.gocovering: hit/miss, LRU capacity eviction, expired-entry miss-and-remove, exact-key eviction, token-ID eviction, MCP-client bulk eviction, virtual-key bulk eviction, user bulk eviction, flush, in-flight deduplication, error sharing without caching, generation-guard discarding a stale fill that raced an eviction, token-ID index rebinding after upsert, nil safety, and integration tests throughGetUserAccessTokenByModefor cache hits, concurrent refresh deduplication, post-delete eviction, post-needs-reauth flush, negative-result non-caching, and force-refresh eviction.Type of change
Affected areas
How to test
go test ./framework/oauth2/... ./transports/bifrost-http/...The new
usertokencache_test.gofile covers the cache unit behaviour and integration throughGetUserAccessTokenByMode. Key scenarios to validate manually:Breaking changes
NewMCPHandlerandNewMCPSessionsHandlerhave new required parameters (MCPOauthTokenCacheManager). Any caller constructing these handlers directly outside the server must pass an implementation ofMCPOauthTokenCacheManager. TheServerCallbacksinterface has two new methods (EvictOauthTokenCacheByID,EvictOauthTokenCacheByMCPClient); any customServerCallbacksimplementation must add them.Security considerations
Cached access tokens are held in process memory and are never persisted. The cache is bounded to 4096 entries by default to prevent unbounded growth from caller-asserted session identities. Evictions are wired to every write path that can invalidate a token, including revocation and needs-reauth transitions, so a revoked or expired token cannot be served from cache beyond the current request's lifetime.
Checklist
docs/contributing/README.mdand followed the guidelines