ros/claude-task-master
1
0
Fork 0
mirror of https://github.com/eyaltoledano/claude-task-master.git synced 2026-01-30 06:12:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(auth): unify browser auth with MFA across all login paths

Browse Source 
- Create shared authenticateWithBrowserMFA utility in auth-ui.ts
- Update auth.command.ts to use shared utility for tm auth login
- Update auth-guard.ts to use shared utility for parse-prd/export
- Fix oauth-service.ts to NOT call onError for MFA_REQUIRED
  (MFA requirement is a continuation, not a failure)

All login paths now use the same MFA-aware browser auth flow:
- tm auth login
- tm parse-prd (Bring it to Hamster)
- tm export
This commit is contained in:
Eyal Toledano
2025-12-01 21:21:10 -05:00
parent edeeef4d92
commit 3038fa42a0
4 changed files with 124 additions and 157 deletions
Download Patch File Download Diff File Expand all files Collapse all files

74
apps/cli/src/commands/auth.command.ts
View File

@@ -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<AuthCredentials> {
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);
}
/**

90
apps/cli/src/utils/auth-guard.ts
View File

@@ -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<AuthCredentials> {
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.

112
apps/cli/src/utils/auth-ui.ts
View File

@@ -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<AuthCredentials>;
verifyMFAWithRetry(
factorId: string,
codeProvider: () => Promise<string>,
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<AuthCredentials> {
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();
}
}