diff --git a/api/server/controllers/AuthController.js b/api/server/controllers/AuthController.js index bb2ad39beb..ed1f9cb100 100644 --- a/api/server/controllers/AuthController.js +++ b/api/server/controllers/AuthController.js @@ -354,98 +354,93 @@ const refreshController = async (req, res) => { */ if (isInvalidGrantError(error) && refreshToken) { // Bridge lookup uses the signed user-id cookie because /refresh is unauthenticated. - const openidUserId = parsedCookies.openid_user_id; - if (openidUserId) { + const userId = getValidOpenIDReuseUserId(parsedCookies); + if (userId) { try { - const payload = jwt.verify(openidUserId, process.env.JWT_REFRESH_SECRET); - const userId = typeof payload === 'object' && payload?.id ? payload.id : null; + const bridgeUser = await getUserById(userId, AUTH_REFRESH_USER_PROJECTION); + if (!bridgeUser) { + return res.status(403).send('Invalid OpenID refresh token'); + } - if (userId) { - const bridgeUser = await getUserById(userId, AUTH_REFRESH_USER_PROJECTION); - if (!bridgeUser) { - return res.status(403).send('Invalid OpenID refresh token'); - } + const bridgedRefreshToken = await getRefreshTokenBridge({ + oldRefreshToken: refreshToken, + userId, + tenantId: bridgeUser.tenantId, + openidIssuer: bridgeUser.openidIssuer, + }); - const bridgedRefreshToken = await getRefreshTokenBridge({ - oldRefreshToken: refreshToken, - userId, - tenantId: bridgeUser.tenantId, - openidIssuer: bridgeUser.openidIssuer, - }); + if (bridgedRefreshToken) { + logger.info( + '[refreshController] Recovered via refresh-token bridge after invalid_grant', + { + userId, + }, + ); - if (bridgedRefreshToken) { - logger.info( - '[refreshController] Recovered via refresh-token bridge after invalid_grant', - { - userId, - }, - ); + // Retry with the recovered (rotated) refresh token + try { + const { + tokenset: retryTokenset, + claims: retryClaims, + openidIssuer: retryOpenidIssuer, + user: retryUser, + error: retryError, + } = await refreshOpenIDUser({ + refreshToken: bridgedRefreshToken, + strategyName: 'refreshController (bridge recovery)', + }); - // Retry with the recovered (rotated) refresh token - try { - const { - tokenset: retryTokenset, - claims: retryClaims, - openidIssuer: retryOpenidIssuer, - user: retryUser, - error: retryError, - } = await refreshOpenIDUser({ - refreshToken: bridgedRefreshToken, - strategyName: 'refreshController (bridge recovery)', - }); - - if (retryUser && !retryError) { - if (retryUser._id.toString() !== userId) { - logger.warn( - '[refreshController] Bridge recovery resolved a different user; refusing token issuance', - { - cookieUserId: userId, - resolvedUserId: retryUser._id.toString(), - }, - ); - return res.status(403).send('Invalid OpenID refresh token'); - } - - try { - /** - * Keep the stale-cookie bridge briefly so parallel /refresh requests that - * already sent the old cookie can recover too. Re-storing also shrinks the - * remaining replay window from REFRESH_TOKEN_EXPIRY (potentially days) to - * this short grace TTL while Mongo/expiresAt cleanup removes it. - */ - await storeRefreshTokenBridge({ - oldRefreshToken: refreshToken, - newRefreshToken: retryTokenset.refresh_token || bridgedRefreshToken, - userId, - tenantId: bridgeUser.tenantId, - openidIssuer: bridgeUser.openidIssuer, - ttl: OPENID_REFRESH_BRIDGE_GRACE_MS, - }); - } catch (graceError) { - logger.warn( - '[refreshController] Bridge grace-period storage failed after successful recovery', - graceError, - ); - } - return sendOpenIDAuthResponse({ - tokenset: retryTokenset, - user: retryUser, - existingRefreshToken: bridgedRefreshToken, - openidSubject: retryClaims?.sub, - openidIssuer: retryOpenidIssuer, - req, - res, - }); + if (retryUser && !retryError) { + if (retryUser._id.toString() !== userId) { + logger.warn( + '[refreshController] Bridge recovery resolved a different user; refusing token issuance', + { + cookieUserId: userId, + resolvedUserId: retryUser._id.toString(), + }, + ); + return res.status(403).send('Invalid OpenID refresh token'); } - } catch (retryError) { - logger.error('[refreshController] Bridge recovery retry failed', retryError); - // Fall through to generic error response + + try { + /** + * Keep the stale-cookie bridge briefly so parallel /refresh requests that + * already sent the old cookie can recover too. Re-storing also shrinks the + * remaining replay window from REFRESH_TOKEN_EXPIRY (potentially days) to + * this short grace TTL while Mongo/expiresAt cleanup removes it. + */ + await storeRefreshTokenBridge({ + oldRefreshToken: refreshToken, + newRefreshToken: retryTokenset.refresh_token || bridgedRefreshToken, + userId, + tenantId: bridgeUser.tenantId, + openidIssuer: bridgeUser.openidIssuer, + ttl: OPENID_REFRESH_BRIDGE_GRACE_MS, + }); + } catch (graceError) { + logger.warn( + '[refreshController] Bridge grace-period storage failed after successful recovery', + graceError, + ); + } + return sendOpenIDAuthResponse({ + tokenset: retryTokenset, + user: retryUser, + existingRefreshToken: bridgedRefreshToken, + openidSubject: retryClaims?.sub, + openidIssuer: retryOpenidIssuer, + req, + res, + }); } + } catch (retryError) { + logger.error('[refreshController] Bridge recovery retry failed', retryError); + // Fall through to generic error response } } - } catch (verifyError) { - logger.debug('[refreshController] Could not verify openid_user_id for bridge lookup', { - error: verifyError.message, + } catch (bridgeError) { + logger.debug('[refreshController] Refresh-token bridge lookup failed', { + error: bridgeError.message, }); } } diff --git a/api/server/controllers/AuthController.spec.js b/api/server/controllers/AuthController.spec.js index 9bf013730b..cd99f8e1c3 100644 --- a/api/server/controllers/AuthController.spec.js +++ b/api/server/controllers/AuthController.spec.js @@ -914,6 +914,17 @@ describe('refreshController – OpenID path', () => { expect(res.send).toHaveBeenCalledWith('Invalid OpenID refresh token'); }); + it('does not use the bridge when signed user-id cookie payload is invalid', async () => { + setOpenIDReuseCookies(jwt.sign({ id: 123 }, process.env.JWT_REFRESH_SECRET)); + openIdClient.refreshTokenGrant.mockRejectedValue(new Error('invalid_grant')); + + await refreshController(req, res); + + expect(getUserById).not.toHaveBeenCalled(); + expect(getRefreshTokenBridge).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(403); + }); + it('recovers stale refresh-token cookies and keeps a short grace bridge', async () => { setOpenIDReuseCookies(); req.session = {}; diff --git a/api/server/services/RefreshTokenBridge.js b/api/server/services/RefreshTokenBridge.js index 9f474a91b2..bd6cb005a2 100644 --- a/api/server/services/RefreshTokenBridge.js +++ b/api/server/services/RefreshTokenBridge.js @@ -103,7 +103,7 @@ async function storeRefreshTokenBridge({ * @param {string} args.oldRefreshToken — the token to look up (hashed for key) * @param {string} args.userId — current user._id (must match the bridged context) * @param {string} [args.tenantId] — current user.tenantId (optional but verified if present) - * @param {string} [args.openidIssuer] — current user.openidIssuer (optional but verified if present) + * @param {string} [args.openidIssuer] — current user.openidIssuer (must match stored issuer after normalization) * @returns {Promise} the rotated refresh token if found and valid, null otherwise */ async function getRefreshTokenBridge({ oldRefreshToken, userId, tenantId, openidIssuer }) { @@ -124,7 +124,13 @@ async function getRefreshTokenBridge({ oldRefreshToken, userId, tenantId, openid return null; } - if (bridge.openidIssuer && bridge.openidIssuer !== identity.openidIssuer) { + const bridgeIdentity = resolveBridgeIdentity({ + userId: bridge.userId, + tenantId: bridge.tenantId, + openidIssuer: bridge.openidIssuer, + }); + + if (!bridgeIdentity || bridgeIdentity.openidIssuer !== identity.openidIssuer) { logger.warn('[RefreshTokenBridge] Bridge lookup failed: issuer mismatch', { tokenHash: oldRefreshTokenHash, }); diff --git a/api/server/services/RefreshTokenBridge.spec.js b/api/server/services/RefreshTokenBridge.spec.js index 87f1b58dbf..631920aa45 100644 --- a/api/server/services/RefreshTokenBridge.spec.js +++ b/api/server/services/RefreshTokenBridge.spec.js @@ -165,5 +165,57 @@ describe('RefreshTokenBridge', () => { expect(result).toBeNull(); expect(decryptV2).not.toHaveBeenCalled(); }); + + it('returns null when only the expected issuer is present', async () => { + db.findRefreshTokenBridge.mockResolvedValue({ + encryptedNewRefreshToken: 'encrypted:rt-new', + userId: 'user-123', + createdAt: new Date(), + }); + + const result = await getRefreshTokenBridge({ + oldRefreshToken: 'rt-old', + userId: 'user-123', + openidIssuer: 'https://issuer.example.com', + }); + + expect(result).toBeNull(); + expect(decryptV2).not.toHaveBeenCalled(); + }); + + it('returns null when only the stored issuer is present', async () => { + db.findRefreshTokenBridge.mockResolvedValue({ + encryptedNewRefreshToken: 'encrypted:rt-new', + userId: 'user-123', + openidIssuer: 'https://issuer.example.com', + createdAt: new Date(), + }); + + const result = await getRefreshTokenBridge({ + oldRefreshToken: 'rt-old', + userId: 'user-123', + }); + + expect(result).toBeNull(); + expect(decryptV2).not.toHaveBeenCalled(); + }); + + it('normalizes the stored issuer before validation', async () => { + db.findRefreshTokenBridge.mockResolvedValue({ + encryptedNewRefreshToken: 'encrypted:rt-new', + userId: 'user-123', + openidIssuer: 'https://issuer.example.com/.well-known/openid-configuration', + createdAt: new Date(), + }); + + const result = await getRefreshTokenBridge({ + oldRefreshToken: 'rt-old', + userId: 'user-123', + openidIssuer: 'https://issuer.example.com/', + }); + + expect(decryptV2).toHaveBeenCalledWith('encrypted:rt-new'); + expect(result).toBe('rt-new'); + }); }); }); diff --git a/packages/data-schemas/src/methods/refreshTokenBridge.spec.ts b/packages/data-schemas/src/methods/refreshTokenBridge.spec.ts index 0b162916c3..9e211c7777 100644 --- a/packages/data-schemas/src/methods/refreshTokenBridge.spec.ts +++ b/packages/data-schemas/src/methods/refreshTokenBridge.spec.ts @@ -32,6 +32,18 @@ beforeEach(async () => { }); describe('RefreshTokenBridge Methods', () => { + it('keeps the lookup indexes aligned with the data-layer query shape', () => { + const indexKeys = mongoose.models.RefreshTokenBridge.schema.indexes().map(([key]) => key); + + expect(indexKeys).toContainEqual({ oldRefreshTokenHash: 1, userId: 1, tenantId: 1 }); + expect(indexKeys).not.toContainEqual({ + oldRefreshTokenHash: 1, + userId: 1, + tenantId: 1, + openidIssuer: 1, + }); + }); + it('upserts and finds a bridge by old token hash, user, and tenant', async () => { await methods.upsertRefreshTokenBridge({ oldRefreshTokenHash: 'old-hash', diff --git a/packages/data-schemas/src/methods/refreshTokenBridge.ts b/packages/data-schemas/src/methods/refreshTokenBridge.ts index b9fbab992a..038563c4d3 100644 --- a/packages/data-schemas/src/methods/refreshTokenBridge.ts +++ b/packages/data-schemas/src/methods/refreshTokenBridge.ts @@ -24,11 +24,14 @@ export function createRefreshTokenBridgeMethods(mongoose: typeof import('mongoos ) => Promise; findRefreshTokenBridge: (query: RefreshTokenBridgeQuery) => Promise; } { + const getRefreshTokenBridgeModel = () => + mongoose.models.RefreshTokenBridge as Model; + async function upsertRefreshTokenBridge( bridgeData: RefreshTokenBridgeCreateData, ): Promise { try { - const RefreshTokenBridge = mongoose.models.RefreshTokenBridge as Model; + const RefreshTokenBridge = getRefreshTokenBridgeModel(); const filter = bridgeFilter(bridgeData); const update: UpdateQuery = { $set: { @@ -58,7 +61,7 @@ export function createRefreshTokenBridgeMethods(mongoose: typeof import('mongoos query: RefreshTokenBridgeQuery, ): Promise { try { - const RefreshTokenBridge = mongoose.models.RefreshTokenBridge as Model; + const RefreshTokenBridge = getRefreshTokenBridgeModel(); return await RefreshTokenBridge.findOne({ ...bridgeFilter(query), expiresAt: { $gt: new Date() }, diff --git a/packages/data-schemas/src/schema/refreshTokenBridge.ts b/packages/data-schemas/src/schema/refreshTokenBridge.ts index 31e095b8f5..a2634c9666 100644 --- a/packages/data-schemas/src/schema/refreshTokenBridge.ts +++ b/packages/data-schemas/src/schema/refreshTokenBridge.ts @@ -38,11 +38,5 @@ refreshTokenBridgeSchema.index( { oldRefreshTokenHash: 1, userId: 1, tenantId: 1 }, { unique: true }, ); -refreshTokenBridgeSchema.index({ - oldRefreshTokenHash: 1, - userId: 1, - tenantId: 1, - openidIssuer: 1, -}); export default refreshTokenBridgeSchema; diff --git a/packages/data-schemas/src/types/refreshTokenBridge.ts b/packages/data-schemas/src/types/refreshTokenBridge.ts index 10b23ca56d..9acd1f5975 100644 --- a/packages/data-schemas/src/types/refreshTokenBridge.ts +++ b/packages/data-schemas/src/types/refreshTokenBridge.ts @@ -23,5 +23,4 @@ export interface RefreshTokenBridgeQuery { oldRefreshTokenHash: string; userId: string; tenantId?: string; - openidIssuer?: string; }