diff --git a/apps/cli/src/commands/auth.command.ts b/apps/cli/src/commands/auth.command.ts index 891a5cd2..440d35ba 100644 --- a/apps/cli/src/commands/auth.command.ts +++ b/apps/cli/src/commands/auth.command.ts @@ -4,7 +4,6 @@ */ import { - AUTH_TIMEOUT_MS, type AuthCredentials, AuthManager, AuthenticationError @@ -12,14 +11,8 @@ import { import chalk from 'chalk'; import { Command } from 'commander'; import inquirer from 'inquirer'; -import open from 'open'; import ora from 'ora'; -import { - AuthCountdownTimer, - displayAuthInstructions, - displayWaitingForAuth, - handleMFAFlow -} from '../utils/auth-ui.js'; +import { authenticateWithBrowserMFA, handleMFAFlow } from '../utils/auth-ui.js'; import { displayError } from '../utils/error-handler.js'; import * as ui from '../utils/ui.js'; import { ContextCommand } from './context.command.js'; @@ -497,70 +490,11 @@ Examples: /** * Authenticate with browser using OAuth 2.0 with PKCE - * Uses shared countdown timer from auth-ui.ts - * Includes MFA handling if user has MFA enabled + * Uses shared authenticateWithBrowserMFA for consistent login UX + * across all commands (auth login, parse-prd, export, etc.) */ private async authenticateWithBrowser(): Promise { - const countdownTimer = new AuthCountdownTimer(AUTH_TIMEOUT_MS); - - try { - // Use AuthManager's new unified OAuth flow method with callbacks - const credentials = await this.authManager.authenticateWithOAuth({ - // Callback to handle browser opening - openBrowser: async (authUrl) => { - await open(authUrl); - }, - timeout: AUTH_TIMEOUT_MS, - - // Callback when auth URL is ready - onAuthUrl: (authUrl) => { - displayAuthInstructions(authUrl); - }, - - // Callback when waiting for authentication - onWaitingForAuth: () => { - displayWaitingForAuth(); - countdownTimer.start(); - }, - - // Callback on success - onSuccess: () => { - countdownTimer.stop('success'); - }, - - // Callback on error - onError: () => { - countdownTimer.stop('failure'); - } - }); - - return credentials; - } catch (error) { - // Check if MFA is required BEFORE showing failure message - if ( - error instanceof AuthenticationError && - error.code === 'MFA_REQUIRED' - ) { - // Stop spinner without showing failure - MFA is required, not a failure - countdownTimer.stop('mfa'); - - if (!error.mfaChallenge?.factorId) { - throw new AuthenticationError( - 'MFA challenge information missing', - 'MFA_VERIFICATION_FAILED' - ); - } - - // Use shared MFA flow handler - return this.handleMFAVerification(error); - } - - countdownTimer.stop('failure'); - throw error; - } finally { - // Ensure cleanup - countdownTimer.cleanup(); - } + return authenticateWithBrowserMFA(this.authManager); } /** diff --git a/apps/cli/src/utils/auth-guard.ts b/apps/cli/src/utils/auth-guard.ts index fcf6d254..a4028993 100644 --- a/apps/cli/src/utils/auth-guard.ts +++ b/apps/cli/src/utils/auth-guard.ts @@ -3,24 +3,14 @@ * Provides reusable authentication checking and OAuth flow triggering * for commands that require authentication. * - * Includes MFA (Multi-Factor Authentication) support. + * Uses the shared authenticateWithBrowserMFA utility for consistent + * login UX across all commands (auth login, parse-prd, export, etc.) */ -import { - AUTH_TIMEOUT_MS, - type AuthCredentials, - AuthDomain, - AuthenticationError -} from '@tm/core'; +import { type AuthCredentials, AuthDomain } from '@tm/core'; import chalk from 'chalk'; import inquirer from 'inquirer'; -import open from 'open'; -import { - AuthCountdownTimer, - displayAuthInstructions, - displayWaitingForAuth, - handleMFAFlow -} from './auth-ui.js'; +import { authenticateWithBrowserMFA } from './auth-ui.js'; /** * Options for the auth guard @@ -112,9 +102,9 @@ export async function ensureAuthenticated( } } - // Trigger OAuth flow + // Trigger OAuth flow using shared browser auth with MFA support try { - const credentials = await authenticateWithBrowser(authDomain); + const credentials = await authenticateWithBrowserMFA(authDomain); return { authenticated: true, credentials @@ -127,74 +117,6 @@ export async function ensureAuthenticated( } } -/** - * Authenticate with browser using OAuth 2.0 with PKCE - * Includes MFA handling if the user has MFA enabled. - */ -async function authenticateWithBrowser( - authDomain: AuthDomain -): Promise { - const countdownTimer = new AuthCountdownTimer(AUTH_TIMEOUT_MS); - - try { - const credentials = await authDomain.authenticateWithOAuth({ - // Callback to handle browser opening - openBrowser: async (authUrl: string) => { - await open(authUrl); - }, - timeout: AUTH_TIMEOUT_MS, - - // Callback when auth URL is ready - onAuthUrl: (authUrl: string) => { - displayAuthInstructions(authUrl); - }, - - // Callback when waiting for authentication - onWaitingForAuth: () => { - displayWaitingForAuth(); - countdownTimer.start(); - }, - - // Callback on success - onSuccess: () => { - countdownTimer.stop('success'); - }, - - // Callback on error - onError: () => { - countdownTimer.stop('failure'); - } - }); - - return credentials; - } catch (error: unknown) { - // Check if MFA is required BEFORE showing failure message - if (error instanceof AuthenticationError && error.code === 'MFA_REQUIRED') { - // Stop spinner without showing failure - MFA is required, not a failure - countdownTimer.stop('mfa'); - - if (!error.mfaChallenge?.factorId) { - throw new AuthenticationError( - 'MFA challenge information missing', - 'MFA_VERIFICATION_FAILED' - ); - } - - // Use shared MFA flow handler - return handleMFAFlow( - authDomain.verifyMFAWithRetry.bind(authDomain), - error.mfaChallenge.factorId - ); - } - - countdownTimer.stop('failure'); - throw error; - } finally { - // Ensure cleanup - countdownTimer.cleanup(); - } -} - /** * Higher-order function that wraps a command action with auth checking. * Use this to easily protect any command that requires authentication. diff --git a/apps/cli/src/utils/auth-ui.ts b/apps/cli/src/utils/auth-ui.ts index a6f679af..1b0899cc 100644 --- a/apps/cli/src/utils/auth-ui.ts +++ b/apps/cli/src/utils/auth-ui.ts @@ -11,10 +11,12 @@ import { AUTH_TIMEOUT_MS, type AuthCredentials, AuthenticationError, - MFA_MAX_ATTEMPTS + MFA_MAX_ATTEMPTS, + type OAuthFlowOptions } from '@tm/core'; import chalk from 'chalk'; import inquirer from 'inquirer'; +import open from 'open'; import ora, { type Ora } from 'ora'; import * as ui from './ui.js'; @@ -267,3 +269,111 @@ export async function handleMFAFlow( 'MFA_VERIFICATION_FAILED' ); } + +/** + * Authentication provider interface for browser OAuth with MFA + * Can be satisfied by either AuthManager or AuthDomain + */ +export interface BrowserAuthProvider { + authenticateWithOAuth(options?: OAuthFlowOptions): Promise; + verifyMFAWithRetry( + factorId: string, + codeProvider: () => Promise, + options?: { + maxAttempts?: number; + onInvalidCode?: (attempt: number, remaining: number) => void; + } + ): Promise<{ + success: boolean; + credentials?: AuthCredentials; + attemptsUsed: number; + }>; +} + +/** + * Shared browser authentication with MFA support + * + * This is the SINGLE implementation of browser-based OAuth login with MFA handling. + * Used by both the auth login command and any protected commands that trigger login + * (like parse-prd when "Bring it to Hamster" is selected). + * + * @param authProvider - AuthManager or AuthDomain instance + * @returns AuthCredentials on success + * @throws AuthenticationError on failure + * + * @example + * ```typescript + * // From auth command + * const authManager = AuthManager.getInstance(); + * const credentials = await authenticateWithBrowserMFA(authManager); + * + * // From auth-guard (for protected commands) + * const authDomain = new AuthDomain(); + * const credentials = await authenticateWithBrowserMFA(authDomain); + * ``` + */ +export async function authenticateWithBrowserMFA( + authProvider: BrowserAuthProvider +): Promise { + const countdownTimer = new AuthCountdownTimer(AUTH_TIMEOUT_MS); + + try { + const credentials = await authProvider.authenticateWithOAuth({ + // Callback to handle browser opening + openBrowser: async (authUrl: string) => { + await open(authUrl); + }, + timeout: AUTH_TIMEOUT_MS, + + // Callback when auth URL is ready + onAuthUrl: (authUrl: string) => { + displayAuthInstructions(authUrl); + }, + + // Callback when waiting for authentication + onWaitingForAuth: () => { + displayWaitingForAuth(); + countdownTimer.start(); + }, + + // Callback on success + onSuccess: () => { + countdownTimer.stop('success'); + }, + + // Don't handle onError here - we need to check error type in catch block + // to differentiate between MFA_REQUIRED (not a failure) and actual failures + onError: () => { + // Timer will be stopped in catch block with appropriate status + } + }); + + return credentials; + } catch (error: unknown) { + // Check if MFA is required BEFORE showing failure message + if (error instanceof AuthenticationError && error.code === 'MFA_REQUIRED') { + // Stop spinner without showing failure - MFA is required, not a failure + countdownTimer.stop('mfa'); + + if (!error.mfaChallenge?.factorId) { + throw new AuthenticationError( + 'MFA challenge information missing', + 'MFA_VERIFICATION_FAILED' + ); + } + + // Use shared MFA flow handler + return handleMFAFlow( + authProvider.verifyMFAWithRetry.bind(authProvider), + error.mfaChallenge.factorId + ); + } + + // Only show failure for actual errors, not MFA requirement + countdownTimer.stop('failure'); + throw error; + } finally { + // Ensure cleanup + countdownTimer.cleanup(); + } +} 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 86140699..ae1873c2 100644 --- a/packages/tm-core/src/modules/auth/services/oauth-service.ts +++ b/packages/tm-core/src/modules/auth/services/oauth-service.ts @@ -114,8 +114,9 @@ export class OAuthService { error ); - // Notify error - if (onError) { + // Only notify error for actual failures, NOT for MFA_REQUIRED + // MFA requirement is a continuation of the auth flow, not an error + if (onError && authError.code !== 'MFA_REQUIRED') { onError(authError); }