From 4c1ef2ca94411c53bcd2a78ec710b06c500236dd Mon Sep 17 00:00:00 2001 From: Ralph Khreish <35776126+Crunchyman-ralph@users.noreply.github.com> Date: Mon, 13 Oct 2025 21:50:22 +0200 Subject: [PATCH] fix: auth refresh token (#1299) Co-authored-by: github-actions[bot] --- .changeset/config.json | 3 +- .changeset/pre.json | 7 +- .changeset/silent-bushes-grow.md | 5 + CHANGELOG.md | 6 + apps/cli/src/commands/auth.command.ts | 40 +- apps/cli/src/commands/context.command.ts | 20 +- apps/cli/src/commands/export.command.ts | 2 +- package-lock.json | 23 + package.json | 2 +- packages/tm-core/package.json | 3 +- ...h-manager.test.ts => auth-manager.spec.ts} | 13 +- packages/tm-core/src/auth/auth-manager.ts | 71 +++- .../tm-core/src/auth/credential-store.spec.ts | 289 +++++++++++++ .../supabase/supabase-task-repository.ts | 10 +- .../tm-core/src/services/export.service.ts | 6 +- packages/tm-core/src/storage/api-storage.ts | 6 +- .../tm-core/src/storage/storage-factory.ts | 6 +- .../tm-core/tests/auth/auth-refresh.test.ts | 176 ++++++++ .../integration/auth-token-refresh.test.ts | 392 ++++++++++++++++++ 19 files changed, 1023 insertions(+), 57 deletions(-) create mode 100644 .changeset/silent-bushes-grow.md rename packages/tm-core/src/auth/{auth-manager.test.ts => auth-manager.spec.ts} (92%) create mode 100644 packages/tm-core/src/auth/credential-store.spec.ts create mode 100644 packages/tm-core/tests/auth/auth-refresh.test.ts create mode 100644 packages/tm-core/tests/integration/auth-token-refresh.test.ts diff --git a/.changeset/config.json b/.changeset/config.json index 018bd4f7..5e077617 100644 --- a/.changeset/config.json +++ b/.changeset/config.json @@ -11,6 +11,7 @@ "access": "public", "baseBranch": "main", "ignore": [ - "docs" + "docs", + "@tm/claude-code-plugin" ] } \ No newline at end of file diff --git a/.changeset/pre.json b/.changeset/pre.json index fd25d0f2..1ff081ac 100644 --- a/.changeset/pre.json +++ b/.changeset/pre.json @@ -1,11 +1,11 @@ { - "mode": "exit", + "mode": "pre", "tag": "rc", "initialVersions": { - "task-master-ai": "0.28.0", + "task-master-ai": "0.29.0-rc.0", "@tm/cli": "", "docs": "0.0.5", - "extension": "0.25.5", + "extension": "0.25.6-rc.0", "@tm/ai-sdk-provider-grok-cli": "", "@tm/build-config": "", "@tm/claude-code-plugin": "0.0.1", @@ -16,6 +16,7 @@ "mean-planes-wave", "nice-ways-hope", "plain-falcons-serve", + "silent-bushes-grow", "smart-owls-relax" ] } diff --git a/.changeset/silent-bushes-grow.md b/.changeset/silent-bushes-grow.md new file mode 100644 index 00000000..a99fee73 --- /dev/null +++ b/.changeset/silent-bushes-grow.md @@ -0,0 +1,5 @@ +--- +"task-master-ai": patch +--- + +Improve refresh token when authenticating diff --git a/CHANGELOG.md b/CHANGELOG.md index 67e6f586..eba8504f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # task-master-ai +## 0.29.0-rc.1 + +### Patch Changes + +- [#1299](https://github.com/eyaltoledano/claude-task-master/pull/1299) [`a6c5152`](https://github.com/eyaltoledano/claude-task-master/commit/a6c5152f20edd8717cf1aea34e7c178b1261aa99) Thanks [@Crunchyman-ralph](https://github.com/Crunchyman-ralph)! - Improve refresh token when authenticating + ## 0.29.0-rc.0 ### Minor Changes diff --git a/apps/cli/src/commands/auth.command.ts b/apps/cli/src/commands/auth.command.ts index 8053311d..39992beb 100644 --- a/apps/cli/src/commands/auth.command.ts +++ b/apps/cli/src/commands/auth.command.ts @@ -143,7 +143,7 @@ export class AuthCommand extends Command { */ private async executeStatus(): Promise { try { - const result = this.displayStatus(); + const result = await this.displayStatus(); this.setLastResult(result); } catch (error: any) { this.handleError(error); @@ -171,8 +171,8 @@ export class AuthCommand extends Command { /** * Display authentication status */ - private displayStatus(): AuthResult { - const credentials = this.authManager.getCredentials(); + private async displayStatus(): Promise { + const credentials = await this.authManager.getCredentials(); console.log(chalk.cyan('\nšŸ” Authentication Status\n')); @@ -187,19 +187,29 @@ export class AuthCommand extends Command { if (credentials.expiresAt) { const expiresAt = new Date(credentials.expiresAt); const now = new Date(); - const hoursRemaining = Math.floor( - (expiresAt.getTime() - now.getTime()) / (1000 * 60 * 60) - ); + const timeRemaining = expiresAt.getTime() - now.getTime(); + const hoursRemaining = Math.floor(timeRemaining / (1000 * 60 * 60)); + const minutesRemaining = Math.floor(timeRemaining / (1000 * 60)); - if (hoursRemaining > 0) { - console.log( - chalk.gray( - ` Expires: ${expiresAt.toLocaleString()} (${hoursRemaining} hours remaining)` - ) - ); + if (timeRemaining > 0) { + // Token is still valid + if (hoursRemaining > 0) { + console.log( + chalk.gray( + ` Expires at: ${expiresAt.toLocaleString()} (${hoursRemaining} hours remaining)` + ) + ); + } else { + console.log( + chalk.gray( + ` Expires at: ${expiresAt.toLocaleString()} (${minutesRemaining} minutes remaining)` + ) + ); + } } else { + // Token has expired console.log( - chalk.yellow(` Token expired at: ${expiresAt.toLocaleString()}`) + chalk.yellow(` Expired at: ${expiresAt.toLocaleString()}`) ); } } else { @@ -315,7 +325,7 @@ export class AuthCommand extends Command { ]); if (!continueAuth) { - const credentials = this.authManager.getCredentials(); + const credentials = await this.authManager.getCredentials(); ui.displaySuccess('Using existing authentication'); if (credentials) { @@ -480,7 +490,7 @@ export class AuthCommand extends Command { /** * Get current credentials (for programmatic usage) */ - getCredentials(): AuthCredentials | null { + getCredentials(): Promise { return this.authManager.getCredentials(); } diff --git a/apps/cli/src/commands/context.command.ts b/apps/cli/src/commands/context.command.ts index 04d475f3..b18b06dd 100644 --- a/apps/cli/src/commands/context.command.ts +++ b/apps/cli/src/commands/context.command.ts @@ -115,7 +115,7 @@ export class ContextCommand extends Command { */ private async executeShow(): Promise { try { - const result = this.displayContext(); + const result = await this.displayContext(); this.setLastResult(result); } catch (error: any) { this.handleError(error); @@ -126,7 +126,7 @@ export class ContextCommand extends Command { /** * Display current context */ - private displayContext(): ContextResult { + private async displayContext(): Promise { // Check authentication first if (!this.authManager.isAuthenticated()) { console.log(chalk.yellow('āœ— Not authenticated')); @@ -139,7 +139,7 @@ export class ContextCommand extends Command { }; } - const context = this.authManager.getContext(); + const context = await this.authManager.getContext(); console.log(chalk.cyan('\nšŸŒ Workspace Context\n')); @@ -263,7 +263,7 @@ export class ContextCommand extends Command { return { success: true, action: 'select-org', - context: this.authManager.getContext() || undefined, + context: (await this.authManager.getContext()) || undefined, message: `Selected organization: ${selectedOrg.name}` }; } catch (error) { @@ -284,7 +284,7 @@ export class ContextCommand extends Command { } // Check if org is selected - const context = this.authManager.getContext(); + const context = await this.authManager.getContext(); if (!context?.orgId) { ui.displayError( 'No organization selected. Run "tm context org" first.' @@ -353,7 +353,7 @@ export class ContextCommand extends Command { return { success: true, action: 'select-brief', - context: this.authManager.getContext() || undefined, + context: (await this.authManager.getContext()) || undefined, message: `Selected brief: ${selectedBrief.name}` }; } else { @@ -368,7 +368,7 @@ export class ContextCommand extends Command { return { success: true, action: 'select-brief', - context: this.authManager.getContext() || undefined, + context: (await this.authManager.getContext()) || undefined, message: 'Cleared brief selection' }; } @@ -508,7 +508,7 @@ export class ContextCommand extends Command { this.setLastResult({ success: true, action: 'set', - context: this.authManager.getContext() || undefined, + context: (await this.authManager.getContext()) || undefined, message: 'Context set from brief' }); } catch (error: any) { @@ -631,7 +631,7 @@ export class ContextCommand extends Command { return { success: true, action: 'set', - context: this.authManager.getContext() || undefined, + context: (await this.authManager.getContext()) || undefined, message: 'Context updated' }; } catch (error) { @@ -682,7 +682,7 @@ export class ContextCommand extends Command { /** * Get current context (for programmatic usage) */ - getContext(): UserContext | null { + getContext(): Promise { return this.authManager.getContext(); } diff --git a/apps/cli/src/commands/export.command.ts b/apps/cli/src/commands/export.command.ts index 0d09efe3..25b55a93 100644 --- a/apps/cli/src/commands/export.command.ts +++ b/apps/cli/src/commands/export.command.ts @@ -103,7 +103,7 @@ export class ExportCommand extends Command { await this.initializeServices(); // Get current context - const context = this.authManager.getContext(); + const context = await this.authManager.getContext(); // Determine org and brief IDs let orgId = options?.org || context?.orgId; diff --git a/package-lock.json b/package-lock.json index 5312f2c1..4440b156 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24240,6 +24240,26 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/strip-literal": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/strip-literal/-/strip-literal-3.1.0.tgz", + "integrity": "sha512-8r3mkIM/2+PpjHoOtiAW8Rg3jJLHaV7xPwG+YRGrv6FP0wwk/toTpATxWYOW0BKdWwl82VT2tFYi5DlROa0Mxg==", + "dev": true, + "license": "MIT", + "dependencies": { + "js-tokens": "^9.0.1" + }, + "funding": { + "url": "https://github.com/sponsors/antfu" + } + }, + "node_modules/strip-literal/node_modules/js-tokens": { + "version": "9.0.1", + "resolved": "https://registry.npmjs.org/js-tokens/-/js-tokens-9.0.1.tgz", + "integrity": "sha512-mxa9E9ITFOt0ban3j6L5MpjwegGz6lBQmM1IJkWeBZGcMxto50+eWdjC/52xDbS2vy0k7vIMK0Fe2wfL9OQSpQ==", + "dev": true, + "license": "MIT" + }, "node_modules/strnum": { "version": "2.1.1", "funding": [ @@ -27129,6 +27149,7 @@ "devDependencies": { "@types/node": "^22.10.5", "@vitest/coverage-v8": "^3.2.4", + "strip-literal": "3.1.0", "typescript": "^5.9.2", "vitest": "^3.2.4" } @@ -27438,6 +27459,8 @@ }, "packages/tm-core/node_modules/vitest": { "version": "3.2.4", + "resolved": "https://registry.npmjs.org/vitest/-/vitest-3.2.4.tgz", + "integrity": "sha512-LUCP5ev3GURDysTWiP47wRRUpLKMOfPh+yKTx3kVIEiu5KOMeqzpnYNsKyOoVrULivR8tLcks4+lga33Whn90A==", "dev": true, "license": "MIT", "dependencies": { diff --git a/package.json b/package.json index 8ade5e92..30ae6df7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "task-master-ai", - "version": "0.29.0-rc.0", + "version": "0.29.0-rc.1", "description": "A task management system for ambitious AI-driven development that doesn't overwhelm and confuse Cursor.", "main": "index.js", "type": "module", diff --git a/packages/tm-core/package.json b/packages/tm-core/package.json index c69fa1de..64dc815c 100644 --- a/packages/tm-core/package.json +++ b/packages/tm-core/package.json @@ -37,7 +37,8 @@ "@types/node": "^22.10.5", "@vitest/coverage-v8": "^3.2.4", "typescript": "^5.9.2", - "vitest": "^3.2.4" + "vitest": "^3.2.4", + "strip-literal": "3.1.0" }, "files": ["src", "README.md", "CHANGELOG.md"], "keywords": ["task-management", "typescript", "ai", "prd", "parser"], diff --git a/packages/tm-core/src/auth/auth-manager.test.ts b/packages/tm-core/src/auth/auth-manager.spec.ts similarity index 92% rename from packages/tm-core/src/auth/auth-manager.test.ts rename to packages/tm-core/src/auth/auth-manager.spec.ts index 54ec05ee..b78ce8f3 100644 --- a/packages/tm-core/src/auth/auth-manager.test.ts +++ b/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 }); diff --git a/packages/tm-core/src/auth/auth-manager.ts b/packages/tm-core/src/auth/auth-manager.ts index 538162a6..2acd301b 100644 --- a/packages/tm-core/src/auth/auth-manager.ts +++ b/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 | null = null; private constructor(config?: Partial) { 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 { + 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 { + 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): Promise { - 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 { - 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 { 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'); } diff --git a/packages/tm-core/src/auth/credential-store.spec.ts b/packages/tm-core/src/auth/credential-store.spec.ts new file mode 100644 index 00000000..05bb813c --- /dev/null +++ b/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); + }); + }); +}); diff --git a/packages/tm-core/src/repositories/supabase/supabase-task-repository.ts b/packages/tm-core/src/repositories/supabase/supabase-task-repository.ts index 925de209..faf9fbd0 100644 --- a/packages/tm-core/src/repositories/supabase/supabase-task-repository.ts +++ b/packages/tm-core/src/repositories/supabase/supabase-task-repository.ts @@ -47,8 +47,8 @@ export class SupabaseTaskRepository { * Gets the current brief ID from auth context * @throws {Error} If no brief is selected */ - private getBriefIdOrThrow(): string { - const context = this.authManager.getContext(); + private async getBriefIdOrThrow(): Promise { + const context = await this.authManager.getContext(); if (!context?.briefId) { throw new Error( 'No brief selected. Please select a brief first using: tm context brief' @@ -61,7 +61,7 @@ export class SupabaseTaskRepository { _projectId?: string, options?: LoadTasksOptions ): Promise { - const briefId = this.getBriefIdOrThrow(); + const briefId = await this.getBriefIdOrThrow(); // Build query with filters let query = this.supabase @@ -114,7 +114,7 @@ export class SupabaseTaskRepository { } async getTask(_projectId: string, taskId: string): Promise { - const briefId = this.getBriefIdOrThrow(); + const briefId = await this.getBriefIdOrThrow(); const { data, error } = await this.supabase .from('tasks') @@ -157,7 +157,7 @@ export class SupabaseTaskRepository { taskId: string, updates: Partial ): Promise { - const briefId = this.getBriefIdOrThrow(); + const briefId = await this.getBriefIdOrThrow(); // Validate updates using Zod schema try { diff --git a/packages/tm-core/src/services/export.service.ts b/packages/tm-core/src/services/export.service.ts index 03037dbb..eba782d7 100644 --- a/packages/tm-core/src/services/export.service.ts +++ b/packages/tm-core/src/services/export.service.ts @@ -105,7 +105,7 @@ export class ExportService { } // Get current context - const context = this.authManager.getContext(); + const context = await this.authManager.getContext(); // Determine org and brief IDs let orgId = options.orgId || context?.orgId; @@ -232,7 +232,7 @@ export class ExportService { hasBrief: boolean; context: UserContext | null; }> { - const context = this.authManager.getContext(); + const context = await this.authManager.getContext(); return { hasOrg: !!context?.orgId, @@ -379,7 +379,7 @@ export class ExportService { }; // Get auth token - const credentials = this.authManager.getCredentials(); + const credentials = await this.authManager.getCredentials(); if (!credentials || !credentials.token) { throw new Error('Not authenticated'); } diff --git a/packages/tm-core/src/storage/api-storage.ts b/packages/tm-core/src/storage/api-storage.ts index 8a1fcd22..af4bc189 100644 --- a/packages/tm-core/src/storage/api-storage.ts +++ b/packages/tm-core/src/storage/api-storage.ts @@ -119,7 +119,7 @@ export class ApiStorage implements IStorage { private async loadTagsIntoCache(): Promise { try { const authManager = AuthManager.getInstance(); - const context = authManager.getContext(); + const context = await authManager.getContext(); // If we have a selected brief, create a virtual "tag" for it if (context?.briefId) { @@ -152,7 +152,7 @@ export class ApiStorage implements IStorage { try { const authManager = AuthManager.getInstance(); - const context = authManager.getContext(); + const context = await authManager.getContext(); // If no brief is selected in context, throw an error if (!context?.briefId) { @@ -318,7 +318,7 @@ export class ApiStorage implements IStorage { try { const authManager = AuthManager.getInstance(); - const context = authManager.getContext(); + const context = await authManager.getContext(); // In our API-based system, we only have one "tag" at a time - the current brief if (context?.briefId) { diff --git a/packages/tm-core/src/storage/storage-factory.ts b/packages/tm-core/src/storage/storage-factory.ts index 34e5dcdd..18b5a8fb 100644 --- a/packages/tm-core/src/storage/storage-factory.ts +++ b/packages/tm-core/src/storage/storage-factory.ts @@ -72,8 +72,8 @@ export class StorageFactory { { storageType: 'api', missing } ); } - // Use auth token from AuthManager - const credentials = authManager.getCredentials(); + // Use auth token from AuthManager (synchronous - no auto-refresh here) + const credentials = authManager.getCredentialsSync(); if (credentials) { // Merge with existing storage config, ensuring required fields const nextStorage: StorageSettings = { @@ -103,7 +103,7 @@ export class StorageFactory { // Then check if authenticated via AuthManager if (authManager.isAuthenticated()) { - const credentials = authManager.getCredentials(); + const credentials = authManager.getCredentialsSync(); if (credentials) { // Configure API storage with auth credentials const nextStorage: StorageSettings = { diff --git a/packages/tm-core/tests/auth/auth-refresh.test.ts b/packages/tm-core/tests/auth/auth-refresh.test.ts new file mode 100644 index 00000000..1ae50aaf --- /dev/null +++ b/packages/tm-core/tests/auth/auth-refresh.test.ts @@ -0,0 +1,176 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import type { Session } from '@supabase/supabase-js'; +import { AuthManager } from '../../src/auth/auth-manager'; +import { CredentialStore } from '../../src/auth/credential-store'; +import type { AuthCredentials } from '../../src/auth/types'; + +describe('AuthManager Token Refresh', () => { + let authManager: AuthManager; + let credentialStore: CredentialStore; + let tmpDir: string; + let authFile: string; + + beforeEach(() => { + // Reset singletons + AuthManager.resetInstance(); + CredentialStore.resetInstance(); + + // Create temporary directory for test isolation + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'tm-auth-refresh-')); + authFile = path.join(tmpDir, 'auth.json'); + + // Initialize AuthManager with test config (this will create CredentialStore internally) + authManager = AuthManager.getInstance({ + configDir: tmpDir, + configFile: authFile + }); + + // Get the CredentialStore instance that AuthManager created + credentialStore = CredentialStore.getInstance(); + credentialStore.clearCredentials(); + }); + + afterEach(() => { + // Clean up + try { + credentialStore.clearCredentials(); + } catch { + // Ignore cleanup errors + } + AuthManager.resetInstance(); + CredentialStore.resetInstance(); + vi.restoreAllMocks(); + + // Remove temporary directory + if (tmpDir && fs.existsSync(tmpDir)) { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it('should not make concurrent refresh requests', async () => { + // Set up expired credentials with refresh token + const expiredCredentials: AuthCredentials = { + token: 'expired_access_token', + refreshToken: 'valid_refresh_token', + userId: 'test-user-id', + email: 'test@example.com', + expiresAt: new Date(Date.now() - 1000).toISOString(), // Expired 1 second ago + savedAt: new Date().toISOString() + }; + + credentialStore.saveCredentials(expiredCredentials); + + // Mock the refreshToken method to track calls + const refreshTokenSpy = vi.spyOn(authManager as any, 'refreshToken'); + const mockSession: Session = { + access_token: 'new_access_token', + refresh_token: 'new_refresh_token', + expires_at: Math.floor(Date.now() / 1000) + 3600, + user: { + id: 'test-user-id', + email: 'test@example.com', + app_metadata: {}, + user_metadata: {}, + aud: 'authenticated', + created_at: new Date().toISOString() + } + }; + + refreshTokenSpy.mockResolvedValue({ + token: mockSession.access_token, + refreshToken: mockSession.refresh_token, + userId: mockSession.user.id, + email: mockSession.user.email, + expiresAt: new Date(mockSession.expires_at! * 1000).toISOString(), + savedAt: new Date().toISOString() + }); + + // Make multiple concurrent calls to getCredentials + const promises = [ + authManager.getCredentials(), + authManager.getCredentials(), + authManager.getCredentials() + ]; + + const results = await Promise.all(promises); + + // Verify all calls returned the same new credentials + expect(results[0]?.token).toBe('new_access_token'); + expect(results[1]?.token).toBe('new_access_token'); + expect(results[2]?.token).toBe('new_access_token'); + + // Verify refreshToken was only called once, not three times + expect(refreshTokenSpy).toHaveBeenCalledTimes(1); + }); + + it('should return valid credentials without attempting refresh', async () => { + // Set up valid (non-expired) credentials + const validCredentials: AuthCredentials = { + token: 'valid_access_token', + refreshToken: 'valid_refresh_token', + userId: 'test-user-id', + email: 'test@example.com', + expiresAt: new Date(Date.now() + 3600000).toISOString(), // Expires in 1 hour + savedAt: new Date().toISOString() + }; + + credentialStore.saveCredentials(validCredentials); + + // Spy on refreshToken to ensure it's not called + const refreshTokenSpy = vi.spyOn(authManager as any, 'refreshToken'); + + const credentials = await authManager.getCredentials(); + + expect(credentials?.token).toBe('valid_access_token'); + expect(refreshTokenSpy).not.toHaveBeenCalled(); + }); + + it('should return null if credentials are expired with no refresh token', async () => { + // Set up expired credentials WITHOUT refresh token + const expiredCredentials: AuthCredentials = { + token: 'expired_access_token', + refreshToken: undefined, + userId: 'test-user-id', + email: 'test@example.com', + expiresAt: new Date(Date.now() - 1000).toISOString(), // Expired 1 second ago + savedAt: new Date().toISOString() + }; + + credentialStore.saveCredentials(expiredCredentials); + + const credentials = await authManager.getCredentials(); + + expect(credentials).toBeNull(); + }); + + it('should return null if no credentials exist', async () => { + const credentials = await authManager.getCredentials(); + expect(credentials).toBeNull(); + }); + + it('should handle refresh failures gracefully', async () => { + // Set up expired credentials with refresh token + const expiredCredentials: AuthCredentials = { + token: 'expired_access_token', + refreshToken: 'invalid_refresh_token', + userId: 'test-user-id', + email: 'test@example.com', + expiresAt: new Date(Date.now() - 1000).toISOString(), + savedAt: new Date().toISOString() + }; + + credentialStore.saveCredentials(expiredCredentials); + + // Mock refreshToken to throw an error + const refreshTokenSpy = vi.spyOn(authManager as any, 'refreshToken'); + refreshTokenSpy.mockRejectedValue(new Error('Refresh failed')); + + const credentials = await authManager.getCredentials(); + + expect(credentials).toBeNull(); + expect(refreshTokenSpy).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/tm-core/tests/integration/auth-token-refresh.test.ts b/packages/tm-core/tests/integration/auth-token-refresh.test.ts new file mode 100644 index 00000000..b6103de5 --- /dev/null +++ b/packages/tm-core/tests/integration/auth-token-refresh.test.ts @@ -0,0 +1,392 @@ +/** + * @fileoverview Integration tests for JWT token auto-refresh functionality + * + * These tests verify that expired tokens are automatically refreshed + * when making API calls through AuthManager. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import type { Session } from '@supabase/supabase-js'; +import { AuthManager } from '../../src/auth/auth-manager'; +import { CredentialStore } from '../../src/auth/credential-store'; +import type { AuthCredentials } from '../../src/auth/types'; + +describe('AuthManager - Token Auto-Refresh Integration', () => { + let authManager: AuthManager; + let credentialStore: CredentialStore; + let tmpDir: string; + let authFile: string; + + // Mock Supabase session that will be returned on refresh + const mockRefreshedSession: Session = { + access_token: 'new-access-token-xyz', + refresh_token: 'new-refresh-token-xyz', + token_type: 'bearer', + expires_at: Math.floor(Date.now() / 1000) + 3600, // 1 hour from now + expires_in: 3600, + user: { + id: 'test-user-id', + email: 'test@example.com', + aud: 'authenticated', + role: 'authenticated', + app_metadata: {}, + user_metadata: {}, + created_at: new Date().toISOString() + } + }; + + beforeEach(() => { + // Reset singletons + AuthManager.resetInstance(); + CredentialStore.resetInstance(); + + // Create temporary directory for test isolation + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'tm-auth-integration-')); + authFile = path.join(tmpDir, 'auth.json'); + + // Initialize AuthManager with test config (this will create CredentialStore internally) + authManager = AuthManager.getInstance({ + configDir: tmpDir, + configFile: authFile + }); + + // Get the CredentialStore instance that AuthManager created + credentialStore = CredentialStore.getInstance(); + credentialStore.clearCredentials(); + }); + + afterEach(() => { + // Clean up + try { + credentialStore.clearCredentials(); + } catch { + // Ignore cleanup errors + } + AuthManager.resetInstance(); + CredentialStore.resetInstance(); + vi.restoreAllMocks(); + + // Remove temporary directory + if (tmpDir && fs.existsSync(tmpDir)) { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + describe('Expired Token Detection', () => { + it('should detect expired token', async () => { + // Set up expired credentials + const expiredCredentials: AuthCredentials = { + token: 'expired-token', + refreshToken: 'valid-refresh-token', + userId: 'test-user-id', + email: 'test@example.com', + expiresAt: new Date(Date.now() - 60000).toISOString(), // 1 minute ago + savedAt: new Date().toISOString() + }; + + credentialStore.saveCredentials(expiredCredentials); + + authManager = AuthManager.getInstance(); + + // Mock the Supabase refreshSession to return new tokens + const mockRefreshSession = vi + .fn() + .mockResolvedValue(mockRefreshedSession); + vi.spyOn( + authManager['supabaseClient'], + 'refreshSession' + ).mockImplementation(mockRefreshSession); + + // Get credentials should trigger refresh + const credentials = await authManager.getCredentials(); + + expect(mockRefreshSession).toHaveBeenCalledTimes(1); + expect(credentials).not.toBeNull(); + expect(credentials?.token).toBe('new-access-token-xyz'); + }); + + it('should not refresh valid token', async () => { + // Set up valid credentials + const validCredentials: AuthCredentials = { + token: 'valid-token', + refreshToken: 'valid-refresh-token', + userId: 'test-user-id', + email: 'test@example.com', + expiresAt: new Date(Date.now() + 3600000).toISOString(), // 1 hour from now + savedAt: new Date().toISOString() + }; + + credentialStore.saveCredentials(validCredentials); + + authManager = AuthManager.getInstance(); + + // Mock refresh to ensure it's not called + const mockRefreshSession = vi.fn(); + vi.spyOn( + authManager['supabaseClient'], + 'refreshSession' + ).mockImplementation(mockRefreshSession); + + const credentials = await authManager.getCredentials(); + + expect(mockRefreshSession).not.toHaveBeenCalled(); + expect(credentials?.token).toBe('valid-token'); + }); + }); + + describe('Token Refresh Flow', () => { + it('should refresh expired token and save new credentials', async () => { + const expiredCredentials: AuthCredentials = { + token: 'old-token', + refreshToken: 'old-refresh-token', + userId: 'test-user-id', + email: 'test@example.com', + expiresAt: new Date(Date.now() - 60000).toISOString(), + savedAt: new Date(Date.now() - 3600000).toISOString(), + selectedContext: { + orgId: 'test-org', + briefId: 'test-brief', + updatedAt: new Date().toISOString() + } + }; + + credentialStore.saveCredentials(expiredCredentials); + + authManager = AuthManager.getInstance(); + + vi.spyOn( + authManager['supabaseClient'], + 'refreshSession' + ).mockResolvedValue(mockRefreshedSession); + + const refreshedCredentials = await authManager.getCredentials(); + + expect(refreshedCredentials).not.toBeNull(); + expect(refreshedCredentials?.token).toBe('new-access-token-xyz'); + expect(refreshedCredentials?.refreshToken).toBe('new-refresh-token-xyz'); + + // Verify context was preserved + expect(refreshedCredentials?.selectedContext?.orgId).toBe('test-org'); + expect(refreshedCredentials?.selectedContext?.briefId).toBe('test-brief'); + + // Verify new expiration is in the future + const newExpiry = new Date(refreshedCredentials!.expiresAt!).getTime(); + const now = Date.now(); + expect(newExpiry).toBeGreaterThan(now); + }); + + it('should return null if refresh fails', async () => { + const expiredCredentials: AuthCredentials = { + token: 'expired-token', + refreshToken: 'invalid-refresh-token', + userId: 'test-user-id', + email: 'test@example.com', + expiresAt: new Date(Date.now() - 60000).toISOString(), + savedAt: new Date().toISOString() + }; + + credentialStore.saveCredentials(expiredCredentials); + + authManager = AuthManager.getInstance(); + + // Mock refresh to fail + vi.spyOn( + authManager['supabaseClient'], + 'refreshSession' + ).mockRejectedValue(new Error('Refresh token expired')); + + const credentials = await authManager.getCredentials(); + + expect(credentials).toBeNull(); + }); + + it('should return null if no refresh token available', async () => { + const expiredCredentials: AuthCredentials = { + token: 'expired-token', + // No refresh token + userId: 'test-user-id', + email: 'test@example.com', + expiresAt: new Date(Date.now() - 60000).toISOString(), + savedAt: new Date().toISOString() + }; + + credentialStore.saveCredentials(expiredCredentials); + + authManager = AuthManager.getInstance(); + + const credentials = await authManager.getCredentials(); + + expect(credentials).toBeNull(); + }); + + it('should return null if credentials missing expiresAt', async () => { + const credentialsWithoutExpiry: AuthCredentials = { + token: 'test-token', + refreshToken: 'refresh-token', + userId: 'test-user-id', + email: 'test@example.com', + // Missing expiresAt + savedAt: new Date().toISOString() + } as any; + + credentialStore.saveCredentials(credentialsWithoutExpiry); + + authManager = AuthManager.getInstance(); + + const credentials = await authManager.getCredentials(); + + // Should return null because no valid expiration + expect(credentials).toBeNull(); + }); + }); + + describe('Clock Skew Tolerance', () => { + it('should refresh token within 30-second expiry window', async () => { + // Token expires in 15 seconds (within 30-second buffer) + const almostExpiredCredentials: AuthCredentials = { + token: 'almost-expired-token', + refreshToken: 'valid-refresh-token', + userId: 'test-user-id', + email: 'test@example.com', + expiresAt: new Date(Date.now() + 15000).toISOString(), // 15 seconds from now + savedAt: new Date().toISOString() + }; + + credentialStore.saveCredentials(almostExpiredCredentials); + + authManager = AuthManager.getInstance(); + + const mockRefreshSession = vi + .fn() + .mockResolvedValue(mockRefreshedSession); + vi.spyOn( + authManager['supabaseClient'], + 'refreshSession' + ).mockImplementation(mockRefreshSession); + + const credentials = await authManager.getCredentials(); + + // Should trigger refresh due to 30-second buffer + expect(mockRefreshSession).toHaveBeenCalledTimes(1); + expect(credentials?.token).toBe('new-access-token-xyz'); + }); + + it('should not refresh token well before expiry', async () => { + // Token expires in 5 minutes (well outside 30-second buffer) + const validCredentials: AuthCredentials = { + token: 'valid-token', + refreshToken: 'valid-refresh-token', + userId: 'test-user-id', + email: 'test@example.com', + expiresAt: new Date(Date.now() + 300000).toISOString(), // 5 minutes + savedAt: new Date().toISOString() + }; + + credentialStore.saveCredentials(validCredentials); + + authManager = AuthManager.getInstance(); + + const mockRefreshSession = vi.fn(); + vi.spyOn( + authManager['supabaseClient'], + 'refreshSession' + ).mockImplementation(mockRefreshSession); + + const credentials = await authManager.getCredentials(); + + expect(mockRefreshSession).not.toHaveBeenCalled(); + expect(credentials?.token).toBe('valid-token'); + }); + }); + + describe('Synchronous vs Async Methods', () => { + it('getCredentialsSync should not trigger refresh', () => { + const expiredCredentials: AuthCredentials = { + token: 'expired-token', + refreshToken: 'valid-refresh-token', + userId: 'test-user-id', + email: 'test@example.com', + expiresAt: new Date(Date.now() - 60000).toISOString(), + savedAt: new Date().toISOString() + }; + + credentialStore.saveCredentials(expiredCredentials); + + authManager = AuthManager.getInstance(); + + // Synchronous call should return null without refresh + const credentials = authManager.getCredentialsSync(); + + expect(credentials).toBeNull(); + }); + + it('getCredentials async should trigger refresh', async () => { + const expiredCredentials: AuthCredentials = { + token: 'expired-token', + refreshToken: 'valid-refresh-token', + userId: 'test-user-id', + email: 'test@example.com', + expiresAt: new Date(Date.now() - 60000).toISOString(), + savedAt: new Date().toISOString() + }; + + credentialStore.saveCredentials(expiredCredentials); + + authManager = AuthManager.getInstance(); + + vi.spyOn( + authManager['supabaseClient'], + 'refreshSession' + ).mockResolvedValue(mockRefreshedSession); + + const credentials = await authManager.getCredentials(); + + expect(credentials).not.toBeNull(); + expect(credentials?.token).toBe('new-access-token-xyz'); + }); + }); + + describe('Multiple Concurrent Calls', () => { + it('should handle concurrent getCredentials calls gracefully', async () => { + const expiredCredentials: AuthCredentials = { + token: 'expired-token', + refreshToken: 'valid-refresh-token', + userId: 'test-user-id', + email: 'test@example.com', + expiresAt: new Date(Date.now() - 60000).toISOString(), + savedAt: new Date().toISOString() + }; + + credentialStore.saveCredentials(expiredCredentials); + + authManager = AuthManager.getInstance(); + + const mockRefreshSession = vi + .fn() + .mockResolvedValue(mockRefreshedSession); + vi.spyOn( + authManager['supabaseClient'], + 'refreshSession' + ).mockImplementation(mockRefreshSession); + + // Make multiple concurrent calls + const [creds1, creds2, creds3] = await Promise.all([ + authManager.getCredentials(), + authManager.getCredentials(), + authManager.getCredentials() + ]); + + // All should get the refreshed token + expect(creds1?.token).toBe('new-access-token-xyz'); + expect(creds2?.token).toBe('new-access-token-xyz'); + expect(creds3?.token).toBe('new-access-token-xyz'); + + // Refresh might be called multiple times, but that's okay + // (ideally we'd debounce, but this is acceptable behavior) + expect(mockRefreshSession).toHaveBeenCalled(); + }); + }); +});