Skip to content

fix(web): MFA status detection false positive + auth architecture docs#205

Merged
FSM1 merged 6 commits into
mainfrom
fix/mfa-status-false-positive
Feb 26, 2026
Merged

fix(web): MFA status detection false positive + auth architecture docs#205
FSM1 merged 6 commits into
mainfrom
fix/mfa-status-false-positive

Conversation

@FSM1

@FSM1 FSM1 commented Feb 26, 2026

Copy link
Copy Markdown
Owner

Summary

  • Fix MFA status detection: Changed details.totalFactors >= 2 to details.totalFactors > 2 in useMfa.ts. Every Web3Auth MPC Core Kit account starts with 2 default factors (JWT verifier share + hashedShare cloud custodial key), so the old check was always true — falsely showing MFA as [ENABLED] for every user. After enableMFA() deletes the hashedShare and creates device + recovery factors, totalFactors becomes 3+.
  • Add authentication architecture doc: New docs/AUTHENTICATION_ARCHITECTURE.md covering the MPC Core Kit key hierarchy, custom verifier flow, factor lifecycle (pre/post MFA), security analysis of semi-custodial vs non-custodial trust models, cross-device approval protocol, and MFA status detection logic.
  • Capture todo: Alternative MFA factor types research (passkey via WebAuthn PRF, password-derived key, secondary OAuth) noted for future phases.

Test plan

  • Verify fresh account (no MFA enrolled) shows [DISABLED] status badge on Security tab
  • Verify account after enableMFA() shows [ENABLED] status badge
  • TypeScript compiles: pnpm --filter web exec tsc --noEmit
  • Review docs/AUTHENTICATION_ARCHITECTURE.md for accuracy

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Fixed false positive MFA status detection that incorrectly marked newly created accounts as having MFA enabled.
  • Documentation

    • Added comprehensive authentication architecture documentation covering end-to-end authentication flows, key derivation, MFA state transitions, and cross-device approval processes.

FSM1 and others added 6 commits February 26, 2026 19:56
- Change totalFactors threshold from >= 2 to > 2
- Every account starts with 2 default factors (JWT verifier + hashedShare)
- MFA now correctly shows DISABLED for fresh accounts
- Update JSDoc comment explaining the threshold reasoning

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Entire-Checkpoint: 8f7f303fdcd2
Tasks completed: 1/1
- Fix MFA status threshold check and update comment

SUMMARY: .planning/quick/022-fix-mfa-status-detection-false-positive/022-SUMMARY.md

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Entire-Checkpoint: 9a543d13476b
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Entire-Checkpoint: e2779b1c1b44
Entire-Checkpoint: 2812ad9160af
Area: auth
Entire-Checkpoint: e672eaa58cd6
Covers MPC Core Kit key hierarchy, custom verifier flow, factor
lifecycle (pre/post MFA), security analysis of semi-custodial vs
non-custodial trust models, and cross-device approval protocol.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Entire-Checkpoint: 0381e2ce7b39
@coderabbitai

coderabbitai Bot commented Feb 26, 2026

Copy link
Copy Markdown

Walkthrough

This PR fixes an MFA status detection false positive by adjusting the enabled threshold from ≥2 to >2 factors in useMfa.ts, eliminating incorrect MFA enabled status on fresh accounts. It includes planning documents for the fix and a new proposal for alternative MFA factor types, plus comprehensive authentication architecture documentation.

Changes

Cohort / File(s) Summary
Planning and State Updates
.planning/STATE.md, .planning/quick/022-fix-mfa-status-detection-false-positive/022-PLAN.md, .planning/quick/022-fix-mfa-status-detection-false-positive/022-SUMMARY.md, .planning/todos/pending/2026-02-26-alternative-mfa-factor-types.md
Updated progress tracking, session continuity, and task logs reflecting completion of task 022. New planning document for alternative MFA factor types proposal (Passkey/WebAuthn PRF, password-derived keys, secondary OAuth).
MFA Status Detection Logic
apps/web/src/hooks/useMfa.ts
Changed MFA enabled threshold from totalFactors >= 2 to totalFactors > 2 with updated JSDoc comment explaining that default accounts have two initial factors (JWT verifier share and hashedShare) and MFA requires 3+ factors.
Authentication Documentation
docs/AUTHENTICATION_ARCHITECTURE.md
New comprehensive documentation describing CipherBox Web3Auth MPC Core Kit key architecture, end-to-end authentication flows, factor partitioning across pre/post-MFA states, security analyses, and cross-device approval flows.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • feat(12.4): MFA + Cross-Device Approval #128: Directly relates to useMfa.ts modifications; the referenced PR implements/overhauls the useMfa hook and related MFA flow, while this PR adjusts the MFA-enabled threshold logic within the same file.
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the two main changes: fixing an MFA status detection false positive and adding authentication architecture documentation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/mfa-status-false-positive

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@FSM1 FSM1 changed the title Fix MFA status detection false positive + auth architecture docs fix(web): MFA status detection false positive + auth architecture docs Feb 26, 2026
@FSM1 FSM1 merged commit a395b82 into main Feb 26, 2026
14 of 16 checks passed
FSM1 added a commit that referenced this pull request Feb 27, 2026
Verify and close two active debug sessions after confirming all
issues are fixed on main with commit/PR cross-references.

corekit-auth-uat: 4 issues resolved (ISSUE-001 tab crash, ISSUE-003
JWKS persistence, ISSUE-004 SecurityTab wiring, ISSUE-002 documented).
Added post-session follow-up fixes (#205, #210, #213) and E2E coverage
mapping for 19 previously-skipped UAT test cases.

mfa-banner-footer-overlap: grid-area fix (#143) and z-index follow-up
verified on main with file/line references.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Entire-Checkpoint: b1ae6e4f2098
FSM1 added a commit that referenced this pull request Feb 27, 2026
Verify and close two active debug sessions after confirming all
issues are fixed on main with commit/PR cross-references.

corekit-auth-uat: 4 issues resolved (ISSUE-001 tab crash, ISSUE-003
JWKS persistence, ISSUE-004 SecurityTab wiring, ISSUE-002 documented).
Added post-session follow-up fixes (#205, #210, #213) and E2E coverage
mapping for 19 previously-skipped UAT test cases.

mfa-banner-footer-overlap: grid-area fix (#143) and z-index follow-up
verified on main with file/line references.


Entire-Checkpoint: b1ae6e4f2098

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
@FSM1 FSM1 deleted the fix/mfa-status-false-positive branch March 4, 2026 01:16
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.

1 participant