Repository navigation
security(auth): disabling MFA does not re-verify the password #754
Description
Activity
audit — 2026-09-25
Verified:
backend/internal/application/auth/mfa_usecase.go:240still has// TODO: Verify password before disabling (requires user repo + password verification). The use case deletes the TOTP secret (DisableMFA), then the backup codes (DeleteBackupCodes), as two writes outside a transaction (CLAUDE.md rule 7). If the second write fails, the backup codes survive the deactivation.Criteria: numbered here so the issue meets the Ready definition.
POSTdisable-MFA requires the current password (PasswordHasher.Verify). A wrong or missing password returns 401 with a generic body, and each attempt counts against the per-account throttle (security(auth): one rate-limit bucket for all auth routes locks users out, login timing reveals registered addresses, no per-account throttle #688).- The TOTP secret and the backup codes are deleted in one transaction. A test forces the second write to fail and proves that nothing was deleted.
- The deactivation writes an entry to the chained audit trail and notifies the user (email via feat(notifications): implement a proper notification system (replace NoOpNotifier) #757, in-app until then).
- If the member's role requires MFA (
mfaRequiredRoles/mfaRequiredBusinessRoles), deactivation is refused with 403. - Tests:
TestDisableMFA_Success,TestDisableMFA_NotFound,TestDisableMFA_Unauthorized(wrong password), plus the transactional rollback test from criterion 2.
Next: Sprint 1 of the launch board, first backend item after #807.
Blocked on: nothing
Generated by Claude Code
3 remaining items
backend-go + frontend-react — 2026-09-30
Done — PR #848, 4 commits on
754-securityauth-disabling-mfa-does-not-re-verify-the-password:eae19ebfgorm_mfa_repository.go:DisableMFAdeletes the secret and the backup codes in one transaction. The secret is hard-deleted, because the UNIQUEuser_idwould otherwise block re-enrolment.48d0b517mfa_usecase.go/mfa_handler.go/main.go: the endpoint re-checks the password (401wrong_password), limits each account to 5 attempts per 15 min in Redis on top of the per-IPauthRateLimit(429), refuses roles that require MFA (403), returns 409 for SSO accounts with no local password, writes anmfa_disableaudit entry with reason codes, and sends an email (reset_mailer.go) plus an in-appmfa_disablednotification.b3d9016fshared/ds/useDismissableLayer.ts: the Modal no longer takes focus away from anautoFocusfield (it used to move it to the close button).6f0eb5bdMFADisableDialog.tsx,useMfa.ts,MFAPolicyPanel.tsx: the Settings dialog. The mutation usesretry: false, because the global retry of 3 turned one wrong password into 4 requests.
Verified — full
go test ./... -count=1: no failures. MFA/Disable/Mailer package tests: ok.vitestsettings+auth+shared+notifications: 32 files, 390 passed.mfaDisable.test.tsx: green 6 runs out of 6 (it was flaky before the DS fix).tscandeslinton touched files: OK. Both new tests fail when their fix is reverted. Live Chromium harness on the realMFAAccountPanelwith the API mocked: focus lands on the password field, a wrong password produces 1 request and a field error in 32 ms, success closes the dialog, and a privileged role sees no button.Criteria — 1 ✅ · 2 ✅ (rollback test on SQLite) · 3 ✅ (audit + email + in-app) · 4 ✅ · 5 ✅
Next — owner review of #848. Before merge: one run against real Postgres and Redis (the
Unscopeddelete inside a transaction, the Redis throttle). Docker wasn't reachable from this session.Blocked on — nothing (@owner decides the merge). Open design question, not blocking: SSO accounts can't disable MFA because they have no password to re-prove. A TOTP re-check would count as an auth design change, so it would need an escalation.
- added 4 commits that reference this issue
on Sep 30, 2026 backend-go + frontend-react — 2026-09-30 (follow-up)
Done — The two remainders from the previous comment, 4 more commits on #848:
e125f6c5gorm_mfa_repository_pg_test.go: rollback proven on a real Postgres. A trigger makes the backup-code delete fail, and the test checks the secret delete rolled back. Gated onDATABASE_URL.94d4daccSSO accounts (no local password) confirm with a current TOTP code:mfa_usecase.go(WithTOTPKey,ErrMFADisableCodeIncorrect),mfa_handler.go(401wrong_code),handler.go(/auth/me→has_password),main.go. The email and in-app copy no longer mention a password.eb5f8e3fMFADisableDialog.tsx: shows a password field or a 6-digit code field depending onhas_password, with a fallback when the server answerswrong_code.9a96d2f2docs/DECISIONS.md: D-061 (owner decision, 2026-09-30).
Verified — Local Postgres 18 (throwaway cluster on :55754) and Redis (:56754), branch server booted from the repo root:
- default policy: 403
mfa_required_by_role - missing or wrong password: 401 ×2
- correct password: 200,
mfa_secrets1→0,mfa_backup_codes8→0 - again: 404
- 6th attempt: 429 across two server processes (Redis key, TTL 900)
- 8 audit rows with their reasons; one chained
create disableinaudit_events; one in-app notification plus a logged email; the password appears in no log
SSO path in the real React UI: the dialog opens on the code field with focus in it; a wrong code gives 401 plus an audit
wrong_code; the correct code gives 200 and both tables at 0.DATABASE_URL=… go test ./...: no failures, and all 7 Postgres-gated tests PASS. The new Postgres test fails when the transaction is removed.vitest32 files / 394 tests OK,mfaDisable.test.tsx10/10 on 4 consecutive runs,tscandeslintOK. Throwaway services are stopped.Criteria — 1 ✅ · 2 ✅ (SQLite + real Postgres) · 3 ✅ · 4 ✅ · 5 ✅
Next — Owner review and merge of #848. Possible follow-up issue: TOTP replay protection (a code can be replayed within about 90 s, at login as well as on disable).
Blocked on — nothing
- added a commit that references this issue
on Sep 30, 2026 backend-go — 2026-09-30 (remainders)
Done — The run on PostgreSQL 16.15 is repeated and green, including the SSO path.
docs/openapi.yamlnow documents/auth/meand/auth/mfa/disable(4c568540). TOTP replay is split out into #849 (PR #851) and the SSO "Change password" card into #850 (PR #852), both stacked on #848.
Verified —DATABASE_URL=<pg16> go test ./...: no failures, 7 Postgres tests PASS. Live HTTP on PG 16: 403 / 401 / 401 / 200 / 404 / 429; SSO wrong_code ×2 then 200.
Criteria — 1 ✅ · 2 ✅ · 3 ✅ · 4 ✅ · 5 ✅
Next — Owner review: merge #848 first, then #851 and #852.
Blocked on — nothing.openapi.generated.tshas pre-existing drift and is not regenerated here.- linked a pull request that will close this issuefix(auth): require the password to turn MFA off (#754) #848
on Sep 30, 2026 - added a commit that references this issue
on Oct 1, 2026
Issue
In
backend/internal/application/auth/mfa_usecase.go, disabling MFAdoes not prompt for the user's current password (
TODO: Verify password before disablingremains in the code).Impact
If a session remains active on a workstation (shared PC, failure to lock the screen),
anyone can disable the account's MFA with a single click, without re-authentication.
This constitutes a critical security control bypass (loss of an authentication
factor without proof of possession of the first factor).
Expected Fix
disabling MFA.
Severity
Critical — authentication bypass.