fix(mfa): honor MFA availability in dashboard toggle + align admin check with login#679
Merged
Merged
Conversation
…eck with login The dashboard showed users as 'MFA Enabled' and offered an Enable-MFA toggle even on instances with MFA disabled, then the server rejected it with a cryptic 'all mfa services are disabled'. Root cause: the admin UpdateUser MFA-enable check only tested the three method flags (ignoring the master --enable-mfa and the email/SMS service availability that login actually requires), and _admin_meta gave the dashboard no MFA-availability signal. - Add isMFAServiceAvailable() mirroring login's exact gating (EnableMFA AND at least one usable method: TOTP, or email OTP+SMTP, or SMS OTP+Twilio). Used by admin UpdateUser and self-service update_profile; disabling MFA stays always-allowed. - Actionable error naming the flags to set. - Expose is_multi_factor_auth_service_enabled on _admin_meta (GraphQL+proto). - Dashboard: disable the Enable-MFA action with an explanatory tooltip when the server can't do MFA; Disable-MFA remains available. - Test: enable rejected when unavailable, allowed when EnableMFA+TOTP, disable always allowed, _admin_meta reflects config.
#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.
Bug
The dashboard showed a user as "MFA Enabled" and offered an Enable-MFA toggle even on an instance running with no MFA flags, then the server rejected enabling with a cryptic
cannot enable multi factor authentication as all mfa services are disabled— leaving the admin confused and unable to re-enable.Root cause
UpdateUserMFA-enable check only tested the three method flags (EnableEmailOTP || EnableTOTPLogin || EnableSMSOTP), ignoring the master--enable-mfaand the email/SMS service availability that login-time MFA actually requires — so it was inconsistent with what login can honor._admin_metagave the dashboard no MFA-availability signal, so the UI couldn't disable/explain the toggle.Fix
isMFAServiceAvailable()mirrors login's exact gating:EnableMFA && ((EnableEmailOTP && IsEmailServiceEnabled) || (EnableSMSOTP && IsSMSServiceEnabled) || EnableTOTPLogin). Used by adminUpdateUserand self-serviceupdate_profile. Disabling MFA stays always-allowed so an admin can turn it off after the server stops offering a method.--enable-mfaplus a method).is_multi_factor_auth_service_enabledadded to_admin_meta(GraphQL + proto, gen committed).Testing
TestMFAServiceAvailability: enable rejected when unavailable; allowed with--enable-mfa --enable-totp-login; disable always allowed;_admin_metareflects config.go build/vet/golangci-lint(0 issues), graphql+proto codegen parity, full sqlite integration suite green; dashboardtsc/build/prettier clean.Note for operators (unchanged behavior, now surfaced): MFA needs
--enable-mfaplus a method — TOTP (--enable-totp-login, no external service), email OTP (--enable-email-otp+ SMTP), or SMS OTP (--enable-sms-otp+ Twilio). v2 reads CLI flags, not env vars.