fix: auth refresh token (#1299)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

packages/tm-core/src/auth/auth-manager.spec.ts

@@ -21,11 +21,16 @@ const CredentialStoreSpy = vi.fn();
 vi.mock('./credential-store.js', () => {
 	return {
 		CredentialStore: class {
+			static getInstance(config?: any) {
+				return new (this as any)(config);
+			}
+			static resetInstance() {
+				// Mock reset instance method
+			}
 			constructor(config: any) {
 				CredentialStoreSpy(config);
-				this.getCredentials = vi.fn(() => null);
 			}
-			getCredentials() {
+			getCredentials(_options?: any) {
 				return null;
 			}
 			saveCredentials() {}
@@ -85,7 +90,7 @@ describe('AuthManager Singleton', () => {
 		expect(instance1).toBe(instance2);
 	});
 
-	it('should use config on first call', () => {
+	it('should use config on first call', async () => {
 		const config = {
 			baseUrl: 'https://test.auth.com',
 			configDir: '/test/config',
@@ -101,7 +106,7 @@ describe('AuthManager Singleton', () => {
 
 		// Verify the config is passed to internal components through observable behavior
 		// getCredentials would look in the configured file path
-		const credentials = instance.getCredentials();
+		const credentials = await instance.getCredentials();
 		expect(credentials).toBeNull(); // File doesn't exist, but config was propagated correctly
 	});
 

packages/tm-core/src/auth/auth-manager.ts

@@ -29,6 +29,8 @@ export class AuthManager {
 	private oauthService: OAuthService;
 	private supabaseClient: SupabaseAuthClient;
 	private organizationService?: OrganizationService;
+	private logger = getLogger('AuthManager');
+	private refreshPromise: Promise<AuthCredentials> | null = null;
 
 	private constructor(config?: Partial<AuthConfig>) {
 		this.credentialStore = CredentialStore.getInstance(config);
@@ -36,7 +38,10 @@ export class AuthManager {
 		this.oauthService = new OAuthService(this.credentialStore, config);
 
 		// Initialize Supabase client with session restoration
-		this.initializeSupabaseSession();
+		// Fire-and-forget with catch handler to prevent unhandled rejections
+		this.initializeSupabaseSession().catch(() => {
+			// Errors are already logged in initializeSupabaseSession
+		});
 	}
 
 	/**
@@ -78,8 +83,60 @@ export class AuthManager {
 
 	/**
 	 * Get stored authentication credentials
+	 * Automatically refreshes the token if expired
 	 */
-	getCredentials(): AuthCredentials | null {
+	async getCredentials(): Promise<AuthCredentials | null> {
+		const credentials = this.credentialStore.getCredentials({
+			allowExpired: true
+		});
+
+		if (!credentials) {
+			return null;
+		}
+
+		// Check if credentials are expired (with 30-second clock skew buffer)
+		const CLOCK_SKEW_MS = 30_000;
+		const isExpired = credentials.expiresAt
+			? new Date(credentials.expiresAt).getTime() <= Date.now() + CLOCK_SKEW_MS
+			: false;
+
+		// If expired and we have a refresh token, attempt refresh
+		if (isExpired && credentials.refreshToken) {
+			// Return existing refresh promise if one is in progress
+			if (this.refreshPromise) {
+				try {
+					return await this.refreshPromise;
+				} catch {
+					return null;
+				}
+			}
+
+			try {
+				this.logger.info('Token expired, attempting automatic refresh...');
+				this.refreshPromise = this.refreshToken();
+				const result = await this.refreshPromise;
+				return result;
+			} catch (error) {
+				this.logger.warn('Automatic token refresh failed:', error);
+				return null;
+			} finally {
+				this.refreshPromise = null;
+			}
+		}
+
+		// Return null if expired and no refresh token
+		if (isExpired) {
+			return null;
+		}
+
+		return credentials;
+	}
+
+	/**
+	 * Get stored authentication credentials (synchronous version)
+	 * Does not attempt automatic refresh
+	 */
+	getCredentialsSync(): AuthCredentials | null {
 		return this.credentialStore.getCredentials();
 	}
 
@@ -171,8 +228,8 @@ export class AuthManager {
 	/**
 	 * Get the current user context (org/brief selection)
 	 */
-	getContext(): UserContext | null {
-		const credentials = this.getCredentials();
+	async getContext(): Promise<UserContext | null> {
+		const credentials = await this.getCredentials();
 		return credentials?.selectedContext || null;
 	}
 
@@ -180,7 +237,7 @@ export class AuthManager {
 	 * Update the user context (org/brief selection)
 	 */
 	async updateContext(context: Partial<UserContext>): Promise<void> {
-		const credentials = this.getCredentials();
+		const credentials = await this.getCredentials();
 		if (!credentials) {
 			throw new AuthenticationError('Not authenticated', 'NOT_AUTHENTICATED');
 		}
@@ -206,7 +263,7 @@ export class AuthManager {
 	 * Clear the user context
 	 */
 	async clearContext(): Promise<void> {
-		const credentials = this.getCredentials();
+		const credentials = await this.getCredentials();
 		if (!credentials) {
 			throw new AuthenticationError('Not authenticated', 'NOT_AUTHENTICATED');
 		}
@@ -223,7 +280,7 @@ export class AuthManager {
 	private async getOrganizationService(): Promise<OrganizationService> {
 		if (!this.organizationService) {
 			// First check if we have credentials with a token
-			const credentials = this.getCredentials();
+			const credentials = await this.getCredentials();
 			if (!credentials || !credentials.token) {
 				throw new AuthenticationError('Not authenticated', 'NOT_AUTHENTICATED');
 			}

packages/tm-core/src/auth/credential-store.spec.ts

@@ -0,0 +1,289 @@
+/**
+ * @fileoverview Unit tests for CredentialStore token expiration handling
+ */
+
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+import fs from 'fs';
+import path from 'path';
+import os from 'os';
+import { CredentialStore } from './credential-store';
+import type { AuthCredentials } from './types';
+
+describe('CredentialStore - Token Expiration', () => {
+	let credentialStore: CredentialStore;
+	let tmpDir: string;
+	let authFile: string;
+
+	beforeEach(() => {
+		// Create temp directory for test credentials
+		tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'tm-cred-test-'));
+		authFile = path.join(tmpDir, 'auth.json');
+
+		// Create instance with test config
+		CredentialStore.resetInstance();
+		credentialStore = CredentialStore.getInstance({
+			configDir: tmpDir,
+			configFile: authFile
+		});
+	});
+
+	afterEach(() => {
+		// Clean up
+		try {
+			if (fs.existsSync(tmpDir)) {
+				fs.rmSync(tmpDir, { recursive: true, force: true });
+			}
+		} catch {
+			// Ignore cleanup errors
+		}
+		CredentialStore.resetInstance();
+	});
+
+	describe('Expiration Detection', () => {
+		it('should return null for expired token', () => {
+			const expiredCredentials: AuthCredentials = {
+				token: 'expired-token',
+				refreshToken: 'refresh-token',
+				userId: 'test-user',
+				email: 'test@example.com',
+				expiresAt: new Date(Date.now() - 60000).toISOString(), // 1 minute ago
+				savedAt: new Date().toISOString()
+			};
+
+			credentialStore.saveCredentials(expiredCredentials);
+
+			const retrieved = credentialStore.getCredentials();
+
+			expect(retrieved).toBeNull();
+		});
+
+		it('should return credentials for valid token', () => {
+			const validCredentials: AuthCredentials = {
+				token: 'valid-token',
+				refreshToken: 'refresh-token',
+				userId: 'test-user',
+				email: 'test@example.com',
+				expiresAt: new Date(Date.now() + 3600000).toISOString(), // 1 hour from now
+				savedAt: new Date().toISOString()
+			};
+
+			credentialStore.saveCredentials(validCredentials);
+
+			const retrieved = credentialStore.getCredentials();
+
+			expect(retrieved).not.toBeNull();
+			expect(retrieved?.token).toBe('valid-token');
+		});
+
+		it('should return expired token when allowExpired is true', () => {
+			const expiredCredentials: AuthCredentials = {
+				token: 'expired-token',
+				refreshToken: 'refresh-token',
+				userId: 'test-user',
+				email: 'test@example.com',
+				expiresAt: new Date(Date.now() - 60000).toISOString(),
+				savedAt: new Date().toISOString()
+			};
+
+			credentialStore.saveCredentials(expiredCredentials);
+
+			const retrieved = credentialStore.getCredentials({ allowExpired: true });
+
+			expect(retrieved).not.toBeNull();
+			expect(retrieved?.token).toBe('expired-token');
+		});
+	});
+
+	describe('Clock Skew Tolerance', () => {
+		it('should reject token expiring within 30-second buffer', () => {
+			// Token expires in 15 seconds (within 30-second buffer)
+			const almostExpiredCredentials: AuthCredentials = {
+				token: 'almost-expired-token',
+				refreshToken: 'refresh-token',
+				userId: 'test-user',
+				email: 'test@example.com',
+				expiresAt: new Date(Date.now() + 15000).toISOString(),
+				savedAt: new Date().toISOString()
+			};
+
+			credentialStore.saveCredentials(almostExpiredCredentials);
+
+			const retrieved = credentialStore.getCredentials();
+
+			expect(retrieved).toBeNull();
+		});
+
+		it('should accept token expiring outside 30-second buffer', () => {
+			// Token expires in 60 seconds (outside 30-second buffer)
+			const validCredentials: AuthCredentials = {
+				token: 'valid-token',
+				refreshToken: 'refresh-token',
+				userId: 'test-user',
+				email: 'test@example.com',
+				expiresAt: new Date(Date.now() + 60000).toISOString(),
+				savedAt: new Date().toISOString()
+			};
+
+			credentialStore.saveCredentials(validCredentials);
+
+			const retrieved = credentialStore.getCredentials();
+
+			expect(retrieved).not.toBeNull();
+			expect(retrieved?.token).toBe('valid-token');
+		});
+	});
+
+	describe('Timestamp Format Handling', () => {
+		it('should handle ISO string timestamps', () => {
+			const credentials: AuthCredentials = {
+				token: 'test-token',
+				refreshToken: 'refresh-token',
+				userId: 'test-user',
+				email: 'test@example.com',
+				expiresAt: new Date(Date.now() + 3600000).toISOString(),
+				savedAt: new Date().toISOString()
+			};
+
+			credentialStore.saveCredentials(credentials);
+
+			const retrieved = credentialStore.getCredentials();
+
+			expect(retrieved).not.toBeNull();
+			expect(typeof retrieved?.expiresAt).toBe('number'); // Normalized to number
+		});
+
+		it('should handle numeric timestamps', () => {
+			const credentials: AuthCredentials = {
+				token: 'test-token',
+				refreshToken: 'refresh-token',
+				userId: 'test-user',
+				email: 'test@example.com',
+				expiresAt: Date.now() + 3600000,
+				savedAt: new Date().toISOString()
+			};
+
+			credentialStore.saveCredentials(credentials);
+
+			const retrieved = credentialStore.getCredentials();
+
+			expect(retrieved).not.toBeNull();
+			expect(typeof retrieved?.expiresAt).toBe('number');
+		});
+
+		it('should return null for invalid timestamp format', () => {
+			// Manually write invalid timestamp to file
+			const invalidCredentials = {
+				token: 'test-token',
+				refreshToken: 'refresh-token',
+				userId: 'test-user',
+				email: 'test@example.com',
+				expiresAt: 'invalid-date',
+				savedAt: new Date().toISOString()
+			};
+
+			fs.writeFileSync(authFile, JSON.stringify(invalidCredentials), {
+				mode: 0o600
+			});
+
+			const retrieved = credentialStore.getCredentials();
+
+			expect(retrieved).toBeNull();
+		});
+
+		it('should return null for missing expiresAt', () => {
+			const credentialsWithoutExpiry = {
+				token: 'test-token',
+				refreshToken: 'refresh-token',
+				userId: 'test-user',
+				email: 'test@example.com',
+				savedAt: new Date().toISOString()
+			};
+
+			fs.writeFileSync(authFile, JSON.stringify(credentialsWithoutExpiry), {
+				mode: 0o600
+			});
+
+			const retrieved = credentialStore.getCredentials();
+
+			expect(retrieved).toBeNull();
+		});
+	});
+
+	describe('Storage Persistence', () => {
+		it('should persist expiresAt as ISO string', () => {
+			const expiryTime = Date.now() + 3600000;
+			const credentials: AuthCredentials = {
+				token: 'test-token',
+				refreshToken: 'refresh-token',
+				userId: 'test-user',
+				email: 'test@example.com',
+				expiresAt: expiryTime,
+				savedAt: new Date().toISOString()
+			};
+
+			credentialStore.saveCredentials(credentials);
+
+			// Read raw file to verify format
+			const fileContent = fs.readFileSync(authFile, 'utf-8');
+			const parsed = JSON.parse(fileContent);
+
+			// Should be stored as ISO string
+			expect(typeof parsed.expiresAt).toBe('string');
+			expect(parsed.expiresAt).toMatch(/^\d{4}-\d{2}-\d{2}T/); // ISO format
+		});
+
+		it('should normalize timestamp on retrieval', () => {
+			const credentials: AuthCredentials = {
+				token: 'test-token',
+				refreshToken: 'refresh-token',
+				userId: 'test-user',
+				email: 'test@example.com',
+				expiresAt: new Date(Date.now() + 3600000).toISOString(),
+				savedAt: new Date().toISOString()
+			};
+
+			credentialStore.saveCredentials(credentials);
+
+			const retrieved = credentialStore.getCredentials();
+
+			// Should be normalized to number for runtime use
+			expect(typeof retrieved?.expiresAt).toBe('number');
+		});
+	});
+
+	describe('hasValidCredentials', () => {
+		it('should return false for expired credentials', () => {
+			const expiredCredentials: AuthCredentials = {
+				token: 'expired-token',
+				refreshToken: 'refresh-token',
+				userId: 'test-user',
+				email: 'test@example.com',
+				expiresAt: new Date(Date.now() - 60000).toISOString(),
+				savedAt: new Date().toISOString()
+			};
+
+			credentialStore.saveCredentials(expiredCredentials);
+
+			expect(credentialStore.hasValidCredentials()).toBe(false);
+		});
+
+		it('should return true for valid credentials', () => {
+			const validCredentials: AuthCredentials = {
+				token: 'valid-token',
+				refreshToken: 'refresh-token',
+				userId: 'test-user',
+				email: 'test@example.com',
+				expiresAt: new Date(Date.now() + 3600000).toISOString(),
+				savedAt: new Date().toISOString()
+			};
+
+			credentialStore.saveCredentials(validCredentials);
+
+			expect(credentialStore.hasValidCredentials()).toBe(true);
+		});
+
+		it('should return false when no credentials exist', () => {
+			expect(credentialStore.hasValidCredentials()).toBe(false);
+		});
+	});
+});