-
Notifications
You must be signed in to change notification settings - Fork 5
Fix : recovery codes download (M2-10266) #2189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
sricharan-varanasi
wants to merge
38
commits into
develop
Choose a base branch
from
fix-recovery-codes-download
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 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
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
This was referenced Dec 30, 2025
- 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
c6be012 to
f311876
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixorfeaturebranches intodeveloporreleasebranches viaSquash and Merge(to keep clean history)📝 Description
🔗 Jira Ticket M2-10266
This PR fixes two critical issues with recovery codes download functionality:
Changes include:
🪤 Peer Testing
Test 1: Download within 5 minutes
Enable MFA or view existing recovery codes (requires TOTP/recovery code verification)
Click "Download Codes" button
Expected outcome: File downloads successfully, modal stays open
Test 2: Download after 5 minutes (token expiry)
View recovery codes and wait 6+ minutes without closing the modal
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
Open recovery codes modal
Click "Download Codes" button
Expected outcome: Download happens, but modal remains open (doesn't auto-close)