diff --git a/server/channels/app/channel.go b/server/channels/app/channel.go index 059c55fc9d..891a1af78b 100644 --- a/server/channels/app/channel.go +++ b/server/channels/app/channel.go @@ -1338,14 +1338,17 @@ func (a *App) UpdateChannelMemberNotifyProps(c request.CTX, data map[string]stri member, err := a.Srv().Store().Channel().UpdateMemberNotifyProps(channelID, userID, filteredProps) if err != nil { var appErr *model.AppError + var invErr *store.ErrInvalidInput var nfErr *store.ErrNotFound switch { case errors.As(err, &appErr): return nil, appErr + case errors.As(err, &invErr): + return nil, model.NewAppError("updateMemberNotifyProps", "app.channel.update_member.notify_props_limit_exceeded.app_error", nil, "", http.StatusBadRequest).Wrap(err) case errors.As(err, &nfErr): return nil, model.NewAppError("updateMemberNotifyProps", MissingChannelMemberError, nil, "", http.StatusNotFound).Wrap(err) default: - return nil, model.NewAppError("updateMemberNotifyProps", "app.channel.get_member.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + return nil, model.NewAppError("updateMemberNotifyProps", "app.channel.update_member.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } } diff --git a/server/channels/app/export.go b/server/channels/app/export.go index f7adfe9b3c..81c669df25 100644 --- a/server/channels/app/export.go +++ b/server/channels/app/export.go @@ -390,7 +390,6 @@ func (a *App) buildUserChannelMemberships(userID string, teamID string) (*[]impo } func (a *App) buildUserNotifyProps(notifyProps model.StringMap) *imports.UserNotifyPropsImportData { - getProp := func(key string) *string { if v, ok := notifyProps[key]; ok { return &v diff --git a/server/channels/store/sqlstore/channel_store.go b/server/channels/store/sqlstore/channel_store.go index 73c5676870..8604b0ca23 100644 --- a/server/channels/store/sqlstore/channel_store.go +++ b/server/channels/store/sqlstore/channel_store.go @@ -11,6 +11,7 @@ import ( "strconv" "strings" "time" + "unicode/utf8" sq "github.com/mattermost/squirrel" "github.com/pkg/errors" @@ -1916,10 +1917,15 @@ func (s SqlChannelStore) UpdateMemberNotifyProps(channelID, userID string, props } defer finalizeTransactionX(tx, &err) + jsonNotifyProps := model.MapToJSON(props) + if utf8.RuneCountInString(jsonNotifyProps) > model.ChannelMemberNotifyPropsMaxRunes { + return nil, store.NewErrInvalidInput("ChannelMember", "NotifyProps", props) + } + if s.DriverName() == model.DatabaseDriverPostgres { sql, args, err2 := s.getQueryBuilder(). Update("channelmembers"). - Set("notifyprops", sq.Expr("notifyprops || ?::jsonb", model.MapToJSON(props))). + Set("notifyprops", sq.Expr("notifyprops || ?::jsonb", jsonNotifyProps)). Where(sq.Eq{ "userid": userID, "channelid": channelID, diff --git a/server/channels/store/storetest/channel_store.go b/server/channels/store/storetest/channel_store.go index debd6cb6d7..bb9a90a4f3 100644 --- a/server/channels/store/storetest/channel_store.go +++ b/server/channels/store/storetest/channel_store.go @@ -1082,6 +1082,15 @@ func testChannelSaveMember(t *testing.T, ss store.Store) { require.IsType(t, &store.ErrConflict{}, nErr) }) + t.Run("should fail if notify props are too big", func(t *testing.T) { + channelID := model.NewId() + props := model.GetDefaultChannelNotifyProps() + props["property"] = strings.Repeat("Z", model.ChannelMemberNotifyPropsMaxRunes) + member := &model.ChannelMember{ChannelId: channelID, UserId: u1.Id, NotifyProps: props} + _, nErr := ss.Channel().SaveMember(member) + require.ErrorContains(t, nErr, "channel_member.is_valid.notify_props") + }) + t.Run("insert member correctly (in channel without channel scheme and team without scheme)", func(t *testing.T) { team := &model.Team{ DisplayName: "Name", @@ -1581,6 +1590,15 @@ func testChannelSaveMultipleMembers(t *testing.T, ss store.Store) { require.IsType(t, &store.ErrConflict{}, nErr) }) + t.Run("should fail if notify props are too big", func(t *testing.T) { + channelID := model.NewId() + props := model.GetDefaultChannelNotifyProps() + props["property"] = strings.Repeat("Z", model.ChannelMemberNotifyPropsMaxRunes) + member := &model.ChannelMember{ChannelId: channelID, UserId: u1.Id, NotifyProps: props} + _, nErr := ss.Channel().SaveMultipleMembers([]*model.ChannelMember{member}) + require.ErrorContains(t, nErr, "channel_member.is_valid.notify_props") + }) + t.Run("insert members correctly (in channel without channel scheme and team without scheme)", func(t *testing.T) { team := &model.Team{ DisplayName: "Name", @@ -2109,6 +2127,18 @@ func testChannelUpdateMember(t *testing.T, ss store.Store) { require.Equal(t, "model.channel_member.is_valid.channel_id.app_error", appErr.Id) }) + t.Run("should fail with invalid error if notify props are too big", func(t *testing.T) { + props := model.GetDefaultChannelNotifyProps() + props["property"] = strings.Repeat("Z", model.ChannelMemberNotifyPropsMaxRunes) + + member := &model.ChannelMember{ChannelId: model.NewId(), UserId: u1.Id, NotifyProps: props} + _, nErr := ss.Channel().UpdateMember(member) + require.Error(t, nErr) + var appErr *model.AppError + require.ErrorAs(t, nErr, &appErr) + require.Equal(t, "model.channel_member.is_valid.notify_props.app_error", appErr.Id) + }) + t.Run("insert member correctly (in channel without channel scheme and team without scheme)", func(t *testing.T) { team := &model.Team{ DisplayName: "Name", @@ -2614,6 +2644,18 @@ func testChannelUpdateMultipleMembers(t *testing.T, ss store.Store) { require.IsType(t, &store.ErrConflict{}, nErr) }) + t.Run("should fail with invalid error if notify props are too big", func(t *testing.T) { + props := model.GetDefaultChannelNotifyProps() + props["property"] = strings.Repeat("Z", model.ChannelMemberNotifyPropsMaxRunes) + + member := &model.ChannelMember{ChannelId: model.NewId(), UserId: u1.Id, NotifyProps: props} + _, nErr := ss.Channel().SaveMultipleMembers([]*model.ChannelMember{member}) + require.Error(t, nErr) + var appErr *model.AppError + require.ErrorAs(t, nErr, &appErr) + require.Equal(t, "model.channel_member.is_valid.notify_props.app_error", appErr.Id) + }) + t.Run("insert members correctly (in channel without channel scheme and team without scheme)", func(t *testing.T) { team := &model.Team{ DisplayName: "Name", @@ -3152,6 +3194,14 @@ func testChannelUpdateMemberNotifyProps(t *testing.T, ss store.Store) { require.NoError(t, nErr) // Verify props. assert.Equal(t, props, member.NotifyProps) + + t.Run("should fail with invalid input if the notify props are too big", func(t *testing.T) { + props["property"] = strings.Repeat("Z", model.ChannelMemberNotifyPropsMaxRunes) + member, err = ss.Channel().UpdateMemberNotifyProps(member.ChannelId, member.UserId, props) + var invErr *store.ErrInvalidInput + require.ErrorAs(t, err, &invErr) + require.Nil(t, member) + }) } func testChannelRemoveMember(t *testing.T, ss store.Store) { diff --git a/server/i18n/en.json b/server/i18n/en.json index 055802a4d1..ebf971ecb2 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -4887,6 +4887,14 @@ "id": "app.channel.update_last_viewed_at_post.app_error", "translation": "Unable to mark channel as unread." }, + { + "id": "app.channel.update_member.app_error", + "translation": "Unable to update the channel member." + }, + { + "id": "app.channel.update_member.notify_props_limit_exceeded.app_error", + "translation": "Unable to update the channel member, notify props size limit exceeded." + }, { "id": "app.channel.user_belongs_to_channels.app_error", "translation": "Unable to determine if the user belongs to a list of channels." @@ -8223,6 +8231,10 @@ "id": "model.channel_member.is_valid.notify_level.app_error", "translation": "Invalid notify level." }, + { + "id": "model.channel_member.is_valid.notify_props.app_error", + "translation": "Notify props size limit exceeded." + }, { "id": "model.channel_member.is_valid.push_level.app_error", "translation": "Invalid push notification level." diff --git a/server/public/model/channel_member.go b/server/public/model/channel_member.go index 23768bb6c1..8201fda346 100644 --- a/server/public/model/channel_member.go +++ b/server/public/model/channel_member.go @@ -6,22 +6,24 @@ package model import ( "net/http" "strings" + "unicode/utf8" ) const ( - ChannelNotifyDefault = "default" - ChannelNotifyAll = "all" - ChannelNotifyMention = "mention" - ChannelNotifyNone = "none" - ChannelMarkUnreadAll = "all" - ChannelMarkUnreadMention = "mention" - IgnoreChannelMentionsDefault = "default" - IgnoreChannelMentionsOff = "off" - IgnoreChannelMentionsOn = "on" - IgnoreChannelMentionsNotifyProp = "ignore_channel_mentions" - ChannelAutoFollowThreadsOff = "off" - ChannelAutoFollowThreadsOn = "on" - ChannelAutoFollowThreads = "channel_auto_follow_threads" + ChannelNotifyDefault = "default" + ChannelNotifyAll = "all" + ChannelNotifyMention = "mention" + ChannelNotifyNone = "none" + ChannelMarkUnreadAll = "all" + ChannelMarkUnreadMention = "mention" + IgnoreChannelMentionsDefault = "default" + IgnoreChannelMentionsOff = "off" + IgnoreChannelMentionsOn = "on" + IgnoreChannelMentionsNotifyProp = "ignore_channel_mentions" + ChannelAutoFollowThreadsOff = "off" + ChannelAutoFollowThreadsOn = "on" + ChannelAutoFollowThreads = "channel_auto_follow_threads" + ChannelMemberNotifyPropsMaxRunes = 800000 ) type ChannelUnread struct { @@ -186,6 +188,11 @@ func (o *ChannelMember) IsValid() *AppError { map[string]any{"Limit": UserRolesMaxLength}, "", http.StatusBadRequest) } + jsonStringNotifyProps := string(ToJSON(o.NotifyProps)) + if utf8.RuneCountInString(jsonStringNotifyProps) > ChannelMemberNotifyPropsMaxRunes { + return NewAppError("ChannelMember.IsValid", "model.channel_member.is_valid.notify_props.app_error", nil, "channel_id="+o.ChannelId+" user_id="+o.UserId, http.StatusBadRequest) + } + return nil } diff --git a/server/public/model/channel_member_test.go b/server/public/model/channel_member_test.go index 04594a7e21..428cd400bc 100644 --- a/server/public/model/channel_member_test.go +++ b/server/public/model/channel_member_test.go @@ -4,6 +4,7 @@ package model import ( + "strings" "testing" "github.com/stretchr/testify/require" @@ -37,4 +38,7 @@ func TestChannelMemberIsValid(t *testing.T) { o.Roles = "" require.Nil(t, o.IsValid(), "should be invalid") + + o.NotifyProps["property"] = strings.Repeat("Z", ChannelMemberNotifyPropsMaxRunes) + require.NotNil(t, o.IsValid(), "should be invalid") }