mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-21 09:32:24 +00:00
🧲 fix: Draw OpenID Refresh Lookups Into System Context (#15451)
* fix: scope refresh user lookups to system context * fix: preserve refresh system context * fix: bind refresh recovery to user tenant
This commit is contained in:
Danny Avila
GitHub
parent
224512851a
commit
97e1e950ad
2 changed files with 210 additions and 124 deletions
|
|
@ -1,7 +1,7 @@
|
|||
const cookies = require('cookie');
|
||||
const jwt = require('jsonwebtoken');
|
||||
const crypto = require('node:crypto');
|
||||
const { logger } = require('@librechat/data-schemas');
|
||||
const { logger, runAsSystem, tenantStorage } = require('@librechat/data-schemas');
|
||||
const {
|
||||
math,
|
||||
isEnabled,
|
||||
|
|
@ -72,6 +72,14 @@ const sanitizeUserForAuthResponse = (user) => {
|
|||
return safeUser;
|
||||
};
|
||||
|
||||
const runInUserTenant = (user, fn) =>
|
||||
user.tenantId
|
||||
? tenantStorage.run(
|
||||
{ tenantId: user.tenantId, userId: user._id.toString() },
|
||||
async () => await fn(),
|
||||
)
|
||||
: runAsSystem(fn);
|
||||
|
||||
const getValidOpenIDReuseUserId = (parsedCookies, refreshToken) => {
|
||||
const openidUserId = parsedCookies.openid_user_id;
|
||||
if (!openidUserId || !process.env.JWT_REFRESH_SECRET) {
|
||||
|
|
@ -345,7 +353,9 @@ const refreshController = async (req, res) => {
|
|||
context: 'refreshController',
|
||||
},
|
||||
async (sendAuthorized) => {
|
||||
const user = await getUserById(reuseUserId, AUTH_REFRESH_USER_PROJECTION);
|
||||
const user = await runAsSystem(async () =>
|
||||
getUserById(reuseUserId, AUTH_REFRESH_USER_PROJECTION),
|
||||
);
|
||||
if (!user || !isReusableOpenIDSessionIdentity(reuseSessionTokens, user)) {
|
||||
return undefined;
|
||||
}
|
||||
|
|
@ -378,99 +388,116 @@ const refreshController = async (req, res) => {
|
|||
|
||||
const refreshUserId =
|
||||
req.session?.openidTokens?.appUserId ?? getValidOpenIDReuseUserId(parsedCookies);
|
||||
const refreshUser = refreshUserId
|
||||
? await getUserById(refreshUserId, AUTH_REFRESH_USER_PROJECTION)
|
||||
: null;
|
||||
if (!refreshUserId) {
|
||||
return res.status(403).send('Invalid OpenID refresh token');
|
||||
}
|
||||
|
||||
const refreshUser = await runAsSystem(async () =>
|
||||
getUserById(refreshUserId, AUTH_REFRESH_USER_PROJECTION),
|
||||
);
|
||||
if (!refreshUser) {
|
||||
return res.status(403).send('Invalid OpenID refresh token');
|
||||
}
|
||||
|
||||
let successfulRefreshToken = refreshToken;
|
||||
let refreshResult;
|
||||
try {
|
||||
refreshResult = await refreshOpenIDUser({
|
||||
req,
|
||||
res,
|
||||
user: refreshUser,
|
||||
refreshToken,
|
||||
browserRefreshToken: parsedCookies.refreshToken,
|
||||
strategyName: 'refreshController',
|
||||
deferPublication: true,
|
||||
});
|
||||
} catch (error) {
|
||||
if (!fallbackRefreshToken || !isInvalidGrantError(error)) {
|
||||
throw error;
|
||||
return await runInUserTenant(refreshUser, async () => {
|
||||
let successfulRefreshToken = refreshToken;
|
||||
let refreshResult;
|
||||
try {
|
||||
refreshResult = await refreshOpenIDUser({
|
||||
req,
|
||||
res,
|
||||
user: refreshUser,
|
||||
refreshToken,
|
||||
browserRefreshToken: parsedCookies.refreshToken,
|
||||
strategyName: 'refreshController',
|
||||
deferPublication: true,
|
||||
});
|
||||
} catch (error) {
|
||||
if (!fallbackRefreshToken || !isInvalidGrantError(error)) {
|
||||
throw error;
|
||||
}
|
||||
logger.info(
|
||||
'[refreshController] Session refresh token was rejected; retrying the distinct browser token',
|
||||
);
|
||||
successfulRefreshToken = fallbackRefreshToken;
|
||||
refreshResult = await refreshOpenIDUser({
|
||||
req,
|
||||
res,
|
||||
user: refreshUser,
|
||||
refreshToken: fallbackRefreshToken,
|
||||
browserRefreshToken: parsedCookies.refreshToken,
|
||||
strategyName: 'refreshController (browser fallback)',
|
||||
deferPublication: true,
|
||||
});
|
||||
}
|
||||
logger.info(
|
||||
'[refreshController] Session refresh token was rejected; retrying the distinct browser token',
|
||||
);
|
||||
successfulRefreshToken = fallbackRefreshToken;
|
||||
refreshResult = await refreshOpenIDUser({
|
||||
const { tokenset, claims, openidIssuer, user, error, migration } = refreshResult;
|
||||
|
||||
if (error || !user) {
|
||||
logger.warn(
|
||||
`[refreshController] Redirecting to /login: error=${error ?? 'null'}, user=${user ? 'exists' : 'null'}`,
|
||||
);
|
||||
return res.status(401).redirect('/login');
|
||||
}
|
||||
|
||||
if (user._id.toString() !== refreshUser._id.toString()) {
|
||||
logger.warn(
|
||||
'[refreshController] Refreshed identity resolved a different user; refusing token issuance',
|
||||
{
|
||||
refreshUserId: refreshUser._id.toString(),
|
||||
resolvedUserId: user._id.toString(),
|
||||
},
|
||||
);
|
||||
return res.status(401).redirect('/login');
|
||||
}
|
||||
|
||||
// Handle migration: update user with openidId if found by email without openidId
|
||||
// Also handle case where user has mismatched openidId (e.g., after database switch)
|
||||
if (migration || user.openidId !== claims.sub) {
|
||||
const reason = migration ? 'migration' : 'openidId mismatch';
|
||||
await updateUser(user._id.toString(), {
|
||||
provider: 'openid',
|
||||
openidId: claims.sub,
|
||||
...(openidIssuer ? { openidIssuer } : {}),
|
||||
});
|
||||
logger.info(
|
||||
`[refreshController] Updated user ${user.email} openidId (${reason}): ${user.openidId ?? 'null'} -> ${claims.sub}`,
|
||||
);
|
||||
}
|
||||
|
||||
if (
|
||||
successfulRefreshToken !== refreshToken &&
|
||||
req.session?.openidTokens?.refreshToken === refreshToken
|
||||
) {
|
||||
delete req.session.openidTokens;
|
||||
}
|
||||
|
||||
const token = await sendOpenIDAuthResponse({
|
||||
tokenset,
|
||||
user,
|
||||
existingRefreshToken: successfulRefreshToken,
|
||||
openidSubject: claims?.sub,
|
||||
openidIssuer,
|
||||
predecessorIdentity: {
|
||||
userId: refreshUser._id.toString(),
|
||||
tenantId: refreshUser.tenantId,
|
||||
openidIssuer: refreshUser.openidIssuer,
|
||||
},
|
||||
rejectedRefreshTokens: successfulRefreshToken === refreshToken ? [] : [refreshToken],
|
||||
req,
|
||||
res,
|
||||
user: refreshUser,
|
||||
refreshToken: fallbackRefreshToken,
|
||||
browserRefreshToken: parsedCookies.refreshToken,
|
||||
strategyName: 'refreshController (browser fallback)',
|
||||
deferPublication: true,
|
||||
});
|
||||
}
|
||||
const { tokenset, claims, openidIssuer, user, error, migration } = refreshResult;
|
||||
|
||||
if (error || !user) {
|
||||
logger.warn(
|
||||
`[refreshController] Redirecting to /login: error=${error ?? 'null'}, user=${user ? 'exists' : 'null'}`,
|
||||
return await withOpenIDResponseDelivery(
|
||||
{
|
||||
res,
|
||||
openidTokens: req.session?.openidTokens,
|
||||
context: 'refreshController',
|
||||
},
|
||||
(sendAuthorized) =>
|
||||
sendAuthorized(() =>
|
||||
res.status(200).send({ token, user: sanitizeUserForAuthResponse(user) }),
|
||||
),
|
||||
);
|
||||
return res.status(401).redirect('/login');
|
||||
}
|
||||
|
||||
// Handle migration: update user with openidId if found by email without openidId
|
||||
// Also handle case where user has mismatched openidId (e.g., after database switch)
|
||||
if (migration || user.openidId !== claims.sub) {
|
||||
const reason = migration ? 'migration' : 'openidId mismatch';
|
||||
await updateUser(user._id.toString(), {
|
||||
provider: 'openid',
|
||||
openidId: claims.sub,
|
||||
...(openidIssuer ? { openidIssuer } : {}),
|
||||
});
|
||||
logger.info(
|
||||
`[refreshController] Updated user ${user.email} openidId (${reason}): ${user.openidId ?? 'null'} -> ${claims.sub}`,
|
||||
);
|
||||
}
|
||||
|
||||
if (
|
||||
successfulRefreshToken !== refreshToken &&
|
||||
req.session?.openidTokens?.refreshToken === refreshToken
|
||||
) {
|
||||
delete req.session.openidTokens;
|
||||
}
|
||||
|
||||
const token = await sendOpenIDAuthResponse({
|
||||
tokenset,
|
||||
user,
|
||||
existingRefreshToken: successfulRefreshToken,
|
||||
openidSubject: claims?.sub,
|
||||
openidIssuer,
|
||||
predecessorIdentity: {
|
||||
userId: refreshUser._id.toString(),
|
||||
tenantId: refreshUser.tenantId,
|
||||
openidIssuer: refreshUser.openidIssuer,
|
||||
},
|
||||
rejectedRefreshTokens: successfulRefreshToken === refreshToken ? [] : [refreshToken],
|
||||
req,
|
||||
res,
|
||||
});
|
||||
return await withOpenIDResponseDelivery(
|
||||
{
|
||||
res,
|
||||
openidTokens: req.session?.openidTokens,
|
||||
context: 'refreshController',
|
||||
},
|
||||
(sendAuthorized) =>
|
||||
sendAuthorized(() =>
|
||||
res.status(200).send({ token, user: sanitizeUserForAuthResponse(user) }),
|
||||
),
|
||||
);
|
||||
} catch (error) {
|
||||
if (isOpenIDRefreshOwnershipError(error)) {
|
||||
clearOpenIDAuthTokens(
|
||||
|
|
@ -494,53 +521,60 @@ const refreshController = async (req, res) => {
|
|||
const userId = getValidOpenIDReuseUserId(parsedCookies, bridgeSourceToken);
|
||||
if (userId) {
|
||||
try {
|
||||
const bridgeUser = await getUserById(userId, AUTH_REFRESH_USER_PROJECTION);
|
||||
const bridgeUser = await runAsSystem(async () =>
|
||||
getUserById(userId, AUTH_REFRESH_USER_PROJECTION),
|
||||
);
|
||||
if (!bridgeUser) {
|
||||
return res.status(403).send('Invalid OpenID refresh token');
|
||||
}
|
||||
|
||||
const bridgedRefreshToken = await getRefreshTokenBridge({
|
||||
oldRefreshToken: bridgeSourceToken,
|
||||
userId,
|
||||
tenantId: bridgeUser.tenantId,
|
||||
openidIssuer: bridgeUser.openidIssuer,
|
||||
});
|
||||
const bridgeResponse = await runInUserTenant(bridgeUser, async () => {
|
||||
const bridgedRefreshToken = await getRefreshTokenBridge({
|
||||
oldRefreshToken: bridgeSourceToken,
|
||||
userId,
|
||||
tenantId: bridgeUser.tenantId,
|
||||
openidIssuer: bridgeUser.openidIssuer,
|
||||
});
|
||||
|
||||
if (bridgedRefreshToken) {
|
||||
logger.info(
|
||||
'[refreshController] Recovered via refresh-token bridge after invalid_grant',
|
||||
{
|
||||
userId,
|
||||
},
|
||||
);
|
||||
|
||||
try {
|
||||
const { appAuthToken } = await recoverOpenIDRefreshBridge({
|
||||
req,
|
||||
res,
|
||||
refreshToken: bridgeSourceToken,
|
||||
bridgedRefreshToken,
|
||||
bridgeUser,
|
||||
});
|
||||
|
||||
return await withOpenIDResponseDelivery(
|
||||
if (bridgedRefreshToken) {
|
||||
logger.info(
|
||||
'[refreshController] Recovered via refresh-token bridge after invalid_grant',
|
||||
{
|
||||
res,
|
||||
openidTokens: req.session?.openidTokens,
|
||||
context: 'refreshController',
|
||||
userId,
|
||||
},
|
||||
(sendAuthorized) =>
|
||||
sendAuthorized(() =>
|
||||
res.status(200).send({
|
||||
token: appAuthToken,
|
||||
user: sanitizeUserForAuthResponse(bridgeUser),
|
||||
}),
|
||||
),
|
||||
);
|
||||
} catch (retryError) {
|
||||
logger.error('[refreshController] Bridge recovery retry failed', retryError);
|
||||
// Fall through to generic error response
|
||||
|
||||
try {
|
||||
const { appAuthToken } = await recoverOpenIDRefreshBridge({
|
||||
req,
|
||||
res,
|
||||
refreshToken: bridgeSourceToken,
|
||||
bridgedRefreshToken,
|
||||
bridgeUser,
|
||||
});
|
||||
|
||||
return await withOpenIDResponseDelivery(
|
||||
{
|
||||
res,
|
||||
openidTokens: req.session?.openidTokens,
|
||||
context: 'refreshController',
|
||||
},
|
||||
(sendAuthorized) =>
|
||||
sendAuthorized(() =>
|
||||
res.status(200).send({
|
||||
token: appAuthToken,
|
||||
user: sanitizeUserForAuthResponse(bridgeUser),
|
||||
}),
|
||||
),
|
||||
);
|
||||
} catch (retryError) {
|
||||
logger.error('[refreshController] Bridge recovery retry failed', retryError);
|
||||
// Fall through to generic error response
|
||||
}
|
||||
}
|
||||
});
|
||||
if (bridgeResponse !== undefined) {
|
||||
return bridgeResponse;
|
||||
}
|
||||
} catch (bridgeError) {
|
||||
logger.warn('[refreshController] Refresh-token bridge lookup failed', {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,26 @@
|
|||
let mockActiveTenantId;
|
||||
const mockRunAsSystem = jest.fn(async (fn) => {
|
||||
const previousTenantId = mockActiveTenantId;
|
||||
mockActiveTenantId = '__SYSTEM__';
|
||||
try {
|
||||
return await fn();
|
||||
} finally {
|
||||
mockActiveTenantId = previousTenantId;
|
||||
}
|
||||
});
|
||||
const mockTenantStorageRun = jest.fn(async (context, fn) => {
|
||||
const previousTenantId = mockActiveTenantId;
|
||||
mockActiveTenantId = context.tenantId;
|
||||
try {
|
||||
return await fn();
|
||||
} finally {
|
||||
mockActiveTenantId = previousTenantId;
|
||||
}
|
||||
});
|
||||
jest.mock('@librechat/data-schemas', () => ({
|
||||
logger: { error: jest.fn(), debug: jest.fn(), warn: jest.fn(), info: jest.fn() },
|
||||
runAsSystem: (fn) => mockRunAsSystem(fn),
|
||||
tenantStorage: { run: (context, fn) => mockTenantStorageRun(context, fn) },
|
||||
}));
|
||||
jest.mock('~/server/services/GraphTokenService', () => ({
|
||||
getGraphApiToken: jest.fn(),
|
||||
|
|
@ -506,6 +527,7 @@ describe('refreshController – OpenID path', () => {
|
|||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
mockActiveTenantId = undefined;
|
||||
delete process.env.OPENID_SCOPE;
|
||||
delete process.env.OPENID_REFRESH_AUDIENCE;
|
||||
process.env.JWT_REFRESH_SECRET = 'test-refresh-secret';
|
||||
|
|
@ -900,6 +922,7 @@ describe('refreshController – OpenID path', () => {
|
|||
'user-db-id',
|
||||
'-password -__v -totpSecret -backupCodes -federatedTokens',
|
||||
);
|
||||
expect(mockRunAsSystem).toHaveBeenCalledTimes(1);
|
||||
expect(setCloudFrontAuthCookies).toHaveBeenCalledWith(req, res, user);
|
||||
expect(res.status).toHaveBeenCalledWith(200);
|
||||
expect(res.send).toHaveBeenCalledWith({
|
||||
|
|
@ -1049,10 +1072,15 @@ describe('refreshController – OpenID path', () => {
|
|||
openidIssuer: baseClaims.iss,
|
||||
},
|
||||
};
|
||||
findOpenIDUser.mockImplementationOnce(async () => {
|
||||
expect(mockActiveTenantId).toBe('tenant-1');
|
||||
return { user: { ...defaultUser }, error: null, migration: false };
|
||||
});
|
||||
|
||||
await refreshController(req, res);
|
||||
|
||||
expect(getUserById).toHaveBeenCalled();
|
||||
expect(mockRunAsSystem).toHaveBeenCalledTimes(1);
|
||||
expect(setCloudFrontAuthCookies).not.toHaveBeenCalled();
|
||||
expectOpenIDRefreshGrant();
|
||||
});
|
||||
|
|
@ -1389,6 +1417,27 @@ describe('refreshController – OpenID path', () => {
|
|||
expect(res.redirect).toHaveBeenCalledWith('/login');
|
||||
});
|
||||
|
||||
it('rejects a refreshed identity that resolves to a different user', async () => {
|
||||
findOpenIDUser.mockResolvedValue({
|
||||
user: { ...defaultUser, _id: 'different-user-id' },
|
||||
error: null,
|
||||
migration: false,
|
||||
});
|
||||
|
||||
await refreshController(req, res);
|
||||
|
||||
expect(setOpenIDAuthTokens).not.toHaveBeenCalled();
|
||||
expect(logger.warn).toHaveBeenCalledWith(
|
||||
'[refreshController] Refreshed identity resolved a different user; refusing token issuance',
|
||||
{
|
||||
refreshUserId: 'user-db-id',
|
||||
resolvedUserId: 'different-user-id',
|
||||
},
|
||||
);
|
||||
expect(res.status).toHaveBeenCalledWith(401);
|
||||
expect(res.redirect).toHaveBeenCalledWith('/login');
|
||||
});
|
||||
|
||||
it('should preserve invalid OpenID refresh token behavior', async () => {
|
||||
openIdClient.refreshTokenGrant.mockRejectedValue(new Error('invalid_grant'));
|
||||
|
||||
|
|
@ -1446,7 +1495,10 @@ describe('refreshController – OpenID path', () => {
|
|||
tenantId: 'tenant-1',
|
||||
openidIssuer: 'https://issuer.example.com',
|
||||
});
|
||||
getRefreshTokenBridge.mockResolvedValue('bridged-refresh');
|
||||
getRefreshTokenBridge.mockImplementationOnce(async () => {
|
||||
expect(mockActiveTenantId).toBe('tenant-1');
|
||||
return 'bridged-refresh';
|
||||
});
|
||||
const nonRotatingTokenset = { ...mockTokenset };
|
||||
delete nonRotatingTokenset.refresh_token;
|
||||
openIdClient.refreshTokenGrant
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue