mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-09-21 15:45:22 +00:00
🛡️ 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.
This commit is contained in:
J.C. Bartle
parent
17a4316600
commit
07c4a906d2
9 changed files with 684 additions and 79 deletions
|
|
@ -1838,6 +1838,28 @@ describe('MCP Routes', () => {
|
|||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Codex Finding 2 regression: the reinitialize route must forward `req`
|
||||
* to reinitMCPServer so that OBO servers can build a session-aware
|
||||
* upstream-token closure. Without this, OBO reinit fails with
|
||||
* `missing_upstream_token` even when the user has a valid session.
|
||||
*/
|
||||
it('should forward req into reinitMCPServer so OBO upstream-token closure has session access', async () => {
|
||||
const mockMcpManager = {
|
||||
disconnectUserConnection: jest.fn().mockResolvedValue(),
|
||||
};
|
||||
mockRegistryInstance.getServerConfig.mockResolvedValue({});
|
||||
require('~/config').getMCPManager.mockReturnValue(mockMcpManager);
|
||||
require('~/config').getFlowStateManager.mockReturnValue({});
|
||||
require('~/cache').getLogStores.mockReturnValue({});
|
||||
const reinitSpy = require('~/server/services/Tools/mcp').reinitMCPServer;
|
||||
reinitSpy.mockResolvedValue({ success: true, serverName: 'obo-server' });
|
||||
|
||||
await request(app).post('/api/mcp/obo-server/reinitialize');
|
||||
|
||||
expect(reinitSpy).toHaveBeenCalledWith(expect.objectContaining({ req: expect.any(Object) }));
|
||||
});
|
||||
|
||||
it('should return 500 when unexpected error occurs', async () => {
|
||||
const mockMcpManager = {
|
||||
disconnectUserConnection: jest.fn(),
|
||||
|
|
|
|||
|
|
@ -689,6 +689,7 @@ router.post(
|
|||
|
||||
const result = await reinitMCPServer({
|
||||
user,
|
||||
req,
|
||||
serverName,
|
||||
serverConfig,
|
||||
configServers,
|
||||
|
|
|
|||
|
|
@ -791,13 +791,27 @@ const setOpenIDAuthTokens = (
|
|||
|
||||
/** Store tokens server-side in session to avoid large cookies */
|
||||
if (req.session) {
|
||||
req.session.openidTokens = {
|
||||
const sessionOpenidTokens = {
|
||||
accessToken: tokenset.access_token,
|
||||
idToken: logoutIdToken,
|
||||
refreshToken: refreshToken,
|
||||
expiresAt: expirationDate.getTime(),
|
||||
lastRefreshedAt: Date.now(),
|
||||
};
|
||||
/**
|
||||
* Capture the access-token's own expiry (unix seconds) when the IdP
|
||||
* advertises one. Lets downstream consumers — notably the OBO inline-
|
||||
* refresh path in `OpenIDSessionRefresh.js` — reuse opaque (non-JWT)
|
||||
* access tokens without burning an IdP refresh on the first tool call.
|
||||
* Without this, the very first OBO call after login or SPA refresh would
|
||||
* always trigger a redundant inline refresh whenever the IdP issues
|
||||
* opaque access tokens (e.g. Microsoft Graph audiences).
|
||||
*/
|
||||
if (typeof tokenset.expires_in === 'number') {
|
||||
sessionOpenidTokens.accessTokenExpiresAt =
|
||||
Math.floor(Date.now() / 1000) + tokenset.expires_in;
|
||||
}
|
||||
req.session.openidTokens = sessionOpenidTokens;
|
||||
} else {
|
||||
logger.warn('[setOpenIDAuthTokens] No session available, falling back to cookies');
|
||||
res.cookie('openid_access_token', tokenset.access_token, {
|
||||
|
|
|
|||
|
|
@ -231,6 +231,48 @@ describe('setOpenIDAuthTokens', () => {
|
|||
expect(req.session.openidTokens.lastRefreshedAt).toEqual(expect.any(Number));
|
||||
});
|
||||
|
||||
/**
|
||||
* Codex Finding 5: persist the access-token's expiry (unix seconds) so the
|
||||
* first OBO call after login or SPA refresh can reuse a still-valid OPAQUE
|
||||
* access token without burning a redundant inline refresh. The expiry comes
|
||||
* from the IdP's `tokenset.expires_in`; downstream consumers (notably
|
||||
* `OpenIDSessionRefresh.getAccessTokenExp`) read it as a fallback when the
|
||||
* access token isn't a JWT and can't be decoded.
|
||||
*/
|
||||
it('should persist accessTokenExpiresAt when tokenset.expires_in is provided', () => {
|
||||
const tokenset = {
|
||||
id_token: 'the-id-token',
|
||||
access_token: 'the-access-token',
|
||||
refresh_token: 'the-refresh-token',
|
||||
expires_in: 3600,
|
||||
};
|
||||
const req = mockRequest();
|
||||
const res = mockResponse();
|
||||
const beforeSec = Math.floor(Date.now() / 1000);
|
||||
|
||||
setOpenIDAuthTokens(tokenset, req, res, 'user-123');
|
||||
|
||||
const persisted = req.session.openidTokens.accessTokenExpiresAt;
|
||||
expect(typeof persisted).toBe('number');
|
||||
expect(persisted).toBeGreaterThanOrEqual(beforeSec + 3590);
|
||||
expect(persisted).toBeLessThanOrEqual(beforeSec + 3610);
|
||||
});
|
||||
|
||||
it('should NOT persist accessTokenExpiresAt when tokenset.expires_in is missing', () => {
|
||||
const tokenset = {
|
||||
id_token: 'the-id-token',
|
||||
access_token: 'the-access-token',
|
||||
refresh_token: 'the-refresh-token',
|
||||
// expires_in deliberately omitted
|
||||
};
|
||||
const req = mockRequest();
|
||||
const res = mockResponse();
|
||||
|
||||
setOpenIDAuthTokens(tokenset, req, res, 'user-123');
|
||||
|
||||
expect(req.session.openidTokens).not.toHaveProperty('accessTokenExpiresAt');
|
||||
});
|
||||
|
||||
it('should return the existing unexpired session id_token when refresh omits one', () => {
|
||||
const existingIdToken = jwt.sign(
|
||||
{ sub: 'user-123', exp: Math.floor(Date.now() / 1000) + 3600 },
|
||||
|
|
|
|||
|
|
@ -835,10 +835,17 @@ function createToolInstance({
|
|||
* for `req.session.openidTokens` and only the request layer can refresh
|
||||
* and persist it via `req.session.save()`. No-op when reuse is off, the
|
||||
* user is non-OpenID, or the session lacks openidTokens.
|
||||
*
|
||||
* `tokenPreference: 'access_token'` is required for OBO: the OBO grant
|
||||
* sends the upstream access token to the IdP as the jwt-bearer assertion,
|
||||
* so freshness must gate on access_token.exp specifically. An expired
|
||||
* access_token under a still-fresh id_token would otherwise reach the
|
||||
* IdP and be rejected.
|
||||
*/
|
||||
const upstreamTokenProvider = createOpenIDSessionTokenProvider({
|
||||
req: capturedReq,
|
||||
user: effectiveUser,
|
||||
tokenPreference: 'access_token',
|
||||
});
|
||||
|
||||
const result = await mcpManager.callTool({
|
||||
|
|
|
|||
|
|
@ -1314,6 +1314,7 @@ describe('User parameter passing tests', () => {
|
|||
expect(mockCreateOpenIDSessionTokenProvider).toHaveBeenCalledWith({
|
||||
req: mockReq,
|
||||
user: mockUser,
|
||||
tokenPreference: 'access_token',
|
||||
});
|
||||
expect(mockCallTool).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
|
|
|
|||
|
|
@ -1,9 +1,34 @@
|
|||
const jwt = require('jsonwebtoken');
|
||||
const crypto = require('node:crypto');
|
||||
const openIdClient = require('openid-client');
|
||||
const { logger } = require('@librechat/data-schemas');
|
||||
const { isEnabled, buildOpenIDRefreshParams } = require('@librechat/api');
|
||||
const { getOpenIdConfig } = require('~/strategies/openidStrategy');
|
||||
|
||||
/**
|
||||
* Shape of `req.session.openidTokens`. Established by `setOpenIDAuthTokens`
|
||||
* (`api/server/services/AuthService.js`) on login/refresh, mutated in place by
|
||||
* this module on inline refresh, and consumed by `refreshController` and
|
||||
* `LogoutController`. Distinct from the snake_case `OIDCTokens` type in
|
||||
* `@librechat/data-schemas` (which describes `IUser.federatedTokens` /
|
||||
* `IUser.openidTokens` — model fields, not the express-session field).
|
||||
*
|
||||
* Express-session's SessionData is open by design, so this contract lives in
|
||||
* comments rather than a TS interface; keep this and AuthService.js in sync
|
||||
* when the shape changes.
|
||||
*
|
||||
* @typedef {Object} SessionOpenIDTokens
|
||||
* @property {string} [accessToken] — IdP access token (may be opaque).
|
||||
* @property {string} [idToken] — IdP ID token (always JWT).
|
||||
* @property {string} [refreshToken] — IdP refresh token.
|
||||
* @property {number} [expiresAt] — SESSION cookie expiry (ms).
|
||||
* @property {number} [lastRefreshedAt] — wall-clock ms of the last server-side rotation.
|
||||
* @property {number} [accessTokenExpiresAt] — access token expiry (unix seconds), captured
|
||||
* from the IdP `tokenset.expires_in` so opaque
|
||||
* access tokens can still be reused without
|
||||
* redundant refreshes.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Skew buffer for the upstream access-token expiry check. Mirrors
|
||||
* `OPENID_REUSE_EXPIRY_BUFFER_SECONDS` in `AuthController.js` so that a token
|
||||
|
|
@ -12,18 +37,68 @@ const { getOpenIdConfig } = require('~/strategies/openidStrategy');
|
|||
const UPSTREAM_TOKEN_EXPIRY_BUFFER_SECONDS = 30;
|
||||
|
||||
/**
|
||||
* In-flight upstream refreshes keyed by user id (openidId preferred, falling
|
||||
* back to local user._id). Mirrors the single-flight pattern in
|
||||
* `OboTokenService.js`. A fan-out of tool calls landing on an expired session
|
||||
* coalesces into one IdP refresh-token grant. Process-local: multi-worker
|
||||
* deployments may double-refresh on the very first concurrent miss across
|
||||
* workers — acceptable because the IdP accepts both and the session store
|
||||
* uses last-write-wins.
|
||||
* In-flight upstream refreshes keyed by `getSingleFlightKey(req, user)` —
|
||||
* a composite of `tenantId:openidIssuer:openidId:sessionId`. See that helper
|
||||
* for the rationale on why each component is needed; in short, per-session
|
||||
* keying prevents refresh-token rotation from breaking sibling sessions, and
|
||||
* tenant+issuer keying prevents cross-tenant token crossover when distinct
|
||||
* users share an IdP `sub`.
|
||||
*
|
||||
* A fan-out of tool calls landing on an expired session within the SAME
|
||||
* session coalesces into one IdP refresh-token grant. Mirrors the
|
||||
* single-flight pattern in `OboTokenService.js`.
|
||||
*
|
||||
* Process-local: multi-worker deployments may double-refresh on the very
|
||||
* first concurrent miss across workers — acceptable because the IdP accepts
|
||||
* both and the session store uses last-write-wins.
|
||||
*/
|
||||
const inFlightRefreshes = new Map();
|
||||
|
||||
function getOpenidUserKey(user) {
|
||||
return user?.openidId || user?.id || user?._id?.toString?.() || null;
|
||||
/**
|
||||
* Returns the single-flight key for a refresh attempt, composed from the
|
||||
* Express session id, the user's tenant (if any), and the IdP issuer + sub.
|
||||
* Tightening past `openidId` alone serves two purposes:
|
||||
*
|
||||
* 1. Same human, multiple sessions: refresh-token rotation by the IdP would
|
||||
* otherwise let session A's refresh invalidate session B's stored
|
||||
* refresh_token, leaving B silently broken. Per-session keying ensures
|
||||
* each session refreshes its own credentials.
|
||||
* 2. Multi-tenant deployments where two distinct users share an IdP `sub`
|
||||
* (different issuers, same sub): tenant + issuer disambiguates them so
|
||||
* tokens never cross tenant boundaries via shared in-flight Promises.
|
||||
*
|
||||
* Concurrent tool calls inside the SAME session still coalesce — the common
|
||||
* case the single-flight is designed for (a fan-out of MCP tool calls in one
|
||||
* agent run) is unaffected.
|
||||
*
|
||||
* Returns null when there's no usable identity at all; callers fall through
|
||||
* to a non-coalesced refresh, which is safe but missing the optimization.
|
||||
*/
|
||||
function getSingleFlightKey(req, user) {
|
||||
const sub = user?.openidId || user?.id || user?._id?.toString?.();
|
||||
if (!sub) {
|
||||
return null;
|
||||
}
|
||||
const sessionId = req?.sessionID || 'no-session';
|
||||
const tenantId = user?.tenantId || 'no-tenant';
|
||||
const issuer = user?.openidIssuer || 'no-issuer';
|
||||
return `${tenantId}:${issuer}:${sub}:${sessionId}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns a short SHA-256 prefix of the single-flight key for use in logs.
|
||||
* Preserves correlation across "started" / "joined" / "completed" log events
|
||||
* for the same refresh attempt without leaking the underlying values:
|
||||
*
|
||||
* - sessionId is effectively a credential (cookie material) and must never
|
||||
* reach log sinks in clear text.
|
||||
* - openidId (the IdP `sub`) and openidIssuer are tenant/user fingerprints.
|
||||
*
|
||||
* 12 hex chars = 48 bits of entropy: ~7×10^14 distinct keys before a 50%
|
||||
* collision chance — more than enough for correlating concurrent refreshes.
|
||||
*/
|
||||
function hashKeyForLogs(key) {
|
||||
return crypto.createHash('sha256').update(key).digest('hex').slice(0, 12);
|
||||
}
|
||||
|
||||
function decodeJwtExp(token) {
|
||||
|
|
@ -43,37 +118,93 @@ function decodeJwtExp(token) {
|
|||
}
|
||||
|
||||
/**
|
||||
* Returns true when the session's primary upstream token (id_token preferred,
|
||||
* access_token fallback) is still valid for at least `buffer` seconds. This
|
||||
* matches the candidate-selection used for refreshController reuse so the two
|
||||
* paths agree on what counts as "still valid".
|
||||
* Returns the access token's expiry in unix seconds, preferring the JWT `exp`
|
||||
* claim and falling back to the persisted `accessTokenExpiresAt` written from
|
||||
* the IdP's `tokenset.expires_in` on the previous refresh.
|
||||
*
|
||||
* The fallback exists because some IdPs (Microsoft Entra for Graph audiences,
|
||||
* Auth0 without a custom audience) issue OPAQUE access tokens whose expiry
|
||||
* cannot be decoded locally. Without this lookup, every OBO call would treat
|
||||
* the session as expired and burn an IdP refresh, risking refresh-token
|
||||
* rotation thrash under concurrent tool calls.
|
||||
*
|
||||
* @param {{ accessToken?: string, accessTokenExpiresAt?: number }} sessionTokens
|
||||
* @returns {number | null} unix seconds, or null when no source proves an expiry
|
||||
*/
|
||||
function isLiveSessionTokenStillValid(sessionTokens) {
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
const candidates = [sessionTokens?.idToken, sessionTokens?.accessToken];
|
||||
for (const token of candidates) {
|
||||
const exp = decodeJwtExp(token);
|
||||
if (exp != null && exp > now + UPSTREAM_TOKEN_EXPIRY_BUFFER_SECONDS) {
|
||||
return true;
|
||||
}
|
||||
function getAccessTokenExp(sessionTokens) {
|
||||
const fromJwt = decodeJwtExp(sessionTokens?.accessToken);
|
||||
if (fromJwt != null) {
|
||||
return fromJwt;
|
||||
}
|
||||
return false;
|
||||
const persisted = sessionTokens?.accessTokenExpiresAt;
|
||||
return typeof persisted === 'number' ? persisted : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Builds the OIDCTokens shape consumed by `resolveOboToken`. `expires_at` is
|
||||
* derived from the id_token JWT exp claim (or access_token as fallback) so it
|
||||
* matches what `extractOpenIDTokenInfo` would have produced from the user
|
||||
* snapshot — keeping `isOpenIDTokenValid`'s comparison meaningful.
|
||||
* Returns true when the session token nominated by `tokenPreference` is still
|
||||
* valid for at least the skew buffer. Required argument (no default) so every
|
||||
* caller is explicit about which token's freshness gates this check.
|
||||
*
|
||||
* Use 'access_token' for OBO and any flow whose downstream sends the access
|
||||
* token to the IdP as an assertion (jwt-bearer / on-behalf-of) — those flows
|
||||
* fail when the access token is expired even if the id_token is still fresh.
|
||||
* Access-token expiry is read via `getAccessTokenExp`, which handles opaque
|
||||
* (non-JWT) tokens by falling back to the persisted `accessTokenExpiresAt`.
|
||||
*
|
||||
* Use 'id_token' for flows whose downstream is the LibreChat backend itself
|
||||
* (e.g. session-token reuse in `refreshController`); the id_token is the
|
||||
* standard JWT signed for the client_id audience and is the bearer the SPA
|
||||
* sends back to LibreChat.
|
||||
*
|
||||
* @param {{ accessToken?: string, idToken?: string, accessTokenExpiresAt?: number }} sessionTokens
|
||||
* @param {'access_token' | 'id_token'} tokenPreference
|
||||
*/
|
||||
function buildOIDCTokensFromSession(sessionTokens) {
|
||||
const expFromJwt =
|
||||
decodeJwtExp(sessionTokens?.idToken) ?? decodeJwtExp(sessionTokens?.accessToken);
|
||||
function isLiveSessionTokenStillValid(sessionTokens, tokenPreference) {
|
||||
if (tokenPreference !== 'access_token' && tokenPreference !== 'id_token') {
|
||||
throw new Error(
|
||||
`[OpenIDSessionRefresh] tokenPreference must be 'access_token' or 'id_token', got: ${tokenPreference}`,
|
||||
);
|
||||
}
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
const exp =
|
||||
tokenPreference === 'access_token'
|
||||
? getAccessTokenExp(sessionTokens)
|
||||
: decodeJwtExp(sessionTokens?.idToken);
|
||||
return exp != null && exp > now + UPSTREAM_TOKEN_EXPIRY_BUFFER_SECONDS;
|
||||
}
|
||||
|
||||
/**
|
||||
* Builds the OIDCTokens shape consumed by `resolveOboToken`. Required
|
||||
* `tokenPreference` selects which token's expiry becomes `expires_at` —
|
||||
* caller intent must match what the downstream consumer actually validates.
|
||||
* `expiresAtOverride` (unix seconds) wins when the caller has an authoritative
|
||||
* value such as the IdP's `tokenset.expires_in` from a fresh refresh response;
|
||||
* use it after refresh so we never attribute a prior token's `exp` to a freshly
|
||||
* rotated counterpart. For 'access_token', the fallback uses `getAccessTokenExp`
|
||||
* so opaque tokens are handled correctly via the persisted `accessTokenExpiresAt`.
|
||||
*
|
||||
* @param {{ accessToken?: string, idToken?: string, refreshToken?: string, accessTokenExpiresAt?: number }} sessionTokens
|
||||
* @param {'access_token' | 'id_token'} tokenPreference
|
||||
* @param {number} [expiresAtOverride] — unix seconds (preferred when present)
|
||||
*/
|
||||
function buildOIDCTokensFromSession(sessionTokens, tokenPreference, expiresAtOverride) {
|
||||
if (tokenPreference !== 'access_token' && tokenPreference !== 'id_token') {
|
||||
throw new Error(
|
||||
`[OpenIDSessionRefresh] tokenPreference must be 'access_token' or 'id_token', got: ${tokenPreference}`,
|
||||
);
|
||||
}
|
||||
let expiresAt = expiresAtOverride;
|
||||
if (expiresAt == null) {
|
||||
expiresAt =
|
||||
tokenPreference === 'access_token'
|
||||
? (getAccessTokenExp(sessionTokens) ?? undefined)
|
||||
: (decodeJwtExp(sessionTokens?.idToken) ?? undefined);
|
||||
}
|
||||
return {
|
||||
access_token: sessionTokens?.accessToken,
|
||||
id_token: sessionTokens?.idToken,
|
||||
refresh_token: sessionTokens?.refreshToken,
|
||||
expires_at: expFromJwt ?? undefined,
|
||||
expires_at: expiresAt ?? undefined,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -92,7 +223,7 @@ async function persistSession(req) {
|
|||
});
|
||||
}
|
||||
|
||||
async function performIdpRefresh(req) {
|
||||
async function performIdpRefresh(req, tokenPreference) {
|
||||
const sessionTokens = req?.session?.openidTokens;
|
||||
const refreshToken = sessionTokens?.refreshToken;
|
||||
if (!refreshToken) {
|
||||
|
|
@ -119,6 +250,30 @@ async function performIdpRefresh(req) {
|
|||
const nextIdToken = tokenset.id_token || sessionTokens.idToken;
|
||||
const nextRefreshToken = tokenset.refresh_token || refreshToken;
|
||||
|
||||
/**
|
||||
* Capture the freshly-issued access-token's expiry (unix seconds) so the
|
||||
* next OBO call can reuse it without a redundant refresh — critical for
|
||||
* opaque (non-JWT) access tokens whose expiry isn't readable from the
|
||||
* token itself. Source order:
|
||||
* 1. tokenset.expires_in — IdP's authoritative value for the new access
|
||||
* token. Always preferred when present.
|
||||
* 2. decodeJwtExp(tokenset.access_token) — only when access_token is
|
||||
* itself a JWT. Decoding is a fact about THIS token, not a guess.
|
||||
*
|
||||
* Deliberately do NOT fall back to id_token's exp: id_token TTL is governed
|
||||
* by IdP session policy and is often longer than access-token TTL. Trusting
|
||||
* it would mark an opaque access token reusable past its real lifetime, so
|
||||
* a stale token would be sent to the OBO IdP and rejected. When neither
|
||||
* source proves an expiry, leave `accessTokenExpiresAt` unset; the next
|
||||
* freshness check will correctly fall through to refresh.
|
||||
*/
|
||||
let nextAccessTokenExp = null;
|
||||
if (typeof tokenset.expires_in === 'number') {
|
||||
nextAccessTokenExp = Math.floor(Date.now() / 1000) + tokenset.expires_in;
|
||||
} else {
|
||||
nextAccessTokenExp = decodeJwtExp(tokenset.access_token);
|
||||
}
|
||||
|
||||
const updatedSessionTokens = {
|
||||
...sessionTokens,
|
||||
accessToken: tokenset.access_token,
|
||||
|
|
@ -126,47 +281,68 @@ async function performIdpRefresh(req) {
|
|||
refreshToken: nextRefreshToken,
|
||||
lastRefreshedAt: Date.now(),
|
||||
};
|
||||
if (nextAccessTokenExp != null) {
|
||||
updatedSessionTokens.accessTokenExpiresAt = nextAccessTokenExp;
|
||||
} else {
|
||||
/** Drop a stale value rather than carry it across an unknown-expiry rotation. */
|
||||
delete updatedSessionTokens.accessTokenExpiresAt;
|
||||
}
|
||||
|
||||
req.session.openidTokens = updatedSessionTokens;
|
||||
await persistSession(req);
|
||||
|
||||
logger.info('[OpenIDSessionRefresh] Inline refresh succeeded');
|
||||
return buildOIDCTokensFromSession(updatedSessionTokens);
|
||||
/**
|
||||
* Pass the same expiry as the explicit `expiresAtOverride` so the returned
|
||||
* OIDCTokens carries it directly, regardless of token preference. After
|
||||
* refresh the IdP's value is authoritative and supersedes any decode.
|
||||
*/
|
||||
return buildOIDCTokensFromSession(
|
||||
updatedSessionTokens,
|
||||
tokenPreference,
|
||||
nextAccessTokenExp ?? undefined,
|
||||
);
|
||||
}
|
||||
|
||||
async function refreshOrReuseSession(req) {
|
||||
async function refreshOrReuseSession(req, tokenPreference) {
|
||||
const sessionTokens = req?.session?.openidTokens;
|
||||
if (!sessionTokens) {
|
||||
logger.debug('[OpenIDSessionRefresh] No session tokens to refresh from');
|
||||
return null;
|
||||
}
|
||||
|
||||
if (isLiveSessionTokenStillValid(sessionTokens)) {
|
||||
if (isLiveSessionTokenStillValid(sessionTokens, tokenPreference)) {
|
||||
logger.debug('[OpenIDSessionRefresh] Live session token reused');
|
||||
return buildOIDCTokensFromSession(sessionTokens);
|
||||
return buildOIDCTokensFromSession(sessionTokens, tokenPreference);
|
||||
}
|
||||
|
||||
return performIdpRefresh(req);
|
||||
return performIdpRefresh(req, tokenPreference);
|
||||
}
|
||||
|
||||
/**
|
||||
* Single-flighted entry point. Concurrent callers for the same user share one
|
||||
* in-flight refresh. The map is cleared in finally so a failed refresh does
|
||||
* not pin subsequent retries.
|
||||
*
|
||||
* @param {import('express').Request} req
|
||||
* @param {import('@librechat/data-schemas').IUser} user
|
||||
* @param {'access_token' | 'id_token'} tokenPreference — required; selects
|
||||
* which token's `exp` gates the live-vs-refresh decision and populates the
|
||||
* returned `expires_at`. OBO callers pass 'access_token'.
|
||||
*/
|
||||
async function refreshOpenIDSession(req, user) {
|
||||
const key = getOpenidUserKey(user);
|
||||
async function refreshOpenIDSession(req, user, tokenPreference) {
|
||||
const key = getSingleFlightKey(req, user);
|
||||
if (!key) {
|
||||
return refreshOrReuseSession(req);
|
||||
return refreshOrReuseSession(req, tokenPreference);
|
||||
}
|
||||
|
||||
const inFlight = inFlightRefreshes.get(key);
|
||||
if (inFlight) {
|
||||
logger.debug(`[OpenIDSessionRefresh] Joining in-flight refresh for user: ${key}`);
|
||||
logger.debug(`[OpenIDSessionRefresh] Joining in-flight refresh (key=${hashKeyForLogs(key)})`);
|
||||
return inFlight;
|
||||
}
|
||||
|
||||
const promise = refreshOrReuseSession(req).finally(() => {
|
||||
const promise = refreshOrReuseSession(req, tokenPreference).finally(() => {
|
||||
if (inFlightRefreshes.get(key) === promise) {
|
||||
inFlightRefreshes.delete(key);
|
||||
}
|
||||
|
|
@ -198,6 +374,11 @@ function isOIDCRefreshApplicable(user) {
|
|||
* call time (not at request validation), which is what makes the walk-away
|
||||
* failure mode recover without a user-visible re-authentication.
|
||||
*
|
||||
* `tokenPreference` is required and identifies which upstream token's freshness
|
||||
* gates the closure. OBO needs 'access_token' because the OBO exchange uses
|
||||
* the access token as the jwt-bearer assertion; using id_token preference here
|
||||
* would let an expired access token reach the IdP under a still-fresh id_token.
|
||||
*
|
||||
* Closure contract (matches `UpstreamTokenProvider` in obo.ts):
|
||||
* - resolves to non-null OIDCTokens when fresh tokens are available.
|
||||
* - resolves to null when refresh is not applicable / no session.
|
||||
|
|
@ -207,9 +388,15 @@ function isOIDCRefreshApplicable(user) {
|
|||
* @param {object} args
|
||||
* @param {import('express').Request} [args.req]
|
||||
* @param {import('@librechat/data-schemas').IUser} [args.user]
|
||||
* @param {'access_token' | 'id_token'} args.tokenPreference
|
||||
* @returns {() => Promise<import('@librechat/data-schemas').OIDCTokens | null>}
|
||||
*/
|
||||
function createOpenIDSessionTokenProvider({ req, user }) {
|
||||
function createOpenIDSessionTokenProvider({ req, user, tokenPreference }) {
|
||||
if (tokenPreference !== 'access_token' && tokenPreference !== 'id_token') {
|
||||
throw new Error(
|
||||
`[OpenIDSessionRefresh] createOpenIDSessionTokenProvider requires tokenPreference 'access_token' or 'id_token', got: ${tokenPreference}`,
|
||||
);
|
||||
}
|
||||
return async function upstreamTokenProvider() {
|
||||
if (!isOIDCRefreshApplicable(user)) {
|
||||
return null;
|
||||
|
|
@ -220,7 +407,7 @@ function createOpenIDSessionTokenProvider({ req, user }) {
|
|||
);
|
||||
return null;
|
||||
}
|
||||
return refreshOpenIDSession(req, user);
|
||||
return refreshOpenIDSession(req, user, tokenPreference);
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -232,5 +419,6 @@ module.exports = {
|
|||
UPSTREAM_TOKEN_EXPIRY_BUFFER_SECONDS,
|
||||
inFlightRefreshes,
|
||||
isLiveSessionTokenStillValid,
|
||||
getAccessTokenExp,
|
||||
},
|
||||
};
|
||||
|
|
|
|||
|
|
@ -31,7 +31,8 @@ const SECRET = 'test-secret';
|
|||
|
||||
const makeJwt = (exp) => jwt.sign({ sub: 'user-123', exp }, SECRET);
|
||||
|
||||
const buildReq = (sessionTokens) => ({
|
||||
const buildReq = (sessionTokens, sessionId = 'session-A') => ({
|
||||
sessionID: sessionId,
|
||||
session: Object.assign(
|
||||
{
|
||||
save: jest.fn((cb) => cb(null)),
|
||||
|
|
@ -40,10 +41,11 @@ const buildReq = (sessionTokens) => ({
|
|||
),
|
||||
});
|
||||
|
||||
const makeOpenIdUser = () => ({
|
||||
const makeOpenIdUser = (overrides = {}) => ({
|
||||
id: 'local-id-1',
|
||||
openidId: 'oidc-sub-123',
|
||||
provider: 'openid',
|
||||
...overrides,
|
||||
});
|
||||
|
||||
describe('OpenIDSessionRefresh', () => {
|
||||
|
|
@ -56,11 +58,31 @@ describe('OpenIDSessionRefresh', () => {
|
|||
});
|
||||
|
||||
describe('createOpenIDSessionTokenProvider closure no-op cases', () => {
|
||||
it('throws when tokenPreference is missing', () => {
|
||||
expect(() =>
|
||||
createOpenIDSessionTokenProvider({
|
||||
req: buildReq({ accessToken: makeJwt(Date.now() / 1000 + 600) }),
|
||||
user: makeOpenIdUser(),
|
||||
}),
|
||||
).toThrow(/tokenPreference/);
|
||||
});
|
||||
|
||||
it('throws when tokenPreference is invalid', () => {
|
||||
expect(() =>
|
||||
createOpenIDSessionTokenProvider({
|
||||
req: buildReq({ accessToken: makeJwt(Date.now() / 1000 + 600) }),
|
||||
user: makeOpenIdUser(),
|
||||
tokenPreference: 'bogus',
|
||||
}),
|
||||
).toThrow(/tokenPreference/);
|
||||
});
|
||||
|
||||
it('returns null when OPENID_REUSE_TOKENS is disabled', async () => {
|
||||
isEnabled.mockReturnValue(false);
|
||||
const provider = createOpenIDSessionTokenProvider({
|
||||
req: buildReq({ accessToken: makeJwt(Date.now() / 1000 + 600) }),
|
||||
user: makeOpenIdUser(),
|
||||
tokenPreference: 'access_token',
|
||||
});
|
||||
await expect(provider()).resolves.toBeNull();
|
||||
expect(openIdClient.refreshTokenGrant).not.toHaveBeenCalled();
|
||||
|
|
@ -70,6 +92,7 @@ describe('OpenIDSessionRefresh', () => {
|
|||
const provider = createOpenIDSessionTokenProvider({
|
||||
req: buildReq({ accessToken: makeJwt(Date.now() / 1000 + 600) }),
|
||||
user: { id: 'local-1', provider: 'local' },
|
||||
tokenPreference: 'access_token',
|
||||
});
|
||||
await expect(provider()).resolves.toBeNull();
|
||||
expect(openIdClient.refreshTokenGrant).not.toHaveBeenCalled();
|
||||
|
|
@ -79,6 +102,7 @@ describe('OpenIDSessionRefresh', () => {
|
|||
const provider = createOpenIDSessionTokenProvider({
|
||||
req: buildReq({ accessToken: makeJwt(Date.now() / 1000 + 600) }),
|
||||
user: undefined,
|
||||
tokenPreference: 'access_token',
|
||||
});
|
||||
await expect(provider()).resolves.toBeNull();
|
||||
});
|
||||
|
|
@ -87,6 +111,7 @@ describe('OpenIDSessionRefresh', () => {
|
|||
const provider = createOpenIDSessionTokenProvider({
|
||||
req: buildReq(undefined),
|
||||
user: makeOpenIdUser(),
|
||||
tokenPreference: 'access_token',
|
||||
});
|
||||
await expect(provider()).resolves.toBeNull();
|
||||
expect(openIdClient.refreshTokenGrant).not.toHaveBeenCalled();
|
||||
|
|
@ -96,33 +121,62 @@ describe('OpenIDSessionRefresh', () => {
|
|||
const provider = createOpenIDSessionTokenProvider({
|
||||
req: undefined,
|
||||
user: makeOpenIdUser(),
|
||||
tokenPreference: 'access_token',
|
||||
});
|
||||
await expect(provider()).resolves.toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('refreshOpenIDSession live-token reuse', () => {
|
||||
it('returns live tokens without calling IdP when id_token still valid past skew', async () => {
|
||||
it('returns live tokens without calling IdP when access_token still valid past skew', async () => {
|
||||
const farFutureExp = Math.floor(Date.now() / 1000) + 600;
|
||||
const sessionTokens = {
|
||||
accessToken: 'opaque-access-token',
|
||||
accessToken: makeJwt(farFutureExp),
|
||||
idToken: makeJwt(farFutureExp),
|
||||
refreshToken: 'rt-1',
|
||||
};
|
||||
const req = buildReq(sessionTokens);
|
||||
|
||||
const result = await refreshOpenIDSession(req, makeOpenIdUser());
|
||||
const result = await refreshOpenIDSession(req, makeOpenIdUser(), 'access_token');
|
||||
|
||||
expect(openIdClient.refreshTokenGrant).not.toHaveBeenCalled();
|
||||
expect(result).toEqual({
|
||||
access_token: 'opaque-access-token',
|
||||
access_token: sessionTokens.accessToken,
|
||||
id_token: sessionTokens.idToken,
|
||||
refresh_token: 'rt-1',
|
||||
expires_at: farFutureExp,
|
||||
});
|
||||
});
|
||||
|
||||
it('falls through to refresh when id_token expires within the skew buffer', async () => {
|
||||
/**
|
||||
* The bug fixed by Codex Finding 1a: id_token can outlive access_token.
|
||||
* Old behavior would declare "live" because id_token is fresh, sending an
|
||||
* expired access_token to the OBO IdP. New behavior must trigger a refresh.
|
||||
*/
|
||||
it('refreshes when access_token is expired even if id_token is still fresh', async () => {
|
||||
const accessExp = Math.floor(Date.now() / 1000) - 30;
|
||||
const idExp = Math.floor(Date.now() / 1000) + 3600;
|
||||
const sessionTokens = {
|
||||
accessToken: makeJwt(accessExp),
|
||||
idToken: makeJwt(idExp),
|
||||
refreshToken: 'rt-asym',
|
||||
};
|
||||
const refreshedExp = Math.floor(Date.now() / 1000) + 3600;
|
||||
openIdClient.refreshTokenGrant.mockResolvedValueOnce({
|
||||
access_token: makeJwt(refreshedExp),
|
||||
id_token: makeJwt(refreshedExp),
|
||||
refresh_token: 'rt-asym-2',
|
||||
expires_in: 3600,
|
||||
});
|
||||
const req = buildReq(sessionTokens);
|
||||
|
||||
const result = await refreshOpenIDSession(req, makeOpenIdUser(), 'access_token');
|
||||
|
||||
expect(openIdClient.refreshTokenGrant).toHaveBeenCalledTimes(1);
|
||||
expect(result.access_token).not.toBe(sessionTokens.accessToken);
|
||||
});
|
||||
|
||||
it('falls through to refresh when access_token expires within the skew buffer', async () => {
|
||||
const veryNearExp = Math.floor(Date.now() / 1000) + 10; // < 30s buffer
|
||||
const sessionTokens = {
|
||||
accessToken: makeJwt(veryNearExp),
|
||||
|
|
@ -131,27 +185,25 @@ describe('OpenIDSessionRefresh', () => {
|
|||
};
|
||||
const refreshedExp = Math.floor(Date.now() / 1000) + 3600;
|
||||
openIdClient.refreshTokenGrant.mockResolvedValueOnce({
|
||||
access_token: 'new-access',
|
||||
access_token: makeJwt(refreshedExp),
|
||||
id_token: makeJwt(refreshedExp),
|
||||
refresh_token: 'rt-3',
|
||||
expires_in: 3600,
|
||||
});
|
||||
const req = buildReq(sessionTokens);
|
||||
|
||||
const result = await refreshOpenIDSession(req, makeOpenIdUser());
|
||||
const result = await refreshOpenIDSession(req, makeOpenIdUser(), 'access_token');
|
||||
|
||||
expect(openIdClient.refreshTokenGrant).toHaveBeenCalledTimes(1);
|
||||
expect(buildOpenIDRefreshParams).toHaveBeenCalled();
|
||||
expect(result.access_token).toBe('new-access');
|
||||
expect(result.refresh_token).toBe('rt-3');
|
||||
expect(req.session.openidTokens.refreshToken).toBe('rt-3');
|
||||
expect(req.session.openidTokens.accessToken).toBe('new-access');
|
||||
expect(req.session.save).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('refreshOpenIDSession refresh path', () => {
|
||||
it('refreshes when id_token is expired and persists session', async () => {
|
||||
it('refreshes when access_token is expired and persists session', async () => {
|
||||
const expiredExp = Math.floor(Date.now() / 1000) - 60;
|
||||
const sessionTokens = {
|
||||
accessToken: makeJwt(expiredExp),
|
||||
|
|
@ -160,21 +212,20 @@ describe('OpenIDSessionRefresh', () => {
|
|||
};
|
||||
const refreshedExp = Math.floor(Date.now() / 1000) + 3600;
|
||||
openIdClient.refreshTokenGrant.mockResolvedValueOnce({
|
||||
access_token: 'new-access',
|
||||
access_token: makeJwt(refreshedExp),
|
||||
id_token: makeJwt(refreshedExp),
|
||||
refresh_token: 'rt-new',
|
||||
expires_in: 3600,
|
||||
});
|
||||
const req = buildReq(sessionTokens);
|
||||
|
||||
const result = await refreshOpenIDSession(req, makeOpenIdUser());
|
||||
const result = await refreshOpenIDSession(req, makeOpenIdUser(), 'access_token');
|
||||
|
||||
expect(openIdClient.refreshTokenGrant).toHaveBeenCalledTimes(1);
|
||||
expect(req.session.save).toHaveBeenCalledTimes(1);
|
||||
expect(result.access_token).toBe('new-access');
|
||||
expect(typeof result.access_token).toBe('string');
|
||||
expect(req.session.openidTokens).toEqual(
|
||||
expect.objectContaining({
|
||||
accessToken: 'new-access',
|
||||
refreshToken: 'rt-new',
|
||||
lastRefreshedAt: expect.any(Number),
|
||||
}),
|
||||
|
|
@ -189,14 +240,15 @@ describe('OpenIDSessionRefresh', () => {
|
|||
idToken: priorIdToken,
|
||||
refreshToken: 'rt-keep',
|
||||
};
|
||||
const refreshedExp = Math.floor(Date.now() / 1000) + 3600;
|
||||
openIdClient.refreshTokenGrant.mockResolvedValueOnce({
|
||||
access_token: 'new-access',
|
||||
access_token: makeJwt(refreshedExp),
|
||||
// id_token and refresh_token both omitted
|
||||
expires_in: 3600,
|
||||
});
|
||||
const req = buildReq(sessionTokens);
|
||||
|
||||
const result = await refreshOpenIDSession(req, makeOpenIdUser());
|
||||
const result = await refreshOpenIDSession(req, makeOpenIdUser(), 'access_token');
|
||||
|
||||
expect(result.id_token).toBe(priorIdToken);
|
||||
expect(result.refresh_token).toBe('rt-keep');
|
||||
|
|
@ -204,6 +256,35 @@ describe('OpenIDSessionRefresh', () => {
|
|||
expect(req.session.openidTokens.refreshToken).toBe('rt-keep');
|
||||
});
|
||||
|
||||
/**
|
||||
* The bug fixed by Codex Finding 1b: when IdP rotates only access_token,
|
||||
* derive expires_at from the IdP's tokenset.expires_in (authoritative for
|
||||
* the new access_token) rather than the prior id_token's exp claim. The
|
||||
* latter would cause `isOpenIDTokenValid` to reject a fresh credential.
|
||||
*/
|
||||
it('uses tokenset.expires_in (not prior id_token exp) for expires_at after rotation-omits-id-token refresh', async () => {
|
||||
const expiredExp = Math.floor(Date.now() / 1000) - 60;
|
||||
const sessionTokens = {
|
||||
accessToken: makeJwt(expiredExp),
|
||||
idToken: makeJwt(expiredExp),
|
||||
refreshToken: 'rt-rot',
|
||||
};
|
||||
// IdP omits id_token; expires_in is the only authoritative expiry source
|
||||
openIdClient.refreshTokenGrant.mockResolvedValueOnce({
|
||||
access_token: makeJwt(Math.floor(Date.now() / 1000) + 3600),
|
||||
// id_token omitted
|
||||
expires_in: 3600,
|
||||
});
|
||||
const req = buildReq(sessionTokens);
|
||||
const beforeSec = Math.floor(Date.now() / 1000);
|
||||
|
||||
const result = await refreshOpenIDSession(req, makeOpenIdUser(), 'access_token');
|
||||
|
||||
// expires_at should be ~now + 3600, NOT the stale prior id_token exp
|
||||
expect(result.expires_at).toBeGreaterThanOrEqual(beforeSec + 3590);
|
||||
expect(result.expires_at).toBeLessThanOrEqual(beforeSec + 3610);
|
||||
});
|
||||
|
||||
it('returns null when session lacks a refresh_token (cannot refresh)', async () => {
|
||||
const expiredExp = Math.floor(Date.now() / 1000) - 60;
|
||||
const sessionTokens = {
|
||||
|
|
@ -213,7 +294,7 @@ describe('OpenIDSessionRefresh', () => {
|
|||
};
|
||||
const req = buildReq(sessionTokens);
|
||||
|
||||
const result = await refreshOpenIDSession(req, makeOpenIdUser());
|
||||
const result = await refreshOpenIDSession(req, makeOpenIdUser(), 'access_token');
|
||||
|
||||
expect(openIdClient.refreshTokenGrant).not.toHaveBeenCalled();
|
||||
expect(result).toBeNull();
|
||||
|
|
@ -229,7 +310,9 @@ describe('OpenIDSessionRefresh', () => {
|
|||
openIdClient.refreshTokenGrant.mockRejectedValueOnce(new Error('invalid_grant'));
|
||||
const req = buildReq(sessionTokens);
|
||||
|
||||
await expect(refreshOpenIDSession(req, makeOpenIdUser())).rejects.toThrow('invalid_grant');
|
||||
await expect(refreshOpenIDSession(req, makeOpenIdUser(), 'access_token')).rejects.toThrow(
|
||||
'invalid_grant',
|
||||
);
|
||||
expect(req.session.save).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
|
@ -246,12 +329,14 @@ describe('OpenIDSessionRefresh', () => {
|
|||
});
|
||||
const req = buildReq(sessionTokens);
|
||||
|
||||
await expect(refreshOpenIDSession(req, makeOpenIdUser())).rejects.toThrow(/no access_token/i);
|
||||
await expect(refreshOpenIDSession(req, makeOpenIdUser(), 'access_token')).rejects.toThrow(
|
||||
/no access_token/i,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('single-flight coalescing', () => {
|
||||
it('shares one refreshTokenGrant call across concurrent waiters with same userId', async () => {
|
||||
it('shares one refreshTokenGrant call across concurrent waiters in the SAME session', async () => {
|
||||
const expiredExp = Math.floor(Date.now() / 1000) - 60;
|
||||
const sessionTokens = {
|
||||
accessToken: makeJwt(expiredExp),
|
||||
|
|
@ -264,18 +349,18 @@ describe('OpenIDSessionRefresh', () => {
|
|||
});
|
||||
openIdClient.refreshTokenGrant.mockReturnValueOnce(grantPromise);
|
||||
|
||||
const req = buildReq(sessionTokens);
|
||||
const req = buildReq(sessionTokens, 'session-shared');
|
||||
const user = makeOpenIdUser();
|
||||
|
||||
const p1 = refreshOpenIDSession(req, user);
|
||||
const p2 = refreshOpenIDSession(req, user);
|
||||
const p1 = refreshOpenIDSession(req, user, 'access_token');
|
||||
const p2 = refreshOpenIDSession(req, user, 'access_token');
|
||||
|
||||
// Both calls land before the IdP responds
|
||||
expect(openIdClient.refreshTokenGrant).toHaveBeenCalledTimes(1);
|
||||
|
||||
const refreshedExp = Math.floor(Date.now() / 1000) + 3600;
|
||||
resolveGrant({
|
||||
access_token: 'new-access',
|
||||
access_token: makeJwt(refreshedExp),
|
||||
id_token: makeJwt(refreshedExp),
|
||||
refresh_token: 'rt-rotated',
|
||||
expires_in: 3600,
|
||||
|
|
@ -283,10 +368,69 @@ describe('OpenIDSessionRefresh', () => {
|
|||
|
||||
const [r1, r2] = await Promise.all([p1, p2]);
|
||||
expect(r1).toBe(r2);
|
||||
expect(r1.access_token).toBe('new-access');
|
||||
expect(__internals.inFlightRefreshes.size).toBe(0);
|
||||
});
|
||||
|
||||
/**
|
||||
* Codex Finding 3: distinct sessions for the same OpenID subject must NOT
|
||||
* share an in-flight refresh, otherwise refresh-token rotation breaks the
|
||||
* non-winning session silently. Per-sessionID keying isolates them.
|
||||
*/
|
||||
it('does NOT share an in-flight refresh across DIFFERENT sessions for the same user', async () => {
|
||||
const expiredExp = Math.floor(Date.now() / 1000) - 60;
|
||||
const reqA = buildReq(
|
||||
{
|
||||
accessToken: makeJwt(expiredExp),
|
||||
idToken: makeJwt(expiredExp),
|
||||
refreshToken: 'rt-A',
|
||||
},
|
||||
'session-A',
|
||||
);
|
||||
const reqB = buildReq(
|
||||
{
|
||||
accessToken: makeJwt(expiredExp),
|
||||
idToken: makeJwt(expiredExp),
|
||||
refreshToken: 'rt-B',
|
||||
},
|
||||
'session-B',
|
||||
);
|
||||
let resolveA;
|
||||
let resolveB;
|
||||
const promiseA = new Promise((resolve) => {
|
||||
resolveA = resolve;
|
||||
});
|
||||
const promiseB = new Promise((resolve) => {
|
||||
resolveB = resolve;
|
||||
});
|
||||
openIdClient.refreshTokenGrant.mockReturnValueOnce(promiseA).mockReturnValueOnce(promiseB);
|
||||
|
||||
const user = makeOpenIdUser();
|
||||
const pA = refreshOpenIDSession(reqA, user, 'access_token');
|
||||
const pB = refreshOpenIDSession(reqB, user, 'access_token');
|
||||
|
||||
// Two refreshes started, one per session
|
||||
expect(openIdClient.refreshTokenGrant).toHaveBeenCalledTimes(2);
|
||||
|
||||
const refreshedExp = Math.floor(Date.now() / 1000) + 3600;
|
||||
resolveA({
|
||||
access_token: makeJwt(refreshedExp),
|
||||
id_token: makeJwt(refreshedExp),
|
||||
refresh_token: 'rt-A-rotated',
|
||||
expires_in: 3600,
|
||||
});
|
||||
resolveB({
|
||||
access_token: makeJwt(refreshedExp),
|
||||
id_token: makeJwt(refreshedExp),
|
||||
refresh_token: 'rt-B-rotated',
|
||||
expires_in: 3600,
|
||||
});
|
||||
|
||||
const [rA, rB] = await Promise.all([pA, pB]);
|
||||
expect(rA).not.toBe(rB);
|
||||
expect(reqA.session.openidTokens.refreshToken).toBe('rt-A-rotated');
|
||||
expect(reqB.session.openidTokens.refreshToken).toBe('rt-B-rotated');
|
||||
});
|
||||
|
||||
it('clears in-flight slot on rejection so subsequent attempts can retry', async () => {
|
||||
const expiredExp = Math.floor(Date.now() / 1000) - 60;
|
||||
const sessionTokens = {
|
||||
|
|
@ -298,19 +442,19 @@ describe('OpenIDSessionRefresh', () => {
|
|||
const req = buildReq(sessionTokens);
|
||||
const user = makeOpenIdUser();
|
||||
|
||||
await expect(refreshOpenIDSession(req, user)).rejects.toThrow('transient');
|
||||
await expect(refreshOpenIDSession(req, user, 'access_token')).rejects.toThrow('transient');
|
||||
expect(__internals.inFlightRefreshes.size).toBe(0);
|
||||
|
||||
// Second attempt: succeed
|
||||
const refreshedExp = Math.floor(Date.now() / 1000) + 3600;
|
||||
openIdClient.refreshTokenGrant.mockResolvedValueOnce({
|
||||
access_token: 'new-access',
|
||||
access_token: makeJwt(refreshedExp),
|
||||
id_token: makeJwt(refreshedExp),
|
||||
refresh_token: 'rt-recovered',
|
||||
expires_in: 3600,
|
||||
});
|
||||
const result = await refreshOpenIDSession(req, user);
|
||||
expect(result.access_token).toBe('new-access');
|
||||
const result = await refreshOpenIDSession(req, user, 'access_token');
|
||||
expect(result.refresh_token).toBe('rt-recovered');
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -318,18 +462,19 @@ describe('OpenIDSessionRefresh', () => {
|
|||
it('returns the live OIDCTokens shape from a valid session', async () => {
|
||||
const farFutureExp = Math.floor(Date.now() / 1000) + 600;
|
||||
const sessionTokens = {
|
||||
accessToken: 'access-1',
|
||||
accessToken: makeJwt(farFutureExp),
|
||||
idToken: makeJwt(farFutureExp),
|
||||
refreshToken: 'rt-1',
|
||||
};
|
||||
const provider = createOpenIDSessionTokenProvider({
|
||||
req: buildReq(sessionTokens),
|
||||
user: makeOpenIdUser(),
|
||||
tokenPreference: 'access_token',
|
||||
});
|
||||
|
||||
const result = await provider();
|
||||
expect(result).toEqual({
|
||||
access_token: 'access-1',
|
||||
access_token: sessionTokens.accessToken,
|
||||
id_token: sessionTokens.idToken,
|
||||
refresh_token: 'rt-1',
|
||||
expires_at: farFutureExp,
|
||||
|
|
@ -347,9 +492,186 @@ describe('OpenIDSessionRefresh', () => {
|
|||
const provider = createOpenIDSessionTokenProvider({
|
||||
req: buildReq(sessionTokens),
|
||||
user: makeOpenIdUser(),
|
||||
tokenPreference: 'access_token',
|
||||
});
|
||||
|
||||
await expect(provider()).rejects.toThrow('invalid_grant');
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Codex Finding 4: opaque (non-JWT) access tokens make `decodeJwtExp` return
|
||||
* null, which would force every OBO call to refresh even when the previous
|
||||
* refresh response advertised a still-valid `expires_in`. The fix persists
|
||||
* `accessTokenExpiresAt` (unix seconds) on each refresh and uses it as a
|
||||
* fallback for the freshness check + `expires_at` derivation.
|
||||
*/
|
||||
describe('opaque access token support (accessTokenExpiresAt fallback)', () => {
|
||||
it('reuses live opaque access_token when accessTokenExpiresAt is in the future', async () => {
|
||||
const farFutureExp = Math.floor(Date.now() / 1000) + 600;
|
||||
const sessionTokens = {
|
||||
accessToken: 'opaque-blob-not-a-jwt',
|
||||
idToken: makeJwt(farFutureExp),
|
||||
refreshToken: 'rt-opaque',
|
||||
accessTokenExpiresAt: farFutureExp,
|
||||
};
|
||||
const req = buildReq(sessionTokens);
|
||||
|
||||
const result = await refreshOpenIDSession(req, makeOpenIdUser(), 'access_token');
|
||||
|
||||
expect(openIdClient.refreshTokenGrant).not.toHaveBeenCalled();
|
||||
expect(result).toEqual({
|
||||
access_token: 'opaque-blob-not-a-jwt',
|
||||
id_token: sessionTokens.idToken,
|
||||
refresh_token: 'rt-opaque',
|
||||
expires_at: farFutureExp,
|
||||
});
|
||||
});
|
||||
|
||||
it('refreshes opaque access_token when accessTokenExpiresAt has passed', async () => {
|
||||
const expiredExp = Math.floor(Date.now() / 1000) - 60;
|
||||
const refreshedExp = Math.floor(Date.now() / 1000) + 3600;
|
||||
const sessionTokens = {
|
||||
accessToken: 'opaque-stale',
|
||||
idToken: makeJwt(refreshedExp), // id_token still valid
|
||||
refreshToken: 'rt-opaque-stale',
|
||||
accessTokenExpiresAt: expiredExp,
|
||||
};
|
||||
openIdClient.refreshTokenGrant.mockResolvedValueOnce({
|
||||
access_token: 'opaque-fresh',
|
||||
// IdP omits id_token (Auth0 rotation off / MS personal); we use expires_in
|
||||
expires_in: 3600,
|
||||
});
|
||||
const req = buildReq(sessionTokens);
|
||||
|
||||
const result = await refreshOpenIDSession(req, makeOpenIdUser(), 'access_token');
|
||||
|
||||
expect(openIdClient.refreshTokenGrant).toHaveBeenCalledTimes(1);
|
||||
expect(result.access_token).toBe('opaque-fresh');
|
||||
});
|
||||
|
||||
it('refreshes opaque access_token when no JWT exp and no accessTokenExpiresAt are present', async () => {
|
||||
const refreshedExp = Math.floor(Date.now() / 1000) + 3600;
|
||||
const sessionTokens = {
|
||||
accessToken: 'opaque-no-expiry',
|
||||
idToken: makeJwt(refreshedExp),
|
||||
refreshToken: 'rt-no-exp',
|
||||
// accessTokenExpiresAt deliberately omitted
|
||||
};
|
||||
openIdClient.refreshTokenGrant.mockResolvedValueOnce({
|
||||
access_token: 'opaque-fresh',
|
||||
expires_in: 3600,
|
||||
});
|
||||
const req = buildReq(sessionTokens);
|
||||
|
||||
await refreshOpenIDSession(req, makeOpenIdUser(), 'access_token');
|
||||
|
||||
expect(openIdClient.refreshTokenGrant).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('persists accessTokenExpiresAt to req.session.openidTokens after a refresh with expires_in', async () => {
|
||||
const expiredExp = Math.floor(Date.now() / 1000) - 60;
|
||||
const sessionTokens = {
|
||||
accessToken: 'opaque-stale',
|
||||
idToken: makeJwt(expiredExp),
|
||||
refreshToken: 'rt-persist',
|
||||
};
|
||||
openIdClient.refreshTokenGrant.mockResolvedValueOnce({
|
||||
access_token: 'opaque-fresh',
|
||||
expires_in: 3600,
|
||||
});
|
||||
const req = buildReq(sessionTokens);
|
||||
const beforeSec = Math.floor(Date.now() / 1000);
|
||||
|
||||
await refreshOpenIDSession(req, makeOpenIdUser(), 'access_token');
|
||||
|
||||
const persistedExp = req.session.openidTokens.accessTokenExpiresAt;
|
||||
expect(typeof persistedExp).toBe('number');
|
||||
expect(persistedExp).toBeGreaterThanOrEqual(beforeSec + 3590);
|
||||
expect(persistedExp).toBeLessThanOrEqual(beforeSec + 3610);
|
||||
});
|
||||
|
||||
it('drops a stale accessTokenExpiresAt when the new tokenset has neither expires_in nor a JWT access_token', async () => {
|
||||
const expiredExp = Math.floor(Date.now() / 1000) - 60;
|
||||
const sessionTokens = {
|
||||
accessToken: 'opaque-old',
|
||||
idToken: makeJwt(expiredExp),
|
||||
refreshToken: 'rt-drop',
|
||||
accessTokenExpiresAt: expiredExp, // stale carry-over
|
||||
};
|
||||
openIdClient.refreshTokenGrant.mockResolvedValueOnce({
|
||||
access_token: 'opaque-fresh-no-meta',
|
||||
// no expires_in, no JWT access_token, no id_token
|
||||
});
|
||||
const req = buildReq(sessionTokens);
|
||||
|
||||
await refreshOpenIDSession(req, makeOpenIdUser(), 'access_token');
|
||||
|
||||
expect(req.session.openidTokens).not.toHaveProperty('accessTokenExpiresAt');
|
||||
});
|
||||
|
||||
it('getAccessTokenExp prefers JWT exp over the persisted accessTokenExpiresAt', () => {
|
||||
const jwtExp = Math.floor(Date.now() / 1000) + 600;
|
||||
const persistedExp = Math.floor(Date.now() / 1000) - 60; // stale
|
||||
const result = __internals.getAccessTokenExp({
|
||||
accessToken: makeJwt(jwtExp),
|
||||
accessTokenExpiresAt: persistedExp,
|
||||
});
|
||||
expect(result).toBe(jwtExp);
|
||||
});
|
||||
|
||||
it('getAccessTokenExp returns null when neither a decodable JWT nor a persisted expiry is present', () => {
|
||||
const result = __internals.getAccessTokenExp({
|
||||
accessToken: 'opaque',
|
||||
});
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
/**
|
||||
* Codex Finding 6: id_token TTL is governed by IdP session policy and is
|
||||
* often longer than access-token TTL. Trusting it as access-token expiry
|
||||
* would mark an opaque access token reusable past its real lifetime,
|
||||
* sending an expired credential to the OBO IdP. The fallback chain must
|
||||
* be expires_in → JWT access_token exp → unset (NOT id_token exp).
|
||||
*/
|
||||
it('does NOT fall back to id_token exp for accessTokenExpiresAt when expires_in is missing', async () => {
|
||||
const longLivedIdTokenExp = Math.floor(Date.now() / 1000) + 86400; // 24h
|
||||
const sessionTokens = {
|
||||
accessToken: 'opaque-old',
|
||||
idToken: makeJwt(Math.floor(Date.now() / 1000) - 60),
|
||||
refreshToken: 'rt-no-fallback',
|
||||
};
|
||||
openIdClient.refreshTokenGrant.mockResolvedValueOnce({
|
||||
access_token: 'opaque-fresh', // opaque, NOT a JWT
|
||||
id_token: makeJwt(longLivedIdTokenExp), // long-lived id_token
|
||||
// no expires_in
|
||||
});
|
||||
const req = buildReq(sessionTokens);
|
||||
|
||||
await refreshOpenIDSession(req, makeOpenIdUser(), 'access_token');
|
||||
|
||||
// The long-lived id_token exp must NOT have been borrowed for the access token.
|
||||
expect(req.session.openidTokens).not.toHaveProperty('accessTokenExpiresAt');
|
||||
});
|
||||
|
||||
it('falls back to JWT access_token exp for accessTokenExpiresAt when expires_in is missing', async () => {
|
||||
const accessExp = Math.floor(Date.now() / 1000) + 1800; // 30min
|
||||
const sessionTokens = {
|
||||
accessToken: 'opaque-old',
|
||||
idToken: makeJwt(Math.floor(Date.now() / 1000) - 60),
|
||||
refreshToken: 'rt-jwt-access',
|
||||
};
|
||||
openIdClient.refreshTokenGrant.mockResolvedValueOnce({
|
||||
access_token: makeJwt(accessExp), // JWT access token
|
||||
id_token: makeJwt(Math.floor(Date.now() / 1000) + 86400), // long-lived; should NOT win
|
||||
// no expires_in
|
||||
});
|
||||
const req = buildReq(sessionTokens);
|
||||
|
||||
await refreshOpenIDSession(req, makeOpenIdUser(), 'access_token');
|
||||
|
||||
// accessTokenExpiresAt comes from the access token's own JWT exp, not the id_token.
|
||||
expect(req.session.openidTokens.accessTokenExpiresAt).toBe(accessExp);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -155,7 +155,11 @@ async function reinitMCPServer({
|
|||
graphTokenResolver: getGraphApiToken,
|
||||
oboTokenResolver: exchangeOboToken,
|
||||
oboTrustChecker: createOboTrustChecker(),
|
||||
upstreamTokenProvider: createOpenIDSessionTokenProvider({ req, user }),
|
||||
upstreamTokenProvider: createOpenIDSessionTokenProvider({
|
||||
req,
|
||||
user,
|
||||
tokenPreference: 'access_token',
|
||||
}),
|
||||
});
|
||||
|
||||
logger.info(`[MCP Reinitialize] Successfully established connection for ${serverName}`);
|
||||
|
|
@ -193,7 +197,11 @@ async function reinitMCPServer({
|
|||
graphTokenResolver: getGraphApiToken,
|
||||
oboTokenResolver: exchangeOboToken,
|
||||
oboTrustChecker: createOboTrustChecker(),
|
||||
upstreamTokenProvider: createOpenIDSessionTokenProvider({ req, user }),
|
||||
upstreamTokenProvider: createOpenIDSessionTokenProvider({
|
||||
req,
|
||||
user,
|
||||
tokenPreference: 'access_token',
|
||||
}),
|
||||
});
|
||||
|
||||
if (discoveryResult.tools && discoveryResult.tools.length > 0) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue