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

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
-}

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)

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)
-}

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
+}