From 79d63193ac96d425050a4b04efeeafbb4eb7c53c Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Thu, 3 Feb 2022 14:38:08 +0530 Subject: [PATCH] MM-41184: Remove /config/migrate API endpoint (#19418) This was introduced to let the functionality happen via mmctl, but it does not differentiate between sysadmins vs people having physical access to machines. Therefore, we remove it and only keep the local endpoint accessible from mmctl --local. https://mattermost.atlassian.net/browse/MM-41184 ```release-note The api/v4/config/migrate API endpoint has been removed in favor of the mmctl --local endpoint. API clients won't be able to access this endpoint without having physical access to the server. ``` --- api4/config.go | 38 -------------------------------------- api4/config_test.go | 26 ++------------------------ i18n/en.json | 4 ---- model/client4.go | 3 +++ 4 files changed, 5 insertions(+), 66 deletions(-) diff --git a/api4/config.go b/api4/config.go index 2a6c0883d0..a6f875f0d1 100644 --- a/api4/config.go +++ b/api4/config.go @@ -35,7 +35,6 @@ func (api *API) InitConfig() { api.BaseRoutes.APIRoot.Handle("/config/reload", api.APISessionRequired(configReload)).Methods("POST") api.BaseRoutes.APIRoot.Handle("/config/client", api.APIHandler(getClientConfig)).Methods("GET") api.BaseRoutes.APIRoot.Handle("/config/environment", api.APISessionRequired(getEnvironmentConfig)).Methods("GET") - api.BaseRoutes.APIRoot.Handle("/config/migrate", api.APISessionRequired(migrateConfig)).Methods("POST") } func init() { @@ -385,40 +384,3 @@ func makeFilterConfigByPermission(accessType filterType) func(c *Context, struct return c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) } } - -func migrateConfig(c *Context, w http.ResponseWriter, r *http.Request) { - props := model.StringInterfaceFromJSON(r.Body) - from, ok := props["from"].(string) - if !ok { - c.SetInvalidParam("from") - return - } - to, ok := props["to"].(string) - if !ok { - c.SetInvalidParam("to") - return - } - - auditRec := c.MakeAuditRecord("migrateConfig", audit.Fail) - defer c.LogAuditRec(auditRec) - - if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) { - c.SetPermissionError(model.PermissionManageSystem) - return - } - - isCloud := c.App.Srv().License() != nil && *c.App.Srv().License().Features.Cloud - if isCloud { - c.Err = model.NewAppError("migrateConfig", "api.config.migrate_config_restricted.app_error", nil, "", http.StatusForbidden) - return - } - - err := config.Migrate(from, to) - if err != nil { - c.Err = model.NewAppError("migrateConfig", "api.config.migrate_config.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } - - auditRec.Success() - ReturnStatusOK(w) -} diff --git a/api4/config_test.go b/api4/config_test.go index 9c0379dc93..e0630598fa 100644 --- a/api4/config_test.go +++ b/api4/config_test.go @@ -770,13 +770,7 @@ func TestMigrateConfig(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - t.Run("user is not system admin", func(t *testing.T) { - response, err := th.Client.MigrateConfig("from", "to") - require.Error(t, err) - CheckForbiddenStatus(t, response) - }) - - th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { + t.Run("LocalClient", func(t *testing.T) { cfg := &model.Config{} cfg.SetDefaults() @@ -796,23 +790,7 @@ func TestMigrateConfig(t *testing.T) { require.NoError(t, err) defer f.RemoveFile("to.json") - _, err = client.MigrateConfig("from.json", "to.json") + _, err = th.LocalClient.MigrateConfig("from.json", "to.json") require.NoError(t, err) }) - - t.Run("Cloud instances should not access to this API", func(t *testing.T) { - require.True(t, th.App.Srv().SetLicense(model.NewTestLicense("cloud"))) - - f, err := config.NewStoreFromDSN("from.json", false, nil, true) - require.NoError(t, err) - defer f.RemoveFile("from.json") - - _, err = config.NewStoreFromDSN("to.json", false, nil, false) - require.NoError(t, err) - defer f.RemoveFile("to.json") - - response, cErr := th.SystemAdminClient.MigrateConfig("from.json", "to.json") - require.Error(t, cErr) - CheckForbiddenStatus(t, response) - }) } diff --git a/i18n/en.json b/i18n/en.json index 2a39d828c2..f30e7a727e 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -1454,10 +1454,6 @@ "id": "api.config.migrate_config.app_error", "translation": "Failed to migrate config store." }, - { - "id": "api.config.migrate_config_restricted.app_error", - "translation": "Config migration is restricted due to instance type." - }, { "id": "api.config.patch_config.diff.app_error", "translation": "Failed to diff configs" diff --git a/model/client4.go b/model/client4.go index b1a3cbea5a..30a75dd6dc 100644 --- a/model/client4.go +++ b/model/client4.go @@ -4438,6 +4438,9 @@ func (c *Client4) UpdateConfig(config *Config) (*Config, *Response, error) { } // MigrateConfig will migrate existing config to the new one. +// DEPRECATED: The config migrate API has been moved to be a purely +// mmctl --local endpoint. This method will be removed in a +// future major release. func (c *Client4) MigrateConfig(from, to string) (*Response, error) { m := make(map[string]string, 2) m["from"] = from