sec(auth): MFA can be replaced with a session and password alone #227

Summary

Disabling MFA requires the password plus a TOTP or recovery code, on the stated reasoning that a stolen session alone should not disable MFA. Re-enrolling requires only the session and the password: MFASetup never checks whether MFA is already enabled, and MFAEnable then overwrites the existing secret and replaces all recovery codes. An attacker holding a live session and the password can enroll their own authenticator, which is strictly weaker than the disable path it is equivalent to.

Location

internal/auth/service_mfa.go:285 at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd

Failure scenario

MFADisable demands password plus a TOTP or recovery code ("a stolen session alone shouldn't disable MFA", service_mfa.go:427-431). But MFASetup never checks user.MFAEnabled; it only re-verifies the password, writes a new pending secret, and MFAEnable promotes it over the existing MFASecret and replaces all recovery codes. An attacker with a live session and the password (the exact threat MFADisable models) calls setup then enable with a code from their own authenticator: the victim's authenticator stops working, the attacker's is enrolled, and the victim's recovery codes are gone. The re-enrol path is strictly weaker than the disable path it is equivalent to.

Evidence

if !s.verifyPassword(password, user.PasswordHash) {
    return nil, fmt.Errorf("%w", ErrMFAInvalidPassword)
}
secret, err := generateMFASecret()
...
user.MFAPendingSecret = secret
user.MFAPendingSecretExpiresAt = &expiresAt

Suggested fix

When user.MFAEnabled is true, require a current TOTP or recovery code in MFASetup (same check as MFADisable) before writing a new pending secret; or refuse setup while enabled and require disable first.

Found by the 2026-09-02 codebase audit, finding A03-007, reported by one reviewer and independently confirmed by a second. Full report: docs/audits/codebase-audit-2026-09-02.md.