From 5dc86d5010fdfaece345bd8118b17c3725e227d8 Mon Sep 17 00:00:00 2001 From: Scott Bishel Date: Wed, 16 Oct 2024 08:26:53 -0600 Subject: [PATCH] MM-56775 Limit Bookmarks Title and URL (#28424) * additional validation for channelbookmark * add fixes for webapp * only set permissions correct for type --------- Co-authored-by: Mattermost Build --- server/channels/app/channel_bookmark_test.go | 8 +- server/public/model/channel_bookmark.go | 17 +++- server/public/model/channel_bookmark_test.go | 88 +++++++++++++++++-- .../channel_bookmarks_create_modal.tsx | 9 +- .../create_modal_name_input.tsx | 3 + 5 files changed, 113 insertions(+), 12 deletions(-) diff --git a/server/channels/app/channel_bookmark_test.go b/server/channels/app/channel_bookmark_test.go index 7e222bf5a9..df3148b1e1 100644 --- a/server/channels/app/channel_bookmark_test.go +++ b/server/channels/app/channel_bookmark_test.go @@ -26,10 +26,14 @@ func createBookmark(name string, bookmarkType model.ChannelBookmarkType, channel bookmark := &model.ChannelBookmark{ ChannelId: channelId, DisplayName: name, - LinkUrl: "https://mattermost.com", Type: bookmarkType, Emoji: ":smile:", - FileId: fileId, + } + if bookmarkType == model.ChannelBookmarkLink { + bookmark.LinkUrl = "https://mattermost.com" + } + if bookmarkType == model.ChannelBookmarkFile { + bookmark.FileId = fileId } return bookmark diff --git a/server/public/model/channel_bookmark.go b/server/public/model/channel_bookmark.go index 4d2a229cf2..d144476f31 100644 --- a/server/public/model/channel_bookmark.go +++ b/server/public/model/channel_bookmark.go @@ -5,6 +5,7 @@ package model import ( "net/http" + "unicode/utf8" ) type ChannelBookmarkType string @@ -14,6 +15,8 @@ const ( ChannelBookmarkFile ChannelBookmarkType = "file" BookmarkFileOwner = "bookmark" MaxBookmarksPerChannel = 50 + DisplayNameMaxRunes = 64 + LinkMaxRunes = 1024 ) type ChannelBookmark struct { @@ -90,7 +93,7 @@ func (o *ChannelBookmark) IsValid() *AppError { return NewAppError("ChannelBookmark.IsValid", "model.channel_bookmark.is_valid.owner_id.app_error", nil, "", http.StatusBadRequest) } - if o.DisplayName == "" { + if o.DisplayName == "" || utf8.RuneCountInString(o.DisplayName) > DisplayNameMaxRunes { return NewAppError("ChannelBookmark.IsValid", "model.channel_bookmark.is_valid.display_name.app_error", nil, "", http.StatusBadRequest) } @@ -98,11 +101,19 @@ func (o *ChannelBookmark) IsValid() *AppError { return NewAppError("ChannelBookmark.IsValid", "model.channel_bookmark.is_valid.type.app_error", nil, "id="+o.Id, http.StatusBadRequest) } - if o.Type == ChannelBookmarkLink && (o.LinkUrl == "" || !IsValidHTTPURL(o.LinkUrl)) { + if o.Type == ChannelBookmarkLink && o.FileId != "" { + return NewAppError("ChannelBookmark.IsValid", "model.channel_bookmark.is_valid.file_id.missing_or_invalid.app_error", nil, "id="+o.Id, http.StatusBadRequest) + } + + if o.Type == ChannelBookmarkFile && o.LinkUrl != "" { return NewAppError("ChannelBookmark.IsValid", "model.channel_bookmark.is_valid.link_url.missing_or_invalid.app_error", nil, "id="+o.Id, http.StatusBadRequest) } - if o.Type == ChannelBookmarkLink && o.ImageUrl != "" && !IsValidHTTPURL(o.ImageUrl) { + if o.Type == ChannelBookmarkLink && (o.LinkUrl == "" || !IsValidHTTPURL(o.LinkUrl) || utf8.RuneCountInString(o.LinkUrl) > LinkMaxRunes) { + return NewAppError("ChannelBookmark.IsValid", "model.channel_bookmark.is_valid.link_url.missing_or_invalid.app_error", nil, "id="+o.Id, http.StatusBadRequest) + } + + if o.Type == ChannelBookmarkLink && o.ImageUrl != "" && (!IsValidHTTPURL(o.ImageUrl) || utf8.RuneCountInString(o.ImageUrl) > LinkMaxRunes) { return NewAppError("ChannelBookmark.IsValid", "model.channel_bookmark.is_valid.image_url.app_error", nil, "id="+o.Id, http.StatusBadRequest) } diff --git a/server/public/model/channel_bookmark_test.go b/server/public/model/channel_bookmark_test.go index 328ec27f68..4a5867778f 100644 --- a/server/public/model/channel_bookmark_test.go +++ b/server/public/model/channel_bookmark_test.go @@ -4,6 +4,7 @@ package model import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -288,7 +289,7 @@ func TestChannelBookmarkIsValid(t *testing.T) { false, }, { - "bookmark of type link with invalid image url", + "bookmark of type link with valid image url", &ChannelBookmark{ Id: NewId(), ChannelId: NewId(), @@ -408,7 +409,7 @@ func TestChannelBookmarkIsValid(t *testing.T) { &ChannelBookmark{ Id: NewId(), OwnerId: NewId(), - ChannelId: "", + ChannelId: NewId(), FileId: NewId(), DisplayName: "display name", SortOrder: 0, @@ -416,7 +417,7 @@ func TestChannelBookmarkIsValid(t *testing.T) { ImageUrl: "", Emoji: "", Type: ChannelBookmarkLink, - CreateAt: 0, + CreateAt: 2, UpdateAt: 3, DeleteAt: 0, }, @@ -427,7 +428,7 @@ func TestChannelBookmarkIsValid(t *testing.T) { &ChannelBookmark{ Id: NewId(), OwnerId: NewId(), - ChannelId: "", + ChannelId: NewId(), FileId: NewId(), DisplayName: "display name", SortOrder: 0, @@ -435,7 +436,84 @@ func TestChannelBookmarkIsValid(t *testing.T) { ImageUrl: "", Emoji: "", Type: ChannelBookmarkFile, - CreateAt: 0, + CreateAt: 2, + UpdateAt: 3, + DeleteAt: 0, + }, + false, + }, + { + "bookmark with long display name > limit", + &ChannelBookmark{ + Id: NewId(), + OwnerId: NewId(), + ChannelId: NewId(), + FileId: "", + DisplayName: strings.Repeat("1", 65), + SortOrder: 0, + LinkUrl: "http://somelink", + ImageUrl: "", + Emoji: "", + Type: ChannelBookmarkLink, + CreateAt: 3, + UpdateAt: 3, + DeleteAt: 0, + }, + false, + }, + { + "bookmark with long display name < limit", + &ChannelBookmark{ + Id: NewId(), + OwnerId: NewId(), + ChannelId: NewId(), + FileId: "", + DisplayName: strings.Repeat("1", 64), + SortOrder: 0, + LinkUrl: "http://somelink", + ImageUrl: "", + Emoji: "", + Type: ChannelBookmarkLink, + CreateAt: 3, + UpdateAt: 3, + DeleteAt: 0, + }, + true, + }, + + { + "bookmark with link url > limit", + &ChannelBookmark{ + Id: NewId(), + OwnerId: NewId(), + ChannelId: NewId(), + FileId: "", + DisplayName: "not last test", + SortOrder: 0, + LinkUrl: "http://somelink?" + strings.Repeat("h", 1024), + ImageUrl: "", + Emoji: "", + Type: ChannelBookmarkLink, + CreateAt: 3, + UpdateAt: 3, + DeleteAt: 0, + }, + false, + }, + { + "bookmark with image url > limit", + &ChannelBookmark{ + Id: NewId(), + OwnerId: NewId(), + ChannelId: NewId(), + FileId: "", + DisplayName: "last test", + SortOrder: 0, + LinkUrl: "", + ImageUrl: "http://somelink?" + strings.Repeat("h", 1024), + Emoji: "", + Type: ChannelBookmarkLink, + CreateAt: 3, UpdateAt: 3, DeleteAt: 0, }, diff --git a/webapp/channels/src/components/channel_bookmarks/channel_bookmarks_create_modal.tsx b/webapp/channels/src/components/channel_bookmarks/channel_bookmarks_create_modal.tsx index 2559b13641..2a30282294 100644 --- a/webapp/channels/src/components/channel_bookmarks/channel_bookmarks_create_modal.tsx +++ b/webapp/channels/src/components/channel_bookmarks/channel_bookmarks_create_modal.tsx @@ -37,6 +37,9 @@ import './bookmark_create_modal.scss'; import CreateModalNameInput from './create_modal_name_input'; import {useCanUploadFiles} from './utils'; +const MAX_LINK_LENGTH = 1024; +const MAX_TITLE_LENGTH = 64; + type Props = { channelId: string; bookmarkType?: ChannelBookmark['type']; @@ -377,6 +380,7 @@ function ChannelBookmarkCreateModal({ {type === 'link' ? ( <> void; } const CreateModalNameInput = ({ + maxLength, type, imageUrl, fileInfo, @@ -138,6 +140,7 @@ const CreateModalNameInput = ({