fix(auth): enforce MFA verification in OAuth browser login flow (#1470)

This commit is contained in:
Eyal Toledano
2025-12-01 17:02:31 -05:00
committed by GitHub
parent 6e4369209e
commit 1dbf242ffd
3 changed files with 79 additions and 11 deletions

View File

@@ -155,7 +155,7 @@ Examples:
async executeLogin(
token?: string,
yes?: boolean,
showHeader: boolean = true
showHeader = true
): Promise<void> {
try {
const result = token
@@ -393,7 +393,7 @@ Examples:
*/
async performInteractiveAuth(
yes?: boolean,
showHeader: boolean = true
showHeader = true
): Promise<AuthResult> {
if (showHeader) {
ui.displayBanner('Task Master Authentication');
@@ -498,6 +498,7 @@ 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
*/
private async authenticateWithBrowser(): Promise<AuthCredentials> {
const countdownTimer = new AuthCountdownTimer(AUTH_TIMEOUT_MS);
@@ -535,6 +536,25 @@ Examples:
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 {
@@ -605,7 +625,7 @@ Examples:
private async performTokenAuth(
token: string,
yes?: boolean,
showHeader: boolean = true
showHeader = true
): Promise<AuthResult> {
if (showHeader) {
ui.displayBanner('Task Master Authentication');

View File

@@ -6,18 +6,19 @@ import crypto from 'crypto';
import http from 'http';
import os from 'os';
import { URL } from 'url';
import { Session } from '@supabase/supabase-js';
import type { Session } from '@supabase/supabase-js';
import { TASKMASTER_VERSION } from '../../../common/constants/index.js';
import { getLogger } from '../../../common/logger/index.js';
import { SupabaseAuthClient } from '../../integration/clients/supabase-client.js';
import type { SupabaseAuthClient } from '../../integration/clients/supabase-client.js';
import { getAuthConfig } from '../config.js';
import { ContextStore } from '../services/context-store.js';
import type { ContextStore } from '../services/context-store.js';
import {
AuthConfig,
AuthCredentials,
type AuthConfig,
type AuthCredentials,
AuthenticationError,
CliData,
OAuthFlowOptions
type CliData,
type MFAChallenge,
type OAuthFlowOptions
} from '../types.js';
export class OAuthService {
@@ -125,7 +126,7 @@ export class OAuthService {
/**
* Start the OAuth flow (internal implementation)
*/
private async startFlow(timeout: number = 300000): Promise<AuthCredentials> {
private async startFlow(timeout = 300000): Promise<AuthCredentials> {
const state = this.generateState();
// Store the original state for verification
@@ -289,6 +290,10 @@ export class OAuthService {
email: session.user.email
});
// Check if MFA is required for this user
// This will throw MFA_REQUIRED error if MFA verification is needed
await this.checkAndThrowIfMFARequired();
// Calculate expiration - can be overridden with TM_TOKEN_EXPIRY_MINUTES
let expiresAt: string | undefined;
const tokenExpiryMinutes = process.env.TM_TOKEN_EXPIRY_MINUTES;
@@ -363,6 +368,10 @@ export class OAuthService {
email: user?.email
});
// Check if MFA is required for this user
// This will throw MFA_REQUIRED error if MFA verification is needed
await this.checkAndThrowIfMFARequired();
// Calculate expiration time - can be overridden with TM_TOKEN_EXPIRY_MINUTES
let expiresAt: string | undefined;
const tokenExpiryMinutes = process.env.TM_TOKEN_EXPIRY_MINUTES;
@@ -429,4 +438,42 @@ export class OAuthService {
getAuthorizationUrl(): string | null {
return this.authorizationUrl;
}
/**
* Check if MFA is required and throw appropriate error if so
* This ensures OAuth flow enforces MFA when user has it enabled
*/
private async checkAndThrowIfMFARequired(): Promise<void> {
const mfaCheck = await this.supabaseClient.checkMFARequired();
if (mfaCheck.required) {
// MFA is required - check if we have complete factor information
if (!mfaCheck.factorId || !mfaCheck.factorType) {
this.logger.error('MFA required but factor information is incomplete', {
mfaCheck
});
throw new AuthenticationError(
'MFA is required but the server returned incomplete factor configuration. Please contact support or try re-enrolling MFA.',
'MFA_REQUIRED_INCOMPLETE'
);
}
this.logger.info('MFA verification required after OAuth login', {
factorId: mfaCheck.factorId,
factorType: mfaCheck.factorType
});
const mfaChallenge: MFAChallenge = {
factorId: mfaCheck.factorId,
factorType: mfaCheck.factorType
};
throw new AuthenticationError(
'MFA verification required. Please provide your authentication code.',
'MFA_REQUIRED',
undefined,
mfaChallenge
);
}
}
}

View File

@@ -116,6 +116,7 @@ export type AuthErrorCode =
| 'CODE_AUTH_FAILED'
| 'INVALID_CODE'
| 'MFA_REQUIRED'
| 'MFA_REQUIRED_INCOMPLETE'
| 'MFA_VERIFICATION_FAILED'
| 'INVALID_MFA_CODE';