diff --git a/e2e-tests/cypress/tests/integration/channels/channel_settings/channel_header_spec.ts b/e2e-tests/cypress/tests/integration/channels/channel_settings/channel_header_spec.ts index 4fb91d9c3e1..9ddef7c5f0d 100644 --- a/e2e-tests/cypress/tests/integration/channels/channel_settings/channel_header_spec.ts +++ b/e2e-tests/cypress/tests/integration/channels/channel_settings/channel_header_spec.ts @@ -45,7 +45,7 @@ describe('Channel Settings', () => { }); // # Create DM with admin and user 1 - cy.apiCreateDirectChannel([user1.id, user1.id]).then(() => { + cy.apiCreateDirectChannel([admin.id, user1.id]).then(() => { // # Go to DM cy.visit(`/${testTeam.name}/messages/@${user1.username}`); diff --git a/server/channels/api4/channel.go b/server/channels/api4/channel.go index 3564ff4feef..0ef99b777bf 100644 --- a/server/channels/api4/channel.go +++ b/server/channels/api4/channel.go @@ -425,9 +425,18 @@ func restoreChannel(c *Context, w http.ResponseWriter, r *http.Request) { } func createDirectChannel(c *Context, w http.ResponseWriter, r *http.Request) { - userIds := model.ArrayFromJSON(r.Body) + userIds, err := model.NonSortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("createDirectChannel", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) + return + } allowed := false + // single userId allowed if creating a self-channel + // NonSortedArrayFromJSON will remove duplicates, so need to add back + if len(userIds) == 1 && userIds[0] == c.AppContext.Session().UserId { + userIds = append(userIds, userIds[0]) + } if len(userIds) != 2 { c.SetInvalidParam("user_ids") return @@ -464,9 +473,9 @@ func createDirectChannel(c *Context, w http.ResponseWriter, r *http.Request) { audit.AddEventParameter(auditRec, "user_id", otherUserId) - canSee, err := c.App.UserCanSeeOtherUser(c.AppContext, c.AppContext.Session().UserId, otherUserId) - if err != nil { - c.Err = err + canSee, appErr := c.App.UserCanSeeOtherUser(c.AppContext, c.AppContext.Session().UserId, otherUserId) + if appErr != nil { + c.Err = appErr return } @@ -475,9 +484,9 @@ func createDirectChannel(c *Context, w http.ResponseWriter, r *http.Request) { return } - sc, err := c.App.GetOrCreateDirectChannel(c.AppContext, userIds[0], userIds[1]) - if err != nil { - c.Err = err + sc, appErr := c.App.GetOrCreateDirectChannel(c.AppContext, userIds[0], userIds[1]) + if appErr != nil { + c.Err = appErr return } @@ -511,9 +520,11 @@ func searchGroupChannels(c *Context, w http.ResponseWriter, r *http.Request) { } func createGroupChannel(c *Context, w http.ResponseWriter, r *http.Request) { - userIds := model.ArrayFromJSON(r.Body) - - if len(userIds) == 0 { + userIds, err := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("createGroupChannel", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) + return + } else if len(userIds) == 0 { c.SetInvalidParam("user_ids") return } @@ -561,9 +572,9 @@ func createGroupChannel(c *Context, w http.ResponseWriter, r *http.Request) { return } - groupChannel, err := c.App.CreateGroupChannel(c.AppContext, userIds, c.AppContext.Session().UserId) - if err != nil { - c.Err = err + groupChannel, appErr := c.App.CreateGroupChannel(c.AppContext, userIds, c.AppContext.Session().UserId) + if appErr != nil { + c.Err = appErr return } @@ -697,7 +708,12 @@ func getChannelsMemberCount(c *Context, w http.ResponseWriter, r *http.Request) return } - channelIDs := model.ArrayFromJSON(r.Body) + channelIDs, sortErr := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if sortErr != nil { + c.Err = model.NewAppError("getChannelsMemberCount", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(sortErr) + return + } + channels, err := c.App.GetChannels(c.AppContext, channelIDs) if err != nil { c.Err = err @@ -711,10 +727,9 @@ func getChannelsMemberCount(c *Context, w http.ResponseWriter, r *http.Request) } } - channelsMemberCount, err := c.App.GetChannelsMemberCount(c.AppContext, channelIDs) - - if err != nil { - c.Err = err + channelsMemberCount, appErr := c.App.GetChannelsMemberCount(c.AppContext, channelIDs) + if appErr != nil { + c.Err = appErr return } @@ -899,8 +914,11 @@ func getPublicChannelsByIdsForTeam(c *Context, w http.ResponseWriter, r *http.Re return } - channelIds := model.ArrayFromJSON(r.Body) - if len(channelIds) == 0 { + channelIds, err := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("getPublicChannelsByIdsForTeam", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) + return + } else if len(channelIds) == 0 { c.SetInvalidParam("channel_ids") return } @@ -917,15 +935,15 @@ func getPublicChannelsByIdsForTeam(c *Context, w http.ResponseWriter, r *http.Re return } - channels, err := c.App.GetPublicChannelsByIdsForTeam(c.AppContext, c.Params.TeamId, channelIds) - if err != nil { - c.Err = err + channels, appErr := c.App.GetPublicChannelsByIdsForTeam(c.AppContext, c.Params.TeamId, channelIds) + if appErr != nil { + c.Err = appErr return } - err = c.App.FillInChannelsProps(c.AppContext, channels) - if err != nil { - c.Err = err + appErr = c.App.FillInChannelsProps(c.AppContext, channels) + if appErr != nil { + c.Err = appErr return } @@ -1439,8 +1457,11 @@ func getChannelMembersByIds(c *Context, w http.ResponseWriter, r *http.Request) return } - userIds := model.ArrayFromJSON(r.Body) - if len(userIds) == 0 { + userIds, err := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("getChannelMembersByIds", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) + return + } else if len(userIds) == 0 { c.SetInvalidParam("user_ids") return } @@ -1450,9 +1471,9 @@ func getChannelMembersByIds(c *Context, w http.ResponseWriter, r *http.Request) return } - members, err := c.App.GetChannelMembersByIds(c.AppContext, c.Params.ChannelId, userIds) - if err != nil { - c.Err = err + members, appErr := c.App.GetChannelMembersByIds(c.AppContext, c.Params.ChannelId, userIds) + if appErr != nil { + c.Err = appErr return } @@ -1562,14 +1583,16 @@ func viewChannel(c *Context, w http.ResponseWriter, r *http.Request) { func readMultipleChannels(c *Context, w http.ResponseWriter, r *http.Request) { c.RequireUserId() - var channelIDs []string - err := json.NewDecoder(r.Body).Decode(&channelIDs) - if err != nil || len(channelIDs) == 0 { - c.SetInvalidParamWithErr("channel_ids", err) + channelIds, err := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("readMultipleChannels", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) + return + } else if len(channelIds) == 0 { + c.SetInvalidParam("channel_ids") return } - times, appErr := c.App.MarkChannelsAsViewed(c.AppContext, channelIDs, c.Params.UserId, c.AppContext.Session().Id, true, c.App.IsCRTEnabledForUser(c.AppContext, c.Params.UserId)) + times, appErr := c.App.MarkChannelsAsViewed(c.AppContext, channelIds, c.Params.UserId, c.AppContext.Session().Id, true, c.App.IsCRTEnabledForUser(c.AppContext, c.Params.UserId)) if appErr != nil { c.Err = appErr return diff --git a/server/channels/api4/channel_category.go b/server/channels/api4/channel_category.go index 5401bc6703d..c3e8c27b475 100644 --- a/server/channels/api4/channel_category.go +++ b/server/channels/api4/channel_category.go @@ -118,7 +118,11 @@ func updateCategoryOrderForTeamForUser(c *Context, w http.ResponseWriter, r *htt auditRec := c.MakeAuditRecord("updateCategoryOrderForTeamForUser", audit.Fail) defer c.LogAuditRec(auditRec) - categoryOrder := model.ArrayFromJSON(r.Body) + categoryOrder, err := model.NonSortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("updateCategoryOrderForTeamForUser", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) + return + } for _, categoryId := range categoryOrder { if !c.App.SessionHasPermissionToCategory(c.AppContext, *c.AppContext.Session(), c.Params.UserId, c.Params.TeamId, categoryId) { @@ -127,9 +131,9 @@ func updateCategoryOrderForTeamForUser(c *Context, w http.ResponseWriter, r *htt } } - err := c.App.UpdateSidebarCategoryOrder(c.AppContext, c.Params.UserId, c.Params.TeamId, categoryOrder) - if err != nil { - c.Err = err + appErr := c.App.UpdateSidebarCategoryOrder(c.AppContext, c.Params.UserId, c.Params.TeamId, categoryOrder) + if appErr != nil { + c.Err = appErr return } diff --git a/server/channels/api4/data_retention.go b/server/channels/api4/data_retention.go index 64d7053e234..3393fccf86a 100644 --- a/server/channels/api4/data_retention.go +++ b/server/channels/api4/data_retention.go @@ -264,10 +264,9 @@ func searchTeamsInPolicy(c *Context, w http.ResponseWriter, r *http.Request) { func addTeamsToPolicy(c *Context, w http.ResponseWriter, r *http.Request) { c.RequirePolicyId() policyId := c.Params.PolicyId - var teamIDs []string - jsonErr := json.NewDecoder(r.Body).Decode(&teamIDs) - if jsonErr != nil { - c.SetInvalidParamWithErr("team_ids", jsonErr) + teamIDs, err := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("addTeamsToPolicy", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) return } auditRec := c.MakeAuditRecord("addTeamsToPolicy", audit.Fail) @@ -279,9 +278,9 @@ func addTeamsToPolicy(c *Context, w http.ResponseWriter, r *http.Request) { return } - err := c.App.AddTeamsToRetentionPolicy(policyId, teamIDs) - if err != nil { - c.Err = err + appErr := c.App.AddTeamsToRetentionPolicy(policyId, teamIDs) + if appErr != nil { + c.Err = appErr return } @@ -292,10 +291,9 @@ func addTeamsToPolicy(c *Context, w http.ResponseWriter, r *http.Request) { func removeTeamsFromPolicy(c *Context, w http.ResponseWriter, r *http.Request) { c.RequirePolicyId() policyId := c.Params.PolicyId - var teamIDs []string - jsonErr := json.NewDecoder(r.Body).Decode(&teamIDs) - if jsonErr != nil { - c.SetInvalidParamWithErr("team_ids", jsonErr) + teamIDs, err := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("removeTeamsFromPolicy", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) return } auditRec := c.MakeAuditRecord("removeTeamsFromPolicy", audit.Fail) @@ -308,9 +306,9 @@ func removeTeamsFromPolicy(c *Context, w http.ResponseWriter, r *http.Request) { return } - err := c.App.RemoveTeamsFromRetentionPolicy(policyId, teamIDs) - if err != nil { - c.Err = err + appErr := c.App.RemoveTeamsFromRetentionPolicy(policyId, teamIDs) + if appErr != nil { + c.Err = appErr return } @@ -385,10 +383,9 @@ func searchChannelsInPolicy(c *Context, w http.ResponseWriter, r *http.Request) func addChannelsToPolicy(c *Context, w http.ResponseWriter, r *http.Request) { c.RequirePolicyId() policyId := c.Params.PolicyId - var channelIDs []string - jsonErr := json.NewDecoder(r.Body).Decode(&channelIDs) - if jsonErr != nil { - c.SetInvalidParamWithErr("channel_ids", jsonErr) + channelIDs, err := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("addChannelsToPolicy", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) return } auditRec := c.MakeAuditRecord("addChannelsToPolicy", audit.Fail) @@ -401,9 +398,9 @@ func addChannelsToPolicy(c *Context, w http.ResponseWriter, r *http.Request) { return } - err := c.App.AddChannelsToRetentionPolicy(policyId, channelIDs) - if err != nil { - c.Err = err + appErr := c.App.AddChannelsToRetentionPolicy(policyId, channelIDs) + if appErr != nil { + c.Err = appErr return } @@ -414,10 +411,9 @@ func addChannelsToPolicy(c *Context, w http.ResponseWriter, r *http.Request) { func removeChannelsFromPolicy(c *Context, w http.ResponseWriter, r *http.Request) { c.RequirePolicyId() policyId := c.Params.PolicyId - var channelIDs []string - jsonErr := json.NewDecoder(r.Body).Decode(&channelIDs) - if jsonErr != nil { - c.SetInvalidParamWithErr("channel_ids", jsonErr) + channelIDs, err := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("removeChannelsFromPolicy", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) return } auditRec := c.MakeAuditRecord("removeChannelsFromPolicy", audit.Fail) @@ -430,9 +426,9 @@ func removeChannelsFromPolicy(c *Context, w http.ResponseWriter, r *http.Request return } - err := c.App.RemoveChannelsFromRetentionPolicy(policyId, channelIDs) - if err != nil { - c.Err = err + appErr := c.App.RemoveChannelsFromRetentionPolicy(policyId, channelIDs) + if appErr != nil { + c.Err = appErr return } diff --git a/server/channels/api4/emoji.go b/server/channels/api4/emoji.go index 4d292675123..33ec728af2e 100644 --- a/server/channels/api4/emoji.go +++ b/server/channels/api4/emoji.go @@ -240,8 +240,11 @@ func getEmojiByName(c *Context, w http.ResponseWriter, r *http.Request) { } func getEmojisByNames(c *Context, w http.ResponseWriter, r *http.Request) { - names := model.ArrayFromJSON(r.Body) - if len(names) == 0 { + names, err := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("getEmojisByNames", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) + return + } else if len(names) == 0 { c.SetInvalidParam("names") return } @@ -258,9 +261,9 @@ func getEmojisByNames(c *Context, w http.ResponseWriter, r *http.Request) { return } - emojis, err := c.App.GetMultipleEmojiByName(c.AppContext, names) - if err != nil { - c.Err = err + emojis, appErr := c.App.GetMultipleEmojiByName(c.AppContext, names) + if appErr != nil { + c.Err = appErr return } diff --git a/server/channels/api4/emoji_test.go b/server/channels/api4/emoji_test.go index fcfe58519f1..b9d604824d0 100644 --- a/server/channels/api4/emoji_test.go +++ b/server/channels/api4/emoji_test.go @@ -316,25 +316,34 @@ func TestGetEmojisByNames(t *testing.T) { t.Run("should return multiple emojis", func(t *testing.T) { emojis, _, err := client.GetEmojisByNames(context.Background(), []string{emoji1.Name, emoji2.Name}) + var emojiIds []string + for _, emoji := range emojis { + emojiIds = append(emojiIds, emoji.Id) + } + require.NoError(t, err) require.Len(t, emojis, 2) - assert.Equal(t, emoji1.Id, emojis[0].Id) - assert.Equal(t, emoji2.Id, emojis[1].Id) + assert.Contains(t, emojiIds, emoji1.Id) + assert.Contains(t, emojiIds, emoji2.Id) }) t.Run("should ignore non-existent emojis", func(t *testing.T) { emojis, _, err := client.GetEmojisByNames(context.Background(), []string{emoji1.Name, emoji2.Name, model.NewId()}) + var emojiIds []string + for _, emoji := range emojis { + emojiIds = append(emojiIds, emoji.Id) + } require.NoError(t, err) require.Len(t, emojis, 2) - assert.Equal(t, emoji1.Id, emojis[0].Id) - assert.Equal(t, emoji2.Id, emojis[1].Id) + assert.Contains(t, emojiIds, emoji1.Id) + assert.Contains(t, emojiIds, emoji2.Id) }) t.Run("should return an error when too many emojis are requested", func(t *testing.T) { names := make([]string, GetEmojisByNamesMax+1) for i := 0; i < len(names); i++ { - names[i] = emoji1.Name + names[i] = model.NewId() } _, _, err := client.GetEmojisByNames(context.Background(), names) diff --git a/server/channels/api4/post.go b/server/channels/api4/post.go index 090c195cbd0..54754fa352b 100644 --- a/server/channels/api4/post.go +++ b/server/channels/api4/post.go @@ -504,9 +504,11 @@ func getPost(c *Context, w http.ResponseWriter, r *http.Request) { // getPostsByIds also sets a header to indicate, if posts were truncated as per the cloud plan's limit. func getPostsByIds(c *Context, w http.ResponseWriter, r *http.Request) { - postIDs := model.ArrayFromJSON(r.Body) - - if len(postIDs) == 0 { + postIDs, err := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("getPostsByIds", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) + return + } else if len(postIDs) == 0 { c.SetInvalidParam("post_ids") return } @@ -516,9 +518,9 @@ func getPostsByIds(c *Context, w http.ResponseWriter, r *http.Request) { return } - postsList, firstInaccessiblePostTime, err := c.App.GetPostsByIds(postIDs) - if err != nil { - c.Err = err + postsList, firstInaccessiblePostTime, appErr := c.App.GetPostsByIds(postIDs) + if appErr != nil { + c.Err = appErr return } @@ -530,9 +532,9 @@ func getPostsByIds(c *Context, w http.ResponseWriter, r *http.Request) { if val, ok := channelMap[post.ChannelId]; ok { channel = val } else { - channel, err = c.App.GetChannel(c.AppContext, post.ChannelId) - if err != nil { - c.Err = err + channel, appErr = c.App.GetChannel(c.AppContext, post.ChannelId) + if appErr != nil { + c.Err = appErr return } channelMap[channel.Id] = channel diff --git a/server/channels/api4/reaction.go b/server/channels/api4/reaction.go index e0cf409a3d3..0c4d555fbc1 100644 --- a/server/channels/api4/reaction.go +++ b/server/channels/api4/reaction.go @@ -119,7 +119,11 @@ func deleteReaction(c *Context, w http.ResponseWriter, r *http.Request) { } func getBulkReactions(c *Context, w http.ResponseWriter, r *http.Request) { - postIds := model.ArrayFromJSON(r.Body) + postIds, err := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("getBulkReactions", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) + return + } for _, postId := range postIds { if !c.App.SessionHasPermissionToChannelByPost(*c.AppContext.Session(), postId, model.PermissionReadChannelContent) { c.SetPermissionError(model.PermissionReadChannelContent) diff --git a/server/channels/api4/role.go b/server/channels/api4/role.go index ba499fe09c1..6be42aab4a3 100644 --- a/server/channels/api4/role.go +++ b/server/channels/api4/role.go @@ -84,9 +84,11 @@ func getRoleByName(c *Context, w http.ResponseWriter, r *http.Request) { } func getRolesByNames(c *Context, w http.ResponseWriter, r *http.Request) { - rolenames := model.ArrayFromJSON(r.Body) - - if len(rolenames) == 0 { + rolenames, err := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("getRolesByNames", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) + return + } else if len(rolenames) == 0 { c.SetInvalidParam("rolenames") return } diff --git a/server/channels/api4/role_test.go b/server/channels/api4/role_test.go index dfcea31ee34..7cdcd27cca6 100644 --- a/server/channels/api4/role_test.go +++ b/server/channels/api4/role_test.go @@ -5,6 +5,7 @@ package api4 import ( "context" + "fmt" "sort" "strings" "testing" @@ -189,7 +190,7 @@ func TestGetRolesByNames(t *testing.T) { // too many roles should error with bad request roles := []string{} for i := 0; i < GetRolesByNamesMax+10; i++ { - roles = append(roles, role1.Name) + roles = append(roles, fmt.Sprintf("role1.Name%v", i)) } _, resp, err := client.GetRolesByNames(context.Background(), roles) diff --git a/server/channels/api4/status.go b/server/channels/api4/status.go index 124ab3d3b86..921fea718fa 100644 --- a/server/channels/api4/status.go +++ b/server/channels/api4/status.go @@ -49,9 +49,11 @@ func getUserStatus(c *Context, w http.ResponseWriter, r *http.Request) { } func getUserStatusesByIds(c *Context, w http.ResponseWriter, r *http.Request) { - userIds := model.ArrayFromJSON(r.Body) - - if len(userIds) == 0 { + userIds, err := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("getUserStatusesByIds", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) + return + } else if len(userIds) == 0 { c.SetInvalidParam("user_ids") return } diff --git a/server/channels/api4/system.go b/server/channels/api4/system.go index a220ba832fc..49f3feadd9d 100644 --- a/server/channels/api4/system.go +++ b/server/channels/api4/system.go @@ -967,7 +967,11 @@ func updateViewedProductNotices(c *Context, w http.ResponseWriter, r *http.Reque defer c.LogAuditRec(auditRec) c.LogAudit("attempt") - ids := model.ArrayFromJSON(r.Body) + ids, err := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("updateViewedProductNotices", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) + return + } appErr := c.App.UpdateViewedProductNotices(c.AppContext.Session().UserId, ids) if appErr != nil { c.Err = appErr diff --git a/server/channels/api4/team.go b/server/channels/api4/team.go index 8c6ac8e9ec7..1a568d4e15e 100644 --- a/server/channels/api4/team.go +++ b/server/channels/api4/team.go @@ -644,10 +644,12 @@ func getTeamMembersByIds(c *Context, w http.ResponseWriter, r *http.Request) { return } - var userIDs []string - err := json.NewDecoder(r.Body).Decode(&userIDs) - if err != nil || len(userIDs) == 0 { - c.SetInvalidParamWithErr("user_ids", err) + userIDs, err := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("getTeamMembersByIds", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) + return + } else if len(userIDs) == 0 { + c.SetInvalidParam("user_ids") return } diff --git a/server/channels/api4/user.go b/server/channels/api4/user.go index 3c1dfff7388..307b04ddabd 100644 --- a/server/channels/api4/user.go +++ b/server/channels/api4/user.go @@ -623,9 +623,11 @@ func getFilteredUsersStats(c *Context, w http.ResponseWriter, r *http.Request) { } func getUsersByGroupChannelIds(c *Context, w http.ResponseWriter, r *http.Request) { - channelIds := model.ArrayFromJSON(r.Body) - - if len(channelIds) == 0 { + channelIds, err := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil || len(channelIds) == 0 { + c.Err = model.NewAppError("getUsersByGroupChannelIds", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) + return + } else if len(channelIds) == 0 { c.SetInvalidParam("channel_ids") return } @@ -636,7 +638,7 @@ func getUsersByGroupChannelIds(c *Context, w http.ResponseWriter, r *http.Reques return } - err := json.NewEncoder(w).Encode(usersByChannelId) + err = json.NewEncoder(w).Encode(usersByChannelId) if err != nil { c.Logger.Warn("Error writing response", mlog.Err(err)) } @@ -953,17 +955,15 @@ func requireGroupAccess(c *web.Context, groupID string) *model.AppError { } func getUsersByIds(c *Context, w http.ResponseWriter, r *http.Request) { - var userIDs []string - err := json.NewDecoder(r.Body).Decode(&userIDs) - if err != nil || len(userIDs) == 0 { - c.SetInvalidParamWithErr("user_ids", err) + userIDs, err := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("getUsersByIds", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) + return + } else if len(userIDs) == 0 { + c.SetInvalidParam("user_ids") return } - // we remove the duplicate IDs as it can bring a significant load to the - // database. - userIDs = model.RemoveDuplicateStrings(userIDs) - sinceString := r.URL.Query().Get("since") options := &store.UserGetByIdsOpts{ @@ -1002,10 +1002,12 @@ func getUsersByIds(c *Context, w http.ResponseWriter, r *http.Request) { } func getUsersByNames(c *Context, w http.ResponseWriter, r *http.Request) { - var usernames []string - err := json.NewDecoder(r.Body).Decode(&usernames) - if err != nil || len(usernames) == 0 { - c.SetInvalidParamWithErr("usernames", err) + usernames, err := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("getUsersByNames", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) + return + } else if len(usernames) == 0 { + c.SetInvalidParam("usernames") return } diff --git a/server/channels/api4/user_local.go b/server/channels/api4/user_local.go index 2171d14f7cd..cf7ffac59b1 100644 --- a/server/channels/api4/user_local.go +++ b/server/channels/api4/user_local.go @@ -233,17 +233,15 @@ func localGetUsers(c *Context, w http.ResponseWriter, r *http.Request) { } func localGetUsersByIds(c *Context, w http.ResponseWriter, r *http.Request) { - userIDs := model.ArrayFromJSON(r.Body) - - if len(userIDs) == 0 { + userIDs, err := model.SortedArrayFromJSON(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes) + if err != nil { + c.Err = model.NewAppError("localGetUsersByIds", model.PayloadParseError, nil, "", http.StatusBadRequest).Wrap(err) + return + } else if len(userIDs) == 0 { c.SetInvalidParam("user_ids") return } - // we remove the duplicate IDs as it can bring a significant load to the - // database. - userIDs = model.RemoveDuplicateStrings(userIDs) - sinceString := r.URL.Query().Get("since") options := &store.UserGetByIdsOpts{ @@ -251,9 +249,9 @@ func localGetUsersByIds(c *Context, w http.ResponseWriter, r *http.Request) { } if sinceString != "" { - since, err := strconv.ParseInt(sinceString, 10, 64) - if err != nil { - c.SetInvalidParamWithErr("since", err) + since, parseErr := strconv.ParseInt(sinceString, 10, 64) + if parseErr != nil { + c.SetInvalidParamWithErr("since", parseErr) return } options.Since = since diff --git a/server/i18n/en.json b/server/i18n/en.json index 4223c820022..2813cbb21d4 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -2376,6 +2376,10 @@ "id": "api.outgoing_webhook.disabled.app_error", "translation": "Outgoing webhooks have been disabled by the system admin." }, + { + "id": "api.payload.parse.error", + "translation": "An error occurred while parsing the payload." + }, { "id": "api.plugin.install.download_failed.app_error", "translation": "An error occurred while downloading the plugin." @@ -8926,6 +8930,10 @@ "id": "model.config.is_valid.max_notify_per_channel.app_error", "translation": "Invalid maximum notifications per channel for team settings. Must be a positive number." }, + { + "id": "model.config.is_valid.max_payload_size.app_error", + "translation": "Invalid max payload size for service settings. Must be a whole number greater than zero." + }, { "id": "model.config.is_valid.max_users.app_error", "translation": "Invalid maximum users per team for team settings. Must be a positive number." diff --git a/server/platform/services/telemetry/telemetry.go b/server/platform/services/telemetry/telemetry.go index 1598d9e0127..7fe795861d4 100644 --- a/server/platform/services/telemetry/telemetry.go +++ b/server/platform/services/telemetry/telemetry.go @@ -495,6 +495,7 @@ func (ts *TelemetryService) trackConfig() { "self_hosted_purchase": *cfg.ServiceSettings.SelfHostedPurchase, "allow_synced_drafts": *cfg.ServiceSettings.AllowSyncedDrafts, "refresh_post_stats_run_time": *cfg.ServiceSettings.RefreshPostStatsRunTime, + "maximum_payload_size": *cfg.ServiceSettings.MaximumPayloadSizeBytes, }) ts.SendTelemetry(TrackConfigTeam, map[string]any{ diff --git a/server/public/model/client4.go b/server/public/model/client4.go index 08ef2ac0ad2..6cfa9b4eaae 100644 --- a/server/public/model/client4.go +++ b/server/public/model/client4.go @@ -103,6 +103,15 @@ func (c *Client4) boolString(value bool) string { return "false" } +func (c *Client4) ArrayFromJSON(data io.Reader) []string { + var objmap []string + json.NewDecoder(data).Decode(&objmap) + if objmap == nil { + return make([]string, 0) + } + return objmap +} + func closeBody(r *http.Response) { if r.Body != nil { _, _ = io.Copy(io.Discard, r.Body) @@ -3136,7 +3145,7 @@ func (c *Client4) GetChannelMembersTimezones(ctx context.Context, channelId stri return nil, BuildResponse(r), err } defer closeBody(r) - return ArrayFromJSON(r.Body), BuildResponse(r), nil + return c.ArrayFromJSON(r.Body), BuildResponse(r), nil } // GetPinnedPosts gets a list of pinned posts. @@ -5818,7 +5827,7 @@ func (c *Client4) GetLogs(ctx context.Context, page, perPage int) ([]string, *Re return nil, BuildResponse(r), err } defer closeBody(r) - return ArrayFromJSON(r.Body), BuildResponse(r), nil + return c.ArrayFromJSON(r.Body), BuildResponse(r), nil } // PostLog is a convenience Web Service call so clients can log messages into @@ -7922,7 +7931,7 @@ func (c *Client4) GetSidebarCategoryOrderForTeamForUser(ctx context.Context, use return nil, BuildResponse(r), err } defer closeBody(r) - return ArrayFromJSON(r.Body), BuildResponse(r), nil + return c.ArrayFromJSON(r.Body), BuildResponse(r), nil } func (c *Client4) UpdateSidebarCategoryOrderForTeamForUser(ctx context.Context, userID, teamID string, order []string) ([]string, *Response, error) { @@ -7936,7 +7945,7 @@ func (c *Client4) UpdateSidebarCategoryOrderForTeamForUser(ctx context.Context, return nil, BuildResponse(r), err } defer closeBody(r) - return ArrayFromJSON(r.Body), BuildResponse(r), nil + return c.ArrayFromJSON(r.Body), BuildResponse(r), nil } func (c *Client4) GetSidebarCategoryForTeamForUser(ctx context.Context, userID, teamID, categoryID, etag string) (*SidebarCategoryWithChannels, *Response, error) { @@ -8396,7 +8405,7 @@ func (c *Client4) ListImports(ctx context.Context) ([]string, *Response, error) return nil, BuildResponse(r), err } defer closeBody(r) - return ArrayFromJSON(r.Body), BuildResponse(r), nil + return c.ArrayFromJSON(r.Body), BuildResponse(r), nil } func (c *Client4) ListExports(ctx context.Context) ([]string, *Response, error) { @@ -8405,7 +8414,7 @@ func (c *Client4) ListExports(ctx context.Context) ([]string, *Response, error) return nil, BuildResponse(r), err } defer closeBody(r) - return ArrayFromJSON(r.Body), BuildResponse(r), nil + return c.ArrayFromJSON(r.Body), BuildResponse(r), nil } func (c *Client4) DeleteExport(ctx context.Context, name string) (*Response, error) { diff --git a/server/public/model/config.go b/server/public/model/config.go index 06226ccb254..ec889947708 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -402,6 +402,7 @@ type ServiceSettings struct { AllowSyncedDrafts *bool `access:"site_posts"` UniqueEmojiReactionLimitPerPost *int `access:"site_posts"` RefreshPostStatsRunTime *string `access:"site_users_and_teams"` + MaximumPayloadSizeBytes *int64 `access:"environment_file_storage,write_restrictable,cloud_restrictable"` } var MattermostGiphySdkKey string @@ -908,6 +909,10 @@ func (s *ServiceSettings) SetDefaults(isUpdate bool) { if s.RefreshPostStatsRunTime == nil { s.RefreshPostStatsRunTime = NewString("00:00") } + + if s.MaximumPayloadSizeBytes == nil { + s.MaximumPayloadSizeBytes = NewInt64(100000) + } } type ClusterSettings struct { @@ -3978,6 +3983,10 @@ func (s *ServiceSettings) isValid() *AppError { } } + if *s.MaximumPayloadSizeBytes <= 0 { + return NewAppError("Config.IsValid", "model.config.is_valid.max_payload_size.app_error", nil, "", http.StatusBadRequest) + } + if *s.ReadTimeout <= 0 { return NewAppError("Config.IsValid", "model.config.is_valid.read_timeout.app_error", nil, "", http.StatusBadRequest) } diff --git a/server/public/model/utils.go b/server/public/model/utils.go index 01559feabff..aac53e6b9a2 100644 --- a/server/public/model/utils.go +++ b/server/public/model/utils.go @@ -29,13 +29,14 @@ import ( ) const ( - LowercaseLetters = "abcdefghijklmnopqrstuvwxyz" - UppercaseLetters = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" - NUMBERS = "0123456789" - SYMBOLS = " !\"\\#$%&'()*+,-./:;<=>?@[]^_`|~" - BinaryParamKey = "MM_BINARY_PARAMETERS" - NoTranslation = "" - maxPropSizeBytes = 1024 * 1024 + LowercaseLetters = "abcdefghijklmnopqrstuvwxyz" + UppercaseLetters = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + NUMBERS = "0123456789" + SYMBOLS = " !\"\\#$%&'()*+,-./:;<=>?@[]^_`|~" + BinaryParamKey = "MM_BINARY_PARAMETERS" + NoTranslation = "" + maxPropSizeBytes = 1024 * 1024 + PayloadParseError = "api.payload.parse.error" ) var ErrMaxPropSizeExceeded = fmt.Errorf("max prop size of %d exceeded", maxPropSizeBytes) @@ -484,17 +485,41 @@ func ArrayToJSON(objmap []string) string { return string(b) } +// Deprecated: ArrayFromJSON is deprecated, +// use SortedArrayFromJSON or NonSortedArrayFromJSON instead func ArrayFromJSON(data io.Reader) []string { var objmap []string - json.NewDecoder(data).Decode(&objmap) if objmap == nil { return make([]string, 0) } - return objmap } +func SortedArrayFromJSON(data io.Reader, maxBytes int64) ([]string, error) { + var obj []string + lr := io.LimitReader(data, maxBytes) + err := json.NewDecoder(lr).Decode(&obj) + if err != nil || obj == nil { + return nil, err + } + + // Remove duplicate IDs as it can bring a significant load to the database. + return RemoveDuplicateStrings(obj), nil +} + +func NonSortedArrayFromJSON(data io.Reader, maxBytes int64) ([]string, error) { + var obj []string + lr := io.LimitReader(data, maxBytes) + err := json.NewDecoder(lr).Decode(&obj) + if err != nil || obj == nil { + return nil, err + } + + // Remove duplicate IDs, but don't sort. + return RemoveDuplicateStringsNonSort(obj), nil +} + func ArrayFromInterface(data any) []string { stringArray := []string{} @@ -737,6 +762,20 @@ func RemoveDuplicateStrings(in []string) []string { return in[:j+1] } +// RemoveDuplicateStringsNonSort does a removal of duplicate +// strings using a map. +func RemoveDuplicateStringsNonSort(in []string) []string { + allKeys := make(map[string]bool) + list := []string{} + for _, item := range in { + if _, value := allKeys[item]; !value { + allKeys[item] = true + list = append(list, item) + } + } + return list +} + func GetPreferredTimezone(timezone StringMap) string { if timezone["useAutomaticTimezone"] == "true" { return timezone["automaticTimezone"] diff --git a/server/public/model/utils_test.go b/server/public/model/utils_test.go index d2ac34dca36..61958df67f0 100644 --- a/server/public/model/utils_test.go +++ b/server/public/model/utils_test.go @@ -5,8 +5,10 @@ package model import ( "bytes" + "encoding/json" "errors" "fmt" + "io" "net/http" "reflect" "strings" @@ -200,6 +202,88 @@ func TestMapJson(t *testing.T) { require.LessOrEqual(t, len(rm2), 0, "make should be invalid") } +func TestSortedArrayFromJSON(t *testing.T) { + t.Run("Successful parse", func(t *testing.T) { + ids := []string{NewId(), NewId(), NewId()} + b, _ := json.Marshal(ids) + a, err := SortedArrayFromJSON(bytes.NewReader(b), 1000) + require.NoError(t, err) + require.ElementsMatch(t, ids, a) + }) + + t.Run("Empty Array", func(t *testing.T) { + ids := []string{} + b, _ := json.Marshal(ids) + a, err := SortedArrayFromJSON(bytes.NewReader(b), 1000) + require.NoError(t, err) + require.Empty(t, a) + }) + + t.Run("Error too large", func(t *testing.T) { + var ids []string + for i := 0; i <= 100; i++ { + ids = append(ids, NewId()) + } + b, _ := json.Marshal(ids) + _, err := SortedArrayFromJSON(bytes.NewReader(b), 1000) + require.Error(t, err) + require.ErrorIs(t, err, io.ErrUnexpectedEOF) + }) + + t.Run("Duplicate keys, returns one", func(t *testing.T) { + var ids []string + id := NewId() + for i := 0; i < 10; i++ { + ids = append(ids, id) + } + b, _ := json.Marshal(ids) + a, err := SortedArrayFromJSON(bytes.NewReader(b), 26000) + require.NoError(t, err) + require.Len(t, a, 1) + }) +} + +func TestNonSortedArrayFromJSON(t *testing.T) { + t.Run("Successful parse", func(t *testing.T) { + ids := []string{NewId(), NewId(), NewId()} + b, _ := json.Marshal(ids) + a, err := NonSortedArrayFromJSON(bytes.NewReader(b), 1000) + require.NoError(t, err) + require.Equal(t, ids, a) + }) + + t.Run("Empty Array", func(t *testing.T) { + ids := []string{} + b, _ := json.Marshal(ids) + a, err := NonSortedArrayFromJSON(bytes.NewReader(b), 1000) + require.NoError(t, err) + require.Empty(t, a) + }) + + t.Run("Error too large", func(t *testing.T) { + var ids []string + for i := 0; i <= 100; i++ { + ids = append(ids, NewId()) + } + b, _ := json.Marshal(ids) + _, err := NonSortedArrayFromJSON(bytes.NewReader(b), 1000) + require.Error(t, err) + require.ErrorIs(t, err, io.ErrUnexpectedEOF) + }) + + t.Run("Duplicate keys, returns one", func(t *testing.T) { + var ids []string + id := NewId() + for i := 0; i <= 10; i++ { + ids = append(ids, id) + } + b, _ := json.Marshal(ids) + a, err := NonSortedArrayFromJSON(bytes.NewReader(b), 26000) + require.NoError(t, err) + require.Len(t, a, 1) + }) +} + func TestIsValidEmail(t *testing.T) { for _, testCase := range []struct { Input string diff --git a/tools/mmgotool/commands/i18n.go b/tools/mmgotool/commands/i18n.go index 450140516d3..909028e21bf 100644 --- a/tools/mmgotool/commands/i18n.go +++ b/tools/mmgotool/commands/i18n.go @@ -466,6 +466,7 @@ func extractForConstants(name string, valueNode ast.Expr) *string { "ExpiredLicenseError": true, "InvalidLicenseError": true, "NoTranslation": true, + "PayloadParseError": true, } if _, ok := validConstants[name]; !ok {