diff --git a/api4/config.go b/api4/config.go index 9eb9106d7b..6148e00e44 100644 --- a/api4/config.go +++ b/api4/config.go @@ -140,11 +140,18 @@ func updateConfig(c *Context, w http.ResponseWriter, r *http.Request) { } // Do not allow plugin uploads to be toggled through the API - cfg.PluginSettings.EnableUploads = appCfg.PluginSettings.EnableUploads + *cfg.PluginSettings.EnableUploads = *appCfg.PluginSettings.EnableUploads // Do not allow certificates to be changed through the API + // This shallow-copies the slice header. So be careful if there are concurrent + // modifications to the slice. cfg.PluginSettings.SignaturePublicKeyFiles = appCfg.PluginSettings.SignaturePublicKeyFiles + // Do not allow marketplace URL to be toggled through the API if EnableUploads are disabled. + if cfg.PluginSettings.EnableUploads != nil && !*appCfg.PluginSettings.EnableUploads { + *cfg.PluginSettings.MarketplaceURL = *appCfg.PluginSettings.MarketplaceURL + } + c.App.HandleMessageExportConfig(cfg, appCfg) if err := cfg.IsValid(); err != nil { @@ -260,6 +267,15 @@ func patchConfig(c *Context, w http.ResponseWriter, r *http.Request) { return } + // Do not allow marketplace URL to be toggled if plugin uploads are disabled. + if cfg.PluginSettings.MarketplaceURL != nil && cfg.PluginSettings.EnableUploads != nil { + // Breaking it down to 2 conditions to make it simple. + if *cfg.PluginSettings.MarketplaceURL != *appCfg.PluginSettings.MarketplaceURL && !*cfg.PluginSettings.EnableUploads { + c.Err = model.NewAppError("patchConfig", "api.config.update_config.not_allowed_security.app_error", map[string]interface{}{"Name": "PluginSettings.MarketplaceURL"}, "", http.StatusForbidden) + return + } + } + if cfg.MessageExportSettings.EnableExport != nil { c.App.HandleMessageExportConfig(cfg, appCfg) } diff --git a/api4/config_test.go b/api4/config_test.go index e0630598fa..86007f7555 100644 --- a/api4/config_test.go +++ b/api4/config_test.go @@ -218,6 +218,35 @@ func TestUpdateConfig(t *testing.T) { }) }) + t.Run("Should not be able to modify PluginSettings.MarketplaceURL if EnableUploads is disabled", func(t *testing.T) { + oldURL := "hello.com" + newURL := "new.com" + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.PluginSettings.EnableUploads = false + *cfg.PluginSettings.MarketplaceURL = oldURL + }) + + cfg2 := th.App.Config().Clone() + *cfg2.PluginSettings.MarketplaceURL = newURL + + cfg2, _, err = th.SystemAdminClient.UpdateConfig(cfg2) + require.NoError(t, err) + assert.Equal(t, oldURL, *cfg2.PluginSettings.MarketplaceURL) + + // Allowing uploads + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.PluginSettings.EnableUploads = true + *cfg.PluginSettings.MarketplaceURL = oldURL + }) + + cfg2 = th.App.Config().Clone() + *cfg2.PluginSettings.MarketplaceURL = newURL + + cfg2, _, err = th.SystemAdminClient.UpdateConfig(cfg2) + require.NoError(t, err) + assert.Equal(t, newURL, *cfg2.PluginSettings.MarketplaceURL) + }) + t.Run("System Admin should not be able to clear Site URL", func(t *testing.T) { siteURL := cfg.ServiceSettings.SiteURL defer th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.SiteURL = siteURL }) @@ -727,6 +756,34 @@ func TestPatchConfig(t *testing.T) { }) }) + t.Run("Should not be able to modify PluginSettings.MarketplaceURL if EnableUploads is disabled", func(t *testing.T) { + oldURL := "hello.com" + newURL := "new.com" + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.PluginSettings.EnableUploads = false + *cfg.PluginSettings.MarketplaceURL = oldURL + }) + + cfg := th.App.Config().Clone() + *cfg.PluginSettings.MarketplaceURL = newURL + + _, _, err := th.SystemAdminClient.PatchConfig(cfg) + require.Error(t, err) + + // Allowing uploads + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.PluginSettings.EnableUploads = true + *cfg.PluginSettings.MarketplaceURL = oldURL + }) + + cfg = th.App.Config().Clone() + *cfg.PluginSettings.MarketplaceURL = newURL + + cfg, _, err = th.SystemAdminClient.PatchConfig(cfg) + require.NoError(t, err) + assert.Equal(t, newURL, *cfg.PluginSettings.MarketplaceURL) + }) + t.Run("System Admin should not be able to clear Site URL", func(t *testing.T) { cfg, _, err := th.SystemAdminClient.GetConfig() require.NoError(t, err)