diff --git a/e2e-tests/cypress/tests/integration/channels/collapsed_reply_threads/files_1_spec.ts b/e2e-tests/cypress/tests/integration/channels/collapsed_reply_threads/files_1_spec.ts index 8846650a6e7..0c13bebf6eb 100644 --- a/e2e-tests/cypress/tests/integration/channels/collapsed_reply_threads/files_1_spec.ts +++ b/e2e-tests/cypress/tests/integration/channels/collapsed_reply_threads/files_1_spec.ts @@ -62,6 +62,8 @@ describe('Collapsed Reply Threads', () => { cy.postMessage('/poll "Do you like https://mattermost.com?"'); cy.getLastPostId().then((pollId) => { + const threadAuthorName = user1.nickname || user1.username; + // # Post a reply on the POLL to create a thread and follow cy.postMessageAs({sender: user1, message: MESSAGES.SMALL, channelId: testChannel.id, rootId: pollId}); @@ -75,7 +77,7 @@ describe('Collapsed Reply Threads', () => { // * Text in ThreadItem should say 'username: Do you like https://mattermost.com?' cy.get('.ThreadItem').last().within(() => { - cy.contains(user1.nickname + ': Do you like https://mattermost.com?').should('be.visible'); + cy.contains(threadAuthorName + ': Do you like https://mattermost.com?').should('be.visible'); cy.contains('Total votes: 1').should('be.visible'); }); @@ -96,7 +98,7 @@ describe('Collapsed Reply Threads', () => { // * Text in ThreadItem should say 'username: Do you like https://mattermost.com?' cy.get('.ThreadItem').last().within(() => { - cy.contains(user1.nickname + ': Do you like https://mattermost.com?').should('be.visible'); + cy.contains(threadAuthorName + ': Do you like https://mattermost.com?').should('be.visible'); cy.contains('This poll has ended. The results are:').should('be.visible'); }); diff --git a/e2e-tests/cypress/tests/integration/channels/integrations/incoming_webhook/incoming_webhook_is_image_only_spec.js b/e2e-tests/cypress/tests/integration/channels/integrations/incoming_webhook/incoming_webhook_is_image_only_spec.js index d883ad3bad7..daa5799e9b2 100644 --- a/e2e-tests/cypress/tests/integration/channels/integrations/incoming_webhook/incoming_webhook_is_image_only_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/integrations/incoming_webhook/incoming_webhook_is_image_only_spec.js @@ -79,7 +79,11 @@ describe('Incoming webhook', () => { attachments: [{image_url: 'https://cdn.pixabay.com/photo/2017/10/10/22/24/wide-format-2839089_960_720.jpg'}], }; cy.postIncomingWebhook({url: hookUrl, data: payload}); - cy.get('.mm-blocks-image').should('be.visible'); + cy.getLastPostId().then((postId) => { + cy.get(`#post_${postId}`).within(() => { + cy.get('.mm-blocks-image').should('be.visible'); + }); + }); }); }); }); diff --git a/server/channels/app/integration_action_test.go b/server/channels/app/integration_action_test.go index 306347a87df..63391e7ccb7 100644 --- a/server/channels/app/integration_action_test.go +++ b/server/channels/app/integration_action_test.go @@ -540,7 +540,10 @@ func TestPostAction(t *testing.T) { require.Nil(t, err) th.App.UpdateConfig(func(cfg *model.Config) { - *cfg.ServiceSettings.SiteURL = "http://127.1.1.1" + // Unreachable localhost port fails immediately (connection refused) instead of + // waiting for OutgoingIntegrationRequestsTimeout on black-holed addresses. + *cfg.ServiceSettings.SiteURL = "http://127.0.0.1:1" + cfg.ServiceSettings.OutgoingIntegrationRequestsTimeout = new(int64(1)) }) interactivePostSiteURL := model.Post{ @@ -562,7 +565,7 @@ func TestPostAction(t *testing.T) { "s": "foo", "n": 3, }, - URL: "http://127.1.1.1/plugins/myplugin/myaction", + URL: "http://127.0.0.1:1/plugins/myplugin/myaction", }, }, }, @@ -2804,6 +2807,9 @@ func TestCreateWebhookPostKeepsMmBlocksActions(t *testing.T) { post, appErr := th.App.CreateWebhookPost(th.Context, hook.UserId, th.BasicChannel, "hello", "user", "http://iconurl", "", model.StringInterface{ + model.PostPropsMmBlocks: []any{ + map[string]any{"type": "button", "text": "Go", "action_id": "actx"}, + }, model.PostPropsMmBlocksActions: inline, }, "", "", nil) @@ -2821,6 +2827,73 @@ func TestCreateWebhookPostKeepsMmBlocksActions(t *testing.T) { assert.Equal(t, "http://127.0.0.1/plugins/myplugin/x", spec.URL) } +// TestCreateWebhookPostKeepsMmBlocksActionsOnInteractiveSplit verifies split +// webhook posts persist mm_blocks_actions on the chunk that carries mm_blocks. +// CreateWebhookPost returns the first split (typically the leading message chunk). +func TestCreateWebhookPostKeepsMmBlocksActionsOnInteractiveSplit(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableIncomingWebhooks = true }) + + hook, hookErr := th.App.CreateIncomingWebhookForChannel(th.BasicUser.Id, th.BasicChannel, &model.IncomingWebhook{ChannelId: th.BasicChannel.Id}) + require.Nil(t, hookErr) + defer func() { + _ = th.App.DeleteIncomingWebhook(hook.Id) + }() + + inline := buildMmBlocksActionsProp( + "actx", + "http://127.0.0.1/plugins/myplugin/x", + nil, + ) + + marker := "mm-blocks-split-" + model.NewId() + longMessage := marker + strings.Repeat("x", th.App.MaxPostSize()+100) + + returned, appErr := th.App.CreateWebhookPost(th.Context, hook.UserId, th.BasicChannel, longMessage, "user", "http://iconurl", "", + model.StringInterface{ + model.PostPropsMmBlocks: []any{ + map[string]any{"type": "text", "text": "interactive body"}, + map[string]any{"type": "button", "text": "Go", "action_id": "actx"}, + }, + model.PostPropsMmBlocksActions: inline, + }, + "", "", nil) + require.Nil(t, appErr) + require.True(t, strings.HasPrefix(returned.Message, marker)) + require.Nil(t, returned.GetProp(model.PostPropsMmBlocks)) + require.Nil(t, returned.GetProp(model.PostPropsMmBlocksActions)) + + list, listErr := th.App.GetPosts(th.Context, th.BasicChannel.Id, 0, 20) + require.Nil(t, listErr) + + var mmBlocksPost *model.Post + messageChunks := 0 + for _, p := range list.Posts { + if p.Message != "" && strings.Contains(longMessage, p.Message) { + messageChunks++ + } + if p.GetProp(model.PostPropsMmBlocks) != nil { + mmBlocksPost = p + } + } + require.Greater(t, messageChunks, 1, "message should be split into multiple posts") + require.NotNil(t, mmBlocksPost) + require.NotEqual(t, returned.Id, mmBlocksPost.Id, "interactive props should be on a later split, not the returned first chunk") + + stored, err := th.App.Srv().Store().Post().GetSingle(th.Context, mmBlocksPost.Id, false) + require.NoError(t, err) + require.NotNil(t, stored.GetProp(model.PostPropsMmBlocksActions), + "mm_blocks_actions must be on the post that carries mm_blocks") + + for _, p := range list.Posts { + if p.GetProp(model.PostPropsMmBlocksActions) != nil { + assert.Equal(t, mmBlocksPost.Id, p.Id, "only the mm_blocks post should keep mm_blocks_actions") + } + } +} + func TestSendEphemeralPostEncryptsMmBlocksActionsCookie(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic(t) diff --git a/server/channels/app/post_metadata_test.go b/server/channels/app/post_metadata_test.go index ce6a0ffc8a9..cb08b415232 100644 --- a/server/channels/app/post_metadata_test.go +++ b/server/channels/app/post_metadata_test.go @@ -1589,6 +1589,56 @@ func TestGetImagesForPost(t *testing.T) { assert.Equal(t, images, map[string]*model.PostImage{}) }) + t.Run("with message attachment image URLs", func(t *testing.T) { + th := Setup(t) + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "127.0.0.1" + }) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + file, err := testutils.ReadTestFile("test.png") + require.NoError(t, err) + + w.Header().Set("Content-Type", "image/png") + _, err = w.Write(file) + require.NoError(t, err) + })) + defer server.Close() + + imageURL := server.URL + "/attachment.png" + thumbURL := server.URL + "/thumb.png" + authorIconURL := server.URL + "/author.png" + footerIconURL := server.URL + "/footer.png" + post := &model.Post{ + Metadata: &model.PostMetadata{}, + Props: model.StringInterface{ + model.PostPropsAttachments: []*model.MessageAttachment{ + { + ImageURL: imageURL, + ThumbURL: thumbURL, + AuthorIcon: authorIconURL, + FooterIcon: footerIconURL, + }, + }, + }, + } + + images := th.App.getImagesForPost(th.Context, post, false) + + expected := &model.PostImage{ + Format: "png", + Width: 408, + Height: 336, + } + assert.Equal(t, map[string]*model.PostImage{ + imageURL: expected, + thumbURL: expected, + authorIconURL: expected, + footerIconURL: expected, + }, images) + }) + t.Run("skips all when unsafe links including interactive props", func(t *testing.T) { th := Setup(t) @@ -1597,7 +1647,7 @@ func TestGetImagesForPost(t *testing.T) { }) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusInternalServerError) + t.Fatalf("unexpected HTTP request to test server: %s %s", r.Method, r.URL.String()) })) t.Cleanup(server.Close) diff --git a/server/channels/app/webhook.go b/server/channels/app/webhook.go index f138feb3b3d..68d16ce2263 100644 --- a/server/channels/app/webhook.go +++ b/server/channels/app/webhook.go @@ -234,15 +234,104 @@ func (a *App) doOutgoingWebhookRequest(url string, body io.Reader, contentType s return &hookResp, nil } +func webhookSplitDeferredProp(key string) bool { + switch key { + case model.PostPropsAttachments, + model.PostPropsMmBlocks, + model.PostPropsBlockKitBlocks, + model.PostPropsAdaptiveCards, + model.PostPropsMmBlocksActions: + return true + default: + return false + } +} + +func webhookPropsJSONArray(raw any) ([]any, bool) { + arr, ok := raw.([]any) + return arr, ok +} + +func splitWebhookPostPropsTooLarge() *model.AppError { + return model.NewAppError("splitWebhookPost", "web.incoming_webhook.split_props_length.app_error", map[string]any{"Max": model.PostPropsMaxUserRunes}, "", http.StatusBadRequest) +} + +func refreshSplitInteractiveActions(split *model.Post, allActions any) { + if allActions == nil { + return + } + model.RefreshInteractiveActionsOnPost(split, allActions) +} + +// cloneWebhookSplitPost returns a split post with a deep-copied props map. Post.Clone only +// shallow-copies Props, so message chunks must not share one map when refresh updates actions. +func cloneWebhookSplitPost(p *model.Post) *model.Post { + split := p.Clone() + if props := p.GetProps(); len(props) > 0 { + newProps := make(map[string]any, len(props)) + maps.Copy(newProps, props) + split.SetProps(newProps) + } else { + split.SetProps(make(map[string]any)) + } + return split +} + +// distributeWebhookJSONArrayProp appends each element of a JSON array prop onto webhook post +// splits, mirroring message attachment distribution. +func distributeWebhookJSONArrayProp(splits []*model.Post, base *model.Post, propKey string, items []any, allActions any) ([]*model.Post, *model.AppError) { + for _, item := range items { + for { + lastSplit := splits[len(splits)-1] + newProps := make(map[string]any) + maps.Copy(newProps, lastSplit.GetProps()) + orig, _ := newProps[propKey].([]any) + newProps[propKey] = append(orig, item) + candidate := lastSplit.Clone() + candidate.SetProps(newProps) + refreshSplitInteractiveActions(candidate, allActions) + if utf8.RuneCountInString(model.StringInterfaceToJSON(candidate.GetProps())) <= model.PostPropsMaxUserRunes { + lastSplit.SetProps(candidate.GetProps()) + break + } + + if len(orig) > 0 { + splits = append(splits, cloneWebhookSplitPost(base)) + continue + } + + return nil, splitWebhookPostPropsTooLarge() + } + } + return splits, nil +} + +func validateWebhookPostInteractiveActions(post *model.Post) *model.AppError { + if err := model.ValidateInteractiveActionsForWebhook(post); err != nil { + return model.NewAppError("CreateWebhookPost", "api.context.invalid_body.app_error", nil, err.Error(), http.StatusBadRequest) + } + return nil +} + func splitWebhookPost(post *model.Post, maxPostSize int) ([]*model.Post, *model.AppError) { + // Fast path: message and full props already fit one post. Pairing was validated + // before split, so mm_blocks_actions need not be subset/refreshed here. + if utf8.RuneCountInString(post.Message) <= maxPostSize { + if utf8.RuneCountInString(model.StringInterfaceToJSON(post.GetProps())) <= model.PostPropsMaxUserRunes { + return []*model.Post{post.Clone()}, nil + } + } + splits := make([]*model.Post, 0) remainingText := post.Message + mmBlocksActions := post.GetProp(model.PostPropsMmBlocksActions) + base := post.Clone() base.Message = "" base.SetProps(make(map[string]any)) for k, v := range post.GetProps() { - if k != model.PostPropsAttachments { + if !webhookSplitDeferredProp(k) { base.AddProp(k, v) } } @@ -252,7 +341,7 @@ func splitWebhookPost(post *model.Post, maxPostSize int) ([]*model.Post, *model. } for utf8.RuneCountInString(remainingText) > maxPostSize { - split := base.Clone() + split := cloneWebhookSplitPost(base) x := 0 for index := range remainingText { x++ @@ -262,11 +351,19 @@ func splitWebhookPost(post *model.Post, maxPostSize int) ([]*model.Post, *model. break } } + refreshSplitInteractiveActions(split, mmBlocksActions) + if utf8.RuneCountInString(model.StringInterfaceToJSON(split.GetProps())) > model.PostPropsMaxUserRunes { + return nil, splitWebhookPostPropsTooLarge() + } splits = append(splits, split) } - split := base.Clone() + split := cloneWebhookSplitPost(base) split.Message = remainingText + refreshSplitInteractiveActions(split, mmBlocksActions) + if utf8.RuneCountInString(model.StringInterfaceToJSON(split.GetProps())) > model.PostPropsMaxUserRunes { + return nil, splitWebhookPostPropsTooLarge() + } splits = append(splits, split) attachments, _ := post.GetProp(model.PostPropsAttachments).([]*model.MessageAttachment) @@ -287,8 +384,7 @@ func splitWebhookPost(post *model.Post, maxPostSize int) ([]*model.Post, *model. } if len(origAttachments) > 0 { - newSplit := base.Clone() - splits = append(splits, newSplit) + splits = append(splits, cloneWebhookSplitPost(base)) continue } @@ -310,6 +406,16 @@ func splitWebhookPost(post *model.Post, maxPostSize int) ([]*model.Post, *model. } } + for _, propKey := range []string{model.PostPropsMmBlocks, model.PostPropsBlockKitBlocks, model.PostPropsAdaptiveCards} { + if items, ok := webhookPropsJSONArray(post.GetProp(propKey)); ok && len(items) > 0 { + var err *model.AppError + splits, err = distributeWebhookJSONArrayProp(splits, base, propKey, items, mmBlocksActions) + if err != nil { + return nil, err + } + } + } + return splits, nil } @@ -373,18 +479,28 @@ func (a *App) CreateWebhookPost(rctx request.CTX, userID string, channel *model. } } + if err := validateWebhookPostInteractiveActions(post); err != nil { + return nil, err + } + splits, err := splitWebhookPost(post, a.MaxPostSize()) if err != nil { return nil, err } - for _, split := range splits { - if _, _, err := a.CreatePost(rctx, split, channel, model.CreatePostFlags{AllowMmBlocksActions: true}); err != nil { + var returnPost *model.Post + for i, split := range splits { + flags := model.CreatePostFlags{AllowMmBlocksActions: split.GetProp(model.PostPropsMmBlocksActions) != nil} + created, _, err := a.CreatePost(rctx, split, channel, flags) + if err != nil { return nil, model.NewAppError("CreateWebhookPost", "api.post.create_webhook_post.creating.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } + if i == 0 { + returnPost = created + } } - return splits[0], nil + return returnPost, nil } func (a *App) CreateIncomingWebhookForChannel(creatorId string, channel *model.Channel, hook *model.IncomingWebhook) (*model.IncomingWebhook, *model.AppError) { diff --git a/server/channels/app/webhook_test.go b/server/channels/app/webhook_test.go index 75274ead5a3..c29630deb44 100644 --- a/server/channels/app/webhook_test.go +++ b/server/channels/app/webhook_test.go @@ -660,11 +660,44 @@ func TestCreateWebhookPostLinks(t *testing.T) { } } +func TestValidateWebhookPostInteractiveActions(t *testing.T) { + mainHelper.Parallel(t) + + t.Run("orphan mm_blocks_actions", func(t *testing.T) { + post := &model.Post{ + Message: "foo", + Props: map[string]any{ + model.PostPropsMmBlocksActions: map[string]any{ + "act": map[string]any{"type": "external", "url": "http://example.com"}, + }, + }, + } + err := validateWebhookPostInteractiveActions(post) + require.NotNil(t, err) + }) + + t.Run("button without mm_blocks_actions", func(t *testing.T) { + post := &model.Post{ + Message: "foo", + Props: map[string]any{ + model.PostPropsMmBlocks: []any{ + map[string]any{"type": "button", "text": "Go", "action_id": "act"}, + }, + }, + } + err := validateWebhookPostInteractiveActions(post) + require.NotNil(t, err) + }) +} + func TestSplitWebhookPost(t *testing.T) { mainHelper.Parallel(t) type TestCase struct { Post *model.Post Expected []*model.Post + + // ExpectSplitError: props/message cannot be split to fit. + ExpectSplitError bool } maxPostSize := 10000 @@ -735,20 +768,147 @@ func TestSplitWebhookPost(t *testing.T) { "foo": strings.Repeat("x", model.PostPropsMaxUserRunes*2), }, }, + ExpectSplitError: true, + }, + "NoSplitFastPath": { + Post: &model.Post{ + Message: "hello", + Props: map[string]any{ + model.PostPropsMmBlocks: []any{ + map[string]any{"type": "button", "text": "Go", "action_id": "act"}, + }, + model.PostPropsMmBlocksActions: map[string]any{ + "act": map[string]any{"type": "external", "url": "http://example.com"}, + }, + }, + }, + Expected: []*model.Post{ + { + Message: "hello", + Props: map[string]any{ + model.PostPropsMmBlocks: []any{ + map[string]any{"type": "button", "text": "Go", "action_id": "act"}, + }, + model.PostPropsMmBlocksActions: map[string]any{ + "act": map[string]any{"type": "external", "url": "http://example.com"}, + }, + }, + }, + }, + }, + "MessageMmactionWithActions": { + // Message exceeds maxPostSize so splitWebhookPost uses the slow path; interactive + // content lands on the final chunk where the mmaction link lives. + Post: &model.Post{ + Message: strings.Repeat("x", maxPostSize) + "Click [go](mmaction://go1)", + Props: map[string]any{ + model.PostPropsMmBlocksActions: map[string]any{ + "go1": map[string]any{"type": "external", "url": "http://example.com"}, + }, + }, + }, + Expected: []*model.Post{ + { + Message: strings.Repeat("x", maxPostSize), + }, + { + Message: "Click [go](mmaction://go1)", + Props: map[string]any{ + model.PostPropsMmBlocksActions: map[string]any{ + "go1": map[string]any{"type": "external", "url": "http://example.com"}, + }, + }, + }, + }, + }, + "BlockKitWithActions": { + Post: &model.Post{ + Message: strings.Repeat("x", maxPostSize) + "hi", + Props: map[string]any{ + model.PostPropsBlockKitBlocks: []any{ + map[string]any{ + "type": "actions", + "elements": []any{ + map[string]any{ + "type": "button", + "text": map[string]any{"type": "plain_text", "text": "Go"}, + "action_id": "bk1", + }, + }, + }, + }, + model.PostPropsMmBlocksActions: map[string]any{ + "bk1": map[string]any{"type": "external", "url": "http://example.com"}, + }, + }, + }, + Expected: []*model.Post{ + { + Message: strings.Repeat("x", maxPostSize), + }, + { + Message: "hi", + Props: map[string]any{ + model.PostPropsBlockKitBlocks: []any{ + map[string]any{ + "type": "actions", + "elements": []any{ + map[string]any{ + "type": "button", + "text": map[string]any{"type": "plain_text", "text": "Go"}, + "action_id": "bk1", + }, + }, + }, + }, + model.PostPropsMmBlocksActions: map[string]any{ + "bk1": map[string]any{"type": "external", "url": "http://example.com"}, + }, + }, + }, + }, + }, + "LongPostWithMmBlocks": { + Post: &model.Post{ + Message: strings.Repeat("本", maxPostSize*3/2), + Props: map[string]any{ + model.PostPropsMmBlocks: []any{ + map[string]any{"type": "text", "text": "block-a"}, + map[string]any{"type": "text", "text": "block-b"}, + }, + }, + }, + Expected: []*model.Post{ + { + Message: strings.Repeat("本", maxPostSize), + }, + { + Message: strings.Repeat("本", maxPostSize/2), + Props: map[string]any{ + model.PostPropsMmBlocks: []any{ + map[string]any{"type": "text", "text": "block-a"}, + map[string]any{"type": "text", "text": "block-b"}, + }, + }, + }, + }, }, } { t.Run(name, func(t *testing.T) { splits, err := splitWebhookPost(tc.Post, maxPostSize) - if tc.Expected == nil { + if tc.ExpectSplitError { require.NotNil(t, err) - } else { - require.Nil(t, err) + return } + require.Nil(t, err) assert.Equal(t, len(tc.Expected), len(splits)) for i, split := range splits { if i < len(tc.Expected) { assert.Equal(t, tc.Expected[i].Message, split.Message) assert.Equal(t, tc.Expected[i].GetProp(model.PostPropsAttachments), split.GetProp(model.PostPropsAttachments)) + assert.Equal(t, tc.Expected[i].GetProp(model.PostPropsMmBlocks), split.GetProp(model.PostPropsMmBlocks)) + assert.Equal(t, tc.Expected[i].GetProp(model.PostPropsBlockKitBlocks), split.GetProp(model.PostPropsBlockKitBlocks)) + assert.Equal(t, tc.Expected[i].GetProp(model.PostPropsMmBlocksActions), split.GetProp(model.PostPropsMmBlocksActions)) } } }) diff --git a/server/public/model/integration_action.go b/server/public/model/integration_action.go index 73487ca3116..aa2143988b1 100644 --- a/server/public/model/integration_action.go +++ b/server/public/model/integration_action.go @@ -91,10 +91,17 @@ func (o *Post) PostActionPreserveState() PostActionPreserve { if o.RootId != "" { rootPostId = o.RootId } + var originalProps map[string]any + if props := o.GetProps(); props != nil { + originalProps = make(map[string]any, len(props)) + for k, v := range props { + originalProps[k] = v + } + } return PostActionPreserve{ Retain: retain, Remove: remove, - OriginalProps: o.GetProps(), + OriginalProps: originalProps, OriginalIsPinned: o.IsPinned, OriginalHasReactions: o.HasReactions, RootPostId: rootPostId, @@ -1024,8 +1031,12 @@ var mmBlocksActionIDRegex = regexp.MustCompile(`^[A-Za-z0-9]+$`) // expected shape and bounds. Each entry must coerce to a valid spec via // mmBlocksEntryMapToSpec. func ValidateMmBlocksActions(o *Post) error { + referenced := CollectInteractiveActionIDsFromPost(o) raw := o.GetProp(PostPropsMmBlocksActions) if raw == nil { + if len(referenced) > 0 { + return fmt.Errorf("interactive content requires mm_blocks_actions") + } return nil } actions, ok := coerceToStringAnyMap(raw) @@ -1074,7 +1085,7 @@ func ValidateMmBlocksActions(o *Post) error { } } } - return nil + return validateMmBlocksActionsPairing(o, actions) } // ValidateActionQuery bounds the size of user-supplied per-click query diff --git a/server/public/model/integration_action_test.go b/server/public/model/integration_action_test.go index 2a46c8fac0d..9c0366204bf 100644 --- a/server/public/model/integration_action_test.go +++ b/server/public/model/integration_action_test.go @@ -1701,6 +1701,15 @@ func TestPost_PostActionPreserveState(t *testing.T) { p := &Post{Id: "replyid", RootId: "rootid"} assert.Equal(t, "rootid", p.PostActionPreserveState().RootPostId) }) + + t.Run("original props snapshot", func(t *testing.T) { + p := &Post{Props: StringInterface{"k": "v"}} + state := p.PostActionPreserveState() + p.Props["k"] = "mutated" + p.Props["new"] = "y" + assert.Equal(t, "v", state.OriginalProps["k"]) + assert.NotContains(t, state.OriginalProps, "new") + }) } func TestNormalizePostActionIntegrationContext(t *testing.T) { @@ -1796,6 +1805,21 @@ func mmBlocksExternalEntry(url string, context map[string]any) map[string]any { return entry } +// ensureMmBlocksReferenceActions adds mm_blocks buttons so each mm_blocks_actions key is referenced. +func ensureMmBlocksReferenceActions(p *Post) { + actions, ok := coerceToStringAnyMap(p.GetProp(PostPropsMmBlocksActions)) + if !ok { + return + } + blocks := make([]any, 0, len(actions)) + for id := range actions { + blocks = append(blocks, map[string]any{ + "type": "button", "text": "Btn", "action_id": id, + }) + } + p.AddProp(PostPropsMmBlocks, blocks) +} + func TestGetMmBlocksActionSpec(t *testing.T) { t.Run("prop absent returns nil", func(t *testing.T) { p := &Post{} @@ -1890,6 +1914,7 @@ func TestValidateMmBlocksActions(t *testing.T) { "btn2": mmBlocksExternalEntry("/plugins/myplugin/action", nil), "btn3": mmBlocksExternalEntry("plugins/myplugin/action", nil), }) + ensureMmBlocksReferenceActions(p) assert.NoError(t, ValidateMmBlocksActions(p)) }) @@ -1900,6 +1925,7 @@ func TestValidateMmBlocksActions(t *testing.T) { } p := &Post{} p.AddProp(PostPropsMmBlocksActions, actions) + ensureMmBlocksReferenceActions(p) err := ValidateMmBlocksActions(p) require.Error(t, err) assert.Contains(t, err.Error(), "exceeds maximum") @@ -1921,6 +1947,7 @@ func TestValidateMmBlocksActions(t *testing.T) { p.AddProp(PostPropsMmBlocksActions, map[string]any{ key: mmBlocksExternalEntry("http://example.com/hook", nil), }) + ensureMmBlocksReferenceActions(p) assert.NoError(t, ValidateMmBlocksActions(p)) }) @@ -1960,6 +1987,7 @@ func TestValidateMmBlocksActions(t *testing.T) { p.AddProp(PostPropsMmBlocksActions, map[string]any{ "btn1": mmBlocksExternalEntry("", nil), }) + ensureMmBlocksReferenceActions(p) err := ValidateMmBlocksActions(p) require.Error(t, err) assert.Contains(t, err.Error(), "non-empty URL") @@ -1973,6 +2001,7 @@ func TestValidateMmBlocksActions(t *testing.T) { p.AddProp(PostPropsMmBlocksActions, map[string]any{ "btn1": mmBlocksExternalEntry("/plugins/../../../etc/passwd", nil), }) + ensureMmBlocksReferenceActions(p) err := ValidateMmBlocksActions(p) require.Error(t, err) assert.Contains(t, err.Error(), "path traversal") @@ -1983,6 +2012,7 @@ func TestValidateMmBlocksActions(t *testing.T) { p.AddProp(PostPropsMmBlocksActions, map[string]any{ "btn1": mmBlocksExternalEntry("/plugins/myplugin/..", nil), }) + ensureMmBlocksReferenceActions(p) err := ValidateMmBlocksActions(p) require.Error(t, err) assert.Contains(t, err.Error(), "path traversal") @@ -2004,6 +2034,7 @@ func TestValidateMmBlocksActions(t *testing.T) { p.AddProp(PostPropsMmBlocksActions, map[string]any{ "btn1": mmBlocksExternalEntry(encoded, nil), }) + ensureMmBlocksReferenceActions(p) err := ValidateMmBlocksActions(p) require.Error(t, err, "url=%q must be rejected", encoded) assert.Contains(t, err.Error(), "path traversal", "url=%q", encoded) @@ -2015,6 +2046,7 @@ func TestValidateMmBlocksActions(t *testing.T) { p.AddProp(PostPropsMmBlocksActions, map[string]any{ "btn1": map[string]any{"url": "http://example.com/hook"}, }) + ensureMmBlocksReferenceActions(p) err := ValidateMmBlocksActions(p) require.Error(t, err) assert.Contains(t, err.Error(), "invalid type or shape") @@ -2028,6 +2060,7 @@ func TestValidateMmBlocksActions(t *testing.T) { "url": "http://example.com/hook", }, }) + ensureMmBlocksReferenceActions(p) err := ValidateMmBlocksActions(p) require.Error(t, err) assert.Contains(t, err.Error(), "invalid type or shape") @@ -2038,6 +2071,7 @@ func TestValidateMmBlocksActions(t *testing.T) { p.AddProp(PostPropsMmBlocksActions, map[string]any{ "btn1": "not-an-object", }) + ensureMmBlocksReferenceActions(p) err := ValidateMmBlocksActions(p) require.Error(t, err) assert.Contains(t, err.Error(), "must be an object") @@ -2048,6 +2082,7 @@ func TestValidateMmBlocksActions(t *testing.T) { p.AddProp(PostPropsMmBlocksActions, map[string]any{ "btn1": mmBlocksExternalEntry("javascript://alert(1)", nil), }) + ensureMmBlocksReferenceActions(p) err := ValidateMmBlocksActions(p) require.Error(t, err) assert.Contains(t, err.Error(), "valid integration URL") @@ -2058,6 +2093,7 @@ func TestValidateMmBlocksActions(t *testing.T) { p.AddProp(PostPropsMmBlocksActions, map[string]any{ "btn1": mmBlocksExternalEntry("http://legit.com", nil), }) + ensureMmBlocksReferenceActions(p) assert.NoError(t, ValidateMmBlocksActions(p)) }) @@ -2066,6 +2102,7 @@ func TestValidateMmBlocksActions(t *testing.T) { p.AddProp(PostPropsMmBlocksActions, map[string]any{ "btn1": mmBlocksExternalEntry("/plugins/foo", nil), }) + ensureMmBlocksReferenceActions(p) assert.NoError(t, ValidateMmBlocksActions(p)) }) @@ -2090,6 +2127,7 @@ func TestValidateMmBlocksActions(t *testing.T) { "query": query, }, }) + ensureMmBlocksReferenceActions(p) err := ValidateMmBlocksActions(p) require.Error(t, err) assert.Contains(t, err.Error(), "static query") @@ -2104,6 +2142,7 @@ func TestValidateMmBlocksActions(t *testing.T) { "query": map[string]any{"k": strings.Repeat("a", MaxActionQueryValueLength+1)}, }, }) + ensureMmBlocksReferenceActions(p) err := ValidateMmBlocksActions(p) require.Error(t, err) assert.Contains(t, err.Error(), "static query") @@ -2122,6 +2161,7 @@ func TestValidateMmBlocksActions(t *testing.T) { "context": ctx, }, }) + ensureMmBlocksReferenceActions(p) err := ValidateMmBlocksActions(p) require.Error(t, err) assert.Contains(t, err.Error(), "context exceeds maximum") @@ -2136,10 +2176,58 @@ func TestValidateMmBlocksActions(t *testing.T) { "context": map[string]any{strings.Repeat("a", MaxActionQueryKeyLength+1): "v"}, }, }) + ensureMmBlocksReferenceActions(p) err := ValidateMmBlocksActions(p) require.Error(t, err) assert.Contains(t, err.Error(), "context key exceeds") }) + + t.Run("orphan mm_blocks_actions entry is rejected", func(t *testing.T) { + p := &Post{} + p.AddProp(PostPropsMmBlocks, []any{ + map[string]any{"type": "button", "text": "Go", "action_id": "needed"}, + }) + p.AddProp(PostPropsMmBlocksActions, map[string]any{ + "needed": mmBlocksExternalEntry("http://example.com/hook", nil), + "unused": mmBlocksExternalEntry("http://example.com/other", nil), + }) + err := ValidateMmBlocksActions(p) + require.Error(t, err) + assert.Contains(t, err.Error(), "not referenced") + }) + + t.Run("missing mm_blocks_actions entry is rejected", func(t *testing.T) { + p := &Post{} + p.AddProp(PostPropsMmBlocks, []any{ + map[string]any{"type": "button", "text": "Go", "action_id": "needed"}, + }) + p.AddProp(PostPropsMmBlocksActions, map[string]any{ + "other": mmBlocksExternalEntry("http://example.com/hook", nil), + }) + err := ValidateMmBlocksActions(p) + require.Error(t, err) + assert.Contains(t, err.Error(), "missing entry") + }) + + t.Run("mm_blocks_actions without interactive references is rejected", func(t *testing.T) { + p := &Post{} + p.AddProp(PostPropsMmBlocksActions, map[string]any{ + "btn1": mmBlocksExternalEntry("http://example.com/hook", nil), + }) + err := ValidateMmBlocksActions(p) + require.Error(t, err) + assert.Contains(t, err.Error(), "must only define actions referenced") + }) + + t.Run("interactive control without mm_blocks_actions is rejected", func(t *testing.T) { + p := &Post{} + p.AddProp(PostPropsMmBlocks, []any{ + map[string]any{"type": "button", "text": "Go", "action_id": "act"}, + }) + err := ValidateMmBlocksActions(p) + require.Error(t, err) + assert.Contains(t, err.Error(), "requires mm_blocks_actions") + }) } func TestStripActionIntegrations_MmBlocksActions(t *testing.T) { diff --git a/server/public/model/post.go b/server/public/model/post.go index 8f443e90ead..6a62c06c7ea 100644 --- a/server/public/model/post.go +++ b/server/public/model/post.go @@ -769,10 +769,10 @@ func (o *Post) AllStrings() []string { } // InteractiveBlocksImageURLs collects non-markdown image URLs from props.mm_blocks, props.blocks -// (Block Kit), and props.cards (Adaptive Cards): direct mm_blocks image URLs, Block Kit image blocks, -// and Adaptive Card Image elements. Markdown ![alt](url) in interactive text is not included; merge -// with URLs from Post.AllStrings separately. Link preview restrictions (e.g. RestrictLinkPreviews) are -// not applied here; callers enforce policy when fetching metadata. +// (Block Kit), props.cards (Adaptive Cards), and message attachments (image_url, thumb_url, author_icon, footer_icon). +// Direct mm_blocks image URLs, Block Kit image blocks, and Adaptive Card Image elements are included. +// Markdown ![alt](url) in interactive text is not included; merge with URLs from Post.AllStrings separately. +// Link preview restrictions (e.g. RestrictLinkPreviews) are not applied here; callers enforce policy when fetching metadata. func (o *Post) InteractiveBlocksImageURLs() []string { props := o.GetProps() if props == nil { @@ -788,6 +788,7 @@ func (o *Post) InteractiveBlocksImageURLs() []string { if raw, ok := props[PostPropsAdaptiveCards]; ok { collectAdaptiveCardImageURLs(raw, &out) } + collectAttachmentsImageURLs(o.Attachments(), &out) return out } @@ -957,10 +958,8 @@ func (o *Post) propsIsValid() error { } } - if props[PostPropsMmBlocksActions] != nil { - if err := ValidateMmBlocksActions(o); err != nil { - multiErr = multierror.Append(multiErr, fmt.Errorf("invalid mm_blocks_actions: %w", err)) - } + if err := ValidateMmBlocksActions(o); err != nil { + multiErr = multierror.Append(multiErr, fmt.Errorf("invalid mm_blocks_actions: %w", err)) } for i, a := range o.Attachments() { diff --git a/server/public/model/post_interactive_blocks.go b/server/public/model/post_interactive_blocks.go index 17d162d6338..48ff291d408 100644 --- a/server/public/model/post_interactive_blocks.go +++ b/server/public/model/post_interactive_blocks.go @@ -3,6 +3,13 @@ package model +import ( + "fmt" + "strings" + + "github.com/mattermost/mattermost/server/public/shared/markdown" +) + func appendHumanReadableInteractiveStrings(o *Post, out *[]string) { props := o.GetProps() if props == nil { @@ -123,8 +130,10 @@ func appendHumanStringsFromBlockKitTree(v any, out *[]string) { } } case "header": - if s, ok := blockMap["text"].(string); ok { - appendNonWhitespaceOnlyMessage(out, s) + if textBlock, ok := blockMap["text"].(map[string]any); ok { + if s, ok := textBlock["text"].(string); ok { + appendNonWhitespaceOnlyMessage(out, s) + } } } } @@ -329,3 +338,397 @@ func collectAdaptiveCardImageURLsFromItem(item any, out *[]string) { } } } + +func collectAttachmentsImageURLs(attachments []*MessageAttachment, out *[]string) { + for _, attachment := range attachments { + if attachment == nil { + continue + } + if attachment.ImageURL != "" { + *out = append(*out, attachment.ImageURL) + } + if attachment.ThumbURL != "" { + *out = append(*out, attachment.ThumbURL) + } + if attachment.AuthorIcon != "" { + *out = append(*out, attachment.AuthorIcon) + } + if attachment.FooterIcon != "" { + *out = append(*out, attachment.FooterIcon) + } + } +} + +const mmactionScheme = "mmaction://" + +// collectMmactionIDsFromText collects action ids from mmaction:// markdown links only. +// Inline code, fenced code blocks, and other non-link text are ignored (same approach as mentions). +func collectMmactionIDsFromText(text string, ids map[string]struct{}) { + markdown.Inspect(text, func(blockOrInline any) bool { + switch v := blockOrInline.(type) { + case *markdown.InlineLink: + collectMmactionIDFromURL(v.Destination(), ids) + case *markdown.ReferenceLink: + if v.ReferenceDefinition != nil { + collectMmactionIDFromURL(v.ReferenceDefinition.Destination(), ids) + } + case *markdown.Autolink: + collectMmactionIDFromURL(v.Destination(), ids) + } + return true + }) +} + +func collectMmactionIDFromURL(url string, ids map[string]struct{}) { + if !strings.HasPrefix(url, mmactionScheme) { + return + } + withoutScheme := url[len(mmactionScheme):] + actionID := withoutScheme + if i := strings.IndexAny(withoutScheme, "/?#"); i >= 0 { + actionID = withoutScheme[:i] + } + if actionID != "" && mmBlocksActionIDRegex.MatchString(actionID) { + ids[actionID] = struct{}{} + } +} + +func mergeActionIDs(into, from map[string]struct{}) { + for id := range from { + into[id] = struct{}{} + } +} + +func collectMmBlockActionIDsFromMap(m map[string]any, ids map[string]struct{}) { + typ, _ := m["type"].(string) + switch typ { + case "text": + if s, ok := m["text"].(string); ok { + collectMmactionIDsFromText(s, ids) + } + case "button", "static_select": + if id, ok := m["action_id"].(string); ok && id != "" { + ids[id] = struct{}{} + } + case "container": + collectMmBlockActionIDsFromArray(m["content"], ids) + case "collapsible": + collectMmBlockActionIDsFromArray(m["header"], ids) + collectMmBlockActionIDsFromArray(m["content"], ids) + case "column_set": + if cols, ok := m["columns"].([]any); ok { + for _, col := range cols { + cm, ok := col.(map[string]any) + if !ok { + continue + } + colTyp, _ := cm["type"].(string) + if colTyp != "column" { + continue + } + collectMmBlockActionIDsFromArray(cm["items"], ids) + } + } + } +} + +func collectMmBlockActionIDsFromArray(raw any, ids map[string]struct{}) { + arr, ok := interactivePropJSONArray(raw) + if !ok { + return + } + for _, el := range arr { + m, ok := el.(map[string]any) + if !ok { + continue + } + collectMmBlockActionIDsFromMap(m, ids) + } +} + +// CollectMmBlockActionIDs returns action_id values referenced by interactive mm_blocks controls. +func CollectMmBlockActionIDs(blocks []any) map[string]struct{} { + ids := make(map[string]struct{}) + for _, b := range blocks { + m, ok := b.(map[string]any) + if !ok { + continue + } + collectMmBlockActionIDsFromMap(m, ids) + } + return ids +} + +func collectBlockKitTextMmaction(raw any, ids map[string]struct{}) { + if raw == nil { + return + } + switch v := raw.(type) { + case string: + collectMmactionIDsFromText(v, ids) + case map[string]any: + if s, ok := v["text"].(string); ok { + collectMmactionIDsFromText(s, ids) + } + } +} + +func collectBlockKitAccessory(accessory map[string]any, ids map[string]struct{}) { + typ, _ := accessory["type"].(string) + switch typ { + case "button", "static_select": + if id, ok := accessory["action_id"].(string); ok && id != "" { + ids[id] = struct{}{} + } + } +} + +func collectBlockKitActionElement(el any, ids map[string]struct{}) { + e, ok := el.(map[string]any) + if !ok { + return + } + typ, _ := e["type"].(string) + switch typ { + case "button", "static_select": + if id, ok := e["action_id"].(string); ok && id != "" { + ids[id] = struct{}{} + } + } +} + +func collectBlockKitActionIDsFromBlock(m map[string]any, ids map[string]struct{}) { + typ, _ := m["type"].(string) + switch typ { + case "actions": + if elements, ok := m["elements"].([]any); ok { + for _, el := range elements { + collectBlockKitActionElement(el, ids) + } + } + case "section": + collectBlockKitTextMmaction(m["text"], ids) + if accessory, ok := m["accessory"].(map[string]any); ok { + collectBlockKitAccessory(accessory, ids) + } + if fields, ok := m["fields"].([]any); ok { + for _, field := range fields { + collectBlockKitTextMmaction(field, ids) + } + } + case "markdown": + collectBlockKitTextMmaction(m["text"], ids) + case "header": + collectBlockKitTextMmaction(m["text"], ids) + } +} + +// CollectBlockKitActionIDs returns action_id values from Block Kit blocks (props.blocks). +func CollectBlockKitActionIDs(blocks []any) map[string]struct{} { + ids := make(map[string]struct{}) + for _, b := range blocks { + m, ok := b.(map[string]any) + if !ok { + continue + } + collectBlockKitActionIDsFromBlock(m, ids) + } + return ids +} + +func collectAdaptiveCardActionElement(action any, ids map[string]struct{}) { + ac, ok := action.(map[string]any) + if !ok { + return + } + typ, _ := ac["type"].(string) + if typ == "Action.Submit" { + if id, ok := ac["id"].(string); ok && id != "" { + ids[id] = struct{}{} + } + } +} + +func collectAdaptiveCardActionIDsFromItem(item any, ids map[string]struct{}) { + itemMap, ok := item.(map[string]any) + if !ok { + return + } + typ, _ := itemMap["type"].(string) + switch typ { + case "TextBlock": + if s, ok := itemMap["text"].(string); ok { + collectMmactionIDsFromText(s, ids) + } + case "Container": + if items, ok := itemMap["items"].([]any); ok { + for _, nested := range items { + collectAdaptiveCardActionIDsFromItem(nested, ids) + } + } + case "ColumnSet": + if columns, ok := itemMap["columns"].([]any); ok { + for _, column := range columns { + columnMap, ok := column.(map[string]any) + if !ok { + continue + } + if items, ok := columnMap["items"].([]any); ok { + for _, nested := range items { + collectAdaptiveCardActionIDsFromItem(nested, ids) + } + } + } + } + case "ActionSet": + if actions, ok := itemMap["actions"].([]any); ok { + for _, action := range actions { + collectAdaptiveCardActionElement(action, ids) + } + } + } +} + +// CollectAdaptiveCardActionIDs returns action ids from Adaptive Cards (props.cards). +func CollectAdaptiveCardActionIDs(cards []any) map[string]struct{} { + ids := make(map[string]struct{}) + for _, card := range cards { + cardMap, ok := card.(map[string]any) + if !ok { + continue + } + if body, ok := cardMap["body"].([]any); ok { + for _, item := range body { + collectAdaptiveCardActionIDsFromItem(item, ids) + } + } + if actions, ok := cardMap["actions"].([]any); ok { + for _, action := range actions { + collectAdaptiveCardActionElement(action, ids) + } + } + } + return ids +} + +// CollectInteractiveActionIDs returns action ids referenced by interactive post props. +func CollectInteractiveActionIDs(props map[string]any) map[string]struct{} { + ids := make(map[string]struct{}) + if props == nil { + return ids + } + if raw, ok := props[PostPropsMmBlocks]; ok { + if blocks, ok := interactivePropJSONArray(raw); ok { + mergeActionIDs(ids, CollectMmBlockActionIDs(blocks)) + } + } + if raw, ok := props[PostPropsBlockKitBlocks]; ok { + if blocks, ok := interactivePropJSONArray(raw); ok { + mergeActionIDs(ids, CollectBlockKitActionIDs(blocks)) + } + } + if raw, ok := props[PostPropsAdaptiveCards]; ok { + if cards, ok := interactivePropJSONArray(raw); ok { + mergeActionIDs(ids, CollectAdaptiveCardActionIDs(cards)) + } + } + return ids +} + +// CollectInteractiveActionIDsFromPost includes mmaction:// links in the post message. +func CollectInteractiveActionIDsFromPost(o *Post) map[string]struct{} { + ids := CollectInteractiveActionIDs(o.GetProps()) + if o.Message != "" { + collectMmactionIDsFromText(o.Message, ids) + } + return ids +} + +// CollectMmactionIDsFromText returns action ids from mmaction:// links in a string. +func CollectMmactionIDsFromText(text string) map[string]struct{} { + ids := make(map[string]struct{}) + collectMmactionIDsFromText(text, ids) + return ids +} + +// SubsetMmBlocksActions returns registry entries referenced by actionIDs. +func SubsetMmBlocksActions(allActions any, actionIDs map[string]struct{}) map[string]any { + if allActions == nil || len(actionIDs) == 0 { + return nil + } + top, ok := allActions.(map[string]any) + if !ok { + return nil + } + out := make(map[string]any, len(actionIDs)) + for id := range actionIDs { + if entry, ok := top[id]; ok { + out[id] = entry + } + } + if len(out) == 0 { + return nil + } + return out +} + +// RefreshInteractiveActionsOnPost sets mm_blocks_actions to the subset needed by this post's interactive content. +func RefreshInteractiveActionsOnPost(o *Post, allActions any) { + ids := CollectInteractiveActionIDsFromPost(o) + props := o.GetProps() + if props == nil { + props = make(map[string]any) + } + if len(ids) == 0 { + delete(props, PostPropsMmBlocksActions) + } else if subset := SubsetMmBlocksActions(allActions, ids); len(subset) > 0 { + props[PostPropsMmBlocksActions] = subset + } else { + delete(props, PostPropsMmBlocksActions) + } + o.SetProps(props) +} + +// ApplyMmBlocksWithActionsToProps sets mm_blocks and refreshes mm_blocks_actions for the props payload. +func ApplyMmBlocksWithActionsToProps(props map[string]any, blocks []any, allActions any) { + props[PostPropsMmBlocks] = blocks + RefreshInteractiveActionsOnPost(&Post{Props: props}, allActions) +} + +// validateMmBlocksActionsPairing requires mm_blocks_actions to define exactly the actions +// referenced by mm_blocks, blocks, cards, and mmaction:// links in the post message. +func validateMmBlocksActionsPairing(o *Post, actions map[string]any) error { + referenced := CollectInteractiveActionIDsFromPost(o) + if len(referenced) == 0 { + if len(actions) > 0 { + return fmt.Errorf("mm_blocks_actions must only define actions referenced by interactive content") + } + return nil + } + for id := range referenced { + if _, ok := actions[id]; !ok { + return fmt.Errorf("mm_blocks_actions missing entry for action_id %q", id) + } + } + for key := range actions { + if _, ok := referenced[key]; !ok { + return fmt.Errorf("mm_blocks_actions entry %q is not referenced by interactive content", key) + } + } + return nil +} + +// ValidateInteractiveActionsForWebhook checks interactive payloads and mm_blocks_actions are paired. +func ValidateInteractiveActionsForWebhook(o *Post) error { + return ValidateMmBlocksActions(o) +} + +// ValidateMmBlocksActionsForWebhook validates mm_blocks-only webhook payloads (legacy helper). +func ValidateMmBlocksActionsForWebhook(blocks []any, actions any) error { + return ValidateInteractiveActionsForWebhook(&Post{ + Props: map[string]any{ + PostPropsMmBlocks: blocks, + PostPropsMmBlocksActions: actions, + }, + }) +} diff --git a/server/public/model/post_interactive_blocks_test.go b/server/public/model/post_interactive_blocks_test.go index 73b2e993b34..6b752caa208 100644 --- a/server/public/model/post_interactive_blocks_test.go +++ b/server/public/model/post_interactive_blocks_test.go @@ -7,9 +7,180 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func TestCollectMmBlockActionIDs(t *testing.T) { + blocks := []any{ + map[string]any{"type": "text", "text": "hi"}, + map[string]any{ + "type": "container", + "content": []any{ + map[string]any{"type": "button", "text": "A", "action_id": "a1"}, + map[string]any{"type": "static_select", "action_id": "s1", "placeholder": "pick"}, + }, + }, + } + ids := CollectMmBlockActionIDs(blocks) + assert.Equal(t, map[string]struct{}{"a1": {}, "s1": {}}, ids) +} + +func TestCollectMmBlockActionIDs_columnSet(t *testing.T) { + blocks := []any{ + map[string]any{ + "type": "column_set", + "columns": []any{ + map[string]any{ + "type": "column", + "items": []any{ + map[string]any{"type": "button", "text": "A", "action_id": "inset"}, + }, + }, + map[string]any{ + "type": "text", + "text": "not a column", + }, + }, + }, + map[string]any{ + "type": "column", + "items": []any{ + map[string]any{"type": "button", "text": "B", "action_id": "orphan"}, + }, + }, + } + ids := CollectMmBlockActionIDs(blocks) + assert.Equal(t, map[string]struct{}{"inset": {}}, ids) +} + +func TestSubsetMmBlocksActions(t *testing.T) { + all := map[string]any{ + "a1": map[string]any{"type": "external", "url": "http://example.com/a"}, + "b2": map[string]any{"type": "external", "url": "http://example.com/b"}, + } + subset := SubsetMmBlocksActions(all, map[string]struct{}{"a1": {}}) + require.Len(t, subset, 1) + assert.Contains(t, subset, "a1") +} + +func TestValidateMmBlocksActionsForWebhook(t *testing.T) { + blocks := []any{ + map[string]any{"type": "button", "text": "Go", "action_id": "act"}, + } + require.Error(t, ValidateMmBlocksActionsForWebhook(blocks, nil)) + require.Error(t, ValidateMmBlocksActionsForWebhook(nil, map[string]any{"act": map[string]any{}})) + require.NoError(t, ValidateMmBlocksActionsForWebhook(blocks, map[string]any{ + "act": map[string]any{"type": "external", "url": "http://example.com"}, + })) +} + +func TestCollectMmactionIDsFromMmBlockText(t *testing.T) { + blocks := []any{ + map[string]any{ + "type": "text", + "text": "Choose [one](mmaction://pick1) or [two](mmaction://pick2)", + }, + } + ids := CollectMmBlockActionIDs(blocks) + assert.Equal(t, map[string]struct{}{"pick1": {}, "pick2": {}}, ids) +} + +func TestCollectMmactionIDsFromText_skipsCode(t *testing.T) { + ids := CollectMmactionIDsFromText("Use [real](mmaction://real1) not `mmaction://inline`") + assert.Equal(t, map[string]struct{}{"real1": {}}, ids) + + ids = CollectMmactionIDsFromText("```\n[mmaction://fence](mmaction://fence)\n```\n[ok](mmaction://ok1)") + assert.Equal(t, map[string]struct{}{"ok1": {}}, ids) +} + +func TestCollectBlockKitActionIDs(t *testing.T) { + blocks := []any{ + map[string]any{ + "type": "section", + "text": map[string]any{"type": "mrkdwn", "text": "[Go](mmaction://md1)"}, + "accessory": map[string]any{ + "type": "button", "text": map[string]any{"type": "plain_text", "text": "Btn"}, + "action_id": "acc1", + }, + }, + map[string]any{ + "type": "actions", + "elements": []any{ + map[string]any{ + "type": "button", + "text": map[string]any{"type": "plain_text", "text": "OK"}, + "action_id": "row1", + }, + }, + }, + } + ids := CollectBlockKitActionIDs(blocks) + assert.Equal(t, map[string]struct{}{"md1": {}, "acc1": {}, "row1": {}}, ids) +} + +func TestCollectAdaptiveCardActionIDs(t *testing.T) { + cards := []any{ + map[string]any{ + "type": "AdaptiveCard", + "body": []any{ + map[string]any{"type": "TextBlock", "text": "See [here](mmaction://cardmd)"}, + map[string]any{ + "type": "ActionSet", + "actions": []any{ + map[string]any{"type": "Action.Submit", "title": "OK", "id": "submit1"}, + }, + }, + }, + "actions": []any{ + map[string]any{"type": "Action.Submit", "title": "Footer", "id": "footer1"}, + }, + }, + } + ids := CollectAdaptiveCardActionIDs(cards) + assert.Equal(t, map[string]struct{}{"cardmd": {}, "submit1": {}, "footer1": {}}, ids) +} + +func TestValidateMmBlocksActions_pairing(t *testing.T) { + post := &Post{ + Props: map[string]any{ + PostPropsMmBlocks: []any{ + map[string]any{"type": "button", "text": "Go", "action_id": "act"}, + }, + PostPropsMmBlocksActions: map[string]any{ + "act": map[string]any{"type": "external", "url": "http://example.com"}, + }, + }, + } + require.NoError(t, ValidateMmBlocksActions(post)) + + extra := post.Clone() + extraProps := extra.GetProps() + extraProps[PostPropsMmBlocksActions] = map[string]any{ + "act": map[string]any{"type": "external", "url": "http://example.com"}, + "extra": map[string]any{"type": "external", "url": "http://example.com/2"}, + } + extra.SetProps(extraProps) + require.Error(t, ValidateMmBlocksActions(extra)) +} + +func TestValidateInteractiveActionsForWebhook_messageMmaction(t *testing.T) { + post := &Post{ + Message: "Click [go](mmaction://go1)", + Props: map[string]any{ + PostPropsMmBlocksActions: map[string]any{ + "go1": map[string]any{"type": "external", "url": "http://example.com"}, + }, + }, + } + require.NoError(t, ValidateInteractiveActionsForWebhook(post)) + + postMissing := &Post{Message: "Click [go](mmaction://go1)"} + require.Error(t, ValidateInteractiveActionsForWebhook(postMissing)) +} + func TestPost_InteractiveBlocksImageURLs(t *testing.T) { + assert.Nil(t, (&Post{}).InteractiveBlocksImageURLs()) + post := &Post{ Props: StringInterface{ PostPropsMmBlocks: []any{ @@ -84,6 +255,17 @@ func TestPost_InteractiveBlocksImageURLs(t *testing.T) { }, }, }, + PostPropsAttachments: []*MessageAttachment{ + { + ImageURL: "https://example.com/attach_main.png", + ThumbURL: "https://example.com/attach_thumb.png", + AuthorIcon: "https://example.com/attach_author.png", + FooterIcon: "https://example.com/attach_footer.png", + }, + {ImageURL: "https://example.com/attach_second.png"}, + nil, + {Text: "no images"}, + }, }, } @@ -101,5 +283,10 @@ func TestPost_InteractiveBlocksImageURLs(t *testing.T) { "https://example.com/ac_card1_a.png", "https://example.com/ac_card1_b.png", "https://example.com/ac_card2.png", + "https://example.com/attach_main.png", + "https://example.com/attach_thumb.png", + "https://example.com/attach_author.png", + "https://example.com/attach_footer.png", + "https://example.com/attach_second.png", }, urls) } diff --git a/server/public/model/post_test.go b/server/public/model/post_test.go index e8a9816fc94..0936d064f83 100644 --- a/server/public/model/post_test.go +++ b/server/public/model/post_test.go @@ -1072,6 +1072,25 @@ func TestPost_AllStrings_interactiveProps(t *testing.T) { require.Contains(t, got, "card-line") } +func TestPost_AllStrings_blockKitHeaderPlainText(t *testing.T) { + p := &Post{ + Props: StringInterface{ + PostPropsBlockKitBlocks: []any{ + map[string]any{ + "type": "header", + "text": map[string]any{ + "type": "plain_text", + "text": "Section title", + "emoji": true, + }, + }, + }, + }, + } + got := p.AllStrings() + require.Contains(t, got, "Section title") +} + func TestPost_AllStrings_includesMessageAttachments(t *testing.T) { p := &Post{ Message: "hi", diff --git a/webapp/channels/src/components/block_renderer/image_block.test.tsx b/webapp/channels/src/components/block_renderer/image_block.test.tsx index d66168bdc63..5cabab16aee 100644 --- a/webapp/channels/src/components/block_renderer/image_block.test.tsx +++ b/webapp/channels/src/components/block_renderer/image_block.test.tsx @@ -85,6 +85,32 @@ describe('ImageBlock', () => { expect(screen.getByTestId('size-aware-image')).toHaveClass('mm-blocks-image__img--person'); }); + it('infers extension from pathname when url has query params', async () => { + const user = userEvent.setup(); + renderWithContext( + + + , + ); + + await user.click(screen.getByTestId('size-aware-image')); + expect(openModal).toHaveBeenCalledWith(expect.objectContaining({ + dialogProps: expect.objectContaining({ + fileInfos: [expect.objectContaining({ + link: 'https://example.com/photo.png?sig=abc123', + extension: 'png', + })], + }), + })); + }); + it('opens file preview modal when image is clicked', async () => { const user = userEvent.setup(); renderWithContext( diff --git a/webapp/channels/src/components/block_renderer/image_block.tsx b/webapp/channels/src/components/block_renderer/image_block.tsx index cb9fbaa15d7..b8a37cc40c0 100644 --- a/webapp/channels/src/components/block_renderer/image_block.tsx +++ b/webapp/channels/src/components/block_renderer/image_block.tsx @@ -26,6 +26,17 @@ type ImageBlockProps = { postId: string; }; +function extensionFromImageURL(src: string): string { + let pathForExt = src; + try { + pathForExt = new URL(src, window.location.href).pathname; + } catch { + // Relative or malformed URLs: fall back to parsing src as-is. + } + const index = pathForExt.lastIndexOf('.'); + return index > 0 ? pathForExt.substring(index + 1) : ''; +} + export const ImageBlock = ({block, postId}: ImageBlockProps) => { const dispatch = useDispatch(); const imagesMetadata = useContext(MmBlocksImagesMetadataContext); @@ -38,8 +49,7 @@ export const ImageBlock = ({block, postId}: ImageBlockProps) => { link = '', ) => { const src = link || url; - const index = src.lastIndexOf('.'); - const extension = index > 0 ? src.substring(index + 1) : ''; + const extension = extensionFromImageURL(src); e.preventDefault(); diff --git a/webapp/channels/src/components/block_renderer/translation/adaptive_cards.test.ts b/webapp/channels/src/components/block_renderer/translation/adaptive_cards.test.ts new file mode 100644 index 00000000000..4b553de2801 --- /dev/null +++ b/webapp/channels/src/components/block_renderer/translation/adaptive_cards.test.ts @@ -0,0 +1,33 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import type {MmImageBlock} from '@mattermost/types/mm_blocks'; + +import {translateAdaptiveCards} from './adaptive_cards'; + +function imageFromCards(width: unknown, height?: unknown): MmImageBlock | undefined { + const blocks = translateAdaptiveCards([{ + type: 'AdaptiveCard', + body: [{ + type: 'Image', + url: 'https://example.com/x.png', + width, + ...(height === undefined ? {} : {height}), + }], + }]); + return blocks.find((b): b is MmImageBlock => b.type === 'image'); +} + +describe('translateAdaptiveCards Image pixel dimensions', () => { + it('accepts plain numbers, px suffix, and decimal literals', () => { + expect(imageFromCards(120)?.max_width).toBe(120); + expect(imageFromCards('80px')?.max_width).toBe(80); + expect(imageFromCards('12.5')?.max_width).toBe(13); + }); + + it('rejects percent and partially numeric strings', () => { + const image = imageFromCards('100%', '10abc'); + expect(image?.max_width).toBeUndefined(); + expect(image?.max_height).toBeUndefined(); + }); +}); diff --git a/webapp/channels/src/components/block_renderer/translation/adaptive_cards.ts b/webapp/channels/src/components/block_renderer/translation/adaptive_cards.ts index 1fb2d42d3fa..cd2ce83e011 100644 --- a/webapp/channels/src/components/block_renderer/translation/adaptive_cards.ts +++ b/webapp/channels/src/components/block_renderer/translation/adaptive_cards.ts @@ -215,7 +215,10 @@ function parseAdaptiveCardPixelDimension(v: unknown): number | undefined { if (px) { return parseInt(px[1], 10); } - const num = parseFloat(trimmed); + if (!(/^\d+(\.\d+)?$/).test(trimmed)) { + return undefined; + } + const num = Number.parseFloat(trimmed); if (Number.isFinite(num) && num > 0) { return Math.round(num); } diff --git a/webapp/channels/src/components/block_renderer/translation/attachments.ts b/webapp/channels/src/components/block_renderer/translation/attachments.ts index da1732b3879..f66300cab04 100644 --- a/webapp/channels/src/components/block_renderer/translation/attachments.ts +++ b/webapp/channels/src/components/block_renderer/translation/attachments.ts @@ -18,10 +18,7 @@ import {isUrlSafe} from 'utils/url'; import {parseMmButtonStyle} from '../utils/button'; -/** Matches legacy `.attachment__author-icon` in `_webhooks.scss` (14×14). */ const AUTHOR_ICON_MAX_PX = 20; - -/** Matches legacy `.attachment__footer-icon` in `message_attachment.tsx` (16×16). */ const FOOTER_ICON_MAX_PX = 16; /** Placeholder so the stretch column still exists when the body is otherwise empty (thumb-only attachment). */ diff --git a/webapp/channels/src/components/block_renderer/translation/block_kit.test.ts b/webapp/channels/src/components/block_renderer/translation/block_kit.test.ts new file mode 100644 index 00000000000..33df9ce7c5d --- /dev/null +++ b/webapp/channels/src/components/block_renderer/translation/block_kit.test.ts @@ -0,0 +1,42 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import type {MmButtonBlock, MmColumnSetBlock} from '@mattermost/types/mm_blocks'; + +import {translateBlockKit} from './block_kit'; + +describe('translateBlockKit section accessory button', () => { + it('requires a non-empty action_id', () => { + const blocks = translateBlockKit([{ + type: 'section', + text: {type: 'plain_text', text: 'Body'}, + accessory: { + type: 'button', + text: {type: 'plain_text', text: 'Go'}, + action_id: '', + }, + }]); + expect(blocks).toEqual([{type: 'text', text: 'Body'}]); + }); + + it('keeps accessory button when action_id is present', () => { + const blocks = translateBlockKit([{ + type: 'section', + text: {type: 'plain_text', text: 'Body'}, + accessory: { + type: 'button', + text: {type: 'plain_text', text: 'Go'}, + action_id: 'go_action', + style: 'primary', + }, + }]); + const columnSet = blocks[0] as MmColumnSetBlock; + const button = columnSet.columns[1].items[0] as MmButtonBlock; + expect(button).toMatchObject({ + type: 'button', + action_id: 'go_action', + text: 'Go', + style: 'primary', + }); + }); +}); diff --git a/webapp/channels/src/components/block_renderer/translation/block_kit.ts b/webapp/channels/src/components/block_renderer/translation/block_kit.ts index 8317a39f79b..60f5bfde66f 100644 --- a/webapp/channels/src/components/block_renderer/translation/block_kit.ts +++ b/webapp/channels/src/components/block_renderer/translation/block_kit.ts @@ -160,12 +160,12 @@ function translateBlockKitAccessory( ): MmBlock | null { if (accessory.type === 'button') { const text = extractBlockKitPlainText(accessory.text); - if (!text) { + if (!text || typeof accessory.action_id !== 'string' || !accessory.action_id) { return null; } return { type: 'button', - action_id: typeof accessory.action_id === 'string' ? accessory.action_id : '', + action_id: accessory.action_id, text, style: parseMmButtonStyle(typeof accessory.style === 'string' ? accessory.style : undefined), }; diff --git a/webapp/channels/src/components/block_renderer/translation/mm_block.test.ts b/webapp/channels/src/components/block_renderer/translation/mm_block.test.ts new file mode 100644 index 00000000000..6f93b93a8af --- /dev/null +++ b/webapp/channels/src/components/block_renderer/translation/mm_block.test.ts @@ -0,0 +1,56 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {translateMMBlocks} from './mm_block'; + +describe('translateMMBlocks interactive blocks', () => { + it('rejects button blocks with empty text or action_id', () => { + expect(translateMMBlocks([ + {type: 'button', text: ' ', action_id: 'ok'}, + {type: 'button', text: 'Go', action_id: ''}, + ])).toEqual([]); + }); + + it('accepts button blocks with non-empty text and action_id', () => { + expect(translateMMBlocks([ + {type: 'button', text: 'Go', action_id: 'go_action'}, + ])).toEqual([{ + type: 'button', + text: 'Go', + action_id: 'go_action', + }]); + }); + + it('rejects static_select blocks with empty placeholder or action_id', () => { + expect(translateMMBlocks([ + { + type: 'static_select', + action_id: 'sel', + placeholder: ' ', + options: [{text: 'A', value: 'a'}], + }, + { + type: 'static_select', + action_id: ' ', + placeholder: 'Pick', + options: [{text: 'A', value: 'a'}], + }, + ])).toEqual([]); + }); + + it('accepts static_select blocks with non-empty placeholder and action_id', () => { + expect(translateMMBlocks([ + { + type: 'static_select', + action_id: 'sel_action', + placeholder: 'Pick one', + options: [{text: 'A', value: 'a'}], + }, + ])).toEqual([{ + type: 'static_select', + action_id: 'sel_action', + placeholder: 'Pick one', + options: [{text: 'A', value: 'a'}], + }]); + }); +}); diff --git a/webapp/channels/src/components/block_renderer/translation/mm_block.ts b/webapp/channels/src/components/block_renderer/translation/mm_block.ts index 9aa205b2270..f4f90b6db57 100644 --- a/webapp/channels/src/components/block_renderer/translation/mm_block.ts +++ b/webapp/channels/src/components/block_renderer/translation/mm_block.ts @@ -201,7 +201,8 @@ function translateButtonBlock(raw: Record): MmButtonBlock | nul if (!keysAreSubset(raw, BUTTON_KEYS)) { return null; } - if (typeof raw.text !== 'string' || typeof raw.action_id !== 'string') { + if (typeof raw.text !== 'string' || raw.text.trim() === '' || + typeof raw.action_id !== 'string' || raw.action_id.trim() === '') { return null; } const styleRaw = raw.style; @@ -268,7 +269,8 @@ function translateStaticSelectBlock(raw: Record): MmStaticSelec if (!keysAreSubset(raw, STATIC_SELECT_KEYS)) { return null; } - if (typeof raw.action_id !== 'string' || typeof raw.placeholder !== 'string') { + if (typeof raw.action_id !== 'string' || raw.action_id.trim() === '' || + typeof raw.placeholder !== 'string' || raw.placeholder.trim() === '') { return null; } let options: MmStaticSelectOption[] | undefined; diff --git a/webapp/channels/src/components/inline_action_button/index.test.tsx b/webapp/channels/src/components/inline_action_button/index.test.tsx index 503b6678816..a0cdcd6453a 100644 --- a/webapp/channels/src/components/inline_action_button/index.test.tsx +++ b/webapp/channels/src/components/inline_action_button/index.test.tsx @@ -6,6 +6,7 @@ import React from 'react'; import {doPostActionWithCookie} from 'mattermost-redux/actions/posts'; import {act, fireEvent, renderWithContext, screen, userEvent} from 'tests/react_testing_utils'; +import {applyIntegrationGotoLocation} from 'utils/integration_navigation'; import InlineActionButton from './index'; @@ -13,7 +14,12 @@ jest.mock('mattermost-redux/actions/posts', () => ({ doPostActionWithCookie: jest.fn(), })); +jest.mock('utils/integration_navigation', () => ({ + applyIntegrationGotoLocation: jest.fn(), +})); + const mockedDoPostActionWithCookie = doPostActionWithCookie as jest.MockedFunction; +const mockedApplyIntegrationGotoLocation = applyIntegrationGotoLocation as jest.MockedFunction; /** * Creates a thunk-shaped mock whose inner promise is externally controllable. @@ -42,6 +48,7 @@ describe('InlineActionButton', () => { beforeEach(() => { mockedDoPostActionWithCookie.mockReset(); + mockedApplyIntegrationGotoLocation.mockReset(); }); test('renders with children as button label', () => { @@ -416,9 +423,10 @@ describe('InlineActionButton', () => { } }); - test('uses doPostActionWithCookie when mmBlocksActionCookie is set', async () => { + test('uses doPostActionWithCookie when mmBlocksActionCookie is set and applies goto_location from response', async () => { + const gotoLocation = '/some-location'; mockedDoPostActionWithCookie.mockImplementation( - () => (() => Promise.resolve({data: {}})) as unknown as ReturnType, + () => (() => Promise.resolve({data: {goto_location: gotoLocation}})) as unknown as ReturnType, ); renderWithContext( @@ -440,5 +448,7 @@ describe('InlineActionButton', () => { {tail: '214', mds: 'C130J'}, 'mm_block', ); + expect(mockedApplyIntegrationGotoLocation).toHaveBeenCalledTimes(1); + expect(mockedApplyIntegrationGotoLocation).toHaveBeenCalledWith(gotoLocation); }); }); diff --git a/webapp/channels/src/components/post_view/interactive_messages/index.tsx b/webapp/channels/src/components/post_view/interactive_messages/index.tsx index 01cd9227ab8..4b6ff43c11b 100644 --- a/webapp/channels/src/components/post_view/interactive_messages/index.tsx +++ b/webapp/channels/src/components/post_view/interactive_messages/index.tsx @@ -8,6 +8,7 @@ // the existing doPostAction layer, consistent with Attachments. import React, {useCallback, useState} from 'react'; +import {useIntl} from 'react-intl'; import {useDispatch} from 'react-redux'; import type {Post} from '@mattermost/types/posts'; @@ -25,6 +26,7 @@ type Props = { const InteractiveMessages = ({post}: Props) => { const dispatch = useDispatch(); + const {formatMessage} = useIntl(); const [actionError, setActionError] = useState(null); const postProps = post.props as Record | undefined; @@ -33,6 +35,10 @@ const InteractiveMessages = ({post}: Props) => { const integrationFormat = getPostInteractiveIntegrationFormat(postProps ?? {}); const handleAction = useCallback(async (actionId: string, selectedOption?: string, query?: Record, attachmentCookie?: string) => { + const actionFailedMessage = formatMessage({ + id: 'post.message_attachment.action_failed', + defaultMessage: 'Action failed to execute', + }); setActionError(null); let actionCookie = ''; if (integrationFormat === 'attachment') { @@ -44,7 +50,7 @@ const InteractiveMessages = ({post}: Props) => { const result = await dispatch(doPostActionWithCookie(post.id, actionId, actionCookie, selectedOption ?? '', query, integrationFormat)); if (result.error) { const message = typeof result.error.message === 'string' && result.error.message ? result.error.message : undefined; - setActionError(message ?? 'Action failed to execute'); + setActionError(message ?? actionFailedMessage); return; } const goToLocation = @@ -57,9 +63,9 @@ const InteractiveMessages = ({post}: Props) => { } } catch (error) { const message = error instanceof Error ? error.message : undefined; - setActionError(message ?? 'Action failed to execute'); + setActionError(message ?? actionFailedMessage); } - }, [dispatch, post.id, integrationFormat, mmBlocksActionCookie]); + }, [dispatch, post.id, integrationFormat, mmBlocksActionCookie, formatMessage]); const blocks = translatePostProps(post.props as Record); if (!blocks || blocks.length === 0) { diff --git a/webapp/channels/src/packages/mattermost-redux/src/actions/posts.test.ts b/webapp/channels/src/packages/mattermost-redux/src/actions/posts.test.ts index 2423f49cd11..425047dc482 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/actions/posts.test.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/actions/posts.test.ts @@ -680,6 +680,72 @@ describe('Actions.Posts', () => { }), expected: new Set(['user1:org1', 'user2:org2', 'user3:org3', 'user4:org4', 'user5:org5', 'user6:org6']), }, + { + name: 'should return at-mentions from mm_blocks text blocks but not button labels', + input: TestHelper.getPostMock({ + props: { + mm_blocks: [ + {type: 'text', text: 'hello @bbb'}, + {type: 'button', text: '@ccc', action_id: 'act'}, + ], + }, + }), + expected: new Set(['bbb']), + }, + { + name: 'should return at-mentions from nested mm_blocks containers', + input: TestHelper.getPostMock({ + props: { + mm_blocks: [ + { + type: 'container', + content: [ + {type: 'text', text: '@ddd in container'}, + ], + }, + ], + }, + }), + expected: new Set(['ddd']), + }, + { + name: 'should return at-mentions from Block Kit markdown and section blocks', + input: TestHelper.getPostMock({ + props: { + blocks: [ + {type: 'markdown', text: 'markdown @eee'}, + { + type: 'section', + text: {type: 'mrkdwn', text: 'section @fff'}, + fields: [{type: 'mrkdwn', text: 'field @ggg'}], + }, + {type: 'header', text: 'header @hhh'}, + ], + }, + }), + expected: new Set(['eee', 'fff', 'ggg', 'hhh']), + }, + { + name: 'should return at-mentions from Adaptive Card TextBlock elements', + input: TestHelper.getPostMock({ + props: { + cards: [ + { + type: 'AdaptiveCard', + version: '1.0', + body: [ + {type: 'TextBlock', text: 'card @iii'}, + { + type: 'Container', + items: [{type: 'TextBlock', text: 'nested @jjj'}], + }, + ], + }, + ], + }, + }), + expected: new Set(['iii', 'jjj']), + }, ]; for (const specialMention of [ diff --git a/webapp/channels/src/packages/mattermost-redux/src/actions/posts.ts b/webapp/channels/src/packages/mattermost-redux/src/actions/posts.ts index 36a59458213..d21ef02e7f7 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/actions/posts.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/actions/posts.ts @@ -37,6 +37,7 @@ import {getUnreadScrollPositionPreference, isCollapsedThreadsEnabled} from 'matt import {getCurrentUserId, getUsersByUsername} from 'mattermost-redux/selectors/entities/users'; import type {ActionResult, DispatchFunc, GetStateFunc, ActionFunc, ActionFuncAsync, ThunkActionFunc} from 'mattermost-redux/types/actions'; import {DelayedDataLoader} from 'mattermost-redux/utils/data_loader'; +import {scanHumanReadableStringsFromInteractiveProps} from 'mattermost-redux/utils/post_interactive_utils'; import {isCombinedUserActivityPost} from 'mattermost-redux/utils/post_list'; import {logError, LogErrorBarMode} from './errors'; @@ -1149,6 +1150,10 @@ export function getNeededAtMentionedUsernamesAndGroups(state: GlobalState, posts } } } + + for (const text of scanHumanReadableStringsFromInteractiveProps(post.props)) { + findNeededUsernamesAndGroups(text); + } } return usernamesAndGroupsToLoad; diff --git a/webapp/channels/src/packages/mattermost-redux/src/utils/post_interactive_utils.ts b/webapp/channels/src/packages/mattermost-redux/src/utils/post_interactive_utils.ts new file mode 100644 index 00000000000..8dbd8869d88 --- /dev/null +++ b/webapp/channels/src/packages/mattermost-redux/src/utils/post_interactive_utils.ts @@ -0,0 +1,181 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +function appendHumanReadableStringsFromMmBlockMap(m: Record, out: string[]) { + const typ = m.type; + if (typeof typ !== 'string') { + return; + } + switch (typ) { + case 'text': + if (typeof m.text === 'string') { + out.push(m.text); + } + break; + case 'container': + appendHumanReadableStringsFromMmBlocksArray(m.content, out); + break; + case 'collapsible': + appendHumanReadableStringsFromMmBlocksArray(m.header, out); + appendHumanReadableStringsFromMmBlocksArray(m.content, out); + break; + case 'column_set': + if (Array.isArray(m.columns)) { + for (const col of m.columns) { + if (col && typeof col === 'object') { + appendHumanReadableStringsFromMmBlockMap(col as Record, out); + } + } + } + break; + case 'column': + appendHumanReadableStringsFromMmBlocksArray(m.items, out); + break; + default: + break; + } +} + +function appendHumanReadableStringsFromMmBlocksArray(raw: unknown, out: string[]) { + if (!Array.isArray(raw)) { + return; + } + for (const el of raw) { + if (el && typeof el === 'object') { + appendHumanReadableStringsFromMmBlockMap(el as Record, out); + } + } +} + +function appendHumanReadableStringsFromMmBlocks(raw: unknown, out: string[]) { + if (!Array.isArray(raw)) { + return; + } + for (const b of raw) { + if (b && typeof b === 'object') { + appendHumanReadableStringsFromMmBlockMap(b as Record, out); + } + } +} + +function appendHumanReadableStringsFromBlockKitTree(raw: unknown, out: string[]) { + if (!Array.isArray(raw)) { + return; + } + for (const block of raw) { + if (!block || typeof block !== 'object') { + continue; + } + const blockMap = block as Record; + const typ = blockMap.type; + if (typeof typ !== 'string') { + continue; + } + switch (typ) { + case 'markdown': + if (typeof blockMap.text === 'string') { + out.push(blockMap.text); + } + break; + case 'section': { + const textBlock = blockMap.text; + if (textBlock && typeof textBlock === 'object' && typeof (textBlock as Record).text === 'string') { + out.push((textBlock as Record).text as string); + } + if (Array.isArray(blockMap.fields)) { + for (const field of blockMap.fields) { + if (field && typeof field === 'object' && typeof (field as Record).text === 'string') { + out.push((field as Record).text as string); + } + } + } + break; + } + case 'header': + if (typeof blockMap.text === 'string') { + out.push(blockMap.text); + } + break; + default: + break; + } + } +} + +function appendHumanReadableStringsFromAdaptiveCardsItem(item: unknown, out: string[]) { + if (!item || typeof item !== 'object') { + return; + } + const itemMap = item as Record; + const typ = itemMap.type; + if (typeof typ !== 'string') { + return; + } + switch (typ) { + case 'TextBlock': + if (typeof itemMap.text === 'string') { + out.push(itemMap.text); + } + break; + case 'Container': + if (Array.isArray(itemMap.items)) { + for (const nested of itemMap.items) { + appendHumanReadableStringsFromAdaptiveCardsItem(nested, out); + } + } + break; + case 'ColumnSet': + if (Array.isArray(itemMap.columns)) { + for (const column of itemMap.columns) { + if (!column || typeof column !== 'object') { + continue; + } + const items = (column as Record).items; + if (Array.isArray(items)) { + for (const nested of items) { + appendHumanReadableStringsFromAdaptiveCardsItem(nested, out); + } + } + } + } + break; + default: + break; + } +} + +function appendHumanReadableStringsFromAdaptiveCardsTree(raw: unknown, out: string[]) { + if (!Array.isArray(raw)) { + return; + } + for (const card of raw) { + if (!card || typeof card !== 'object') { + continue; + } + const body = (card as Record).body; + if (!Array.isArray(body)) { + continue; + } + for (const item of body) { + appendHumanReadableStringsFromAdaptiveCardsItem(item, out); + } + } +} + +// Mirrors model.appendHumanReadableInteractiveStrings (mm_blocks, Block Kit blocks, Adaptive cards). +export function scanHumanReadableStringsFromInteractiveProps(props: Record | undefined): string[] { + const out: string[] = []; + if (!props) { + return out; + } + if (props.mm_blocks) { + appendHumanReadableStringsFromMmBlocks(props.mm_blocks, out); + } + if (props.blocks) { + appendHumanReadableStringsFromBlockKitTree(props.blocks, out); + } + if (props.cards) { + appendHumanReadableStringsFromAdaptiveCardsTree(props.cards, out); + } + return out; +} diff --git a/webapp/channels/src/utils/integration_navigation.test.ts b/webapp/channels/src/utils/integration_navigation.test.ts new file mode 100644 index 00000000000..99a285b4c37 --- /dev/null +++ b/webapp/channels/src/utils/integration_navigation.test.ts @@ -0,0 +1,34 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {applyIntegrationGotoLocation} from './integration_navigation'; + +jest.mock('utils/browser_history', () => ({ + getHistory: () => ({push: jest.fn()}), +})); + +jest.mock('utils/url', () => ({ + getSiteURL: () => 'https://mattermost.example.com', + isUrlSafe: (url: string) => url.startsWith('https://'), +})); + +describe('applyIntegrationGotoLocation', () => { + const openSpy = jest.spyOn(window, 'open').mockImplementation(() => null); + + beforeEach(() => { + openSpy.mockClear(); + }); + + afterAll(() => { + openSpy.mockRestore(); + }); + + it('opens external urls with noopener and noreferrer', () => { + applyIntegrationGotoLocation('https://external.example.com/path'); + expect(openSpy).toHaveBeenCalledWith( + 'https://external.example.com/path', + '_blank', + 'noopener,noreferrer', + ); + }); +}); diff --git a/webapp/channels/src/utils/integration_navigation.ts b/webapp/channels/src/utils/integration_navigation.ts index bd5bd675e6a..1152abaa5ac 100644 --- a/webapp/channels/src/utils/integration_navigation.ts +++ b/webapp/channels/src/utils/integration_navigation.ts @@ -21,5 +21,5 @@ export function applyIntegrationGotoLocation(gotoLocation: string | undefined): getHistory().push(gotoLocation.substring(siteURL.length)); return; } - window.open(gotoLocation); + window.open(gotoLocation, '_blank', 'noopener,noreferrer'); }