MM-42810: Using websocket broadcast hook for permalink preview (#28627)

We use the newly introduced websocket broadcast hook system
to implement permalink preview efficiently. This is essentially
a re-do of https://github.com/mattermost/mattermost/pull/23812
using the new system.

https://mattermost.atlassian.net/browse/MM-42810
```release-note
NONE
```

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
Этот коммит содержится в:
Agniva De Sarker
2024-10-21 09:35:32 +05:30
коммит произвёл GitHub
родитель 6939623e11
Коммит 64677dd554
11 изменённых файлов: 226 добавлений и 126 удалений

Просмотреть файл

@@ -702,8 +702,8 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea
}
usePostedAckHook(message, post.UserId, channel.Type, usersToAck)
published, err := a.publishWebsocketEventForPermalinkPost(c, post, message)
if err != nil {
appErr := a.publishWebsocketEventForPost(c, post, message)
if appErr != nil {
a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeWebsocket, model.NotificationReasonFetchError, model.NotificationNoPlatform)
a.NotificationsLog().Error("Couldn't send websocket notification for permalink post",
mlog.String("type", model.NotificationTypeWebsocket),
@@ -711,28 +711,9 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea
mlog.String("status", model.NotificationStatusError),
mlog.String("reason", model.NotificationReasonFetchError),
mlog.String("sender_id", sender.Id),
mlog.Err(err),
mlog.Err(appErr),
)
return nil, err
}
if !published {
removePermalinkMetadataFromPost(post)
postJSON, jsonErr := post.ToJSON()
if jsonErr != nil {
a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeWebsocket, model.NotificationReasonParseError, model.NotificationNoPlatform)
a.NotificationsLog().Error("JSON parse error",
mlog.String("type", model.NotificationTypeWebsocket),
mlog.String("post_id", post.Id),
mlog.String("status", model.NotificationStatusError),
mlog.String("reason", model.NotificationReasonParseError),
mlog.String("sender_id", sender.Id),
mlog.Err(err),
)
return nil, errors.Wrapf(jsonErr, "failed to encode post to JSON")
}
message.Add("post", postJSON)
a.Publish(message)
return nil, appErr
}
// If this is a reply in a thread, notify participants

Просмотреть файл

@@ -413,7 +413,7 @@ func TestSendNotifications_MentionsFollowers(t *testing.T) {
{
Type: model.PostEmbedPermalink,
URL: postURL,
Data: &model.Post{},
Data: &model.PreviewPost{},
},
},
},

Просмотреть файл

@@ -57,6 +57,9 @@ func (ms *mockSuite) RolesGrantPermission(roleNames []string, permissionId strin
func (ms *mockSuite) UserCanSeeOtherUser(c request.CTX, userID string, otherUserId string) (bool, *model.AppError) {
return true, nil
}
func (ms *mockSuite) HasPermissionToReadChannel(c request.CTX, userID string, channel *model.Channel) bool {
return true
}
func Setup(tb testing.TB, options ...Option) *TestHelper {
if testing.Short() {

Просмотреть файл

@@ -48,6 +48,24 @@ func (_m *SuiteIFace) GetSession(token string) (*model.Session, *model.AppError)
return r0, r1
}
// HasPermissionToReadChannel provides a mock function with given fields: c, userID, channel
func (_m *SuiteIFace) HasPermissionToReadChannel(c request.CTX, userID string, channel *model.Channel) bool {
ret := _m.Called(c, userID, channel)
if len(ret) == 0 {
panic("no return value specified for HasPermissionToReadChannel")
}
var r0 bool
if rf, ok := ret.Get(0).(func(request.CTX, string, *model.Channel) bool); ok {
r0 = rf(c, userID, channel)
} else {
r0 = ret.Get(0).(bool)
}
return r0
}
// RolesGrantPermission provides a mock function with given fields: roleNames, permissionId
func (_m *SuiteIFace) RolesGrantPermission(roleNames []string, permissionId string) bool {
ret := _m.Called(roleNames, permissionId)

Просмотреть файл

@@ -24,6 +24,7 @@ const (
type SuiteIFace interface {
GetSession(token string) (*model.Session, *model.AppError)
RolesGrantPermission(roleNames []string, permissionId string) bool
HasPermissionToReadChannel(c request.CTX, userID string, channel *model.Channel) bool
UserCanSeeOtherUser(c request.CTX, userID string, otherUserId string) (bool, *model.AppError)
}

Просмотреть файл

@@ -647,25 +647,25 @@ func (a *App) UpdatePost(c request.CTX, receivedUpdatedPost *model.Post, safeUpd
}
oldPost := postLists.Posts[receivedUpdatedPost.Id]
var err *model.AppError
var appErr *model.AppError
if oldPost == nil {
err = model.NewAppError("UpdatePost", "api.post.update_post.find.app_error", nil, "id="+receivedUpdatedPost.Id, http.StatusBadRequest)
return nil, err
appErr = model.NewAppError("UpdatePost", "api.post.update_post.find.app_error", nil, "id="+receivedUpdatedPost.Id, http.StatusBadRequest)
return nil, appErr
}
if oldPost.DeleteAt != 0 {
err = model.NewAppError("UpdatePost", "api.post.update_post.permissions_details.app_error", map[string]any{"PostId": receivedUpdatedPost.Id}, "", http.StatusBadRequest)
return nil, err
appErr = model.NewAppError("UpdatePost", "api.post.update_post.permissions_details.app_error", map[string]any{"PostId": receivedUpdatedPost.Id}, "", http.StatusBadRequest)
return nil, appErr
}
if oldPost.IsSystemMessage() {
err = model.NewAppError("UpdatePost", "api.post.update_post.system_message.app_error", nil, "id="+receivedUpdatedPost.Id, http.StatusBadRequest)
return nil, err
appErr = model.NewAppError("UpdatePost", "api.post.update_post.system_message.app_error", nil, "id="+receivedUpdatedPost.Id, http.StatusBadRequest)
return nil, appErr
}
channel, err := a.GetChannel(c, oldPost.ChannelId)
if err != nil {
return nil, err
channel, appErr := a.GetChannel(c, oldPost.ChannelId)
if appErr != nil {
return nil, appErr
}
if channel.DeleteAt != 0 {
@@ -692,8 +692,8 @@ func (a *App) UpdatePost(c request.CTX, receivedUpdatedPost *model.Post, safeUpd
newPost.EditAt = model.GetMillis()
}
if err = a.FillInPostProps(c, newPost, nil); err != nil {
return nil, err
if appErr = a.FillInPostProps(c, newPost, nil); appErr != nil {
return nil, appErr
}
if receivedUpdatedPost.IsRemote() {
@@ -715,7 +715,6 @@ func (a *App) UpdatePost(c request.CTX, receivedUpdatedPost *model.Post, safeUpd
rpost, nErr := a.Srv().Store().Post().Update(c, newPost, oldPost)
if nErr != nil {
var appErr *model.AppError
switch {
case errors.As(nErr, &appErr):
return nil, appErr
@@ -747,26 +746,17 @@ func (a *App) UpdatePost(c request.CTX, receivedUpdatedPost *model.Post, safeUpd
message := model.NewWebSocketEvent(model.WebsocketEventPostEdited, "", rpost.ChannelId, "", nil, "")
published, err := a.publishWebsocketEventForPermalinkPost(c, rpost, message)
if err != nil {
return nil, err
}
if !published {
removePermalinkMetadataFromPost(rpost)
postJSON, jsonErr := rpost.ToJSON()
if jsonErr != nil {
return nil, model.NewAppError("UpdatePost", "app.post.marshal.app_error", nil, "", http.StatusInternalServerError).Wrap(jsonErr)
}
message.Add("post", postJSON)
a.Publish(message)
appErr = a.publishWebsocketEventForPost(c, rpost, message)
if appErr != nil {
return nil, appErr
}
a.invalidateCacheForChannelPosts(rpost.ChannelId)
userID := c.Session().UserId
sanitizedPost, err := a.SanitizePostMetadataForUser(c, rpost, userID)
if err != nil {
mlog.Error("Failed to sanitize post metadata for user", mlog.String("user_id", userID), mlog.Err(err))
sanitizedPost, appErr := a.SanitizePostMetadataForUser(c, rpost, userID)
if appErr != nil {
mlog.Error("Failed to sanitize post metadata for user", mlog.String("user_id", userID), mlog.Err(appErr))
// If we failed to sanitize the post, we still want to remove the metadata.
sanitizedPost = rpost.Clone()
@@ -778,109 +768,119 @@ func (a *App) UpdatePost(c request.CTX, receivedUpdatedPost *model.Post, safeUpd
return rpost, nil
}
func (a *App) publishWebsocketEventForPermalinkPost(c request.CTX, post *model.Post, message *model.WebSocketEvent) (published bool, err *model.AppError) {
var previewedPostID string
if val, ok := post.GetProp(model.PostPropsPreviewedPost).(string); ok {
previewedPostID = val
} else {
return false, nil
func (a *App) publishWebsocketEventForPost(rctx request.CTX, post *model.Post, message *model.WebSocketEvent) *model.AppError {
postJSON, jsonErr := post.ToJSON()
if jsonErr != nil {
a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeAll, model.NotificationReasonMarshalError, model.NotificationNoPlatform)
a.NotificationsLog().Error("Error in marshalling post to JSON",
mlog.String("type", model.NotificationTypeWebsocket),
mlog.String("post_id", post.Id),
mlog.String("status", model.NotificationStatusError),
mlog.String("reason", model.NotificationReasonMarshalError),
)
return model.NewAppError("publishWebsocketEventForPost", "app.post.marshal.app_error", nil, "", http.StatusInternalServerError).Wrap(jsonErr)
}
message.Add("post", postJSON)
appErr := a.setupBroadcastHookForPermalink(rctx, post, message, postJSON)
if appErr != nil {
return appErr
}
if !model.IsValidId(previewedPostID) {
a.Publish(message)
return nil
}
func (a *App) setupBroadcastHookForPermalink(rctx request.CTX, post *model.Post, message *model.WebSocketEvent, postJSON string) *model.AppError {
// We check for the post first, and then the prop to prevent
// any embedded data to remain in case a post does not contain the prop
// but contains the embedded data.
permalinkPreviewedPost := post.GetPreviewPost()
if permalinkPreviewedPost == nil {
return nil
}
previewProp := post.GetPreviewedPostProp()
if previewProp == "" {
return nil
}
// To remain secure by default, we wipe out the metadata unconditionally.
removePermalinkMetadataFromPost(post)
postWithoutPermalinkPreviewJSON, err := post.ToJSON()
if err != nil {
a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeAll, model.NotificationReasonMarshalError, model.NotificationNoPlatform)
a.NotificationsLog().Error("Error in marshalling post to JSON",
mlog.String("type", model.NotificationTypeWebsocket),
mlog.String("post_id", post.Id),
mlog.String("status", model.NotificationStatusError),
mlog.String("reason", model.NotificationReasonMarshalError),
)
return model.NewAppError("publishWebsocketEventForPost", "app.post.marshal.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
message.Add("post", postWithoutPermalinkPreviewJSON)
if !model.IsValidId(previewProp) {
a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeAll, model.NotificationReasonParseError, model.NotificationNoPlatform)
a.NotificationsLog().Error("Invalid post prop id for permalink post",
mlog.String("type", model.NotificationTypeWebsocket),
mlog.String("post_id", post.Id),
mlog.String("status", model.NotificationStatusError),
mlog.String("reason", model.NotificationReasonParseError),
mlog.String("prop_value", previewedPostID),
mlog.String("prop_value", previewProp),
)
c.Logger().Warn("invalid post prop value", mlog.String("prop_key", model.PostPropsPreviewedPost), mlog.String("prop_value", previewedPostID))
return false, nil
rctx.Logger().Warn("invalid post prop value", mlog.String("prop_key", model.PostPropsPreviewedPost), mlog.String("prop_value", previewProp))
// In this case, it will broadcast the message with metadata wiped out
return nil
}
previewedPost, err := a.GetSinglePost(c, previewedPostID, false)
if err != nil {
if err.StatusCode == http.StatusNotFound {
previewedPost, appErr := a.GetSinglePost(rctx, previewProp, false)
if appErr != nil {
if appErr.StatusCode == http.StatusNotFound {
a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeAll, model.NotificationReasonFetchError, model.NotificationNoPlatform)
a.NotificationsLog().Error("permalink post not found",
mlog.String("type", model.NotificationTypeWebsocket),
mlog.String("post_id", post.Id),
mlog.String("status", model.NotificationStatusError),
mlog.String("reason", model.NotificationReasonFetchError),
mlog.String("referenced_post_id", previewedPostID),
mlog.Err(err),
mlog.String("referenced_post_id", previewProp),
mlog.Err(appErr),
)
c.Logger().Warn("permalinked post not found", mlog.String("referenced_post_id", previewedPostID))
return false, nil
rctx.Logger().Warn("permalinked post not found", mlog.String("referenced_post_id", previewProp))
// In this case, it will broadcast the message with metadata wiped out
return nil
}
return false, err
return appErr
}
userIDs, nErr := a.Srv().Store().Channel().GetAllChannelMemberIdsByChannelId(post.ChannelId)
if nErr != nil {
a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeAll, model.NotificationReasonFetchError, model.NotificationNoPlatform)
a.NotificationsLog().Error("Cannot get channel members",
mlog.String("type", model.NotificationTypeWebsocket),
mlog.String("post_id", post.Id),
mlog.String("status", model.NotificationStatusError),
mlog.String("reason", model.NotificationReasonFetchError),
mlog.String("referenced_post_id", previewedPostID),
mlog.Err(nErr),
)
return false, model.NewAppError("publishWebsocketEventForPermalinkPost", "app.channel.get_members.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
permalinkPreviewedChannel, err := a.GetChannel(c, previewedPost.ChannelId)
if err != nil {
if err.StatusCode == http.StatusNotFound {
permalinkPreviewedChannel, appErr := a.GetChannel(rctx, previewedPost.ChannelId)
if appErr != nil {
if appErr.StatusCode == http.StatusNotFound {
a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeAll, model.NotificationReasonFetchError, model.NotificationNoPlatform)
a.NotificationsLog().Error("Cannot get channel",
mlog.String("type", model.NotificationTypeWebsocket),
mlog.String("post_id", post.Id),
mlog.String("status", model.NotificationStatusError),
mlog.String("reason", model.NotificationReasonFetchError),
mlog.String("referenced_post_id", previewedPostID),
mlog.String("referenced_post_id", previewedPost.Id),
)
c.Logger().Warn("channel containing permalinked post not found", mlog.String("referenced_channel_id", previewedPost.ChannelId))
return false, nil
rctx.Logger().Warn("channel containing permalinked post not found", mlog.String("referenced_channel_id", previewedPost.ChannelId))
// In this case, it will broadcast the message with metadata wiped out
return nil
}
return false, err
return appErr
}
originalEmbeds := post.Metadata.Embeds
originalProps := post.GetProps()
permalinkPreviewedPost := post.GetPreviewPost()
for _, userID := range userIDs {
if permalinkPreviewedPost != nil {
post.Metadata.Embeds = originalEmbeds
post.SetProps(originalProps)
}
postForUser := a.sanitizePostMetadataForUserAndChannel(c, post, permalinkPreviewedPost, permalinkPreviewedChannel, userID)
// Using DeepCopy here to avoid a race condition
// between publishing the event and setting the "post" data value below.
messageCopy := message.DeepCopy()
broadcastCopy := messageCopy.GetBroadcast()
broadcastCopy.UserId = userID
messageCopy.SetBroadcast(broadcastCopy)
postJSON, jsonErr := postForUser.ToJSON()
if jsonErr != nil {
c.Logger().Warn("Failed to encode post to JSON", mlog.Err(jsonErr))
}
messageCopy.Add("post", postJSON)
a.Publish(messageCopy)
// In case the user does have permission to read, we set the metadata back.
// Note that this is the return value to the post creator, and has nothing to do
// with the content of the websocket broadcast to that user or any other.
if a.HasPermissionToReadChannel(rctx, post.UserId, permalinkPreviewedChannel) {
post.AddProp(model.PostPropsPreviewedPost, previewProp)
post.Metadata.Embeds = append(post.Metadata.Embeds, &model.PostEmbed{Type: model.PostEmbedPermalink, Data: permalinkPreviewedPost})
}
// Restore the metadata that may have been removed in the sanitization
if permalinkPreviewedPost != nil {
post.Metadata.Embeds = originalEmbeds
post.SetProps(originalProps)
}
return true, nil
usePermalinkHook(message, permalinkPreviewedChannel, postJSON)
return nil
}
func (a *App) PatchPost(c request.CTX, postID string, patch *model.PostPatch) (*model.Post, *model.AppError) {

Просмотреть файл

@@ -1593,12 +1593,12 @@ func TestUpdatePost(t *testing.T) {
testPost, err = th.App.CreatePost(th.Context, testPost, channelForTestPost, false, false)
require.Nil(t, err)
assert.Equal(t, testPost.GetProps(), model.StringInterface{})
assert.Equal(t, model.StringInterface{}, testPost.GetProps())
testPost.Message = permalink
testPost, err = th.App.UpdatePost(th.Context, testPost, false)
require.Nil(t, err)
assert.Equal(t, testPost.GetProps(), model.StringInterface{"previewed_post": referencedPost.Id})
assert.Equal(t, model.StringInterface{model.PostPropsPreviewedPost: referencedPost.Id}, testPost.GetProps())
})
t.Run("sanitizes post metadata appropriately", func(t *testing.T) {

Просмотреть файл

@@ -9,6 +9,7 @@ import (
"slices"
"github.com/mattermost/mattermost/server/public/model"
"github.com/mattermost/mattermost/server/public/shared/request"
"github.com/mattermost/mattermost/server/v8/channels/app/platform"
"github.com/pkg/errors"
)
@@ -17,6 +18,7 @@ const (
broadcastAddMentions = "add_mentions"
broadcastAddFollowers = "add_followers"
broadcastPostedAck = "posted_ack"
broadcastPermalink = "permalink"
)
func (s *Server) makeBroadcastHooks() map[string]platform.BroadcastHook {
@@ -24,6 +26,7 @@ func (s *Server) makeBroadcastHooks() map[string]platform.BroadcastHook {
broadcastAddMentions: &addMentionsBroadcastHook{},
broadcastAddFollowers: &addFollowersBroadcastHook{},
broadcastPostedAck: &postedAckBroadcastHook{},
broadcastPermalink: &permalinkBroadcastHook{},
}
}
@@ -119,7 +122,7 @@ func (h *postedAckBroadcastHook) Process(msg *platform.HookedWebSocketEvent, web
users, err := getTypedArg[model.StringArray](args, "users")
if err != nil {
return errors.Wrap(err, "Invalid users value passed to addFollowersBroadcastHook")
return errors.Wrap(err, "Invalid users value passed to postedAckBroadcastHook")
}
if len(users) > 0 && slices.Contains(users, webConn.UserId) {
@@ -130,6 +133,40 @@ func (h *postedAckBroadcastHook) Process(msg *platform.HookedWebSocketEvent, web
return nil
}
func usePermalinkHook(message *model.WebSocketEvent, previewChannel *model.Channel, postJSON string) {
message.GetBroadcast().AddHook(broadcastPermalink, map[string]any{
"preview_channel": previewChannel,
"post_json": postJSON,
})
}
type permalinkBroadcastHook struct{}
// Process adds the post medata from usePermalinkHook to the websocket event
// if the user has access to the containing channel.
func (h *permalinkBroadcastHook) Process(msg *platform.HookedWebSocketEvent, webConn *platform.WebConn, args map[string]any) error {
previewChannel, err := getTypedArg[*model.Channel](args, "preview_channel")
if err != nil {
return errors.Wrap(err, "Invalid preview_channel value passed to permalinkBroadcastHook")
}
rctx := request.EmptyContext(webConn.Platform.Log())
if !webConn.Suite.HasPermissionToReadChannel(rctx, webConn.UserId, previewChannel) {
// Do nothing.
// In this case, the sanitized post is already attached to the ws event.
return nil
}
// Else, we set the post with permalink preview.
postJSON, err := getTypedArg[string](args, "post_json")
if err != nil {
return errors.Wrap(err, "Invalid post_json value passed to permalinkBroadcastHook")
}
msg.Add("post", postJSON)
return nil
}
func incrementWebsocketCounter(wc *platform.WebConn) {
if wc.Platform.Metrics() == nil {
return

Просмотреть файл

@@ -208,3 +208,59 @@ func TestAddMentionsAndAddFollowersHooks(t *testing.T) {
assert.Equal(t, `["`+userID+`"]`, msg.Event().GetData()["mentions"])
})
}
func TestPermalinkBroadcastHook(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
session, err := th.Server.Platform().CreateSession(th.Context, &model.Session{
UserId: th.BasicUser.Id,
})
require.NoError(t, err)
wc := &platform.WebConn{
Platform: th.Server.Platform(),
Suite: th.App,
UserId: session.UserId,
}
hook := &permalinkBroadcastHook{}
refPost := th.CreatePost(th.BasicChannel)
th.BasicPost.Metadata.Embeds = append(th.BasicPost.Metadata.Embeds, &model.PostEmbed{Type: model.PostEmbedPermalink, Data: &model.Permalink{
PreviewPost: model.NewPreviewPost(refPost, th.BasicTeam, th.BasicChannel),
}})
originalJSON, err := th.BasicPost.ToJSON()
require.NoError(t, err)
wsEvent := model.NewWebSocketEvent(model.WebsocketEventPosted, "", th.BasicPost.ChannelId, "", nil, "")
th.BasicPost.Metadata.Embeds[0].Data = nil
removedJSON, err := th.BasicPost.ToJSON()
require.NoError(t, err)
wsEvent.Add("post", removedJSON)
msg := platform.MakeHookedWebSocketEvent(wsEvent)
// User has permission.
err = hook.Process(msg, wc, map[string]any{
"preview_channel": th.BasicChannel,
"post_json": originalJSON,
})
require.NoError(t, err)
gotJSON, ok := msg.Get("post").(string)
require.True(t, ok)
require.Equal(t, originalJSON, gotJSON)
msg = platform.MakeHookedWebSocketEvent(wsEvent)
// User does not exist, and thus won't have permission to the channel.
wc.UserId = "otheruser"
err = hook.Process(msg, wc, map[string]any{
"preview_channel": th.BasicChannel,
"post_json": originalJSON,
})
require.NoError(t, err)
gotJSON, ok = msg.Get("post").(string)
require.True(t, ok)
require.Equal(t, removedJSON, gotJSON)
}

Просмотреть файл

@@ -22,6 +22,7 @@ const (
NotificationReasonFetchError NotificationReason = "fetch_error"
NotificationReasonParseError NotificationReason = "json_parse_error"
NotificationReasonMarshalError NotificationReason = "json_marshal_error"
NotificationReasonPushProxyError NotificationReason = "push_proxy_error"
NotificationReasonPushProxySendError NotificationReason = "push_proxy_send_error"
NotificationReasonPushProxyRemoveDevice NotificationReason = "push_proxy_remove_device"

Просмотреть файл

@@ -878,8 +878,11 @@ func (o *Post) ForPlugin() *Post {
}
func (o *Post) GetPreviewPost() *PreviewPost {
if o.Metadata == nil {
return nil
}
for _, embed := range o.Metadata.Embeds {
if embed.Type == PostEmbedPermalink {
if embed != nil && embed.Type == PostEmbedPermalink {
if previewPost, ok := embed.Data.(*PreviewPost); ok {
return previewPost
}