From 4d0144fd7a66e6d234d980ed44dd5995ceb1b2ba Mon Sep 17 00:00:00 2001 From: Lakhan Samani Date: Sun, 12 Jul 2026 08:16:40 +0530 Subject: [PATCH 1/2] fix(mfa): honor MFA availability in dashboard toggle + align admin check 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. --- gen/go/authorizer/v1/admin.pb.go | 35 +++++--- gen/openapi/authorizer.swagger.json | 3 + internal/graph/generated/generated.go | 69 +++++++++++++++- internal/graph/model/models_gen.go | 7 +- internal/graph/schema.graphqls | 4 + internal/grpcsrv/handlers/admin_project.go | 7 +- .../mfa_service_availability_test.go | 82 +++++++++++++++++++ internal/service/admin_auth.go | 7 +- internal/service/admin_users.go | 30 ++++--- internal/service/update_profile.go | 13 ++- proto/authorizer/v1/admin.proto | 1 + web/dashboard/src/graphql/queries/index.ts | 1 + web/dashboard/src/pages/Users.tsx | 30 ++++++- 13 files changed, 245 insertions(+), 44 deletions(-) create mode 100644 internal/integration_tests/mfa_service_availability_test.go diff --git a/gen/go/authorizer/v1/admin.pb.go b/gen/go/authorizer/v1/admin.pb.go index 38e6f2e8..52e44640 100644 --- a/gen/go/authorizer/v1/admin.pb.go +++ b/gen/go/authorizer/v1/admin.pb.go @@ -370,9 +370,10 @@ type AdminMeta struct { sizeCache protoimpl.SizeCache unknownFields protoimpl.UnknownFields - Roles []string `protobuf:"bytes,1,rep,name=roles,proto3" json:"roles,omitempty"` - DefaultRoles []string `protobuf:"bytes,2,rep,name=default_roles,json=defaultRoles,proto3" json:"default_roles,omitempty"` - ProtectedRoles []string `protobuf:"bytes,3,rep,name=protected_roles,json=protectedRoles,proto3" json:"protected_roles,omitempty"` + Roles []string `protobuf:"bytes,1,rep,name=roles,proto3" json:"roles,omitempty"` + DefaultRoles []string `protobuf:"bytes,2,rep,name=default_roles,json=defaultRoles,proto3" json:"default_roles,omitempty"` + ProtectedRoles []string `protobuf:"bytes,3,rep,name=protected_roles,json=protectedRoles,proto3" json:"protected_roles,omitempty"` + IsMultiFactorAuthServiceEnabled bool `protobuf:"varint,4,opt,name=is_multi_factor_auth_service_enabled,json=isMultiFactorAuthServiceEnabled,proto3" json:"is_multi_factor_auth_service_enabled,omitempty"` } func (x *AdminMeta) Reset() { @@ -426,6 +427,13 @@ func (x *AdminMeta) GetProtectedRoles() []string { return nil } +func (x *AdminMeta) GetIsMultiFactorAuthServiceEnabled() bool { + if x != nil { + return x.IsMultiFactorAuthServiceEnabled + } + return false +} + type UsersRequest struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -5631,14 +5639,19 @@ var file_authorizer_v1_admin_proto_rawDesc = []byte{ 0x5f, 0x6d, 0x65, 0x74, 0x61, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x18, 0x2e, 0x61, 0x75, 0x74, 0x68, 0x6f, 0x72, 0x69, 0x7a, 0x65, 0x72, 0x2e, 0x76, 0x31, 0x2e, 0x41, 0x64, 0x6d, 0x69, 0x6e, 0x4d, 0x65, 0x74, 0x61, 0x52, 0x09, 0x61, 0x64, 0x6d, 0x69, 0x6e, 0x4d, 0x65, 0x74, 0x61, - 0x22, 0x6f, 0x0a, 0x09, 0x41, 0x64, 0x6d, 0x69, 0x6e, 0x4d, 0x65, 0x74, 0x61, 0x12, 0x14, 0x0a, - 0x05, 0x72, 0x6f, 0x6c, 0x65, 0x73, 0x18, 0x01, 0x20, 0x03, 0x28, 0x09, 0x52, 0x05, 0x72, 0x6f, - 0x6c, 0x65, 0x73, 0x12, 0x23, 0x0a, 0x0d, 0x64, 0x65, 0x66, 0x61, 0x75, 0x6c, 0x74, 0x5f, 0x72, - 0x6f, 0x6c, 0x65, 0x73, 0x18, 0x02, 0x20, 0x03, 0x28, 0x09, 0x52, 0x0c, 0x64, 0x65, 0x66, 0x61, - 0x75, 0x6c, 0x74, 0x52, 0x6f, 0x6c, 0x65, 0x73, 0x12, 0x27, 0x0a, 0x0f, 0x70, 0x72, 0x6f, 0x74, - 0x65, 0x63, 0x74, 0x65, 0x64, 0x5f, 0x72, 0x6f, 0x6c, 0x65, 0x73, 0x18, 0x03, 0x20, 0x03, 0x28, - 0x09, 0x52, 0x0e, 0x70, 0x72, 0x6f, 0x74, 0x65, 0x63, 0x74, 0x65, 0x64, 0x52, 0x6f, 0x6c, 0x65, - 0x73, 0x22, 0x66, 0x0a, 0x0c, 0x55, 0x73, 0x65, 0x72, 0x73, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, + 0x22, 0xbe, 0x01, 0x0a, 0x09, 0x41, 0x64, 0x6d, 0x69, 0x6e, 0x4d, 0x65, 0x74, 0x61, 0x12, 0x14, + 0x0a, 0x05, 0x72, 0x6f, 0x6c, 0x65, 0x73, 0x18, 0x01, 0x20, 0x03, 0x28, 0x09, 0x52, 0x05, 0x72, + 0x6f, 0x6c, 0x65, 0x73, 0x12, 0x23, 0x0a, 0x0d, 0x64, 0x65, 0x66, 0x61, 0x75, 0x6c, 0x74, 0x5f, + 0x72, 0x6f, 0x6c, 0x65, 0x73, 0x18, 0x02, 0x20, 0x03, 0x28, 0x09, 0x52, 0x0c, 0x64, 0x65, 0x66, + 0x61, 0x75, 0x6c, 0x74, 0x52, 0x6f, 0x6c, 0x65, 0x73, 0x12, 0x27, 0x0a, 0x0f, 0x70, 0x72, 0x6f, + 0x74, 0x65, 0x63, 0x74, 0x65, 0x64, 0x5f, 0x72, 0x6f, 0x6c, 0x65, 0x73, 0x18, 0x03, 0x20, 0x03, + 0x28, 0x09, 0x52, 0x0e, 0x70, 0x72, 0x6f, 0x74, 0x65, 0x63, 0x74, 0x65, 0x64, 0x52, 0x6f, 0x6c, + 0x65, 0x73, 0x12, 0x4d, 0x0a, 0x24, 0x69, 0x73, 0x5f, 0x6d, 0x75, 0x6c, 0x74, 0x69, 0x5f, 0x66, + 0x61, 0x63, 0x74, 0x6f, 0x72, 0x5f, 0x61, 0x75, 0x74, 0x68, 0x5f, 0x73, 0x65, 0x72, 0x76, 0x69, + 0x63, 0x65, 0x5f, 0x65, 0x6e, 0x61, 0x62, 0x6c, 0x65, 0x64, 0x18, 0x04, 0x20, 0x01, 0x28, 0x08, + 0x52, 0x1f, 0x69, 0x73, 0x4d, 0x75, 0x6c, 0x74, 0x69, 0x46, 0x61, 0x63, 0x74, 0x6f, 0x72, 0x41, + 0x75, 0x74, 0x68, 0x53, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x45, 0x6e, 0x61, 0x62, 0x6c, 0x65, + 0x64, 0x22, 0x66, 0x0a, 0x0c, 0x55, 0x73, 0x65, 0x72, 0x73, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x40, 0x0a, 0x0a, 0x70, 0x61, 0x67, 0x69, 0x6e, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x20, 0x2e, 0x61, 0x75, 0x74, 0x68, 0x6f, 0x72, 0x69, 0x7a, 0x65, 0x72, 0x2e, 0x76, 0x31, 0x2e, 0x50, 0x61, 0x67, 0x69, 0x6e, 0x61, 0x74, 0x69, 0x6f, 0x6e, diff --git a/gen/openapi/authorizer.swagger.json b/gen/openapi/authorizer.swagger.json index 06dc2f7d..bb491e90 100644 --- a/gen/openapi/authorizer.swagger.json +++ b/gen/openapi/authorizer.swagger.json @@ -2072,6 +2072,9 @@ "items": { "type": "string" } + }, + "is_multi_factor_auth_service_enabled": { + "type": "boolean" } }, "description": "AdminMeta mirrors model.AdminMeta exactly (no version field)." diff --git a/internal/graph/generated/generated.go b/internal/graph/generated/generated.go index 5d52c31b..b553e5c6 100644 --- a/internal/graph/generated/generated.go +++ b/internal/graph/generated/generated.go @@ -47,9 +47,10 @@ type DirectiveRoot struct { type ComplexityRoot struct { AdminMeta struct { - DefaultRoles func(childComplexity int) int - ProtectedRoles func(childComplexity int) int - Roles func(childComplexity int) int + DefaultRoles func(childComplexity int) int + IsMultiFactorAuthServiceEnabled func(childComplexity int) int + ProtectedRoles func(childComplexity int) int + Roles func(childComplexity int) int } AuditLog struct { @@ -768,6 +769,13 @@ func (e *executableSchema) Complexity(ctx context.Context, typeName, field strin return e.complexity.AdminMeta.DefaultRoles(childComplexity), true + case "AdminMeta.is_multi_factor_auth_service_enabled": + if e.complexity.AdminMeta.IsMultiFactorAuthServiceEnabled == nil { + break + } + + return e.complexity.AdminMeta.IsMultiFactorAuthServiceEnabled(childComplexity), true + case "AdminMeta.protected_roles": if e.complexity.AdminMeta.ProtectedRoles == nil { break @@ -4365,6 +4373,10 @@ type AdminMeta { roles: [String!]! default_roles: [String!]! protected_roles: [String!]! + # is_multi_factor_auth_service_enabled reports whether MFA can actually be used + # on this instance (master switch on plus at least one usable method). The + # dashboard uses it to gate the per-user MFA enable control. + is_multi_factor_auth_service_enabled: Boolean! } type User { @@ -8553,6 +8565,50 @@ func (ec *executionContext) fieldContext_AdminMeta_protected_roles(_ context.Con return fc, nil } +func (ec *executionContext) _AdminMeta_is_multi_factor_auth_service_enabled(ctx context.Context, field graphql.CollectedField, obj *model.AdminMeta) (ret graphql.Marshaler) { + fc, err := ec.fieldContext_AdminMeta_is_multi_factor_auth_service_enabled(ctx, field) + if err != nil { + return graphql.Null + } + ctx = graphql.WithFieldContext(ctx, fc) + defer func() { + if r := recover(); r != nil { + ec.Error(ctx, ec.Recover(ctx, r)) + ret = graphql.Null + } + }() + resTmp, err := ec.ResolverMiddleware(ctx, func(rctx context.Context) (any, error) { + ctx = rctx // use context from middleware stack in children + return obj.IsMultiFactorAuthServiceEnabled, nil + }) + if err != nil { + ec.Error(ctx, err) + return graphql.Null + } + if resTmp == nil { + if !graphql.HasFieldError(ctx, fc) { + ec.Errorf(ctx, "must not be null") + } + return graphql.Null + } + res := resTmp.(bool) + fc.Result = res + return ec.marshalNBoolean2bool(ctx, field.Selections, res) +} + +func (ec *executionContext) fieldContext_AdminMeta_is_multi_factor_auth_service_enabled(_ context.Context, field graphql.CollectedField) (fc *graphql.FieldContext, err error) { + fc = &graphql.FieldContext{ + Object: "AdminMeta", + Field: field, + IsMethod: false, + IsResolver: false, + Child: func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { + return nil, errors.New("field of type Boolean does not have child fields") + }, + } + return fc, nil +} + func (ec *executionContext) _AuditLog_id(ctx context.Context, field graphql.CollectedField, obj *model.AuditLog) (ret graphql.Marshaler) { fc, err := ec.fieldContext_AuditLog_id(ctx, field) if err != nil { @@ -23294,6 +23350,8 @@ func (ec *executionContext) fieldContext_Query__admin_meta(_ context.Context, fi return ec.fieldContext_AdminMeta_default_roles(ctx, field) case "protected_roles": return ec.fieldContext_AdminMeta_protected_roles(ctx, field) + case "is_multi_factor_auth_service_enabled": + return ec.fieldContext_AdminMeta_is_multi_factor_auth_service_enabled(ctx, field) } return nil, fmt.Errorf("no field named %q was found under type AdminMeta", field.Name) }, @@ -35300,6 +35358,11 @@ func (ec *executionContext) _AdminMeta(ctx context.Context, sel ast.SelectionSet if out.Values[i] == graphql.Null { out.Invalids++ } + case "is_multi_factor_auth_service_enabled": + out.Values[i] = ec._AdminMeta_is_multi_factor_auth_service_enabled(ctx, field, obj) + if out.Values[i] == graphql.Null { + out.Invalids++ + } default: panic("unknown field " + strconv.Quote(field.Name)) } diff --git a/internal/graph/model/models_gen.go b/internal/graph/model/models_gen.go index 12862a3c..be4b7b34 100644 --- a/internal/graph/model/models_gen.go +++ b/internal/graph/model/models_gen.go @@ -48,9 +48,10 @@ type AdminLoginRequest struct { } type AdminMeta struct { - Roles []string `json:"roles"` - DefaultRoles []string `json:"default_roles"` - ProtectedRoles []string `json:"protected_roles"` + Roles []string `json:"roles"` + DefaultRoles []string `json:"default_roles"` + ProtectedRoles []string `json:"protected_roles"` + IsMultiFactorAuthServiceEnabled bool `json:"is_multi_factor_auth_service_enabled"` } type AdminSignupRequest struct { diff --git a/internal/graph/schema.graphqls b/internal/graph/schema.graphqls index 7b488d44..a19073c9 100644 --- a/internal/graph/schema.graphqls +++ b/internal/graph/schema.graphqls @@ -43,6 +43,10 @@ type AdminMeta { roles: [String!]! default_roles: [String!]! protected_roles: [String!]! + # is_multi_factor_auth_service_enabled reports whether MFA can actually be used + # on this instance (master switch on plus at least one usable method). The + # dashboard uses it to gate the per-user MFA enable control. + is_multi_factor_auth_service_enabled: Boolean! } type User { diff --git a/internal/grpcsrv/handlers/admin_project.go b/internal/grpcsrv/handlers/admin_project.go index 954b9409..1ede826d 100644 --- a/internal/grpcsrv/handlers/admin_project.go +++ b/internal/grpcsrv/handlers/admin_project.go @@ -19,9 +19,10 @@ func projectAdminMeta(m *model.AdminMeta) *authorizerv1.AdminMeta { return nil } return &authorizerv1.AdminMeta{ - Roles: m.Roles, - DefaultRoles: m.DefaultRoles, - ProtectedRoles: m.ProtectedRoles, + Roles: m.Roles, + DefaultRoles: m.DefaultRoles, + ProtectedRoles: m.ProtectedRoles, + IsMultiFactorAuthServiceEnabled: m.IsMultiFactorAuthServiceEnabled, } } diff --git a/internal/integration_tests/mfa_service_availability_test.go b/internal/integration_tests/mfa_service_availability_test.go new file mode 100644 index 00000000..436ec343 --- /dev/null +++ b/internal/integration_tests/mfa_service_availability_test.go @@ -0,0 +1,82 @@ +package integration_tests + +import ( + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + "github.com/authorizerdev/authorizer/internal/graph/model" + "github.com/authorizerdev/authorizer/internal/refs" +) + +// TestMFAServiceAvailability verifies that enabling a user's MFA via the admin +// UpdateUser path is gated on the instance actually being able to do MFA (the +// same criteria login uses), that _admin_meta reports that availability, and +// that disabling MFA is always allowed regardless. +func TestMFAServiceAvailability(t *testing.T) { + t.Run("service disabled: admin_meta false, enable rejected, disable allowed", func(t *testing.T) { + cfg := getTestConfig() // no MFA flags -> service unavailable + ts := initTestSetup(t, cfg) + _, ctx := createContext(ts) + + setAdminCookie(t, ts) + meta, err := ts.GraphQLProvider.AdminMeta(ctx) + require.NoError(t, err) + require.False(t, meta.IsMultiFactorAuthServiceEnabled, "no MFA method configured") + + email := "mfa_off_" + uuid.New().String() + "@authorizer.dev" + su, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{ + Email: &email, Password: "Password@123", ConfirmPassword: "Password@123", + }) + require.NoError(t, err) + + // Enabling MFA is rejected when no MFA service is available. + setAdminCookie(t, ts) + _, err = ts.GraphQLProvider.UpdateUser(ctx, &model.UpdateUserRequest{ + ID: su.User.ID, IsMultiFactorAuthEnabled: refs.NewBoolRef(true), + }) + require.Error(t, err) + + // Force the user MFA-enabled directly, then confirm disabling is allowed + // even though the service is off (an admin must be able to turn it off). + u, err := ts.StorageProvider.GetUserByID(ctx, su.User.ID) + require.NoError(t, err) + u.IsMultiFactorAuthEnabled = refs.NewBoolRef(true) + _, err = ts.StorageProvider.UpdateUser(ctx, u) + require.NoError(t, err) + + setAdminCookie(t, ts) + res, err := ts.GraphQLProvider.UpdateUser(ctx, &model.UpdateUserRequest{ + ID: su.User.ID, IsMultiFactorAuthEnabled: refs.NewBoolRef(false), + }) + require.NoError(t, err) + require.False(t, refs.BoolValue(res.IsMultiFactorAuthEnabled)) + }) + + t.Run("service enabled (mfa+totp): admin_meta true, enable succeeds", func(t *testing.T) { + cfg := getTestConfig() + cfg.EnableMFA = true + cfg.EnableTOTPLogin = true + ts := initTestSetup(t, cfg) + _, ctx := createContext(ts) + + setAdminCookie(t, ts) + meta, err := ts.GraphQLProvider.AdminMeta(ctx) + require.NoError(t, err) + require.True(t, meta.IsMultiFactorAuthServiceEnabled) + + email := "mfa_on_" + uuid.New().String() + "@authorizer.dev" + su, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{ + Email: &email, Password: "Password@123", ConfirmPassword: "Password@123", + }) + require.NoError(t, err) + + setAdminCookie(t, ts) + res, err := ts.GraphQLProvider.UpdateUser(ctx, &model.UpdateUserRequest{ + ID: su.User.ID, IsMultiFactorAuthEnabled: refs.NewBoolRef(true), + }) + require.NoError(t, err) + require.True(t, refs.BoolValue(res.IsMultiFactorAuthEnabled)) + }) +} diff --git a/internal/service/admin_auth.go b/internal/service/admin_auth.go index 9536511f..d7ce3d12 100644 --- a/internal/service/admin_auth.go +++ b/internal/service/admin_auth.go @@ -106,8 +106,9 @@ func (p *provider) AdminMeta(ctx context.Context, meta RequestMetadata) (*model. protectedRoles = []string{} } return &model.AdminMeta{ - Roles: roles, - DefaultRoles: defaultRoles, - ProtectedRoles: protectedRoles, + Roles: roles, + DefaultRoles: defaultRoles, + ProtectedRoles: protectedRoles, + IsMultiFactorAuthServiceEnabled: p.isMFAServiceAvailable(), }, nil, nil } diff --git a/internal/service/admin_users.go b/internal/service/admin_users.go index a785e569..a6b6b116 100644 --- a/internal/service/admin_users.go +++ b/internal/service/admin_users.go @@ -21,6 +21,19 @@ import ( "github.com/authorizerdev/authorizer/internal/validators" ) +// isMFAServiceAvailable reports whether multi-factor auth can actually be used +// on this instance. It mirrors the login-time gating exactly (see login.go): the +// master MFA switch must be on AND at least one method must be fully configured — +// email OTP needs SMTP, SMS OTP needs Twilio, TOTP needs nothing extra. The admin +// UpdateUser and self-service update_profile MFA-enable paths use this so they +// never accept an MFA state that login would be unable to honor. +func (p *provider) isMFAServiceAvailable() bool { + c := p.Config + return c.EnableMFA && ((c.EnableEmailOTP && c.IsEmailServiceEnabled) || + (c.EnableSMSOTP && c.IsSMSServiceEnabled) || + c.EnableTOTPLogin) +} + // Users returns a paginated list of all users, optionally filtered by a // case-insensitive substring search (params.Query) over email/given_name/ // family_name/nickname. Requires super-admin auth. Logic migrated from @@ -160,18 +173,13 @@ func (p *provider) UpdateUser(ctx context.Context, meta RequestMetadata, params } if params.IsMultiFactorAuthEnabled != nil && refs.BoolValue(user.IsMultiFactorAuthEnabled) != refs.BoolValue(params.IsMultiFactorAuthEnabled) { - user.IsMultiFactorAuthEnabled = params.IsMultiFactorAuthEnabled - if refs.BoolValue(params.IsMultiFactorAuthEnabled) { - // Check if totp, email or sms is enabled. - isMailOTPEnvServiceEnabled := p.Config.EnableEmailOTP - isTOTPEnvServiceEnabled := p.Config.EnableTOTPLogin - isSMSOTPEnvServiceEnabled := p.Config.EnableSMSOTP - // Initialize a flag to check if enabling Mail OTP is required. - if !isMailOTPEnvServiceEnabled && !isTOTPEnvServiceEnabled && !isSMSOTPEnvServiceEnabled { - log.Debug().Msg("cannot enable multi factor authentication as all mfa services are disabled") - return nil, nil, errors.New("cannot enable multi factor authentication as all mfa services are disabled") - } + // Only gate the enable action; disabling MFA is always allowed so an admin + // can still turn it off after the server has stopped offering any method. + if refs.BoolValue(params.IsMultiFactorAuthEnabled) && !p.isMFAServiceAvailable() { + log.Debug().Msg("cannot enable multi factor authentication as no mfa method is available") + return nil, nil, errors.New("cannot enable MFA: enable it on the server with --enable-mfa plus at least one method — --enable-totp-login, --enable-email-otp (requires SMTP configured), or --enable-sms-otp (requires Twilio configured)") } + user.IsMultiFactorAuthEnabled = params.IsMultiFactorAuthEnabled } if params.EmailVerified != nil { diff --git a/internal/service/update_profile.go b/internal/service/update_profile.go index ed08cb67..6c2bdad5 100644 --- a/internal/service/update_profile.go +++ b/internal/service/update_profile.go @@ -101,14 +101,11 @@ func (p *provider) UpdateProfile(ctx context.Context, meta RequestMetadata, para } // Check if the user is trying to enable or disable multi-factor authentication (MFA) if params.IsMultiFactorAuthEnabled != nil && refs.BoolValue(user.IsMultiFactorAuthEnabled) != refs.BoolValue(params.IsMultiFactorAuthEnabled) { - // Check if totp, email or sms is enabled - isMailOTPEnvServiceEnabled := p.Config.EnableEmailOTP - isTOTPEnvServiceEnabled := p.Config.EnableTOTPLogin - isSMSOTPEnvServiceEnabled := p.Config.EnableSMSOTP - // Initialize a flag to check if enabling Mail OTP is required - if !isMailOTPEnvServiceEnabled && !isTOTPEnvServiceEnabled && !isSMSOTPEnvServiceEnabled { - log.Debug().Msg("Cannot enable mfa service as all mfa services are disabled") - return nil, nil, FailedPrecondition("cannot enable multi factor authentication as all mfa services are disabled") + // Only gate the enable action; disabling is always allowed (subject to the + // enforce check below). Uses the same availability rule as login-time gating. + if refs.BoolValue(params.IsMultiFactorAuthEnabled) && !p.isMFAServiceAvailable() { + log.Debug().Msg("Cannot enable mfa as no mfa method is available") + return nil, nil, FailedPrecondition("cannot enable MFA: enable it on the server with --enable-mfa plus at least one method — --enable-totp-login, --enable-email-otp (requires SMTP configured), or --enable-sms-otp (requires Twilio configured)") } isMFAEnforced := p.Config.EnforceMFA diff --git a/proto/authorizer/v1/admin.proto b/proto/authorizer/v1/admin.proto index 06e1a456..638121c7 100644 --- a/proto/authorizer/v1/admin.proto +++ b/proto/authorizer/v1/admin.proto @@ -457,6 +457,7 @@ message AdminMeta { repeated string roles = 1; repeated string default_roles = 2; repeated string protected_roles = 3; + bool is_multi_factor_auth_service_enabled = 4; } // === Users === diff --git a/web/dashboard/src/graphql/queries/index.ts b/web/dashboard/src/graphql/queries/index.ts index 39db017c..23e682a7 100644 --- a/web/dashboard/src/graphql/queries/index.ts +++ b/web/dashboard/src/graphql/queries/index.ts @@ -182,6 +182,7 @@ export const AdminRolesQuery = ` query adminMeta { _admin_meta { roles + is_multi_factor_auth_service_enabled } } `; diff --git a/web/dashboard/src/pages/Users.tsx b/web/dashboard/src/pages/Users.tsx index 6f893894..d3334f1b 100644 --- a/web/dashboard/src/pages/Users.tsx +++ b/web/dashboard/src/pages/Users.tsx @@ -12,7 +12,7 @@ import { Copy, Search, } from 'lucide-react'; -import { UserDetailsQuery } from '../graphql/queries'; +import { UserDetailsQuery, AdminRolesQuery } from '../graphql/queries'; import { EnableAccess, RevokeAccess, UpdateUser } from '../graphql/mutation'; import { copyTextToClipboard, getGraphQLErrorMessage } from '../utils'; import EditUserModal from '../components/EditUserModal'; @@ -92,6 +92,25 @@ export default function Users() { const [permissionsUser, setPermissionsUser] = React.useState( null, ); + // Whether the server can actually do MFA (master flag + a usable method). + // Default true so the Enable-MFA action isn't disabled before the meta loads; + // the server enforces the real check regardless. + const [mfaServiceEnabled, setMfaServiceEnabled] = + React.useState(true); + + React.useEffect(() => { + client + .query<{ + _admin_meta?: { is_multi_factor_auth_service_enabled?: boolean }; + }>(AdminRolesQuery, {}) + .toPromise() + .then(({ data }) => { + setMfaServiceEnabled( + data?._admin_meta?.is_multi_factor_auth_service_enabled ?? true, + ); + }); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); const updateUserList = async () => { setLoading(true); @@ -450,12 +469,19 @@ export default function Users() { > Disable MFA - ) : ( + ) : mfaServiceEnabled ? ( multiFactorAuthUpdateHandler(user)} > Enable MFA + ) : ( + + Enable MFA (unavailable) + )} From 9cd44ebe2875c20305c25bf7e400ff1b310a9750 Mon Sep 17 00:00:00 2001 From: Lakhan Samani Date: Sun, 12 Jul 2026 10:43:39 +0530 Subject: [PATCH 2/2] fix(mfa): align availability guidance with --disable-* flags #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. --- internal/graph/generated/generated.go | 5 +++-- internal/graph/schema.graphqls | 5 +++-- internal/service/admin_users.go | 13 +++++++------ internal/service/update_profile.go | 2 +- web/dashboard/src/pages/Users.tsx | 2 +- 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/internal/graph/generated/generated.go b/internal/graph/generated/generated.go index d792351e..993c39e0 100644 --- a/internal/graph/generated/generated.go +++ b/internal/graph/generated/generated.go @@ -4438,8 +4438,9 @@ type AdminMeta { default_roles: [String!]! protected_roles: [String!]! # is_multi_factor_auth_service_enabled reports whether MFA can actually be used - # on this instance (master switch on plus at least one usable method). The - # dashboard uses it to gate the per-user MFA enable control. + # on this instance (at least one usable method: TOTP, or email/SMS OTP with its + # provider configured). The dashboard uses it to gate the per-user MFA enable + # control. is_multi_factor_auth_service_enabled: Boolean! } diff --git a/internal/graph/schema.graphqls b/internal/graph/schema.graphqls index d11976d0..5af5f478 100644 --- a/internal/graph/schema.graphqls +++ b/internal/graph/schema.graphqls @@ -52,8 +52,9 @@ type AdminMeta { default_roles: [String!]! protected_roles: [String!]! # is_multi_factor_auth_service_enabled reports whether MFA can actually be used - # on this instance (master switch on plus at least one usable method). The - # dashboard uses it to gate the per-user MFA enable control. + # on this instance (at least one usable method: TOTP, or email/SMS OTP with its + # provider configured). The dashboard uses it to gate the per-user MFA enable + # control. is_multi_factor_auth_service_enabled: Boolean! } diff --git a/internal/service/admin_users.go b/internal/service/admin_users.go index a6b6b116..525089d6 100644 --- a/internal/service/admin_users.go +++ b/internal/service/admin_users.go @@ -22,11 +22,12 @@ import ( ) // isMFAServiceAvailable reports whether multi-factor auth can actually be used -// on this instance. It mirrors the login-time gating exactly (see login.go): the -// master MFA switch must be on AND at least one method must be fully configured — -// email OTP needs SMTP, SMS OTP needs Twilio, TOTP needs nothing extra. The admin -// UpdateUser and self-service update_profile MFA-enable paths use this so they -// never accept an MFA state that login would be unable to honor. +// on this instance. It mirrors the login-time gating exactly (see login.go): at +// least one method must be usable — email OTP needs SMTP, SMS OTP needs Twilio, +// TOTP needs nothing extra. This is precisely config.EnableMFA, which is derived +// (config.Finalize) as the OR of those usable methods. The admin UpdateUser and +// self-service update_profile MFA-enable paths use this so they never accept an +// MFA state that login would be unable to honor. func (p *provider) isMFAServiceAvailable() bool { c := p.Config return c.EnableMFA && ((c.EnableEmailOTP && c.IsEmailServiceEnabled) || @@ -177,7 +178,7 @@ func (p *provider) UpdateUser(ctx context.Context, meta RequestMetadata, params // can still turn it off after the server has stopped offering any method. if refs.BoolValue(params.IsMultiFactorAuthEnabled) && !p.isMFAServiceAvailable() { log.Debug().Msg("cannot enable multi factor authentication as no mfa method is available") - return nil, nil, errors.New("cannot enable MFA: enable it on the server with --enable-mfa plus at least one method — --enable-totp-login, --enable-email-otp (requires SMTP configured), or --enable-sms-otp (requires Twilio configured)") + return nil, nil, errors.New("cannot enable MFA: no MFA method is available on this server — ensure TOTP is enabled (do not set --disable-totp-login) or configure an email (SMTP) or SMS (Twilio) provider for OTP") } user.IsMultiFactorAuthEnabled = params.IsMultiFactorAuthEnabled } diff --git a/internal/service/update_profile.go b/internal/service/update_profile.go index 6c2bdad5..e0631df6 100644 --- a/internal/service/update_profile.go +++ b/internal/service/update_profile.go @@ -105,7 +105,7 @@ func (p *provider) UpdateProfile(ctx context.Context, meta RequestMetadata, para // enforce check below). Uses the same availability rule as login-time gating. if refs.BoolValue(params.IsMultiFactorAuthEnabled) && !p.isMFAServiceAvailable() { log.Debug().Msg("Cannot enable mfa as no mfa method is available") - return nil, nil, FailedPrecondition("cannot enable MFA: enable it on the server with --enable-mfa plus at least one method — --enable-totp-login, --enable-email-otp (requires SMTP configured), or --enable-sms-otp (requires Twilio configured)") + return nil, nil, FailedPrecondition("cannot enable MFA: no MFA method is available on this server — ensure TOTP is enabled (do not set --disable-totp-login) or configure an email (SMTP) or SMS (Twilio) provider for OTP") } isMFAEnforced := p.Config.EnforceMFA diff --git a/web/dashboard/src/pages/Users.tsx b/web/dashboard/src/pages/Users.tsx index f5fc4714..41f98930 100644 --- a/web/dashboard/src/pages/Users.tsx +++ b/web/dashboard/src/pages/Users.tsx @@ -478,7 +478,7 @@ export default function Users() { ) : ( Enable MFA (unavailable)