Skip to content

Conversation

@sricharan-varanasi
Copy link
Contributor

  • Tests for the changes have been added
  • Related documentation has been added / updated
  • OSS packages added to Curious open source credit page
  • Delivered the fix or feature branches into develop or release branches via Squash and Merge (to keep clean history)

📝 Description

🔗 Jira Ticket M2-10266

This PR fixes two critical issues with recovery codes download functionality:

Changes include:

  • Fixed modal closing bug - Prevented event bubbling when clicking the download button
  • Enforced download token requirement - Removed frontend fallback that bypassed token expiry
  • Respect 5-minute token expiry - All downloads now require valid backend token validation

🪤 Peer Testing

Test 1: Download within 5 minutes

  1. Enable MFA or view existing recovery codes (requires TOTP/recovery code verification)

  2. Click "Download Codes" button

    Expected outcome: File downloads successfully, modal stays open

Test 2: Download after 5 minutes (token expiry)

  1. View recovery codes and wait 6+ minutes without closing the modal

  2. Click "Download Codes" button

    Expected outcome: Error message appears: "Unable to download recovery codes. The download session may have expired (5 minute limit)..."

Test 3: Verify modal doesn't close

  1. Open recovery codes modal

  2. Click "Download Codes" button

    Expected outcome: Download happens, but modal remains open (doesn't auto-close)

- Add comprehensive MFA translation keys to app-en.json
- Add French translations for all MFA features to app-fr.json
- Includes translations for:
  - MFA setup (QR code and manual setup flows)
  - Recovery codes management
  - Authenticator app configuration
  - Identity confirmation dialogs
  - MFA removal flow
  - All button labels and loading states
- Replace hardcoded strings with translation keys:
  - Account Settings title (mfa.accountSettings)
  - Profile section (mfa.profile)
  - Email label (mfa.email)
  - Two-factor authentication title and description
  - Authenticator app title and description
  - Enabled badge (mfa.enabled)
  - Add/Remove buttons (mfa.buttons.add/remove)
  - Recovery options header (mfa.recoveryOptions)
  - Recovery codes title and description
  - View button (mfa.buttons.view)
  - Success message for MFA removal (mfa.remove.successMessage)
- Import useTranslation hook from react-i18next
- Replace hardcoded strings:
  - Modal title (mfa.recoveryCodes.saveTitle)
  - Description paragraphs (saveDescription1, saveDescription2)
  - Button labels (mfa.buttons.savedCodes, downloadCodes)
- Replace hardcoded strings:
  - Modal title (mfa.setup.scanTitle)
  - Description (mfa.setup.scanDescription)
  - Continue button (mfa.buttons.continue)
  - Can't scan link (mfa.setup.cantScanQR)
- Replace hardcoded strings:
  - Modal title (mfa.setup.manualTitle)
  - Description (mfa.setup.manualDescription)
  - Instructions (mfa.setup.manualInstructions)
…lation keys

VerificationForm:
- Replace button labels with translation keys (mfa.buttons.continue, back)

SecretKeyDisplay:
- Replace copy/copied tooltips with translation keys (mfa.secretKey.copy, copied)
ViewRecoveryCodes:
- Replace title and description with translation keys

RemoveMFA:
- Replace title and description with translation keys

RemoveMFAConfirmation:
- Replace all text (title, message, buttons)

ConfirmIdentityVerificationCode:
- Replace placeholder, button labels, and link text

ConfirmIdentityRecoveryCode:
- Replace all text (title, description, placeholder, buttons)
- Added Noto Sans as variables.font.family.input
- Replaced all hardcoded Moderat with appropriate font.family properties
- Replaced all hardcoded Noto Sans with font.family.input
- Fixed incorrect variables.font.family object references
- Follows repo pattern

Files modified:
- AccountSettings.styles.ts
- AccountTab.styles.ts
- MFAManualSetup.styles.ts
- MFARecoveryCodes.styles.ts
- MFASetup.styles.ts
- RemoveMFAConfirmation.styles.ts
- MFADialog.styles.ts
- font.ts
- Follows repo pattern of using centralized SVG sprite
- Ensures consistency across the application

Files modified:
- MFARecoveryCodes.tsx
- MFASetup.tsx
- MFAManualSetup.tsx
- ConfirmIdentityRecoveryCode.tsx
- SecretKeyDisplay.tsx

Files deleted:
- CloseIcon.tsx
- CheckIcon.tsx
- Replace #ba1a1a with variables.palette.error40
- Replace rgba(186, 26, 26, 0.08) with error40 + hex alpha (14 = 8% opacity)
- Replace rgba(186, 26, 26, 0.3) with error40 + hex alpha (4D = 30% opacity)
- Replace rgba(255, 255, 255, 0.9) with white_alpha50
- Replace rgba(0, 0, 0, 0.5) with black + hex alpha (80 = 50% opacity)
- Replace hardcoded Toast colors with palette variables
- Fix incorrect variables.font.family object reference in MFADialog

Files modified:
- MFADialog.styles.ts
- RemoveMFAConfirmation.styles.ts
- AccountTab.styles.ts
- MFASetup.styles.ts
- Toast.tsx
- Create Toast.styles.ts following repo pattern
- Replace inline style object with StyledToast component
- Convert fontSize from px to rem (14px → 1.4rem)
- Use variables.font.weight.regular instead of hardcoded 400
- Improve box-shadow formatting for better readability
- Fix bug where MFA was disabled immediately after code verification
- Now MFA is only disabled after user confirms on final confirmation modal
- Store verification code temporarily and only call API on final confirm
- Ensures cancel button on confirmation modal actually prevents MFA removal
- Create centralized mock data file for MFA tests
- Include mock provisioning URIs, verification codes, and recovery codes
- Add mock API responses for success and error scenarios
- Include mock tokens for download and verification flows
- Provide mock error responses matching backend formats
- Create reusable helper functions for MFA test setup
- Add API mock helpers (success and error scenarios)
- Include utilities for mocking MFA setup, verification, and removal flows
- Add helpers for recovery codes viewing and downloading
- Provide common callback mock creators
- Add API call verification utilities

These helpers reduce duplication and improve test readability.
Add comprehensive test suite for MFA input handler hook:
- Test digit filtering (remove letters, special chars, spaces)
- Test length limiting (reject >6 digits, accept <=6)
- Test error clearing behavior
- Test combined operations (filtering + error clearing)
- Test progressive typing and blocking after 6 digits

Coverage: 12 tests
Focus: Input sanitization, validation, and user interaction flows
- 22 comprehensive tests covering all error scenarios
- Validates proper error message translation for users
- Add comprehensive tests for MFA disable hook
- Cover initiate disable flow with success and error cases
- Test verify and disable with all error scenarios
- Test state management and loading states
- Test edge cases (empty code, rapid calls, missing token)
- Add tests for useViewRecoveryCodes hook
- Test TOTP and recovery code verification flows
- Cover error handling (400, 403, 404, 429, network errors)
- Add helper functions: mockMFAViewCodesInitiate and mockMFAViewCodesVerify
- All tests passing (16/16)
- Fix modal closing on download button click
- Enforce downloadToken requirement
- Remove frontend-only download option
@aws-amplify-us-east-1
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-2189.d2ccder08v9rmu.amplifyapp.com

- Append download link to drawer element instead of document.body
- Prevents ClickAwayListener from detecting click as outside event
- Add session management state (mfaToken, sessionInitialized)
- Initiate MFA session once when modal opens via useEffect
- Reuse existing mfaToken for both TOTP and recovery code verification
- Wrap initiateSession in useCallback for stable reference
- Add useRef to prevent duplicate API calls in StrictMode
- Add resetSession function to clear all session state
- Call resetSession when modal closes
- Ensures new MFA session token on every 'View Recovery Codes' click
- Add onRetry prop to ConfirmIdentityVerificationCode and ConfirmIdentityRecoveryCode
- Disable input field when onRetry is provided
- Replace Continue/Back buttons with Try Again button
@sricharan-varanasi sricharan-varanasi force-pushed the fix-recovery-codes-download branch from c6be012 to f311876 Compare January 2, 2026 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants