From 8c8b1bbc9c1b13c35ed7982cd1006a8d30625671 Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Thu, 21 Mar 2019 15:46:38 -0400 Subject: [PATCH] MM-14441: restrict system admin config (#10477) * tweak utils.Merge docs * move merge_test to utils_test package for easier testing * utils: support MergeConfig and StructFieldFilter * constrain updating certain fields by the restricted system admin --- api4/config.go | 24 +++- api4/config_test.go | 24 ++++ config/common.go | 12 -- config/common_test.go | 11 +- config/export_test.go | 5 - config/utils.go | 12 ++ i18n/en.json | 4 + model/config.go | 300 +++++++++++++++++++++--------------------- utils/merge.go | 69 +++++----- utils/merge_test.go | 75 ++++++++--- 10 files changed, 316 insertions(+), 220 deletions(-) diff --git a/api4/config.go b/api4/config.go index b55876284a..815a3da334 100644 --- a/api4/config.go +++ b/api4/config.go @@ -5,8 +5,11 @@ package api4 import ( "net/http" + "reflect" + "github.com/mattermost/mattermost-server/config" "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils" ) func (api *API) InitConfig() { @@ -58,13 +61,30 @@ func updateConfig(c *Context, w http.ResponseWriter, r *http.Request) { return } + appCfg := c.App.Config() + if *c.App.Config().ExperimentalSettings.RestrictSystemAdmin { + // Start with the current configuration, and only merge values not marked as being + // restricted. + var err error + cfg, err = config.Merge(appCfg, cfg, &utils.MergeConfig{ + StructFieldFilter: func(structField reflect.StructField, base, patch reflect.Value) bool { + restricted := structField.Tag.Get("restricted") == "true" + + return !restricted + }, + }) + if err != nil { + c.Err = model.NewAppError("updateConfig", "api.config.update_config.restricted_merge.app_error", nil, err.Error(), http.StatusInternalServerError) + } + } + // Do not allow plugin uploads to be toggled through the API - cfg.PluginSettings.EnableUploads = c.App.Config().PluginSettings.EnableUploads + cfg.PluginSettings.EnableUploads = appCfg.PluginSettings.EnableUploads // If the Message Export feature has been toggled in the System Console, rewrite the ExportFromTimestamp field to an // appropriate value. The rewriting occurs here to ensure it doesn't affect values written to the config file // directly and not through the System Console UI. - if *cfg.MessageExportSettings.EnableExport != *c.App.Config().MessageExportSettings.EnableExport { + if *cfg.MessageExportSettings.EnableExport != *appCfg.MessageExportSettings.EnableExport { if *cfg.MessageExportSettings.EnableExport && *cfg.MessageExportSettings.ExportFromTimestamp == int64(0) { // When the feature is toggled on, use the current timestamp as the start time for future exports. cfg.MessageExportSettings.ExportFromTimestamp = model.NewInt64(model.GetMillis()) diff --git a/api4/config_test.go b/api4/config_test.go index 41a6b786b8..89040c3c3a 100644 --- a/api4/config_test.go +++ b/api4/config_test.go @@ -196,6 +196,30 @@ func TestUpdateConfigMessageExportSpecialHandling(t *testing.T) { assert.Equal(t, int64(0), *th.App.Config().MessageExportSettings.ExportFromTimestamp) } +func TestUpdateConfigRestrictSystemAdmin(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ExperimentalSettings.RestrictSystemAdmin = true }) + + originalCfg, resp := th.SystemAdminClient.GetConfig() + CheckNoError(t, resp) + + cfg := originalCfg.Clone() + *cfg.TeamSettings.SiteName = "MyFancyName" // Allowed + *cfg.ServiceSettings.SiteURL = "http://example.com" // Ignored + + returnedCfg, resp := th.SystemAdminClient.UpdateConfig(cfg) + CheckNoError(t, resp) + + require.Equal(t, "MyFancyName", *returnedCfg.TeamSettings.SiteName) + require.Equal(t, *originalCfg.ServiceSettings.SiteURL, *returnedCfg.ServiceSettings.SiteURL) + + actualCfg, resp := th.SystemAdminClient.GetConfig() + CheckNoError(t, resp) + + require.Equal(t, returnedCfg, actualCfg) +} + func TestGetEnvironmentConfig(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_SITEURL", "http://example.mattermost.com") os.Setenv("MM_SERVICESETTINGS_ENABLECUSTOMEMOJI", "true") diff --git a/config/common.go b/config/common.go index de0444f5ec..4e0e89b7d3 100644 --- a/config/common.go +++ b/config/common.go @@ -4,7 +4,6 @@ package config import ( - "github.com/mattermost/mattermost-server/utils" "io" "sync" @@ -141,14 +140,3 @@ func (cs *commonStore) validate(cfg *model.Config) error { return nil } - -// mergeConfig merges two configs together. The receiver's values are overwritten with the patch's -// values except when the patch's values are nil. -func (cs *commonStore) mergeConfig(patch *model.Config) (*model.Config, error) { - ret, err := utils.Merge(cs.config, patch) - if err != nil { - return nil, err - } - retC := ret.(model.Config) - return &retC, nil -} diff --git a/config/common_test.go b/config/common_test.go index 1a373bbb69..b754e1c4c5 100644 --- a/config/common_test.go +++ b/config/common_test.go @@ -1,10 +1,11 @@ package config_test import ( + "testing" + "github.com/mattermost/mattermost-server/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "testing" "github.com/mattermost/mattermost-server/model" ) @@ -85,7 +86,7 @@ func TestMergeConfigs(t *testing.T) { patch, err := config.NewMemoryStore() require.NoError(t, err) - merged, err := base.MergeConfig(patch.Get()) + merged, err := config.Merge(base.Get(), patch.Get(), nil) require.NoError(t, err) assert.Equal(t, patch.Get(), merged) @@ -95,7 +96,7 @@ func TestMergeConfigs(t *testing.T) { require.NoError(t, err) patch := base.Get().Clone() - merged, err := base.MergeConfig(patch) + merged, err := config.Merge(base.Get(), patch, nil) require.NoError(t, err) assert.Equal(t, base.Get(), merged) @@ -107,7 +108,7 @@ func TestMergeConfigs(t *testing.T) { patch := base.Get().Clone() patch.ServiceSettings.SiteURL = newString("http://newhost.ca") - merged, err := base.MergeConfig(patch) + merged, err := config.Merge(base.Get(), patch, nil) require.NoError(t, err) assert.NotEqual(t, base.Get(), merged) @@ -124,7 +125,7 @@ func TestMergeConfigs(t *testing.T) { expected.ServiceSettings.SiteURL = newString("http://newhost.ca") expected.GoogleSettings.Enable = newBool(true) - merged, err := base.MergeConfig(patch) + merged, err := config.Merge(base.Get(), patch, nil) require.NoError(t, err) assert.NotEqual(t, base.Get(), merged) diff --git a/config/export_test.go b/config/export_test.go index 7e5392ef09..b8ef523982 100644 --- a/config/export_test.go +++ b/config/export_test.go @@ -26,8 +26,3 @@ func InitializeConfigurationsTable(db *sqlx.DB) error { func ResolveConfigFilePath(path string) (string, error) { return resolveConfigFilePath(path) } - -// GetCommonStore exposes the internal commonStore to test only. -func (ms *memoryStore) MergeConfig(patch *model.Config) (*model.Config, error) { - return ms.mergeConfig(patch) -} diff --git a/config/utils.go b/config/utils.go index 8b8bb4c6f8..457f3721e7 100644 --- a/config/utils.go +++ b/config/utils.go @@ -128,3 +128,15 @@ func FixInvalidLocales(cfg *model.Config) bool { return changed } + +// Merge merges two configs together. The receiver's values are overwritten with the patch's +// values except when the patch's values are nil. +func Merge(cfg *model.Config, patch *model.Config, mergeConfig *utils.MergeConfig) (*model.Config, error) { + ret, err := utils.Merge(cfg, patch, mergeConfig) + if err != nil { + return nil, err + } + + retCfg := ret.(model.Config) + return &retCfg, nil +} diff --git a/i18n/en.json b/i18n/en.json index 683e1b2457..b22664dbe8 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -990,6 +990,10 @@ "id": "api.config.client.old_format.app_error", "translation": "New format for the client configuration is not supported yet. Please specify format=old in the query string." }, + { + "id": "api.config.update_config.restricted_merge.app_error", + "translation": "Failed to merge given config." + }, { "id": "api.context.404.app_error", "translation": "Sorry, we could not find the page." diff --git a/model/config.go b/model/config.go index c6f6525551..40e3c2db84 100644 --- a/model/config.go +++ b/model/config.go @@ -212,25 +212,25 @@ var ServerTLSSupportedCiphers = map[string]uint16{ } type ServiceSettings struct { - SiteURL *string - WebsocketURL *string - LicenseFileLocation *string - ListenAddress *string - ConnectionSecurity *string - TLSCertFile *string - TLSKeyFile *string - TLSMinVer *string - TLSStrictTransport *bool - TLSStrictTransportMaxAge *int64 - TLSOverwriteCiphers []string - UseLetsEncrypt *bool - LetsEncryptCertificateCacheFile *string - Forward80To443 *bool - ReadTimeout *int - WriteTimeout *int - MaximumLoginAttempts *int - GoroutineHealthThreshold *int - GoogleDeveloperKey *string + SiteURL *string `restricted:"true"` + WebsocketURL *string `restricted:"true"` + LicenseFileLocation *string `restricted:"true"` + ListenAddress *string `restricted:"true"` + ConnectionSecurity *string `restricted:"true"` + TLSCertFile *string `restricted:"true"` + TLSKeyFile *string `restricted:"true"` + TLSMinVer *string `restricted:"true"` + TLSStrictTransport *bool `restricted:"true"` + TLSStrictTransportMaxAge *int64 `restricted:"true"` + TLSOverwriteCiphers []string `restricted:"true"` + UseLetsEncrypt *bool `restricted:"true"` + LetsEncryptCertificateCacheFile *string `restricted:"true"` + Forward80To443 *bool `restricted:"true"` + ReadTimeout *int `restricted:"true"` + WriteTimeout *int `restricted:"true"` + MaximumLoginAttempts *int `restricted:"true"` + GoroutineHealthThreshold *int `restricted:"true"` + GoogleDeveloperKey *string `restricted:"true"` EnableOAuthServiceProvider *bool EnableIncomingWebhooks *bool EnableOutgoingWebhooks *bool @@ -239,27 +239,27 @@ type ServiceSettings struct { EnablePostUsernameOverride *bool EnablePostIconOverride *bool EnableLinkPreviews *bool - EnableTesting *bool - EnableDeveloper *bool - EnableSecurityFixAlert *bool - EnableInsecureOutgoingConnections *bool - AllowedUntrustedInternalConnections *string + EnableTesting *bool `restricted:"true"` + EnableDeveloper *bool `restricted:"true"` + EnableSecurityFixAlert *bool `restricted:"true"` + EnableInsecureOutgoingConnections *bool `restricted:"true"` + AllowedUntrustedInternalConnections *string `restricted:"true"` EnableMultifactorAuthentication *bool EnforceMultifactorAuthentication *bool EnableUserAccessTokens *bool - AllowCorsFrom *string - CorsExposedHeaders *string - CorsAllowCredentials *bool - CorsDebug *bool - AllowCookiesForSubdomains *bool - SessionLengthWebInDays *int - SessionLengthMobileInDays *int - SessionLengthSSOInDays *int - SessionCacheInMinutes *int - SessionIdleTimeoutInMinutes *int - WebsocketSecurePort *int - WebsocketPort *int - WebserverMode *string + AllowCorsFrom *string `restricted:"true"` + CorsExposedHeaders *string `restricted:"true"` + CorsAllowCredentials *bool `restricted:"true"` + CorsDebug *bool `restricted:"true"` + AllowCookiesForSubdomains *bool `restricted:"true"` + SessionLengthWebInDays *int `restricted:"true"` + SessionLengthMobileInDays *int `restricted:"true"` + SessionLengthSSOInDays *int `restricted:"true"` + SessionCacheInMinutes *int `restricted:"true"` + SessionIdleTimeoutInMinutes *int `restricted:"true"` + WebsocketSecurePort *int `restricted:"true"` + WebsocketPort *int `restricted:"true"` + WebserverMode *string `restricted:"true"` EnableCustomEmoji *bool EnableEmojiPicker *bool EnableGifPicker *bool @@ -269,14 +269,14 @@ type ServiceSettings struct { DEPRECATED_DO_NOT_USE_RestrictPostDelete *string `json:"RestrictPostDelete"` // This field is deprecated and must not be used. DEPRECATED_DO_NOT_USE_AllowEditPost *string `json:"AllowEditPost"` // This field is deprecated and must not be used. PostEditTimeLimit *int - TimeBetweenUserTypingUpdatesMilliseconds *int64 - EnablePostSearch *bool - MinimumHashtagLength *int - EnableUserTypingMessages *bool - EnableChannelViewedMessages *bool - EnableUserStatuses *bool - ExperimentalEnableAuthenticationTransfer *bool - ClusterLogTimeoutMilliseconds *int + TimeBetweenUserTypingUpdatesMilliseconds *int64 `restricted:"true"` + EnablePostSearch *bool `restricted:"true"` + MinimumHashtagLength *int `restricted:"true"` + EnableUserTypingMessages *bool `restricted:"true"` + EnableChannelViewedMessages *bool `restricted:"true"` + EnableUserStatuses *bool `restricted:"true"` + ExperimentalEnableAuthenticationTransfer *bool `restricted:"true"` + ClusterLogTimeoutMilliseconds *int `restricted:"true"` CloseUnusedDirectMessages *bool EnablePreviewFeatures *bool EnableTutorial *bool @@ -288,11 +288,11 @@ type ServiceSettings struct { DEPRECATED_DO_NOT_USE_ImageProxyOptions *string `json:"ImageProxyOptions" mapstructure:"ImageProxyOptions"` // This field is deprecated and must not be used. EnableAPITeamDeletion *bool ExperimentalEnableHardenedMode *bool - DisableLegacyMFA *bool - ExperimentalStrictCSRFEnforcement *bool + DisableLegacyMFA *bool `restricted:"true"` + ExperimentalStrictCSRFEnforcement *bool `restricted:"true"` EnableEmailInvitations *bool ExperimentalLdapGroupSync *bool - DisableBotsWhenOwnerIsDeactivated *bool + DisableBotsWhenOwnerIsDeactivated *bool `restricted:"true"` } func (s *ServiceSettings) SetDefaults() { @@ -638,17 +638,17 @@ func (s *ServiceSettings) SetDefaults() { } type ClusterSettings struct { - Enable *bool - ClusterName *string - OverrideHostname *string - UseIpAddress *bool - UseExperimentalGossip *bool - ReadOnlyConfig *bool - GossipPort *int - StreamingPort *int - MaxIdleConns *int - MaxIdleConnsPerHost *int - IdleConnTimeoutMilliseconds *int + Enable *bool `restricted:"true"` + ClusterName *string `restricted:"true"` + OverrideHostname *string `restricted:"true"` + UseIpAddress *bool `restricted:"true"` + UseExperimentalGossip *bool `restricted:"true"` + ReadOnlyConfig *bool `restricted:"true"` + GossipPort *int `restricted:"true"` + StreamingPort *int `restricted:"true"` + MaxIdleConns *int `restricted:"true"` + MaxIdleConnsPerHost *int `restricted:"true"` + IdleConnTimeoutMilliseconds *int `restricted:"true"` } func (s *ClusterSettings) SetDefaults() { @@ -698,9 +698,9 @@ func (s *ClusterSettings) SetDefaults() { } type MetricsSettings struct { - Enable *bool - BlockProfileRate *int - ListenAddress *string + Enable *bool `restricted:"true"` + BlockProfileRate *int `restricted:"true"` + ListenAddress *string `restricted:"true"` } func (s *MetricsSettings) SetDefaults() { @@ -720,10 +720,10 @@ func (s *MetricsSettings) SetDefaults() { type ExperimentalSettings struct { ClientSideCertEnable *bool ClientSideCertCheck *string - DisablePostMetadata *bool - EnableClickToReply *bool - LinkMetadataTimeoutMilliseconds *int64 - RestrictSystemAdmin *bool + DisablePostMetadata *bool `restricted:"true"` + EnableClickToReply *bool `restricted:"true"` + LinkMetadataTimeoutMilliseconds *int64 `restricted:"true"` + RestrictSystemAdmin *bool `restricted:"true"` } func (s *ExperimentalSettings) SetDefaults() { @@ -753,7 +753,7 @@ func (s *ExperimentalSettings) SetDefaults() { } type AnalyticsSettings struct { - MaxUsersForStatistics *int + MaxUsersForStatistics *int `restricted:"true"` } func (s *AnalyticsSettings) SetDefaults() { @@ -803,16 +803,16 @@ func (s *SSOSettings) setDefaults() { } type SqlSettings struct { - DriverName *string - DataSource *string - DataSourceReplicas []string - DataSourceSearchReplicas []string - MaxIdleConns *int - ConnMaxLifetimeMilliseconds *int - MaxOpenConns *int - Trace *bool - AtRestEncryptKey *string - QueryTimeout *int + DriverName *string `restricted:"true"` + DataSource *string `restricted:"true"` + DataSourceReplicas []string `restricted:"true"` + DataSourceSearchReplicas []string `restricted:"true"` + MaxIdleConns *int `restricted:"true"` + ConnMaxLifetimeMilliseconds *int `restricted:"true"` + MaxOpenConns *int `restricted:"true"` + Trace *bool `restricted:"true"` + AtRestEncryptKey *string `restricted:"true"` + QueryTimeout *int `restricted:"true"` } func (s *SqlSettings) SetDefaults() { @@ -858,16 +858,16 @@ func (s *SqlSettings) SetDefaults() { } type LogSettings struct { - EnableConsole *bool - ConsoleLevel *string - ConsoleJson *bool - EnableFile *bool - FileLevel *string - FileJson *bool - FileFormat *string - FileLocation *string - EnableWebhookDebugging *bool - EnableDiagnostics *bool + EnableConsole *bool `restricted:"true"` + ConsoleLevel *string `restricted:"true"` + ConsoleJson *bool `restricted:"true"` + EnableFile *bool `restricted:"true"` + FileLevel *string `restricted:"true"` + FileJson *bool `restricted:"true"` + FileFormat *string `restricted:"true"` + FileLocation *string `restricted:"true"` + EnableWebhookDebugging *bool `restricted:"true"` + EnableDiagnostics *bool `restricted:"true"` } func (s *LogSettings) SetDefaults() { @@ -947,20 +947,20 @@ type FileSettings struct { EnableMobileUpload *bool EnableMobileDownload *bool MaxFileSize *int64 - DriverName *string - Directory *string + DriverName *string `restricted:"true"` + Directory *string `restricted:"true"` EnablePublicLink *bool PublicLinkSalt *string InitialFont *string - AmazonS3AccessKeyId *string - AmazonS3SecretAccessKey *string - AmazonS3Bucket *string - AmazonS3Region *string - AmazonS3Endpoint *string - AmazonS3SSL *bool - AmazonS3SignV2 *bool - AmazonS3SSE *bool - AmazonS3Trace *bool + AmazonS3AccessKeyId *string `restricted:"true"` + AmazonS3SecretAccessKey *string `restricted:"true"` + AmazonS3Bucket *string `restricted:"true"` + AmazonS3Region *string `restricted:"true"` + AmazonS3Endpoint *string `restricted:"true"` + AmazonS3SSL *bool `restricted:"true"` + AmazonS3SignV2 *bool `restricted:"true"` + AmazonS3SSE *bool `restricted:"true"` + AmazonS3Trace *bool `restricted:"true"` } func (s *FileSettings) SetDefaults() { @@ -1051,12 +1051,12 @@ type EmailSettings struct { FeedbackEmail *string ReplyToAddress *string FeedbackOrganization *string - EnableSMTPAuth *bool - SMTPUsername *string - SMTPPassword *string - SMTPServer *string - SMTPPort *string - ConnectionSecurity *string + EnableSMTPAuth *bool `restricted:"true"` + SMTPUsername *string `restricted:"true"` + SMTPPassword *string `restricted:"true"` + SMTPServer *string `restricted:"true"` + SMTPPort *string `restricted:"true"` + ConnectionSecurity *string `restricted:"true"` SendPushNotifications *bool PushNotificationServer *string PushNotificationContents *string @@ -1064,7 +1064,7 @@ type EmailSettings struct { EmailBatchingBufferSize *int EmailBatchingInterval *int EnablePreviewModeBanner *bool - SkipServerCertificateVerification *bool + SkipServerCertificateVerification *bool `restricted:"true"` EmailNotificationContentsType *string LoginButtonColor *string LoginButtonBorderColor *string @@ -1202,13 +1202,13 @@ func (s *EmailSettings) SetDefaults() { } type RateLimitSettings struct { - Enable *bool - PerSec *int - MaxBurst *int - MemoryStoreSize *int - VaryByRemoteAddr *bool - VaryByUser *bool - VaryByHeader string + Enable *bool `restricted:"true"` + PerSec *int `restricted:"true"` + MaxBurst *int `restricted:"true"` + MemoryStoreSize *int `restricted:"true"` + VaryByRemoteAddr *bool `restricted:"true"` + VaryByUser *bool `restricted:"true"` + VaryByHeader string `restricted:"true"` } func (s *RateLimitSettings) SetDefaults() { @@ -1253,11 +1253,11 @@ func (s *PrivacySettings) setDefaults() { } type SupportSettings struct { - TermsOfServiceLink *string - PrivacyPolicyLink *string - AboutLink *string - HelpLink *string - ReportAProblemLink *string + TermsOfServiceLink *string `restricted:"true"` + PrivacyPolicyLink *string `restricted:"true"` + AboutLink *string `restricted:"true"` + HelpLink *string `restricted:"true"` + ReportAProblemLink *string `restricted:"true"` SupportEmail *string CustomTermsOfServiceEnabled *bool CustomTermsOfServiceReAcceptancePeriod *int @@ -1551,12 +1551,12 @@ func (s *TeamSettings) SetDefaults() { } type ClientRequirements struct { - AndroidLatestVersion string - AndroidMinVersion string - DesktopLatestVersion string - DesktopMinVersion string - IosLatestVersion string - IosMinVersion string + AndroidLatestVersion string `restricted:"true"` + AndroidMinVersion string `restricted:"true"` + DesktopLatestVersion string `restricted:"true"` + DesktopMinVersion string `restricted:"true"` + IosLatestVersion string `restricted:"true"` + IosMinVersion string `restricted:"true"` } type LdapSettings struct { @@ -1901,9 +1901,9 @@ func (s *SamlSettings) SetDefaults() { } type NativeAppSettings struct { - AppDownloadLink *string - AndroidAppDownloadLink *string - IosAppDownloadLink *string + AppDownloadLink *string `restricted:"true"` + AndroidAppDownloadLink *string `restricted:"true"` + IosAppDownloadLink *string `restricted:"true"` } func (s *NativeAppSettings) SetDefaults() { @@ -1921,25 +1921,25 @@ func (s *NativeAppSettings) SetDefaults() { } type ElasticsearchSettings struct { - ConnectionUrl *string - Username *string - Password *string - EnableIndexing *bool - EnableSearching *bool - EnableAutocomplete *bool - Sniff *bool - PostIndexReplicas *int - PostIndexShards *int - ChannelIndexReplicas *int - ChannelIndexShards *int - UserIndexReplicas *int - UserIndexShards *int - AggregatePostsAfterDays *int - PostsAggregatorJobStartTime *string - IndexPrefix *string - LiveIndexingBatchSize *int - BulkIndexingTimeWindowSeconds *int - RequestTimeoutSeconds *int + ConnectionUrl *string `restricted:"true"` + Username *string `restricted:"true"` + Password *string `restricted:"true"` + EnableIndexing *bool `restricted:"true"` + EnableSearching *bool `restricted:"true"` + EnableAutocomplete *bool `restricted:"true"` + Sniff *bool `restricted:"true"` + PostIndexReplicas *int `restricted:"true"` + PostIndexShards *int `restricted:"true"` + ChannelIndexReplicas *int `restricted:"true"` + ChannelIndexShards *int `restricted:"true"` + UserIndexReplicas *int `restricted:"true"` + UserIndexShards *int `restricted:"true"` + AggregatePostsAfterDays *int `restricted:"true"` + PostsAggregatorJobStartTime *string `restricted:"true"` + IndexPrefix *string `restricted:"true"` + LiveIndexingBatchSize *int `restricted:"true"` + BulkIndexingTimeWindowSeconds *int `restricted:"true"` + RequestTimeoutSeconds *int `restricted:"true"` } func (s *ElasticsearchSettings) SetDefaults() { @@ -2051,8 +2051,8 @@ func (s *DataRetentionSettings) SetDefaults() { } type JobSettings struct { - RunJobs *bool - RunScheduler *bool + RunJobs *bool `restricted:"true"` + RunScheduler *bool `restricted:"true"` } func (s *JobSettings) SetDefaults() { @@ -2071,9 +2071,9 @@ type PluginState struct { type PluginSettings struct { Enable *bool - EnableUploads *bool - Directory *string - ClientDirectory *string + EnableUploads *bool `restricted:"true"` + Directory *string `restricted:"true"` + ClientDirectory *string `restricted:"true"` Plugins map[string]map[string]interface{} PluginStates map[string]*PluginState } diff --git a/utils/merge.go b/utils/merge.go index 8ac86dbda7..968b4992a5 100644 --- a/utils/merge.go +++ b/utils/merge.go @@ -8,37 +8,40 @@ import ( "reflect" ) -// Merge will return a new struct/map/slice of the same type as base and patch, with patch merged into base. -// Specifically, patch's values will be preferred except when patch's value is `nil`. -// Note: a referenced value (eg. *bool) will only be `nil` if the pointer is nil. If the value is a zero value, -// then that is considered a legitimate value. Eg, *bool(false) will overwrite *bool(true). +// StructFieldFilter defines a callback function used to decide if a patch value should be applied. +type StructFieldFilter func(structField reflect.StructField, base reflect.Value, patch reflect.Value) bool + +// MergeConfig allows for optional merge customizations. +type MergeConfig struct { + StructFieldFilter StructFieldFilter +} + +// Merge will return a new value of the same type as base and patch, recursively merging non-nil values from patch on top of base. // // Restrictions/guarantees: -// - base and patch will not be modified -// - base and patch can be pointers or values // - base and patch must be the same type -// - if slices are different, this rule applies: -// - if patch is not nil, overwrite the base slice. -// - otherwise, keep the base slice -// - maps will be merged according to the following rules: -// - if patch is not nil, replace the base map completely -// - otherwise, keep the base map -// - reference values (eg. slice/ptr/map) will be cloned -// - channel values are not supported at the moment +// - base and patch will never be modified +// - values from patch are always selected when non-nil +// - structs are merged recursively +// - maps and slices are treated as pointers, and merged as a single value // -// Usage: callers need to cast the returned interface back into the original type, eg: +// Note that callers need to cast the returned interface back into the original type: // func mergeTestStruct(base, patch *testStruct) (*testStruct, error) { // ret, err := merge(base, patch) // if err != nil { // return nil, err // } +// // retTS := ret.(testStruct) // return &retTS, nil // } -func Merge(base interface{}, patch interface{}) (interface{}, error) { +func Merge(base interface{}, patch interface{}, mergeConfig *MergeConfig) (interface{}, error) { if reflect.TypeOf(base) != reflect.TypeOf(patch) { - return nil, fmt.Errorf("cannot merge different types. base type: %s, patch type: %s", - reflect.TypeOf(base), reflect.TypeOf(patch)) + return nil, fmt.Errorf( + "cannot merge different types. base type: %s, patch type: %s", + reflect.TypeOf(base), + reflect.TypeOf(patch), + ) } commonType := reflect.TypeOf(base) @@ -52,7 +55,7 @@ func Merge(base interface{}, patch interface{}) (interface{}, error) { ret := reflect.New(commonType) - val, ok := merge(baseVal, patchVal) + val, ok := merge(baseVal, patchVal, mergeConfig) if ok { ret.Elem().Set(val) } @@ -60,7 +63,7 @@ func Merge(base interface{}, patch interface{}) (interface{}, error) { } // merge recursively merges patch into base and returns the new struct, ptr, slice/map, or value -func merge(base, patch reflect.Value) (reflect.Value, bool) { +func merge(base, patch reflect.Value, mergeConfig *MergeConfig) (reflect.Value, bool) { commonType := base.Type() switch commonType.Kind() { @@ -70,7 +73,13 @@ func merge(base, patch reflect.Value) (reflect.Value, bool) { if !merged.Field(i).CanSet() { continue } - val, ok := merge(base.Field(i), patch.Field(i)) + if mergeConfig != nil && mergeConfig.StructFieldFilter != nil { + if !mergeConfig.StructFieldFilter(commonType.Field(i), base.Field(i), patch.Field(i)) { + merged.Field(i).Set(base.Field(i)) + continue + } + } + val, ok := merge(base.Field(i), patch.Field(i), mergeConfig) if ok { merged.Field(i).Set(val) } @@ -85,13 +94,13 @@ func merge(base, patch reflect.Value) (reflect.Value, bool) { // clone reference values (if any) if base.IsNil() { - val, _ := merge(patch.Elem(), patch.Elem()) + val, _ := merge(patch.Elem(), patch.Elem(), mergeConfig) mergedPtr.Elem().Set(val) } else if patch.IsNil() { - val, _ := merge(base.Elem(), base.Elem()) + val, _ := merge(base.Elem(), base.Elem(), mergeConfig) mergedPtr.Elem().Set(val) } else { - val, _ := merge(base.Elem(), patch.Elem()) + val, _ := merge(base.Elem(), patch.Elem(), mergeConfig) mergedPtr.Elem().Set(val) } return mergedPtr, true @@ -105,7 +114,7 @@ func merge(base, patch reflect.Value) (reflect.Value, bool) { merged := reflect.MakeSlice(commonType, 0, patch.Len()) for i := 0; i < patch.Len(); i++ { // recursively merge patch with itself. This will clone reference values. - val, _ := merge(patch.Index(i), patch.Index(i)) + val, _ := merge(patch.Index(i), patch.Index(i), mergeConfig) merged = reflect.Append(merged, val) } return merged, true @@ -115,7 +124,7 @@ func merge(base, patch reflect.Value) (reflect.Value, bool) { for i := 0; i < base.Len(); i++ { // recursively merge base with itself. This will clone reference values. - val, _ := merge(base.Index(i), base.Index(i)) + val, _ := merge(base.Index(i), base.Index(i), mergeConfig) merged = reflect.Append(merged, val) } return merged, true @@ -135,7 +144,7 @@ func merge(base, patch reflect.Value) (reflect.Value, bool) { } for _, key := range mapPtr.MapKeys() { // clone reference values - val, ok := merge(mapPtr.MapIndex(key), mapPtr.MapIndex(key)) + val, ok := merge(mapPtr.MapIndex(key), mapPtr.MapIndex(key), mergeConfig) if !ok { val = reflect.New(mapPtr.MapIndex(key).Type()).Elem() } @@ -151,11 +160,11 @@ func merge(base, patch reflect.Value) (reflect.Value, bool) { // clone reference values (if any) if base.IsNil() { - val, _ = merge(patch.Elem(), patch.Elem()) + val, _ = merge(patch.Elem(), patch.Elem(), mergeConfig) } else if patch.IsNil() { - val, _ = merge(base.Elem(), base.Elem()) + val, _ = merge(base.Elem(), base.Elem(), mergeConfig) } else { - val, _ = merge(base.Elem(), patch.Elem()) + val, _ = merge(base.Elem(), patch.Elem(), mergeConfig) } return val, true diff --git a/utils/merge_test.go b/utils/merge_test.go index 0a66dfbba9..668f6074f6 100644 --- a/utils/merge_test.go +++ b/utils/merge_test.go @@ -1,11 +1,14 @@ -package utils +package utils_test import ( "fmt" + "reflect" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/mattermost/mattermost-server/utils" ) // Test merging maps alone. This isolates the complexity of merging maps from merging maps recursively in @@ -1156,6 +1159,38 @@ func TestMergeWithVeryComplexStruct(t *testing.T) { }) } +func TestMergeWithStructFieldFilter(t *testing.T) { + t.Run("filter skips merging from patch", func(t *testing.T) { + t1 := evenSimpler{newBool(true), &evenSimpler2{newString("base")}} + t2 := evenSimpler{newBool(false), &evenSimpler2{newString("patch")}} + expected := evenSimpler{newBool(true), &evenSimpler2{newString("base")}} + + merged, err := mergeEvenSimplerWithConfig(t1, t2, &utils.MergeConfig{ + StructFieldFilter: func(structField reflect.StructField, base, patch reflect.Value) bool { + return false + }, + }) + require.NoError(t, err) + + assert.Equal(t, expected, *merged) + }) + + t.Run("filter skips merging configured fields from patch", func(t *testing.T) { + t1 := evenSimpler{newBool(true), &evenSimpler2{newString("base")}} + t2 := evenSimpler{newBool(false), &evenSimpler2{newString("patch")}} + expected := evenSimpler{newBool(false), &evenSimpler2{newString("base")}} + + merged, err := mergeEvenSimplerWithConfig(t1, t2, &utils.MergeConfig{ + StructFieldFilter: func(structField reflect.StructField, base, patch reflect.Value) bool { + return structField.Name == "B" + }, + }) + require.NoError(t, err) + + assert.Equal(t, expected, *merged) + }) +} + type testStruct struct { I int I8 int8 @@ -1504,11 +1539,10 @@ func setupStructs(t *testing.T) { map[int]*string{1: newString("Another"), 2: newString("map of"), 3: newString("pointers, wow!")}, mergeStructEmbedBaseA, &mergeStructEmbedBaseB, } - } func mergeSimple(base, patch simple) (*simple, error) { - ret, err := Merge(base, patch) + ret, err := utils.Merge(base, patch, nil) if err != nil { return nil, err } @@ -1517,7 +1551,16 @@ func mergeSimple(base, patch simple) (*simple, error) { } func mergeEvenSimpler(base, patch evenSimpler) (*evenSimpler, error) { - ret, err := Merge(base, patch) + ret, err := utils.Merge(base, patch, nil) + if err != nil { + return nil, err + } + retTS := ret.(evenSimpler) + return &retTS, nil +} + +func mergeEvenSimplerWithConfig(base, patch evenSimpler, mergeConfig *utils.MergeConfig) (*evenSimpler, error) { + ret, err := utils.Merge(base, patch, mergeConfig) if err != nil { return nil, err } @@ -1526,7 +1569,7 @@ func mergeEvenSimpler(base, patch evenSimpler) (*evenSimpler, error) { } func mergeSliceStruct(base, patch sliceStruct) (*sliceStruct, error) { - ret, err := Merge(base, patch) + ret, err := utils.Merge(base, patch, nil) if err != nil { return nil, err } @@ -1535,7 +1578,7 @@ func mergeSliceStruct(base, patch sliceStruct) (*sliceStruct, error) { } func mergeMapPtr(base, patch mapPtr) (*mapPtr, error) { - ret, err := Merge(base, patch) + ret, err := utils.Merge(base, patch, nil) if err != nil { return nil, err } @@ -1544,7 +1587,7 @@ func mergeMapPtr(base, patch mapPtr) (*mapPtr, error) { } func mergeMapPtrState(base, patch mapPtrState) (*mapPtrState, error) { - ret, err := Merge(base, patch) + ret, err := utils.Merge(base, patch, nil) if err != nil { return nil, err } @@ -1553,7 +1596,7 @@ func mergeMapPtrState(base, patch mapPtrState) (*mapPtrState, error) { } func mergeMapPtrState2(base, patch mapPtrState2) (*mapPtrState2, error) { - ret, err := Merge(base, patch) + ret, err := utils.Merge(base, patch, nil) if err != nil { return nil, err } @@ -1562,7 +1605,7 @@ func mergeMapPtrState2(base, patch mapPtrState2) (*mapPtrState2, error) { } func mergeTestStructs(base, patch testStruct) (*testStruct, error) { - ret, err := Merge(base, patch) + ret, err := utils.Merge(base, patch, nil) if err != nil { return nil, err } @@ -1571,7 +1614,7 @@ func mergeTestStructs(base, patch testStruct) (*testStruct, error) { } func mergeStringIntMap(base, patch map[string]int) (map[string]int, error) { - ret, err := Merge(base, patch) + ret, err := utils.Merge(base, patch, nil) if err != nil { return nil, err } @@ -1580,7 +1623,7 @@ func mergeStringIntMap(base, patch map[string]int) (map[string]int, error) { } func mergeStringPtrIntMap(base, patch map[string]*int) (map[string]*int, error) { - ret, err := Merge(base, patch) + ret, err := utils.Merge(base, patch, nil) if err != nil { return nil, err } @@ -1589,7 +1632,7 @@ func mergeStringPtrIntMap(base, patch map[string]*int) (map[string]*int, error) } func mergeStringSliceIntMap(base, patch map[string][]int) (map[string][]int, error) { - ret, err := Merge(base, patch) + ret, err := utils.Merge(base, patch, nil) if err != nil { return nil, err } @@ -1598,7 +1641,7 @@ func mergeStringSliceIntMap(base, patch map[string][]int) (map[string][]int, err } func mergeMapOfMap(base, patch map[string]map[string]*int) (map[string]map[string]*int, error) { - ret, err := Merge(base, patch) + ret, err := utils.Merge(base, patch, nil) if err != nil { return nil, err } @@ -1607,7 +1650,7 @@ func mergeMapOfMap(base, patch map[string]map[string]*int) (map[string]map[strin } func mergeInterfaceMap(base, patch map[string]interface{}) (map[string]interface{}, error) { - ret, err := Merge(base, patch) + ret, err := utils.Merge(base, patch, nil) if err != nil { return nil, err } @@ -1616,7 +1659,7 @@ func mergeInterfaceMap(base, patch map[string]interface{}) (map[string]interface } func mergeStringSlices(base, patch []string) ([]string, error) { - ret, err := Merge(base, patch) + ret, err := utils.Merge(base, patch, nil) if err != nil { return nil, err } @@ -1625,7 +1668,7 @@ func mergeStringSlices(base, patch []string) ([]string, error) { } func mergeTestStructsPtrs(base, patch *testStruct) (*testStruct, error) { - ret, err := Merge(base, patch) + ret, err := utils.Merge(base, patch, nil) if err != nil { return nil, err }