mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-09-21 15:45:22 +00:00
* 🕰️ fix: Guard `expires_in` So a Token Response Cannot Outlive Its Credential RFC 6749 §5.1 makes `expires_in` only RECOMMENDED, so a token response may legally omit it. Four sites derived a lifetime from the raw field, where `undefined * 1000` is `NaN`. `NaN` is not a short TTL, it is no TTL. `@keyv/redis` writes the key without `PX` because `NaN` is falsy, so the entry is stored in Redis with no expiration at all; the in-memory backend embeds `expires: NaN` and every check compares with `>`, always false against `NaN`. The namespace default does not stand in either, since Keyv applies it with `??=` and `NaN` is neither `null` nor `undefined`. The exchanged access token was therefore cached permanently at `openidStrategy.js` and `GraphApiService.js`, and once it genuinely expired the poisoned entry kept being served with no path to eviction. The same omission is sharper in `ActionService.js`, where `new Date(NaN).toISOString()` throws `RangeError: Invalid time value`. Both call sites are inside a `try`, so the failure surfaces as a generic "Failed to authenticate OAuth tool" that names nothing, and the refresh site falls through to `requestLogin()` on every attempt, looping with no exit. The rule had six hand-written homes and three of them were wrong, so it now has one. A new `packages/api/src/oauth/expiry.ts` normalizes `expires_in` to a positive finite number of seconds or nothing, and exposes the two shapes callers actually need: a cache TTL that falls back rather than returning `NaN`, and an absolute expiry that is absent rather than Invalid. The four unguarded sites adopt it, and the two ad-hoc guards in `openidStrategy.js` and `OboTokenService.js` are consolidated onto it. `createHandleOAuthToken` is folded in as well. Its guard already handled `null` and unparseable strings but admitted `NaN`, since `typeof NaN === 'number'` satisfied its first branch. The `mcp/oauth` sites are deliberately left alone: `tokens.ts` guards on truthiness and carries richer logic that reads a JWT access token's own expiry when the response omits one, and the file is being reworked in #13901. Closes #15318 Closes #15319 * 🕰️ fix: Address `expires_in` Guard Review Round 1 Preserve an explicitly elapsed lifetime instead of collapsing it into "unknown". `expires_in: 0` is the provider stating the credential is already dead, which is information; treating it as absent handed it the one-hour fallback in `createHandleOAuthToken` and dropped the expiry entirely in `ActionService`, so a credential declared expired could be used and retained for up to an hour. Both sites preserved that value before this branch, so the collapse was a regression introduced here. `normalizeExpiresIn` now returns any finite number, positive or not, and reports `undefined` only for a lifetime that is genuinely unusable. `getTokenExpiresAt` therefore yields a past timestamp for an elapsed lifetime, so callers refresh rather than guess. Cache TTLs cannot pass such a value through raw: Keyv reads a TTL of exactly `0` as "no expiry", turning a dead credential into the immortal entry this module exists to prevent. `getTokenCacheTtlMs` floors an elapsed lifetime at one millisecond, which expires immediately without ever writing an entry that outlives its credential. Parse numeric strings with `Number` rather than `parseInt`, which truncates a complete value such as `"3.6e3"` to `3` and would expire an hour-long credential after three seconds, re-exchanging against the identity provider on every request. An empty or blank string is rejected rather than read as zero, since `Number('')` is `0`. * 🕰️ fix: Bound `expires_in` to Lifetimes a Date Can Represent Parsing the complete numeric string last round made an overflow reachable that `parseInt` had been masking. `parseInt('1e13', 10)` was `1`; `Number('1e13')` is `1e13`, and `1e13` seconds is 1e16 ms, past the ECMAScript time value range of ±8.64e15. Every derived timestamp was therefore an Invalid Date whose `toISOString()` throws `RangeError: Invalid time value` — the exact failure this branch exists to remove, reintroduced by its own fix. The token model derives the same way at `packages/data-schemas/src/methods/token.ts:19`, so storage and authentication would fail with it. A lifetime is now reported as unusable unless it can still produce a valid `Date`. The bound is the time value range halved, leaving room for the `Date.now()` every derived timestamp adds. At roughly 137,000 years it rejects nothing a provider could mean: a one-year refresh token and even a hundred-year lifetime still pass through untouched, while `1e13`, `Number.MAX_SAFE_INTEGER` and `1e300` take the caller's fallback instead of poisoning a timestamp. The invariant tests now carry the overflow shapes rather than a fixed list of small ones, since a guard that only sees the inputs its author imagined is how the previous round's regression got in.
203 lines
6.3 KiB
JavaScript
203 lines
6.3 KiB
JavaScript
const client = require('openid-client');
|
|
const { logger } = require('@librechat/data-schemas');
|
|
const { CacheKeys } = require('librechat-data-provider');
|
|
const {
|
|
normalizeExpiresIn,
|
|
getTokenCacheTtlMs,
|
|
DEFAULT_OAUTH_TOKEN_TTL_SECONDS,
|
|
} = require('@librechat/api');
|
|
const { getOpenIdConfig } = require('~/strategies/openidStrategy');
|
|
const getLogStores = require('~/cache/getLogStores');
|
|
|
|
const RETRYABLE_STATUS_CODES = new Set([429, 500, 502, 503, 504]);
|
|
const RETRYABLE_ERROR_CODES = new Set(['ETIMEDOUT', 'ECONNRESET', 'EAI_AGAIN', 'ENOTFOUND']);
|
|
const OBO_RETRY_DELAY_MS = 300;
|
|
|
|
/**
|
|
* In-flight OBO exchanges keyed by `${openidId}:${scopes}`.
|
|
*
|
|
* Without coalescing, parallel tool calls that arrive on a cache miss each issue
|
|
* their own jwt-bearer request to the IdP. Under fan-out, Entra intermittently
|
|
* returns errors that look non-retryable, surfacing as "identity provider
|
|
* rejected the OBO token exchange." A user retry then hits the populated cache
|
|
* and succeeds, which matches the observed flakiness. Sharing a single upstream
|
|
* exchange per key removes the thundering herd.
|
|
*/
|
|
const inFlightExchanges = new Map();
|
|
|
|
function getErrorStatus(error) {
|
|
return error?.status ?? error?.statusCode ?? error?.response?.status;
|
|
}
|
|
|
|
function getErrorCode(error) {
|
|
return typeof error?.code === 'string' ? error.code.toUpperCase() : undefined;
|
|
}
|
|
|
|
function isRetryableOboExchangeError(error) {
|
|
const status = getErrorStatus(error);
|
|
if (status != null && RETRYABLE_STATUS_CODES.has(status)) {
|
|
return true;
|
|
}
|
|
|
|
const code = getErrorCode(error);
|
|
if (code != null && RETRYABLE_ERROR_CODES.has(code)) {
|
|
return true;
|
|
}
|
|
|
|
const message = String(error?.message ?? '').toLowerCase();
|
|
return (
|
|
message.includes('timed out') ||
|
|
message.includes('timeout') ||
|
|
message.includes('econnreset') ||
|
|
message.includes('socket hang up') ||
|
|
message.includes('temporarily unavailable') ||
|
|
message.includes('too many requests') ||
|
|
message.includes('service unavailable')
|
|
);
|
|
}
|
|
|
|
function tagOboExchangeError(error, retryable) {
|
|
if (error && typeof error === 'object') {
|
|
error.retryable = retryable;
|
|
error.oboFailureReason = 'exchange_failed';
|
|
}
|
|
return error;
|
|
}
|
|
|
|
async function delay(ms) {
|
|
await new Promise((resolve) => setTimeout(resolve, ms));
|
|
}
|
|
|
|
async function performOboExchange({ user, accessToken, scopes, config, tokensCache, cacheKey }) {
|
|
const requestGrant = async () =>
|
|
client.genericGrantRequest(config, 'urn:ietf:params:oauth:grant-type:jwt-bearer', {
|
|
scope: scopes,
|
|
assertion: accessToken,
|
|
requested_token_use: 'on_behalf_of',
|
|
});
|
|
|
|
let grantResponse;
|
|
try {
|
|
grantResponse = await requestGrant();
|
|
} catch (error) {
|
|
const retryable = isRetryableOboExchangeError(error);
|
|
if (!retryable) {
|
|
throw tagOboExchangeError(error, false);
|
|
}
|
|
|
|
logger.warn(
|
|
`[OboTokenService] Transient OBO exchange failure for user: ${user.openidId}, retrying once`,
|
|
error,
|
|
);
|
|
await delay(OBO_RETRY_DELAY_MS);
|
|
|
|
try {
|
|
grantResponse = await requestGrant();
|
|
} catch (retryError) {
|
|
throw tagOboExchangeError(retryError, isRetryableOboExchangeError(retryError));
|
|
}
|
|
}
|
|
|
|
const tokenResponse = {
|
|
access_token: grantResponse.access_token,
|
|
token_type: 'Bearer',
|
|
expires_in: normalizeExpiresIn(grantResponse.expires_in) ?? DEFAULT_OAUTH_TOKEN_TTL_SECONDS,
|
|
scope: scopes,
|
|
};
|
|
|
|
await tokensCache.set(
|
|
cacheKey,
|
|
tokenResponse,
|
|
getTokenCacheTtlMs(grantResponse.expires_in, DEFAULT_OAUTH_TOKEN_TTL_SECONDS),
|
|
);
|
|
|
|
logger.debug(
|
|
`[OboTokenService] Successfully obtained and cached OBO token for user: ${user.openidId}`,
|
|
);
|
|
return tokenResponse;
|
|
}
|
|
|
|
/**
|
|
* Exchange a user's access token for a downstream-scoped token via the
|
|
* OAuth 2.0 On-Behalf-Of (jwt-bearer) grant.
|
|
*
|
|
* Concurrent callers for the same `${openidId}:${scopes}` key share a single
|
|
* upstream exchange (see `inFlightExchanges`) so a fan-out of tool calls right
|
|
* after a cache miss does not produce N parallel requests to the IdP.
|
|
*
|
|
* @param {Object} user - User object with OpenID information
|
|
* @param {string} accessToken - Federated access token used as OBO assertion
|
|
* @param {string} scopes - Scopes to request for the downstream service
|
|
* @param {boolean} [fromCache=true] - When true, read from cache and join any
|
|
* in-flight exchange. When false, bypass both and force a fresh exchange.
|
|
* @returns {Promise<Object>} Token response with access_token and expires_in
|
|
*/
|
|
async function exchangeOboToken(user, accessToken, scopes, fromCache = true) {
|
|
if (!user.openidId) {
|
|
throw new Error('User must be authenticated via OpenID to perform OBO token exchange');
|
|
}
|
|
|
|
if (!accessToken) {
|
|
throw new Error('Access token is required for OBO exchange');
|
|
}
|
|
|
|
if (!scopes) {
|
|
throw new Error('Scopes are required for OBO exchange');
|
|
}
|
|
|
|
const config = getOpenIdConfig();
|
|
if (!config) {
|
|
throw new Error('OpenID configuration not available');
|
|
}
|
|
|
|
const cacheKey = `${user.openidId}:${scopes}`;
|
|
const tokensCache = getLogStores(CacheKeys.OPENID_EXCHANGED_TOKENS);
|
|
|
|
if (fromCache) {
|
|
const cachedToken = await tokensCache.get(cacheKey);
|
|
if (cachedToken) {
|
|
logger.debug(`[OboTokenService] Using cached token for user: ${user.openidId}`);
|
|
return cachedToken;
|
|
}
|
|
|
|
const inFlight = inFlightExchanges.get(cacheKey);
|
|
if (inFlight) {
|
|
logger.debug(`[OboTokenService] Joining in-flight OBO exchange for user: ${user.openidId}`);
|
|
return inFlight;
|
|
}
|
|
}
|
|
|
|
logger.debug(
|
|
`[OboTokenService] Requesting new OBO token for user: ${user.openidId}, scopes: ${scopes}`,
|
|
);
|
|
|
|
const exchangePromise = performOboExchange({
|
|
user,
|
|
accessToken,
|
|
scopes,
|
|
config,
|
|
tokensCache,
|
|
cacheKey,
|
|
});
|
|
|
|
if (fromCache) {
|
|
inFlightExchanges.set(cacheKey, exchangePromise);
|
|
exchangePromise
|
|
.finally(() => {
|
|
if (inFlightExchanges.get(cacheKey) === exchangePromise) {
|
|
inFlightExchanges.delete(cacheKey);
|
|
}
|
|
})
|
|
.catch(() => {
|
|
/* The original rejection is delivered to the awaiting caller; this
|
|
* chain exists only to run cleanup, so swallow it here to avoid an
|
|
* unhandled-rejection warning on the cleanup promise. */
|
|
});
|
|
}
|
|
|
|
return exchangePromise;
|
|
}
|
|
|
|
module.exports = {
|
|
exchangeOboToken,
|
|
};
|