⏳ fix: Keep Elapsed Expiries Elapsed and Revoke the Superseded Session

Two of the five findings from the Codex pass on fcdc15885 — the two that are
defects in code this branch introduced rather than design questions about the
bridge.

`getSkewedTokenExpiresAtMs` floored every result at a second in the future,
including an expiry the provider had already declared elapsed. An exchange
answering `expires_in: 0` or a past `expires_at` was handed to the MCP
connection stamped valid for another second, which only moves the failure
downstream. The floor now applies to a lifetime that is still live, which is
what it was for; an elapsed one stays elapsed so the caller rejects it. Same
for the cache TTL, which falls back to the elapsed-credential floor.

Bridge recovery left the stale token's durable Session behind. Only the token
it recovered through was passed as `existingRefreshToken`, so that one's
session was replaced while the token the browser actually presented kept its
record until its original expiry. That record, with the marker cookie still
bound to it, is what authorizes local image access for OpenID users — so a
copy of the stale cookie outlived the rotation it had lost. Revoked
explicitly on successful recovery.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SxKWxwqxAGckYpRsYTqx3F

api/server/controllers/AuthController.js

@@ -2,7 +2,7 @@ const cookies = require('cookie');
 const jwt = require('jsonwebtoken');
 const crypto = require('node:crypto');
 const openIdClient = require('openid-client');
-const { logger } = require('@librechat/data-schemas');
+const { logger, runAsSystem } = require('@librechat/data-schemas');
 const {
   math,
   isEnabled,
@@ -24,6 +24,7 @@ const {
 } = require('~/server/services/AuthService');
 const {
   deleteAllUserSessions,
+  deleteSession,
   getUserById,
   findSession,
   updateUser,
@@ -183,6 +184,7 @@ const sendOpenIDAuthResponse = async ({
   tokenset,
   user,
   existingRefreshToken,
+  revokedRefreshToken,
   openidSubject,
   openidIssuer,
   req,
@@ -201,6 +203,23 @@ const sendOpenIDAuthResponse = async ({
     user.tenantId,
     existingRefreshToken,
   );
+  /**
+   * Bridge recovery replaces a token the IdP already rejected, and only the token it recovered
+   * through is passed above. The stale one the browser presented keeps its own durable Session
+   * until its original expiry otherwise, and that record — with the marker cookie still bound to
+   * it — is what authorizes local image access, so a copy of that cookie would outlive the
+   * rotation it lost. Revoked explicitly here.
+   */
+  if (revokedRefreshToken && revokedRefreshToken !== existingRefreshToken) {
+    try {
+      await runAsSystem(() => deleteSession({ refreshToken: revokedRefreshToken }));
+    } catch (error) {
+      logger.warn(
+        '[refreshController] Failed to revoke the superseded refresh-token session',
+        error,
+      );
+    }
+  }
   const token = setOpenIDAuthTokens(tokenset, req, res, {
     userId,
     existingRefreshToken,
@@ -450,6 +469,7 @@ const refreshController = async (req, res) => {
                     tokenset: retryTokenset,
                     user: retryUser,
                     existingRefreshToken: bridgedRefreshToken,
+                    revokedRefreshToken: refreshToken,
                     openidSubject: retryClaims?.sub,
                     openidIssuer: retryOpenidIssuer,
                     req,

api/server/controllers/AuthController.spec.js

@@ -1,5 +1,6 @@
 jest.mock('@librechat/data-schemas', () => ({
   logger: { error: jest.fn(), debug: jest.fn(), warn: jest.fn(), info: jest.fn() },
+  runAsSystem: (callback) => callback(),
 }));
 jest.mock('~/server/services/GraphTokenService', () => ({
   getGraphApiToken: jest.fn(),
@@ -17,6 +18,7 @@ jest.mock('~/strategies', () => ({ getOpenIdConfig: jest.fn(), getOpenIdEmail: j
 jest.mock('openid-client', () => ({ refreshTokenGrant: jest.fn() }));
 jest.mock('~/models', () => ({
   deleteAllUserSessions: jest.fn(),
+  deleteSession: jest.fn(),
   getUserById: jest.fn(),
   findSession: jest.fn(),
   updateUser: jest.fn(),
@@ -84,7 +86,7 @@ const {
   setAuthTokens,
 } = require('~/server/services/AuthService');
 const { getOpenIdConfig, getOpenIdEmail } = require('~/strategies');
-const { getUserById, findSession, updateUser } = require('~/models');
+const { getUserById, findSession, updateUser, deleteSession } = require('~/models');
 const {
   getRefreshTokenBridge,
   storeRefreshTokenBridge,
@@ -1040,6 +1042,9 @@ describe('refreshController – OpenID path', () => {
       'tenant-1',
       'bridged-refresh',
     );
+    /** The stale token the browser presented is dead upstream, but its durable Session — and the
+     *  marker bound to it — would otherwise keep authorizing local image access until expiry. */
+    expect(deleteSession).toHaveBeenCalledWith({ refreshToken: 'stored-refresh' });
     const lookupIdentity = getRefreshTokenBridge.mock.calls[0][0];
     const graceIdentity = storeRefreshTokenBridge.mock.calls[0][0];
     expect(graceIdentity).toEqual(

packages/api/src/oauth/expiry.spec.ts

@@ -211,12 +211,20 @@ describe('skew helpers', () => {
     expect(getSkewedTokenCacheTtlMs(expiresAt, now)).toBe(120_000 - bufferMs);
   });
 
-  it('floors a lifetime shorter than the buffer to a usable minimum, never 0', () => {
+  it('floors a live lifetime shorter than the buffer to a usable minimum, never 0', () => {
     const expiresAt = now + 10_000;
 
     expect(getSkewedTokenExpiresAtMs(expiresAt, now)).toBe(now + 1000);
     expect(getSkewedTokenCacheTtlMs(expiresAt, now)).toBe(1000);
-    expect(getSkewedTokenCacheTtlMs(now - 60_000, now)).toBe(1000);
+  });
+
+  /** The floor exists to keep a short-but-real credential usable, not to revive a dead one: a
+   *  provider that declares an elapsed expiry must not have it stamped into the future. */
+  it('leaves an already-elapsed expiry elapsed', () => {
+    expect(getSkewedTokenExpiresAtMs(now - 60_000, now)).toBe(now - 60_000);
+    expect(getSkewedTokenExpiresAtMs(now, now)).toBe(now);
+    expect(getSkewedTokenCacheTtlMs(now - 60_000, now)).toBe(1);
+    expect(getSkewedTokenCacheTtlMs(now, now)).toBe(1);
   });
 });
 

packages/api/src/oauth/expiry.ts

@@ -129,11 +129,20 @@ export function getTokenExpiresAtMs({
  * buffer, so a credential handed on with this stamp cannot be accepted into its final seconds.
  */
 export function getSkewedTokenExpiresAtMs(expiresAt: number, now: number): number {
+  /** An expiry already in the past is the provider saying the credential is dead. Flooring it to a
+   *  moment in the future would hand a consumer a token that cannot work, so it stays elapsed and
+   *  the caller rejects the exchange instead of failing downstream. */
+  if (expiresAt <= now) {
+    return expiresAt;
+  }
   return Math.max(now + MIN_LIVE_TOKEN_TTL_MS, expiresAt - OPENID_EXPIRY_BUFFER_SECONDS * 1000);
 }
 
 /** Cache TTL for a token whose absolute expiry is already known, buffered as above. */
 export function getSkewedTokenCacheTtlMs(expiresAt: number, now: number): number {
+  if (expiresAt <= now) {
+    return EXPIRED_CACHE_TTL_MS;
+  }
   return Math.max(MIN_LIVE_TOKEN_TTL_MS, expiresAt - now - OPENID_EXPIRY_BUFFER_SECONDS * 1000);
 }