Skip to content

Conversation

@maxto024
Copy link
Contributor

@maxto024 maxto024 commented Dec 1, 2025

🔒 Multi-Factor Authentication (MFA) Implementation

Overview

This PR implements comprehensive Multi-Factor Authentication (MFA) for MindLogger using Time-based One-Time Password (TOTP) authentication. The implementation includes enrollment, login, recovery codes, and disable flows with enterprise-grade security features.

Branch: mfa-review-documentation
Related PRs: #1973, #1976, #1977, #1978

✨ Features Implemented

1. TOTP-Based MFA (#1973)

  • QR code generation for authenticator apps (Google Authenticator, Authy, etc.)
  • Encrypted TOTP secret storage
  • Time-window validation (30-90 second tolerance)

2. Recovery Codes (#1976)

  • 10 single-use recovery codes generated during enrollment
  • Secure hash + encryption storage pattern
  • Downloadable text file with codes

3. MFA Login Flow (#1977)

  • Seamless integration with existing login
  • JWT-based MFA session management
  • Redis session storage with 5-minute TTL

4. Security Features (#1978)

  • Rate Limiting: 5 attempts per session, 10 global attempts with 15-min lockout
  • Replay Protection: TOTP codes can't be reused
  • Session Security: Automatic expiration and validation

🧪 Testing Instructions

Prerequisites

  • Docker and Docker Compose installed
  • Postman installed (Download here)
  • Backend running: docker compose up -d

Step 1: Environment Setup

  1. Ensure your .env file includes these MFA variables:
# MFA Configuration
MFA__TOTP_ENCRYPTION_KEY=_laj8VzSPpUSUTMxb1gISn37xjVz4zFpTUHd3wm3AFw=
MFA__RECOVERY_CODE_ENCRYPTION_KEY=xK9vL2mN4pQ6rS8tU0wV1yA3bC5dE7fG9hI1jK3lM5n=
AUTHENTICATION__MFA_TOKEN__SECRET_KEY="a4b6c8d0e2f4g6h8i0j2k4l6m8n0o2p4q6r8s0t2u4v6w8x0y2z4a6b8c0d2e4f6"

Step 2: Using the Postman Collection

Import the Collection

  1. Open Postman
  2. Click "Import" button (top left)
  3. Select file: MFA_Testing_Collection.postman_collection.json
  4. Click "Import"

Collection Structure

The collection contains 7 test folders that should be run in order:

📁 MindLogger MFA Testing
├── 📁 1. User Registration
├── 📁 2. MFA Enrollment
├── 📁 3. MFA Login with TOTP
├── 📁 4. Recovery Code Generation
├── 📁 5. Recovery Code Authentication
├── 📁 6. Rate Limiting Tests
└── 📁 7. MFA Disable Flow

Running the Tests

Method 1: Run Entire Collection (Recommended)
  1. Click on the collection name "MindLogger MFA Testing"
  2. Click "Run" button (blue play icon)
  3. In the Runner:
    • Ensure all folders are selected
    • Set iterations: 1
    • Delay: 1000ms (important for TOTP time windows)
  4. Click "Run MindLogger MFA Testing"
Method 2: Run Individual Folders

Run each folder in sequence by clicking the "▶" next to each folder name.

What Each Test Folder Does

1️⃣ User Registration

  • Creates a test user with unique email
  • Sets variables: test_email, test_password
  • ✅ Success: Status 201, user created

2️⃣ MFA Enrollment

  • Login → Initiate MFA → Verify TOTP
  • Saves: totp_secret, access_token
  • ✅ Success: QR code received, enrollment completed

3️⃣ MFA Login with TOTP

  • Tests full MFA login flow
  • Automatically generates valid TOTP code
  • ✅ Success: Login requires MFA, TOTP verified

4️⃣ Recovery Code Generation

  • Views and downloads recovery codes
  • Saves first code for testing
  • ✅ Success: 10 recovery codes received

5️⃣ Recovery Code Authentication

  • Tests login with recovery code
  • Verifies one-time use
  • ✅ Success: Login successful, code consumed

6️⃣ Rate Limiting Tests

  • Tests session limits (5 attempts)
  • Tests global limits (10 attempts)
  • ✅ Success: 429 errors after limits

7️⃣ MFA Disable Flow

  • Initiates disable → Verifies with TOTP
  • Tests direct login after disable
  • ✅ Success: MFA removed, direct login works

Step 3: Verifying Results

In Postman Runner:

  • All tests should show ✅ green checkmarks
  • Total run time: ~2-3 minutes
  • 30+ assertions should pass

Key Success Indicators:

  1. Enrollment: "MFA enrollment successful" in test results
  2. TOTP Login: "MFA challenge received" → "TOTP verified"
  3. Recovery: "Recovery code authentication successful"
  4. Rate Limit: "Rate limiting triggered correctly"
  5. Disable: "MFA successfully disabled"

✅ Checklist for Reviewers

  • Run Postman collection successfully
  • Review security implementation
  • Check error handling
  • Verify rate limiting works
  • Test recovery code flow
  • Confirm disable flow removes MFA

🔧 Minor Fixes Needed

  1. Remove duplicate code in users.py (lines 233-236)
  2. Replace hardcoded rate limits with settings values
  3. Add security headers to recovery code downloads

🚀 Ready for Production

The implementation is production-ready with the minor fixes above. All security features have been thoroughly tested and verified.


Questions? Check the detailed review documents or run the automated tests for hands-on verification.

- Add POST /users/me/mfa/totp/initiate endpoint for MFA enrollment
- Implement CRUD methods: update_pending_mfa, clear_pending_mfa, update_mfa_status
- Add TOTPInitiateResponse domain model
- Generate TOTP secret, encrypt, and store in pending_mfa_secret field
- Return provisioning URI for QR code generation in authenticator apps
- Requires JWT authentication via get_current_user dependency
sricharan-varanasi and others added 19 commits November 21, 2025 12:52
- Add RecoveryCodeSchema with user_id FK, code_hash, code_encrypted, used, used_at fields
- Add RecoveryCode internal and PublicRecoveryCode domain models
- Add recovery_codes relationship and recovery_codes_generated_at to User model
- Create Alembic migration for recovery_codes table with CASCADE delete
- Add RecoveryCodeCRUD with create, get, mark_as_used, and delete operations
- Migration verified: table, indexes, and constraints created successfully
- Add recovery code service with generation, hashing, and encryption utilities
- Implement generate_recovery_codes() orchestration function
- Add encryption configuration (MFA__RECOVERY_CODE_ENCRYPTION_KEY)
- Create comprehensive integration tests (6 test cases)
- Fix migration: Add migrated_date and migrated_updated columns

Features:
- Generate cryptographically secure recovery codes (XXXXX-XXXXX format)
- Bcrypt hashing for verification security
- Fernet encryption for display capability
- Batch database insertion with transaction support
- User timestamp tracking (recovery_codes_generated_at)

Security:
- 51.7 bits entropy (36^10 combinations)
- Secrets module for cryptographic randomness
- Defense-in-depth: hash for verification, encrypt for display
- Bcrypt cost factor 12, Fernet AES-128

Tests:
- End-to-end generation flow
- Hash verification correctness
- Encryption/decryption roundtrip
- Code format validation (XXXXX-XXXXX)
- Uniqueness guarantee
- Default count parameter
- Step 3.1: Add recovery_codes field to TOTPVerifyResponse schema
  * Optional list[str] | None field with default=None
  * Backward compatible with existing clients

- Step 3.2: Add conditional recovery code generation logic
  * Import generate_recovery_codes service
  * Check if first-time setup (recovery_codes_generated_at is None)
  * Generate 10 codes on first enrollment only
  * Return codes=None on re-enrollment

- Step 3.4: Update endpoint documentation
  * Add docstring explaining recovery code behavior
  * Add OpenAPI summary and description
  * Document one-time display warning

- Step 3.5: Add comprehensive integration tests
  * test_first_time_setup_generates_recovery_codes
  * test_recovery_codes_stored_in_database
  * test_user_timestamp_set_after_generation
  * test_reenrollment_no_codes_returned
  * test_invalid_code_no_recovery_codes
  * test_recovery_codes_format
  * All 6 tests passing

Files modified:
- src/apps/users/domain.py (schema)
- src/apps/users/api/users.py (logic + docs)
- src/apps/users/router.py (OpenAPI docs)
- src/apps/users/tests/test_mfa_recovery_codes.py (new test file)
- Add GET /me/mfa/recovery-codes endpoint to view recovery codes with usage status
- Add GET /me/mfa/recovery-codes/download endpoint to download codes as text file
- Add RecoveryCodesListResponse and RecoveryCodeItemResponse domain objects
- Add errors for MFA not enabled and codes not found
- Add integration tests for both endpoints
- Move recovery code generation logic to service layer
- Update TOTP verify to use service for code generation
@maxto024 maxto024 added documentation Improvements or additions to documentation Do not merge Pull request cannot be merged for some reason labels Dec 1, 2025
@maxto024 maxto024 changed the base branch from MFA-disable to develop December 10, 2025 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Do not merge Pull request cannot be merged for some reason documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants