mirror of
https://github.com/mattermost/mattermost.git
synced 2026-09-19 02:06:37 +08:00
MM-68976 Preserve PluginSettings.SignaturePublicKeyFiles on config patch endpoint (#36868)
* MM-68976 Preserve PluginSettings.SignaturePublicKeyFiles on config patch endpoint The full PUT /api/v4/config endpoint silently preserves PluginSettings.SignaturePublicKeyFiles (added in #13682), but the sparse PUT /api/v4/config/patch endpoint had no equivalent guard, so a session with sysconsole_write_plugins could modify the field through it. Mirror the full update endpoint's behavior by silently preserving the existing value in patchConfig, and add a regression test equivalent to the one in TestUpdateConfig. * MM-68976 Document SignaturePublicKeyFiles as non-modifiable in config API spec Both the update and patch config endpoints preserve PluginSettings.SignaturePublicKeyFiles; note this in the OpenAPI descriptions alongside the existing PluginSettings.EnableUploads note.
This commit is contained in:
@@ -673,8 +673,8 @@
|
||||
summary: Update configuration
|
||||
description: >
|
||||
Submit a new configuration for the server to use. As of server version
|
||||
4.8, the `PluginSettings.EnableUploads` setting cannot be modified by
|
||||
this endpoint.
|
||||
4.8, the `PluginSettings.EnableUploads` and `PluginSettings.SignaturePublicKeyFiles`
|
||||
settings cannot be modified by this endpoint.
|
||||
|
||||
Note that the parameters that aren't set in the configuration that you
|
||||
provide will be reset to default values. Therefore, if you want to
|
||||
@@ -815,8 +815,8 @@
|
||||
summary: Patch configuration
|
||||
description: >
|
||||
Submit configuration to patch. As of server version 4.8, the
|
||||
`PluginSettings.EnableUploads` setting cannot be modified by this
|
||||
endpoint.
|
||||
`PluginSettings.EnableUploads` and `PluginSettings.SignaturePublicKeyFiles`
|
||||
settings cannot be modified by this endpoint.
|
||||
|
||||
##### Permissions
|
||||
|
||||
|
||||
@@ -308,6 +308,11 @@ func patchConfig(c *Context, w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
// Do not allow certificates to be changed through the API. Mirror the full
|
||||
// update endpoint by silently preserving the existing value rather than
|
||||
// rejecting the request.
|
||||
cfg.PluginSettings.SignaturePublicKeyFiles = appCfg.PluginSettings.SignaturePublicKeyFiles
|
||||
|
||||
// Do not allow import directory to be changed through the API
|
||||
if cfg.ImportSettings.Directory != nil && *cfg.ImportSettings.Directory != *appCfg.ImportSettings.Directory {
|
||||
c.Err = model.NewAppError("patchConfig", "api.config.update_config.not_allowed_security.app_error", map[string]any{"Name": "ImportSettings.Directory"}, "", http.StatusForbidden)
|
||||
|
||||
@@ -912,6 +912,20 @@ func TestPatchConfig(t *testing.T) {
|
||||
assert.Equal(t, newURL, *cfg.PluginSettings.MarketplaceURL)
|
||||
})
|
||||
|
||||
t.Run("Should not be able to modify PluginSettings.SignaturePublicKeyFiles", func(t *testing.T) {
|
||||
// Mirror the behavior of the full update endpoint (TestUpdateConfig):
|
||||
// changes to this field are silently preserved, not rejected.
|
||||
oldPublicKeys := th.App.Config().PluginSettings.SignaturePublicKeyFiles
|
||||
|
||||
cfg := th.App.Config().Clone()
|
||||
cfg.PluginSettings.SignaturePublicKeyFiles = append(cfg.PluginSettings.SignaturePublicKeyFiles, "new_signature")
|
||||
|
||||
updatedConfig, _, err := th.SystemAdminClient.PatchConfig(context.Background(), cfg)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, oldPublicKeys, updatedConfig.PluginSettings.SignaturePublicKeyFiles)
|
||||
assert.Equal(t, oldPublicKeys, th.App.Config().PluginSettings.SignaturePublicKeyFiles)
|
||||
})
|
||||
|
||||
t.Run("System Admin should not be able to clear Site URL", func(t *testing.T) {
|
||||
cfg, _, err := th.SystemAdminClient.GetConfig(context.Background())
|
||||
require.NoError(t, err)
|
||||
|
||||
Reference in New Issue
Block a user