diff --git a/CLAUDE.md b/CLAUDE.md index ec4c6dd3..58f3dec9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -177,3 +177,4 @@ Apply standard software engineering principles: - **Add a changeset for code changes** - Run `npx changeset` after making code changes (not needed for docs-only PRs) - When creating changesets, remember that it's user-facing, meaning we don't have to get into the specifics of the code, but rather mention what the end-user is getting or fixing from this changeset - Run `npm run turbo:typecheck` before pushing to ensure TypeScript type checks pass +- Run `npm run test -w ` to test a package diff --git a/apps/cli/src/commands/auth.command.ts b/apps/cli/src/commands/auth.command.ts index 83a68aea..30c14542 100644 --- a/apps/cli/src/commands/auth.command.ts +++ b/apps/cli/src/commands/auth.command.ts @@ -551,13 +551,15 @@ Examples: private async handleMFAVerification( mfaError: AuthenticationError ): Promise { - if (!mfaError.mfaChallenge) { + if (!mfaError.mfaChallenge?.factorId) { throw new AuthenticationError( 'MFA challenge information missing', 'MFA_VERIFICATION_FAILED' ); } + const { factorId } = mfaError.mfaChallenge; + console.log( chalk.yellow( '\n⚠️ Multi-factor authentication is enabled on your account' @@ -571,7 +573,7 @@ Examples: // Use @tm/core's retry logic - presentation layer just handles UI const result = await this.authManager.verifyMFAWithRetry( - mfaError.mfaChallenge.factorId, + factorId, async () => { // Prompt for MFA code try { diff --git a/packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts b/packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts index ae22d17c..ee7e41af 100644 --- a/packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts +++ b/packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts @@ -339,7 +339,9 @@ describe('AuthManager - MFA Retry Logic', () => { authManager.verifyMFAWithRetry('factor-123', codeProvider, { maxAttempts: 0 }) - ).rejects.toThrow('Invalid maxAttempts value: 0. Must be at least 1.'); + ).rejects.toThrow( + 'Invalid maxAttempts value: 0. Must be a positive integer.' + ); // Test with negative await expect( @@ -352,7 +354,9 @@ describe('AuthManager - MFA Retry Logic', () => { authManager.verifyMFAWithRetry('factor-123', codeProvider, { maxAttempts: -1 }) - ).rejects.toThrow('Invalid maxAttempts value: -1. Must be at least 1.'); + ).rejects.toThrow( + 'Invalid maxAttempts value: -1. Must be a positive integer.' + ); // Verify code provider was never called expect(codeProvider).not.toHaveBeenCalled(); 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 f295cff9..38f12b02 100644 --- a/packages/tm-core/src/modules/auth/managers/auth-manager.ts +++ b/packages/tm-core/src/modules/auth/managers/auth-manager.ts @@ -163,9 +163,13 @@ export class AuthManager { const onInvalidCode = options?.onInvalidCode; // Guard against invalid maxAttempts values - if (maxAttempts < 1) { + if ( + !Number.isFinite(maxAttempts) || + !Number.isInteger(maxAttempts) || + maxAttempts < 1 + ) { throw new TypeError( - `Invalid maxAttempts value: ${maxAttempts}. Must be at least 1.` + `Invalid maxAttempts value: ${maxAttempts}. Must be a positive integer.` ); } diff --git a/packages/tm-core/src/modules/auth/services/session-manager.spec.ts b/packages/tm-core/src/modules/auth/services/session-manager.spec.ts index a38072fe..9e0b7d25 100644 --- a/packages/tm-core/src/modules/auth/services/session-manager.spec.ts +++ b/packages/tm-core/src/modules/auth/services/session-manager.spec.ts @@ -397,17 +397,16 @@ describe('SessionManager', () => { mockContextStore as any ); - try { - await sessionManager.authenticateWithCode('test-code'); - expect.fail('Should have thrown MFA_REQUIRED error'); - } catch (error) { - expect(error).toBeInstanceOf(AuthenticationError); - expect((error as AuthenticationError).code).toBe('MFA_REQUIRED'); - expect((error as AuthenticationError).mfaChallenge).toEqual({ + const promise = sessionManager.authenticateWithCode('test-code'); + + await expect(promise).rejects.toThrow(AuthenticationError); + await expect(promise).rejects.toMatchObject({ + code: 'MFA_REQUIRED', + mfaChallenge: { factorId: 'factor-123', factorType: 'totp' - }); - } + } + }); }); }); diff --git a/packages/tm-core/src/modules/auth/services/session-manager.ts b/packages/tm-core/src/modules/auth/services/session-manager.ts index f3d9bea6..085c4005 100644 --- a/packages/tm-core/src/modules/auth/services/session-manager.ts +++ b/packages/tm-core/src/modules/auth/services/session-manager.ts @@ -42,7 +42,10 @@ export class SessionManager { await this.migrateLegacyAuth(); } catch (error) { // Log but don't throw - initialization errors are handled gracefully - this.logger.debug('Session initialization completed with warnings'); + this.logger.debug( + 'Session initialization completed with warnings', + error + ); } } @@ -102,7 +105,7 @@ export class SessionManager { await this.waitForInitialization(); try { const session = await this.supabaseClient.getSession(); - return session !== null; + return session != null; } catch { return false; } diff --git a/packages/tm-core/src/modules/integration/clients/supabase-client.spec.ts b/packages/tm-core/src/modules/integration/clients/supabase-client.spec.ts index 1dcc8c28..28344f42 100644 --- a/packages/tm-core/src/modules/integration/clients/supabase-client.spec.ts +++ b/packages/tm-core/src/modules/integration/clients/supabase-client.spec.ts @@ -37,7 +37,15 @@ describe('SupabaseAuthClient', () => { let authClient: InstanceType; let mockSupabaseClient: any; + // Store original env values for cleanup + let originalSupabaseUrl: string | undefined; + let originalSupabaseAnonKey: string | undefined; + beforeEach(() => { + // Store original values + originalSupabaseUrl = process.env.TM_SUPABASE_URL; + originalSupabaseAnonKey = process.env.TM_SUPABASE_ANON_KEY; + // Set required environment variables process.env.TM_SUPABASE_URL = 'https://test.supabase.co'; process.env.TM_SUPABASE_ANON_KEY = 'test-anon-key'; @@ -67,6 +75,21 @@ describe('SupabaseAuthClient', () => { vi.clearAllMocks(); }); + afterEach(() => { + // Restore original env values + if (originalSupabaseUrl === undefined) { + delete process.env.TM_SUPABASE_URL; + } else { + process.env.TM_SUPABASE_URL = originalSupabaseUrl; + } + + if (originalSupabaseAnonKey === undefined) { + delete process.env.TM_SUPABASE_ANON_KEY; + } else { + process.env.TM_SUPABASE_ANON_KEY = originalSupabaseAnonKey; + } + }); + describe('verifyMFA', () => { it('should verify MFA and refresh session to upgrade to AAL2', async () => { // Mock the challenge response