feat(mfa)!: enable MFA by default and make it optional#682
Merged
Conversation
MFA methods are now on by default and opted out via --disable-totp-login, --disable-email-otp, --disable-sms-otp; the old --enable-* / --enable-mfa flags are removed. Availability is derived from the enabled methods in Config.Finalize() (wired via the root PersistentPreRun so `mcp` is consistent), fixing the state where MFA read "enabled" while every method was unavailable. Email/SMS OTP only count when their provider is configured. --enforce-mfa now defaults to false: MFA is optional and skippable, enable later via profile edit; set --enforce-mfa to require it. BREAKING CHANGE: --enable-totp-login, --enable-mfa, --enable-email-otp, --enable-sms-otp removed; --enforce-mfa default flips true -> false.
lakhansamani
force-pushed
the
feat/mfa-default-on
branch
from
fd405c3 to
4b16d48
Compare
lakhansamani
added a commit
that referenced
this pull request
Jul 12, 2026
#682 removed the --enable-* MFA flags in favor of default-on methods opted out via --disable-*, and made EnableMFA derived. Update the admin/self-service error messages, dashboard tooltip, and schema/comment wording to reference the current flag surface instead of the removed --enable-mfa/--enable-totp-login/--enable-email-otp/--enable-sms-otp. Regenerate GraphQL for the reworded schema description. No behavior change: isMFAServiceAvailable still gates on the same derived availability condition.
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
1 participant
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
Makes MFA on by default and optional per user, and fixes the config contradiction behind the "all MFA services are disabled" error (issue class S1685).
Previously
--enforce-mfadefaulted totruewhile--enable-mfa/--enable-totp-login/--enable-email-otp/--enable-sms-otpall defaulted tofalse. Enforcement flagged users'IsMultiFactorAuthEnabled=trueat signup, but login never actually challenged because the global + per-method flags were off — MFA was "enforced" yet non-functional, and re-enabling after a disable threw "cannot enable multi factor authentication as all mfa services are disabled".What changed
Flags (breaking):
--enable-totp-login--disable-totp-login--enable-email-otp--disable-email-otp--enable-sms-otp--disable-sms-otp--enable-mfa--enforce-mfa(wastrue)--enforce-mfafalseBehavior:
--disable-*.EnableMFAis derived (Config.Finalize()) =TOTP || (emailOTP && emailSvc) || (smsOTP && smsSvc), so MFA can never read "enabled" while every method is unavailable.IsMultiFactorAuthEnabled=false) and enable later via profile edit — no new DB field; the existing bool is the record.--enforce-mfaforces it when set (unchanged signup enforcement logic, only the default flips).Wiring: derivation runs in a root-command
PersistentPreRunsoauthorizerandauthorizer mcpstay consistent (the inline service-availability derivation moved intoConfig.Finalize()).Auth-gating logic is untouched
login.go/verify_email.go/resend_otp.go/meta.gostill read the positiveEnable*fields — only how those fields are populated changed. Zero changes to the challenge logic.Testing
TestFinalizeMFADerivation— full availability truth table (TOTP-only, provider-gated email/SMS, all-disabled).TestMFA*,TestSignup*,TestLogin*,TestUpdateProfile*,TestUpdateUser*) passes on SQLite./metareportsis_multi_factor_auth_enabled: true(via TOTP);--disable-totp-loginwith no providers →false.gofmtclean,golangci-lint0 issues,go vetclean.Migration
Existing deployments passing the removed
--enable-*flags must switch to the--disable-*inverse, and any relying on the implicit--enforce-mfa=truedefault must now pass it explicitly. Documented in CHANGELOG under Unreleased. Acceptable for the2.4.0-rcline.