Skip to content

Conversation

carlosmonastyrski
Copy link
Contributor

Description 📣

Improved 2FA flow to be more intuitive, separated the recovery tokens from the normal 2FA token (previously check both on the same flow). Also force users to download the recovery codes in order to minimize the possibility of locked users.

Type ✨

  • Bug fix
  • New feature
  • Improvement
  • Breaking change
  • Documentation

Tests 🛠️

# Here's some code block to paste some code snippets

@maidul98
Copy link
Collaborator

maidul98 commented Sep 20, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Summary

This PR significantly improves the 2FA authentication flow by separating recovery code verification from regular TOTP verification, creating a more intuitive and secure user experience. The changes span both frontend and backend components to implement a comprehensive redesign of the 2FA system.

Backend Changes:

  • Added a new dedicated endpoint /api/v2/auth/mfa/verify/recovery-code for recovery code verification, separating it from the general MFA verification flow
  • Enhanced the verifyMfaToken service to accept an isRecoveryCode parameter, allowing different handling logic for regular TOTP codes vs recovery codes
  • Modified the TOTP registration service to return recovery codes after successful verification, eliminating the need for separate API calls
  • Updated API schemas to properly expose recovery codes in responses

Frontend Changes:

  • Redesigned the main MFA component (Mfa.tsx) to provide separate input modes for TOTP codes (6 digits) and recovery codes (8 characters), with a toggle button to switch between them
  • Added a new RecoveryCodesDownload.tsx component that enforces mandatory download of recovery codes before users can complete 2FA setup, preventing account lockouts
  • Enhanced the TOTP registration flow to automatically display recovery codes after verification and require download before completion
  • Improved responsive design and user messaging throughout the 2FA components

The changes integrate seamlessly with the existing authentication system while providing clearer separation of concerns. The mandatory recovery code download addresses a critical UX issue where users could get locked out of their accounts, reducing support burden and improving security practices. The separation of verification flows enables better error messaging and user guidance during authentication.

Confidence score: 4/5

  • This PR is safe to merge with minimal risk, implementing well-structured improvements to the 2FA flow
  • Score reflects solid implementation with good separation of concerns, though the extensive changes across authentication components warrant careful testing
  • Pay close attention to the new recovery code verification endpoint and the mandatory download flow to ensure they work correctly in production

10 files reviewed, 3 comments

Edit Code Review Bot Settings | Greptile

Copy link
Contributor

@scott-ray-wilson scott-ray-wilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

much better! few comments

Copy link
Contributor

@scott-ray-wilson scott-ray-wilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two last small comments

@scott-ray-wilson scott-ray-wilson self-requested a review September 23, 2025 20:52
Copy link
Contributor

@scott-ray-wilson scott-ray-wilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@carlosmonastyrski carlosmonastyrski merged commit c040e7f into main Sep 23, 2025
10 of 11 checks passed
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.

3 participants