mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-09-30 12:52:10 +00:00
🛠️ fix: preserve OIDC refresh-token sync on save failures
Sync OpenID refresh-token cookie/bridge state before persisting the session so a transient session-store failure cannot lose an IdP-rotated refresh token. Also trigger sync when the session refresh token differs from the browser refresh-token marker, not only when the current grant rotates the token. This lets later writable refreshes repair stale browser cookies left behind by SSE refreshes. Route refresh bridge identity through the shared identity helper with the threaded OBO identity context, falling back to request/user context when needed. Add regression coverage for session-save failures, stale browser cookie repair, non-writable bridge storage, and shared-helper identity fallback.
This commit is contained in:
parent
9131186bd1
commit
27c6eb1e35
2 changed files with 157 additions and 14 deletions
|
|
@ -7,6 +7,7 @@ const {
|
|||
math,
|
||||
createAuthIdentityContext,
|
||||
createOpenIDRefreshIdentityTuple,
|
||||
createRefreshTokenBridgeIdentity,
|
||||
serializeAuthIdentityTuple,
|
||||
buildOpenIDRefreshParams,
|
||||
setRefreshTokenCookie,
|
||||
|
|
@ -358,7 +359,7 @@ async function syncRefreshTokenCookie({
|
|||
}
|
||||
}
|
||||
|
||||
async function performIdpRefreshGrant(req, res, user, tokenPreference) {
|
||||
async function performIdpRefreshGrant(req, res, user, tokenPreference, identityContext) {
|
||||
const sessionTokens = req?.session?.openidTokens;
|
||||
const refreshToken = sessionTokens?.refreshToken;
|
||||
if (!refreshToken) {
|
||||
|
|
@ -385,8 +386,8 @@ async function performIdpRefreshGrant(req, res, user, tokenPreference) {
|
|||
const nextIdToken = tokenset.id_token || sessionTokens.idToken;
|
||||
const nextRefreshToken = tokenset.refresh_token || refreshToken;
|
||||
const browserRefreshToken = sessionTokens.browserRefreshToken || refreshToken;
|
||||
const willWriteRefreshTokenCookie =
|
||||
nextRefreshToken !== refreshToken && canWriteRefreshTokenCookie(res);
|
||||
const needsRefreshTokenSync = nextRefreshToken !== browserRefreshToken;
|
||||
const willWriteRefreshTokenCookie = needsRefreshTokenSync && canWriteRefreshTokenCookie(res);
|
||||
|
||||
/**
|
||||
* Capture the freshly-issued access-token's expiry (unix seconds) so the
|
||||
|
|
@ -428,23 +429,33 @@ async function performIdpRefreshGrant(req, res, user, tokenPreference) {
|
|||
}
|
||||
|
||||
req.session.openidTokens = updatedSessionTokens;
|
||||
await persistSession(req);
|
||||
|
||||
/**
|
||||
* Keep the browser refresh-token cookie in sync when the IdP rotated it.
|
||||
* If headers are already sent (SSE streaming), store a recovery bridge instead.
|
||||
* Keep the browser refresh-token cookie in sync with the session token. If headers are
|
||||
* already sent (SSE streaming), store a recovery bridge instead. Do this before the
|
||||
* session save so a transient session-store failure cannot lose an IdP-rotated token.
|
||||
*/
|
||||
if (nextRefreshToken !== refreshToken) {
|
||||
if (needsRefreshTokenSync) {
|
||||
const bridgeIdentity = createRefreshTokenBridgeIdentity({
|
||||
user,
|
||||
requestUser: req?.user,
|
||||
userId: identityContext?.appUserId,
|
||||
tenantId: identityContext?.tenantId,
|
||||
openidIssuer: identityContext?.openidIssuer,
|
||||
});
|
||||
|
||||
await syncRefreshTokenCookie({
|
||||
res,
|
||||
newRefreshToken: nextRefreshToken,
|
||||
oldRefreshToken: browserRefreshToken,
|
||||
userId: user?.id || user?._id?.toString?.() || req.user?.id || req.user?._id?.toString?.(),
|
||||
tenantId: user?.tenantId ?? req.user?.tenantId,
|
||||
openidIssuer: user?.openidIssuer ?? req.user?.openidIssuer,
|
||||
userId: bridgeIdentity?.userId,
|
||||
tenantId: bridgeIdentity?.tenantId,
|
||||
openidIssuer: bridgeIdentity?.openidIssuer,
|
||||
});
|
||||
}
|
||||
|
||||
await persistSession(req);
|
||||
|
||||
logger.info('[OpenIDSessionRefresh] Inline refresh succeeded');
|
||||
/**
|
||||
* Pass the same expiry as the explicit `expiresAtOverride` so the returned
|
||||
|
|
@ -465,7 +476,7 @@ async function performIdpRefresh(req, res, user, tokenPreference, identityContex
|
|||
const refreshToken = req?.session?.openidTokens?.refreshToken;
|
||||
const key = createOpenIDRefreshFlightKey({ req, user, refreshToken, identityContext });
|
||||
if (!key) {
|
||||
return performIdpRefreshGrant(req, res, user, tokenPreference);
|
||||
return performIdpRefreshGrant(req, res, user, tokenPreference, identityContext);
|
||||
}
|
||||
|
||||
let flight;
|
||||
|
|
@ -476,7 +487,7 @@ async function performIdpRefresh(req, res, user, tokenPreference, identityContex
|
|||
'[OpenIDSessionRefresh] Failed to acquire shared refresh flight; refreshing directly',
|
||||
error,
|
||||
);
|
||||
return performIdpRefreshGrant(req, res, user, tokenPreference);
|
||||
return performIdpRefreshGrant(req, res, user, tokenPreference, identityContext);
|
||||
}
|
||||
|
||||
if (!flight.acquired) {
|
||||
|
|
@ -492,11 +503,17 @@ async function performIdpRefresh(req, res, user, tokenPreference, identityContex
|
|||
logger.warn('[OpenIDSessionRefresh] Shared refresh flight unavailable; refreshing directly', {
|
||||
key: hashKeyForLogs(key),
|
||||
});
|
||||
return performIdpRefreshGrant(req, res, user, tokenPreference);
|
||||
return performIdpRefreshGrant(req, res, user, tokenPreference, identityContext);
|
||||
}
|
||||
|
||||
try {
|
||||
const resolvedTokens = await performIdpRefreshGrant(req, res, user, tokenPreference);
|
||||
const resolvedTokens = await performIdpRefreshGrant(
|
||||
req,
|
||||
res,
|
||||
user,
|
||||
tokenPreference,
|
||||
identityContext,
|
||||
);
|
||||
try {
|
||||
await completeOpenIDRefreshFlight({
|
||||
key,
|
||||
|
|
|
|||
|
|
@ -43,6 +43,31 @@ jest.mock('@librechat/api', () => ({
|
|||
serializeAuthIdentityTuple: jest.fn(
|
||||
(tuple) => `${tuple.tenantId}\x1f${tuple.openidIssuer}\x1f${tuple.subject}`,
|
||||
),
|
||||
createRefreshTokenBridgeIdentity: jest.fn(
|
||||
({ user, requestUser, userId, tenantId, openidIssuer }) => {
|
||||
const normalize = (value) => {
|
||||
if (value == null) {
|
||||
return undefined;
|
||||
}
|
||||
const normalized = typeof value === 'string' ? value.trim() : value.toString().trim();
|
||||
return normalized || undefined;
|
||||
};
|
||||
const resolvedUserId =
|
||||
normalize(userId) ??
|
||||
normalize(user?._id) ??
|
||||
normalize(user?.id) ??
|
||||
normalize(requestUser?._id) ??
|
||||
normalize(requestUser?.id);
|
||||
if (!resolvedUserId) {
|
||||
return null;
|
||||
}
|
||||
return {
|
||||
userId: resolvedUserId,
|
||||
tenantId: tenantId ?? user?.tenantId ?? requestUser?.tenantId,
|
||||
openidIssuer: openidIssuer ?? user?.openidIssuer ?? requestUser?.openidIssuer,
|
||||
};
|
||||
},
|
||||
),
|
||||
buildOpenIDRefreshParams: jest.fn(() => ({ scope: 'openid profile' })),
|
||||
setRefreshTokenCookie: jest.fn((res, refreshToken, expires) => {
|
||||
res.cookie('refreshToken', refreshToken, { expires });
|
||||
|
|
@ -465,6 +490,107 @@ describe('OpenIDSessionRefresh', () => {
|
|||
expect(setOpenIDMarkerCookies).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('repairs a stale browser cookie when a stable refresh omits refresh_token', async () => {
|
||||
const refreshedExp = Math.floor(Date.now() / 1000) + 3600;
|
||||
openIdClient.refreshTokenGrant.mockResolvedValueOnce({
|
||||
access_token: makeJwt(refreshedExp),
|
||||
id_token: makeJwt(refreshedExp),
|
||||
expires_in: 3600,
|
||||
});
|
||||
const req = buildReq(buildExpiredSession('rt-session-current', 'rt-browser-stale'));
|
||||
const res = buildRes({ headersSent: false });
|
||||
|
||||
await refreshOpenIDSession(req, res, makeOpenIdUser(), 'access_token');
|
||||
|
||||
expect(setRefreshTokenCookie).toHaveBeenCalledWith(
|
||||
res,
|
||||
'rt-session-current',
|
||||
expect.any(Date),
|
||||
);
|
||||
expect(storeRefreshTokenBridge).not.toHaveBeenCalled();
|
||||
expect(req.session.openidTokens.refreshToken).toBe('rt-session-current');
|
||||
expect(req.session.openidTokens.browserRefreshToken).toBe('rt-session-current');
|
||||
});
|
||||
|
||||
it('stores a bridge for stale browser cookies when a stable refresh cannot write cookies', async () => {
|
||||
const refreshedExp = Math.floor(Date.now() / 1000) + 3600;
|
||||
openIdClient.refreshTokenGrant.mockResolvedValueOnce({
|
||||
access_token: makeJwt(refreshedExp),
|
||||
id_token: makeJwt(refreshedExp),
|
||||
expires_in: 3600,
|
||||
});
|
||||
const req = buildReq(buildExpiredSession('rt-session-current', 'rt-browser-stale'));
|
||||
const res = buildRes({ headersSent: true });
|
||||
|
||||
await refreshOpenIDSession(req, res, makeOpenIdUser(), 'access_token');
|
||||
|
||||
expect(setRefreshTokenCookie).not.toHaveBeenCalled();
|
||||
expect(setOpenIDMarkerCookies).not.toHaveBeenCalled();
|
||||
expect(storeRefreshTokenBridge).toHaveBeenCalledWith({
|
||||
oldRefreshToken: 'rt-browser-stale',
|
||||
newRefreshToken: 'rt-session-current',
|
||||
userId: 'local-id-1',
|
||||
tenantId: undefined,
|
||||
openidIssuer: undefined,
|
||||
});
|
||||
expect(req.session.openidTokens.browserRefreshToken).toBe('rt-browser-stale');
|
||||
});
|
||||
|
||||
it('resolves bridge identity through the shared helper when identity context is absent', async () => {
|
||||
const refreshedExp = Math.floor(Date.now() / 1000) + 3600;
|
||||
openIdClient.refreshTokenGrant.mockResolvedValueOnce({
|
||||
access_token: makeJwt(refreshedExp),
|
||||
id_token: makeJwt(refreshedExp),
|
||||
refresh_token: 'rt-rotated',
|
||||
expires_in: 3600,
|
||||
});
|
||||
const req = buildReq(buildExpiredSession('rt-old'));
|
||||
const res = buildRes({ headersSent: true });
|
||||
|
||||
await refreshOpenIDSession(
|
||||
req,
|
||||
res,
|
||||
makeOpenIdUser({
|
||||
id: 'public-id',
|
||||
_id: { toString: () => 'mongo-id' },
|
||||
tenantId: 'tenant-1',
|
||||
openidIssuer: 'https://issuer.example.com',
|
||||
}),
|
||||
'access_token',
|
||||
);
|
||||
|
||||
expect(storeRefreshTokenBridge).toHaveBeenCalledWith({
|
||||
oldRefreshToken: 'rt-old',
|
||||
newRefreshToken: 'rt-rotated',
|
||||
userId: 'mongo-id',
|
||||
tenantId: 'tenant-1',
|
||||
openidIssuer: 'https://issuer.example.com',
|
||||
});
|
||||
});
|
||||
|
||||
it('syncs the rotated cookie before surfacing a session save failure', async () => {
|
||||
const refreshedExp = Math.floor(Date.now() / 1000) + 3600;
|
||||
openIdClient.refreshTokenGrant.mockResolvedValueOnce({
|
||||
access_token: makeJwt(refreshedExp),
|
||||
id_token: makeJwt(refreshedExp),
|
||||
refresh_token: 'rt-rotated',
|
||||
expires_in: 3600,
|
||||
});
|
||||
const req = buildReq(buildExpiredSession('rt-old'));
|
||||
const res = buildRes({ headersSent: false });
|
||||
req.session.save.mockImplementationOnce((cb) => cb(new Error('session store down')));
|
||||
|
||||
await expect(
|
||||
refreshOpenIDSession(req, res, makeOpenIdUser(), 'access_token'),
|
||||
).rejects.toThrow('session store down');
|
||||
|
||||
expect(setRefreshTokenCookie).toHaveBeenCalledWith(res, 'rt-rotated', expect.any(Date));
|
||||
expect(setOpenIDMarkerCookies).toHaveBeenCalledTimes(1);
|
||||
expect(storeRefreshTokenBridge).not.toHaveBeenCalled();
|
||||
expect(req.session.openidTokens.refreshToken).toBe('rt-rotated');
|
||||
expect(req.session.openidTokens.browserRefreshToken).toBe('rt-rotated');
|
||||
});
|
||||
|
||||
it('stores a recovery bridge when response headers are already sent (streaming path)', async () => {
|
||||
const refreshedExp = Math.floor(Date.now() / 1000) + 3600;
|
||||
openIdClient.refreshTokenGrant.mockResolvedValueOnce({
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue