Commit graph

3 commits

Author SHA1 Message Date
J.C. Bartle
bf251b820f ♻️ refactor: Pass OBO upstream-token closure into MCP instead of req
Build the OpenID upstream-token provider at the request boundary and thread
only the closure through MCP handling, so the MCP service layer no longer
receives the raw Express request. The closure still reads/refreshes the live
session at tool-call time, preserving the walk-away recovery.

- Drop `req`/`capturedReq` from createMCPTools, createMCPTool, reconnectServer,
  createToolInstance, and reinitMCPServer; forward `upstreamTokenProvider`
  instead. Closure is constructed in loadTools, loadToolDefinitionsWrapper, and
  the reinitialize route, where req/res are in scope.
- OBO: fall back to user.federatedTokens when the provider yields no live
  session, so OIDC remote-agent calls (verified bearer, no session) still work.
- Inline refresh: mirror a rotated refresh token to the refreshToken cookie via
  a shared setRefreshTokenCookie helper, guarded by !res.headersSent (no-op on
  the streaming path; session copy stays authoritative).
- Single-flight: hydrate a joining request's own session from the resolved
  tokens so a later OBO call doesn't replay a rotated-away refresh token.

Addresses owner feedback and three review findings.
2026-06-23 11:59:01 -04:00
J.C. Bartle
07c4a906d2 🛡️ fix: Harden OBO inline-refresh against token type and session edge cases
- Token-preference asymmetry: live-token reuse and expires_at derivation
  now strictly gate on the access_token, not the id_token. Added a
  required `tokenPreference` parameter on isLiveSessionTokenStillValid,
  buildOIDCTokensFromSession, and createOpenIDSessionTokenProvider
  so every call site is explicit. Dropped the bogus id_token-exp
  fallback in performIdpRefresh — id_token TTL is governed by IdP
  session policy and would mark a short-lived access_token reusable
  past its real lifetime.
- Missing req in /reinitialize route: the manual reconnect
  endpoint now forwards req into reinitMCPServer, so OBO servers can
  build a session-aware upstream-token closure instead of failing with
  missing_upstream_token.
- Single-flight key collisions: composed key as
  tenantId:openidIssuer:openidId:sessionId via getSingleFlightKey.
  Concurrent calls in the same session still coalesce; separate sessions
  never share an in-flight refresh, preventing refresh-token rotation
  from breaking sibling sessions and preventing cross-tenant token
  crossover when distinct users share an IdP sub.
- Opaque access token reuse): persist accessTokenExpiresAt
  (unix seconds, from tokenset.expires_in) on each refresh AND on initial
  login / SPA refresh in setOpenIDAuthTokens. New getAccessTokenExp
  helper falls back to it when the access token isn't a JWT, avoiding
  redundant inline refreshes for Microsoft Graph and Auth0 default
  audiences.
- Log hygiene: the single-flight key (containing sessionId,
  openidId, openidIssuer, tenantId) is now SHA-256-hashed in the
  "Joining in-flight refresh" debug log. Preserves cross-line correlation
  via a 12-char prefix without leaking credential or PII material.

Documented req.session.openidTokens shape contract via JSDoc typedef so
the new accessTokenExpiresAt field has a discoverable home alongside the
existing accessToken/idToken/refreshToken/expiresAt/lastRefreshedAt.

Tests: OpenIDSessionRefresh.spec.js up to 30 passing (added coverage for
opaque-token reuse, JWT-access-token-exp fallback, no-id_token-fallback
regression, cross-session no-coalesce, persistence on refresh, and a
guard against stale accessTokenExpiresAt carryover). AuthService.spec.js
adds two cases covering accessTokenExpiresAt persistence on login.
mcp.spec.js (route) gains a regression test asserting req flows into
reinitMCPServer.
2026-06-13 17:52:01 -04:00
J.C. Bartle
6e46fbbb59 🧊 fix: Inline-refresh OpenID session tokens at MCP OBO call time
Resolves the walk-away failure mode where MCP tool calls using OBO auth
fail with "No valid OpenID access token is available for OBO exchange"
after a user idles past their access-token lifetime. The strategy-time
snapshot on `user.federatedTokens` could expire mid-stream before
`resolveOboToken` ran, while `req.session.openidTokens` carried a still-
valid (or refreshable) token that nothing read.

- New OpenIDSessionRefresh service: per-user single-flighted closure that
  reads `req.session.openidTokens` at OBO time and inline-refreshes via
  `openid-client.refreshTokenGrant` when expired (30s skew), persisting
  via `req.session.save()`. No cookie writes (headers already flushed).
- `resolveOboToken` gains a required UpstreamTokenProvider parameter
  (typed as `() => Promise<OIDCTokens | null>`, reusing the shared shape
  from @librechat/data-schemas). Compile-time guarantee that every call
  site is updated.
- New `session_refresh_failed` OboTokenResolutionReason distinguishes
  "session expired and IdP rejected refresh" from "no upstream token
  ever existed."
- `req` threaded through createMCPTool/createMCPTools/createToolInstance
  to construct the closure with captured request, plus fail-closed
  guards in MCPConnectionFactory.getOboTokens and MCPManager.callTool
  when the closure isn't plumbed.
- Startup warning in MCPServersInitializer when OBO is configured but
  OPENID_REUSE_TOKENS is unset (the strategy populating
  user.federatedTokens is only registered under reuse, so OBO would
  fail every call without it).

Tests: 16 new in OpenIDSessionRefresh.spec.js; obo.spec.ts extended
for the new param + error reason; wiring smoke tests in MCPManager,
MCPConnectionFactory, MCPServersInitializer, and MCP.spec.js.
2026-06-11 21:36:04 -04:00