From e4075dae18d2764ed748550fbb955dcd99191739 Mon Sep 17 00:00:00 2001 From: Doug Lauder Date: Mon, 15 May 2023 10:37:48 -0400 Subject: [PATCH] MM-43077 Allow inline JSON in config.json for advanced logging config (#23324) * add AdvancedLoggingJSON to LogSettings and deprecate AdvancedLoggingConfig * allow embedded JSON in config for advanced logging. --- server/channels/app/audit.go | 7 +- server/channels/app/platform/config.go | 7 +- server/config/logconfigsrc.go | 59 ++++++-- server/config/logconfigsrc_test.go | 23 +-- server/config/migrate.go | 8 +- server/config/utils.go | 5 +- server/config/utils_test.go | 2 +- .../platform/services/telemetry/telemetry.go | 9 +- server/public/model/config.go | 131 +++++++++++++----- server/public/utils/json.go | 30 ++++ server/public/utils/json_test.go | 117 ++++++++++++++++ 11 files changed, 324 insertions(+), 74 deletions(-) diff --git a/server/channels/app/audit.go b/server/channels/app/audit.go index 6445d433bc..bcd1aedf32 100644 --- a/server/channels/app/audit.go +++ b/server/channels/app/audit.go @@ -11,6 +11,7 @@ import ( "github.com/mattermost/mattermost-server/server/public/model" "github.com/mattermost/mattermost-server/server/public/shared/mlog" + "github.com/mattermost/mattermost-server/server/public/utils" "github.com/mattermost/mattermost-server/server/v8/channels/audit" "github.com/mattermost/mattermost-server/server/v8/channels/store" "github.com/mattermost/mattermost-server/server/v8/config" @@ -109,14 +110,14 @@ func (s *Server) configureAudit(adt *audit.Audit, bAllowAdvancedLogging bool) er adt.OnError = s.onAuditError var logConfigSrc config.LogConfigSrc - dsn := *s.platform.Config().ExperimentalAuditSettings.AdvancedLoggingConfig - if bAllowAdvancedLogging && dsn != "" { + dsn := s.platform.Config().ExperimentalAuditSettings.GetAdvancedLoggingConfig() + if bAllowAdvancedLogging && !utils.IsEmptyJSON(dsn) { var err error logConfigSrc, err = config.NewLogConfigSrc(dsn, s.platform.GetConfigStore()) if err != nil { return fmt.Errorf("invalid config source for audit, %w", err) } - mlog.Debug("Loaded audit configuration", mlog.String("source", dsn)) + mlog.Debug("Loaded audit configuration", mlog.String("source", string(dsn))) } // ExperimentalAuditSettings provides basic file audit (E0, E10); logConfigSrc provides advanced config (E20). diff --git a/server/channels/app/platform/config.go b/server/channels/app/platform/config.go index 00d32eb6a3..09eb3ab57a 100644 --- a/server/channels/app/platform/config.go +++ b/server/channels/app/platform/config.go @@ -19,6 +19,7 @@ import ( "github.com/mattermost/mattermost-server/server/public/model" "github.com/mattermost/mattermost-server/server/public/shared/mlog" + "github.com/mattermost/mattermost-server/server/public/utils" "github.com/mattermost/mattermost-server/server/v8/channels/product" "github.com/mattermost/mattermost-server/server/v8/channels/store" "github.com/mattermost/mattermost-server/server/v8/config" @@ -120,14 +121,14 @@ func (ps *PlatformService) ConfigureLogger(name string, logger *mlog.Logger, log // file is loaded. If no valid E20 license exists then advanced logging will be // shutdown once license is loaded/checked. var err error - dsn := *logSettings.AdvancedLoggingConfig var logConfigSrc config.LogConfigSrc - if dsn != "" { + dsn := logSettings.GetAdvancedLoggingConfig() + if !utils.IsEmptyJSON(dsn) { logConfigSrc, err = config.NewLogConfigSrc(dsn, ps.configStore) if err != nil { return fmt.Errorf("invalid config source for %s, %w", name, err) } - ps.logger.Info("Loaded configuration for "+name, mlog.String("source", dsn)) + ps.logger.Info("Loaded configuration for "+name, mlog.String("source", string(dsn))) } cfg, err := config.MloggerConfigFromLoggerConfig(logSettings, logConfigSrc, getPath) diff --git a/server/config/logconfigsrc.go b/server/config/logconfigsrc.go index c98e85f0f0..a6f4e433d0 100644 --- a/server/config/logconfigsrc.go +++ b/server/config/logconfigsrc.go @@ -7,13 +7,20 @@ import ( "encoding/json" "errors" "path/filepath" + "strconv" "strings" "sync" "github.com/mattermost/mattermost-server/server/public/shared/mlog" ) +const ( + LogConfigSrcTypeJSON LogConfigSrcType = "json" + LogConfigSrcTypeFile LogConfigSrcType = "file" +) + type LogSrcListener func(old, new mlog.LoggerConfiguration) +type LogConfigSrcType string // LogConfigSrc abstracts the Advanced Logging configuration so that implementations can // fetch from file, database, etc. @@ -22,7 +29,10 @@ type LogConfigSrc interface { Get() mlog.LoggerConfiguration // Set updates the dsn specifying the source and reloads - Set(dsn string, configStore *Store) (err error) + Set(dsn []byte, configStore *Store) (err error) + + // GetType returns the type of config source (JSON, file, ...) + GetType() LogConfigSrcType // Close cleans up resources. Close() error @@ -30,8 +40,8 @@ type LogConfigSrc interface { // NewLogConfigSrc creates an advanced logging configuration source, backed by a // file, JSON string, or database. -func NewLogConfigSrc(dsn string, configStore *Store) (LogConfigSrc, error) { - if dsn == "" { +func NewLogConfigSrc(dsn json.RawMessage, configStore *Store) (LogConfigSrc, error) { + if len(dsn) == 0 { return nil, errors.New("dsn should not be empty") } @@ -39,17 +49,28 @@ func NewLogConfigSrc(dsn string, configStore *Store) (LogConfigSrc, error) { return nil, errors.New("configStore should not be nil") } - dsn = strings.TrimSpace(dsn) - + // check if embedded JSON if isJSONMap(dsn) { return newJSONSrc(dsn) } - path := dsn + // Now we're treating the DSN as a string which may contain escaped JSON or be a filespec. + str := strings.TrimSpace(string(dsn)) + if s, err := strconv.Unquote(str); err == nil { + str = s + } + + // check if escaped JSON + strBytes := []byte(str) + if isJSONMap(strBytes) { + return newJSONSrc(strBytes) + } + // If this is a file based config we need the full path so it can be watched. - if strings.HasPrefix(configStore.String(), "file://") && !filepath.IsAbs(dsn) { + path := str + if strings.HasPrefix(configStore.String(), "file://") && !filepath.IsAbs(path) { configPath := strings.TrimPrefix(configStore.String(), "file://") - path = filepath.Join(filepath.Dir(configPath), dsn) + path = filepath.Join(filepath.Dir(configPath), path) } return newFileSrc(path, configStore) @@ -63,7 +84,7 @@ type jsonSrc struct { cfg mlog.LoggerConfiguration } -func newJSONSrc(data string) (*jsonSrc, error) { +func newJSONSrc(data json.RawMessage) (*jsonSrc, error) { src := &jsonSrc{} return src, src.Set(data, nil) } @@ -76,8 +97,8 @@ func (src *jsonSrc) Get() mlog.LoggerConfiguration { } // Set updates the JSON specifying the source and reloads -func (src *jsonSrc) Set(data string, _ *Store) error { - cfg, err := logTargetCfgFromJSON([]byte(data)) +func (src *jsonSrc) Set(data []byte, _ *Store) error { + cfg, err := logTargetCfgFromJSON(data) if err != nil { return err } @@ -86,6 +107,11 @@ func (src *jsonSrc) Set(data string, _ *Store) error { return nil } +// GetType returns the config source type. +func (src *jsonSrc) GetType() LogConfigSrcType { + return LogConfigSrcTypeJSON +} + func (src *jsonSrc) set(cfg mlog.LoggerConfiguration) { src.mutex.Lock() defer src.mutex.Unlock() @@ -112,7 +138,7 @@ func newFileSrc(path string, configStore *Store) (*fileSrc, error) { src := &fileSrc{ path: path, } - if err := src.Set(path, configStore); err != nil { + if err := src.Set([]byte(path), configStore); err != nil { return nil, err } return src, nil @@ -128,8 +154,8 @@ func (src *fileSrc) Get() mlog.LoggerConfiguration { // Set updates the dsn specifying the file source and reloads. // The file will be watched for changes and reloaded as needed, // and all listeners notified. -func (src *fileSrc) Set(path string, configStore *Store) error { - data, err := configStore.GetFile(path) +func (src *fileSrc) Set(path []byte, configStore *Store) error { + data, err := configStore.GetFile(string(path)) if err != nil { return err } @@ -143,6 +169,11 @@ func (src *fileSrc) Set(path string, configStore *Store) error { return nil } +// GetType returns the config source type. +func (src *fileSrc) GetType() LogConfigSrcType { + return LogConfigSrcTypeFile +} + func (src *fileSrc) set(cfg mlog.LoggerConfiguration) { src.mutex.Lock() defer src.mutex.Unlock() diff --git a/server/config/logconfigsrc_test.go b/server/config/logconfigsrc_test.go index 50e90ec4a7..6b79b14ab3 100644 --- a/server/config/logconfigsrc_test.go +++ b/server/config/logconfigsrc_test.go @@ -4,36 +4,41 @@ package config import ( + "encoding/json" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -const ( - validJSON = `{"file":{ "Type":"file"}}` - badJSON = `{"file":{ Type="file"}}` +var ( + validJSON = []byte(`{"file":{ "Type":"file"}}`) + badJSON = []byte(`{"file":{ Type="file"}}`) + validEscapedJSON = []byte(`"{\"file\":{ \"Type\":\"file\"}}"`) + badEscapedJSON = []byte(`"{\"file\":{ Type:\"file\"}}"`) ) func TestNewLogConfigSrc(t *testing.T) { store := NewTestMemoryStore() require.NotNil(t, store) - err := store.SetFile("advancedlogging.conf", []byte(validJSON)) + err := store.SetFile("advancedlogging.conf", validJSON) require.NoError(t, err) tests := []struct { name string - dsn string + dsn json.RawMessage configStore *Store wantErr bool wantType LogConfigSrc }{ - {name: "empty dsn", dsn: "", configStore: store, wantErr: true, wantType: nil}, - {name: "garbage dsn", dsn: "!@wfejwcevioj", configStore: store, wantErr: true, wantType: nil}, + {name: "empty dsn", dsn: []byte(""), configStore: store, wantErr: true, wantType: nil}, + {name: "garbage dsn", dsn: []byte("!@wfejwcevioj"), configStore: store, wantErr: true, wantType: nil}, {name: "valid json dsn", dsn: validJSON, configStore: store, wantErr: false, wantType: &jsonSrc{}}, {name: "invalid json dsn", dsn: badJSON, configStore: store, wantErr: true, wantType: nil}, - {name: "valid filespec dsn", dsn: "advancedlogging.conf", configStore: store, wantErr: false, wantType: &fileSrc{}}, - {name: "invalid filespec dsn", dsn: "/nobody/here.conf", configStore: store, wantErr: true, wantType: nil}, + {name: "valid escaped json dsn", dsn: validEscapedJSON, configStore: store, wantErr: false, wantType: &jsonSrc{}}, + {name: "invalid escaped json dsn", dsn: badEscapedJSON, configStore: store, wantErr: true, wantType: nil}, + {name: "valid filespec dsn", dsn: []byte("advancedlogging.conf"), configStore: store, wantErr: false, wantType: &fileSrc{}}, + {name: "invalid filespec dsn", dsn: []byte("/nobody/here.conf"), configStore: store, wantErr: true, wantType: nil}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/server/config/migrate.go b/server/config/migrate.go index ac4b4e597f..f2a505bfd3 100644 --- a/server/config/migrate.go +++ b/server/config/migrate.go @@ -32,9 +32,11 @@ func Migrate(from, to string) error { *sourceConfig.SamlSettings.PrivateKeyFile, } - // Only migrate advanced logging config if it is not embedded JSON. - if !isJSONMap(*sourceConfig.LogSettings.AdvancedLoggingConfig) { - files = append(files, *sourceConfig.LogSettings.AdvancedLoggingConfig) + // Only migrate advanced logging config if it is a filespec. + dsn := sourceConfig.LogSettings.GetAdvancedLoggingConfig() + cfgSource, err := NewLogConfigSrc(dsn, source) + if err == nil && cfgSource.GetType() == LogConfigSrcTypeFile { + files = append(files, string(dsn)) } files = append(files, sourceConfig.PluginSettings.SignaturePublicKeyFiles...) diff --git a/server/config/utils.go b/server/config/utils.go index d47e393274..fe840ad083 100644 --- a/server/config/utils.go +++ b/server/config/utils.go @@ -179,9 +179,10 @@ func IsDatabaseDSN(dsn string) bool { strings.HasPrefix(dsn, "postgresql://") } -func isJSONMap(data string) bool { +func isJSONMap(data []byte) bool { var m map[string]any - return json.Unmarshal([]byte(data), &m) == nil + err := json.Unmarshal(data, &m) + return err == nil } func GetValueByPath(path []string, obj any) (any, bool) { diff --git a/server/config/utils_test.go b/server/config/utils_test.go index 07be9bd92d..5485360735 100644 --- a/server/config/utils_test.go +++ b/server/config/utils_test.go @@ -221,7 +221,7 @@ func TestIsJSONMap(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := isJSONMap(tt.data); got != tt.want { + if got := isJSONMap([]byte(tt.data)); got != tt.want { t.Errorf("isJSONMap() = %v, want %v", got, tt.want) } }) diff --git a/server/platform/services/telemetry/telemetry.go b/server/platform/services/telemetry/telemetry.go index 6504c423b4..a5237665ec 100644 --- a/server/platform/services/telemetry/telemetry.go +++ b/server/platform/services/telemetry/telemetry.go @@ -533,7 +533,8 @@ func (ts *TelemetryService) trackConfig() { "file_json": cfg.LogSettings.FileJson, "enable_webhook_debugging": cfg.LogSettings.EnableWebhookDebugging, "isdefault_file_location": isDefault(cfg.LogSettings.FileLocation, ""), - "advanced_logging_config": *cfg.LogSettings.AdvancedLoggingConfig != "", + "advanced_logging_json": len(cfg.LogSettings.AdvancedLoggingJSON) != 0, + "advanced_logging_config": cfg.LogSettings.AdvancedLoggingConfig != nil && *cfg.LogSettings.AdvancedLoggingConfig != "", }) ts.SendTelemetry(TrackConfigAudit, map[string]any{ @@ -543,7 +544,8 @@ func (ts *TelemetryService) trackConfig() { "file_max_backups": *cfg.ExperimentalAuditSettings.FileMaxBackups, "file_compress": *cfg.ExperimentalAuditSettings.FileCompress, "file_max_queue_size": *cfg.ExperimentalAuditSettings.FileMaxQueueSize, - "advanced_logging_config": *cfg.ExperimentalAuditSettings.AdvancedLoggingConfig != "", + "advanced_logging_json": len(cfg.ExperimentalAuditSettings.AdvancedLoggingJSON) != 0, + "advanced_logging_config": cfg.ExperimentalAuditSettings.AdvancedLoggingConfig != nil && *cfg.ExperimentalAuditSettings.AdvancedLoggingConfig != "", }) ts.SendTelemetry(TrackConfigNotificationLog, map[string]any{ @@ -554,7 +556,8 @@ func (ts *TelemetryService) trackConfig() { "file_level": *cfg.NotificationLogSettings.FileLevel, "file_json": *cfg.NotificationLogSettings.FileJson, "isdefault_file_location": isDefault(*cfg.NotificationLogSettings.FileLocation, ""), - "advanced_logging_config": *cfg.NotificationLogSettings.AdvancedLoggingConfig != "", + "advanced_logging_json": len(cfg.NotificationLogSettings.AdvancedLoggingJSON) != 0, + "advanced_logging_config": cfg.NotificationLogSettings.AdvancedLoggingConfig != nil && *cfg.NotificationLogSettings.AdvancedLoggingConfig != "", }) ts.SendTelemetry(TrackConfigPassword, map[string]any{ diff --git a/server/public/model/config.go b/server/public/model/config.go index 5b0934f432..0902de2b9e 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -22,6 +22,7 @@ import ( "github.com/mattermost/ldap" "github.com/mattermost/mattermost-server/server/public/shared/mlog" + "github.com/mattermost/mattermost-server/server/public/utils" ) const ( @@ -1238,19 +1239,20 @@ func (s *SqlSettings) SetDefaults(isUpdate bool) { } type LogSettings struct { - EnableConsole *bool `access:"environment_logging,write_restrictable,cloud_restrictable"` - ConsoleLevel *string `access:"environment_logging,write_restrictable,cloud_restrictable"` - ConsoleJson *bool `access:"environment_logging,write_restrictable,cloud_restrictable"` - EnableColor *bool `access:"environment_logging,write_restrictable,cloud_restrictable"` // telemetry: none - EnableFile *bool `access:"environment_logging,write_restrictable,cloud_restrictable"` - FileLevel *string `access:"environment_logging,write_restrictable,cloud_restrictable"` - FileJson *bool `access:"environment_logging,write_restrictable,cloud_restrictable"` - FileLocation *string `access:"environment_logging,write_restrictable,cloud_restrictable"` - EnableWebhookDebugging *bool `access:"environment_logging,write_restrictable,cloud_restrictable"` - EnableDiagnostics *bool `access:"environment_logging,write_restrictable,cloud_restrictable"` // telemetry: none - VerboseDiagnostics *bool `access:"environment_logging,write_restrictable,cloud_restrictable"` // telemetry: none - EnableSentry *bool `access:"environment_logging,write_restrictable,cloud_restrictable"` // telemetry: none - AdvancedLoggingConfig *string `access:"environment_logging,write_restrictable,cloud_restrictable"` + EnableConsole *bool `access:"environment_logging,write_restrictable,cloud_restrictable"` + ConsoleLevel *string `access:"environment_logging,write_restrictable,cloud_restrictable"` + ConsoleJson *bool `access:"environment_logging,write_restrictable,cloud_restrictable"` + EnableColor *bool `access:"environment_logging,write_restrictable,cloud_restrictable"` // telemetry: none + EnableFile *bool `access:"environment_logging,write_restrictable,cloud_restrictable"` + FileLevel *string `access:"environment_logging,write_restrictable,cloud_restrictable"` + FileJson *bool `access:"environment_logging,write_restrictable,cloud_restrictable"` + FileLocation *string `access:"environment_logging,write_restrictable,cloud_restrictable"` + EnableWebhookDebugging *bool `access:"environment_logging,write_restrictable,cloud_restrictable"` + EnableDiagnostics *bool `access:"environment_logging,write_restrictable,cloud_restrictable"` // telemetry: none + VerboseDiagnostics *bool `access:"environment_logging,write_restrictable,cloud_restrictable"` // telemetry: none + EnableSentry *bool `access:"environment_logging,write_restrictable,cloud_restrictable"` // telemetry: none + AdvancedLoggingJSON json.RawMessage `access:"environment_logging,write_restrictable,cloud_restrictable"` + AdvancedLoggingConfig *string `access:"environment_logging,write_restrictable,cloud_restrictable"` // Deprecated: use `AdvancedLoggingJSON` } func NewLogSettings() *LogSettings { @@ -1308,20 +1310,40 @@ func (s *LogSettings) SetDefaults() { s.FileJson = NewBool(true) } - if s.AdvancedLoggingConfig == nil { - s.AdvancedLoggingConfig = NewString("") + if utils.IsEmptyJSON(s.AdvancedLoggingJSON) { + // copy any non-empty AdvancedLoggingConfig (deprecated) to the new field. + if s.AdvancedLoggingConfig != nil && !utils.IsEmptyJSON([]byte(*s.AdvancedLoggingConfig)) { + s.AdvancedLoggingJSON = utils.StringPtrToJSON(s.AdvancedLoggingConfig) + + } else { + s.AdvancedLoggingJSON = []byte("{}") + } } + s.AdvancedLoggingConfig = nil +} + +// GetAdvancedLoggingConfig returns the advanced logging config as a []byte. +// AdvancedLoggingJSON takes precident over the deprecated AdvancedLoggingConfig. +func (s *LogSettings) GetAdvancedLoggingConfig() []byte { + if !utils.IsEmptyJSON(s.AdvancedLoggingJSON) { + return s.AdvancedLoggingJSON + } + if s.AdvancedLoggingConfig != nil && !utils.IsEmptyJSON([]byte(*s.AdvancedLoggingConfig)) { + return []byte(*s.AdvancedLoggingConfig) + } + return []byte("{}") } type ExperimentalAuditSettings struct { - FileEnabled *bool `access:"experimental_features,write_restrictable,cloud_restrictable"` - FileName *string `access:"experimental_features,write_restrictable,cloud_restrictable"` // telemetry: none - FileMaxSizeMB *int `access:"experimental_features,write_restrictable,cloud_restrictable"` - FileMaxAgeDays *int `access:"experimental_features,write_restrictable,cloud_restrictable"` - FileMaxBackups *int `access:"experimental_features,write_restrictable,cloud_restrictable"` - FileCompress *bool `access:"experimental_features,write_restrictable,cloud_restrictable"` - FileMaxQueueSize *int `access:"experimental_features,write_restrictable,cloud_restrictable"` - AdvancedLoggingConfig *string `access:"experimental_features,write_restrictable,cloud_restrictable"` + FileEnabled *bool `access:"experimental_features,write_restrictable,cloud_restrictable"` + FileName *string `access:"experimental_features,write_restrictable,cloud_restrictable"` // telemetry: none + FileMaxSizeMB *int `access:"experimental_features,write_restrictable,cloud_restrictable"` + FileMaxAgeDays *int `access:"experimental_features,write_restrictable,cloud_restrictable"` + FileMaxBackups *int `access:"experimental_features,write_restrictable,cloud_restrictable"` + FileCompress *bool `access:"experimental_features,write_restrictable,cloud_restrictable"` + FileMaxQueueSize *int `access:"experimental_features,write_restrictable,cloud_restrictable"` + AdvancedLoggingJSON json.RawMessage `access:"experimental_features,write_restrictable,cloud_restrictable"` + AdvancedLoggingConfig *string `access:"experimental_features,write_restrictable,cloud_restrictable"` // Deprecated: use `AdvancedLoggingJSON` } func (s *ExperimentalAuditSettings) SetDefaults() { @@ -1353,21 +1375,40 @@ func (s *ExperimentalAuditSettings) SetDefaults() { s.FileMaxQueueSize = NewInt(1000) } - if s.AdvancedLoggingConfig == nil { - s.AdvancedLoggingConfig = NewString("") + if utils.IsEmptyJSON(s.AdvancedLoggingJSON) { + // copy any non-empty AdvancedLoggingConfig (deprecated) to the new field. + if s.AdvancedLoggingConfig != nil && !utils.IsEmptyJSON([]byte(*s.AdvancedLoggingConfig)) { + s.AdvancedLoggingJSON = utils.StringPtrToJSON(s.AdvancedLoggingConfig) + } else { + s.AdvancedLoggingJSON = []byte("{}") + } } + s.AdvancedLoggingConfig = nil +} + +// GetAdvancedLoggingConfig returns the advanced logging config as a []byte. +// AdvancedLoggingJSON takes precident over the deprecated AdvancedLoggingConfig. +func (s *ExperimentalAuditSettings) GetAdvancedLoggingConfig() []byte { + if !utils.IsEmptyJSON(s.AdvancedLoggingJSON) { + return s.AdvancedLoggingJSON + } + if s.AdvancedLoggingConfig != nil && !utils.IsEmptyJSON([]byte(*s.AdvancedLoggingConfig)) { + return []byte(*s.AdvancedLoggingConfig) + } + return []byte("{}") } type NotificationLogSettings struct { - EnableConsole *bool `access:"write_restrictable,cloud_restrictable"` - ConsoleLevel *string `access:"write_restrictable,cloud_restrictable"` - ConsoleJson *bool `access:"write_restrictable,cloud_restrictable"` - EnableColor *bool `access:"write_restrictable,cloud_restrictable"` // telemetry: none - EnableFile *bool `access:"write_restrictable,cloud_restrictable"` - FileLevel *string `access:"write_restrictable,cloud_restrictable"` - FileJson *bool `access:"write_restrictable,cloud_restrictable"` - FileLocation *string `access:"write_restrictable,cloud_restrictable"` - AdvancedLoggingConfig *string `access:"write_restrictable,cloud_restrictable"` + EnableConsole *bool `access:"write_restrictable,cloud_restrictable"` + ConsoleLevel *string `access:"write_restrictable,cloud_restrictable"` + ConsoleJson *bool `access:"write_restrictable,cloud_restrictable"` + EnableColor *bool `access:"write_restrictable,cloud_restrictable"` // telemetry: none + EnableFile *bool `access:"write_restrictable,cloud_restrictable"` + FileLevel *string `access:"write_restrictable,cloud_restrictable"` + FileJson *bool `access:"write_restrictable,cloud_restrictable"` + FileLocation *string `access:"write_restrictable,cloud_restrictable"` + AdvancedLoggingJSON json.RawMessage `access:"write_restrictable,cloud_restrictable"` + AdvancedLoggingConfig *string `access:"write_restrictable,cloud_restrictable"` // Deprecated: use `AdvancedLoggingJSON` } func (s *NotificationLogSettings) SetDefaults() { @@ -1403,9 +1444,27 @@ func (s *NotificationLogSettings) SetDefaults() { s.FileJson = NewBool(true) } - if s.AdvancedLoggingConfig == nil { - s.AdvancedLoggingConfig = NewString("") + if utils.IsEmptyJSON(s.AdvancedLoggingJSON) { + // copy any non-empty AdvancedLoggingConfig (deprecated) to the new field. + if s.AdvancedLoggingConfig != nil && !utils.IsEmptyJSON([]byte(*s.AdvancedLoggingConfig)) { + s.AdvancedLoggingJSON = utils.StringPtrToJSON(s.AdvancedLoggingConfig) + } else { + s.AdvancedLoggingJSON = []byte("{}") + } } + s.AdvancedLoggingConfig = nil +} + +// GetAdvancedLoggingConfig returns the advanced logging config as a []byte. +// AdvancedLoggingJSON takes precident over the deprecated AdvancedLoggingConfig. +func (s *NotificationLogSettings) GetAdvancedLoggingConfig() []byte { + if !utils.IsEmptyJSON(s.AdvancedLoggingJSON) { + return s.AdvancedLoggingJSON + } + if s.AdvancedLoggingConfig != nil && !utils.IsEmptyJSON([]byte(*s.AdvancedLoggingConfig)) { + return []byte(*s.AdvancedLoggingConfig) + } + return []byte("{}") } type PasswordSettings struct { diff --git a/server/public/utils/json.go b/server/public/utils/json.go index 39d8646922..4e311bcba0 100644 --- a/server/public/utils/json.go +++ b/server/public/utils/json.go @@ -6,6 +6,7 @@ package utils import ( "bytes" "encoding/json" + "strings" "github.com/pkg/errors" ) @@ -54,3 +55,32 @@ func NewHumanizedJSONError(err error, data []byte, offset int64) *HumanizedJSONE Err: errors.Wrapf(err, "parsing error at line %d, character %d", line, character), } } + +func IsEmptyJSON(j json.RawMessage) bool { + if len(j) == 0 || bytes.Equal(j, []byte("{}")) || bytes.Equal(j, []byte("\"\"")) || bytes.Equal(j, []byte("[]")) { + return true + } + return false +} + +func StringPtrToJSON(ptr *string) json.RawMessage { + if ptr == nil || len(*ptr) == 0 { + return []byte("{}") + } + s := *ptr + + if strings.HasPrefix(s, "{") && strings.HasSuffix(s, "}") { + return []byte(s) + } + + if strings.HasPrefix(s, "[") && strings.HasSuffix(s, "]") { + return []byte(s) + } + + if strings.HasPrefix(s, "\"") && strings.HasSuffix(s, "\"") { + return []byte(s) + } + + // This must be a bare string which will need quotes to make a valid JSON document. + return []byte("\"" + s + "\"") +} diff --git a/server/public/utils/json_test.go b/server/public/utils/json_test.go index 0f2bad3e33..145c6d276a 100644 --- a/server/public/utils/json_test.go +++ b/server/public/utils/json_test.go @@ -11,6 +11,7 @@ import ( "github.com/pkg/errors" "github.com/stretchr/testify/assert" + "github.com/mattermost/mattermost-server/server/public/model" "github.com/mattermost/mattermost-server/server/public/utils" ) @@ -233,3 +234,119 @@ func TestNewHumanizedJSONError(t *testing.T) { }) } } + +func TestIsJSONEmpty(t *testing.T) { + t.Parallel() + + testCases := []struct { + Description string + Data []byte + Empty bool + }{ + { + "nil []byte is empty", + nil, + true, + }, + { + "Zero length slice is empty", + []byte(""), + true, + }, + { + "braces are empty", + []byte("{}"), + true, + }, + { + "square brackets are empty", + []byte("[]"), + true, + }, + { + "empty string is empty", + []byte("\"\""), + true, + }, + { + "map is not empty", + []byte("{\"foo\":7}"), + false, + }, + { + "array is not empty", + []byte("[1,2,3]"), + false, + }, + { + "string is not empty", + []byte("\"hello\""), + false, + }, + } + + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.Description, func(t *testing.T) { + empty := utils.IsEmptyJSON(testCase.Data) + assert.Equal(t, testCase.Empty, empty) + + if !testCase.Empty { + // don't really need to test the JSON unmarshaller but this is included + // to ensure the test cases stay valid. + var v interface{} + err := json.Unmarshal(testCase.Data, &v) + assert.NoError(t, err) + } + }) + } +} + +func TestStringPtrToJSON(t *testing.T) { + t.Parallel() + + testCases := []struct { + Description string + Ptr *string + Expect json.RawMessage + }{ + { + "nil string ptr", + nil, + []byte("{}"), + }, + { + "Zero length string", + model.NewString(""), + []byte("{}"), + }, + { + "JSON map", + model.NewString("{\"foo\":7}"), + []byte("{\"foo\":7}"), + }, + { + "JSON array", + model.NewString("[1,2,3]"), + []byte("[1,2,3]"), + }, + { + "JSON string", + model.NewString("\"hello\""), + []byte("\"hello\""), + }, + { + "bare string", + model.NewString("hello"), + []byte("\"hello\""), + }, + } + + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.Description, func(t *testing.T) { + j := utils.StringPtrToJSON(testCase.Ptr) + assert.Equal(t, testCase.Expect, j) + }) + } +}