diff --git a/packages/tm-core/src/auth/auth-manager.test.ts b/packages/tm-core/src/auth/auth-manager.test.ts index e7238921..04178764 100644 --- a/packages/tm-core/src/auth/auth-manager.test.ts +++ b/packages/tm-core/src/auth/auth-manager.test.ts @@ -21,6 +21,7 @@ describe('AuthManager Singleton', () => { beforeEach(() => { // Reset singleton before each test AuthManager.resetInstance(); + vi.clearAllMocks(); }); it('should return the same instance on multiple calls', () => { @@ -39,6 +40,12 @@ describe('AuthManager Singleton', () => { const instance = AuthManager.getInstance(config); expect(instance).toBeDefined(); + + // Verify the config is passed to internal components + // This would be observable when attempting operations that use the config + // For example, getCredentials would look in the configured file path + const credentials = instance.getCredentials(); + expect(credentials).toBeNull(); // File doesn't exist, but it should check the right path }); it('should warn when config is provided after initialization', () => { diff --git a/packages/tm-core/src/auth/auth-manager.ts b/packages/tm-core/src/auth/auth-manager.ts index f59931cf..1d678906 100644 --- a/packages/tm-core/src/auth/auth-manager.ts +++ b/packages/tm-core/src/auth/auth-manager.ts @@ -41,7 +41,7 @@ export class AuthManager { 'getInstance called with config after initialization; config is ignored.' ); } - return AuthManager.instance; + return AuthManager.instance!; } /** @@ -74,29 +74,6 @@ export class AuthManager { return this.oauthService.getAuthorizationUrl(); } - /** - * Authenticate with API key - * Note: This would require a custom implementation or Supabase RLS policies - */ - async authenticateWithApiKey(apiKey: string): Promise { - const token = apiKey.trim(); - if (!token || token.length < 10) { - throw new AuthenticationError('Invalid API key', 'INVALID_API_KEY'); - } - - const authData: AuthCredentials = { - token, - tokenType: 'api_key', - userId: 'api-user', - email: undefined, - expiresAt: undefined, // API keys don't expire - savedAt: new Date().toISOString() - }; - - this.credentialStore.saveCredentials(authData); - return authData; - } - /** * Refresh authentication token */ diff --git a/packages/tm-core/src/auth/credential-store.test.ts b/packages/tm-core/src/auth/credential-store.test.ts new file mode 100644 index 00000000..93330521 --- /dev/null +++ b/packages/tm-core/src/auth/credential-store.test.ts @@ -0,0 +1,414 @@ +/** + * Tests for CredentialStore with numeric and string timestamp handling + */ + +import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; +import { CredentialStore } from './credential-store.js'; +import { AuthenticationError } from './types.js'; +import type { AuthCredentials } from './types.js'; +import fs from 'fs'; +import path from 'path'; +import os from 'os'; + +// Mock fs module +vi.mock('fs'); + +// Mock logger +const mockLogger = { + warn: vi.fn(), + info: vi.fn(), + debug: vi.fn(), + error: vi.fn() +}; + +vi.mock('../logger/index.js', () => ({ + getLogger: () => mockLogger +})); + +describe('CredentialStore', () => { + let store: CredentialStore; + const testDir = '/test/config'; + const configFile = '/test/config/auth.json'; + + beforeEach(() => { + vi.clearAllMocks(); + store = new CredentialStore({ + configDir: testDir, + configFile: configFile, + baseUrl: 'https://api.test.com' + }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + describe('getCredentials with timestamp migration', () => { + it('should handle string ISO timestamp correctly', () => { + const futureDate = new Date(Date.now() + 3600000); // 1 hour from now + const mockCredentials: AuthCredentials = { + token: 'test-token', + userId: 'user-123', + email: 'test@example.com', + expiresAt: futureDate.toISOString(), + tokenType: 'standard', + savedAt: new Date().toISOString() + }; + + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue(JSON.stringify(mockCredentials)); + + const result = store.getCredentials(); + + expect(result).not.toBeNull(); + expect(result?.token).toBe('test-token'); + // The timestamp should be normalized to numeric milliseconds + expect(typeof result?.expiresAt).toBe('number'); + expect(result?.expiresAt).toBe(futureDate.getTime()); + }); + + it('should handle numeric timestamp correctly', () => { + const futureTimestamp = Date.now() + 7200000; // 2 hours from now + const mockCredentials = { + token: 'test-token', + userId: 'user-456', + email: 'test2@example.com', + expiresAt: futureTimestamp, + tokenType: 'standard', + savedAt: new Date().toISOString() + }; + + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue(JSON.stringify(mockCredentials)); + + const result = store.getCredentials(); + + expect(result).not.toBeNull(); + expect(result?.token).toBe('test-token'); + // Numeric timestamp should remain as-is + expect(typeof result?.expiresAt).toBe('number'); + expect(result?.expiresAt).toBe(futureTimestamp); + }); + + it('should reject invalid string timestamp', () => { + const mockCredentials = { + token: 'test-token', + userId: 'user-789', + expiresAt: 'invalid-date-string', + tokenType: 'standard', + savedAt: new Date().toISOString() + }; + + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue(JSON.stringify(mockCredentials)); + + const result = store.getCredentials(); + + expect(result).toBeNull(); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'No valid expiration time provided for token' + ); + }); + + it('should reject NaN timestamp', () => { + const mockCredentials = { + token: 'test-token', + userId: 'user-nan', + expiresAt: NaN, + tokenType: 'standard', + savedAt: new Date().toISOString() + }; + + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue(JSON.stringify(mockCredentials)); + + const result = store.getCredentials(); + + expect(result).toBeNull(); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'No valid expiration time provided for token' + ); + }); + + it('should reject Infinity timestamp', () => { + const mockCredentials = { + token: 'test-token', + userId: 'user-inf', + expiresAt: Infinity, + tokenType: 'standard', + savedAt: new Date().toISOString() + }; + + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue(JSON.stringify(mockCredentials)); + + const result = store.getCredentials(); + + expect(result).toBeNull(); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'No valid expiration time provided for token' + ); + }); + + it('should handle missing expiresAt field', () => { + const mockCredentials = { + token: 'test-token', + userId: 'user-no-expiry', + tokenType: 'standard', + savedAt: new Date().toISOString() + // No expiresAt field + }; + + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue(JSON.stringify(mockCredentials)); + + const result = store.getCredentials(); + + expect(result).toBeNull(); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'No valid expiration time provided for token' + ); + }); + + it('should check token expiration correctly', () => { + const expiredTimestamp = Date.now() - 3600000; // 1 hour ago + const mockCredentials = { + token: 'expired-token', + userId: 'user-expired', + expiresAt: expiredTimestamp, + tokenType: 'standard', + savedAt: new Date().toISOString() + }; + + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue(JSON.stringify(mockCredentials)); + + const result = store.getCredentials(); + + expect(result).toBeNull(); + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('Authentication token has expired'), + expect.any(Object) + ); + }); + + it('should allow expired tokens when requested', () => { + const expiredTimestamp = Date.now() - 3600000; // 1 hour ago + const mockCredentials = { + token: 'expired-token', + userId: 'user-expired', + expiresAt: expiredTimestamp, + tokenType: 'standard', + savedAt: new Date().toISOString() + }; + + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue(JSON.stringify(mockCredentials)); + + const result = store.getCredentials({ allowExpired: true }); + + expect(result).not.toBeNull(); + expect(result?.token).toBe('expired-token'); + }); + }); + + describe('saveCredentials with timestamp normalization', () => { + beforeEach(() => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.mkdirSync).mockImplementation(() => undefined); + vi.mocked(fs.writeFileSync).mockImplementation(() => undefined); + vi.mocked(fs.renameSync).mockImplementation(() => undefined); + }); + + it('should normalize string timestamp to ISO string when saving', () => { + const futureDate = new Date(Date.now() + 3600000); + const credentials: AuthCredentials = { + token: 'test-token', + userId: 'user-123', + expiresAt: futureDate.toISOString(), + tokenType: 'standard', + savedAt: new Date().toISOString() + }; + + store.saveCredentials(credentials); + + expect(fs.writeFileSync).toHaveBeenCalledWith( + expect.stringContaining('.tmp'), + expect.stringContaining('"expiresAt":'), + expect.any(Object) + ); + + // Check that the written data contains a valid ISO string + const writtenData = vi.mocked(fs.writeFileSync).mock.calls[0][1] as string; + const parsed = JSON.parse(writtenData); + expect(typeof parsed.expiresAt).toBe('string'); + expect(new Date(parsed.expiresAt).toISOString()).toBe(parsed.expiresAt); + }); + + it('should convert numeric timestamp to ISO string when saving', () => { + const futureTimestamp = Date.now() + 7200000; + const credentials: AuthCredentials = { + token: 'test-token', + userId: 'user-456', + expiresAt: futureTimestamp, + tokenType: 'standard', + savedAt: new Date().toISOString() + }; + + store.saveCredentials(credentials); + + const writtenData = vi.mocked(fs.writeFileSync).mock.calls[0][1] as string; + const parsed = JSON.parse(writtenData); + expect(typeof parsed.expiresAt).toBe('string'); + expect(new Date(parsed.expiresAt).getTime()).toBe(futureTimestamp); + }); + + it('should reject invalid string timestamp when saving', () => { + const credentials: AuthCredentials = { + token: 'test-token', + userId: 'user-789', + expiresAt: 'invalid-date' as any, + tokenType: 'standard', + savedAt: new Date().toISOString() + }; + + expect(() => store.saveCredentials(credentials)).toThrow(AuthenticationError); + expect(() => store.saveCredentials(credentials)).toThrow('Invalid expiresAt format'); + }); + + it('should reject NaN timestamp when saving', () => { + const credentials: AuthCredentials = { + token: 'test-token', + userId: 'user-nan', + expiresAt: NaN as any, + tokenType: 'standard', + savedAt: new Date().toISOString() + }; + + expect(() => store.saveCredentials(credentials)).toThrow(AuthenticationError); + expect(() => store.saveCredentials(credentials)).toThrow('Invalid expiresAt format'); + }); + + it('should reject Infinity timestamp when saving', () => { + const credentials: AuthCredentials = { + token: 'test-token', + userId: 'user-inf', + expiresAt: Infinity as any, + tokenType: 'standard', + savedAt: new Date().toISOString() + }; + + expect(() => store.saveCredentials(credentials)).toThrow(AuthenticationError); + expect(() => store.saveCredentials(credentials)).toThrow('Invalid expiresAt format'); + }); + + it('should handle missing expiresAt when saving', () => { + const credentials: AuthCredentials = { + token: 'test-token', + userId: 'user-no-expiry', + tokenType: 'standard', + savedAt: new Date().toISOString() + // No expiresAt + }; + + store.saveCredentials(credentials); + + const writtenData = vi.mocked(fs.writeFileSync).mock.calls[0][1] as string; + const parsed = JSON.parse(writtenData); + expect(parsed.expiresAt).toBeUndefined(); + }); + + it('should not mutate the original credentials object', () => { + const originalTimestamp = Date.now() + 3600000; + const credentials: AuthCredentials = { + token: 'test-token', + userId: 'user-123', + expiresAt: originalTimestamp, + tokenType: 'standard', + savedAt: new Date().toISOString() + }; + + const originalCredentialsCopy = { ...credentials }; + + store.saveCredentials(credentials); + + // Original object should not be modified + expect(credentials).toEqual(originalCredentialsCopy); + expect(credentials.expiresAt).toBe(originalTimestamp); + }); + }); + + describe('corrupt file handling', () => { + it('should quarantine corrupt file on JSON parse error', () => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue('invalid json {'); + vi.mocked(fs.renameSync).mockImplementation(() => undefined); + + const result = store.getCredentials(); + + expect(result).toBeNull(); + expect(fs.renameSync).toHaveBeenCalledWith( + configFile, + expect.stringContaining('.corrupt-') + ); + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('Quarantined corrupt auth file') + ); + }); + + it('should handle quarantine failure gracefully', () => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue('invalid json {'); + vi.mocked(fs.renameSync).mockImplementation(() => { + throw new Error('Permission denied'); + }); + + const result = store.getCredentials(); + + expect(result).toBeNull(); + expect(mockLogger.debug).toHaveBeenCalledWith( + expect.stringContaining('Could not quarantine corrupt file') + ); + }); + }); + + describe('cleanupCorruptFiles', () => { + it('should remove old corrupt files', () => { + const now = Date.now(); + const oldFile = 'auth.json.corrupt-' + (now - 8 * 24 * 60 * 60 * 1000); // 8 days old + const newFile = 'auth.json.corrupt-' + (now - 1000); // 1 second old + + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readdirSync).mockReturnValue([ + { name: oldFile, isFile: () => true }, + { name: newFile, isFile: () => true }, + { name: 'auth.json', isFile: () => true } + ] as any); + vi.mocked(fs.statSync).mockImplementation((filePath) => { + if (filePath.includes(oldFile)) { + return { mtimeMs: now - 8 * 24 * 60 * 60 * 1000 } as any; + } + return { mtimeMs: now - 1000 } as any; + }); + vi.mocked(fs.unlinkSync).mockImplementation(() => undefined); + + store.cleanupCorruptFiles(); + + expect(fs.unlinkSync).toHaveBeenCalledWith(expect.stringContaining(oldFile)); + expect(fs.unlinkSync).not.toHaveBeenCalledWith(expect.stringContaining(newFile)); + }); + + it('should handle cleanup errors gracefully', () => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readdirSync).mockImplementation(() => { + throw new Error('Permission denied'); + }); + + // Should not throw + expect(() => store.cleanupCorruptFiles()).not.toThrow(); + expect(mockLogger.debug).toHaveBeenCalledWith( + expect.stringContaining('Error during corrupt file cleanup') + ); + }); + }); +}); \ No newline at end of file diff --git a/packages/tm-core/src/auth/credential-store.ts b/packages/tm-core/src/auth/credential-store.ts index c4d47f85..bb8fd015 100644 --- a/packages/tm-core/src/auth/credential-store.ts +++ b/packages/tm-core/src/auth/credential-store.ts @@ -29,30 +29,38 @@ export class CredentialStore { fs.readFileSync(this.config.configFile, 'utf-8') ) as AuthCredentials; - // Parse expiration time for validation (expects ISO string format) + // Normalize/migrate timestamps to numeric (handles both number and ISO string) let expiresAtMs: number | undefined; - - if (authData.expiresAt) { - expiresAtMs = Date.parse(authData.expiresAt); - if (isNaN(expiresAtMs)) { - // Invalid date string - treat as expired - this.logger.error(`Invalid expiresAt format: ${authData.expiresAt}`); - return null; - } + if (typeof authData.expiresAt === 'number') { + expiresAtMs = Number.isFinite(authData.expiresAt) ? authData.expiresAt : undefined; + } else if (typeof authData.expiresAt === 'string') { + const parsed = Date.parse(authData.expiresAt); + expiresAtMs = Number.isNaN(parsed) ? undefined : parsed; + } else { + expiresAtMs = undefined; } - // Check if token is expired (API keys never expire) - const isApiKey = authData.tokenType === 'api_key'; - if ( - !isApiKey && - expiresAtMs && - expiresAtMs < Date.now() && - !options?.allowExpired - ) { - this.logger.warn('Authentication token has expired'); + // Validate expiration time for tokens + if (expiresAtMs === undefined) { + this.logger.warn('No valid expiration time provided for token'); return null; } + // Update the authData with normalized timestamp + authData.expiresAt = expiresAtMs; + + // Check if the token has expired + const now = Date.now(); + const allowExpired = options?.allowExpired ?? false; + if (now >= expiresAtMs && !allowExpired) { + this.logger.warn('Authentication token has expired', { + expiresAt: authData.expiresAt, + currentTime: new Date(now).toISOString() + }); + return null; + } + + // Return valid token return authData; } catch (error) { this.logger.error( @@ -87,18 +95,29 @@ export class CredentialStore { fs.mkdirSync(this.config.configDir, { recursive: true, mode: 0o700 }); } - // Add timestamp - authData.savedAt = new Date().toISOString(); + // Add timestamp without mutating caller's object + authData = { ...authData, savedAt: new Date().toISOString() }; - // Validate expiresAt is a valid ISO string if present - if (authData.expiresAt) { - const ms = Date.parse(authData.expiresAt); - if (isNaN(ms)) { + // Validate and normalize expiresAt timestamp + if (authData.expiresAt !== undefined) { + let validTimestamp: number | undefined; + + if (typeof authData.expiresAt === 'number') { + validTimestamp = Number.isFinite(authData.expiresAt) ? authData.expiresAt : undefined; + } else if (typeof authData.expiresAt === 'string') { + const parsed = Date.parse(authData.expiresAt); + validTimestamp = Number.isNaN(parsed) ? undefined : parsed; + } + + if (validTimestamp === undefined) { throw new AuthenticationError( `Invalid expiresAt format: ${authData.expiresAt}`, 'SAVE_FAILED' ); } + + // Store as ISO string for consistency + authData.expiresAt = new Date(validTimestamp).toISOString(); } // Save credentials atomically with secure permissions @@ -156,16 +175,19 @@ export class CredentialStore { try { const dir = path.dirname(this.config.configFile); const baseName = path.basename(this.config.configFile); - const corruptPattern = new RegExp(`^${baseName}\\.corrupt-\\d+$`); + const escapedBaseName = baseName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const corruptPattern = new RegExp(`^${escapedBaseName}\\.corrupt-\\d+$`); if (!fs.existsSync(dir)) { return; } - const files = fs.readdirSync(dir); + const entries = fs.readdirSync(dir, { withFileTypes: true }); const now = Date.now(); - for (const file of files) { + for (const entry of entries) { + if (!entry.isFile()) continue; + const file = entry.name; if (corruptPattern.test(file)) { const filePath = path.join(dir, file); try { diff --git a/packages/tm-core/src/auth/types.ts b/packages/tm-core/src/auth/types.ts index a86250aa..a2dc5d72 100644 --- a/packages/tm-core/src/auth/types.ts +++ b/packages/tm-core/src/auth/types.ts @@ -7,8 +7,8 @@ export interface AuthCredentials { refreshToken?: string; userId: string; email?: string; - expiresAt?: string; - tokenType?: 'standard' | 'api_key'; + expiresAt?: string | number; + tokenType?: 'standard'; savedAt: string; } @@ -57,7 +57,6 @@ export type AuthErrorCode = | 'INVALID_STATE' | 'NO_TOKEN' | 'TOKEN_EXCHANGE_FAILED' - | 'INVALID_API_KEY' | 'INVALID_CREDENTIALS' | 'NO_REFRESH_TOKEN' | 'NOT_AUTHENTICATED' @@ -65,7 +64,10 @@ export type AuthErrorCode = | 'CONFIG_MISSING' | 'SAVE_FAILED' | 'CLEAR_FAILED' - | 'STORAGE_ERROR'; + | 'STORAGE_ERROR' + | 'NOT_SUPPORTED' + | 'REFRESH_FAILED' + | 'INVALID_RESPONSE'; /** * Authentication error class diff --git a/packages/tm-core/src/config/config-manager.ts b/packages/tm-core/src/config/config-manager.ts index 51867b63..5ef5003a 100644 --- a/packages/tm-core/src/config/config-manager.ts +++ b/packages/tm-core/src/config/config-manager.ts @@ -142,17 +142,23 @@ export class ConfigManager { // Return the configured type (including 'auto') const storageType = storage?.type || 'auto'; + const basePath = storage?.basePath; if (storageType === 'api' || storageType === 'auto') { return { type: storageType, + basePath, apiEndpoint: storage?.apiEndpoint, apiAccessToken: storage?.apiAccessToken, apiConfigured: Boolean(storage?.apiEndpoint || storage?.apiAccessToken) }; } - return { type: storageType, apiConfigured: false }; + return { + type: storageType, + basePath, + apiConfigured: false + }; } /** @@ -183,9 +189,10 @@ export class ConfigManager { } /** - * Check if using API storage + * Check if explicitly configured to use API storage + * Excludes 'auto' type */ - isUsingApiStorage(): boolean { + isApiExplicitlyConfigured(): boolean { return this.getStorageConfig().type === 'api'; } @@ -218,6 +225,7 @@ export class ConfigManager { await this.persistence.saveConfig(this.config); // Re-initialize to respect precedence + this.initialized = false; await this.initialize(); } diff --git a/packages/tm-core/src/interfaces/configuration.interface.ts b/packages/tm-core/src/interfaces/configuration.interface.ts index b41e5717..eecad240 100644 --- a/packages/tm-core/src/interfaces/configuration.interface.ts +++ b/packages/tm-core/src/interfaces/configuration.interface.ts @@ -88,6 +88,8 @@ export type StorageType = 'file' | 'api' | 'auto'; export interface RuntimeStorageConfig { /** Storage backend type */ type: StorageType; + /** Base path for file storage (if configured) */ + basePath?: string; /** API endpoint for API storage (Hamster integration) */ apiEndpoint?: string; /** Access token for API authentication */ @@ -107,19 +109,15 @@ export interface RuntimeStorageConfig { export interface StorageSettings extends Omit { /** Storage backend type - 'auto' detects based on auth status */ - type: 'file' | 'api' | 'auto'; + type: StorageType; /** Base path for file storage */ basePath?: string; - /** API endpoint for API storage (Hamster integration) */ - apiEndpoint?: string; - /** Access token for API authentication */ - apiAccessToken?: string; - /** - * Indicates whether API is configured (has endpoint or token) + /** + * Indicates whether API is configured * @computed Derived automatically from presence of apiEndpoint or apiAccessToken * @internal Should not be set manually in user config - computed by ConfigManager */ - apiConfigured?: boolean; + readonly apiConfigured?: boolean; /** Enable automatic backups */ enableBackup: boolean; /** Maximum number of backups to retain */ diff --git a/packages/tm-core/src/storage/storage-factory.ts b/packages/tm-core/src/storage/storage-factory.ts index 60546d3c..873ff0f6 100644 --- a/packages/tm-core/src/storage/storage-factory.ts +++ b/packages/tm-core/src/storage/storage-factory.ts @@ -28,9 +28,10 @@ export class StorageFactory { storageConfig: RuntimeStorageConfig, projectPath: string ): IStorage { - // Wrap the storage config in the expected format + // Wrap the storage config in the expected format, including projectPath + // This ensures ApiStorage receives the projectPath for projectId return StorageFactory.create( - { storage: storageConfig } as Partial, + { storage: storageConfig, projectPath } as Partial, projectPath ); }