diff --git a/api4/user.go b/api4/user.go index 735570a964..a98c7484ea 100644 --- a/api4/user.go +++ b/api4/user.go @@ -874,7 +874,7 @@ func checkUserMfa(c *Context, w http.ResponseWriter, r *http.Request) { resp := map[string]interface{}{} resp["mfa_required"] = false - if license := c.App.License(); license == nil || !*license.Features.MFA || !*c.App.Config().ServiceSettings.EnableMultifactorAuthentication { + if !*c.App.Config().ServiceSettings.EnableMultifactorAuthentication { w.Write([]byte(model.StringInterfaceToJson(resp))) return } diff --git a/app/app.go b/app/app.go index e96f98e031..2c527ed3a6 100644 --- a/app/app.go +++ b/app/app.go @@ -39,7 +39,6 @@ type App struct { Ldap einterfaces.LdapInterface MessageExport einterfaces.MessageExportInterface Metrics einterfaces.MetricsInterface - Mfa einterfaces.MfaInterface Saml einterfaces.SamlInterface HTTPService httpservice.HTTPService diff --git a/app/authentication.go b/app/authentication.go index 566eac8042..ccc14efbe2 100644 --- a/app/authentication.go +++ b/app/authentication.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/services/mfa" "github.com/mattermost/mattermost-server/utils" ) @@ -145,15 +146,12 @@ func (a *App) CheckUserPostflightAuthenticationCriteria(user *model.User) *model } func (a *App) CheckUserMfa(user *model.User, token string) *model.AppError { - if license := a.License(); !user.MfaActive || license == nil || !*license.Features.MFA || !*a.Config().ServiceSettings.EnableMultifactorAuthentication { + if !user.MfaActive || !*a.Config().ServiceSettings.EnableMultifactorAuthentication { return nil } - if a.Mfa == nil { - return model.NewAppError("checkUserMfa", "api.user.check_user_mfa.not_available.app_error", nil, "", http.StatusNotImplemented) - } - - ok, err := a.Mfa.ValidateToken(user.MfaSecret, token) + mfaService := mfa.New(a, a.Srv.Store) + ok, err := mfaService.ValidateToken(user.MfaSecret, token) if err != nil { return err } diff --git a/app/enterprise.go b/app/enterprise.go index 46c00f9c56..4b18e22780 100644 --- a/app/enterprise.go +++ b/app/enterprise.go @@ -95,12 +95,6 @@ func RegisterMetricsInterface(f func(*App) einterfaces.MetricsInterface) { metricsInterface = f } -var mfaInterface func(*App) einterfaces.MfaInterface - -func RegisterMfaInterface(f func(*App) einterfaces.MfaInterface) { - mfaInterface = f -} - var samlInterface func(*App) einterfaces.SamlInterface func RegisterSamlInterface(f func(*App) einterfaces.SamlInterface) { @@ -131,9 +125,6 @@ func (s *Server) initEnterprise() { if metricsInterface != nil { s.Metrics = metricsInterface(s.FakeApp()) } - if mfaInterface != nil { - s.Mfa = mfaInterface(s.FakeApp()) - } if samlInterface != nil { s.Saml = samlInterface(s.FakeApp()) s.AddConfigListener(func(_, cfg *model.Config) { diff --git a/app/options.go b/app/options.go index 2e1ad08df4..3abf133c1f 100644 --- a/app/options.go +++ b/app/options.go @@ -58,7 +58,6 @@ func ServerConnector(s *Server) AppOption { a.Ldap = s.Ldap a.MessageExport = s.MessageExport a.Metrics = s.Metrics - a.Mfa = s.Mfa a.Saml = s.Saml } } diff --git a/app/server.go b/app/server.go index 4bd03b099d..b8d660bdf0 100644 --- a/app/server.go +++ b/app/server.go @@ -121,7 +121,6 @@ type Server struct { Ldap einterfaces.LdapInterface MessageExport einterfaces.MessageExportInterface Metrics einterfaces.MetricsInterface - Mfa einterfaces.MfaInterface Saml einterfaces.SamlInterface } diff --git a/app/user.go b/app/user.go index 925fccd4d1..d0e7b56935 100644 --- a/app/user.go +++ b/app/user.go @@ -28,6 +28,7 @@ import ( "github.com/mattermost/mattermost-server/einterfaces" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/services/mfa" "github.com/mattermost/mattermost-server/utils" ) @@ -589,16 +590,13 @@ func (a *App) sanitizeProfiles(users []*model.User, asAdmin bool) []*model.User } func (a *App) GenerateMfaSecret(userId string) (*model.MfaSecret, *model.AppError) { - if a.Mfa == nil { - return nil, model.NewAppError("generateMfaSecret", "api.user.generate_mfa_qr.not_available.app_error", nil, "", http.StatusNotImplemented) - } - user, err := a.GetUser(userId) if err != nil { return nil, err } - secret, img, err := a.Mfa.GenerateSecret(user) + mfaService := mfa.New(a, a.Srv.Store) + secret, img, err := mfaService.GenerateSecret(user) if err != nil { return nil, err } @@ -608,11 +606,6 @@ func (a *App) GenerateMfaSecret(userId string) (*model.MfaSecret, *model.AppErro } func (a *App) ActivateMfa(userId, token string) *model.AppError { - if a.Mfa == nil { - err := model.NewAppError("ActivateMfa", "api.user.update_mfa.not_available.app_error", nil, "", http.StatusNotImplemented) - return err - } - result := <-a.Srv.Store.User().Get(userId) if result.Err != nil { return result.Err @@ -623,7 +616,8 @@ func (a *App) ActivateMfa(userId, token string) *model.AppError { return model.NewAppError("ActivateMfa", "api.user.activate_mfa.email_and_ldap_only.app_error", nil, "", http.StatusBadRequest) } - if err := a.Mfa.Activate(user, token); err != nil { + mfaService := mfa.New(a, a.Srv.Store) + if err := mfaService.Activate(user, token); err != nil { return err } @@ -631,12 +625,8 @@ func (a *App) ActivateMfa(userId, token string) *model.AppError { } func (a *App) DeactivateMfa(userId string) *model.AppError { - if a.Mfa == nil { - err := model.NewAppError("DeactivateMfa", "api.user.update_mfa.not_available.app_error", nil, "", http.StatusNotImplemented) - return err - } - - if err := a.Mfa.Deactivate(userId); err != nil { + mfaService := mfa.New(a, a.Srv.Store) + if err := mfaService.Deactivate(userId); err != nil { return err } diff --git a/i18n/en.json b/i18n/en.json index c45040557a..e652a8dc2d 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -2118,10 +2118,6 @@ "id": "api.user.check_user_mfa.bad_code.app_error", "translation": "Invalid MFA token." }, - { - "id": "api.user.check_user_mfa.not_available.app_error", - "translation": "MFA is not configured or supported on this server" - }, { "id": "api.user.check_user_password.invalid.app_error", "translation": "Login failed because of invalid password" @@ -2202,10 +2198,6 @@ "id": "api.user.email_to_oauth.not_available.app_error", "translation": "Authentication Transfer not configured or available on this server." }, - { - "id": "api.user.generate_mfa_qr.not_available.app_error", - "translation": "MFA not configured or available on this server" - }, { "id": "api.user.get_authorization_code.unsupported.app_error", "translation": "Unsupported OAuth service provider" @@ -2366,10 +2358,6 @@ "id": "api.user.update_active.permissions.app_error", "translation": "You do not have the appropriate permissions" }, - { - "id": "api.user.update_mfa.not_available.app_error", - "translation": "MFA not configured or available on this server" - }, { "id": "api.user.update_oauth_user_attrs.get_user.app_error", "translation": "Could not get user from {{.Service}} user object" @@ -3574,42 +3562,6 @@ "id": "ent.message_export.global_relay_export.deliver.unable_to_open_zip_file_data.app_error", "translation": "Unable to open the export temporary file" }, - { - "id": "ent.mfa.activate.authenticate.app_error", - "translation": "Error attempting to authenticate MFA token" - }, - { - "id": "ent.mfa.activate.bad_token.app_error", - "translation": "Invalid MFA token" - }, - { - "id": "ent.mfa.activate.save_active.app_error", - "translation": "Unable to update MFA active status for the user" - }, - { - "id": "ent.mfa.deactivate.save_active.app_error", - "translation": "Unable to update MFA active status for the user" - }, - { - "id": "ent.mfa.deactivate.save_secret.app_error", - "translation": "Error clearing the MFA secret" - }, - { - "id": "ent.mfa.generate_qr_code.create_code.app_error", - "translation": "Error generating QR code" - }, - { - "id": "ent.mfa.generate_qr_code.save_secret.app_error", - "translation": "Error saving the MFA secret" - }, - { - "id": "ent.mfa.license_disable.app_error", - "translation": "Your license does not support using multi-factor authentication" - }, - { - "id": "ent.mfa.validate_token.authenticate.app_error", - "translation": "Error trying to authenticate MFA token" - }, { "id": "ent.migration.migratetoldap.duplicate_field", "translation": "Unable to migrate AD/LDAP users with specified field. Duplicate entry detected. Please remove all duplcates and try again." @@ -3722,6 +3674,42 @@ "id": "mattermost.bulletin.subject", "translation": "Mattermost Security Bulletin" }, + { + "id": "mfa.activate.authenticate.app_error", + "translation": "Error attempting to authenticate MFA token" + }, + { + "id": "mfa.activate.bad_token.app_error", + "translation": "Invalid MFA token" + }, + { + "id": "mfa.activate.save_active.app_error", + "translation": "Unable to update MFA active status for the user" + }, + { + "id": "mfa.deactivate.save_active.app_error", + "translation": "Unable to update MFA active status for the user" + }, + { + "id": "mfa.deactivate.save_secret.app_error", + "translation": "Error clearing the MFA secret" + }, + { + "id": "mfa.generate_qr_code.create_code.app_error", + "translation": "Error generating QR code" + }, + { + "id": "mfa.generate_qr_code.save_secret.app_error", + "translation": "Error saving the MFA secret" + }, + { + "id": "mfa.mfa_disabled.app_error", + "translation": "Multi-factor authentication has been disabled on this server." + }, + { + "id": "mfa.validate_token.authenticate.app_error", + "translation": "Error trying to authenticate MFA token" + }, { "id": "migrations.worker.run_advanced_permissions_phase_2_migration.invalid_progress", "translation": "Migration failed due to invalid progress data." diff --git a/services/mfa/mfa.go b/services/mfa/mfa.go new file mode 100644 index 0000000000..e404f96e71 --- /dev/null +++ b/services/mfa/mfa.go @@ -0,0 +1,146 @@ +// Copyright (c) 2016 Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package mfa + +import ( + b32 "encoding/base32" + "fmt" + "net/http" + "net/url" + "strings" + + "github.com/dgryski/dgoogauth" + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/services/configservice" + "github.com/mattermost/mattermost-server/store" + "github.com/mattermost/rsc/qr" +) + +const ( + MFA_SECRET_SIZE = 20 +) + +type Mfa struct { + ConfigService configservice.ConfigService + Store store.Store +} + +func New(configService configservice.ConfigService, store store.Store) Mfa { + return Mfa{configService, store} +} + +func (m *Mfa) checkConfig() *model.AppError { + if !*m.ConfigService.Config().ServiceSettings.EnableMultifactorAuthentication { + return model.NewAppError("checkConfig", "mfa.mfa_disabled.app_error", nil, "", http.StatusNotImplemented) + } + + return nil +} + +func getIssuerFromUrl(uri string) string { + issuer := "Mattermost" + siteUrl := strings.TrimSpace(uri) + + if len(siteUrl) > 0 { + siteUrl = strings.TrimPrefix(siteUrl, "https://") + siteUrl = strings.TrimPrefix(siteUrl, "http://") + issuer = strings.TrimPrefix(siteUrl, "www.") + } + + return url.QueryEscape(issuer) +} + +func (m *Mfa) GenerateSecret(user *model.User) (string, []byte, *model.AppError) { + if err := m.checkConfig(); err != nil { + return "", nil, err + } + + issuer := getIssuerFromUrl(*m.ConfigService.Config().ServiceSettings.SiteURL) + + secret := b32.StdEncoding.EncodeToString([]byte(model.NewRandomString(MFA_SECRET_SIZE))) + + authLink := fmt.Sprintf("otpauth://totp/%s:%s?secret=%s&issuer=%s", issuer, user.Email, secret, issuer) + + code, err := qr.Encode(authLink, qr.H) + + if err != nil { + return "", nil, model.NewAppError("GenerateQrCode", "mfa.generate_qr_code.create_code.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + img := code.PNG() + + if result := <-m.Store.User().UpdateMfaSecret(user.Id, secret); result.Err != nil { + return "", nil, model.NewAppError("GenerateQrCode", "mfa.generate_qr_code.save_secret.app_error", nil, result.Err.Error(), http.StatusInternalServerError) + } + + return secret, img, nil +} + +func (m *Mfa) Activate(user *model.User, token string) *model.AppError { + if err := m.checkConfig(); err != nil { + return err + } + + otpConfig := &dgoogauth.OTPConfig{ + Secret: user.MfaSecret, + WindowSize: 3, + HotpCounter: 0, + } + + trimmedToken := strings.TrimSpace(token) + + ok, err := otpConfig.Authenticate(trimmedToken) + if err != nil { + return model.NewAppError("Activate", "mfa.activate.authenticate.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + if !ok { + return model.NewAppError("Activate", "mfa.activate.bad_token.app_error", nil, "", http.StatusUnauthorized) + } + + if result := <-m.Store.User().UpdateMfaActive(user.Id, true); result.Err != nil { + return model.NewAppError("Activate", "mfa.activate.save_active.app_error", nil, result.Err.Error(), http.StatusInternalServerError) + } + + return nil +} + +func (m *Mfa) Deactivate(userId string) *model.AppError { + if err := m.checkConfig(); err != nil { + return err + } + + achan := m.Store.User().UpdateMfaActive(userId, false) + schan := m.Store.User().UpdateMfaSecret(userId, "") + + if result := <-achan; result.Err != nil { + return model.NewAppError("Deactivate", "mfa.deactivate.save_active.app_error", nil, result.Err.Error(), http.StatusInternalServerError) + } + + if result := <-schan; result.Err != nil { + return model.NewAppError("Deactivate", "mfa.deactivate.save_secret.app_error", nil, result.Err.Error(), http.StatusInternalServerError) + } + + return nil +} + +func (m *Mfa) ValidateToken(secret, token string) (bool, *model.AppError) { + if err := m.checkConfig(); err != nil { + return false, err + } + + otpConfig := &dgoogauth.OTPConfig{ + Secret: secret, + WindowSize: 3, + HotpCounter: 0, + } + + trimmedToken := strings.TrimSpace(token) + ok, err := otpConfig.Authenticate(trimmedToken) + if err != nil { + return false, model.NewAppError("ValidateToken", "mfa.validate_token.authenticate.app_error", nil, err.Error(), http.StatusBadRequest) + } + + return ok, nil +} diff --git a/services/mfa/mfa_test.go b/services/mfa/mfa_test.go new file mode 100644 index 0000000000..34b7bbc713 --- /dev/null +++ b/services/mfa/mfa_test.go @@ -0,0 +1,73 @@ +// Copyright (c) 2015 Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package mfa + +import ( + "net/url" + "testing" + + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/plugin/plugintest/mock" + "github.com/mattermost/mattermost-server/store" + "github.com/mattermost/mattermost-server/store/storetest/mocks" + "github.com/mattermost/mattermost-server/utils/testutils" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGenerateSecret(t *testing.T) { + user := &model.User{Id: model.NewId(), Roles: "system_user"} + + config := model.Config{} + config.SetDefaults() + config.ServiceSettings.EnableMultifactorAuthentication = model.NewBool(true) + configService := testutils.StaticConfigService{Cfg: &config} + storeMock := mocks.Store{} + userStoreMock := mocks.UserStore{} + userStoreMock.On("UpdateMfaSecret", user.Id, mock.AnythingOfType("string")).Return(func(userId string, secret string) store.StoreChannel { + return store.Do(func(result *store.StoreResult) { + result.Data = nil + result.Err = nil + }) + }) + storeMock.On("User").Return(&userStoreMock) + + mfa := Mfa{configService, &storeMock} + + secret, img, err := mfa.GenerateSecret(user) + require.Nil(t, err) + + assert.Len(t, secret, 32) + + if len(img) == 0 { + t.Fatal("no image set") + } + + config.ServiceSettings.EnableMultifactorAuthentication = model.NewBool(false) + + _, _, err = mfa.GenerateSecret(user) + require.NotNil(t, err) +} + +func TestGetIssuerFromUrl(t *testing.T) { + cases := []struct { + Input string + Expected string + }{ + {"http://somewebsite.com", url.QueryEscape("somewebsite.com")}, + {"https://somewebsite.com", url.QueryEscape("somewebsite.com")}, + {"https://some.website.com", url.QueryEscape("some.website.com")}, + {" https://www.somewebsite.com", url.QueryEscape("somewebsite.com")}, + {"http://somewebsite.com/chat", url.QueryEscape("somewebsite.com/chat")}, + {"somewebsite.com ", url.QueryEscape("somewebsite.com")}, + {"http://localhost:8065", url.QueryEscape("localhost:8065")}, + {"", "Mattermost"}, + {" ", "Mattermost"}, + } + + for _, c := range cases { + assert.Equal(t, c.Expected, getIssuerFromUrl(c.Input)) + } +} diff --git a/utils/config.go b/utils/config.go index 7b750753f1..7fc644155f 100644 --- a/utils/config.go +++ b/utils/config.go @@ -771,6 +771,7 @@ func GenerateLimitedClientConfig(c *model.Config, diagnosticId string, license * props["EnableCustomBrand"] = strconv.FormatBool(*c.TeamSettings.EnableCustomBrand) props["CustomBrandText"] = *c.TeamSettings.CustomBrandText props["CustomDescriptionText"] = *c.TeamSettings.CustomDescriptionText + props["EnableMultifactorAuthentication"] = strconv.FormatBool(*c.ServiceSettings.EnableMultifactorAuthentication) if license != nil { if *license.Features.LDAP { @@ -781,10 +782,6 @@ func GenerateLimitedClientConfig(c *model.Config, diagnosticId string, license * props["LdapLoginButtonTextColor"] = *c.LdapSettings.LoginButtonTextColor } - if *license.Features.MFA { - props["EnableMultifactorAuthentication"] = strconv.FormatBool(*c.ServiceSettings.EnableMultifactorAuthentication) - } - if *license.Features.SAML { props["EnableSaml"] = strconv.FormatBool(*c.SamlSettings.Enable) props["SamlLoginButtonText"] = *c.SamlSettings.LoginButtonText