fix: improve token expiration logic to prefer JWT expiry over stale config (#10)

* fix: improve token expiration logic to prefer JWT expiry over stale config

* remove expiresAt handling from auth session and token logic

Author: anserwaseem

src/auth/session.ts

@@ -3,7 +3,7 @@ import {
   writeGlobalConfig,
   type EnsembleUserConfig,
 } from '../config/globalConfig.js';
-import { decodeIdTokenClaims, getIdTokenExpiryMs, isTokenExpired } from './token.js';
+import { decodeIdTokenClaims, isTokenExpired } from './token.js';
 import { getEnsembleFirebaseApiKey } from '../config/env.js';
 
 const DEFAULT_REFRESH_API_BASE = 'https://securetoken.googleapis.com/v1/token';
@@ -37,7 +37,6 @@ async function refreshIdToken(refreshToken: string): Promise<{
   idToken: string;
   refreshToken: string;
   userId?: string;
-  expiresAt?: number;
 }> {
   const apiKey = getEnsembleFirebaseApiKey();
   if (!apiKey) {
@@ -62,17 +61,10 @@ async function refreshIdToken(refreshToken: string): Promise<{
     throw new Error(`Token refresh failed: ${reason}`);
   }
 
-  const expiresInSec = Number(data.expires_in);
-  const expiresAt =
-    Number.isFinite(expiresInSec) && expiresInSec > 0
-      ? Date.now() + expiresInSec * 1000
-      : getIdTokenExpiryMs(data.id_token);
-
   return {
     idToken: data.id_token,
     refreshToken: data.refresh_token ?? refreshToken,
     userId: data.user_id,
-    expiresAt,
   };
 }
 
@@ -124,7 +116,7 @@ export async function getValidAuthSession(): Promise<AuthSessionResult> {
     };
   }
 
-  if (!isTokenExpired(user.idToken, user.expiresAt)) {
+  if (!isTokenExpired(user.idToken)) {
     return {
       ok: true,
       idToken: user.idToken,
@@ -152,7 +144,6 @@ export async function getValidAuthSession(): Promise<AuthSessionResult> {
       email: claims.email ?? user.email,
       idToken: refreshed.idToken,
       refreshToken: refreshed.refreshToken,
-      expiresAt: refreshed.expiresAt,
     };
     const updatedConfig: EnsembleUserConfig = {
       ...config,

src/auth/token.ts

@@ -40,33 +40,8 @@ export function getIdTokenExpiryMs(idToken: string): number | undefined {
   return typeof exp === 'number' ? exp * 1000 : undefined;
 }
 
-export function normalizeExpiresAt(expiresAt: unknown): number | undefined {
-  if (typeof expiresAt === 'number' && Number.isFinite(expiresAt)) {
-    // Accept either epoch seconds or epoch milliseconds.
-    return expiresAt > 1_000_000_000_000 ? expiresAt : expiresAt * 1000;
-  }
-
-  if (typeof expiresAt === 'string') {
-    const trimmed = expiresAt.trim();
-    if (!trimmed) return undefined;
-
-    // Numeric strings are supported too.
-    const asNumber = Number(trimmed);
-    if (Number.isFinite(asNumber)) {
-      return asNumber > 1_000_000_000_000 ? asNumber : asNumber * 1000;
-    }
-
-    const dateMs = Date.parse(trimmed);
-    if (!Number.isNaN(dateMs)) {
-      return dateMs;
-    }
-  }
-
-  return undefined;
-}
-
-export function isTokenExpired(idToken: string, expiresAt?: number, bufferSeconds = 60): boolean {
-  const expiry = normalizeExpiresAt(expiresAt) ?? getIdTokenExpiryMs(idToken);
+export function isTokenExpired(idToken: string, bufferSeconds = 60): boolean {
+  const expiry = getIdTokenExpiryMs(idToken);
   if (expiry === undefined) return true;
   return expiry <= Date.now() + bufferSeconds * 1000;
 }

src/commands/login.ts

@@ -9,12 +9,7 @@ import {
   writeGlobalConfig,
   type EnsembleUserConfig,
 } from '../config/globalConfig.js';
-import {
-  decodeIdTokenClaims,
-  getIdTokenExpiryMs,
-  isTokenExpired,
-  normalizeExpiresAt,
-} from '../auth/token.js';
+import { decodeIdTokenClaims, isTokenExpired } from '../auth/token.js';
 import { resolveVerboseFlag } from '../core/cliError.js';
 import { getEnsembleAuthBaseUrl } from '../config/env.js';
 import { ui } from '../core/ui.js';
@@ -61,20 +56,19 @@ export async function loginCommand(options: LoginOptions = {}): Promise<void> {
   const idToken = existing?.user?.idToken;
   if (idToken && !isTokenExpired(idToken)) {
     const claims = decodeIdTokenClaims(idToken);
-    const normalizedExpiresAt = existing.user?.expiresAt ?? getIdTokenExpiryMs(idToken);
     const mergedUser = {
-      ...(existing.user ?? { uid: claims.uid ?? 'cli-user', idToken }),
       uid: existing.user?.uid ?? claims.uid ?? 'cli-user',
       name: existing.user?.name ?? claims.name ?? undefined,
       email: existing.user?.email ?? claims.email ?? undefined,
       idToken,
-      expiresAt: normalizedExpiresAt,
+      refreshToken: existing.user?.refreshToken,
     };
 
     if (
+      existing.user?.uid !== mergedUser.uid ||
       existing.user?.name !== mergedUser.name ||
       existing.user?.email !== mergedUser.email ||
-      existing.user?.expiresAt !== mergedUser.expiresAt
+      existing.user?.refreshToken !== mergedUser.refreshToken
     ) {
       await writeGlobalConfig({
         ...existing,
@@ -101,7 +95,6 @@ export async function loginCommand(options: LoginOptions = {}): Promise<void> {
   const tokenPromise = new Promise<{
     token: string;
     refreshToken?: string;
-    expiresAt?: number;
   }>((resolve, reject) => {
     const timeout = setTimeout(() => {
       server.close();
@@ -129,7 +122,6 @@ export async function loginCommand(options: LoginOptions = {}): Promise<void> {
             const data = JSON.parse(body) as {
               token?: string;
               refreshToken?: string;
-              expiresAt?: number | string;
               state?: string;
             };
             // Some auth providers may not echo back our `cliState` in the callback payload.
@@ -145,7 +137,6 @@ export async function loginCommand(options: LoginOptions = {}): Promise<void> {
               resolve({
                 token: data.token,
                 refreshToken: typeof data.refreshToken === 'string' ? data.refreshToken : undefined,
-                expiresAt: normalizeExpiresAt(data.expiresAt),
               });
             } else {
               res.writeHead(400, {
@@ -191,7 +182,6 @@ export async function loginCommand(options: LoginOptions = {}): Promise<void> {
   let callbackData: {
     token: string;
     refreshToken?: string;
-    expiresAt?: number;
   };
   try {
     callbackData = await tokenPromise;
@@ -203,7 +193,6 @@ export async function loginCommand(options: LoginOptions = {}): Promise<void> {
 
   const token = callbackData.token;
   const { uid, name, email } = decodeIdTokenClaims(token);
-  const expiresAt = callbackData.expiresAt ?? getIdTokenExpiryMs(token);
 
   const newConfig: EnsembleUserConfig = {
     ...existing,
@@ -213,7 +202,6 @@ export async function loginCommand(options: LoginOptions = {}): Promise<void> {
       email: email ?? undefined,
       idToken: token,
       refreshToken: callbackData.refreshToken,
-      expiresAt,
     },
   };
 

src/config/globalConfig.ts

@@ -9,7 +9,6 @@ export interface EnsembleUserConfig {
     email?: string;
     idToken: string;
     refreshToken?: string;
-    expiresAt?: number;
   };
   [key: string]: unknown;
 }

tests/auth/session.test.ts

@@ -182,6 +182,38 @@ describe('getValidAuthSession', () => {
     }
   });
 
+  it('returns ok without refresh when jwt is valid even if legacy config has stale expiresAt', async () => {
+    const token = makeJwt({
+      userId: 'u1',
+      email: 'a@b.com',
+      exp: Math.floor(Date.now() / 1000) + 3600,
+    });
+    vi.mocked(globalConfig.readGlobalConfig).mockResolvedValue({
+      user: {
+        uid: 'u1',
+        email: 'a@b.com',
+        idToken: token,
+        refreshToken: 'refresh-123',
+        expiresAt: Date.now() - 3600_000,
+      },
+    } as EnsembleUserConfig);
+
+    const fetchMock = vi.fn();
+    const originalFetch = globalThis.fetch;
+    globalThis.fetch = fetchMock;
+
+    const result = await getValidAuthSession();
+
+    globalThis.fetch = originalFetch;
+
+    expect(result.ok).toBe(true);
+    if (result.ok) {
+      expect(result.idToken).toBe(token);
+      expect(result.refreshed).toBe(false);
+    }
+    expect(fetchMock).not.toHaveBeenCalled();
+  });
+
   it('refreshes token when expired and refresh token exists', async () => {
     const oldToken = makeJwt({
       userId: 'u1',

tests/auth/token.test.ts

@@ -1,10 +1,5 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
-import {
-  decodeIdTokenClaims,
-  getIdTokenExpiryMs,
-  normalizeExpiresAt,
-  isTokenExpired,
-} from '../../src/auth/token.js';
+import { decodeIdTokenClaims, getIdTokenExpiryMs, isTokenExpired } from '../../src/auth/token.js';
 
 function base64urlEncode(str: string): string {
   return Buffer.from(str, 'utf8')
@@ -86,35 +81,6 @@ describe('getIdTokenExpiryMs', () => {
   });
 });
 
-describe('normalizeExpiresAt', () => {
-  it('accepts epoch milliseconds as-is', () => {
-    const ms = 1735689600000;
-    expect(normalizeExpiresAt(ms)).toBe(ms);
-  });
-
-  it('converts epoch seconds to milliseconds', () => {
-    expect(normalizeExpiresAt(1735689600)).toBe(1735689600000);
-  });
-
-  it('accepts numeric string (seconds)', () => {
-    expect(normalizeExpiresAt('1735689600')).toBe(1735689600000);
-  });
-
-  it('accepts numeric string (milliseconds)', () => {
-    expect(normalizeExpiresAt('1735689600000')).toBe(1735689600000);
-  });
-
-  it('returns undefined for empty string', () => {
-    expect(normalizeExpiresAt('')).toBeUndefined();
-    expect(normalizeExpiresAt('   ')).toBeUndefined();
-  });
-
-  it('returns undefined for invalid input', () => {
-    expect(normalizeExpiresAt(null)).toBeUndefined();
-    expect(normalizeExpiresAt(undefined)).toBeUndefined();
-  });
-});
-
 describe('isTokenExpired', () => {
   beforeEach(() => {
     vi.useFakeTimers();
@@ -142,14 +108,13 @@ describe('isTokenExpired', () => {
     vi.setSystemTime(new Date('2025-01-01T12:00:30Z'));
     // exp = 2025-01-01 13:00 UTC; with 60s buffer, token still valid at 12:00:30
     const token = makeJwt({ userId: 'u1', exp: 1735736400 });
-    expect(isTokenExpired(token, undefined, 60)).toBe(false);
-    expect(isTokenExpired(token, undefined, 0)).toBe(false);
+    expect(isTokenExpired(token, 60)).toBe(false);
+    expect(isTokenExpired(token, 0)).toBe(false);
   });
 
-  it('uses expiresAt when provided', () => {
+  it('returns true when jwt has no exp claim', () => {
     vi.setSystemTime(new Date('2025-01-01T12:00:00Z'));
     const token = makeJwt({ userId: 'u1' });
-    const futureMs = new Date('2025-06-01').getTime();
-    expect(isTokenExpired(token, futureMs)).toBe(false);
+    expect(isTokenExpired(token)).toBe(true);
   });
 });