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 <build@mattermost.com>

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

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)
 	}
 

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,
 			},

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' ? (
                     <>
                         <Input
+                            maxLength={MAX_LINK_LENGTH}
                             type='text'
                             name='bookmark-link'
                             containerClassName='linkInput'
@@ -449,13 +453,14 @@ function ChannelBookmarkCreateModal({
                             />
                         </FieldLabel>
                         <CreateModalNameInput
+                            maxLength={MAX_TITLE_LENGTH}
                             type={type}
                             imageUrl={icon}
                             fileInfo={pendingFile || fileInfo}
                             emoji={emoji}
                             setEmoji={setEmoji}
-                            displayName={displayName}
+                            displayName={displayName?.substring(0, MAX_TITLE_LENGTH)}
-                            placeholder={displayNameValue}
+                            placeholder={displayNameValue?.substring(0, MAX_TITLE_LENGTH)}
                             setDisplayName={setDisplayName}
                             onAddCustomEmojiClick={onHide}
                             showEmojiPicker={showEmojiPicker}

webapp/channels/src/components/channel_bookmarks/create_modal_name_input.tsx

@@ -20,6 +20,7 @@ import {isKeyPressed} from 'utils/keyboard';
 import BookmarkIcon from './bookmark_icon';
 
 type Props = {
+    maxLength: number;
     type: ChannelBookmark['type'];
     fileInfo: FileInfo | undefined;
     imageUrl: string | undefined;
@@ -33,6 +34,7 @@ type Props = {
     onAddCustomEmojiClick?: () => void;
 }
 const CreateModalNameInput = ({
+    maxLength,
     type,
     imageUrl,
     fileInfo,
@@ -138,6 +140,7 @@ const CreateModalNameInput = ({
                     <ChevronDownIcon size={'12px'}/>
                 </button>
                 <Input
+                    maxLength={maxLength}
                     type='text'
                     name='bookmark-display-name'
                     onChange={handleInputChange}