MM-41323: Secure plugin marketplace modification (#19889)

We disable plugin marketplace modification if plugin uploads
are disabled

https://mattermost.atlassian.net/browse/MM-41323
```release-note
NONE
```

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

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

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)