From a38c1c54b4ee58d467226eabf34383c35c8ede86 Mon Sep 17 00:00:00 2001 From: Scott Bishel Date: Thu, 1 Feb 2024 15:33:24 -0700 Subject: [PATCH] MM 55199 Limit User Preferences (#25579) * limit size and number of preferences in updatePreferences * update unit tests * fix another location and add tests * update utility function to return an error * update unit tests * fix review comments * revert package-lock.json * more review mods * update number of preferences allowed * update unit tests for new limit * update name of config setting * Update server/public/model/utils.go Co-authored-by: Ben Schumacher * add check for error id * use existing utility function, remove model.ObjectFromJSON() * Update preference.go - remove blank line --------- Co-authored-by: Ben Schumacher Co-authored-by: Mattermost Build --- api/v4/source/preferences.yaml | 4 ++ server/channels/api4/preference.go | 18 +++++-- server/channels/api4/preference_test.go | 72 +++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 4 deletions(-) diff --git a/api/v4/source/preferences.yaml b/api/v4/source/preferences.yaml index 4956051660..9a4c24bc49 100644 --- a/api/v4/source/preferences.yaml +++ b/api/v4/source/preferences.yaml @@ -59,6 +59,8 @@ type: array items: $ref: '#/components/schemas/Preference' + minItems: 1 + maxItems: 100 responses: "200": description: User preferences saved successful @@ -102,6 +104,8 @@ type: array items: $ref: '#/components/schemas/Preference' + minItems: 1 + maxItems: 100 responses: "200": description: User preferences saved successful diff --git a/server/channels/api4/preference.go b/server/channels/api4/preference.go index 14f3ee96b9..0eeb2fce7a 100644 --- a/server/channels/api4/preference.go +++ b/server/channels/api4/preference.go @@ -12,6 +12,8 @@ import ( "github.com/mattermost/mattermost/server/v8/channels/audit" ) +const maxUpdatePreferences = 100 + func (api *API) InitPreference() { api.BaseRoutes.Preferences.Handle("", api.APISessionRequired(getPreferences)).Methods("GET") api.BaseRoutes.Preferences.Handle("", api.APISessionRequired(updatePreferences)).Methods("PUT") @@ -101,8 +103,12 @@ func updatePreferences(c *Context, w http.ResponseWriter, r *http.Request) { } var preferences model.Preferences - if jsonErr := json.NewDecoder(r.Body).Decode(&preferences); jsonErr != nil { - c.SetInvalidParamWithErr("preferences", jsonErr) + err := model.StructFromJSONLimited(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes, &preferences) + if err != nil { + c.SetInvalidParamWithErr("preferences", err) + return + } else if len(preferences) == 0 || len(preferences) > maxUpdatePreferences { + c.SetInvalidParam("preferences") return } @@ -149,8 +155,12 @@ func deletePreferences(c *Context, w http.ResponseWriter, r *http.Request) { } var preferences model.Preferences - if jsonErr := json.NewDecoder(r.Body).Decode(&preferences); jsonErr != nil { - c.SetInvalidParamWithErr("preferences", jsonErr) + err := model.StructFromJSONLimited(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes, &preferences) + if err != nil { + c.SetInvalidParamWithErr("preferences", err) + return + } else if len(preferences) == 0 || len(preferences) > maxUpdatePreferences { + c.SetInvalidParam("preferences") return } diff --git a/server/channels/api4/preference_test.go b/server/channels/api4/preference_test.go index deba0c8250..3c8ab60ad2 100644 --- a/server/channels/api4/preference_test.go +++ b/server/channels/api4/preference_test.go @@ -257,6 +257,42 @@ func TestUpdatePreferences(t *testing.T) { CheckUnauthorizedStatus(t, resp) } +func TestUpdatePreferencesOverload(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + client := th.Client + + th.LoginBasic() + user1 := th.BasicUser + + t.Run("No preferences", func(t *testing.T) { + preferences1 := model.Preferences{} + // should error if no preferences + resp, err := client.UpdatePreferences(context.Background(), user1.Id, preferences1) + require.Error(t, err) + CheckErrorID(t, err, "api.context.invalid_body_param.app_error") + CheckBadRequestStatus(t, resp) + }) + + t.Run("Too many preferences", func(t *testing.T) { + preferences1 := model.Preferences{} + category := model.NewId() + // should error if too many preferences + for i := 0; i <= 100; i++ { + preferences1 = append(preferences1, model.Preference{ + UserId: user1.Id, + Category: category, + Name: model.NewId(), + Value: model.NewId(), + }) + } + resp, err := client.UpdatePreferences(context.Background(), user1.Id, preferences1) + require.Error(t, err) + CheckErrorID(t, err, "api.context.invalid_body_param.app_error") + CheckBadRequestStatus(t, resp) + }) +} + func TestUpdatePreferencesWebsocket(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() @@ -589,6 +625,42 @@ func TestDeletePreferences(t *testing.T) { CheckUnauthorizedStatus(t, resp) } +func TestDeletePreferencesOverload(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + client := th.Client + + th.LoginBasic() + user1 := th.BasicUser + + t.Run("No preferences", func(t *testing.T) { + preferences1 := model.Preferences{} + // should error if no preferences + resp, err := client.DeletePreferences(context.Background(), user1.Id, preferences1) + require.Error(t, err) + CheckErrorID(t, err, "api.context.invalid_body_param.app_error") + CheckBadRequestStatus(t, resp) + }) + + t.Run("Too many preferences", func(t *testing.T) { + category := model.NewId() + preferences1 := model.Preferences{} + // should error if too many preferences + for i := 0; i <= 100; i++ { + preferences1 = append(preferences1, model.Preference{ + UserId: user1.Id, + Category: category, + Name: model.NewId(), + Value: model.NewId(), + }) + } + resp, err := client.DeletePreferences(context.Background(), user1.Id, preferences1) + require.Error(t, err) + CheckErrorID(t, err, "api.context.invalid_body_param.app_error") + CheckBadRequestStatus(t, resp) + }) +} + func TestDeletePreferencesWebsocket(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown()