chore: apply requested changes (#1442)

This commit is contained in:
Ralph Khreish
2025-11-24 23:00:13 +01:00
committed by GitHub
parent 0195feffee
commit 01d4d9930f
7 changed files with 53 additions and 17 deletions

View File

@@ -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) - **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 - 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 turbo:typecheck` before pushing to ensure TypeScript type checks pass
- Run `npm run test -w <package-name>` to test a package

View File

@@ -551,13 +551,15 @@ Examples:
private async handleMFAVerification( private async handleMFAVerification(
mfaError: AuthenticationError mfaError: AuthenticationError
): Promise<AuthCredentials> { ): Promise<AuthCredentials> {
if (!mfaError.mfaChallenge) { if (!mfaError.mfaChallenge?.factorId) {
throw new AuthenticationError( throw new AuthenticationError(
'MFA challenge information missing', 'MFA challenge information missing',
'MFA_VERIFICATION_FAILED' 'MFA_VERIFICATION_FAILED'
); );
} }
const { factorId } = mfaError.mfaChallenge;
console.log( console.log(
chalk.yellow( chalk.yellow(
'\n⚠ Multi-factor authentication is enabled on your account' '\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 // Use @tm/core's retry logic - presentation layer just handles UI
const result = await this.authManager.verifyMFAWithRetry( const result = await this.authManager.verifyMFAWithRetry(
mfaError.mfaChallenge.factorId, factorId,
async () => { async () => {
// Prompt for MFA code // Prompt for MFA code
try { try {

View File

@@ -339,7 +339,9 @@ describe('AuthManager - MFA Retry Logic', () => {
authManager.verifyMFAWithRetry('factor-123', codeProvider, { authManager.verifyMFAWithRetry('factor-123', codeProvider, {
maxAttempts: 0 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 // Test with negative
await expect( await expect(
@@ -352,7 +354,9 @@ describe('AuthManager - MFA Retry Logic', () => {
authManager.verifyMFAWithRetry('factor-123', codeProvider, { authManager.verifyMFAWithRetry('factor-123', codeProvider, {
maxAttempts: -1 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 // Verify code provider was never called
expect(codeProvider).not.toHaveBeenCalled(); expect(codeProvider).not.toHaveBeenCalled();

View File

@@ -163,9 +163,13 @@ export class AuthManager {
const onInvalidCode = options?.onInvalidCode; const onInvalidCode = options?.onInvalidCode;
// Guard against invalid maxAttempts values // Guard against invalid maxAttempts values
if (maxAttempts < 1) { if (
!Number.isFinite(maxAttempts) ||
!Number.isInteger(maxAttempts) ||
maxAttempts < 1
) {
throw new TypeError( throw new TypeError(
`Invalid maxAttempts value: ${maxAttempts}. Must be at least 1.` `Invalid maxAttempts value: ${maxAttempts}. Must be a positive integer.`
); );
} }

View File

@@ -397,17 +397,16 @@ describe('SessionManager', () => {
mockContextStore as any mockContextStore as any
); );
try { const promise = sessionManager.authenticateWithCode('test-code');
await sessionManager.authenticateWithCode('test-code');
expect.fail('Should have thrown MFA_REQUIRED error'); await expect(promise).rejects.toThrow(AuthenticationError);
} catch (error) { await expect(promise).rejects.toMatchObject({
expect(error).toBeInstanceOf(AuthenticationError); code: 'MFA_REQUIRED',
expect((error as AuthenticationError).code).toBe('MFA_REQUIRED'); mfaChallenge: {
expect((error as AuthenticationError).mfaChallenge).toEqual({
factorId: 'factor-123', factorId: 'factor-123',
factorType: 'totp' factorType: 'totp'
}); }
} });
}); });
}); });

View File

@@ -42,7 +42,10 @@ export class SessionManager {
await this.migrateLegacyAuth(); await this.migrateLegacyAuth();
} catch (error) { } catch (error) {
// Log but don't throw - initialization errors are handled gracefully // 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(); await this.waitForInitialization();
try { try {
const session = await this.supabaseClient.getSession(); const session = await this.supabaseClient.getSession();
return session !== null; return session != null;
} catch { } catch {
return false; return false;
} }

View File

@@ -37,7 +37,15 @@ describe('SupabaseAuthClient', () => {
let authClient: InstanceType<typeof SupabaseAuthClient>; let authClient: InstanceType<typeof SupabaseAuthClient>;
let mockSupabaseClient: any; let mockSupabaseClient: any;
// Store original env values for cleanup
let originalSupabaseUrl: string | undefined;
let originalSupabaseAnonKey: string | undefined;
beforeEach(() => { beforeEach(() => {
// Store original values
originalSupabaseUrl = process.env.TM_SUPABASE_URL;
originalSupabaseAnonKey = process.env.TM_SUPABASE_ANON_KEY;
// Set required environment variables // Set required environment variables
process.env.TM_SUPABASE_URL = 'https://test.supabase.co'; process.env.TM_SUPABASE_URL = 'https://test.supabase.co';
process.env.TM_SUPABASE_ANON_KEY = 'test-anon-key'; process.env.TM_SUPABASE_ANON_KEY = 'test-anon-key';
@@ -67,6 +75,21 @@ describe('SupabaseAuthClient', () => {
vi.clearAllMocks(); 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', () => { describe('verifyMFA', () => {
it('should verify MFA and refresh session to upgrade to AAL2', async () => { it('should verify MFA and refresh session to upgrade to AAL2', async () => {
// Mock the challenge response // Mock the challenge response