-
Notifications
You must be signed in to change notification settings - Fork 5
refactor: Mfa disable refactor (M2-10278) #2190
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
48
commits into
develop
Choose a base branch
from
MFA-disable-refactor
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
- 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
- Update MFADisableVerifyResponse to return codeValidated and confirmationToken - Add MFADisableConfirmRequest type for final confirmation step - Add MFADisableConfirmResponse type for disable completion
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
5e3deb1 to
b3a4d39
Compare
4 tasks
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-10278
This PR refactors the MFA disable flow from a 2-step process to a more secure 3-step process, and updates all associated tests to match the new implementation.
Previous 2-step flow:
mfaToken)mfaToken, MFA disabled immediately)New 3-step flow:
mfaToken)confirmationToken)confirmationToken)Changes include:
useRemoveMFAhook to implement 3-step disable flowconfirmationTokenstate management/initiate,/verify,/confirmmockMFADisableVerifyCode()andmockMFADisableConfirm()vi.resetAllMocks()to prevent test interference🪤 Peer Testing
Requires valid MFA-enabled test account
Test Case 1: Successful MFA Disable with TOTP Code
Log in to an account with MFA enabled
Navigate to Settings → Account Settings → Security
Click "Disable Two-Factor Authentication"
Enter your current TOTP code from authenticator app
Confirm the disable action
Expected outcome:
Test Case 2: Invalid Code - Should Not Proceed to Confirmation
Follow steps 1-3 from Test Case 1
Enter an invalid TOTP code (e.g.,
000000)Click submit/verify button
Expected outcome:
Test Case 3: Cancel on Confirmation Screen
Follow steps 1-4 from Test Case 1 (enter correct TOTP code)
Verify confirmation screen is displayed
Click "Cancel" button on confirmation screen
Expected outcome:
/confirmendpointTest Case 4: Multiple Invalid Attempts Error Handling
Follow steps 1-3 from Test Case 1
Enter invalid TOTP code (e.g.,
000000)Repeat 5+ times
Expected outcome:
Test Case 5: Disable with Recovery Code
Follow steps 1-3 from Test Case 1
Click "Use recovery code instead"
Enter a valid recovery code (format:
XXXXX-XXXXX)Confirm the disable action
Expected outcome:
Test Case 6: Session Expiration
Start the disable flow
Wait for session to expire (or simulate timeout)
Attempt to proceed
Expected outcome:
Test Case 7: Network Error Handling
Start the disable flow
Disconnect network or use browser dev tools to simulate offline
Attempt to submit verification code
Expected outcome:
✏️ Notes
Breaking Changes:
verifyAndDisablemethod replaced withverifyCode+confirmDisableTest Coverage:
Related: