From d63a40c6ddc1ed2fe418206a19acd3e9dc27fd99 Mon Sep 17 00:00:00 2001 From: Ralph Khreish <35776126+Crunchyman-ralph@users.noreply.github.com> Date: Tue, 28 Oct 2025 13:33:17 +0100 Subject: [PATCH] fix: improve auth refresh (#1340) --- .changeset/lazy-suits-smell.md | 5 + apps/cli/src/commands/auth.command.ts | 244 +++++-- apps/cli/src/commands/context.command.ts | 32 +- apps/cli/src/commands/export.command.ts | 3 +- .../tm-core/src/modules/auth/auth-domain.ts | 43 +- packages/tm-core/src/modules/auth/index.ts | 2 +- .../src/modules/auth/managers/auth-manager.ts | 261 ++++++-- .../modules/auth/services/context-store.ts | 181 ++++++ .../auth/services/credential-store.spec.ts | 308 --------- .../auth/services/credential-store.test.ts | 600 ------------------ .../modules/auth/services/credential-store.ts | 277 -------- .../modules/auth/services/oauth-service.ts | 48 +- .../auth/services/supabase-session-storage.ts | 247 +++---- packages/tm-core/src/modules/auth/types.ts | 4 +- .../integration/clients/supabase-client.ts | 51 +- .../integration/services/export.service.ts | 9 +- .../storage/services/storage-factory.ts | 42 +- .../modules/tasks/services/task-service.ts | 2 +- 18 files changed, 831 insertions(+), 1528 deletions(-) create mode 100644 .changeset/lazy-suits-smell.md create mode 100644 packages/tm-core/src/modules/auth/services/context-store.ts delete mode 100644 packages/tm-core/src/modules/auth/services/credential-store.spec.ts delete mode 100644 packages/tm-core/src/modules/auth/services/credential-store.test.ts delete mode 100644 packages/tm-core/src/modules/auth/services/credential-store.ts diff --git a/.changeset/lazy-suits-smell.md b/.changeset/lazy-suits-smell.md new file mode 100644 index 00000000..c98cd8df --- /dev/null +++ b/.changeset/lazy-suits-smell.md @@ -0,0 +1,5 @@ +--- +"task-master-ai": patch +--- + +Improve session persistence reliability \ No newline at end of file diff --git a/apps/cli/src/commands/auth.command.ts b/apps/cli/src/commands/auth.command.ts index b6cd41f5..55665546 100644 --- a/apps/cli/src/commands/auth.command.ts +++ b/apps/cli/src/commands/auth.command.ts @@ -62,8 +62,22 @@ export class AuthCommand extends Command { private addLoginCommand(): void { this.command('login') .description('Authenticate with tryhamster.com') - .action(async () => { - await this.executeLogin(); + .argument( + '[token]', + 'Authentication token (optional, for SSH/remote environments)' + ) + .option('-y, --yes', 'Skip interactive prompts') + .addHelpText( + 'after', + ` +Examples: + $ tm auth login # Browser-based OAuth flow (interactive) + $ tm auth login # Token-based authentication + $ tm auth login -y # Non-interactive token auth (for scripts) +` + ) + .action(async (token?: string, options?: { yes?: boolean }) => { + await this.executeLogin(token, options?.yes); }); } @@ -103,9 +117,11 @@ export class AuthCommand extends Command { /** * Execute login command */ - private async executeLogin(): Promise { + private async executeLogin(token?: string, yes?: boolean): Promise { try { - const result = await this.performInteractiveAuth(); + const result = token + ? await this.performTokenAuth(token, yes) + : await this.performInteractiveAuth(yes); this.setLastResult(result); if (!result.success) { @@ -143,7 +159,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) { displayError(error); @@ -169,21 +185,28 @@ export class AuthCommand extends Command { /** * Display authentication status */ - private displayStatus(): AuthResult { - const credentials = this.authManager.getCredentials(); - + private async displayStatus(): Promise { console.log(chalk.cyan('\nšŸ” Authentication Status\n')); - if (credentials) { - console.log(chalk.green('āœ“ Authenticated')); - console.log(chalk.gray(` Email: ${credentials.email || 'N/A'}`)); - console.log(chalk.gray(` User ID: ${credentials.userId}`)); - console.log( - chalk.gray(` Token Type: ${credentials.tokenType || 'standard'}`) - ); + // Check if user has valid session + const hasSession = await this.authManager.hasValidSession(); - if (credentials.expiresAt) { - const expiresAt = new Date(credentials.expiresAt); + if (hasSession) { + // Get session from Supabase (has tokens and expiry) + const session = await this.authManager.getSession(); + + // Get user context (has email, userId, org/brief selection) + const context = this.authManager.getContext(); + const contextStore = this.authManager.getStoredContext(); + + console.log(chalk.green('āœ“ Authenticated')); + console.log(chalk.gray(` Email: ${contextStore?.email || 'N/A'}`)); + console.log(chalk.gray(` User ID: ${contextStore?.userId || 'N/A'}`)); + console.log(chalk.gray(` Token Type: standard`)); + + // Display expiration info + if (session?.expires_at) { + const expiresAt = new Date(session.expires_at * 1000); const now = new Date(); const timeRemaining = expiresAt.getTime() - now.getTime(); const hoursRemaining = Math.floor(timeRemaining / (1000 * 60 * 60)); @@ -210,13 +233,32 @@ export class AuthCommand extends Command { chalk.yellow(` Expired at: ${expiresAt.toLocaleString()}`) ); } - } else { - console.log(chalk.gray(' Expires: Never (API key)')); } - console.log( - chalk.gray(` Saved: ${new Date(credentials.savedAt).toLocaleString()}`) - ); + // Display context if available + if (context) { + console.log(chalk.gray('\n Context:')); + if (context.orgName) { + console.log(chalk.gray(` Organization: ${context.orgName}`)); + } + if (context.briefName) { + console.log(chalk.gray(` Brief: ${context.briefName}`)); + } + } + + // Build credentials for backward compatibility + const credentials = { + token: session?.access_token || '', + refreshToken: session?.refresh_token, + userId: contextStore?.userId || '', + email: contextStore?.email, + expiresAt: session?.expires_at + ? new Date(session.expires_at * 1000).toISOString() + : undefined, + tokenType: 'standard' as const, + savedAt: contextStore?.lastUpdated || new Date().toISOString(), + selectedContext: context || undefined + }; return { success: true, @@ -307,11 +349,12 @@ export class AuthCommand extends Command { /** * Perform interactive authentication */ - private async performInteractiveAuth(): Promise { + private async performInteractiveAuth(yes?: boolean): Promise { ui.displayBanner('Task Master Authentication'); + const isAuthenticated = await this.authManager.hasValidSession(); - // Check if already authenticated - if (this.authManager.isAuthenticated()) { + // Check if already authenticated (skip if --yes is used) + if (isAuthenticated && !yes) { const { continueAuth } = await inquirer.prompt([ { type: 'confirm', @@ -323,7 +366,7 @@ export class AuthCommand extends Command { ]); if (!continueAuth) { - const credentials = this.authManager.getCredentials(); + const credentials = await this.authManager.getAuthCredentials(); ui.displaySuccess('Using existing authentication'); if (credentials) { @@ -349,35 +392,43 @@ export class AuthCommand extends Command { chalk.gray(` Logged in as: ${credentials.email || credentials.userId}`) ); - // Post-auth: Set up workspace context - console.log(); // Add spacing - try { - const contextCommand = new ContextCommand(); - const contextResult = await contextCommand.setupContextInteractive(); - if (contextResult.success) { - if (contextResult.orgSelected && contextResult.briefSelected) { + // Post-auth: Set up workspace context (skip if --yes flag is used) + if (!yes) { + console.log(); // Add spacing + try { + const contextCommand = new ContextCommand(); + const contextResult = await contextCommand.setupContextInteractive(); + if (contextResult.success) { + if (contextResult.orgSelected && contextResult.briefSelected) { + console.log( + chalk.green('āœ“ Workspace context configured successfully') + ); + } else if (contextResult.orgSelected) { + console.log(chalk.green('āœ“ Organization selected')); + } + } else { console.log( - chalk.green('āœ“ Workspace context configured successfully') + chalk.yellow('āš  Context setup was skipped or encountered issues') + ); + console.log( + chalk.gray(' You can set up context later with "tm context"') ); - } else if (contextResult.orgSelected) { - console.log(chalk.green('āœ“ Organization selected')); } - } else { - console.log( - chalk.yellow('āš  Context setup was skipped or encountered issues') - ); + } catch (contextError) { + console.log(chalk.yellow('āš  Context setup encountered an error')); console.log( chalk.gray(' You can set up context later with "tm context"') ); + if (process.env.DEBUG) { + console.error(chalk.gray((contextError as Error).message)); + } } - } catch (contextError) { - console.log(chalk.yellow('āš  Context setup encountered an error')); + } else { console.log( - chalk.gray(' You can set up context later with "tm context"') + chalk.gray( + '\n Skipped interactive setup. Use "tm context" to configure later.' + ) ); - if (process.env.DEBUG) { - console.error(chalk.gray((contextError as Error).message)); - } } return { @@ -450,6 +501,96 @@ export class AuthCommand extends Command { } } + /** + * Authenticate with token + */ + private async authenticateWithToken(token: string): Promise { + const spinner = ora('Verifying authentication token...').start(); + + try { + const credentials = await this.authManager.authenticateWithCode(token); + spinner.succeed('Successfully authenticated!'); + return credentials; + } catch (error) { + spinner.fail('Authentication failed'); + throw error; + } + } + + /** + * Perform token-based authentication flow + */ + private async performTokenAuth( + token: string, + yes?: boolean + ): Promise { + ui.displayBanner('Task Master Authentication'); + + try { + // Authenticate with the token + const credentials = await this.authenticateWithToken(token); + + ui.displaySuccess('Authentication successful!'); + console.log( + chalk.gray(` Logged in as: ${credentials.email || credentials.userId}`) + ); + + // Post-auth: Set up workspace context (skip if --yes flag is used) + if (!yes) { + console.log(); // Add spacing + try { + const contextCommand = new ContextCommand(); + const contextResult = await contextCommand.setupContextInteractive(); + if (contextResult.success) { + if (contextResult.orgSelected && contextResult.briefSelected) { + console.log( + chalk.green('āœ“ Workspace context configured successfully') + ); + } else if (contextResult.orgSelected) { + console.log(chalk.green('āœ“ Organization selected')); + } + } else { + console.log( + chalk.yellow('āš  Context setup was skipped or encountered issues') + ); + console.log( + chalk.gray(' You can set up context later with "tm context"') + ); + } + } catch (contextError) { + console.log(chalk.yellow('āš  Context setup encountered an error')); + console.log( + chalk.gray(' You can set up context later with "tm context"') + ); + if (process.env.DEBUG) { + console.error(chalk.gray((contextError as Error).message)); + } + } + } else { + console.log( + chalk.gray( + '\n Skipped interactive setup. Use "tm context" to configure later.' + ) + ); + } + + return { + success: true, + action: 'login', + credentials, + message: 'Authentication successful' + }; + } catch (error) { + displayError(error, { skipExit: true }); + + return { + success: false, + action: 'login', + message: `Authentication failed: ${(error as Error).message}` + }; + } + } + /** * Set the last result for programmatic access */ @@ -464,18 +605,11 @@ export class AuthCommand extends Command { return this.lastResult; } - /** - * Get current authentication status (for programmatic usage) - */ - isAuthenticated(): boolean { - return this.authManager.isAuthenticated(); - } - /** * Get current credentials (for programmatic usage) */ - getCredentials(): AuthCredentials | null { - return this.authManager.getCredentials(); + async getCredentials(): Promise { + return this.authManager.getAuthCredentials(); } /** diff --git a/apps/cli/src/commands/context.command.ts b/apps/cli/src/commands/context.command.ts index f032aeec..92498748 100644 --- a/apps/cli/src/commands/context.command.ts +++ b/apps/cli/src/commands/context.command.ts @@ -113,7 +113,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) { displayError(error); @@ -123,9 +123,10 @@ export class ContextCommand extends Command { /** * Display current context */ - private displayContext(): ContextResult { + private async displayContext(): Promise { // Check authentication first - if (!this.authManager.isAuthenticated()) { + const hasSession = await this.authManager.hasValidSession(); + if (!hasSession) { console.log(chalk.yellow('āœ— Not authenticated')); console.log(chalk.gray('\n Run "tm auth login" to authenticate first')); @@ -200,7 +201,8 @@ export class ContextCommand extends Command { private async executeSelectOrg(): Promise { try { // Check authentication - if (!this.authManager.isAuthenticated()) { + const hasSession = await this.authManager.hasValidSession(); + if (!hasSession) { ui.displayError('Not authenticated. Run "tm auth login" first.'); process.exit(1); } @@ -250,7 +252,7 @@ export class ContextCommand extends Command { ]); // Update context - this.authManager.updateContext({ + await this.authManager.updateContext({ orgId: selectedOrg.id, orgName: selectedOrg.name, orgSlug: selectedOrg.slug, @@ -279,7 +281,8 @@ export class ContextCommand extends Command { private async executeSelectBrief(): Promise { try { // Check authentication - if (!this.authManager.isAuthenticated()) { + const hasSession = await this.authManager.hasValidSession(); + if (!hasSession) { ui.displayError('Not authenticated. Run "tm auth login" first.'); process.exit(1); } @@ -371,7 +374,7 @@ export class ContextCommand extends Command { const briefName = selectedBrief.document?.title || `Brief ${selectedBrief.id.slice(0, 8)}`; - this.authManager.updateContext({ + await this.authManager.updateContext({ briefId: selectedBrief.id, briefName: briefName }); @@ -386,7 +389,7 @@ export class ContextCommand extends Command { }; } else { // Clear brief selection - this.authManager.updateContext({ + await this.authManager.updateContext({ briefId: undefined, briefName: undefined }); @@ -412,7 +415,8 @@ export class ContextCommand extends Command { private async executeClear(): Promise { try { // Check authentication - if (!this.authManager.isAuthenticated()) { + const hasSession = await this.authManager.hasValidSession(); + if (!hasSession) { ui.displayError('Not authenticated. Run "tm auth login" first.'); process.exit(1); } @@ -458,7 +462,8 @@ export class ContextCommand extends Command { private async executeSet(options: any): Promise { try { // Check authentication - if (!this.authManager.isAuthenticated()) { + const hasSession = await this.authManager.hasValidSession(); + if (!hasSession) { ui.displayError('Not authenticated. Run "tm auth login" first.'); process.exit(1); } @@ -481,7 +486,8 @@ export class ContextCommand extends Command { let spinner: Ora | undefined; try { // Check authentication - if (!this.authManager.isAuthenticated()) { + const hasSession = await this.authManager.hasValidSession(); + if (!hasSession) { ui.displayError('Not authenticated. Run "tm auth login" first.'); process.exit(1); } @@ -520,7 +526,7 @@ export class ContextCommand extends Command { // Update context: set org and brief const briefName = brief.document?.title || `Brief ${brief.id.slice(0, 8)}`; - this.authManager.updateContext({ + await this.authManager.updateContext({ orgId: brief.accountId, orgName, orgSlug, @@ -642,7 +648,7 @@ export class ContextCommand extends Command { }; } - this.authManager.updateContext(context); + await this.authManager.updateContext(context); ui.displaySuccess('Context updated'); // Display what was set diff --git a/apps/cli/src/commands/export.command.ts b/apps/cli/src/commands/export.command.ts index 84cf2b3a..1ca2011f 100644 --- a/apps/cli/src/commands/export.command.ts +++ b/apps/cli/src/commands/export.command.ts @@ -96,7 +96,8 @@ export class ExportCommand extends Command { try { // Check authentication - if (!this.authManager.isAuthenticated()) { + const hasSession = await this.authManager.hasValidSession(); + if (!hasSession) { ui.displayError('Not authenticated. Run "tm auth login" first.'); process.exit(1); } diff --git a/packages/tm-core/src/modules/auth/auth-domain.ts b/packages/tm-core/src/modules/auth/auth-domain.ts index e137a66b..10f38dc3 100644 --- a/packages/tm-core/src/modules/auth/auth-domain.ts +++ b/packages/tm-core/src/modules/auth/auth-domain.ts @@ -44,17 +44,38 @@ export class AuthDomain { // ========== Authentication ========== /** - * Check if user is authenticated + * Check if valid Supabase session exists */ - isAuthenticated(): boolean { - return this.authManager.isAuthenticated(); + async hasValidSession(): Promise { + return this.authManager.hasValidSession(); + } + + /** + * Get the current Supabase session with full details + */ + async getSession() { + return this.authManager.getSession(); + } + + /** + * Get stored user context (userId, email) + */ + getStoredContext() { + return this.authManager.getStoredContext(); } /** * Get stored credentials */ - getCredentials(): AuthCredentials | null { - return this.authManager.getCredentials(); + async getCredentials(): Promise { + return this.authManager.getAuthCredentials(); + } + + /** + * Get access token from current session + */ + async getAccessToken(): Promise { + return this.authManager.getAccessToken(); } /** @@ -66,6 +87,14 @@ export class AuthDomain { return this.authManager.authenticateWithOAuth(options); } + /** + * Authenticate using a one-time token + * Useful for CLI authentication in SSH/remote environments + */ + async authenticateWithCode(token: string): Promise { + return this.authManager.authenticateWithCode(token); + } + /** * Get OAuth authorization URL */ @@ -99,14 +128,14 @@ export class AuthDomain { /** * Update user context */ - updateContext(context: Partial): void { + async updateContext(context: Partial): Promise { return this.authManager.updateContext(context); } /** * Clear user context */ - clearContext(): void { + async clearContext(): Promise { return this.authManager.clearContext(); } diff --git a/packages/tm-core/src/modules/auth/index.ts b/packages/tm-core/src/modules/auth/index.ts index 922519e9..0396a494 100644 --- a/packages/tm-core/src/modules/auth/index.ts +++ b/packages/tm-core/src/modules/auth/index.ts @@ -4,7 +4,7 @@ export { AuthDomain, type StorageDisplayInfo } from './auth-domain.js'; export { AuthManager } from './managers/auth-manager.js'; -export { CredentialStore } from './services/credential-store.js'; +export { ContextStore, type StoredContext } from './services/context-store.js'; export { OAuthService } from './services/oauth-service.js'; export { SupabaseSessionStorage } from './services/supabase-session-storage.js'; export type { diff --git a/packages/tm-core/src/modules/auth/managers/auth-manager.ts b/packages/tm-core/src/modules/auth/managers/auth-manager.ts index 883105df..79a730a3 100644 --- a/packages/tm-core/src/modules/auth/managers/auth-manager.ts +++ b/packages/tm-core/src/modules/auth/managers/auth-manager.ts @@ -9,7 +9,7 @@ import { AuthConfig, UserContext } from '../types.js'; -import { CredentialStore } from '../services/credential-store.js'; +import { ContextStore } from '../services/context-store.js'; import { OAuthService } from '../services/oauth-service.js'; import { SupabaseAuthClient } from '../../integration/clients/supabase-client.js'; import { @@ -19,6 +19,9 @@ import { type RemoteTask } from '../services/organization.service.js'; import { getLogger } from '../../../common/logger/index.js'; +import fs from 'fs'; +import path from 'path'; +import os from 'os'; /** * Authentication manager class @@ -26,22 +29,38 @@ import { getLogger } from '../../../common/logger/index.js'; export class AuthManager { private static instance: AuthManager | null = null; private static readonly staticLogger = getLogger('AuthManager'); - private credentialStore: CredentialStore; + private contextStore: ContextStore; private oauthService: OAuthService; public supabaseClient: SupabaseAuthClient; private organizationService?: OrganizationService; private readonly logger = getLogger('AuthManager'); + private readonly LEGACY_AUTH_FILE = path.join( + os.homedir(), + '.taskmaster', + 'auth.json' + ); private constructor(config?: Partial) { - this.credentialStore = CredentialStore.getInstance(config); + this.contextStore = ContextStore.getInstance(); this.supabaseClient = new SupabaseAuthClient(); - this.oauthService = new OAuthService(this.credentialStore, config); + // Pass the supabase client to OAuthService so they share the same instance + this.oauthService = new OAuthService( + this.contextStore, + this.supabaseClient, + config + ); // Initialize Supabase client with session restoration // Fire-and-forget with catch handler to prevent unhandled rejections this.initializeSupabaseSession().catch(() => { // Errors are already logged in initializeSupabaseSession }); + + // Migrate legacy auth.json if it exists + // Fire-and-forget with catch handler + this.migrateLegacyAuth().catch(() => { + // Errors are already logged in migrateLegacyAuth + }); } /** @@ -56,6 +75,32 @@ export class AuthManager { } } + /** + * Migrate legacy auth.json to Supabase session + * Called once during AuthManager initialization + */ + private async migrateLegacyAuth(): Promise { + if (!fs.existsSync(this.LEGACY_AUTH_FILE)) { + return; + } + + try { + // If we have a valid Supabase session, delete legacy file + const hasSession = await this.hasValidSession(); + if (hasSession) { + fs.unlinkSync(this.LEGACY_AUTH_FILE); + this.logger.info('Migrated to Supabase auth, removed legacy auth.json'); + return; + } + + // Otherwise, user needs to re-authenticate + this.logger.warn('Legacy auth.json found but no valid Supabase session.'); + this.logger.warn('Please run: task-master auth login'); + } catch (error) { + this.logger.debug('Error during legacy auth migration:', error); + } + } + /** * Get singleton instance */ @@ -76,16 +121,42 @@ export class AuthManager { */ static resetInstance(): void { AuthManager.instance = null; - CredentialStore.resetInstance(); + ContextStore.resetInstance(); } /** - * Get stored authentication credentials - * Returns credentials as-is (even if expired). Refresh must be triggered explicitly - * via refreshToken() or will occur automatically when using the Supabase client for API calls. + * Get access token from current Supabase session + * @returns Access token or null if not authenticated */ - getCredentials(): AuthCredentials | null { - return this.credentialStore.getCredentials(); + async getAccessToken(): Promise { + const session = await this.supabaseClient.getSession(); + return session?.access_token || null; + } + + /** + * Get authentication credentials from Supabase session + * Modern replacement for legacy getCredentials() + * @returns AuthCredentials object or null if not authenticated + */ + async getAuthCredentials(): Promise { + const session = await this.supabaseClient.getSession(); + if (!session) return null; + + const user = session.user; + const context = this.contextStore.getUserContext(); + + return { + token: session.access_token, + refreshToken: session.refresh_token, + userId: user.id, + email: user.email, + expiresAt: session.expires_at + ? new Date(session.expires_at * 1000).toISOString() + : undefined, + tokenType: 'standard', + savedAt: new Date().toISOString(), + selectedContext: context || undefined + }; } /** @@ -97,6 +168,69 @@ export class AuthManager { return this.oauthService.authenticate(options); } + /** + * Authenticate using a one-time token + * This is useful for CLI authentication in SSH/remote environments + * where browser-based auth is not practical + */ + async authenticateWithCode(token: string): Promise { + try { + this.logger.info('Authenticating with one-time token...'); + + // Verify the token and get session from Supabase + const session = await this.supabaseClient.verifyOneTimeCode(token); + + if (!session || !session.access_token) { + throw new AuthenticationError( + 'Failed to obtain access token from token', + 'NO_TOKEN' + ); + } + + // Get user information + const user = await this.supabaseClient.getUser(); + + if (!user) { + throw new AuthenticationError( + 'Failed to get user information', + 'INVALID_RESPONSE' + ); + } + + // Store user context + this.contextStore.saveContext({ + userId: user.id, + email: user.email + }); + + // Build credentials response + const context = this.contextStore.getUserContext(); + const credentials: AuthCredentials = { + token: session.access_token, + refreshToken: session.refresh_token, + userId: user.id, + email: user.email, + expiresAt: session.expires_at + ? new Date(session.expires_at * 1000).toISOString() + : undefined, + tokenType: 'standard', + savedAt: new Date().toISOString(), + selectedContext: context || undefined + }; + + this.logger.info('Successfully authenticated with token'); + return credentials; + } catch (error) { + if (error instanceof AuthenticationError) { + throw error; + } + throw new AuthenticationError( + `Token authentication failed: ${(error as Error).message}`, + 'CODE_AUTH_FAILED' + ); + } + } + /** * Get the authorization URL (for browser opening) */ @@ -106,6 +240,8 @@ export class AuthManager { /** * Refresh authentication token using Supabase session + * Note: Supabase handles token refresh automatically via the session storage adapter. + * This method is mainly for explicit refresh requests. */ async refreshToken(): Promise { try { @@ -119,13 +255,15 @@ export class AuthManager { ); } - // Get existing credentials to preserve context - const existingCredentials = this.credentialStore.getCredentials({ - allowExpired: true + // Sync user info to context store + this.contextStore.saveContext({ + userId: session.user.id, + email: session.user.email }); - // Update authentication data from session - const newAuthData: AuthCredentials = { + // Build credentials response + const context = this.contextStore.getContext(); + const credentials: AuthCredentials = { token: session.access_token, refreshToken: session.refresh_token, userId: session.user.id, @@ -134,11 +272,10 @@ export class AuthManager { ? new Date(session.expires_at * 1000).toISOString() : undefined, savedAt: new Date().toISOString(), - selectedContext: existingCredentials?.selectedContext + selectedContext: context?.selectedContext }; - this.credentialStore.saveCredentials(newAuthData); - return newAuthData; + return credentials; } catch (error) { if (error instanceof AuthenticationError) { throw error; @@ -162,81 +299,91 @@ export class AuthManager { this.logger.warn('Failed to sign out from Supabase:', error); } - // Always clear local credentials (removes auth.json file) - this.credentialStore.clearCredentials(); + // Clear app context + this.contextStore.clearContext(); + // Session is cleared by supabaseClient.signOut() + + // Clear legacy auth.json if it exists + try { + if (fs.existsSync(this.LEGACY_AUTH_FILE)) { + fs.unlinkSync(this.LEGACY_AUTH_FILE); + this.logger.debug('Cleared legacy auth.json'); + } + } catch (error) { + // Ignore errors clearing legacy file + this.logger.debug('No legacy credentials to clear'); + } } /** - * Check if authenticated (credentials exist, regardless of expiration) - * @returns true if credentials are stored, including expired credentials + * Check if valid Supabase session exists + * @returns true if a valid session exists */ - isAuthenticated(): boolean { - return this.credentialStore.hasCredentials(); + async hasValidSession(): Promise { + try { + const session = await this.supabaseClient.getSession(); + return session !== null; + } catch { + return false; + } + } + + /** + * Get the current Supabase session + */ + async getSession() { + return this.supabaseClient.getSession(); + } + + /** + * Get stored user context (userId, email) + */ + getStoredContext() { + return this.contextStore.getContext(); } /** * Get the current user context (org/brief selection) */ getContext(): UserContext | null { - const credentials = this.getCredentials(); - return credentials?.selectedContext || null; + return this.contextStore.getUserContext(); } /** * Update the user context (org/brief selection) */ - updateContext(context: Partial): void { - const credentials = this.getCredentials(); - if (!credentials) { + async updateContext(context: Partial): Promise { + if (!(await this.hasValidSession())) { throw new AuthenticationError('Not authenticated', 'NOT_AUTHENTICATED'); } - // Merge with existing context - const existingContext = credentials.selectedContext || {}; - const newContext: UserContext = { - ...existingContext, - ...context, - updatedAt: new Date().toISOString() - }; - - // Save updated credentials with new context - const updatedCredentials: AuthCredentials = { - ...credentials, - selectedContext: newContext - }; - - this.credentialStore.saveCredentials(updatedCredentials); + this.contextStore.updateUserContext(context); } /** * Clear the user context */ - clearContext(): void { - const credentials = this.getCredentials(); - if (!credentials) { + async clearContext(): Promise { + if (!(await this.hasValidSession())) { throw new AuthenticationError('Not authenticated', 'NOT_AUTHENTICATED'); } - // Remove context from credentials - const { selectedContext, ...credentialsWithoutContext } = credentials; - this.credentialStore.saveCredentials(credentialsWithoutContext); + this.contextStore.clearUserContext(); } /** * Get the organization service instance - * Uses the Supabase client with the current session or token + * Uses the Supabase client with the current session */ private async getOrganizationService(): Promise { if (!this.organizationService) { - // First check if we have credentials with a token - const credentials = this.getCredentials(); - if (!credentials || !credentials.token) { + // Check if we have a valid Supabase session + const session = await this.supabaseClient.getSession(); + + if (!session) { throw new AuthenticationError('Not authenticated', 'NOT_AUTHENTICATED'); } - // Initialize session if needed (this will load from our storage adapter) - await this.supabaseClient.initialize(); - // Use the SupabaseAuthClient which now has the session const supabaseClient = this.supabaseClient.getClient(); this.organizationService = new OrganizationService(supabaseClient as any); diff --git a/packages/tm-core/src/modules/auth/services/context-store.ts b/packages/tm-core/src/modules/auth/services/context-store.ts new file mode 100644 index 00000000..1a1ce69d --- /dev/null +++ b/packages/tm-core/src/modules/auth/services/context-store.ts @@ -0,0 +1,181 @@ +/** + * Context storage for app-specific user preferences + * + * This store manages user preferences and context separate from auth tokens. + * - selectedContext (org/brief selection) + * - userId and email (for convenience) + * - Any other app-specific data + * + * Stored at: ~/.taskmaster/context.json + */ + +import fs from 'fs'; +import path from 'path'; +import { UserContext, AuthenticationError } from '../types.js'; +import { getLogger } from '../../../common/logger/index.js'; + +const DEFAULT_CONTEXT_FILE = path.join( + process.env.HOME || process.env.USERPROFILE || '~', + '.taskmaster', + 'context.json' +); + +export interface StoredContext { + userId?: string; + email?: string; + selectedContext?: UserContext; + lastUpdated: string; +} + +export class ContextStore { + private static instance: ContextStore | null = null; + private logger = getLogger('ContextStore'); + private contextPath: string; + + private constructor(contextPath: string = DEFAULT_CONTEXT_FILE) { + this.contextPath = contextPath; + } + + /** + * Get singleton instance + */ + static getInstance(contextPath?: string): ContextStore { + if (!ContextStore.instance) { + ContextStore.instance = new ContextStore(contextPath); + } + return ContextStore.instance; + } + + /** + * Reset singleton (for testing) + */ + static resetInstance(): void { + ContextStore.instance = null; + } + + /** + * Get stored context + */ + getContext(): StoredContext | null { + try { + if (!fs.existsSync(this.contextPath)) { + return null; + } + + const data = JSON.parse(fs.readFileSync(this.contextPath, 'utf8')); + this.logger.debug('Loaded context from disk'); + return data; + } catch (error) { + this.logger.error('Failed to read context:', error); + return null; + } + } + + /** + * Save context + */ + saveContext(context: Partial): void { + try { + // Load existing context + const existing = this.getContext() || {}; + + // Merge with new data + const updated: StoredContext = { + ...existing, + ...context, + lastUpdated: new Date().toISOString() + }; + + // Ensure directory exists + const dir = path.dirname(this.contextPath); + if (!fs.existsSync(dir)) { + fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); + } + + // Write atomically + const tempFile = `${this.contextPath}.tmp`; + fs.writeFileSync(tempFile, JSON.stringify(updated, null, 2), { + mode: 0o600 + }); + fs.renameSync(tempFile, this.contextPath); + + this.logger.debug('Saved context to disk'); + } catch (error) { + throw new AuthenticationError( + `Failed to save context: ${(error as Error).message}`, + 'SAVE_FAILED', + error + ); + } + } + + /** + * Update user context (org/brief selection) + */ + updateUserContext(userContext: Partial): void { + const existing = this.getContext(); + const currentUserContext = existing?.selectedContext || {}; + + const updated: UserContext = { + ...currentUserContext, + ...userContext, + updatedAt: new Date().toISOString() + }; + + this.saveContext({ + ...existing, + selectedContext: updated + }); + } + + /** + * Get user context (org/brief selection) + */ + getUserContext(): UserContext | null { + const context = this.getContext(); + return context?.selectedContext || null; + } + + /** + * Clear user context + */ + clearUserContext(): void { + const existing = this.getContext(); + if (existing) { + const { selectedContext, ...rest } = existing; + this.saveContext(rest); + } + } + + /** + * Clear all context + */ + clearContext(): void { + try { + if (fs.existsSync(this.contextPath)) { + fs.unlinkSync(this.contextPath); + this.logger.debug('Cleared context from disk'); + } + } catch (error) { + throw new AuthenticationError( + `Failed to clear context: ${(error as Error).message}`, + 'CLEAR_FAILED', + error + ); + } + } + + /** + * Check if context exists + */ + hasContext(): boolean { + return this.getContext() !== null; + } + + /** + * Get context file path + */ + getContextPath(): string { + return this.contextPath; + } +} diff --git a/packages/tm-core/src/modules/auth/services/credential-store.spec.ts b/packages/tm-core/src/modules/auth/services/credential-store.spec.ts deleted file mode 100644 index 9ac43d0d..00000000 --- a/packages/tm-core/src/modules/auth/services/credential-store.spec.ts +++ /dev/null @@ -1,308 +0,0 @@ -/** - * @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({ allowExpired: false }); - - 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({ allowExpired: false }); - - 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'); - }); - - it('should return expired token by default (allowExpired defaults to true)', () => { - const expiredCredentials: AuthCredentials = { - token: 'expired-token-default', - refreshToken: 'refresh-token', - userId: 'test-user', - email: 'test@example.com', - expiresAt: new Date(Date.now() - 60000).toISOString(), - savedAt: new Date().toISOString() - }; - - credentialStore.saveCredentials(expiredCredentials); - - // Call without options - should default to allowExpired: true - const retrieved = credentialStore.getCredentials(); - - expect(retrieved).not.toBeNull(); - expect(retrieved?.token).toBe('expired-token-default'); - }); - }); - - 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({ allowExpired: false }); - - 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({ allowExpired: false }); - - 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({ allowExpired: false }); - - 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({ allowExpired: false }); - - 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({ allowExpired: false }); - - 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({ allowExpired: false }); - - 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({ allowExpired: false }); - - // Should be normalized to number for runtime use - expect(typeof retrieved?.expiresAt).toBe('number'); - }); - }); - - describe('hasCredentials', () => { - it('should return true 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.hasCredentials()).toBe(true); - }); - - 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.hasCredentials()).toBe(true); - }); - - it('should return false when no credentials exist', () => { - expect(credentialStore.hasCredentials()).toBe(false); - }); - }); -}); diff --git a/packages/tm-core/src/modules/auth/services/credential-store.test.ts b/packages/tm-core/src/modules/auth/services/credential-store.test.ts deleted file mode 100644 index 4518278d..00000000 --- a/packages/tm-core/src/modules/auth/services/credential-store.test.ts +++ /dev/null @@ -1,600 +0,0 @@ -/** - * Tests for CredentialStore with numeric and string timestamp handling - */ - -import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; -import { CredentialStore } from '../services/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({ allowExpired: false }); - - 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'); - }); - - it('should return expired tokens by default (allowExpired defaults to true)', () => { - const expiredTimestamp = Date.now() - 3600000; // 1 hour ago - const mockCredentials = { - token: 'expired-token-default', - 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) - ); - - // Call without options - should default to allowExpired: true - const result = store.getCredentials(); - - expect(result).not.toBeNull(); - expect(result?.token).toBe('expired-token-default'); - expect(mockLogger.warn).not.toHaveBeenCalledWith( - expect.stringContaining('Authentication token has expired') - ); - }); - }); - - 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() - }; - - let err: unknown; - try { - store.saveCredentials(credentials); - } catch (e) { - err = e; - } - expect(err).toBeInstanceOf(AuthenticationError); - expect((err as Error).message).toContain('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() - }; - - let err: unknown; - try { - store.saveCredentials(credentials); - } catch (e) { - err = e; - } - expect(err).toBeInstanceOf(AuthenticationError); - expect((err as Error).message).toContain('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() - }; - - let err: unknown; - try { - store.saveCredentials(credentials); - } catch (e) { - err = e; - } - expect(err).toBeInstanceOf(AuthenticationError); - expect((err as Error).message).toContain('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('clearCredentials', () => { - it('should delete the auth file when it exists', () => { - // Mock file exists - vi.mocked(fs.existsSync).mockReturnValue(true); - vi.mocked(fs.unlinkSync).mockImplementation(() => undefined); - - store.clearCredentials(); - - expect(fs.existsSync).toHaveBeenCalledWith('/test/config/auth.json'); - expect(fs.unlinkSync).toHaveBeenCalledWith('/test/config/auth.json'); - }); - - it('should not throw when auth file does not exist', () => { - // Mock file does not exist - vi.mocked(fs.existsSync).mockReturnValue(false); - - // Should not throw - expect(() => store.clearCredentials()).not.toThrow(); - - // Should not try to unlink non-existent file - expect(fs.unlinkSync).not.toHaveBeenCalled(); - }); - - it('should throw AuthenticationError when unlink fails', () => { - vi.mocked(fs.existsSync).mockReturnValue(true); - vi.mocked(fs.unlinkSync).mockImplementation(() => { - throw new Error('Permission denied'); - }); - - let err: unknown; - try { - store.clearCredentials(); - } catch (e) { - err = e; - } - - expect(err).toBeInstanceOf(AuthenticationError); - expect((err as Error).message).toContain('Failed to clear credentials'); - expect((err as Error).message).toContain('Permission denied'); - }); - }); - - describe('hasCredentials', () => { - it('should return true when valid unexpired credentials exist', () => { - const futureDate = new Date(Date.now() + 3600000); // 1 hour from now - const credentials = { - token: 'valid-token', - userId: 'user-123', - expiresAt: futureDate.toISOString(), - tokenType: 'standard', - savedAt: new Date().toISOString() - }; - - vi.mocked(fs.existsSync).mockReturnValue(true); - vi.mocked(fs.readFileSync).mockReturnValue(JSON.stringify(credentials)); - - expect(store.hasCredentials()).toBe(true); - }); - - it('should return true when credentials are expired', () => { - const pastDate = new Date(Date.now() - 3600000); // 1 hour ago - const credentials = { - token: 'expired-token', - userId: 'user-123', - expiresAt: pastDate.toISOString(), - tokenType: 'standard', - savedAt: new Date().toISOString() - }; - - vi.mocked(fs.existsSync).mockReturnValue(true); - vi.mocked(fs.readFileSync).mockReturnValue(JSON.stringify(credentials)); - - expect(store.hasCredentials()).toBe(true); - }); - - it('should return false when no credentials exist', () => { - vi.mocked(fs.existsSync).mockReturnValue(false); - - expect(store.hasCredentials()).toBe(false); - }); - - it('should return false when file contains invalid JSON', () => { - vi.mocked(fs.existsSync).mockReturnValue(true); - vi.mocked(fs.readFileSync).mockReturnValue('invalid json {'); - vi.mocked(fs.renameSync).mockImplementation(() => undefined); - - expect(store.hasCredentials()).toBe(false); - }); - - it('should return false for credentials without expiry', () => { - const credentials = { - token: 'no-expiry-token', - userId: 'user-123', - tokenType: 'standard', - savedAt: new Date().toISOString() - }; - - vi.mocked(fs.existsSync).mockReturnValue(true); - vi.mocked(fs.readFileSync).mockReturnValue(JSON.stringify(credentials)); - - // Credentials without expiry are considered invalid - expect(store.hasCredentials()).toBe(false); - - // Should log warning about missing expiration - expect(mockLogger.warn).toHaveBeenCalledWith( - 'No valid expiration time provided for token' - ); - }); - - it('should use allowExpired=true', () => { - // Spy on getCredentials to verify it's called with correct params - const getCredentialsSpy = vi.spyOn(store, 'getCredentials'); - - vi.mocked(fs.existsSync).mockReturnValue(false); - store.hasCredentials(); - - expect(getCredentialsSpy).toHaveBeenCalledWith({ allowExpired: true }); - }); - }); - - 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') - ); - }); - }); -}); diff --git a/packages/tm-core/src/modules/auth/services/credential-store.ts b/packages/tm-core/src/modules/auth/services/credential-store.ts deleted file mode 100644 index c0acbf78..00000000 --- a/packages/tm-core/src/modules/auth/services/credential-store.ts +++ /dev/null @@ -1,277 +0,0 @@ -/** - * Credential storage and management - */ - -import fs from 'fs'; -import path from 'path'; -import { AuthCredentials, AuthenticationError, AuthConfig } from '../types.js'; -import { getAuthConfig } from '../config.js'; -import { getLogger } from '../../../common/logger/index.js'; - -/** - * CredentialStore manages the persistence and retrieval of authentication credentials. - * - * Runtime vs Persisted Shape: - * - When retrieved (getCredentials): expiresAt is normalized to number (milliseconds since epoch) - * - When persisted (saveCredentials): expiresAt is stored as ISO string for readability - * - * This normalization ensures consistent runtime behavior while maintaining - * human-readable persisted format in the auth.json file. - */ -export class CredentialStore { - private static instance: CredentialStore | null = null; - private logger = getLogger('CredentialStore'); - private config: AuthConfig; - // Clock skew tolerance for expiry checks (30 seconds) - private readonly CLOCK_SKEW_MS = 30_000; - // Track if we've already warned about missing expiration to avoid spam - private hasWarnedAboutMissingExpiration = false; - - private constructor(config?: Partial) { - this.config = getAuthConfig(config); - } - - /** - * Get the singleton instance of CredentialStore - */ - static getInstance(config?: Partial): CredentialStore { - if (!CredentialStore.instance) { - CredentialStore.instance = new CredentialStore(config); - } else if (config) { - // Warn if config is provided after initialization - const logger = getLogger('CredentialStore'); - logger.warn( - 'getInstance called with config after initialization; config is ignored.' - ); - } - return CredentialStore.instance; - } - - /** - * Reset the singleton instance (useful for testing) - */ - static resetInstance(): void { - CredentialStore.instance = null; - } - - /** - * Get stored authentication credentials - * @param options.allowExpired - Whether to return expired credentials (default: true) - * @returns AuthCredentials with expiresAt as number (milliseconds) for runtime use - */ - getCredentials({ - allowExpired = true - }: { allowExpired?: boolean } = {}): AuthCredentials | null { - try { - if (!fs.existsSync(this.config.configFile)) { - return null; - } - - const authData = JSON.parse( - fs.readFileSync(this.config.configFile, 'utf-8') - ) as AuthCredentials; - - // Normalize/migrate timestamps to numeric (handles both number and ISO string) - let expiresAtMs: number | undefined; - 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; - } - - // Validate expiration time for tokens - if (expiresAtMs === undefined) { - // Only log this warning once to avoid spam during auth flows - if (!this.hasWarnedAboutMissingExpiration) { - this.logger.warn('No valid expiration time provided for token'); - this.hasWarnedAboutMissingExpiration = true; - } - return null; - } - - // Update the authData with normalized timestamp - authData.expiresAt = expiresAtMs; - - // Check if the token has expired (with clock skew tolerance) - const now = Date.now(); - if (now >= expiresAtMs - this.CLOCK_SKEW_MS && !allowExpired) { - this.logger.warn( - 'Authentication token has expired or is about to expire', - { - expiresAt: authData.expiresAt, - currentTime: new Date(now).toISOString(), - skewWindow: `${this.CLOCK_SKEW_MS / 1000}s` - } - ); - return null; - } - - // Return credentials (even if expired) to enable refresh flows - return authData; - } catch (error) { - this.logger.error( - `Failed to read auth credentials: ${(error as Error).message}` - ); - - // Quarantine corrupt file to prevent repeated errors - try { - if (fs.existsSync(this.config.configFile)) { - const corruptFile = `${this.config.configFile}.corrupt-${Date.now()}-${process.pid}-${Math.random().toString(36).slice(2, 8)}`; - fs.renameSync(this.config.configFile, corruptFile); - this.logger.warn(`Quarantined corrupt auth file to: ${corruptFile}`); - } - } catch (quarantineError) { - // If we can't quarantine, log but don't throw - this.logger.debug( - `Could not quarantine corrupt file: ${(quarantineError as Error).message}` - ); - } - - return null; - } - } - - /** - * Save authentication credentials - * @param authData - Credentials with expiresAt as number or string (will be persisted as ISO string) - */ - saveCredentials(authData: AuthCredentials): void { - try { - // Ensure directory exists - if (!fs.existsSync(this.config.configDir)) { - fs.mkdirSync(this.config.configDir, { recursive: true, mode: 0o700 }); - } - - // Add timestamp without mutating caller's object - authData = { ...authData, savedAt: new Date().toISOString() }; - - // 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 - const tempFile = `${this.config.configFile}.tmp`; - fs.writeFileSync(tempFile, JSON.stringify(authData, null, 2), { - mode: 0o600 - }); - fs.renameSync(tempFile, this.config.configFile); - - // Reset the warning flag so it can be shown again for future invalid tokens - this.hasWarnedAboutMissingExpiration = false; - } catch (error) { - throw new AuthenticationError( - `Failed to save auth credentials: ${(error as Error).message}`, - 'SAVE_FAILED', - error - ); - } - } - - /** - * Clear stored credentials - */ - clearCredentials(): void { - try { - if (fs.existsSync(this.config.configFile)) { - fs.unlinkSync(this.config.configFile); - } - } catch (error) { - throw new AuthenticationError( - `Failed to clear credentials: ${(error as Error).message}`, - 'CLEAR_FAILED', - error - ); - } - } - - /** - * Check if credentials exist (regardless of expiration status) - * @returns true if credentials are stored, including expired credentials - */ - hasCredentials(): boolean { - const credentials = this.getCredentials({ allowExpired: true }); - return credentials !== null; - } - - /** - * Get configuration - */ - getConfig(): AuthConfig { - return { ...this.config }; - } - - /** - * Clean up old corrupt auth files - * Removes corrupt files older than the specified age - */ - cleanupCorruptFiles(maxAgeMs: number = 7 * 24 * 60 * 60 * 1000): void { - try { - const dir = path.dirname(this.config.configFile); - const baseName = path.basename(this.config.configFile); - const prefix = `${baseName}.corrupt-`; - - if (!fs.existsSync(dir)) { - return; - } - - const entries = fs.readdirSync(dir, { withFileTypes: true }); - const now = Date.now(); - - for (const entry of entries) { - if (!entry.isFile()) continue; - const file = entry.name; - - // Check if file matches pattern: baseName.corrupt-{timestamp} - if (!file.startsWith(prefix)) continue; - const suffix = file.slice(prefix.length); - if (!/^\d+$/.test(suffix)) continue; // Fixed regex, not from variable input - - const filePath = path.join(dir, file); - try { - const stats = fs.statSync(filePath); - const age = now - stats.mtimeMs; - - if (age > maxAgeMs) { - fs.unlinkSync(filePath); - this.logger.debug(`Cleaned up old corrupt file: ${file}`); - } - } catch (error) { - // Ignore errors for individual file cleanup - this.logger.debug( - `Could not clean up corrupt file ${file}: ${(error as Error).message}` - ); - } - } - } catch (error) { - // Log but don't throw - this is a cleanup operation - this.logger.debug( - `Error during corrupt file cleanup: ${(error as Error).message}` - ); - } - } -} diff --git a/packages/tm-core/src/modules/auth/services/oauth-service.ts b/packages/tm-core/src/modules/auth/services/oauth-service.ts index 4a7dfe46..81f197c1 100644 --- a/packages/tm-core/src/modules/auth/services/oauth-service.ts +++ b/packages/tm-core/src/modules/auth/services/oauth-service.ts @@ -13,15 +13,16 @@ import { AuthConfig, CliData } from '../types.js'; -import { CredentialStore } from '../services/credential-store.js'; +import { ContextStore } from '../services/context-store.js'; import { SupabaseAuthClient } from '../../integration/clients/supabase-client.js'; import { getAuthConfig } from '../config.js'; import { getLogger } from '../../../common/logger/index.js'; import packageJson from '../../../../../../package.json' with { type: 'json' }; +import { Session } from '@supabase/supabase-js'; export class OAuthService { private logger = getLogger('OAuthService'); - private credentialStore: CredentialStore; + private contextStore: ContextStore; private supabaseClient: SupabaseAuthClient; private baseUrl: string; private authorizationUrl: string | null = null; @@ -30,11 +31,12 @@ export class OAuthService { private resolveAuthorizationReady: (() => void) | null = null; constructor( - credentialStore: CredentialStore, + contextStore: ContextStore, + supabaseClient: SupabaseAuthClient, config: Partial = {} ) { - this.credentialStore = credentialStore; - this.supabaseClient = new SupabaseAuthClient(); + this.contextStore = contextStore; + this.supabaseClient = supabaseClient; const authConfig = getAuthConfig(config); this.baseUrl = authConfig.baseUrl; } @@ -274,13 +276,19 @@ export class OAuthService { // Handle authorization code for PKCE flow const code = url.searchParams.get('code'); + this.logger.info(`Code: ${code}, type: ${type}`); if (code && type === 'pkce_callback') { try { this.logger.info('Received authorization code for PKCE flow'); - // Exchange code for session using PKCE const session = await this.supabaseClient.exchangeCodeForSession(code); + // Save user info to context store + this.contextStore.saveContext({ + userId: session.user.id, + email: session.user.email + }); + // Calculate expiration - can be overridden with TM_TOKEN_EXPIRY_MINUTES let expiresAt: string | undefined; const tokenExpiryMinutes = process.env.TM_TOKEN_EXPIRY_MINUTES; @@ -294,7 +302,7 @@ export class OAuthService { : undefined; } - // Save authentication data + // Return credentials for backward compatibility const authData: AuthCredentials = { token: session.access_token, refreshToken: session.refresh_token, @@ -305,8 +313,6 @@ export class OAuthService { savedAt: new Date().toISOString() }; - this.credentialStore.saveCredentials(authData); - if (server.listening) { server.close(); } @@ -331,26 +337,32 @@ export class OAuthService { (type === 'oauth_success' || type === 'session_transfer') ) { try { - this.logger.info(`Received tokens via ${type}`); + this.logger.info( + `\n\n==============================================\n Received tokens via ${type}\n==============================================\n` + ); // Create a session with the tokens and set it in Supabase client - const session = { + // This automatically saves the session to session.json via SupabaseSessionStorage + const session: Session = { access_token: accessToken, refresh_token: refreshToken || '', - expires_at: expiresIn - ? Math.floor(Date.now() / 1000) + parseInt(expiresIn) - : undefined, - expires_in: expiresIn ? parseInt(expiresIn) : undefined, + expires_in: expiresIn ? parseInt(expiresIn) : 0, token_type: 'bearer', user: null as any // Will be populated by setSession }; // Set the session in Supabase client - await this.supabaseClient.setSession(session as any); + await this.supabaseClient.setSession(session); // Get user info from the session const user = await this.supabaseClient.getUser(); + // Save user info to context store + this.contextStore.saveContext({ + userId: user?.id || 'unknown', + email: user?.email + }); + // Calculate expiration time - can be overridden with TM_TOKEN_EXPIRY_MINUTES let expiresAt: string | undefined; const tokenExpiryMinutes = process.env.TM_TOKEN_EXPIRY_MINUTES; @@ -364,7 +376,7 @@ export class OAuthService { : undefined; } - // Save authentication data + // Return credentials for backward compatibility const authData: AuthCredentials = { token: accessToken, refreshToken: refreshToken || undefined, @@ -375,8 +387,6 @@ export class OAuthService { savedAt: new Date().toISOString() }; - this.credentialStore.saveCredentials(authData); - if (server.listening) { server.close(); } diff --git a/packages/tm-core/src/modules/auth/services/supabase-session-storage.ts b/packages/tm-core/src/modules/auth/services/supabase-session-storage.ts index f425d232..5949a8a3 100644 --- a/packages/tm-core/src/modules/auth/services/supabase-session-storage.ts +++ b/packages/tm-core/src/modules/auth/services/supabase-session-storage.ts @@ -1,206 +1,123 @@ /** - * Custom storage adapter for Supabase Auth sessions in CLI environment - * Implements the SupportedStorage interface required by Supabase Auth + * Barebones storage adapter for Supabase Auth sessions * - * This adapter bridges Supabase's session management with our existing - * auth.json credential storage, maintaining backward compatibility + * This is a simple key-value store that lets Supabase manage sessions + * without interference. No parsing, no merging, no guessing - just storage. + * + * Supabase handles: + * - Session refresh and token rotation + * - Expiry checking + * - Token validation + * + * We handle: + * - Simple get/set/remove/clear operations + * - Persistence to ~/.taskmaster/session.json */ import type { SupportedStorage } from '@supabase/supabase-js'; -import { CredentialStore } from './credential-store.js'; -import type { AuthCredentials } from '../types.js'; +import fs from 'fs'; +import path from 'path'; import { getLogger } from '../../../common/logger/index.js'; -const STORAGE_KEY = 'sb-taskmaster-auth-token'; +const DEFAULT_SESSION_FILE = path.join( + process.env.HOME || process.env.USERPROFILE || '~', + '.taskmaster', + 'session.json' +); export class SupabaseSessionStorage implements SupportedStorage { - private store: CredentialStore; + private storage: Map = new Map(); + private persistPath: string; private logger = getLogger('SupabaseSessionStorage'); - constructor(store: CredentialStore) { - this.store = store; + constructor(persistPath: string = DEFAULT_SESSION_FILE) { + this.persistPath = persistPath; + this.load(); } /** - * Build a Supabase session object from our credentials + * Load session data from disk on initialization */ - private buildSessionFromCredentials(credentials: AuthCredentials): any { - // Create a session object that Supabase expects - const session = { - access_token: credentials.token, - refresh_token: credentials.refreshToken || '', - // Don't default to arbitrary values - let Supabase handle refresh - ...(credentials.expiresAt && { - expires_at: Math.floor(new Date(credentials.expiresAt).getTime() / 1000) - }), - token_type: 'bearer', - user: { - id: credentials.userId, - email: credentials.email || '' - } - }; - return session; - } - - /** - * Parse a Supabase session back to our credentials - */ - private parseSessionToCredentials( - sessionData: any - ): Partial { + private load(): void { try { - // Handle both string and object formats (Supabase may pass either) - const session = - typeof sessionData === 'string' ? JSON.parse(sessionData) : sessionData; - - return { - token: session.access_token, - refreshToken: session.refresh_token, - userId: session.user?.id, - email: session.user?.email, - expiresAt: session.expires_at - ? new Date(session.expires_at * 1000).toISOString() - : undefined - }; + if (fs.existsSync(this.persistPath)) { + const data = JSON.parse(fs.readFileSync(this.persistPath, 'utf8')); + Object.entries(data).forEach(([k, v]) => + this.storage.set(k, v as string) + ); + this.logger.debug('Loaded session from disk', { + keys: Array.from(this.storage.keys()) + }); + } } catch (error) { - this.logger.error('Error parsing session:', error); - return {}; + this.logger.error('Failed to load session:', error); + // Don't throw - allow starting with fresh session } } /** - * Get item from storage - Supabase will request the session with a specific key + * Persist session data to disk immediately + */ + private persist(): void { + try { + // Ensure directory exists + const dir = path.dirname(this.persistPath); + if (!fs.existsSync(dir)) { + fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); + } + + // Write atomically with temp file + const data = Object.fromEntries(this.storage); + const tempFile = `${this.persistPath}.tmp`; + fs.writeFileSync(tempFile, JSON.stringify(data, null, 2), { + mode: 0o600 + }); + fs.renameSync(tempFile, this.persistPath); + + this.logger.debug('Persisted session to disk'); + } catch (error) { + this.logger.error('Failed to persist session:', error); + // Don't throw - session is still in memory + } + } + + /** + * Get item from storage + * Supabase will call this to retrieve session data */ getItem(key: string): string | null { - // Supabase uses a specific key pattern for sessions - if (key === STORAGE_KEY || key.includes('auth-token')) { - try { - // Get credentials and let Supabase handle expiry/refresh internally - const credentials = this.store.getCredentials(); - - // Only return a session if we have BOTH access token AND refresh token - // Supabase will handle refresh if session is expired - if (!credentials?.token || !credentials?.refreshToken) { - this.logger.debug('No valid credentials found'); - return null; - } - - const session = this.buildSessionFromCredentials(credentials); - return JSON.stringify(session); - } catch (error) { - this.logger.error('Error getting session:', error); - } - } - // Return null if no valid session exists - Supabase expects this - return null; + const value = this.storage.get(key) ?? null; + this.logger.debug('getItem called', { key, hasValue: !!value }); + return value; } /** - * Set item in storage - Supabase will store the session with a specific key - * CRITICAL: This is called during refresh token rotation - must be atomic + * Set item in storage + * Supabase will call this to store/update session data + * CRITICAL: This is called during token refresh - must persist immediately */ setItem(key: string, value: string): void { - // Only handle Supabase session keys - if (key === STORAGE_KEY || key.includes('auth-token')) { - try { - this.logger.info('Supabase called setItem - storing refreshed session'); - - // Parse the session and update our credentials - const sessionUpdates = this.parseSessionToCredentials(value); - const existingCredentials = this.store.getCredentials({ - allowExpired: true - }); - - // CRITICAL: Only save if we have both tokens - prevents partial session states - // Refresh token rotation means we MUST persist the new refresh token immediately - if (!sessionUpdates.token || !sessionUpdates.refreshToken) { - this.logger.warn( - 'Received incomplete session update - skipping save to prevent token rotation issues', - { - hasToken: !!sessionUpdates.token, - hasRefreshToken: !!sessionUpdates.refreshToken - } - ); - return; - } - - // Log the refresh token rotation for debugging - const isRotation = - existingCredentials?.refreshToken !== sessionUpdates.refreshToken; - if (isRotation) { - this.logger.debug( - 'Refresh token rotated - storing new refresh token atomically' - ); - } - - // Build updated credentials - ATOMIC update of both tokens - const userId = sessionUpdates.userId ?? existingCredentials?.userId; - - // Runtime assertion: userId is required for AuthCredentials - if (!userId) { - this.logger.error( - 'Cannot save credentials: userId is missing from both session update and existing credentials' - ); - throw new Error('Invalid session state: userId is required'); - } - - const updatedCredentials: AuthCredentials = { - ...(existingCredentials ?? {}), - token: sessionUpdates.token, - refreshToken: sessionUpdates.refreshToken, - expiresAt: sessionUpdates.expiresAt, - userId, - email: sessionUpdates.email ?? existingCredentials?.email, - savedAt: new Date().toISOString(), - selectedContext: existingCredentials?.selectedContext - } as AuthCredentials; - - // Save synchronously to ensure atomicity during refresh - this.store.saveCredentials(updatedCredentials); - - this.logger.info( - 'Successfully saved refreshed credentials from Supabase', - { - tokenRotated: isRotation, - expiresAt: updatedCredentials.expiresAt - } - ); - } catch (error) { - this.logger.error('Error setting session:', error); - } - } + this.logger.debug('setItem called', { key }); + this.storage.set(key, value); + this.persist(); // Immediately persist on every write } /** - * Remove item from storage - Called when signing out + * Remove item from storage + * Supabase will call this during sign out */ removeItem(key: string): void { - if (key === STORAGE_KEY || key.includes('auth-token')) { - // Don't actually remove credentials, just clear the tokens - // This preserves other data like selectedContext - try { - const credentials = this.store.getCredentials({ allowExpired: true }); - if (credentials) { - // Keep context but clear auth tokens - const clearedCredentials: AuthCredentials = { - ...credentials, - token: '', - refreshToken: undefined, - expiresAt: undefined - } as AuthCredentials; - this.store.saveCredentials(clearedCredentials); - } - } catch (error) { - this.logger.error('Error removing session:', error); - } - } + this.logger.debug('removeItem called', { key }); + this.storage.delete(key); + this.persist(); } /** * Clear all session data */ clear(): void { - // Clear auth tokens but preserve context - this.removeItem(STORAGE_KEY); + this.logger.debug('clear called'); + this.storage.clear(); + this.persist(); } } diff --git a/packages/tm-core/src/modules/auth/types.ts b/packages/tm-core/src/modules/auth/types.ts index 4a6b8c76..d0693a42 100644 --- a/packages/tm-core/src/modules/auth/types.ts +++ b/packages/tm-core/src/modules/auth/types.ts @@ -81,7 +81,9 @@ export type AuthErrorCode = | 'PKCE_INIT_FAILED' | 'PKCE_FAILED' | 'CODE_EXCHANGE_FAILED' - | 'SESSION_SET_FAILED'; + | 'SESSION_SET_FAILED' + | 'CODE_AUTH_FAILED' + | 'INVALID_CODE'; /** * Authentication error class diff --git a/packages/tm-core/src/modules/integration/clients/supabase-client.ts b/packages/tm-core/src/modules/integration/clients/supabase-client.ts index 3fede755..ad0cedfb 100644 --- a/packages/tm-core/src/modules/integration/clients/supabase-client.ts +++ b/packages/tm-core/src/modules/integration/clients/supabase-client.ts @@ -11,17 +11,14 @@ import { import { AuthenticationError } from '../../auth/types.js'; import { getLogger } from '../../../common/logger/index.js'; import { SupabaseSessionStorage } from '../../auth/services/supabase-session-storage.js'; -import { CredentialStore } from '../../auth/services/credential-store.js'; export class SupabaseAuthClient { private client: SupabaseJSClient | null = null; private sessionStorage: SupabaseSessionStorage; private logger = getLogger('SupabaseAuthClient'); - private credentialStore: CredentialStore; constructor() { - this.credentialStore = CredentialStore.getInstance(); - this.sessionStorage = new SupabaseSessionStorage(this.credentialStore); + this.sessionStorage = new SupabaseSessionStorage(); } /** @@ -314,4 +311,50 @@ export class SupabaseAuthClient { ); } } + + /** + * Verify a one-time token and create a session + * Used for CLI authentication with pre-generated tokens + */ + async verifyOneTimeCode(token: string): Promise { + const client = this.getClient(); + + try { + this.logger.info('Verifying authentication token...'); + + // Use Supabase's verifyOtp for token verification + // Using token_hash with magiclink type doesn't require email + const { data, error } = await client.auth.verifyOtp({ + token_hash: token, + type: 'magiclink' + }); + + if (error) { + this.logger.error('Failed to verify token:', error); + throw new AuthenticationError( + `Failed to verify token: ${error.message}`, + 'INVALID_CODE' + ); + } + + if (!data?.session) { + throw new AuthenticationError( + 'No session returned from token verification', + 'INVALID_RESPONSE' + ); + } + + this.logger.info('Successfully verified authentication token'); + return data.session; + } catch (error) { + if (error instanceof AuthenticationError) { + throw error; + } + + throw new AuthenticationError( + `Token verification failed: ${(error as Error).message}`, + 'CODE_AUTH_FAILED' + ); + } + } } diff --git a/packages/tm-core/src/modules/integration/services/export.service.ts b/packages/tm-core/src/modules/integration/services/export.service.ts index 6907c73b..2a355f2d 100644 --- a/packages/tm-core/src/modules/integration/services/export.service.ts +++ b/packages/tm-core/src/modules/integration/services/export.service.ts @@ -99,8 +99,9 @@ export class ExportService { * Export tasks to a brief */ async exportTasks(options: ExportTasksOptions): Promise { + const isAuthenticated = await this.authManager.hasValidSession(); // Validate authentication - if (!this.authManager.isAuthenticated()) { + if (!isAuthenticated) { throw new TaskMasterError( 'Authentication required for export', ERROR_CODES.AUTHENTICATION_ERROR @@ -383,8 +384,8 @@ export class ExportService { }; // Get auth token - const credentials = await this.authManager.getCredentials(); - if (!credentials || !credentials.token) { + const accessToken = await this.authManager.getAccessToken(); + if (!accessToken) { throw new Error('Not authenticated'); } @@ -393,7 +394,7 @@ export class ExportService { method: 'POST', headers: { 'Content-Type': 'application/json', - Authorization: `Bearer ${credentials.token}` + Authorization: `Bearer ${accessToken}` }, body: JSON.stringify(requestBody) }); diff --git a/packages/tm-core/src/modules/storage/services/storage-factory.ts b/packages/tm-core/src/modules/storage/services/storage-factory.ts index 39772e91..8811c9ee 100644 --- a/packages/tm-core/src/modules/storage/services/storage-factory.ts +++ b/packages/tm-core/src/modules/storage/services/storage-factory.ts @@ -29,10 +29,10 @@ export class StorageFactory { * @param projectPath - Project root path (for file storage) * @returns Storage implementation */ - static createFromStorageConfig( + static async createFromStorageConfig( storageConfig: RuntimeStorageConfig, projectPath: string - ): IStorage { + ): Promise { // Wrap the storage config in the expected format, including projectPath // This ensures ApiStorage receives the projectPath for projectId return StorageFactory.create( @@ -47,10 +47,10 @@ export class StorageFactory { * @param projectPath - Project root path (for file storage) * @returns Storage implementation */ - static create( + static async create( config: Partial, projectPath: string - ): IStorage { + ): Promise { const storageType = config.storage?.type || 'auto'; const logger = getLogger('StorageFactory'); @@ -68,21 +68,22 @@ export class StorageFactory { // Check if authenticated via AuthManager const authManager = AuthManager.getInstance(); - if (!authManager.isAuthenticated()) { + const hasSession = await authManager.hasValidSession(); + if (!hasSession) { throw new TaskMasterError( `API storage not fully configured (${missing.join(', ') || 'credentials missing'}). Run: tm auth login, or set the missing field(s).`, ERROR_CODES.MISSING_CONFIGURATION, { storageType: 'api', missing } ); } - // Use auth token from AuthManager (synchronous - no auto-refresh here) - const credentials = authManager.getCredentials(); - if (credentials) { + // Use auth token from AuthManager + const accessToken = await authManager.getAccessToken(); + if (accessToken) { // Merge with existing storage config, ensuring required fields const nextStorage: StorageSettings = { ...(config.storage as StorageSettings), type: 'api', - apiAccessToken: credentials.token, + apiAccessToken: accessToken, apiEndpoint: config.storage?.apiEndpoint || process.env.TM_BASE_DOMAIN || @@ -104,15 +105,26 @@ export class StorageFactory { return StorageFactory.createApiStorage(config); } - // Then check if authenticated via AuthManager - if (authManager.isAuthenticated()) { - const credentials = authManager.getCredentials(); - if (credentials) { - // Configure API storage with auth credentials + // Then check if authenticated via Supabase + const hasSession = await authManager.hasValidSession(); + if (hasSession) { + const accessToken = await authManager.getAccessToken(); + const context = authManager.getContext(); + + // Validate we have the necessary context for API storage + if (!context?.briefId) { + logger.debug( + 'šŸ“ User authenticated but no brief selected, using file storage' + ); + return StorageFactory.createFileStorage(projectPath, config); + } + + if (accessToken) { + // Configure API storage with Supabase session token const nextStorage: StorageSettings = { ...(config.storage as StorageSettings), type: 'api', - apiAccessToken: credentials.token, + apiAccessToken: accessToken, apiEndpoint: config.storage?.apiEndpoint || process.env.TM_BASE_DOMAIN || diff --git a/packages/tm-core/src/modules/tasks/services/task-service.ts b/packages/tm-core/src/modules/tasks/services/task-service.ts index 94143f52..78751d66 100644 --- a/packages/tm-core/src/modules/tasks/services/task-service.ts +++ b/packages/tm-core/src/modules/tasks/services/task-service.ts @@ -71,7 +71,7 @@ export class TaskService { const storageConfig = this.configManager.getStorageConfig(); const projectRoot = this.configManager.getProjectRoot(); - this.storage = StorageFactory.createFromStorageConfig( + this.storage = await StorageFactory.createFromStorageConfig( storageConfig, projectRoot );