diff --git a/api4/emoji.go b/api4/emoji.go index 0da0b3951e..84c70be62f 100644 --- a/api4/emoji.go +++ b/api4/emoji.go @@ -37,11 +37,6 @@ func createEmoji(c *Context, w http.ResponseWriter, r *http.Request) { return } - if len(*c.App.Config().FileSettings.DriverName) == 0 { - c.Err = model.NewAppError("createEmoji", "api.emoji.storage.app_error", nil, "", http.StatusNotImplemented) - return - } - if r.ContentLength > app.MaxEmojiFileSize { c.Err = model.NewAppError("createEmoji", "api.emoji.create.too_large.app_error", nil, "", http.StatusRequestEntityTooLarge) return @@ -224,11 +219,6 @@ func getEmojiImage(c *Context, w http.ResponseWriter, r *http.Request) { return } - if len(*c.App.Config().FileSettings.DriverName) == 0 { - c.Err = model.NewAppError("getEmojiImage", "api.emoji.storage.app_error", nil, "", http.StatusNotImplemented) - return - } - image, imageType, err := c.App.GetEmojiImage(c.Params.EmojiId) if err != nil { c.Err = err diff --git a/api4/emoji_test.go b/api4/emoji_test.go index 6f05994a41..30b0d8f2f1 100644 --- a/api4/emoji_test.go +++ b/api4/emoji_test.go @@ -486,12 +486,6 @@ func TestGetEmojiImage(t *testing.T) { defer th.TearDown() Client := th.Client - EnableCustomEmoji := *th.App.Config().ServiceSettings.EnableCustomEmoji - DriverName := *th.App.Config().FileSettings.DriverName - defer func() { - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCustomEmoji = EnableCustomEmoji }) - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.DriverName = DriverName }) - }() th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCustomEmoji = true }) emoji1 := &model.Emoji{ @@ -508,14 +502,8 @@ func TestGetEmojiImage(t *testing.T) { CheckNotImplementedStatus(t, resp) CheckErrorMessage(t, resp, "api.emoji.disabled.app_error") - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.DriverName = "" }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCustomEmoji = true }) - - _, resp = Client.GetEmojiImage(emoji1.Id) - CheckNotImplementedStatus(t, resp) - CheckErrorMessage(t, resp, "api.emoji.storage.app_error") - - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.DriverName = DriverName }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.DriverName = "local" }) emojiImage, resp := Client.GetEmojiImage(emoji1.Id) CheckNoError(t, resp) diff --git a/api4/file_test.go b/api4/file_test.go index 01ff53dabc..00b78b39c5 100644 --- a/api4/file_test.go +++ b/api4/file_test.go @@ -928,14 +928,8 @@ func TestGetFileLink(t *testing.T) { t.Skip("skipping because no file driver is enabled") } - enablePublicLink := th.App.Config().FileSettings.EnablePublicLink - publicLinkSalt := *th.App.Config().FileSettings.PublicLinkSalt - defer func() { - th.App.UpdateConfig(func(cfg *model.Config) { cfg.FileSettings.EnablePublicLink = enablePublicLink }) - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.PublicLinkSalt = publicLinkSalt }) - }() th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnablePublicLink = true }) - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.PublicLinkSalt = model.NewId() }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.PublicLinkSalt = model.NewRandomString(32) }) fileId := "" if data, err := testutils.ReadTestFile("test.png"); err != nil { @@ -1119,18 +1113,8 @@ func TestGetPublicFile(t *testing.T) { Client := th.Client channel := th.BasicChannel - if *th.App.Config().FileSettings.DriverName == "" { - t.Skip("skipping because no file driver is enabled") - } - - enablePublicLink := th.App.Config().FileSettings.EnablePublicLink - publicLinkSalt := *th.App.Config().FileSettings.PublicLinkSalt - defer func() { - th.App.UpdateConfig(func(cfg *model.Config) { cfg.FileSettings.EnablePublicLink = enablePublicLink }) - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.PublicLinkSalt = publicLinkSalt }) - }() th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnablePublicLink = true }) - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.PublicLinkSalt = GenerateTestId() }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.PublicLinkSalt = model.NewRandomString(32) }) fileId := "" if data, err := testutils.ReadTestFile("test.png"); err != nil { @@ -1168,7 +1152,7 @@ func TestGetPublicFile(t *testing.T) { // test after the salt has changed th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnablePublicLink = true }) - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.PublicLinkSalt = GenerateTestId() }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.PublicLinkSalt = model.NewRandomString(32) }) if resp, err := http.Get(link); err == nil && resp.StatusCode != http.StatusBadRequest { t.Fatal("should've failed to get image with public link after salt changed") diff --git a/app/admin.go b/app/admin.go index 7e2a23332a..695f004819 100644 --- a/app/admin.go +++ b/app/admin.go @@ -12,7 +12,7 @@ import ( "net/http" - "github.com/mattermost/mattermost-server/einterfaces" + "github.com/mattermost/mattermost-server/config" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/services/mailservice" @@ -160,40 +160,15 @@ func (a *App) GetEnvironmentConfig() map[string]interface{} { return a.EnvironmentConfig() } -func validateLdapFilter(cfg *model.Config, ldap einterfaces.LdapInterface) *model.AppError { - if !*cfg.LdapSettings.Enable || ldap == nil || *cfg.LdapSettings.UserFilter == "" { - return nil +func (a *App) SaveConfig(newCfg *model.Config, sendConfigChangeClusterMessage bool) *model.AppError { + oldCfg, err := a.Srv.configStore.Set(newCfg) + if err == config.ErrReadOnlyConfiguration { + return model.NewAppError("saveConfig", "ent.cluster.save_config.error", nil, err.Error(), http.StatusForbidden) + } else if err != nil { + return model.NewAppError("saveConfig", "app.save_config.app_error", nil, err.Error(), http.StatusInternalServerError) } - return ldap.ValidateFilter(*cfg.LdapSettings.UserFilter) -} - -func (a *App) SaveConfig(cfg *model.Config, sendConfigChangeClusterMessage bool) *model.AppError { - oldCfg := a.Config() - cfg.SetDefaults() - a.Desanitize(cfg) - - if err := cfg.IsValid(); err != nil { - return err - } - - if err := validateLdapFilter(cfg, a.Ldap); err != nil { - return err - } - - if *a.Config().ClusterSettings.Enable && *a.Config().ClusterSettings.ReadOnlyConfig { - return model.NewAppError("saveConfig", "ent.cluster.save_config.error", nil, "", http.StatusForbidden) - } - - a.DisableConfigWatch() - - a.UpdateConfig(func(update *model.Config) { - *update = *cfg - }) a.PersistConfig() - a.ReloadConfig() - a.EnableConfigWatch() - if a.Metrics != nil { if *a.Config().MetricsSettings.Enable { a.Metrics.StartServer() @@ -203,7 +178,7 @@ func (a *App) SaveConfig(cfg *model.Config, sendConfigChangeClusterMessage bool) } if a.Cluster != nil { - err := a.Cluster.ConfigChanged(cfg, oldCfg, sendConfigChangeClusterMessage) + err := a.Cluster.ConfigChanged(newCfg, oldCfg, sendConfigChangeClusterMessage) if err != nil { return err } diff --git a/app/config.go b/app/config.go index e024548bb5..f671b99a07 100644 --- a/app/config.go +++ b/app/config.go @@ -15,7 +15,6 @@ import ( "net/url" "runtime/debug" "strconv" - "strings" "time" "github.com/mattermost/mattermost-server/config" @@ -29,10 +28,7 @@ const ( ) func (s *Server) Config() *model.Config { - if cfg := s.config.Load(); cfg != nil { - return cfg.(*model.Config) - } - return &model.Config{} + return s.configStore.Get() } func (a *App) Config() *model.Config { @@ -40,10 +36,7 @@ func (a *App) Config() *model.Config { } func (s *Server) EnvironmentConfig() map[string]interface{} { - if s.envConfig != nil { - return s.envConfig - } - return map[string]interface{}{} + return s.configStore.GetEnvironmentOverrides() } func (a *App) EnvironmentConfig() map[string]interface{} { @@ -54,9 +47,9 @@ func (s *Server) UpdateConfig(f func(*model.Config)) { old := s.Config() updated := old.Clone() f(updated) - s.config.Store(updated) - - s.InvokeConfigListeners(old, updated) + if _, err := s.configStore.Set(updated); err != nil { + mlog.Error("Failed to update config", mlog.Err(err)) + } } func (a *App) UpdateConfig(f func(*model.Config)) { @@ -64,46 +57,23 @@ func (a *App) UpdateConfig(f func(*model.Config)) { } func (a *App) PersistConfig() { - config.SaveConfig(a.ConfigFileName(), a.Config()) -} - -func (s *Server) LoadConfig(configFile string) *model.AppError { - old := s.Config() - - cfg, configPath, envConfig, err := config.LoadConfig(configFile) - if err != nil { - return err + if err := a.Srv.configStore.Save(); err != nil { + mlog.Error("Failed to persist config", mlog.Err(err)) } - *cfg.ServiceSettings.SiteURL = strings.TrimRight(*cfg.ServiceSettings.SiteURL, "/") - s.config.Store(cfg) - - s.configFile = configPath - s.envConfig = envConfig - - s.InvokeConfigListeners(old, cfg) - return nil } -func (a *App) LoadConfig(configFile string) *model.AppError { - return a.Srv.LoadConfig(configFile) -} - -func (s *Server) ReloadConfig() *model.AppError { +func (s *Server) ReloadConfig() error { debug.FreeOSMemory() - if err := s.LoadConfig(s.configFile); err != nil { + if err := s.configStore.Load(); err != nil { return err } return nil } -func (a *App) ReloadConfig() *model.AppError { +func (a *App) ReloadConfig() error { return a.Srv.ReloadConfig() } -func (a *App) ConfigFileName() string { - return a.Srv.configFile -} - func (a *App) ClientConfig() map[string]string { return a.Srv.clientConfig } @@ -116,40 +86,11 @@ func (a *App) LimitedClientConfig() map[string]string { return a.Srv.limitedClientConfig } -func (s *Server) EnableConfigWatch() { - if s.configWatcher == nil && !s.disableConfigWatch { - configWatcher, err := config.NewConfigWatcher(s.configFile, func() { - s.ReloadConfig() - }) - if err != nil { - mlog.Error(fmt.Sprint(err)) - } - s.configWatcher = configWatcher - } -} - -func (a *App) EnableConfigWatch() { - a.Srv.EnableConfigWatch() -} - -func (s *Server) DisableConfigWatch() { - if s.configWatcher != nil { - s.configWatcher.Close() - s.configWatcher = nil - } -} - -func (a *App) DisableConfigWatch() { - a.Srv.DisableConfigWatch() -} - // Registers a function with a given to be called when the config is reloaded and may have changed. The function // will be called with two arguments: the old config and the new config. AddConfigListener returns a unique ID // for the listener that can later be used to remove it. func (s *Server) AddConfigListener(listener func(*model.Config, *model.Config)) string { - id := model.NewId() - s.configListeners[id] = listener - return id + return s.configStore.AddListener(listener) } func (a *App) AddConfigListener(listener func(*model.Config, *model.Config)) string { @@ -158,19 +99,13 @@ func (a *App) AddConfigListener(listener func(*model.Config, *model.Config)) str // Removes a listener function by the unique ID returned when AddConfigListener was called func (s *Server) RemoveConfigListener(id string) { - delete(s.configListeners, id) + s.configStore.RemoveListener(id) } func (a *App) RemoveConfigListener(id string) { a.Srv.RemoveConfigListener(id) } -func (s *Server) InvokeConfigListeners(old, current *model.Config) { - for _, listener := range s.configListeners { - listener(old, current) - } -} - // EnsureAsymmetricSigningKey ensures that an asymmetric signing key exists and future calls to // AsymmetricSigningKey will always return a valid signing key. func (a *App) ensureAsymmetricSigningKey() error { @@ -306,51 +241,6 @@ func (a *App) regenerateClientConfig() { a.Srv.clientConfigHash = fmt.Sprintf("%x", md5.Sum(clientConfigJSON)) } -func (a *App) Desanitize(cfg *model.Config) { - actual := a.Config() - - if cfg.LdapSettings.BindPassword != nil && *cfg.LdapSettings.BindPassword == model.FAKE_SETTING { - *cfg.LdapSettings.BindPassword = *actual.LdapSettings.BindPassword - } - - if *cfg.FileSettings.PublicLinkSalt == model.FAKE_SETTING { - *cfg.FileSettings.PublicLinkSalt = *actual.FileSettings.PublicLinkSalt - } - if *cfg.FileSettings.AmazonS3SecretAccessKey == model.FAKE_SETTING { - cfg.FileSettings.AmazonS3SecretAccessKey = actual.FileSettings.AmazonS3SecretAccessKey - } - - if *cfg.EmailSettings.InviteSalt == model.FAKE_SETTING { - cfg.EmailSettings.InviteSalt = actual.EmailSettings.InviteSalt - } - if *cfg.EmailSettings.SMTPPassword == model.FAKE_SETTING { - cfg.EmailSettings.SMTPPassword = actual.EmailSettings.SMTPPassword - } - - if *cfg.GitLabSettings.Secret == model.FAKE_SETTING { - *cfg.GitLabSettings.Secret = *actual.GitLabSettings.Secret - } - - if *cfg.SqlSettings.DataSource == model.FAKE_SETTING { - *cfg.SqlSettings.DataSource = *actual.SqlSettings.DataSource - } - if *cfg.SqlSettings.AtRestEncryptKey == model.FAKE_SETTING { - cfg.SqlSettings.AtRestEncryptKey = actual.SqlSettings.AtRestEncryptKey - } - - if *cfg.ElasticsearchSettings.Password == model.FAKE_SETTING { - *cfg.ElasticsearchSettings.Password = *actual.ElasticsearchSettings.Password - } - - for i := range cfg.SqlSettings.DataSourceReplicas { - cfg.SqlSettings.DataSourceReplicas[i] = actual.SqlSettings.DataSourceReplicas[i] - } - - for i := range cfg.SqlSettings.DataSourceSearchReplicas { - cfg.SqlSettings.DataSourceSearchReplicas[i] = actual.SqlSettings.DataSourceSearchReplicas[i] - } -} - func (a *App) GetCookieDomain() string { if *a.Config().ServiceSettings.AllowCookiesForSubdomains { if siteURL, err := url.Parse(*a.Config().ServiceSettings.SiteURL); err == nil { diff --git a/app/config_test.go b/app/config_test.go index 682a146359..97084331f5 100644 --- a/app/config_test.go +++ b/app/config_test.go @@ -4,67 +4,29 @@ package app import ( - "io/ioutil" "strconv" - "strings" "testing" "time" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/store/sqlstore" "github.com/mattermost/mattermost-server/utils" - "github.com/mattermost/mattermost-server/utils/fileutils" ) -func TestLoadConfig(t *testing.T) { - tempConfig, err := ioutil.TempFile("", "") - require.Nil(t, err) - - input, err := ioutil.ReadFile(fileutils.FindConfigFile("config.json")) - require.Nil(t, err) - lines := strings.Split(string(input), "\n") - for i, line := range lines { - if strings.Contains(line, "SiteURL") { - lines[i] = ` "SiteURL": "http://localhost:8065/",` - } - } - output := strings.Join(lines, "\n") - err = ioutil.WriteFile(tempConfig.Name(), []byte(output), 0644) - require.Nil(t, err) - tempConfig.Close() - - a := App{ - Srv: &Server{}, - } - appErr := a.LoadConfig(tempConfig.Name()) - require.Nil(t, appErr) - - assert.Equal(t, "http://localhost:8065", *a.Config().ServiceSettings.SiteURL) -} - func TestConfigListener(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() originalSiteName := th.App.Config().TeamSettings.SiteName - th.App.UpdateConfig(func(cfg *model.Config) { - *cfg.TeamSettings.SiteName = "test123" - }) listenerCalled := false listener := func(oldConfig *model.Config, newConfig *model.Config) { - if listenerCalled { - t.Fatal("listener called twice") - } + assert.False(t, listenerCalled, "listener called twice") - if *oldConfig.TeamSettings.SiteName != "test123" { - t.Fatal("old config contains incorrect site name") - } else if *newConfig.TeamSettings.SiteName != *originalSiteName { - t.Fatal("new config contains incorrect site name") - } + assert.Equal(t, *originalSiteName, *oldConfig.TeamSettings.SiteName, "old config contains incorrect site name") + assert.Equal(t, "test123", *newConfig.TeamSettings.SiteName, "new config contains incorrect site name") listenerCalled = true } @@ -73,22 +35,19 @@ func TestConfigListener(t *testing.T) { listener2Called := false listener2 := func(oldConfig *model.Config, newConfig *model.Config) { - if listener2Called { - t.Fatal("listener2 called twice") - } + assert.False(t, listener2Called, "listener2 called twice") listener2Called = true } listener2Id := th.App.AddConfigListener(listener2) defer th.App.RemoveConfigListener(listener2Id) - th.App.ReloadConfig() + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.TeamSettings.SiteName = "test123" + }) - if !listenerCalled { - t.Fatal("listener should've been called") - } else if !listener2Called { - t.Fatal("listener 2 should've been called") - } + assert.True(t, listenerCalled, "listener should've been called") + assert.True(t, listener2Called, "listener 2 should've been called") } func TestAsymmetricSigningKey(t *testing.T) { diff --git a/app/emoji.go b/app/emoji.go index 666558b2a3..ac8d5a07a0 100644 --- a/app/emoji.go +++ b/app/emoji.go @@ -31,6 +31,14 @@ const ( ) func (a *App) CreateEmoji(sessionUserId string, emoji *model.Emoji, multiPartImageData *multipart.Form) (*model.Emoji, *model.AppError) { + if !*a.Config().ServiceSettings.EnableCustomEmoji { + return nil, model.NewAppError("UploadEmojiImage", "api.emoji.disabled.app_error", nil, "", http.StatusNotImplemented) + } + + if len(*a.Config().FileSettings.DriverName) == 0 { + return nil, model.NewAppError("GetEmoji", "api.emoji.storage.app_error", nil, "", http.StatusNotImplemented) + } + // wipe the emoji id so that existing emojis can't get overwritten emoji.Id = "" @@ -79,6 +87,14 @@ func (a *App) GetEmojiList(page, perPage int, sort string) ([]*model.Emoji, *mod } func (a *App) UploadEmojiImage(id string, imageData *multipart.FileHeader) *model.AppError { + if !*a.Config().ServiceSettings.EnableCustomEmoji { + return model.NewAppError("UploadEmojiImage", "api.emoji.disabled.app_error", nil, "", http.StatusNotImplemented) + } + + if len(*a.Config().FileSettings.DriverName) == 0 { + return model.NewAppError("UploadEmojiImage", "api.emoji.storage.app_error", nil, "", http.StatusNotImplemented) + } + file, err := imageData.Open() if err != nil { return model.NewAppError("uploadEmojiImage", "api.emoji.upload.open.app_error", nil, "", http.StatusBadRequest) diff --git a/app/enterprise.go b/app/enterprise.go index 47a88705f0..a0bfb8cd29 100644 --- a/app/enterprise.go +++ b/app/enterprise.go @@ -8,7 +8,6 @@ import ( ejobs "github.com/mattermost/mattermost-server/einterfaces/jobs" tjobs "github.com/mattermost/mattermost-server/jobs/interfaces" "github.com/mattermost/mattermost-server/model" - "github.com/mattermost/mattermost-server/utils" ) var accountMigrationInterface func(*App) einterfaces.AccountMigrationInterface @@ -119,11 +118,6 @@ func (s *Server) initEnterprise() { } if ldapInterface != nil { s.Ldap = ldapInterface(s.FakeApp()) - s.AddConfigListener(func(_, cfg *model.Config) { - if err := validateLdapFilter(cfg, s.Ldap); err != nil { - panic(utils.T(err.Id)) - } - }) } if messageExportInterface != nil { s.MessageExport = messageExportInterface(s.FakeApp()) diff --git a/app/options.go b/app/options.go index c6b56808a3..2fe02f6a4e 100644 --- a/app/options.go +++ b/app/options.go @@ -6,6 +6,7 @@ package app import ( "github.com/pkg/errors" + "github.com/mattermost/mattermost-server/config" "github.com/mattermost/mattermost-server/store" ) @@ -38,8 +39,19 @@ func StoreOverride(override interface{}) Option { func ConfigFile(file string, watch bool) Option { return func(s *Server) error { - s.configFile = file - s.disableConfigWatch = !watch + configStore, err := config.NewFileStore(file, watch) + if err != nil { + return errors.Wrap(err, "failed to apply ConfigFile option") + } + + s.configStore = configStore + return nil + } +} + +func ConfigStore(configStore config.Store) Option { + return func(s *Server) error { + s.configStore = configStore return nil } diff --git a/app/post_test.go b/app/post_test.go index 4585f030d1..a06dc00bde 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -7,7 +7,6 @@ import ( "fmt" "net/http" "sync" - "sync/atomic" "testing" "time" @@ -533,8 +532,7 @@ func TestMaxPostSize(t *testing.T) { app := App{ Srv: &Server{ - Store: mockStore, - config: atomic.Value{}, + Store: mockStore, }, } diff --git a/app/server.go b/app/server.go index 5ac6acb3cc..44281023c9 100644 --- a/app/server.go +++ b/app/server.go @@ -35,7 +35,6 @@ import ( "github.com/mattermost/mattermost-server/services/timezones" "github.com/mattermost/mattermost-server/store" "github.com/mattermost/mattermost-server/utils" - "github.com/mattermost/mattermost-server/utils/fileutils" ) var MaxNotificationsPerChannelDefault int64 = 1000000 @@ -75,10 +74,6 @@ type Server struct { runjobs bool Jobs *jobs.JobServer - config atomic.Value - envConfig map[string]interface{} - configFile string - configListeners map[string]func(*model.Config, *model.Config) clusterLeaderListeners sync.Map licenseValue atomic.Value @@ -96,8 +91,7 @@ type Server struct { licenseListenerId string logListenerId string clusterLeaderListenerId string - disableConfigWatch bool - configWatcher *config.ConfigWatcher + configStore config.Store asymmetricSigningKey *ecdsa.PrivateKey pluginCommands []*PluginCommand @@ -137,8 +131,6 @@ func NewServer(options ...Option) (*Server, error) { s := &Server{ goroutineExitSignal: make(chan struct{}, 1), RootRouter: rootRouter, - configFile: "config.json", - configListeners: make(map[string]func(*model.Config, *model.Config)), licenseListeners: map[string]func(){}, sessionCache: utils.NewLru(model.SESSION_CACHE_SIZE), seenPendingPostIdsCache: utils.NewLru(PENDING_POST_IDS_CACHE_SIZE), @@ -150,11 +142,14 @@ func NewServer(options ...Option) (*Server, error) { } } - if err := s.LoadConfig(s.configFile); err != nil { - return nil, err - } + if s.configStore == nil { + configStore, err := config.NewFileStore("config.json", true) + if err != nil { + return nil, errors.Wrap(err, "failed to load config") + } - s.EnableConfigWatch() + s.configStore = configStore + } // Initalize logging s.Log = mlog.NewLogger(utils.MloggerConfigFromLoggerConfig(&s.Config().LogSettings)) @@ -198,7 +193,7 @@ func NewServer(options ...Option) (*Server, error) { mlog.Info(fmt.Sprintf("Enterprise Enabled: %v", model.BuildEnterpriseReady)) pwd, _ := os.Getwd() mlog.Info(fmt.Sprintf("Current working directory is %v", pwd)) - mlog.Info(fmt.Sprintf("Loaded config file from %v", fileutils.FindConfigFile(s.configFile))) + mlog.Info("Loaded config", mlog.String("source", s.configStore.String())) license := s.License() @@ -323,7 +318,7 @@ func (s *Server) Shutdown() error { s.RemoveConfigListener(s.configListenerId) s.RemoveConfigListener(s.logListenerId) - s.DisableConfigWatch() + s.configStore.Close() if s.Cluster != nil { s.Cluster.StopInterNodeCommunication() diff --git a/app/server_test.go b/app/server_test.go index 8755b9ecb7..665fe66201 100644 --- a/app/server_test.go +++ b/app/server_test.go @@ -12,6 +12,7 @@ import ( "strings" "testing" + "github.com/mattermost/mattermost-server/config" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils/fileutils" "github.com/stretchr/testify/require" @@ -32,14 +33,14 @@ func TestStartServerSuccess(t *testing.T) { } func TestStartServerRateLimiterCriticalError(t *testing.T) { - s, err := NewServer() - require.NoError(t, err) - // Attempt to use Rate Limiter with an invalid config - s.UpdateConfig(func(cfg *model.Config) { - *cfg.RateLimitSettings.Enable = true - *cfg.RateLimitSettings.MaxBurst = -100 - }) + ms, err := config.NewMemoryStore(true) + require.NoError(t, err) + *ms.Config.RateLimitSettings.Enable = true + *ms.Config.RateLimitSettings.MaxBurst = -100 + + s, err := NewServer(ConfigStore(ms)) + require.NoError(t, err) serverErr := s.Start() s.Shutdown() diff --git a/cmd/mattermost/commands/config.go b/cmd/mattermost/commands/config.go index 5963ac87d3..7cba4fa6f1 100644 --- a/cmd/mattermost/commands/config.go +++ b/cmd/mattermost/commands/config.go @@ -167,24 +167,18 @@ func configShowCmdF(command *cobra.Command, args []string) error { } defer app.Shutdown() - // check that no arguments are given err = cobra.NoArgs(command, args) if err != nil { return err } - // set up the config object - config := app.Config() - - // pretty print - fmt.Printf("%s", prettyPrintStruct(*config)) + fmt.Printf("%s", prettyPrintStruct(*app.Config())) return nil } // printConfigValues function prints out the value of the configSettings working recursively or // gives an error if config setting is not in the file. func printConfigValues(configMap map[string]interface{}, configSetting []string, name string) (string, error) { - res, ok := configMap[configSetting[0]] if !ok { return "", fmt.Errorf("%s configuration setting is not in the file", name) @@ -217,37 +211,26 @@ func configSetCmdF(command *cobra.Command, args []string) error { configSetting := args[0] newVal := args[1:] - // Update the config - - // first disable the watchers - app.DisableConfigWatch() - // create the function to update config oldConfig := app.Config() newConfig := app.Config() f := updateConfigValue(configSetting, newVal, oldConfig, newConfig) - // update the config app.UpdateConfig(f) - - // Verify new config if err := newConfig.IsValid(); err != nil { return err } - if err := config.ValidateLocales(app.Config()); err != nil { + // UpdateConfig above would have already fixed these invalid locales, but we check again + // in the context of an explicit change to these parameters to avoid saving the fixed + // settings in the first place. + if changed := config.FixInvalidLocales(newConfig); changed { return errors.New("Invalid locale configuration") } // make the changes persist app.PersistConfig() - // reload config - app.ReloadConfig() - - // Enable config watchers - app.EnableConfigWatch() - return nil } diff --git a/cmd/mattermost/commands/plugin_test.go b/cmd/mattermost/commands/plugin_test.go index b69a37dc2b..70c127baeb 100644 --- a/cmd/mattermost/commands/plugin_test.go +++ b/cmd/mattermost/commands/plugin_test.go @@ -31,14 +31,16 @@ func TestPlugin(t *testing.T) { th.CheckCommand(t, "plugin", "add", filepath.Join(path, "testplugin.tar.gz")) th.CheckCommand(t, "plugin", "enable", "testplugin") - cfg, _, _, err := config.LoadConfig(th.ConfigPath()) + fs, err := config.NewFileStore(th.ConfigPath(), false) require.Nil(t, err) - assert.Equal(t, cfg.PluginSettings.PluginStates["testplugin"].Enable, true) + assert.True(t, fs.Get().PluginSettings.PluginStates["testplugin"].Enable) + fs.Close() th.CheckCommand(t, "plugin", "disable", "testplugin") - cfg, _, _, err = config.LoadConfig(th.ConfigPath()) + fs, err = config.NewFileStore(th.ConfigPath(), false) require.Nil(t, err) - assert.Equal(t, cfg.PluginSettings.PluginStates["testplugin"].Enable, false) + assert.False(t, fs.Get().PluginSettings.PluginStates["testplugin"].Enable) + fs.Close() th.CheckCommand(t, "plugin", "list") diff --git a/config/README.md b/config/README.md index 4fbc6b5d72..4896f72f84 100644 --- a/config/README.md +++ b/config/README.md @@ -1,3 +1,3 @@ -# CONFIG.JSON +# config.json This is the system configuration file for your Mattermost server. Settings are specific to different editions of Mattermost. Please read the documentation before making changes: https://about.mattermost.com/default-config-docs/ diff --git a/config/config.go b/config/client.go similarity index 55% rename from config/config.go rename to config/client.go index 826a69e1dc..ffdd502b3a 100644 --- a/config/config.go +++ b/config/client.go @@ -4,393 +4,14 @@ package config import ( - "bytes" - "encoding/json" "fmt" - "io" - "io/ioutil" - "os" - "path/filepath" - "reflect" "strconv" "strings" - "github.com/fsnotify/fsnotify" - "github.com/mattermost/viper" - "github.com/pkg/errors" - - "net/http" - - "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" - "github.com/mattermost/mattermost-server/utils" - "github.com/mattermost/mattermost-server/utils/fileutils" - "github.com/mattermost/mattermost-server/utils/jsonutils" ) -var ( - termsOfServiceEnabledAndEmpty = model.NewAppError( - "Config.IsValid", - "model.config.is_valid.support.custom_terms_of_service_text.app_error", - nil, - "", - http.StatusBadRequest, - ) -) - -func SaveConfig(fileName string, config *model.Config) *model.AppError { - b, err := json.MarshalIndent(config, "", " ") - if err != nil { - return model.NewAppError("SaveConfig", "utils.config.save_config.saving.app_error", - map[string]interface{}{"Filename": fileName}, err.Error(), http.StatusBadRequest) - } - - err = ioutil.WriteFile(fileName, b, 0644) - if err != nil { - return model.NewAppError("SaveConfig", "utils.config.save_config.saving.app_error", - map[string]interface{}{"Filename": fileName}, err.Error(), http.StatusInternalServerError) - } - - return nil -} - -type ConfigWatcher struct { - watcher *fsnotify.Watcher - close chan struct{} - closed chan struct{} -} - -func NewConfigWatcher(cfgFileName string, f func()) (*ConfigWatcher, error) { - watcher, err := fsnotify.NewWatcher() - if err != nil { - return nil, errors.Wrapf(err, "failed to create config watcher for file: "+cfgFileName) - } - - configFile := filepath.Clean(cfgFileName) - configDir, _ := filepath.Split(configFile) - watcher.Add(configDir) - - ret := &ConfigWatcher{ - watcher: watcher, - close: make(chan struct{}), - closed: make(chan struct{}), - } - - go func() { - defer close(ret.closed) - defer watcher.Close() - - for { - select { - case event := <-watcher.Events: - // we only care about the config file - if filepath.Clean(event.Name) == configFile { - if event.Op&fsnotify.Write == fsnotify.Write || event.Op&fsnotify.Create == fsnotify.Create { - mlog.Info(fmt.Sprintf("Config file watcher detected a change reloading %v", cfgFileName)) - - if _, _, configReadErr := ReadConfigFile(cfgFileName, true); configReadErr == nil { - f() - } else { - mlog.Error(fmt.Sprintf("Failed to read while watching config file at %v with err=%v", cfgFileName, configReadErr.Error())) - } - } - } - case err := <-watcher.Errors: - mlog.Error(fmt.Sprintf("Failed while watching config file at %v with err=%v", cfgFileName, err.Error())) - case <-ret.close: - return - } - } - }() - - return ret, nil -} - -func (w *ConfigWatcher) Close() { - close(w.close) - <-w.closed -} - -// ReadConfig reads and parses the given configuration. -func ReadConfig(r io.Reader, allowEnvironmentOverrides bool) (*model.Config, map[string]interface{}, error) { - // Pre-flight check the syntax of the configuration file to improve error messaging. - configData, err := ioutil.ReadAll(r) - if err != nil { - return nil, nil, err - } else { - var rawConfig interface{} - if err := json.Unmarshal(configData, &rawConfig); err != nil { - return nil, nil, jsonutils.HumanizeJsonError(err, configData) - } - } - - v := newViper(allowEnvironmentOverrides) - if err := v.ReadConfig(bytes.NewReader(configData)); err != nil { - return nil, nil, err - } - - var config model.Config - unmarshalErr := v.Unmarshal(&config) - // https://github.com/spf13/viper/issues/324 - // https://github.com/spf13/viper/issues/348 - if unmarshalErr == nil { - config.PluginSettings.Plugins = make(map[string]map[string]interface{}) - unmarshalErr = v.UnmarshalKey("pluginsettings.plugins", &config.PluginSettings.Plugins) - } - if unmarshalErr == nil { - config.PluginSettings.PluginStates = make(map[string]*model.PluginState) - unmarshalErr = v.UnmarshalKey("pluginsettings.pluginstates", &config.PluginSettings.PluginStates) - } - - envConfig := v.EnvSettings() - - var envErr error - if envConfig, envErr = fixEnvSettingsCase(envConfig); envErr != nil { - return nil, nil, envErr - } - - return &config, envConfig, unmarshalErr -} - -func newViper(allowEnvironmentOverrides bool) *viper.Viper { - v := viper.New() - - v.SetConfigType("json") - - if allowEnvironmentOverrides { - v.SetEnvPrefix("mm") - v.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) - v.AutomaticEnv() - } - - // Set zeroed defaults for all the config settings so that Viper knows what environment variables - // it needs to be looking for. The correct defaults will later be applied using Config.SetDefaults. - defaults := getDefaultsFromStruct(model.Config{}) - - for key, value := range defaults { - if key == "PluginSettings.Plugins" || key == "PluginSettings.PluginStates" { - continue - } - - v.SetDefault(key, value) - } - - return v -} - -func getDefaultsFromStruct(s interface{}) map[string]interface{} { - return flattenStructToMap(structToMap(reflect.TypeOf(s))) -} - -// Converts a struct type into a nested map with keys matching the struct's fields and values -// matching the zeroed value of the corresponding field. -func structToMap(t reflect.Type) (out map[string]interface{}) { - defer func() { - if r := recover(); r != nil { - mlog.Error(fmt.Sprintf("Panicked in structToMap. This should never happen. %v", r)) - } - }() - - if t.Kind() != reflect.Struct { - // Should never hit this, but this will prevent a panic if that does happen somehow - return nil - } - - out = map[string]interface{}{} - - for i := 0; i < t.NumField(); i++ { - field := t.Field(i) - - var value interface{} - - switch field.Type.Kind() { - case reflect.Struct: - value = structToMap(field.Type) - case reflect.Ptr: - indirectType := field.Type.Elem() - - if indirectType.Kind() == reflect.Struct { - // Follow pointers to structs since we need to define defaults for their fields - value = structToMap(indirectType) - } else { - value = nil - } - default: - value = reflect.Zero(field.Type).Interface() - } - - out[field.Name] = value - } - - return -} - -// Flattens a nested map so that the result is a single map with keys corresponding to the -// path through the original map. For example, -// { -// "a": { -// "b": 1 -// }, -// "c": "sea" -// } -// would flatten to -// { -// "a.b": 1, -// "c": "sea" -// } -func flattenStructToMap(in map[string]interface{}) map[string]interface{} { - out := make(map[string]interface{}) - - for key, value := range in { - if valueAsMap, ok := value.(map[string]interface{}); ok { - sub := flattenStructToMap(valueAsMap) - - for subKey, subValue := range sub { - out[key+"."+subKey] = subValue - } - } else { - out[key] = value - } - } - - return out -} - -// Fixes the case of the environment variables sent back from Viper since Viper stores -// everything as lower case. -func fixEnvSettingsCase(in map[string]interface{}) (out map[string]interface{}, err error) { - defer func() { - if r := recover(); r != nil { - mlog.Error(fmt.Sprintf("Panicked in fixEnvSettingsCase. This should never happen. %v", r)) - out = in - } - }() - - var fixCase func(map[string]interface{}, reflect.Type) map[string]interface{} - fixCase = func(in map[string]interface{}, t reflect.Type) map[string]interface{} { - if t.Kind() != reflect.Struct { - // Should never hit this, but this will prevent a panic if that does happen somehow - return nil - } - - fixCaseOut := make(map[string]interface{}, len(in)) - - for i := 0; i < t.NumField(); i++ { - field := t.Field(i) - - key := field.Name - if value, ok := in[strings.ToLower(key)]; ok { - if valueAsMap, ok := value.(map[string]interface{}); ok { - fixCaseOut[key] = fixCase(valueAsMap, field.Type) - } else { - fixCaseOut[key] = value - } - } - } - - return fixCaseOut - } - - out = fixCase(in, reflect.TypeOf(model.Config{})) - - return -} - -// ReadConfigFile reads and parses the configuration at the given file path. -func ReadConfigFile(path string, allowEnvironmentOverrides bool) (*model.Config, map[string]interface{}, error) { - f, err := os.Open(path) - if err != nil { - return nil, nil, err - } - defer f.Close() - return ReadConfig(f, allowEnvironmentOverrides) -} - -// EnsureConfigFile will attempt to locate a config file with the given name. If it does not exist, -// it will attempt to locate a default config file, and copy it to a file named fileName in the same -// directory. In either case, the config file path is returned. -func EnsureConfigFile(fileName string) (string, error) { - if configFile := fileutils.FindConfigFile(fileName); configFile != "" { - return configFile, nil - } - if defaultPath := fileutils.FindConfigFile("default.json"); defaultPath != "" { - destPath := filepath.Join(filepath.Dir(defaultPath), fileName) - src, err := os.Open(defaultPath) - if err != nil { - return "", err - } - defer src.Close() - dest, err := os.OpenFile(destPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600) - if err != nil { - return "", err - } - defer dest.Close() - if _, err := io.Copy(dest, src); err == nil { - return destPath, nil - } - } - return "", fmt.Errorf("no config file found") -} - -// LoadConfig will try to search around for the corresponding config file. It will search -// /tmp/fileName then attempt ./config/fileName, then ../config/fileName and last it will look at -// fileName. -func LoadConfig(fileName string) (*model.Config, string, map[string]interface{}, *model.AppError) { - var configPath string - - if fileName != filepath.Base(fileName) { - configPath = fileName - } else { - if path, err := EnsureConfigFile(fileName); err != nil { - appErr := model.NewAppError("LoadConfig", "utils.config.load_config.opening.panic", map[string]interface{}{"Filename": fileName, "Error": err.Error()}, "", 0) - return nil, "", nil, appErr - } else { - configPath = path - } - } - - config, envConfig, err := ReadConfigFile(configPath, true) - if err != nil { - appErr := model.NewAppError("LoadConfig", "utils.config.load_config.decoding.panic", map[string]interface{}{"Filename": fileName, "Error": err.Error()}, "", 0) - return nil, "", nil, appErr - } - - needSave := config.SqlSettings.AtRestEncryptKey == nil || len(*config.SqlSettings.AtRestEncryptKey) == 0 || - config.FileSettings.PublicLinkSalt == nil || len(*config.FileSettings.PublicLinkSalt) == 0 || - config.EmailSettings.InviteSalt == nil || len(*config.EmailSettings.InviteSalt) == 0 - - config.SetDefaults() - - // Don't treat it as an error right now if custom terms of service are enabled but text is empty. - // This is because terms of service text will be fetched from database at a later state, but - // the flag indicating it is enabled is fetched from config file right away. - if err := config.IsValid(); err != nil && err.Id != termsOfServiceEnabledAndEmpty.Id { - return nil, "", nil, err - } - - if needSave { - if err := SaveConfig(configPath, config); err != nil { - mlog.Warn(err.Error()) - } - } - - if err := ValidateLocales(config); err != nil { - if err := SaveConfig(configPath, config); err != nil { - mlog.Warn(err.Error()) - } - } - - if *config.FileSettings.DriverName == model.IMAGE_DRIVER_LOCAL { - dir := config.FileSettings.Directory - dirString := *dir - if len(*dir) > 0 && dirString[len(dirString)-1:] != "/" { - *config.FileSettings.Directory += "/" - } - } - - return config, configPath, envConfig, nil -} - +// GenerateClientConfig renders the given configuration for a client. func GenerateClientConfig(c *model.Config, diagnosticId string, license *model.License) map[string]string { props := GenerateLimitedClientConfig(c, diagnosticId, license) @@ -564,6 +185,7 @@ func GenerateClientConfig(c *model.Config, diagnosticId string, license *model.L return props } +// GenerateLimitedClientConfig renders the given configuration for an untrusted client. func GenerateLimitedClientConfig(c *model.Config, diagnosticId string, license *model.License) map[string]string { props := make(map[string]string) @@ -680,44 +302,3 @@ func GenerateLimitedClientConfig(c *model.Config, diagnosticId string, license * return props } - -func ValidateLocales(cfg *model.Config) *model.AppError { - var err *model.AppError - locales := utils.GetSupportedLocales() - if _, ok := locales[*cfg.LocalizationSettings.DefaultServerLocale]; !ok { - *cfg.LocalizationSettings.DefaultServerLocale = model.DEFAULT_LOCALE - err = model.NewAppError("ValidateLocales", "utils.config.supported_server_locale.app_error", nil, "", http.StatusBadRequest) - } - - if _, ok := locales[*cfg.LocalizationSettings.DefaultClientLocale]; !ok { - *cfg.LocalizationSettings.DefaultClientLocale = model.DEFAULT_LOCALE - err = model.NewAppError("ValidateLocales", "utils.config.supported_client_locale.app_error", nil, "", http.StatusBadRequest) - } - - if len(*cfg.LocalizationSettings.AvailableLocales) > 0 { - isDefaultClientLocaleInAvailableLocales := false - for _, word := range strings.Split(*cfg.LocalizationSettings.AvailableLocales, ",") { - if _, ok := locales[word]; !ok { - *cfg.LocalizationSettings.AvailableLocales = "" - isDefaultClientLocaleInAvailableLocales = true - err = model.NewAppError("ValidateLocales", "utils.config.supported_available_locales.app_error", nil, "", http.StatusBadRequest) - break - } - - if word == *cfg.LocalizationSettings.DefaultClientLocale { - isDefaultClientLocaleInAvailableLocales = true - } - } - - availableLocales := *cfg.LocalizationSettings.AvailableLocales - - if !isDefaultClientLocaleInAvailableLocales { - availableLocales += "," + *cfg.LocalizationSettings.DefaultClientLocale - err = model.NewAppError("ValidateLocales", "utils.config.add_client_locale.app_error", nil, "", http.StatusBadRequest) - } - - *cfg.LocalizationSettings.AvailableLocales = strings.Join(utils.RemoveDuplicatesFromStringArray(strings.Split(availableLocales, ",")), ",") - } - - return err -} diff --git a/config/client_test.go b/config/client_test.go new file mode 100644 index 0000000000..981956353c --- /dev/null +++ b/config/client_test.go @@ -0,0 +1,233 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package config_test + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/mattermost/mattermost-server/config" + "github.com/mattermost/mattermost-server/model" +) + +func TestGetClientConfig(t *testing.T) { + t.Parallel() + testCases := []struct { + description string + config *model.Config + diagnosticId string + license *model.License + expectedFields map[string]string + }{ + { + "unlicensed", + &model.Config{ + EmailSettings: model.EmailSettings{ + EmailNotificationContentsType: sToP(model.EMAIL_NOTIFICATION_CONTENTS_FULL), + }, + ThemeSettings: model.ThemeSettings{ + // Ignored, since not licensed. + AllowCustomThemes: bToP(false), + }, + ServiceSettings: model.ServiceSettings{ + WebsocketURL: sToP("ws://mattermost.example.com:8065"), + WebsocketPort: iToP(80), + WebsocketSecurePort: iToP(443), + }, + }, + "", + nil, + map[string]string{ + "DiagnosticId": "", + "EmailNotificationContentsType": "full", + "AllowCustomThemes": "true", + "EnforceMultifactorAuthentication": "false", + "WebsocketURL": "ws://mattermost.example.com:8065", + "WebsocketPort": "80", + "WebsocketSecurePort": "443", + }, + }, + { + "licensed, but not for theme management", + &model.Config{ + EmailSettings: model.EmailSettings{ + EmailNotificationContentsType: sToP(model.EMAIL_NOTIFICATION_CONTENTS_FULL), + }, + ThemeSettings: model.ThemeSettings{ + // Ignored, since not licensed. + AllowCustomThemes: bToP(false), + }, + }, + "tag1", + &model.License{ + Features: &model.Features{ + ThemeManagement: bToP(false), + }, + }, + map[string]string{ + "DiagnosticId": "tag1", + "EmailNotificationContentsType": "full", + "AllowCustomThemes": "true", + }, + }, + { + "licensed for theme management", + &model.Config{ + EmailSettings: model.EmailSettings{ + EmailNotificationContentsType: sToP(model.EMAIL_NOTIFICATION_CONTENTS_FULL), + }, + ThemeSettings: model.ThemeSettings{ + AllowCustomThemes: bToP(false), + }, + }, + "tag2", + &model.License{ + Features: &model.Features{ + ThemeManagement: bToP(true), + }, + }, + map[string]string{ + "DiagnosticId": "tag2", + "EmailNotificationContentsType": "full", + "AllowCustomThemes": "false", + }, + }, + { + "licensed for enforcement", + &model.Config{ + ServiceSettings: model.ServiceSettings{ + EnforceMultifactorAuthentication: bToP(true), + }, + }, + "tag1", + &model.License{ + Features: &model.Features{ + MFA: bToP(true), + }, + }, + map[string]string{ + "EnforceMultifactorAuthentication": "true", + }, + }, + { + "experimental channel organization enabled", + &model.Config{ + ServiceSettings: model.ServiceSettings{ + ExperimentalChannelOrganization: bToP(true), + }, + }, + "tag1", + nil, + map[string]string{ + "ExperimentalChannelOrganization": "true", + }, + }, + { + "experimental channel organization disabled, but experimental group unread channels on", + &model.Config{ + ServiceSettings: model.ServiceSettings{ + ExperimentalChannelOrganization: bToP(false), + ExperimentalGroupUnreadChannels: sToP(model.GROUP_UNREAD_CHANNELS_DEFAULT_ON), + }, + }, + "tag1", + nil, + map[string]string{ + "ExperimentalChannelOrganization": "true", + }, + }, + } + + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.description, func(t *testing.T) { + t.Parallel() + + testCase.config.SetDefaults() + if testCase.license != nil { + testCase.license.Features.SetDefaults() + } + + configMap := config.GenerateClientConfig(testCase.config, testCase.diagnosticId, testCase.license) + for expectedField, expectedValue := range testCase.expectedFields { + actualValue, ok := configMap[expectedField] + if assert.True(t, ok, fmt.Sprintf("config does not contain %v", expectedField)) { + assert.Equal(t, expectedValue, actualValue) + } + } + }) + } +} + +func TestGetLimitedClientConfig(t *testing.T) { + t.Parallel() + testCases := []struct { + description string + config *model.Config + diagnosticId string + license *model.License + expectedFields map[string]string + }{ + { + "unlicensed", + &model.Config{ + EmailSettings: model.EmailSettings{ + EmailNotificationContentsType: sToP(model.EMAIL_NOTIFICATION_CONTENTS_FULL), + }, + ThemeSettings: model.ThemeSettings{ + // Ignored, since not licensed. + AllowCustomThemes: bToP(false), + }, + ServiceSettings: model.ServiceSettings{ + WebsocketURL: sToP("ws://mattermost.example.com:8065"), + WebsocketPort: iToP(80), + WebsocketSecurePort: iToP(443), + }, + }, + "", + nil, + map[string]string{ + "DiagnosticId": "", + "EnforceMultifactorAuthentication": "false", + "WebsocketURL": "ws://mattermost.example.com:8065", + "WebsocketPort": "80", + "WebsocketSecurePort": "443", + }, + }, + } + + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.description, func(t *testing.T) { + t.Parallel() + + testCase.config.SetDefaults() + if testCase.license != nil { + testCase.license.Features.SetDefaults() + } + + configMap := config.GenerateLimitedClientConfig(testCase.config, testCase.diagnosticId, testCase.license) + for expectedField, expectedValue := range testCase.expectedFields { + actualValue, ok := configMap[expectedField] + if assert.True(t, ok, fmt.Sprintf("config does not contain %v", expectedField)) { + assert.Equal(t, expectedValue, actualValue) + } + } + }) + } +} + +func sToP(s string) *string { + return &s +} + +func bToP(b bool) *bool { + return &b +} + +func iToP(i int) *int { + return &i +} diff --git a/config/emitter.go b/config/emitter.go new file mode 100644 index 0000000000..540fc79a56 --- /dev/null +++ b/config/emitter.go @@ -0,0 +1,39 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package config + +import ( + "sync" + + "github.com/mattermost/mattermost-server/model" +) + +// emitter enables threadsafe registration and broadcasting to configuration listeners +type emitter struct { + listeners sync.Map +} + +// AddListener adds a callback function to invoke when the configuration is modified. +func (e *emitter) AddListener(listener Listener) string { + id := model.NewId() + + e.listeners.Store(id, listener) + + return id +} + +// RemoveListener removes a callback function using an id returned from AddListener. +func (e *emitter) RemoveListener(id string) { + e.listeners.Delete(id) +} + +// invokeConfigListeners synchronously notifies all listeners about the configuration change. +func (e *emitter) invokeConfigListeners(oldCfg, newCfg *model.Config) { + e.listeners.Range(func(key, value interface{}) bool { + listener := value.(Listener) + listener(oldCfg, newCfg) + + return true + }) +} diff --git a/config/emitter_test.go b/config/emitter_test.go new file mode 100644 index 0000000000..90460b36cf --- /dev/null +++ b/config/emitter_test.go @@ -0,0 +1,53 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package config + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/mattermost/mattermost-server/model" +) + +func TestEmitter(t *testing.T) { + var e emitter + + expectedOldCfg := &model.Config{} + expectedNewCfg := &model.Config{} + + listener1 := false + id1 := e.AddListener(func(oldCfg, newCfg *model.Config) { + assert.Equal(t, expectedOldCfg, oldCfg) + assert.Equal(t, expectedNewCfg, newCfg) + listener1 = true + }) + + listener2 := false + id2 := e.AddListener(func(oldCfg, newCfg *model.Config) { + assert.Equal(t, expectedOldCfg, oldCfg) + assert.Equal(t, expectedNewCfg, newCfg) + listener2 = true + }) + + e.invokeConfigListeners(expectedOldCfg, expectedNewCfg) + assert.True(t, listener1, "listener 1 not called") + assert.True(t, listener2, "listener 2 not called") + + e.RemoveListener(id2) + + listener1 = false + listener2 = false + e.invokeConfigListeners(expectedOldCfg, expectedNewCfg) + assert.True(t, listener1, "listener 1 not called") + assert.False(t, listener2, "listener 2 should not have been called") + + e.RemoveListener(id1) + + listener1 = false + listener2 = false + e.invokeConfigListeners(expectedOldCfg, expectedNewCfg) + assert.False(t, listener1, "listener 1 should not have been called") + assert.False(t, listener2, "listener 2 should not have been called") +} diff --git a/config/file.go b/config/file.go new file mode 100644 index 0000000000..c561b2bbbd --- /dev/null +++ b/config/file.go @@ -0,0 +1,308 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package config + +import ( + "bytes" + "fmt" + "io" + "io/ioutil" + "os" + "path/filepath" + "sync" + + "github.com/pkg/errors" + + "github.com/mattermost/mattermost-server/mlog" + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils/fileutils" +) + +var ( + ErrReadOnlyConfiguration = errors.New("configuration is read-only") +) + +// FileStore is a config store backed by a file such as config/config.json. +type FileStore struct { + emitter + + config *model.Config + environmentOverrides map[string]interface{} + configLock sync.RWMutex + path string + watch bool + watcher *watcher +} + +// NewFileStore creates a new instance of a config store backed by the given file path. +// +// If watch is true, any external changes to the file will force a reload. +func NewFileStore(path string, watch bool) (fs *FileStore, err error) { + resolvedPath, err := resolveConfigFilePath(path) + if err != nil { + return nil, err + } + + fs = &FileStore{ + path: resolvedPath, + watch: watch, + } + if err = fs.Load(); err != nil { + return nil, errors.Wrap(err, "failed to load") + } + + if fs.watch { + if err = fs.startWatcher(); err != nil { + mlog.Error("failed to start config watcher", mlog.String("path", path), mlog.Err(err)) + } + } + + return fs, nil +} + +// resolveConfigFilePath attempts to resolve the given configuration file path to an absolute path. +// +// Consideration is given to maintaining backwards compatibility when resolving the path to the +// configuration file. +func resolveConfigFilePath(path string) (string, error) { + // Absolute paths are explicit and require no resolution. + if filepath.IsAbs(path) { + return path, nil + } + + // Search for the given relative path (or plain filename) in various directories, + // resolving to the corresponding absolute path if found. FindConfigFile takes into account + // various common search paths rooted both at the current working directory and relative + // to the executable. + if configFile := fileutils.FindConfigFile(path); configFile != "" { + return configFile, nil + } + + // Otherwise, search for the config/ folder using the same heuristics as above, and build + // an absolute path anchored there and joining the given input path (or plain filename). + if configFolder, found := fileutils.FindDir("config"); found { + return filepath.Join(configFolder, path), nil + } + + // Fail altogether if we can't even find the config/ folder. This should only happen if + // the executable is relocated away from the supporting files. + return "", fmt.Errorf("failed to find config file %s", path) +} + +// Get fetches the current, cached configuration. +func (fs *FileStore) Get() *model.Config { + fs.configLock.RLock() + defer fs.configLock.RUnlock() + + return fs.config +} + +// GetEnvironmentOverrides fetches the configuration fields overridden by environment variables. +func (fs *FileStore) GetEnvironmentOverrides() map[string]interface{} { + fs.configLock.RLock() + defer fs.configLock.RUnlock() + + return fs.environmentOverrides +} + +// Set replaces the current configuration in its entirety, without updating the backing store. +func (fs *FileStore) Set(newCfg *model.Config) (*model.Config, error) { + fs.configLock.Lock() + var unlockOnce sync.Once + defer unlockOnce.Do(fs.configLock.Unlock) + + oldCfg := fs.config + + // TODO: disallow attempting to save a directly modified config (comparing pointers). This + // wouldn't be an exhaustive check, given the use of pointers throughout the data + // structure, but might prevent common mistakes. Requires upstream changes first. + // if newCfg == oldCfg { + // return nil, errors.New("old configuration modified instead of cloning") + // } + + newCfg = newCfg.Clone() + newCfg.SetDefaults() + + // Sometimes the config is received with "fake" data in sensitive fields. Apply the real + // data from the existing config as necessary. + desanitize(oldCfg, newCfg) + + if err := newCfg.IsValid(); err != nil { + return nil, errors.Wrap(err, "new configuration is invalid") + } + + if *oldCfg.ClusterSettings.Enable && *oldCfg.ClusterSettings.ReadOnlyConfig { + return nil, ErrReadOnlyConfiguration + } + + // Ideally, Set would persist automatically and abstract this completely away from the + // client. Doing so requires a few upstream changes first, so for now an explicit Save() + // remains required. + // if err := fs.persist(newCfg); err != nil { + // return nil, errors.Wrap(err, "failed to persist") + // } + + fs.config = newCfg + + unlockOnce.Do(fs.configLock.Unlock) + + // Notify listeners synchronously. Ideally, this would be asynchronous, but existing code + // assumes this and there would be increased complexity to avoid racing updates. + fs.invokeConfigListeners(oldCfg, newCfg) + + return oldCfg, nil +} + +// persist writes the configuration to the configured file. +func (fs *FileStore) persist(cfg *model.Config) error { + fs.stopWatcher() + + b, err := marshalConfig(cfg) + if err != nil { + return errors.Wrap(err, "failed to serialize") + } + + err = ioutil.WriteFile(fs.path, b, 0644) + if err != nil { + return errors.Wrap(err, "failed to write file") + } + + if fs.watch { + if err = fs.startWatcher(); err != nil { + mlog.Error("failed to start config watcher", mlog.String("path", fs.path), mlog.Err(err)) + } + } + + return nil +} + +// Load updates the current configuration from the backing store. +func (fs *FileStore) Load() (err error) { + var needsSave bool + var f io.ReadCloser + + f, err = os.Open(fs.path) + if os.IsNotExist(err) { + needsSave = true + defaultCfg := model.Config{} + defaultCfg.SetDefaults() + + var defaultCfgBytes []byte + defaultCfgBytes, err = marshalConfig(&defaultCfg) + if err != nil { + return errors.Wrap(err, "failed to serialize default config") + } + + f = ioutil.NopCloser(bytes.NewReader(defaultCfgBytes)) + + } else if err != nil { + return errors.Wrapf(err, "failed to open %s for reading", fs.path) + } + defer func() { + closeErr := f.Close() + if err == nil && closeErr != nil { + err = errors.Wrap(closeErr, "failed to close") + } + }() + + allowEnvironmentOverrides := true + loadedCfg, environmentOverrides, err := unmarshalConfig(f, allowEnvironmentOverrides) + if err != nil { + return errors.Wrapf(err, "failed to unmarshal config from %s", fs.path) + } + + // SetDefaults generates various keys and salts if not previously configured. Determine if + // such a change will be made before invoking. This method will not effect the save: that + // remains the responsibility of the caller. + needsSave = needsSave || loadedCfg.SqlSettings.AtRestEncryptKey == nil || len(*loadedCfg.SqlSettings.AtRestEncryptKey) == 0 + needsSave = needsSave || loadedCfg.FileSettings.PublicLinkSalt == nil || len(*loadedCfg.FileSettings.PublicLinkSalt) == 0 + needsSave = needsSave || loadedCfg.EmailSettings.InviteSalt == nil || len(*loadedCfg.EmailSettings.InviteSalt) == 0 + + loadedCfg.SetDefaults() + + if err := loadedCfg.IsValid(); err != nil { + return errors.Wrap(err, "invalid config") + } + + if changed := fixConfig(loadedCfg); changed { + needsSave = true + } + + fs.configLock.Lock() + var unlockOnce sync.Once + defer unlockOnce.Do(fs.configLock.Unlock) + + if needsSave { + if err = fs.persist(loadedCfg); err != nil { + return errors.Wrap(err, "failed to persist required changes after load") + } + } + + oldCfg := fs.config + fs.config = loadedCfg + fs.environmentOverrides = environmentOverrides + + unlockOnce.Do(fs.configLock.Unlock) + + // Notify listeners synchronously. Ideally, this would be asynchronous, but existing code + // assumes this and there would be increased complexity to avoid racing updates. + fs.invokeConfigListeners(oldCfg, loadedCfg) + + return nil +} + +// Save writes the current configuration to the backing store. +func (fs *FileStore) Save() error { + fs.configLock.Lock() + defer fs.configLock.Unlock() + + return fs.persist(fs.config) +} + +// startWatcher starts a watcher to monitor for external config file changes. +func (fs *FileStore) startWatcher() error { + if fs.watcher != nil { + return nil + } + + watcher, err := newWatcher(fs.path, func() { + if err := fs.Load(); err != nil { + mlog.Error("failed to reload file on change", mlog.String("path", fs.path), mlog.Err(err)) + } + }) + if err != nil { + return err + } + + fs.watcher = watcher + + return nil +} + +// stopWatcher stops any previously started watcher. +func (fs *FileStore) stopWatcher() { + if fs.watcher == nil { + return + } + + if err := fs.watcher.Close(); err != nil { + mlog.Error("failed to close watcher", mlog.Err(err)) + } + fs.watcher = nil +} + +// String returns the path to the file backing the config. +func (fs *FileStore) String() string { + return "file://" + fs.path +} + +// Close cleans up resources associated with the store. +func (fs *FileStore) Close() error { + fs.configLock.Lock() + defer fs.configLock.Unlock() + + fs.stopWatcher() + + return nil +} diff --git a/config/file_test.go b/config/file_test.go new file mode 100644 index 0000000000..646a6e74d8 --- /dev/null +++ b/config/file_test.go @@ -0,0 +1,655 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package config_test + +import ( + "io/ioutil" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mattermost/mattermost-server/config" + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils" +) + +var emptyConfig, readOnlyConfig, minimalConfig, invalidConfig, fixesRequiredConfig, ldapConfig, testConfig *model.Config + +func init() { + emptyConfig = &model.Config{} + readOnlyConfig = &model.Config{ + ClusterSettings: model.ClusterSettings{ + Enable: bToP(true), + ReadOnlyConfig: bToP(true), + }, + } + minimalConfig = &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: sToP("http://minimal"), + }, + SqlSettings: model.SqlSettings{ + AtRestEncryptKey: sToP("abcdefghijklmnopqrstuvwxyz0123456789"), + }, + FileSettings: model.FileSettings{ + PublicLinkSalt: sToP("abcdefghijklmnopqrstuvwxyz0123456789"), + }, + EmailSettings: model.EmailSettings{ + InviteSalt: sToP("abcdefghijklmnopqrstuvwxyz0123456789"), + }, + LocalizationSettings: model.LocalizationSettings{ + DefaultServerLocale: sToP("en"), + DefaultClientLocale: sToP("en"), + }, + } + invalidConfig = &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: sToP("invalid"), + }, + } + fixesRequiredConfig = &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: sToP("http://trailingslash/"), + }, + SqlSettings: model.SqlSettings{ + AtRestEncryptKey: sToP("abcdefghijklmnopqrstuvwxyz0123456789"), + }, + FileSettings: model.FileSettings{ + DriverName: sToP(model.IMAGE_DRIVER_LOCAL), + Directory: sToP("/path/to/directory"), + PublicLinkSalt: sToP("abcdefghijklmnopqrstuvwxyz0123456789"), + }, + EmailSettings: model.EmailSettings{ + InviteSalt: sToP("abcdefghijklmnopqrstuvwxyz0123456789"), + }, + LocalizationSettings: model.LocalizationSettings{ + DefaultServerLocale: sToP("garbage"), + DefaultClientLocale: sToP("garbage"), + }, + } + ldapConfig = &model.Config{ + LdapSettings: model.LdapSettings{ + BindPassword: sToP("password"), + }, + } + testConfig = &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: sToP("http://TestStoreNew"), + }, + } +} + +func setupConfigFile(t *testing.T, cfg *model.Config) (string, func()) { + os.Clearenv() + t.Helper() + + tempDir, err := ioutil.TempDir("", "setupConfigFile") + require.NoError(t, err) + + f, err := ioutil.TempFile(tempDir, "setupConfigFile") + require.NoError(t, err) + + cfgData, err := config.MarshalConfig(cfg) + require.NoError(t, err) + + ioutil.WriteFile(f.Name(), cfgData, 0644) + + return f.Name(), func() { + os.RemoveAll(tempDir) + } +} + +// assertFileEqualsConfig verifies the on disk contents of the given path equal the given config. +func assertFileEqualsConfig(t *testing.T, expectedCfg *model.Config, path string) { + f, err := os.Open(path) + require.Nil(t, err) + + // These fields require special initialization for our tests. + expectedCfg = expectedCfg.Clone() + expectedCfg.MessageExportSettings.GlobalRelaySettings = &model.GlobalRelayMessageExportSettings{} + expectedCfg.PluginSettings.Plugins = make(map[string]map[string]interface{}) + expectedCfg.PluginSettings.PluginStates = make(map[string]*model.PluginState) + + actualCfg, _, err := config.UnmarshalConfig(f, false) + require.Nil(t, err) + + assert.Equal(t, expectedCfg, actualCfg) +} + +// assertFileNotEqualsConfig verifies the on disk contents of the given path does not equal the given config. +func assertFileNotEqualsConfig(t *testing.T, expectedCfg *model.Config, path string) { + f, err := os.Open(path) + require.Nil(t, err) + + // These fields require special initialization for our tests. + expectedCfg = expectedCfg.Clone() + expectedCfg.MessageExportSettings.GlobalRelaySettings = &model.GlobalRelayMessageExportSettings{} + expectedCfg.PluginSettings.Plugins = make(map[string]map[string]interface{}) + expectedCfg.PluginSettings.PluginStates = make(map[string]*model.PluginState) + + actualCfg, _, err := config.UnmarshalConfig(f, false) + require.Nil(t, err) + + assert.NotEqual(t, expectedCfg, actualCfg) +} + +func TestFileStoreNew(t *testing.T) { + utils.TranslationsPreInit() + + t.Run("absolute path, initialization required", func(t *testing.T) { + path, tearDown := setupConfigFile(t, testConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + assert.Equal(t, "http://TestStoreNew", *fs.Get().ServiceSettings.SiteURL) + assertFileNotEqualsConfig(t, testConfig, path) + }) + + t.Run("absolute path, already minimally configured", func(t *testing.T) { + path, tearDown := setupConfigFile(t, minimalConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + assert.Equal(t, "http://minimal", *fs.Get().ServiceSettings.SiteURL) + assertFileEqualsConfig(t, minimalConfig, path) + }) + + t.Run("absolute path, file does not exist", func(t *testing.T) { + tempDir, err := ioutil.TempDir("", "TestFileStoreNew") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + + path := filepath.Join(tempDir, "does_not_exist") + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + assert.Equal(t, model.SERVICE_SETTINGS_DEFAULT_SITE_URL, *fs.Get().ServiceSettings.SiteURL) + assertFileNotEqualsConfig(t, testConfig, path) + }) + + t.Run("absolute path, path to file does not exist", func(t *testing.T) { + tempDir, err := ioutil.TempDir("", "TestFileStoreNew") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + + path := filepath.Join(tempDir, "does/not/exist") + _, err = config.NewFileStore(path, false) + require.Error(t, err) + }) + + t.Run("relative path, file exists", func(t *testing.T) { + os.Clearenv() + + err := os.MkdirAll("TestFileStoreNew/a/b/c", 0700) + require.NoError(t, err) + defer os.RemoveAll("TestFileStoreNew") + + path := "TestFileStoreNew/a/b/c/config.json" + + cfgData, err := config.MarshalConfig(testConfig) + require.NoError(t, err) + + ioutil.WriteFile(path, cfgData, 0644) + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + assert.Equal(t, "http://TestStoreNew", *fs.Get().ServiceSettings.SiteURL) + assertFileNotEqualsConfig(t, testConfig, path) + }) + + t.Run("relative path, file does not exist", func(t *testing.T) { + os.Clearenv() + + err := os.MkdirAll("TestFileStoreNew/a/b/c", 0700) + require.NoError(t, err) + defer os.RemoveAll("TestFileStoreNew") + + path := "TestFileStoreNew/a/b/c/config.json" + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + assert.Equal(t, model.SERVICE_SETTINGS_DEFAULT_SITE_URL, *fs.Get().ServiceSettings.SiteURL) + assertFileNotEqualsConfig(t, testConfig, path) + }) +} + +func TestFileStoreGet(t *testing.T) { + path, tearDown := setupConfigFile(t, testConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + cfg := fs.Get() + assert.Equal(t, "http://TestStoreNew", *cfg.ServiceSettings.SiteURL) + + cfg2 := fs.Get() + assert.Equal(t, "http://TestStoreNew", *cfg.ServiceSettings.SiteURL) + + assert.True(t, cfg == cfg2, "Get() returned different configuration instances") + + newCfg := &model.Config{} + oldCfg, err := fs.Set(newCfg) + + assert.True(t, oldCfg == cfg, "returned config after set() changed original") + assert.False(t, newCfg == cfg, "returned config should have been different from original") +} + +func TestFileStoreGetEnivironmentOverrides(t *testing.T) { + path, tearDown := setupConfigFile(t, testConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + assert.Equal(t, "http://TestStoreNew", *fs.Get().ServiceSettings.SiteURL) + assert.Empty(t, fs.GetEnvironmentOverrides()) + + os.Setenv("MM_SERVICESETTINGS_SITEURL", "http://override") + + fs, err = config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + assert.Equal(t, "http://override", *fs.Get().ServiceSettings.SiteURL) + assert.Equal(t, map[string]interface{}{"ServiceSettings": map[string]interface{}{"SiteURL": true}}, fs.GetEnvironmentOverrides()) +} + +func TestFileStoreSet(t *testing.T) { + t.Run("set same pointer value", func(t *testing.T) { + t.Skip("not yet implemented") + + path, tearDown := setupConfigFile(t, emptyConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + _, err = fs.Set(fs.Get()) + if assert.Error(t, err) { + assert.EqualError(t, err, "old configuration modified instead of cloning") + } + }) + + t.Run("defaults required", func(t *testing.T) { + path, tearDown := setupConfigFile(t, minimalConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + oldCfg := fs.Get() + + newCfg := &model.Config{} + + retCfg, err := fs.Set(newCfg) + require.NoError(t, err) + assert.Equal(t, oldCfg, retCfg) + + assert.Equal(t, model.SERVICE_SETTINGS_DEFAULT_SITE_URL, *fs.Get().ServiceSettings.SiteURL) + }) + + t.Run("desanitization required", func(t *testing.T) { + path, tearDown := setupConfigFile(t, ldapConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + oldCfg := fs.Get() + + newCfg := &model.Config{} + newCfg.LdapSettings.BindPassword = sToP(model.FAKE_SETTING) + + retCfg, err := fs.Set(newCfg) + require.NoError(t, err) + assert.Equal(t, oldCfg, retCfg) + + assert.Equal(t, "password", *fs.Get().LdapSettings.BindPassword) + }) + + t.Run("invalid", func(t *testing.T) { + path, tearDown := setupConfigFile(t, emptyConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + newCfg := &model.Config{} + newCfg.ServiceSettings.SiteURL = sToP("invalid") + + _, err = fs.Set(newCfg) + if assert.Error(t, err) { + assert.EqualError(t, err, "new configuration is invalid: Config.IsValid: model.config.is_valid.site_url.app_error, ") + } + + assert.Equal(t, model.SERVICE_SETTINGS_DEFAULT_SITE_URL, *fs.Get().ServiceSettings.SiteURL) + }) + + t.Run("read-only", func(t *testing.T) { + path, tearDown := setupConfigFile(t, readOnlyConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + newCfg := &model.Config{} + + _, err = fs.Set(newCfg) + if assert.Error(t, err) { + assert.Equal(t, err, config.ErrReadOnlyConfiguration) + } + + assert.Equal(t, model.SERVICE_SETTINGS_DEFAULT_SITE_URL, *fs.Get().ServiceSettings.SiteURL) + }) + + t.Run("persist failed", func(t *testing.T) { + t.Skip("skipping persistence test inside Set") + path, tearDown := setupConfigFile(t, emptyConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + os.Chmod(path, 0500) + + newCfg := &model.Config{} + + _, err = fs.Set(newCfg) + if assert.Error(t, err) { + assert.True(t, strings.HasPrefix(err.Error(), "failed to persist: failed to write file")) + } + + assert.Equal(t, model.SERVICE_SETTINGS_DEFAULT_SITE_URL, *fs.Get().ServiceSettings.SiteURL) + }) + + t.Run("listeners notified", func(t *testing.T) { + path, tearDown := setupConfigFile(t, emptyConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + oldCfg := fs.Get() + + called := make(chan bool, 1) + callback := func(oldfg, newCfg *model.Config) { + called <- true + } + fs.AddListener(callback) + + newCfg := &model.Config{} + + retCfg, err := fs.Set(newCfg) + require.NoError(t, err) + assert.Equal(t, oldCfg, retCfg) + + select { + case <-called: + case <-time.After(5 * time.Second): + t.Fatal("callback should have been called when config written") + } + }) + + t.Run("watcher restarted", func(t *testing.T) { + if testing.Short() { + t.Skip("skipping watcher test in short mode") + } + + path, tearDown := setupConfigFile(t, emptyConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, true) + require.NoError(t, err) + defer fs.Close() + + _, err = fs.Set(&model.Config{}) + require.NoError(t, err) + + // Let the initial call to invokeConfigListeners finish. + time.Sleep(1 * time.Second) + + called := make(chan bool, 1) + callback := func(oldfg, newCfg *model.Config) { + called <- true + } + fs.AddListener(callback) + + // Rewrite the config to the file on disk + cfgData, err := config.MarshalConfig(emptyConfig) + require.NoError(t, err) + + ioutil.WriteFile(path, cfgData, 0644) + select { + case <-called: + case <-time.After(5 * time.Second): + t.Fatal("callback should have been called when config written") + } + }) +} + +func TestFileStoreLoad(t *testing.T) { + t.Run("file no longer exists", func(t *testing.T) { + path, tearDown := setupConfigFile(t, emptyConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + os.Remove(path) + + err = fs.Load() + require.NoError(t, err) + assertFileNotEqualsConfig(t, emptyConfig, path) + }) + + t.Run("honour environment", func(t *testing.T) { + path, tearDown := setupConfigFile(t, minimalConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + os.Setenv("MM_SERVICESETTINGS_SITEURL", "http://override") + + err = fs.Load() + require.NoError(t, err) + assert.Equal(t, "http://override", *fs.Get().ServiceSettings.SiteURL) + assert.Equal(t, map[string]interface{}{"ServiceSettings": map[string]interface{}{"SiteURL": true}}, fs.GetEnvironmentOverrides()) + }) + + t.Run("invalid", func(t *testing.T) { + path, tearDown := setupConfigFile(t, emptyConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + cfgData, err := config.MarshalConfig(invalidConfig) + require.NoError(t, err) + + ioutil.WriteFile(path, cfgData, 0644) + + err = fs.Load() + if assert.Error(t, err) { + assert.EqualError(t, err, "invalid config: Config.IsValid: model.config.is_valid.site_url.app_error, ") + } + }) + + t.Run("fixes required", func(t *testing.T) { + path, tearDown := setupConfigFile(t, fixesRequiredConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + err = fs.Load() + require.NoError(t, err) + assertFileNotEqualsConfig(t, fixesRequiredConfig, path) + assert.Equal(t, "http://trailingslash", *fs.Get().ServiceSettings.SiteURL) + assert.Equal(t, "/path/to/directory/", *fs.Get().FileSettings.Directory) + assert.Equal(t, "en", *fs.Get().LocalizationSettings.DefaultServerLocale) + assert.Equal(t, "en", *fs.Get().LocalizationSettings.DefaultClientLocale) + }) + + t.Run("listeners notifed", func(t *testing.T) { + path, tearDown := setupConfigFile(t, emptyConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + called := make(chan bool, 1) + callback := func(oldfg, newCfg *model.Config) { + called <- true + } + fs.AddListener(callback) + + err = fs.Load() + require.NoError(t, err) + + select { + case <-called: + case <-time.After(5 * time.Second): + t.Fatal("callback should have been called when config loaded") + } + }) +} + +func TestFileStoreWatcherEmitter(t *testing.T) { + if testing.Short() { + t.Skip("skipping watcher test in short mode") + } + + t.Parallel() + + path, tearDown := setupConfigFile(t, emptyConfig) + defer tearDown() + + t.Run("disabled", func(t *testing.T) { + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + // Let the initial call to invokeConfigListeners finish. + time.Sleep(1 * time.Second) + + called := make(chan bool, 1) + callback := func(oldfg, newCfg *model.Config) { + called <- true + } + fs.AddListener(callback) + + // Rewrite the config to the file on disk + cfgData, err := config.MarshalConfig(emptyConfig) + require.NoError(t, err) + + ioutil.WriteFile(path, cfgData, 0644) + select { + case <-called: + t.Fatal("callback should not have been called since watching disabled") + case <-time.After(1 * time.Second): + } + }) + + t.Run("enabled", func(t *testing.T) { + fs, err := config.NewFileStore(path, true) + require.NoError(t, err) + defer fs.Close() + + called := make(chan bool, 1) + callback := func(oldfg, newCfg *model.Config) { + called <- true + } + fs.AddListener(callback) + + // Rewrite the config to the file on disk + cfgData, err := config.MarshalConfig(emptyConfig) + require.NoError(t, err) + + ioutil.WriteFile(path, cfgData, 0644) + select { + case <-called: + case <-time.After(5 * time.Second): + t.Fatal("callback should have been called when config written") + } + }) +} + +func TestFileStoreSave(t *testing.T) { + path, tearDown := setupConfigFile(t, minimalConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, true) + require.NoError(t, err) + defer fs.Close() + + newCfg := &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: sToP("http://new"), + }, + } + + t.Run("set without save", func(t *testing.T) { + _, err = fs.Set(newCfg) + require.NoError(t, err) + + err = fs.Load() + require.NoError(t, err) + + assert.Equal(t, "http://minimal", *fs.Get().ServiceSettings.SiteURL) + }) + + t.Run("set with save", func(t *testing.T) { + _, err = fs.Set(newCfg) + require.NoError(t, err) + + err = fs.Save() + require.NoError(t, err) + + err = fs.Load() + require.NoError(t, err) + + assert.Equal(t, "http://new", *fs.Get().ServiceSettings.SiteURL) + }) +} + +func TestFileStoreString(t *testing.T) { + path, tearDown := setupConfigFile(t, emptyConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + defer fs.Close() + + assert.Equal(t, "file://"+path, fs.String()) +} diff --git a/config/main_test.go b/config/main_test.go new file mode 100644 index 0000000000..0717f3edd7 --- /dev/null +++ b/config/main_test.go @@ -0,0 +1,17 @@ +package config + +import ( + "io" + + "github.com/mattermost/mattermost-server/model" +) + +// MarshalConfig exposes the internal marshalConfig to tests only. +func MarshalConfig(cfg *model.Config) ([]byte, error) { + return marshalConfig(cfg) +} + +// UnmarshalConfig exposes the internal unmarshalConfig to tests only. +func UnmarshalConfig(r io.Reader, allowEnvironmentOverrides bool) (*model.Config, map[string]interface{}, error) { + return unmarshalConfig(r, allowEnvironmentOverrides) +} diff --git a/config/memory.go b/config/memory.go new file mode 100644 index 0000000000..e93f68e453 --- /dev/null +++ b/config/memory.go @@ -0,0 +1,103 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package config + +import ( + "bytes" + "encoding/json" + "io/ioutil" + + "github.com/pkg/errors" + + "github.com/mattermost/mattermost-server/model" +) + +// memoryStore implements the Store interface. It is meant primarily for testing. +type memoryStore struct { + emitter + + Config *model.Config + EnvironmentOverrides map[string]interface{} + + allowEnvironmentOverrides bool +} + +// NewMemoryStore creates a new memoryStore instance. +func NewMemoryStore(allowEnvironmentOverrides bool) (*memoryStore, error) { + defaultCfg := &model.Config{} + defaultCfg.SetDefaults() + + ms := &memoryStore{ + Config: defaultCfg, + allowEnvironmentOverrides: allowEnvironmentOverrides, + } + + if err := ms.Load(); err != nil { + return nil, err + } + + return ms, nil +} + +// Get fetches the current, cached configuration. +func (ms *memoryStore) Get() *model.Config { + return ms.Config +} + +// GetEnvironmentOverrides fetches the configuration fields overridden by environment variables. +func (ms *memoryStore) GetEnvironmentOverrides() map[string]interface{} { + return ms.EnvironmentOverrides +} + +// Set replaces the current configuration in its entirety. +func (ms *memoryStore) Set(newCfg *model.Config) (*model.Config, error) { + oldCfg := ms.Config + + newCfg.SetDefaults() + ms.Config = newCfg + + return oldCfg, nil +} + +// serialize converts the given configuration into JSON bytes for persistence. +func (ms *memoryStore) serialize(cfg *model.Config) ([]byte, error) { + return json.MarshalIndent(cfg, "", " ") +} + +// Load applies environment overrides to the current config as if a re-load had occurred. +func (ms *memoryStore) Load() (err error) { + var cfgBytes []byte + cfgBytes, err = ms.serialize(ms.Config) + if err != nil { + return errors.Wrap(err, "failed to serialize config") + } + + f := ioutil.NopCloser(bytes.NewReader(cfgBytes)) + + allowEnvironmentOverrides := true + loadedCfg, environmentOverrides, err := unmarshalConfig(f, allowEnvironmentOverrides) + if err != nil { + return errors.Wrap(err, "failed to load config") + } + + ms.Config = loadedCfg + ms.EnvironmentOverrides = environmentOverrides + + return nil +} + +// Save does nothing, as there is no backing store. +func (ms *memoryStore) Save() error { + return nil +} + +// String returns a hard-coded description, as there is no backing store. +func (ms *memoryStore) String() string { + return "mock://" +} + +// Close does nothing for a mock store. +func (ms *memoryStore) Close() error { + return nil +} diff --git a/config/store.go b/config/store.go new file mode 100644 index 0000000000..7066928fb9 --- /dev/null +++ b/config/store.go @@ -0,0 +1,41 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package config + +import ( + "github.com/mattermost/mattermost-server/model" +) + +// Listener is a callback function invoked when the configuration changes. +type Listener func(oldConfig *model.Config, newConfig *model.Config) + +// Store abstracts the act of getting and setting the configuration. +type Store interface { + // Get fetches the current, cached configuration. + Get() *model.Config + + // GetEnvironmentOverrides fetches the configuration fields overridden by environment variables. + GetEnvironmentOverrides() map[string]interface{} + + // Set replaces the current configuration in its entirety, without updating the backing store. + Set(*model.Config) (*model.Config, error) + + // Load updates the current configuration from the backing store, possibly initializing. + Load() (err error) + + // Save writes the current configuration to the backing store. + Save() error + + // AddListener adds a callback function to invoke when the configuration is modified. + AddListener(listener Listener) string + + // RemoveListener removes a callback function using an id returned from AddListener. + RemoveListener(id string) + + // String describes the backing store for the config. + String() string + + // Close cleans up resources associated with the store. + Close() error +} diff --git a/config/unmarshal.go b/config/unmarshal.go new file mode 100644 index 0000000000..b72ba8b28a --- /dev/null +++ b/config/unmarshal.go @@ -0,0 +1,211 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package config + +import ( + "bytes" + "encoding/json" + "io" + "io/ioutil" + "reflect" + "strings" + + "github.com/mattermost/viper" + "github.com/pkg/errors" + + "github.com/mattermost/mattermost-server/mlog" + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils/jsonutils" +) + +// newViper creates an instance of viper.Viper configured for parsing a configuration. +func newViper(allowEnvironmentOverrides bool) *viper.Viper { + v := viper.New() + + v.SetConfigType("json") + + if allowEnvironmentOverrides { + v.SetEnvPrefix("mm") + v.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) + v.AutomaticEnv() + } + + // Set zeroed defaults for all the config settings so that Viper knows what environment variables + // it needs to be looking for. The correct defaults will later be applied using Config.SetDefaults. + defaults := getDefaultsFromStruct(model.Config{}) + + for key, value := range defaults { + if key == "PluginSettings.Plugins" || key == "PluginSettings.PluginStates" { + continue + } + + v.SetDefault(key, value) + } + + return v +} + +func getDefaultsFromStruct(s interface{}) map[string]interface{} { + return flattenStructToMap(structToMap(reflect.TypeOf(s))) +} + +// Converts a struct type into a nested map with keys matching the struct's fields and values +// matching the zeroed value of the corresponding field. +func structToMap(t reflect.Type) (out map[string]interface{}) { + defer func() { + if r := recover(); r != nil { + mlog.Error("Panicked in structToMap. This should never happen.", mlog.Any("err", r)) + } + }() + + if t.Kind() != reflect.Struct { + // Should never hit this, but this will prevent a panic if that does happen somehow + return nil + } + + out = map[string]interface{}{} + + for i := 0; i < t.NumField(); i++ { + field := t.Field(i) + + var value interface{} + + switch field.Type.Kind() { + case reflect.Struct: + value = structToMap(field.Type) + case reflect.Ptr: + indirectType := field.Type.Elem() + + if indirectType.Kind() == reflect.Struct { + // Follow pointers to structs since we need to define defaults for their fields + value = structToMap(indirectType) + } else { + value = nil + } + default: + value = reflect.Zero(field.Type).Interface() + } + + out[field.Name] = value + } + + return +} + +// Flattens a nested map so that the result is a single map with keys corresponding to the +// path through the original map. For example, +// { +// "a": { +// "b": 1 +// }, +// "c": "sea" +// } +// would flatten to +// { +// "a.b": 1, +// "c": "sea" +// } +func flattenStructToMap(in map[string]interface{}) map[string]interface{} { + out := make(map[string]interface{}) + + for key, value := range in { + if valueAsMap, ok := value.(map[string]interface{}); ok { + sub := flattenStructToMap(valueAsMap) + + for subKey, subValue := range sub { + out[key+"."+subKey] = subValue + } + } else { + out[key] = value + } + } + + return out +} + +// marshalConfig converts the given configuration into JSON bytes for persistence. +func marshalConfig(cfg *model.Config) ([]byte, error) { + return json.MarshalIndent(cfg, "", " ") +} + +// unmarshalConfig unmarshals a raw configuration into a Config model and environment variable overrides. +func unmarshalConfig(r io.Reader, allowEnvironmentOverrides bool) (*model.Config, map[string]interface{}, error) { + // Pre-flight check the syntax of the configuration file to improve error messaging. + configData, err := ioutil.ReadAll(r) + if err != nil { + return nil, nil, errors.Wrapf(err, "failed to read") + } + + var rawConfig interface{} + if err = json.Unmarshal(configData, &rawConfig); err != nil { + return nil, nil, jsonutils.HumanizeJsonError(err, configData) + } + + v := newViper(allowEnvironmentOverrides) + if err := v.ReadConfig(bytes.NewReader(configData)); err != nil { + return nil, nil, err + } + + var config model.Config + unmarshalErr := v.Unmarshal(&config) + // https://github.com/spf13/viper/issues/324 + // https://github.com/spf13/viper/issues/348 + if unmarshalErr == nil { + config.PluginSettings.Plugins = make(map[string]map[string]interface{}) + unmarshalErr = v.UnmarshalKey("pluginsettings.plugins", &config.PluginSettings.Plugins) + } + if unmarshalErr == nil { + config.PluginSettings.PluginStates = make(map[string]*model.PluginState) + unmarshalErr = v.UnmarshalKey("pluginsettings.pluginstates", &config.PluginSettings.PluginStates) + } + + envConfig := v.EnvSettings() + + var envErr error + if envConfig, envErr = fixEnvSettingsCase(envConfig); envErr != nil { + return nil, nil, envErr + } + + return &config, envConfig, unmarshalErr +} + +// Fixes the case of the environment variables sent back from Viper since Viper stores everything +// as lower case. +func fixEnvSettingsCase(in map[string]interface{}) (out map[string]interface{}, err error) { + defer func() { + if r := recover(); r != nil { + mlog.Error("Panicked in fixEnvSettingsCase. This should never happen.", mlog.Any("err", r)) + out = in + } + }() + + var fixCase func(map[string]interface{}, reflect.Type) map[string]interface{} + fixCase = func(in map[string]interface{}, t reflect.Type) map[string]interface{} { + if t.Kind() != reflect.Struct { + // Should never hit this, but this will prevent a panic if that does happen somehow + return nil + } + + fixCaseOut := make(map[string]interface{}, len(in)) + + for i := 0; i < t.NumField(); i++ { + field := t.Field(i) + + key := field.Name + if value, ok := in[strings.ToLower(key)]; ok { + if valueAsMap, ok := value.(map[string]interface{}); ok { + fixCaseOut[key] = fixCase(valueAsMap, field.Type) + } else { + fixCaseOut[key] = value + } + } + } + + return fixCaseOut + } + + out = fixCase(in, reflect.TypeOf(model.Config{})) + + return +} diff --git a/config/config_test.go b/config/unmarshal_test.go similarity index 50% rename from config/config_test.go rename to config/unmarshal_test.go index 7be4dfaa7f..66cd4ec3e9 100644 --- a/config/config_test.go +++ b/config/unmarshal_test.go @@ -5,7 +5,6 @@ package config import ( "bytes" - "fmt" "os" "strings" "testing" @@ -17,29 +16,41 @@ import ( "github.com/mattermost/mattermost-server/utils" ) -func TestConfig(t *testing.T) { - utils.TranslationsPreInit() - _, _, _, err := LoadConfig("config.json") - require.Nil(t, err) +func TestGetDefaultsFromStruct(t *testing.T) { + s := struct { + TestSettings struct { + IntValue int + BoolValue bool + StringValue string + } + PointerToTestSettings *struct { + Value int + } + }{} + + defaults := getDefaultsFromStruct(s) + + assert.Equal(t, defaults["TestSettings.IntValue"], 0) + assert.Equal(t, defaults["TestSettings.BoolValue"], false) + assert.Equal(t, defaults["TestSettings.StringValue"], "") + assert.Equal(t, defaults["PointerToTestSettings.Value"], 0) + assert.NotContains(t, defaults, "PointerToTestSettings") + assert.Len(t, defaults, 4) } -func TestReadConfig(t *testing.T) { - utils.TranslationsPreInit() - - _, _, err := ReadConfig(bytes.NewReader([]byte(``)), false) +func TestUnmarshalConfig(t *testing.T) { + _, _, err := unmarshalConfig(bytes.NewReader([]byte(``)), false) require.EqualError(t, err, "parsing error at line 1, character 1: unexpected end of JSON input") - _, _, err = ReadConfig(bytes.NewReader([]byte(` + _, _, err = unmarshalConfig(bytes.NewReader([]byte(` { malformed `)), false) require.EqualError(t, err, "parsing error at line 3, character 5: invalid character 'm' looking for beginning of object key string") } -func TestReadConfig_PluginSettings(t *testing.T) { - utils.TranslationsPreInit() - - config, _, err := ReadConfig(bytes.NewReader([]byte(`{ +func TestUnmarshalConfig_PluginSettings(t *testing.T) { + config, _, err := unmarshalConfig(bytes.NewReader([]byte(`{ "PluginSettings": { "Directory": "/temp/mattermost-plugins", "Plugins": { @@ -111,29 +122,7 @@ func TestReadConfig_PluginSettings(t *testing.T) { } } -func TestReadConfig_ImageProxySettings(t *testing.T) { - utils.TranslationsPreInit() - - t.Run("deprecated settings should still be read properly", func(t *testing.T) { - config, _, err := ReadConfig(bytes.NewReader([]byte(`{ - "ServiceSettings": { - "ImageProxyType": "OldImageProxyType", - "ImageProxyURL": "OldImageProxyURL", - "ImageProxyOptions": "OldImageProxyOptions" - } - }`)), false) - - require.Nil(t, err) - - assert.Equal(t, model.NewString("OldImageProxyType"), config.ServiceSettings.DEPRECATED_DO_NOT_USE_ImageProxyType) - assert.Equal(t, model.NewString("OldImageProxyURL"), config.ServiceSettings.DEPRECATED_DO_NOT_USE_ImageProxyURL) - assert.Equal(t, model.NewString("OldImageProxyOptions"), config.ServiceSettings.DEPRECATED_DO_NOT_USE_ImageProxyOptions) - }) -} - func TestConfigFromEnviroVars(t *testing.T) { - utils.TranslationsPreInit() - config := `{ "ServiceSettings": { "EnableCommands": true, @@ -166,16 +155,11 @@ func TestConfigFromEnviroVars(t *testing.T) { os.Setenv("MM_TEAMSETTINGS_SITENAME", "From Environment") os.Setenv("MM_TEAMSETTINGS_CUSTOMBRANDTEXT", "Custom Brand") - cfg, envCfg, err := ReadConfig(strings.NewReader(config), true) + cfg, envCfg, err := unmarshalConfig(strings.NewReader(config), true) require.Nil(t, err) - if *cfg.TeamSettings.SiteName != "From Environment" { - t.Fatal("Couldn't read config from environment var") - } - - if *cfg.TeamSettings.CustomBrandText != "Custom Brand" { - t.Fatal("Couldn't read config from environment var") - } + assert.Equal(t, "From Environment", *cfg.TeamSettings.SiteName) + assert.Equal(t, "Custom Brand", *cfg.TeamSettings.CustomBrandText) if teamSettings, ok := envCfg["TeamSettings"]; !ok { t.Fatal("TeamSettings is missing from envConfig") @@ -194,12 +178,10 @@ func TestConfigFromEnviroVars(t *testing.T) { os.Unsetenv("MM_TEAMSETTINGS_SITENAME") os.Unsetenv("MM_TEAMSETTINGS_CUSTOMBRANDTEXT") - cfg, envCfg, err = ReadConfig(strings.NewReader(config), true) + cfg, envCfg, err = unmarshalConfig(strings.NewReader(config), true) require.Nil(t, err) - if *cfg.TeamSettings.SiteName != "Mattermost" { - t.Fatal("should have been reset") - } + assert.Equal(t, "Mattermost", *cfg.TeamSettings.SiteName) if _, ok := envCfg["TeamSettings"]; ok { t.Fatal("TeamSettings should be missing from envConfig") @@ -210,7 +192,7 @@ func TestConfigFromEnviroVars(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_ENABLECOMMANDS", "false") defer os.Unsetenv("MM_SERVICESETTINGS_ENABLECOMMANDS") - cfg, envCfg, err := ReadConfig(strings.NewReader(config), true) + cfg, envCfg, err := unmarshalConfig(strings.NewReader(config), true) require.Nil(t, err) if *cfg.ServiceSettings.EnableCommands { @@ -232,12 +214,10 @@ func TestConfigFromEnviroVars(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_READTIMEOUT", "400") defer os.Unsetenv("MM_SERVICESETTINGS_READTIMEOUT") - cfg, envCfg, err := ReadConfig(strings.NewReader(config), true) + cfg, envCfg, err := unmarshalConfig(strings.NewReader(config), true) require.Nil(t, err) - if *cfg.ServiceSettings.ReadTimeout != 400 { - t.Fatal("Couldn't read config from environment var") - } + assert.Equal(t, 400, *cfg.ServiceSettings.ReadTimeout) if serviceSettings, ok := envCfg["ServiceSettings"]; !ok { t.Fatal("ServiceSettings is missing from envConfig") @@ -254,12 +234,10 @@ func TestConfigFromEnviroVars(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_SITEURL", "https://example.com") defer os.Unsetenv("MM_SERVICESETTINGS_SITEURL") - cfg, envCfg, err := ReadConfig(strings.NewReader(config), true) + cfg, envCfg, err := unmarshalConfig(strings.NewReader(config), true) require.Nil(t, err) - if *cfg.ServiceSettings.SiteURL != "https://example.com" { - t.Fatal("Couldn't read config from environment var") - } + assert.Equal(t, "https://example.com", *cfg.ServiceSettings.SiteURL) if serviceSettings, ok := envCfg["ServiceSettings"]; !ok { t.Fatal("ServiceSettings is missing from envConfig") @@ -276,12 +254,10 @@ func TestConfigFromEnviroVars(t *testing.T) { os.Setenv("MM_SUPPORTSETTINGS_TERMSOFSERVICELINK", "") defer os.Unsetenv("MM_SUPPORTSETTINGS_TERMSOFSERVICELINK") - cfg, envCfg, err := ReadConfig(strings.NewReader(config), true) + cfg, envCfg, err := unmarshalConfig(strings.NewReader(config), true) require.Nil(t, err) - if *cfg.SupportSettings.TermsOfServiceLink != "" { - t.Fatal("Couldn't read empty TermsOfServiceLink from environment var") - } + assert.Empty(t, *cfg.SupportSettings.TermsOfServiceLink) if supportSettings, ok := envCfg["SupportSettings"]; !ok { t.Fatal("SupportSettings is missing from envConfig") @@ -302,7 +278,7 @@ func TestConfigFromEnviroVars(t *testing.T) { defer os.Unsetenv("MM_PLUGINSETTINGS_DIRECTORY") defer os.Unsetenv("MM_PLUGINSETTINGS_CLIENTDIRECTORY") - cfg, envCfg, err := ReadConfig(strings.NewReader(config), true) + cfg, envCfg, err := unmarshalConfig(strings.NewReader(config), true) require.Nil(t, err) assert.Equal(t, false, *cfg.PluginSettings.Enable) @@ -331,7 +307,7 @@ func TestConfigFromEnviroVars(t *testing.T) { defer os.Unsetenv("MM_PLUGINSETTINGS_PLUGINS_JIRA_SECRET") defer os.Unsetenv("MM_PLUGINSETTINGS_PLUGINSTATES_JIRA_ENABLE") - cfg, envCfg, err := ReadConfig(strings.NewReader(config), true) + cfg, envCfg, err := unmarshalConfig(strings.NewReader(config), true) require.Nil(t, err) if pluginsJira, ok := cfg.PluginSettings.Plugins["jira"]; !ok { @@ -376,325 +352,22 @@ func TestConfigFromEnviroVars(t *testing.T) { }) } -func TestValidateLocales(t *testing.T) { +func TestReadConfig_ImageProxySettings(t *testing.T) { utils.TranslationsPreInit() - cfg, _, _, err := LoadConfig("config.json") - require.Nil(t, err) - *cfg.LocalizationSettings.DefaultServerLocale = "en" - *cfg.LocalizationSettings.DefaultClientLocale = "en" - *cfg.LocalizationSettings.AvailableLocales = "" - - // t.Logf("*cfg.LocalizationSettings.DefaultClientLocale: %+v", *cfg.LocalizationSettings.DefaultClientLocale) - if err := ValidateLocales(cfg); err != nil { - t.Fatal("Should have not returned an error") - } - - // validate DefaultServerLocale - *cfg.LocalizationSettings.DefaultServerLocale = "junk" - if err := ValidateLocales(cfg); err != nil { - if *cfg.LocalizationSettings.DefaultServerLocale != "en" { - t.Fatal("DefaultServerLocale should have assigned to en as a default value") - } - } else { - t.Fatal("Should have returned an error validating DefaultServerLocale") - } - - *cfg.LocalizationSettings.DefaultServerLocale = "" - if err := ValidateLocales(cfg); err != nil { - if *cfg.LocalizationSettings.DefaultServerLocale != "en" { - t.Fatal("DefaultServerLocale should have assigned to en as a default value") - } - } else { - t.Fatal("Should have returned an error validating DefaultServerLocale") - } - - *cfg.LocalizationSettings.AvailableLocales = "en" - *cfg.LocalizationSettings.DefaultServerLocale = "de" - if err := ValidateLocales(cfg); err != nil { - if strings.Contains(*cfg.LocalizationSettings.AvailableLocales, *cfg.LocalizationSettings.DefaultServerLocale) { - t.Fatal("DefaultServerLocale should not be added to AvailableLocales") - } - t.Fatal("Should have not returned an error validating DefaultServerLocale") - } - - // validate DefaultClientLocale - *cfg.LocalizationSettings.AvailableLocales = "" - *cfg.LocalizationSettings.DefaultClientLocale = "junk" - if err := ValidateLocales(cfg); err != nil { - if *cfg.LocalizationSettings.DefaultClientLocale != "en" { - t.Fatal("DefaultClientLocale should have assigned to en as a default value") - } - } else { - - t.Fatal("Should have returned an error validating DefaultClientLocale") - } - - *cfg.LocalizationSettings.DefaultClientLocale = "" - if err := ValidateLocales(cfg); err != nil { - if *cfg.LocalizationSettings.DefaultClientLocale != "en" { - t.Fatal("DefaultClientLocale should have assigned to en as a default value") - } - } else { - t.Fatal("Should have returned an error validating DefaultClientLocale") - } - - *cfg.LocalizationSettings.AvailableLocales = "en" - *cfg.LocalizationSettings.DefaultClientLocale = "de" - if err := ValidateLocales(cfg); err != nil { - if !strings.Contains(*cfg.LocalizationSettings.AvailableLocales, *cfg.LocalizationSettings.DefaultClientLocale) { - t.Fatal("DefaultClientLocale should have added to AvailableLocales") - } - } else { - t.Fatal("Should have returned an error validating DefaultClientLocale") - } - - // validate AvailableLocales - *cfg.LocalizationSettings.DefaultServerLocale = "en" - *cfg.LocalizationSettings.DefaultClientLocale = "en" - *cfg.LocalizationSettings.AvailableLocales = "junk" - if err := ValidateLocales(cfg); err != nil { - if *cfg.LocalizationSettings.AvailableLocales != "" { - t.Fatal("AvailableLocales should have assigned to empty string as a default value") - } - } else { - t.Fatal("Should have returned an error validating AvailableLocales") - } - - *cfg.LocalizationSettings.AvailableLocales = "en,de,junk" - if err := ValidateLocales(cfg); err != nil { - if *cfg.LocalizationSettings.AvailableLocales != "" { - t.Fatal("AvailableLocales should have assigned to empty string as a default value") - } - } else { - t.Fatal("Should have returned an error validating AvailableLocales") - } - - *cfg.LocalizationSettings.DefaultServerLocale = "fr" - *cfg.LocalizationSettings.DefaultClientLocale = "de" - *cfg.LocalizationSettings.AvailableLocales = "en" - if err := ValidateLocales(cfg); err != nil { - if strings.Contains(*cfg.LocalizationSettings.AvailableLocales, *cfg.LocalizationSettings.DefaultServerLocale) { - t.Fatal("DefaultServerLocale should not be added to AvailableLocales") - } - if !strings.Contains(*cfg.LocalizationSettings.AvailableLocales, *cfg.LocalizationSettings.DefaultClientLocale) { - t.Fatal("DefaultClientLocale should have added to AvailableLocales") - } - } else { - t.Fatal("Should have returned an error validating AvailableLocales") - } -} - -func TestGetClientConfig(t *testing.T) { - t.Parallel() - testCases := []struct { - description string - config *model.Config - diagnosticId string - license *model.License - expectedFields map[string]string - }{ - { - "unlicensed", - &model.Config{ - EmailSettings: model.EmailSettings{ - EmailNotificationContentsType: sToP(model.EMAIL_NOTIFICATION_CONTENTS_FULL), - }, - ThemeSettings: model.ThemeSettings{ - // Ignored, since not licensed. - AllowCustomThemes: bToP(false), - }, - ServiceSettings: model.ServiceSettings{ - WebsocketURL: sToP("ws://mattermost.example.com:8065"), - WebsocketPort: iToP(80), - WebsocketSecurePort: iToP(443), - }, - }, - "", - nil, - map[string]string{ - "DiagnosticId": "", - "EmailNotificationContentsType": "full", - "AllowCustomThemes": "true", - "EnforceMultifactorAuthentication": "false", - "WebsocketURL": "ws://mattermost.example.com:8065", - "WebsocketPort": "80", - "WebsocketSecurePort": "443", - }, - }, - { - "licensed, but not for theme management", - &model.Config{ - EmailSettings: model.EmailSettings{ - EmailNotificationContentsType: sToP(model.EMAIL_NOTIFICATION_CONTENTS_FULL), - }, - ThemeSettings: model.ThemeSettings{ - // Ignored, since not licensed. - AllowCustomThemes: bToP(false), - }, - }, - "tag1", - &model.License{ - Features: &model.Features{ - ThemeManagement: bToP(false), - }, - }, - map[string]string{ - "DiagnosticId": "tag1", - "EmailNotificationContentsType": "full", - "AllowCustomThemes": "true", - }, - }, - { - "licensed for theme management", - &model.Config{ - EmailSettings: model.EmailSettings{ - EmailNotificationContentsType: sToP(model.EMAIL_NOTIFICATION_CONTENTS_FULL), - }, - ThemeSettings: model.ThemeSettings{ - AllowCustomThemes: bToP(false), - }, - }, - "tag2", - &model.License{ - Features: &model.Features{ - ThemeManagement: bToP(true), - }, - }, - map[string]string{ - "DiagnosticId": "tag2", - "EmailNotificationContentsType": "full", - "AllowCustomThemes": "false", - }, - }, - { - "licensed for enforcement", - &model.Config{ - ServiceSettings: model.ServiceSettings{ - EnforceMultifactorAuthentication: bToP(true), - }, - }, - "tag1", - &model.License{ - Features: &model.Features{ - MFA: bToP(true), - }, - }, - map[string]string{ - "EnforceMultifactorAuthentication": "true", - }, - }, - } - - for _, testCase := range testCases { - testCase := testCase - t.Run(testCase.description, func(t *testing.T) { - t.Parallel() - - testCase.config.SetDefaults() - if testCase.license != nil { - testCase.license.Features.SetDefaults() + t.Run("deprecated settings should still be read properly", func(t *testing.T) { + config, _, err := unmarshalConfig(bytes.NewReader([]byte(`{ + "ServiceSettings": { + "ImageProxyType": "OldImageProxyType", + "ImageProxyURL": "OldImageProxyURL", + "ImageProxyOptions": "OldImageProxyOptions" } + }`)), false) - configMap := GenerateClientConfig(testCase.config, testCase.diagnosticId, testCase.license) - for expectedField, expectedValue := range testCase.expectedFields { - actualValue, ok := configMap[expectedField] - if assert.True(t, ok, fmt.Sprintf("config does not contain %v", expectedField)) { - assert.Equal(t, expectedValue, actualValue) - } - } - }) - } -} - -func TestGetLimitedClientConfig(t *testing.T) { - t.Parallel() - testCases := []struct { - description string - config *model.Config - diagnosticId string - license *model.License - expectedFields map[string]string - }{ - { - "unlicensed", - &model.Config{ - EmailSettings: model.EmailSettings{ - EmailNotificationContentsType: sToP(model.EMAIL_NOTIFICATION_CONTENTS_FULL), - }, - ThemeSettings: model.ThemeSettings{ - // Ignored, since not licensed. - AllowCustomThemes: bToP(false), - }, - ServiceSettings: model.ServiceSettings{ - WebsocketURL: sToP("ws://mattermost.example.com:8065"), - WebsocketPort: iToP(80), - WebsocketSecurePort: iToP(443), - }, - }, - "", - nil, - map[string]string{ - "DiagnosticId": "", - "EnforceMultifactorAuthentication": "false", - "WebsocketURL": "ws://mattermost.example.com:8065", - "WebsocketPort": "80", - "WebsocketSecurePort": "443", - }, - }, - } - - for _, testCase := range testCases { - testCase := testCase - t.Run(testCase.description, func(t *testing.T) { - t.Parallel() - - testCase.config.SetDefaults() - if testCase.license != nil { - testCase.license.Features.SetDefaults() - } - - configMap := GenerateLimitedClientConfig(testCase.config, testCase.diagnosticId, testCase.license) - for expectedField, expectedValue := range testCase.expectedFields { - actualValue, ok := configMap[expectedField] - if assert.True(t, ok, fmt.Sprintf("config does not contain %v", expectedField)) { - assert.Equal(t, expectedValue, actualValue) - } - } - }) - } -} - -func sToP(s string) *string { - return &s -} - -func bToP(b bool) *bool { - return &b -} - -func iToP(i int) *int { - return &i -} - -func TestGetDefaultsFromStruct(t *testing.T) { - s := struct { - TestSettings struct { - IntValue int - BoolValue bool - StringValue string - } - PointerToTestSettings *struct { - Value int - } - }{} - - defaults := getDefaultsFromStruct(s) - - assert.Equal(t, defaults["TestSettings.IntValue"], 0) - assert.Equal(t, defaults["TestSettings.BoolValue"], false) - assert.Equal(t, defaults["TestSettings.StringValue"], "") - assert.Equal(t, defaults["PointerToTestSettings.Value"], 0) - assert.NotContains(t, defaults, "PointerToTestSettings") - assert.Len(t, defaults, 4) + require.Nil(t, err) + + assert.Equal(t, model.NewString("OldImageProxyType"), config.ServiceSettings.DEPRECATED_DO_NOT_USE_ImageProxyType) + assert.Equal(t, model.NewString("OldImageProxyURL"), config.ServiceSettings.DEPRECATED_DO_NOT_USE_ImageProxyURL) + assert.Equal(t, model.NewString("OldImageProxyOptions"), config.ServiceSettings.DEPRECATED_DO_NOT_USE_ImageProxyOptions) + }) } diff --git a/config/utils.go b/config/utils.go new file mode 100644 index 0000000000..704cc80514 --- /dev/null +++ b/config/utils.go @@ -0,0 +1,133 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package config + +import ( + "strings" + + "github.com/mattermost/mattermost-server/mlog" + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils" +) + +// desanitize replaces fake settings with their actual values. +func desanitize(actual, target *model.Config) { + if target.LdapSettings.BindPassword != nil && *target.LdapSettings.BindPassword == model.FAKE_SETTING { + *target.LdapSettings.BindPassword = *actual.LdapSettings.BindPassword + } + + if *target.FileSettings.PublicLinkSalt == model.FAKE_SETTING { + *target.FileSettings.PublicLinkSalt = *actual.FileSettings.PublicLinkSalt + } + if *target.FileSettings.AmazonS3SecretAccessKey == model.FAKE_SETTING { + target.FileSettings.AmazonS3SecretAccessKey = actual.FileSettings.AmazonS3SecretAccessKey + } + + if *target.EmailSettings.InviteSalt == model.FAKE_SETTING { + target.EmailSettings.InviteSalt = actual.EmailSettings.InviteSalt + } + if *target.EmailSettings.SMTPPassword == model.FAKE_SETTING { + target.EmailSettings.SMTPPassword = actual.EmailSettings.SMTPPassword + } + + if *target.GitLabSettings.Secret == model.FAKE_SETTING { + target.GitLabSettings.Secret = actual.GitLabSettings.Secret + } + + if *target.SqlSettings.DataSource == model.FAKE_SETTING { + *target.SqlSettings.DataSource = *actual.SqlSettings.DataSource + } + if *target.SqlSettings.AtRestEncryptKey == model.FAKE_SETTING { + target.SqlSettings.AtRestEncryptKey = actual.SqlSettings.AtRestEncryptKey + } + + if *target.ElasticsearchSettings.Password == model.FAKE_SETTING { + *target.ElasticsearchSettings.Password = *actual.ElasticsearchSettings.Password + } + + target.SqlSettings.DataSourceReplicas = make([]string, len(actual.SqlSettings.DataSourceReplicas)) + for i := range target.SqlSettings.DataSourceReplicas { + target.SqlSettings.DataSourceReplicas[i] = actual.SqlSettings.DataSourceReplicas[i] + } + + target.SqlSettings.DataSourceSearchReplicas = make([]string, len(actual.SqlSettings.DataSourceReplicas)) + for i := range target.SqlSettings.DataSourceSearchReplicas { + target.SqlSettings.DataSourceSearchReplicas[i] = actual.SqlSettings.DataSourceSearchReplicas[i] + } +} + +// fixConfig patches invalid or missing data in the configuration, returning true if changed. +func fixConfig(cfg *model.Config) bool { + changed := false + + // Ensure SiteURL has no trailing slash. + if strings.HasSuffix(*cfg.ServiceSettings.SiteURL, "/") { + *cfg.ServiceSettings.SiteURL = strings.TrimRight(*cfg.ServiceSettings.SiteURL, "/") + changed = true + } + + // Ensure the directory for a local file store has a trailing slash. + if *cfg.FileSettings.DriverName == model.IMAGE_DRIVER_LOCAL { + if !strings.HasSuffix(*cfg.FileSettings.Directory, "/") { + *cfg.FileSettings.Directory += "/" + changed = true + } + } + + if FixInvalidLocales(cfg) { + changed = true + } + + return changed +} + +// FixInvalidLocales checks and corrects the given config for invalid locale-related settings. +// +// Ideally, this function would be completely internal, but it's currently exposed to allow the cli +// to test the config change before allowing the save. +func FixInvalidLocales(cfg *model.Config) bool { + var changed bool + + locales := utils.GetSupportedLocales() + if _, ok := locales[*cfg.LocalizationSettings.DefaultServerLocale]; !ok { + *cfg.LocalizationSettings.DefaultServerLocale = model.DEFAULT_LOCALE + mlog.Warn("DefaultServerLocale must be one of the supported locales. Setting DefaultServerLocale to en as default value.") + changed = true + } + + if _, ok := locales[*cfg.LocalizationSettings.DefaultClientLocale]; !ok { + *cfg.LocalizationSettings.DefaultClientLocale = model.DEFAULT_LOCALE + mlog.Warn("DefaultClientLocale must be one of the supported locales. Setting DefaultClientLocale to en as default value.") + changed = true + } + + if len(*cfg.LocalizationSettings.AvailableLocales) > 0 { + isDefaultClientLocaleInAvailableLocales := false + for _, word := range strings.Split(*cfg.LocalizationSettings.AvailableLocales, ",") { + if _, ok := locales[word]; !ok { + *cfg.LocalizationSettings.AvailableLocales = "" + isDefaultClientLocaleInAvailableLocales = true + mlog.Warn("AvailableLocales must include DefaultClientLocale. Setting AvailableLocales to all locales as default value.") + changed = true + break + } + + if word == *cfg.LocalizationSettings.DefaultClientLocale { + isDefaultClientLocaleInAvailableLocales = true + } + } + + availableLocales := *cfg.LocalizationSettings.AvailableLocales + + if !isDefaultClientLocaleInAvailableLocales { + availableLocales += "," + *cfg.LocalizationSettings.DefaultClientLocale + mlog.Warn("Adding DefaultClientLocale to AvailableLocales.") + changed = true + } + + *cfg.LocalizationSettings.AvailableLocales = strings.Join(utils.RemoveDuplicatesFromStringArray(strings.Split(availableLocales, ",")), ",") + } + + return changed +} diff --git a/config/utils_test.go b/config/utils_test.go new file mode 100644 index 0000000000..ed56b65c26 --- /dev/null +++ b/config/utils_test.go @@ -0,0 +1,158 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package config + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils" +) + +func TestDesanitize(t *testing.T) { + actual := &model.Config{} + actual.SetDefaults() + + // These setting should be ignored + actual.LdapSettings.Enable = bToP(false) + actual.FileSettings.DriverName = sToP("s3") + + // These settings should be desanitized into target. + actual.LdapSettings.BindPassword = sToP("bind_password") + actual.FileSettings.PublicLinkSalt = sToP("public_link_salt") + actual.FileSettings.AmazonS3SecretAccessKey = sToP("amazon_s3_secret_access_key") + actual.EmailSettings.InviteSalt = sToP("invite_salt") + actual.EmailSettings.SMTPPassword = sToP("smtp_password") + actual.GitLabSettings.Secret = sToP("secret") + actual.SqlSettings.DataSource = sToP("data_source") + actual.SqlSettings.AtRestEncryptKey = sToP("at_rest_encrypt_key") + actual.ElasticsearchSettings.Password = sToP("password") + actual.SqlSettings.DataSourceReplicas = append(actual.SqlSettings.DataSourceReplicas, "replica0") + actual.SqlSettings.DataSourceReplicas = append(actual.SqlSettings.DataSourceReplicas, "replica1") + actual.SqlSettings.DataSourceSearchReplicas = append(actual.SqlSettings.DataSourceSearchReplicas, "search_replica0") + actual.SqlSettings.DataSourceSearchReplicas = append(actual.SqlSettings.DataSourceSearchReplicas, "search_replica1") + + target := &model.Config{} + target.SetDefaults() + + // These setting should be ignored + target.LdapSettings.Enable = bToP(true) + target.FileSettings.DriverName = sToP("file") + + // These settings should be updated from actual + target.LdapSettings.BindPassword = sToP(model.FAKE_SETTING) + target.FileSettings.PublicLinkSalt = sToP(model.FAKE_SETTING) + target.FileSettings.AmazonS3SecretAccessKey = sToP(model.FAKE_SETTING) + target.EmailSettings.InviteSalt = sToP(model.FAKE_SETTING) + target.EmailSettings.SMTPPassword = sToP(model.FAKE_SETTING) + target.GitLabSettings.Secret = sToP(model.FAKE_SETTING) + target.SqlSettings.DataSource = sToP(model.FAKE_SETTING) + target.SqlSettings.AtRestEncryptKey = sToP(model.FAKE_SETTING) + target.ElasticsearchSettings.Password = sToP(model.FAKE_SETTING) + target.SqlSettings.DataSourceReplicas = append(target.SqlSettings.DataSourceReplicas, "old_replica0") + target.SqlSettings.DataSourceSearchReplicas = append(target.SqlSettings.DataSourceReplicas, "old_search_replica0") + + actual_clone := actual.Clone() + desanitize(actual, target) + assert.Equal(t, actual_clone, actual, "actual should not have been changed") + + // Verify the settings that should have been left untouched in target + assert.True(t, *target.LdapSettings.Enable, "LdapSettings.Enable should not have changed") + assert.Equal(t, "file", *target.FileSettings.DriverName, "FileSettings.DriverName should not have been changed") + + // Verify the settings that should have been desanitized into target + assert.Equal(t, *actual.LdapSettings.BindPassword, *target.LdapSettings.BindPassword) + assert.Equal(t, *actual.FileSettings.PublicLinkSalt, *target.FileSettings.PublicLinkSalt) + assert.Equal(t, *actual.FileSettings.AmazonS3SecretAccessKey, *target.FileSettings.AmazonS3SecretAccessKey) + assert.Equal(t, *actual.EmailSettings.InviteSalt, *target.EmailSettings.InviteSalt) + assert.Equal(t, *actual.EmailSettings.SMTPPassword, *target.EmailSettings.SMTPPassword) + assert.Equal(t, *actual.GitLabSettings.Secret, *target.GitLabSettings.Secret) + assert.Equal(t, *actual.SqlSettings.DataSource, *target.SqlSettings.DataSource) + assert.Equal(t, *actual.SqlSettings.AtRestEncryptKey, *target.SqlSettings.AtRestEncryptKey) + assert.Equal(t, *actual.ElasticsearchSettings.Password, *target.ElasticsearchSettings.Password) + assert.Equal(t, actual.SqlSettings.DataSourceReplicas, target.SqlSettings.DataSourceReplicas) + assert.Equal(t, actual.SqlSettings.DataSourceSearchReplicas, target.SqlSettings.DataSourceSearchReplicas) +} + +func TestFixInvalidLocales(t *testing.T) { + utils.TranslationsPreInit() + + cfg := &model.Config{} + cfg.SetDefaults() + + *cfg.LocalizationSettings.DefaultServerLocale = "en" + *cfg.LocalizationSettings.DefaultClientLocale = "en" + *cfg.LocalizationSettings.AvailableLocales = "" + + changed := FixInvalidLocales(cfg) + assert.False(t, changed) + + *cfg.LocalizationSettings.DefaultServerLocale = "junk" + changed = FixInvalidLocales(cfg) + assert.True(t, changed) + assert.Equal(t, "en", *cfg.LocalizationSettings.DefaultServerLocale) + + *cfg.LocalizationSettings.DefaultServerLocale = "" + changed = FixInvalidLocales(cfg) + assert.True(t, changed) + assert.Equal(t, "en", *cfg.LocalizationSettings.DefaultServerLocale) + + *cfg.LocalizationSettings.AvailableLocales = "en" + *cfg.LocalizationSettings.DefaultServerLocale = "de" + changed = FixInvalidLocales(cfg) + assert.False(t, changed) + assert.NotContains(t, *cfg.LocalizationSettings.AvailableLocales, *cfg.LocalizationSettings.DefaultServerLocale, "DefaultServerLocale should not be added to AvailableLocales") + + *cfg.LocalizationSettings.AvailableLocales = "" + *cfg.LocalizationSettings.DefaultClientLocale = "junk" + changed = FixInvalidLocales(cfg) + assert.True(t, changed) + assert.Equal(t, "en", *cfg.LocalizationSettings.DefaultClientLocale) + + *cfg.LocalizationSettings.DefaultClientLocale = "" + changed = FixInvalidLocales(cfg) + assert.True(t, changed) + assert.Equal(t, "en", *cfg.LocalizationSettings.DefaultClientLocale) + + *cfg.LocalizationSettings.AvailableLocales = "en" + *cfg.LocalizationSettings.DefaultClientLocale = "de" + changed = FixInvalidLocales(cfg) + assert.True(t, changed) + assert.Contains(t, *cfg.LocalizationSettings.AvailableLocales, *cfg.LocalizationSettings.DefaultServerLocale, "DefaultClientLocale should have been added to AvailableLocales") + + // validate AvailableLocales + *cfg.LocalizationSettings.DefaultServerLocale = "en" + *cfg.LocalizationSettings.DefaultClientLocale = "en" + *cfg.LocalizationSettings.AvailableLocales = "junk" + changed = FixInvalidLocales(cfg) + assert.True(t, changed) + assert.Equal(t, "", *cfg.LocalizationSettings.AvailableLocales) + + *cfg.LocalizationSettings.AvailableLocales = "en,de,junk" + changed = FixInvalidLocales(cfg) + assert.True(t, changed) + assert.Equal(t, "", *cfg.LocalizationSettings.AvailableLocales) + + *cfg.LocalizationSettings.DefaultServerLocale = "fr" + *cfg.LocalizationSettings.DefaultClientLocale = "de" + *cfg.LocalizationSettings.AvailableLocales = "en" + changed = FixInvalidLocales(cfg) + assert.True(t, changed) + assert.NotContains(t, *cfg.LocalizationSettings.AvailableLocales, *cfg.LocalizationSettings.DefaultServerLocale, "DefaultServerLocale should not be added to AvailableLocales") + assert.Contains(t, *cfg.LocalizationSettings.AvailableLocales, *cfg.LocalizationSettings.DefaultClientLocale, "DefaultClientLocale should have been added to AvailableLocales") +} + +func sToP(s string) *string { + return &s +} + +func bToP(b bool) *bool { + return &b +} + +func iToP(i int) *int { + return &i +} diff --git a/config/watcher.go b/config/watcher.go new file mode 100644 index 0000000000..87bc78d0a8 --- /dev/null +++ b/config/watcher.go @@ -0,0 +1,82 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package config + +import ( + "path/filepath" + + "github.com/fsnotify/fsnotify" + "github.com/pkg/errors" + + "github.com/mattermost/mattermost-server/mlog" +) + +// watcher monitors a file for changes +type watcher struct { + emitter + + fsWatcher *fsnotify.Watcher + close chan struct{} + closed chan struct{} +} + +// newWatcher creates a new instance of watcher to monitor for file changes. +func newWatcher(path string, callback func()) (w *watcher, err error) { + fsWatcher, err := fsnotify.NewWatcher() + if err != nil { + return nil, errors.Wrapf(err, "failed to create fsnotify watcher for %s", path) + } + + path = filepath.Clean(path) + + // Watch the entire containing directory. + configDir, _ := filepath.Split(path) + if err := fsWatcher.Add(configDir); err != nil { + if closeErr := fsWatcher.Close(); closeErr != nil { + mlog.Error("failed to stop fsnotify watcher for %s", mlog.String("path", path), mlog.Err(closeErr)) + } + return nil, errors.Wrapf(err, "failed to watch directory %s", configDir) + } + + w = &watcher{ + fsWatcher: fsWatcher, + close: make(chan struct{}), + closed: make(chan struct{}), + } + + go func() { + defer close(w.closed) + defer func() { + if err := fsWatcher.Close(); err != nil { + mlog.Error("failed to stop fsnotify watcher for %s", mlog.String("path", path)) + } + }() + + for { + select { + case event := <-fsWatcher.Events: + // We only care about the given file. + if filepath.Clean(event.Name) == path { + if event.Op&fsnotify.Write == fsnotify.Write || event.Op&fsnotify.Create == fsnotify.Create { + mlog.Info("Config file watcher detected a change", mlog.String("path", path)) + go callback() + } + } + case err := <-fsWatcher.Errors: + mlog.Error("Failed while watching config file", mlog.String("path", path), mlog.Err(err)) + case <-w.close: + return + } + } + }() + + return w, nil +} + +func (w *watcher) Close() error { + close(w.close) + <-w.closed + + return nil +} diff --git a/config/watcher_test.go b/config/watcher_test.go new file mode 100644 index 0000000000..40c925fc32 --- /dev/null +++ b/config/watcher_test.go @@ -0,0 +1,63 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package config + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestWatcherInvalidDirectory(t *testing.T) { + if testing.Short() { + t.Skip("skipping watcher test in short mode") + } + + callback := func() {} + _, err := newWatcher("/does/not/exist", callback) + require.Error(t, err, "should have failed to watch a non-existent directory") +} + +func TestWatcher(t *testing.T) { + if testing.Short() { + t.Skip("skipping watcher test in short mode") + } + + tempDir, err := ioutil.TempDir("", "TestWatcher") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + + f, err := ioutil.TempFile(tempDir, "TestWatcher") + require.NoError(t, err) + defer f.Close() + defer os.Remove(f.Name()) + + called := make(chan bool) + callback := func() { + called <- true + } + watcher, err := newWatcher(f.Name(), callback) + require.NoError(t, err) + defer watcher.Close() + + // Write to a different file + ioutil.WriteFile(filepath.Join(tempDir, "unrelated"), []byte("data"), 0644) + select { + case <-called: + t.Fatal("callback should not have been called for unrelated file") + case <-time.After(1 * time.Second): + } + + // Write to the watched file + ioutil.WriteFile(f.Name(), []byte("data"), 0644) + select { + case <-called: + case <-time.After(5 * time.Second): + t.Fatal("callback should have been called when file written") + } +} diff --git a/einterfaces/ldap.go b/einterfaces/ldap.go index f996d5888c..b7dc18a9b8 100644 --- a/einterfaces/ldap.go +++ b/einterfaces/ldap.go @@ -14,7 +14,6 @@ type LdapInterface interface { CheckPassword(id string, password string) *model.AppError CheckPasswordAuthData(authData string, password string) *model.AppError SwitchToLdap(userId, ldapId, ldapPassword string) *model.AppError - ValidateFilter(filter string) *model.AppError StartSynchronizeJob(waitForJobToFinish bool) (*model.Job, *model.AppError) RunTest() *model.AppError GetAllLdapUsers() ([]*model.User, *model.AppError) diff --git a/i18n/en.json b/i18n/en.json index 000c257e18..60048b8c5f 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -3182,6 +3182,10 @@ "id": "app.notification.subject.notification.full", "translation": "[{{ .SiteName }}] Notification in {{ .TeamName}} on {{.Month}} {{.Day}}, {{.Year}}" }, + { + "id": "app.save_config.app_error", + "translation": "An error occurred saving the configuration" + }, { "id": "app.plugin.cluster.save_config.app_error", "translation": "The plugin configuration in your config.json file must be updated manually when using ReadOnlyConfig with clustering enabled." diff --git a/model/config.go b/model/config.go index d7b9626d8d..3a3f282b67 100644 --- a/model/config.go +++ b/model/config.go @@ -16,6 +16,8 @@ import ( "strconv" "strings" "time" + + "github.com/go-ldap/ldap" ) const ( @@ -2529,6 +2531,12 @@ func (ls *LdapSettings) isValid() *AppError { if *ls.LoginIdAttribute == "" { return NewAppError("Config.IsValid", "model.config.is_valid.ldap_login_id", nil, "", http.StatusBadRequest) } + + if *ls.UserFilter != "" { + if _, err := ldap.CompileFilter(*ls.UserFilter); err != nil { + return NewAppError("ValidateFilter", "ent.ldap.validate_filter.app_error", nil, err.Error(), http.StatusBadRequest) + } + } } return nil diff --git a/model/config_test.go b/model/config_test.go index 039445bdf5..0330988ddc 100644 --- a/model/config_test.go +++ b/model/config_test.go @@ -689,3 +689,156 @@ func TestImageProxySettingsIsValid(t *testing.T) { }) } } + +func TestLdapSettingsIsValid(t *testing.T) { + for _, test := range []struct { + Name string + LdapSettings LdapSettings + ExpectError bool + }{ + { + Name: "disabled", + LdapSettings: LdapSettings{ + Enable: NewBool(false), + }, + ExpectError: false, + }, + { + Name: "missing server", + LdapSettings: LdapSettings{ + Enable: NewBool(true), + LdapServer: NewString(""), + BaseDN: NewString("basedn"), + EmailAttribute: NewString("email"), + UsernameAttribute: NewString("username"), + IdAttribute: NewString("id"), + LoginIdAttribute: NewString("loginid"), + UserFilter: NewString(""), + }, + ExpectError: true, + }, + { + Name: "empty user filter", + LdapSettings: LdapSettings{ + Enable: NewBool(true), + LdapServer: NewString("server"), + BaseDN: NewString("basedn"), + EmailAttribute: NewString("email"), + UsernameAttribute: NewString("username"), + IdAttribute: NewString("id"), + LoginIdAttribute: NewString("loginid"), + UserFilter: NewString(""), + }, + ExpectError: false, + }, + { + Name: "valid user filter #1", + LdapSettings: LdapSettings{ + Enable: NewBool(true), + LdapServer: NewString("server"), + BaseDN: NewString("basedn"), + EmailAttribute: NewString("email"), + UsernameAttribute: NewString("username"), + IdAttribute: NewString("id"), + LoginIdAttribute: NewString("loginid"), + UserFilter: NewString("(property=value)"), + }, + ExpectError: false, + }, + { + Name: "invalid user filter #1", + LdapSettings: LdapSettings{ + Enable: NewBool(true), + LdapServer: NewString("server"), + BaseDN: NewString("basedn"), + EmailAttribute: NewString("email"), + UsernameAttribute: NewString("username"), + IdAttribute: NewString("id"), + LoginIdAttribute: NewString("loginid"), + UserFilter: NewString("("), + }, + ExpectError: true, + }, + { + Name: "invalid user filter #2", + LdapSettings: LdapSettings{ + Enable: NewBool(true), + LdapServer: NewString("server"), + BaseDN: NewString("basedn"), + EmailAttribute: NewString("email"), + UsernameAttribute: NewString("username"), + IdAttribute: NewString("id"), + LoginIdAttribute: NewString("loginid"), + UserFilter: NewString("()"), + }, + ExpectError: true, + }, + { + Name: "valid user filter #2", + LdapSettings: LdapSettings{ + Enable: NewBool(true), + LdapServer: NewString("server"), + BaseDN: NewString("basedn"), + EmailAttribute: NewString("email"), + UsernameAttribute: NewString("username"), + IdAttribute: NewString("id"), + LoginIdAttribute: NewString("loginid"), + UserFilter: NewString("(&(property=value)(otherthing=othervalue))"), + }, + ExpectError: false, + }, + { + Name: "valid user filter #3", + LdapSettings: LdapSettings{ + Enable: NewBool(true), + LdapServer: NewString("server"), + BaseDN: NewString("basedn"), + EmailAttribute: NewString("email"), + UsernameAttribute: NewString("username"), + IdAttribute: NewString("id"), + LoginIdAttribute: NewString("loginid"), + UserFilter: NewString("(&(property=value)(|(otherthing=othervalue)(other=thing)))"), + }, + ExpectError: false, + }, + { + Name: "invalid user filter #3", + LdapSettings: LdapSettings{ + Enable: NewBool(true), + LdapServer: NewString("server"), + BaseDN: NewString("basedn"), + EmailAttribute: NewString("email"), + UsernameAttribute: NewString("username"), + IdAttribute: NewString("id"), + LoginIdAttribute: NewString("loginid"), + UserFilter: NewString("(&(property=value)(|(otherthing=othervalue)(other=thing))"), + }, + ExpectError: true, + }, + { + Name: "invalid user filter #4", + LdapSettings: LdapSettings{ + Enable: NewBool(true), + LdapServer: NewString("server"), + BaseDN: NewString("basedn"), + EmailAttribute: NewString("email"), + UsernameAttribute: NewString("username"), + IdAttribute: NewString("id"), + LoginIdAttribute: NewString("loginid"), + UserFilter: NewString("(&(property=value)((otherthing=othervalue)(other=thing)))"), + }, + ExpectError: true, + }, + } { + t.Run(test.Name, func(t *testing.T) { + test.LdapSettings.SetDefaults() + + err := test.LdapSettings.isValid() + if test.ExpectError { + assert.NotNil(t, err) + } else { + assert.Nil(t, err) + } + }) + } +} diff --git a/services/mailservice/mail_test.go b/services/mailservice/mail_test.go index 04c5035604..c8d196b226 100644 --- a/services/mailservice/mail_test.go +++ b/services/mailservice/mail_test.go @@ -21,9 +21,11 @@ import ( ) func TestMailConnectionFromConfig(t *testing.T) { - cfg, _, _, err := config.LoadConfig("config.json") + fs, err := config.NewFileStore("config.json", false) require.Nil(t, err) + cfg := fs.Get() + if conn, err := ConnectToSMTPServer(cfg); err != nil { t.Log(err) t.Fatal("Should connect to the STMP Server") @@ -44,9 +46,11 @@ func TestMailConnectionFromConfig(t *testing.T) { } func TestMailConnectionAdvanced(t *testing.T) { - cfg, _, _, err := config.LoadConfig("config.json") + fs, err := config.NewFileStore("config.json", false) require.Nil(t, err) + cfg := fs.Get() + if conn, err := ConnectToSMTPServerAdvanced( &SmtpConnectionInfo{ ConnectionSecurity: *cfg.EmailSettings.ConnectionSecurity, @@ -94,10 +98,13 @@ func TestMailConnectionAdvanced(t *testing.T) { } func TestSendMailUsingConfig(t *testing.T) { - cfg, _, _, err := config.LoadConfig("config.json") - require.Nil(t, err) utils.T = utils.GetUserTranslations("en") + fs, err := config.NewFileStore("config.json", false) + require.Nil(t, err) + + cfg := fs.Get() + var emailTo = "test@example.com" var emailSubject = "Testing this email" var emailBody = "This is a test from autobot" @@ -136,10 +143,13 @@ func TestSendMailUsingConfig(t *testing.T) { } func TestSendMailUsingConfigAdvanced(t *testing.T) { - cfg, _, _, err := config.LoadConfig("config.json") - require.Nil(t, err) utils.T = utils.GetUserTranslations("en") + fs, err := config.NewFileStore("config.json", false) + require.Nil(t, err) + + cfg := fs.Get() + var mimeTo = "test@example.com" var smtpTo = "test2@example.com" var from = mail.Address{Name: "Nobody", Address: "nobody@mattermost.com"}