feat: move MFA enrolment onto BetterAuth's twoFactor plugin (D1) - #87
Open
LucaGerlich wants to merge 2 commits into
Open
feat: move MFA enrolment onto BetterAuth's twoFactor plugin (D1)#87LucaGerlich wants to merge 2 commits into
LucaGerlich wants to merge 2 commits into
Conversation
The login gate already used the plugin, but enrolment wrote custom user.mfaEnabled/mfaSecret/mfaBackupCodes columns the gate never read, so enabling MFA had no effect. The settings UI now enrols through authClient.twoFactor (enable, verifyTotp, disable, generateBackupCodes), renders the TOTP URI as a QR code client-side and shows backup codes once. - twoFactor plugin: allowPasswordless for LDAP/SSO accounts, encrypted backup codes; the twoFactor table gains the plugin's verified, failedVerificationCount and lockedUntil columns; the mfa* user columns are dropped (existing enrolments must re-enrol) - Audit: enrolment, removal, backup-code regeneration and TOTP/backup-code logins are recorded from the BetterAuth after hook; the credentials login audit no longer fires on the password step of a 2FA login - Compliance dashboard: MFA coverage is computed from twoFactorEnabled, and all counts are scoped to the admin's organization (they were global) - Removed: /api/auth/mfa routes, lib/mfa.ts, otplib, encryptArray helpers
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
🟡 Changes recommended
The new twoFactor.verified column defaults to true, which conflicts with its intended “enrolment confirmation” purpose and could undermine the verification step unless corrected.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR resolves the long-standing mismatch where MFA enrolment was implemented outside BetterAuth while login enforcement relied on BetterAuth’s twoFactor plugin. It rewires enrolment end-to-end through the plugin, removes the legacy MFA implementation (code + DB columns + dependency), adds audit logging for 2FA events via an auth hook, and fixes cross-tenant compliance dashboard counts by scoping them to the admin’s organization.
Changes:
- Replaced the custom MFA enrolment/verify/disable flow with
authClient.twoFactor.*in the user settings UI; removed legacy MFA routes and helpers (andotplib). - Added
auth-two-factor-audit+ anafterhook inauth-server.tsto audit 2FA enable/disable/regen and 2FA-completed logins. - Scoped compliance dashboard counts (including MFA coverage) to the caller’s organization.
File summaries
| File | Description |
|---|---|
| TECHNICAL_DEBT.md | Marks D1 as resolved and documents the BetterAuth-based MFA approach. |
| src/lib/mfa.ts | Removes legacy MFA secret/URI/token/backup-code implementation. |
| src/lib/encryption.ts | Removes array encryption helpers previously used for MFA backup codes. |
| src/lib/auth-two-factor-audit.ts | Adds 2FA endpoint classification + audit-log writing helpers. |
| src/lib/auth-server.ts | Integrates BetterAuth twoFactor options and adds 2FA after-hook auditing + session enrichment. |
| src/lib/auth-client.ts | Removes legacy mfaEnabled user field from client typing. |
| src/lib/tests/mfa.test.ts | Removes tests for deleted legacy MFA helpers. |
| src/lib/tests/encryption.test.ts | Removes tests for deleted encrypt/decrypt array helpers. |
| src/lib/tests/auth-two-factor-audit.test.ts | Adds unit tests for new 2FA auditing helpers. |
| src/app/user/[id]/settings/ui/MfaSettings.tsx | Rewrites MFA settings UI to use BetterAuth twoFactor enable/verify/disable/generate codes. |
| src/app/user/[id]/settings/page.tsx | Switches settings page to use twoFactorEnabled + authProvider. |
| src/app/api/user/route.ts | Updates user sanitization to drop removed MFA columns. |
| src/app/api/auth/mfa/verify/route.ts | Deletes legacy MFA verify endpoint. |
| src/app/api/auth/mfa/setup/route.ts | Deletes legacy MFA setup endpoint. |
| src/app/api/auth/mfa/disable/route.ts | Deletes legacy MFA disable endpoint. |
| src/app/api/admin/compliance/route.ts | Scopes compliance counts to the admin’s organization and adds MFA enabled user count. |
| src/app/admin/compliance/ui/ComplianceDashboard.tsx | Uses real MFA coverage to compute compliance status. |
| prisma/schema.prisma | Drops legacy MFA user fields; adds BetterAuth-managed 2FA lockout/verification fields. |
| prisma/migrations/20260908_betterauth_two_factor/migration.sql | Drops legacy MFA user columns and adds BetterAuth twoFactor columns. |
| package.json | Bumps version to 0.10.0 and removes otplib. |
| docs/DEVELOPMENT_NOTES.md | Updates encryption notes to reflect BetterAuth-managed 2FA secret/codes encryption. |
| docs/DEPLOYMENT.md | Updates env-var descriptions and deployment notes for BetterAuth secret impact on MFA. |
| docs/DATABASE_MIGRATION_GUIDE.md | Updates encryption/migration notes to include BetterAuth-managed 2FA encryption. |
| CHANGELOG.md | Adds 0.10.0 release notes for BetterAuth MFA + compliance scoping. |
| bun.lock | Removes otplib and related dependencies from lockfile. |
Review details
Suppressed comments (1)
src/app/user/[id]/settings/ui/MfaSettings.tsx:161
- After regenerating backup codes, the password remains in component state even though it’s no longer needed for the backup-code display step. Clearing it reduces exposure of sensitive data in memory.
setBackupCodes(result.data.backupCodes);
setStep("backup");
- Files reviewed: 24/25 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+10
to
+13
| ALTER TABLE "public"."twoFactor" | ||
| ADD COLUMN IF NOT EXISTS "verified" BOOLEAN NOT NULL DEFAULT true, | ||
| ADD COLUMN IF NOT EXISTS "failedVerificationCount" INTEGER NOT NULL DEFAULT 0, | ||
| ADD COLUMN IF NOT EXISTS "lockedUntil" TIMESTAMPTZ(6); |
Comment on lines
+1255
to
+1258
| // Plugin-managed: enrolment confirmation and verification lockout | ||
| verified Boolean @default(true) | ||
| failedVerificationCount Int @default(0) | ||
| lockedUntil DateTime? @db.Timestamptz(6) |
Comment on lines
+117
to
+119
| setTotpUri(result.data.totpURI); | ||
| setBackupCodes(result.data.backupCodes); | ||
| setStep("qr"); |
Preview builds executed the migration against whatever DATABASE_URL the Preview environment holds. On the personal deployment that is the production database, so the preview build of this branch applied the column-dropping MFA migration to production before release. The build command now migrates only when VERCEL_ENV is production; previews compile against the existing schema.
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
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.
Summary
Resolves TECHNICAL_DEBT.md decision D1 (MFA bypassable). The login gate already used BetterAuth's
twoFactorplugin; enrolment wrote custom columns the gate never read. Enrolment now runs on the plugin end to end and the custom implementation is deleted (−461 lines,otplibremoved).authClient.twoFactor(enable → QR → verify → backup codes; disable; regenerate backup codes). LDAP/SSO accounts enrol without a password (allowPasswordless), local accounts confirm with theirs.20260908_betterauth_two_factor: dropsuser.mfaEnabled/mfaSecret/mfaBackupCodes, adds the plugin'sverified,failedVerificationCount,lockedUntilcolumns totwoFactor.afterhook (lib/auth-two-factor-audit.ts); the credentials login audit waits for the second factor.Breaking: users who had MFA "enabled" must enrol again. The old flag was never enforced, so no protection is lost, but notify them before deploying.
Verification
tsc --noEmit0 errors, ESLint clean,next build✓auth-two-factor-auditsuiteManual test plan
/mfa-verify→ TOTP accepted → audit log has LOGIN methodtotp🤖 Generated with Claude Code
https://claude.ai/code/session_01GcY31e5LXkP6Vtfqi5FvPt