From 2ff0fe343e9637ec42b2254071b410d20af5a7b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Thu, 30 Nov 2023 11:43:51 +0100 Subject: [PATCH] [MM-45272] Fix MM-45272 (#24701) * Fix MM-45272 * Properly handle permalinks * Fix * Fix tests * Handle only not found case for team member * Fix lint * Use proper config value * Separate permission in several statements * Add tests * Fix lint * Revert changes on utils * Address feedback and more fixes * Address feedback * Fix test * Fix test and related bug * Fix and reorder test * Address feedback * Address feedback --------- Co-authored-by: Mattermost Build --- server/channels/api4/post_test.go | 10 +- server/channels/app/authorization.go | 10 +- server/channels/app/authorization_test.go | 114 ++++++++++++ server/channels/app/post.go | 44 ++++- server/channels/app/post_metadata.go | 16 +- server/channels/app/post_metadata_test.go | 4 +- server/channels/app/post_test.go | 173 +++++++----------- .../channel_identifier_router/actions.ts | 2 +- .../src/components/permalink_view/actions.ts | 58 +++--- .../permalink_view/permalink_view.test.tsx | 99 ++++++---- .../team_controller/team_controller.tsx | 4 +- webapp/channels/src/utils/channel_utils.tsx | 4 +- webapp/platform/client/src/client4.ts | 9 +- webapp/platform/types/src/posts.ts | 12 ++ 14 files changed, 381 insertions(+), 178 deletions(-) diff --git a/server/channels/api4/post_test.go b/server/channels/api4/post_test.go index 0d28c057ac..9136c6c4e7 100644 --- a/server/channels/api4/post_test.go +++ b/server/channels/api4/post_test.go @@ -3795,7 +3795,7 @@ func TestPostGetInfo(t *testing.T) { dmPost, _, err := client.CreatePost(context.Background(), &model.Post{ChannelId: dmChannel.Id}) require.NoError(t, err) - openTeam, _, err := sysadminClient.CreateTeam(context.Background(), &model.Team{Type: model.TeamOpen, Name: "open-team", DisplayName: "Open Team"}) + openTeam, _, err := sysadminClient.CreateTeam(context.Background(), &model.Team{Type: model.TeamOpen, Name: "open-team", DisplayName: "Open Team", AllowOpenInvite: true}) require.NoError(t, err) openTeamOpenChannel, _, err := sysadminClient.CreateChannel(context.Background(), &model.Channel{TeamId: openTeam.Id, Type: model.ChannelTypeOpen, Name: "open-team-open-channel", DisplayName: "Open Team - Open Channel"}) require.NoError(t, err) @@ -3803,7 +3803,7 @@ func TestPostGetInfo(t *testing.T) { require.NoError(t, err) // Alt team is a team without the sysadmin in it. - altOpenTeam, _, err := client.CreateTeam(context.Background(), &model.Team{Type: model.TeamOpen, Name: "alt-open-team", DisplayName: "Alt Open Team"}) + altOpenTeam, _, err := client.CreateTeam(context.Background(), &model.Team{Type: model.TeamOpen, Name: "alt-open-team", DisplayName: "Alt Open Team", AllowOpenInvite: true}) require.NoError(t, err) altOpenTeamOpenChannel, _, err := client.CreateChannel(context.Background(), &model.Channel{TeamId: altOpenTeam.Id, Type: model.ChannelTypeOpen, Name: "alt-open-team-open-channel", DisplayName: "Open Team - Open Channel"}) require.NoError(t, err) @@ -4008,8 +4008,12 @@ func TestPostGetInfo(t *testing.T) { require.Equal(t, tc.channel.DisplayName, info.ChannelDisplayName) require.Equal(t, tc.hasJoinedChannel, info.HasJoinedChannel) if tc.team != nil { + teamType := "I" + if tc.team.AllowOpenInvite { + teamType = "O" + } require.Equal(t, tc.team.Id, info.TeamId) - require.Equal(t, tc.team.Type, info.TeamType) + require.Equal(t, teamType, info.TeamType) require.Equal(t, tc.team.DisplayName, info.TeamDisplayName) require.Equal(t, tc.hasJoinedTeam, info.HasJoinedTeam) } diff --git a/server/channels/app/authorization.go b/server/channels/app/authorization.go index 4e11559e91..2f762c7e02 100644 --- a/server/channels/app/authorization.go +++ b/server/channels/app/authorization.go @@ -380,5 +380,13 @@ func (a *App) HasPermissionToReadChannel(c request.CTX, userID string, channel * if !*a.Config().TeamSettings.ExperimentalViewArchivedChannels && channel.DeleteAt != 0 { return false } - return a.HasPermissionToChannel(c, userID, channel.Id, model.PermissionReadChannelContent) || (channel.Type == model.ChannelTypeOpen && a.HasPermissionToTeam(c, userID, channel.TeamId, model.PermissionReadPublicChannel)) + if a.HasPermissionToChannel(c, userID, channel.Id, model.PermissionReadChannelContent) { + return true + } + + if channel.Type == model.ChannelTypeOpen && !*a.Config().ComplianceSettings.Enable { + return a.HasPermissionToTeam(c, userID, channel.TeamId, model.PermissionReadPublicChannel) + } + + return false } diff --git a/server/channels/app/authorization_test.go b/server/channels/app/authorization_test.go index 414ac2d660..95417d94fa 100644 --- a/server/channels/app/authorization_test.go +++ b/server/channels/app/authorization_test.go @@ -553,3 +553,117 @@ func TestSessionHasPermissionToGroup(t *testing.T) { } } } + +func TestHasPermissionToReadChannel(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + ttcc := []struct { + name string + configViewArchived bool + configComplianceEnabled bool + channelDeleted bool + canReadChannel bool + channelIsOpen bool + canReadPublicChannel bool + expected bool + }{ + { + name: "Cannot read archived channels if the config doesn't allow it", + configViewArchived: false, + configComplianceEnabled: true, + channelDeleted: true, + canReadChannel: true, + channelIsOpen: true, + canReadPublicChannel: true, + expected: false, + }, + { + name: "Can read if it has permissions to read", + configViewArchived: false, + configComplianceEnabled: true, + channelDeleted: false, + canReadChannel: true, + channelIsOpen: false, + canReadPublicChannel: true, + expected: true, + }, + { + name: "Cannot read private channels if it has no permission", + configViewArchived: false, + configComplianceEnabled: false, + channelDeleted: false, + canReadChannel: false, + channelIsOpen: false, + canReadPublicChannel: true, + expected: false, + }, + { + name: "Cannot read open channels if compliance is enabled", + configViewArchived: false, + configComplianceEnabled: true, + channelDeleted: false, + canReadChannel: false, + channelIsOpen: true, + canReadPublicChannel: true, + expected: false, + }, + { + name: "Cannot read open channels if it has no team permissions", + configViewArchived: false, + configComplianceEnabled: false, + channelDeleted: false, + canReadChannel: false, + channelIsOpen: true, + canReadPublicChannel: false, + expected: false, + }, + { + name: "Can read open channels if it has team permissions and compliance is not enabled", + configViewArchived: false, + configComplianceEnabled: false, + channelDeleted: false, + canReadChannel: false, + channelIsOpen: true, + canReadPublicChannel: true, + expected: true, + }, + } + + for _, tc := range ttcc { + t.Run(tc.name, func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + configViewArchived := tc.configViewArchived + configComplianceEnabled := tc.configComplianceEnabled + cfg.TeamSettings.ExperimentalViewArchivedChannels = &configViewArchived + cfg.ComplianceSettings.Enable = &configComplianceEnabled + }) + + team := th.CreateTeam() + if tc.canReadPublicChannel { + th.LinkUserToTeam(th.BasicUser2, team) + } + + var channel *model.Channel + if tc.channelIsOpen { + channel = th.CreateChannel(th.Context, team) + } else { + channel = th.CreatePrivateChannel(th.Context, team) + } + if tc.canReadChannel { + _, err := th.App.AddUserToChannel(th.Context, th.BasicUser2, channel, false) + require.Nil(t, err) + } + + if tc.channelDeleted { + err := th.App.DeleteChannel(th.Context, channel, th.SystemAdminUser.Id) + require.Nil(t, err) + channel, err = th.App.GetChannel(th.Context, channel.Id) + require.Nil(t, err) + } + + result := th.App.HasPermissionToReadChannel(th.Context, th.BasicUser2.Id, channel) + require.Equal(t, tc.expected, result) + }) + } +} diff --git a/server/channels/app/post.go b/server/channels/app/post.go index 5a179f010a..859a23738a 100644 --- a/server/channels/app/post.go +++ b/server/channels/app/post.go @@ -799,10 +799,13 @@ func (a *App) publishWebsocketEventForPermalinkPost(c request.CTX, post *model.P return false, err } + originalEmbeds := post.Metadata.Embeds + originalProps := post.GetProps() permalinkPreviewedPost := post.GetPreviewPost() for _, userID := range userIDs { if permalinkPreviewedPost != nil { - post.Metadata.Embeds[0].Data = permalinkPreviewedPost + post.Metadata.Embeds = originalEmbeds + post.SetProps(originalProps) } postForUser := a.sanitizePostMetadataForUserAndChannel(c, post, permalinkPreviewedPost, permalinkPreviewedChannel, userID) @@ -822,6 +825,12 @@ func (a *App) publishWebsocketEventForPermalinkPost(c request.CTX, post *model.P a.Publish(messageCopy) } + // Restore the metadata that may have been removed in the sanitization + if permalinkPreviewedPost != nil { + post.Metadata.Embeds = originalEmbeds + post.SetProps(originalProps) + } + return true, nil } @@ -2033,7 +2042,7 @@ func (a *App) GetPostIfAuthorized(c request.CTX, postID string, session *model.S } if !a.SessionHasPermissionToChannel(c, *session, channel.Id, model.PermissionReadChannelContent) { - if channel.Type == model.ChannelTypeOpen { + if channel.Type == model.ChannelTypeOpen && !*a.Config().ComplianceSettings.Enable { if !a.SessionHasPermissionToTeam(*session, channel.TeamId, model.PermissionReadPublicChannel) { return nil, a.MakePermissionError(session, []*model.Permission{model.PermissionReadPublicChannel}) } @@ -2233,10 +2242,23 @@ func (a *App) GetPostInfo(c request.CTX, postID string) (*model.PostInfo, *model return nil, appErr } - if team.Type == model.TeamOpen { - hasPermissionToAccessTeam = a.HasPermissionToTeam(c, userID, team.Id, model.PermissionJoinPublicTeams) - } else if team.Type == model.TeamInvite { - hasPermissionToAccessTeam = a.HasPermissionToTeam(c, userID, team.Id, model.PermissionJoinPrivateTeams) + teamMember, appErr := a.GetTeamMember(c, channel.TeamId, userID) + if appErr != nil && appErr.StatusCode != http.StatusNotFound { + return nil, appErr + } + + if appErr == nil { + if teamMember.DeleteAt == 0 { + hasPermissionToAccessTeam = true + } + } + + if !hasPermissionToAccessTeam { + if team.AllowOpenInvite { + hasPermissionToAccessTeam = a.HasPermissionToTeam(c, userID, team.Id, model.PermissionJoinPublicTeams) + } else { + hasPermissionToAccessTeam = a.HasPermissionToTeam(c, userID, team.Id, model.PermissionJoinPrivateTeams) + } } } else { // This happens in case of DMs and GMs. @@ -2269,12 +2291,16 @@ func (a *App) GetPostInfo(c request.CTX, postID string) (*model.PostInfo, *model HasJoinedChannel: channelMemberErr == nil, } if team != nil { - _, teamMemberErr := a.GetTeamMember(c, team.Id, userID) + teamMember, teamMemberErr := a.GetTeamMember(c, team.Id, userID) + teamType := model.TeamInvite + if team.AllowOpenInvite { + teamType = model.TeamOpen + } info.TeamId = team.Id - info.TeamType = team.Type + info.TeamType = teamType info.TeamDisplayName = team.DisplayName - info.HasJoinedTeam = teamMemberErr == nil + info.HasJoinedTeam = teamMemberErr == nil && teamMember.DeleteAt == 0 } return &info, nil } diff --git a/server/channels/app/post_metadata.go b/server/channels/app/post_metadata.go index f5fb25e958..c67e3e6102 100644 --- a/server/channels/app/post_metadata.go +++ b/server/channels/app/post_metadata.go @@ -196,7 +196,19 @@ func (a *App) sanitizePostMetadataForUserAndChannel(c request.CTX, post *model.P } if previewedChannel != nil && !a.HasPermissionToReadChannel(c, userID, previewedChannel) { - post.Metadata.Embeds[0].Data = nil + // Remove all permalink embeds and only keep non-permalink embeds. + // We always have only one permalink embed even if the post + // contains multiple permalinks. + var newEmbeds []*model.PostEmbed + for _, embed := range post.Metadata.Embeds { + if embed.Type != model.PostEmbedPermalink { + newEmbeds = append(newEmbeds, embed) + } + } + + post.Metadata.Embeds = newEmbeds + + post.DelProp(model.PostPropsPreviewedPost) } return post @@ -229,6 +241,8 @@ func (a *App) SanitizePostMetadataForUser(c request.CTX, post *model.Post, userI } post.Metadata.Embeds = newEmbeds + + post.DelProp(model.PostPropsPreviewedPost) } return post, nil diff --git a/server/channels/app/post_metadata_test.go b/server/channels/app/post_metadata_test.go index 69c1f3707e..de80cdb23e 100644 --- a/server/channels/app/post_metadata_test.go +++ b/server/channels/app/post_metadata_test.go @@ -2820,7 +2820,7 @@ func TestSanitizePostMetadataForUserAndChannel(t *testing.T) { require.Nil(t, appErr) actual = th.App.sanitizePostMetadataForUserAndChannel(th.Context, post, previewedPost, directChannel, guest.Id) - assert.Nil(t, actual.Metadata.Embeds[0].Data) + assert.Len(t, actual.Metadata.Embeds, 0) }) t.Run("should not preview for archived channels", func(t *testing.T) { @@ -2882,7 +2882,7 @@ func TestSanitizePostMetadataForUserAndChannel(t *testing.T) { }) actual = th.App.sanitizePostMetadataForUserAndChannel(th.Context, post, previewedPost, publicChannel, th.BasicUser.Id) - assert.Nil(t, actual.Metadata.Embeds[0].Data) + assert.Len(t, actual.Metadata.Embeds, 0) }) } diff --git a/server/channels/app/post_test.go b/server/channels/app/post_test.go index e56c737327..554c7b5dd5 100644 --- a/server/channels/app/post_test.go +++ b/server/channels/app/post_test.go @@ -1001,51 +1001,49 @@ func TestCreatePost(t *testing.T) { directChannel, err := th.App.createDirectChannel(th.Context, user1.Id, user2.Id) require.Nil(t, err) - referencedPost := &model.Post{ - ChannelId: th.BasicChannel.Id, - Message: "hello world", - UserId: th.BasicUser.Id, - } - th.Context.Session().UserId = th.BasicUser.Id - referencedPost, err = th.App.CreatePost(th.Context, referencedPost, th.BasicChannel, false, false) - require.Nil(t, err) - - permalink := fmt.Sprintf("%s/%s/pl/%s", *th.App.Config().ServiceSettings.SiteURL, th.BasicTeam.Name, referencedPost.Id) - testCases := []struct { Description string Channel *model.Channel Author string - Assert func(t assert.TestingT, object any, msgAndArgs ...any) bool + Length int }{ { Description: "removes metadata from post for members who cannot read channel", Channel: directChannel, Author: user1.Id, - Assert: assert.Nil, + Length: 0, }, { Description: "does not remove metadata from post for members who can read channel", Channel: th.BasicChannel, Author: th.BasicUser.Id, - Assert: assert.NotNil, + Length: 1, }, } for _, testCase := range testCases { t.Run(testCase.Description, func(t *testing.T) { - previewPost := &model.Post{ + referencedPost := &model.Post{ ChannelId: testCase.Channel.Id, - Message: permalink, + Message: "hello world", UserId: testCase.Author, } - - previewPost, err = th.App.CreatePost(th.Context, previewPost, testCase.Channel, false, false) + referencedPost, err = th.App.CreatePost(th.Context, referencedPost, testCase.Channel, false, false) require.Nil(t, err) - testCase.Assert(t, previewPost.Metadata.Embeds[0].Data) + permalink := fmt.Sprintf("%s/%s/pl/%s", *th.App.Config().ServiceSettings.SiteURL, th.BasicTeam.Name, referencedPost.Id) + previewPost := &model.Post{ + ChannelId: th.BasicChannel.Id, + Message: permalink, + UserId: th.BasicUser.Id, + } + + previewPost, err = th.App.CreatePost(th.Context, previewPost, th.BasicChannel, false, false) + require.Nil(t, err) + + require.Len(t, previewPost.Metadata.Embeds, testCase.Length) }) } }) @@ -1451,73 +1449,6 @@ func TestUpdatePost(t *testing.T) { require.Nil(t, err) assert.Equal(t, testPost.GetProps(), model.StringInterface{"previewed_post": referencedPost.Id}) }) - - t.Run("sanitizes post metadata appropriately", func(t *testing.T) { - th := Setup(t).InitBasic() - defer th.TearDown() - - th.App.UpdateConfig(func(cfg *model.Config) { - *cfg.ServiceSettings.SiteURL = "http://mymattermost.com" - }) - - th.AddUserToChannel(th.BasicUser, th.BasicChannel) - - user1 := th.CreateUser() - user2 := th.CreateUser() - directChannel, err := th.App.createDirectChannel(th.Context, user1.Id, user2.Id) - require.Nil(t, err) - - referencedPost := &model.Post{ - ChannelId: th.BasicChannel.Id, - Message: "hello world", - UserId: th.BasicUser.Id, - } - - th.Context.Session().UserId = th.BasicUser.Id - - referencedPost, err = th.App.CreatePost(th.Context, referencedPost, th.BasicChannel, false, false) - require.Nil(t, err) - - permalink := fmt.Sprintf("%s/%s/pl/%s", *th.App.Config().ServiceSettings.SiteURL, th.BasicTeam.Name, referencedPost.Id) - - testCases := []struct { - Description string - Channel *model.Channel - Author string - Assert func(t assert.TestingT, object any, msgAndArgs ...any) bool - }{ - { - Description: "removes metadata from post for members who cannot read channel", - Channel: directChannel, - Author: user1.Id, - Assert: assert.Nil, - }, - { - Description: "does not remove metadata from post for members who can read channel", - Channel: th.BasicChannel, - Author: th.BasicUser.Id, - Assert: assert.NotNil, - }, - } - - for _, testCase := range testCases { - t.Run(testCase.Description, func(t *testing.T) { - previewPost := &model.Post{ - ChannelId: testCase.Channel.Id, - UserId: testCase.Author, - } - - previewPost, err = th.App.CreatePost(th.Context, previewPost, testCase.Channel, false, false) - require.Nil(t, err) - - previewPost.Message = permalink - previewPost, err = th.App.UpdatePost(th.Context, previewPost, false) - require.Nil(t, err) - - testCase.Assert(t, previewPost.Metadata.Embeds[0].Data) - }) - } - }) } func TestSearchPostsForUser(t *testing.T) { @@ -3043,26 +2974,64 @@ func TestGetPostIfAuthorized(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - privateChannel := th.CreatePrivateChannel(th.Context, th.BasicTeam) - post, err := th.App.CreatePost(th.Context, &model.Post{UserId: th.BasicUser.Id, ChannelId: privateChannel.Id, Message: "Hello"}, privateChannel, false, false) - require.Nil(t, err) - require.NotNil(t, post) + t.Run("Private channel", func(t *testing.T) { + privateChannel := th.CreatePrivateChannel(th.Context, th.BasicTeam) + post, err := th.App.CreatePost(th.Context, &model.Post{UserId: th.BasicUser.Id, ChannelId: privateChannel.Id, Message: "Hello"}, privateChannel, false, false) + require.Nil(t, err) + require.NotNil(t, post) - session1, err := th.App.CreateSession(th.Context, &model.Session{UserId: th.BasicUser.Id, Props: model.StringMap{}}) - require.Nil(t, err) - require.NotNil(t, session1) + session1, err := th.App.CreateSession(th.Context, &model.Session{UserId: th.BasicUser.Id, Props: model.StringMap{}}) + require.Nil(t, err) + require.NotNil(t, session1) - session2, err := th.App.CreateSession(th.Context, &model.Session{UserId: th.BasicUser2.Id, Props: model.StringMap{}}) - require.Nil(t, err) - require.NotNil(t, session2) + session2, err := th.App.CreateSession(th.Context, &model.Session{UserId: th.BasicUser2.Id, Props: model.StringMap{}}) + require.Nil(t, err) + require.NotNil(t, session2) - // User is not authorized to get post - _, err = th.App.GetPostIfAuthorized(th.Context, post.Id, session2, false) - require.NotNil(t, err) + // User is not authorized to get post + _, err = th.App.GetPostIfAuthorized(th.Context, post.Id, session2, false) + require.NotNil(t, err) - // User is authorized to get post - _, err = th.App.GetPostIfAuthorized(th.Context, post.Id, session1, false) - require.Nil(t, err) + // User is authorized to get post + _, err = th.App.GetPostIfAuthorized(th.Context, post.Id, session1, false) + require.Nil(t, err) + }) + + t.Run("Public channel", func(t *testing.T) { + publicChannel := th.CreateChannel(th.Context, th.BasicTeam) + post, err := th.App.CreatePost(th.Context, &model.Post{UserId: th.BasicUser.Id, ChannelId: publicChannel.Id, Message: "Hello"}, publicChannel, false, false) + require.Nil(t, err) + require.NotNil(t, post) + + session1, err := th.App.CreateSession(th.Context, &model.Session{UserId: th.BasicUser.Id, Props: model.StringMap{}}) + require.Nil(t, err) + require.NotNil(t, session1) + + session2, err := th.App.CreateSession(th.Context, &model.Session{UserId: th.BasicUser2.Id, Props: model.StringMap{}}) + require.Nil(t, err) + require.NotNil(t, session2) + + // User is authorized to get post + _, err = th.App.GetPostIfAuthorized(th.Context, post.Id, session2, false) + require.Nil(t, err) + + // User is authorized to get post + _, err = th.App.GetPostIfAuthorized(th.Context, post.Id, session1, false) + require.Nil(t, err) + + th.App.UpdateConfig(func(c *model.Config) { + b := true + c.ComplianceSettings.Enable = &b + }) + + // User is not authorized to get post + _, err = th.App.GetPostIfAuthorized(th.Context, post.Id, session2, false) + require.NotNil(t, err) + + // User is authorized to get post + _, err = th.App.GetPostIfAuthorized(th.Context, post.Id, session1, false) + require.Nil(t, err) + }) } func TestShouldNotRefollowOnOthersReply(t *testing.T) { diff --git a/webapp/channels/src/components/channel_layout/channel_identifier_router/actions.ts b/webapp/channels/src/components/channel_layout/channel_identifier_router/actions.ts index b826d9f4ee..08cc9ab3ef 100644 --- a/webapp/channels/src/components/channel_layout/channel_identifier_router/actions.ts +++ b/webapp/channels/src/components/channel_layout/channel_identifier_router/actions.ts @@ -190,7 +190,7 @@ export function goToChannelByChannelName(match: Match, history: History): Action const isSystemAdmin = UserUtils.isSystemAdmin(user?.roles); if (isSystemAdmin) { if (channel?.type === Constants.PRIVATE_CHANNEL) { - const joinPromptResult = await dispatch(joinPrivateChannelPrompt(teamObj, channel)); + const joinPromptResult = await dispatch(joinPrivateChannelPrompt(teamObj, channel.display_name)); if ('data' in joinPromptResult && !joinPromptResult.data.join) { return {data: undefined}; } diff --git a/webapp/channels/src/components/permalink_view/actions.ts b/webapp/channels/src/components/permalink_view/actions.ts index 0a94692bf8..bc1d107794 100644 --- a/webapp/channels/src/components/permalink_view/actions.ts +++ b/webapp/channels/src/components/permalink_view/actions.ts @@ -7,7 +7,8 @@ import type {Post} from '@mattermost/types/posts'; import {getChannel, getChannelMember, selectChannel, joinChannel, getChannelStats} from 'mattermost-redux/actions/channels'; import {getPostThread} from 'mattermost-redux/actions/posts'; import {getMissingProfilesByIds} from 'mattermost-redux/actions/users'; -import {getCurrentChannel} from 'mattermost-redux/selectors/entities/channels'; +import {Client4} from 'mattermost-redux/client'; +import {getCurrentChannel, getChannel as getChannelFromRedux} from 'mattermost-redux/selectors/entities/channels'; import {isCollapsedThreadsEnabled} from 'mattermost-redux/selectors/entities/preferences'; import {getCurrentTeam, getTeam} from 'mattermost-redux/selectors/entities/teams'; import {getCurrentUser} from 'mattermost-redux/selectors/entities/users'; @@ -88,25 +89,48 @@ export function focusPost(postId: string, returnTo = '', currentUserId: string, if (privateChannelJoinPromptVisible) { return; } - const {data} = await dispatch(getPostThread(postId)); - if (!data) { + let postInfo; + try { + postInfo = await Client4.getPostInfo(postId); + } catch (e) { getHistory().replace(`/error?type=${ErrorPageTypes.PERMALINK_NOT_FOUND}&returnTo=${returnTo}`); return; } - if (data.first_inaccessible_post_time) { + const state = getState(); + const currentTeam = getCurrentTeam(state); + + if (!postInfo.has_joined_channel) { + // Prompt system admin before joining the private channel + const user = getCurrentUser(state); + if (postInfo.channel_type === Constants.PRIVATE_CHANNEL && isSystemAdmin(user.roles)) { + privateChannelJoinPromptVisible = true; + const joinPromptResult = await dispatch(joinPrivateChannelPrompt(currentTeam, postInfo.channel_display_name)); + privateChannelJoinPromptVisible = false; + if ('data' in joinPromptResult && !joinPromptResult.data.join) { + return; + } + } + await dispatch(joinChannel(currentUserId, '', postInfo.channel_id)); + } + + const {data: threadData} = await dispatch(getPostThread(postId)); + + if (!threadData) { + getHistory().replace(`/error?type=${ErrorPageTypes.PERMALINK_NOT_FOUND}&returnTo=${returnTo}`); + return; + } + + if (threadData.first_inaccessible_post_time) { getHistory().replace(`/error?type=${ErrorPageTypes.CLOUD_ARCHIVED}&returnTo=${returnTo}`); return; } - const state = getState(); const isCollapsed = isCollapsedThreadsEnabled(state); - const channelId = data.posts[data.order[0]].channel_id; - let channel = state.entities.channels.channels[channelId]; - const currentTeam = getCurrentTeam(state); - const teamId = currentTeam.id; + const channelId = threadData.posts[threadData.order[0]].channel_id; + let channel = getChannelFromRedux(state, channelId); if (!channel) { const {data: channelData} = await dispatch(getChannel(channelId)); @@ -119,6 +143,7 @@ export function focusPost(postId: string, returnTo = '', currentUserId: string, channel = channelData; } + const teamId = channel.team_id || currentTeam.id; let myMember = state.entities.channels.myMembers[channelId]; if (!myMember) { @@ -134,17 +159,8 @@ export function focusPost(postId: string, returnTo = '', currentUserId: string, } if (!myMember) { - // Prompt system admin before joining the private channel - const user = getCurrentUser(state); - if (channel.type === Constants.PRIVATE_CHANNEL && isSystemAdmin(user.roles)) { - privateChannelJoinPromptVisible = true; - const joinPromptResult = await dispatch(joinPrivateChannelPrompt(currentTeam, channel)); - privateChannelJoinPromptVisible = false; - if ('data' in joinPromptResult && !joinPromptResult.data.join) { - return; - } - } - await dispatch(joinChannel(currentUserId, '', channelId, channel.name)); + getHistory().replace(`/error?type=${ErrorPageTypes.PERMALINK_NOT_FOUND}&returnTo=${returnTo}`); + return; } } @@ -161,7 +177,7 @@ export function focusPost(postId: string, returnTo = '', currentUserId: string, dispatch(loadNewGMIfNeeded(channel.id)); } - const post = data.posts[postId]; + const post = threadData.posts[postId]; if (isCollapsed && isComment(post)) { const {data} = await dispatch(focusReplyPost(post, channel, teamId, returnTo, option)); diff --git a/webapp/channels/src/components/permalink_view/permalink_view.test.tsx b/webapp/channels/src/components/permalink_view/permalink_view.test.tsx index 0f005ed6f1..1c36551d86 100644 --- a/webapp/channels/src/components/permalink_view/permalink_view.test.tsx +++ b/webapp/channels/src/components/permalink_view/permalink_view.test.tsx @@ -174,24 +174,46 @@ describe('components/PermalinkView', () => { }; describe('focusPost', () => { - test('should redirect to error page for DM channel not a member of', async () => { - const testStore = await mockStore(initialState); - await testStore.dispatch(focusPost('dmpostid1', undefined, baseProps.currentUserId)); + beforeEach(() => { + TestHelper.initBasic(Client4); + }); - expect(getPostThread).toHaveBeenCalledWith('dmpostid1'); + afterEach(() => { + TestHelper.tearDown(); + }); + + function nockInfoForPost(postId: string) { + nock(Client4.getPostRoute(postId)). + get('/info'). + reply(200, { + has_joined_channel: true, + }); + } + test('should redirect to error page for DM channel not a member of', async () => { + const postId = 'dmpostid1'; + TestHelper.initBasic(Client4); + nockInfoForPost(postId); + + const testStore = await mockStore(initialState); + await testStore.dispatch(focusPost(postId, undefined, baseProps.currentUserId)); + + expect(getPostThread).toHaveBeenCalledWith(postId); expect(testStore.getActions()).toEqual([ - {type: 'MOCK_GET_POST_THREAD', data: {posts: {dmpostid1: {id: 'dmpostid1', message: 'some message', channel_id: 'dmchannelid'}}, order: ['dmpostid1']}}, + {type: 'MOCK_GET_POST_THREAD', data: {posts: {dmpostid1: {id: postId, message: 'some message', channel_id: 'dmchannelid'}}, order: ['dmpostid1']}}, ]); expect(getHistory().replace).toHaveBeenCalledWith(`/error?type=${ErrorPageTypes.PERMALINK_NOT_FOUND}&returnTo=`); }); test('should redirect to error page for GM channel not a member of', async () => { - const testStore = await mockStore(initialState); - await testStore.dispatch(focusPost('gmpostid1', undefined, baseProps.currentUserId)); + const postId = 'gmpostid1'; + nockInfoForPost(postId); - expect(getPostThread).toHaveBeenCalledWith('gmpostid1'); + const testStore = await mockStore(initialState); + await testStore.dispatch(focusPost(postId, undefined, baseProps.currentUserId)); + + expect(getPostThread).toHaveBeenCalledWith(postId); expect(testStore.getActions()).toEqual([ - {type: 'MOCK_GET_POST_THREAD', data: {posts: {gmpostid1: {id: 'gmpostid1', message: 'some message', channel_id: 'gmchannelid'}}, order: ['gmpostid1']}}, + {type: 'MOCK_GET_POST_THREAD', data: {posts: {gmpostid1: {id: postId, message: 'some message', channel_id: 'gmchannelid'}}, order: ['gmpostid1']}}, ]); expect(getHistory().replace).toHaveBeenCalledWith(`/error?type=${ErrorPageTypes.PERMALINK_NOT_FOUND}&returnTo=`); }); @@ -199,7 +221,9 @@ describe('components/PermalinkView', () => { test('should redirect to DM link with postId for permalink', async () => { const dateNowOrig = Date.now; Date.now = () => new Date(0).getMilliseconds(); - const nextTick = () => new Promise((res) => process.nextTick(res)); + + const postId = 'dmpostid1'; + nockInfoForPost(postId); TestHelper.initBasic(Client4); nock(Client4.getUsersRoute()). @@ -220,13 +244,12 @@ describe('components/PermalinkView', () => { }; const testStore = mockStore(modifiedState); - testStore.dispatch(focusPost('dmpostid1', undefined, baseProps.currentUserId)); + await testStore.dispatch(focusPost(postId, undefined, baseProps.currentUserId)); - await nextTick(); expect.assertions(3); - expect(getPostThread).toHaveBeenCalledWith('dmpostid1'); + expect(getPostThread).toHaveBeenCalledWith(postId); expect(testStore.getActions()).toEqual([ - {type: 'MOCK_GET_POST_THREAD', data: {posts: {dmpostid1: {id: 'dmpostid1', message: 'some message', channel_id: 'dmchannelid'}}, order: ['dmpostid1']}}, + {type: 'MOCK_GET_POST_THREAD', data: {posts: {dmpostid1: {id: postId, message: 'some message', channel_id: 'dmchannelid'}}, order: [postId]}}, {type: 'MOCK_GET_MISSING_PROFILES', userIds: ['dmchannel']}, { type: 'RECEIVED_PREFERENCES', @@ -236,17 +259,19 @@ describe('components/PermalinkView', () => { ], }, {type: 'MOCK_SELECT_CHANNEL', args: ['dmchannelid']}, - {type: 'RECEIVED_FOCUSED_POST', channelId: 'dmchannelid', data: 'dmpostid1'}, + {type: 'RECEIVED_FOCUSED_POST', channelId: 'dmchannelid', data: postId}, {type: 'MOCK_LOAD_CHANNELS_FOR_CURRENT_USER'}, {type: 'MOCK_GET_CHANNEL_STATS', args: ['dmchannelid']}, ]); expect(getHistory().replace).toHaveBeenCalledWith('/currentteam/messages/@otherUser/dmpostid1'); Date.now = dateNowOrig; - TestHelper.tearDown(); }); test('should redirect to GM link with postId for permalink', async () => { + const postId = 'gmpostid1'; + nockInfoForPost(postId); + const modifiedState = { entities: { ...initialState.entities, @@ -261,13 +286,13 @@ describe('components/PermalinkView', () => { }; const testStore = await mockStore(modifiedState); - await testStore.dispatch(focusPost('gmpostid1', undefined, baseProps.currentUserId)); + await testStore.dispatch(focusPost(postId, undefined, baseProps.currentUserId)); - expect(getPostThread).toHaveBeenCalledWith('gmpostid1'); + expect(getPostThread).toHaveBeenCalledWith(postId); expect(testStore.getActions()).toEqual([ - {type: 'MOCK_GET_POST_THREAD', data: {posts: {gmpostid1: {id: 'gmpostid1', message: 'some message', channel_id: 'gmchannelid'}}, order: ['gmpostid1']}}, + {type: 'MOCK_GET_POST_THREAD', data: {posts: {gmpostid1: {id: postId, message: 'some message', channel_id: 'gmchannelid'}}, order: [postId]}}, {type: 'MOCK_SELECT_CHANNEL', args: ['gmchannelid']}, - {type: 'RECEIVED_FOCUSED_POST', channelId: 'gmchannelid', data: 'gmpostid1'}, + {type: 'RECEIVED_FOCUSED_POST', channelId: 'gmchannelid', data: postId}, {type: 'MOCK_LOAD_CHANNELS_FOR_CURRENT_USER'}, {type: 'MOCK_GET_CHANNEL_STATS', args: ['gmchannelid']}, ]); @@ -275,23 +300,26 @@ describe('components/PermalinkView', () => { }); test('should redirect to channel link with postId for permalink', async () => { - const testStore = await mockStore(initialState); - await testStore.dispatch(focusPost('postid1', undefined, baseProps.currentUserId)); + const postId = 'postid1'; + nockInfoForPost(postId); - expect(getPostThread).toHaveBeenCalledWith('postid1'); + const testStore = await mockStore(initialState); + await testStore.dispatch(focusPost(postId, undefined, baseProps.currentUserId)); + + expect(getPostThread).toHaveBeenCalledWith(postId); expect(testStore.getActions()).toEqual([ { type: 'MOCK_GET_POST_THREAD', data: { posts: { - replypostid1: {id: 'replypostid1', message: 'some message', channel_id: 'channelid1', root_id: 'postid1'}, - postid1: {id: 'postid1', message: 'some message', channel_id: 'channelid1'}, + replypostid1: {id: 'replypostid1', message: 'some message', channel_id: 'channelid1', root_id: postId}, + postid1: {id: postId, message: 'some message', channel_id: 'channelid1'}, }, - order: ['postid1', 'replypostid1'], + order: [postId, 'replypostid1'], }, }, {type: 'MOCK_SELECT_CHANNEL', args: ['channelid1']}, - {type: 'RECEIVED_FOCUSED_POST', channelId: 'channelid1', data: 'postid1'}, + {type: 'RECEIVED_FOCUSED_POST', channelId: 'channelid1', data: postId}, {type: 'MOCK_LOAD_CHANNELS_FOR_CURRENT_USER'}, {type: 'MOCK_GET_CHANNEL_STATS', args: ['channelid1']}, ]); @@ -299,6 +327,9 @@ describe('components/PermalinkView', () => { }); test('should not redirect to channel link with postId for a reply permalink when collapsedThreads enabled and option is set true', async () => { + const postId = 'replypostid1'; + nockInfoForPost(postId); + const newState = { entities: { ...initialState.entities, @@ -313,34 +344,34 @@ describe('components/PermalinkView', () => { jest.spyOn(Channels, 'getCurrentChannel').mockReturnValue({id: 'channelid1', name: 'channel1', type: 'O', team_id: 'current_team_id'}); const testStore = await mockStore(newState); - await testStore.dispatch(focusPost('replypostid1', '#', initialState.entities.users.currentUserId, {skipRedirectReplyPermalink: true})); + await testStore.dispatch(focusPost(postId, '#', initialState.entities.users.currentUserId, {skipRedirectReplyPermalink: true})); - expect(getPostThread).toHaveBeenCalledWith('replypostid1'); + expect(getPostThread).toHaveBeenCalledWith(postId); expect(testStore.getActions()).toEqual([ { type: 'MOCK_GET_POST_THREAD', data: { posts: { - replypostid1: {id: 'replypostid1', message: 'some message', channel_id: 'channelid1', root_id: 'postid1'}, + replypostid1: {id: postId, message: 'some message', channel_id: 'channelid1', root_id: 'postid1'}, postid1: {id: 'postid1', message: 'some message', channel_id: 'channelid1'}, }, - order: ['postid1', 'replypostid1'], + order: ['postid1', postId], }, }, { type: 'MOCK_GET_POST_THREAD', data: { posts: { - replypostid1: {id: 'replypostid1', message: 'some message', channel_id: 'channelid1', root_id: 'postid1'}, + replypostid1: {id: postId, message: 'some message', channel_id: 'channelid1', root_id: 'postid1'}, postid1: {id: 'postid1', message: 'some message', channel_id: 'channelid1'}, }, - order: ['postid1', 'replypostid1'], + order: ['postid1', postId], }, }, - {type: 'MOCK_SELECT_POST_AND_HIGHLIGHT', args: [{id: 'replypostid1', message: 'some message', channel_id: 'channelid1', root_id: 'postid1'}]}, + {type: 'MOCK_SELECT_POST_AND_HIGHLIGHT', args: [{id: postId, message: 'some message', channel_id: 'channelid1', root_id: 'postid1'}]}, {type: 'MOCK_LOAD_CHANNELS_FOR_CURRENT_USER'}, {type: 'MOCK_GET_CHANNEL_STATS', args: ['channelid1']}, ]); diff --git a/webapp/channels/src/components/team_controller/team_controller.tsx b/webapp/channels/src/components/team_controller/team_controller.tsx index 1a986a6126..d4bc59bf5f 100644 --- a/webapp/channels/src/components/team_controller/team_controller.tsx +++ b/webapp/channels/src/components/team_controller/team_controller.tsx @@ -206,6 +206,8 @@ function TeamController(props: Props) { return null; } + const teamLoaded = team?.name.toLowerCase() === teamNameParam?.toLowerCase(); + return ( ))} - + ); } diff --git a/webapp/channels/src/utils/channel_utils.tsx b/webapp/channels/src/utils/channel_utils.tsx index 7667b18852..3c59158b3d 100644 --- a/webapp/channels/src/utils/channel_utils.tsx +++ b/webapp/channels/src/utils/channel_utils.tsx @@ -71,14 +71,14 @@ type JoinPrivateChannelPromptResult = { }; }; -export function joinPrivateChannelPrompt(team: Team, channel: Channel, handleOnCancel = true): ActionFunc { +export function joinPrivateChannelPrompt(team: Team, channelDisplayName: string, handleOnCancel = true): ActionFunc { return async (dispatch: DispatchFunc, getState: GetStateFunc) => { const result: JoinPrivateChannelPromptResult = await new Promise((resolve) => { const modalData = { modalId: ModalIdentifiers.JOIN_CHANNEL_PROMPT, dialogType: JoinPrivateChannelModal, dialogProps: { - channelName: channel.display_name, + channelName: channelDisplayName, onJoin: () => { LocalStorageStore.setTeamIdJoinedOnLoad(null); resolve({ diff --git a/webapp/platform/client/src/client4.ts b/webapp/platform/client/src/client4.ts index c9b84a862f..05f1c31ec3 100644 --- a/webapp/platform/client/src/client4.ts +++ b/webapp/platform/client/src/client4.ts @@ -106,7 +106,7 @@ import type { MarketplaceApp, MarketplacePlugin, } from '@mattermost/types/marketplace'; -import {Post, PostList, PostSearchResults, OpenGraphMetadata, PostsUsageResponse, TeamsUsageResponse, PaginatedPostList, FilesUsageResponse, PostAcknowledgement, PostAnalytics} from '@mattermost/types/posts'; +import {Post, PostList, PostSearchResults, PostsUsageResponse, TeamsUsageResponse, PaginatedPostList, FilesUsageResponse, PostAcknowledgement, PostAnalytics, PostInfo} from '@mattermost/types/posts'; import {Draft} from '@mattermost/types/drafts'; import {Reaction} from '@mattermost/types/reactions'; import {Role} from '@mattermost/types/roles'; @@ -2160,6 +2160,13 @@ export default class Client4 { ); }; + getPostInfo = (postId: string) => { + return this.doFetch( + `${this.getPostRoute(postId)}/info`, + {method: 'get'}, + ) + } + getPostsByIds = (postIds: string[]) => { return this.doFetch( `${this.getPostsRoute()}/ids`, diff --git a/webapp/platform/types/src/posts.ts b/webapp/platform/types/src/posts.ts index eda29f4cc9..bde9ba9f92 100644 --- a/webapp/platform/types/src/posts.ts +++ b/webapp/platform/types/src/posts.ts @@ -5,6 +5,7 @@ import {Channel, ChannelType} from './channels'; import {CustomEmoji} from './emojis'; import {FileInfo} from './files'; import {Reaction} from './reactions'; +import { TeamType } from './teams'; import {UserProfile} from './users'; import { RelationOneToOne, @@ -208,4 +209,15 @@ export type ActivityEntry = { actorId: string[]; userIds: string[]; usernames: string[]; +} + +export type PostInfo = { + channel_id: string; + channel_type: ChannelType; + channel_display_name: string; + has_joined_channel: boolean; + team_id: string; + team_type: TeamType; + team_display_name: string; + has_joined_team: boolean; } \ No newline at end of file