From f626d9879963d372f8458539209b2adad9a51267 Mon Sep 17 00:00:00 2001 From: Christopher Poile Date: Thu, 29 Jan 2026 15:41:37 -0500 Subject: [PATCH] manual cherrypick: [MM-67130] Fix permalink preview permissions (#34909) (#35114) * manual cherrypick: [MM-67130] Fix permalink preview permissions (#34909) * remove permalink embeds when user loses access to orginating channel * remove posts & embeds on team_leave event; simplify preview index.ts * cleanup * remove dead code * more dead code elimination * linter --- .../src/actions/websocket_actions.jsx | 3 +- .../src/reducers/entities/posts.test.ts | 260 ++++++++++++++++++ .../src/reducers/entities/posts.ts | 69 +++-- 3 files changed, 315 insertions(+), 17 deletions(-) diff --git a/webapp/channels/src/actions/websocket_actions.jsx b/webapp/channels/src/actions/websocket_actions.jsx index 0641a4836f..37f06cba4f 100644 --- a/webapp/channels/src/actions/websocket_actions.jsx +++ b/webapp/channels/src/actions/websocket_actions.jsx @@ -918,7 +918,8 @@ export function handleLeaveTeamEvent(msg) { const currentUser = getCurrentUser(state); if (currentUser.id === msg.data.user_id) { - dispatch({type: TeamTypes.LEAVE_TEAM, data: {id: msg.data.team_id}}); + // Include channel IDs so reducers can clean up posts/embeds for those channels + dispatch({type: TeamTypes.LEAVE_TEAM, data: {id: msg.data.team_id, channelIds: channels}}); // if they are on the team being removed redirect them to default team if (getCurrentTeamId(state) === msg.data.team_id) { diff --git a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/posts.test.ts b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/posts.test.ts index e95822dbf9..54ec923f41 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/posts.test.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/posts.test.ts @@ -6,6 +6,7 @@ import type {Post, PostOrderBlock} from '@mattermost/types/posts'; import { ChannelTypes, PostTypes, + TeamTypes, ThreadTypes, CloudTypes, } from 'mattermost-redux/action_types'; @@ -538,6 +539,265 @@ describe('posts', () => { }); } + describe('when a channel is left (LEAVE_CHANNEL)', () => { + it('MM-67130 should remove permalink embed referencing the left channel', () => { + const state = deepFreeze({ + post1: { + id: 'post1', + channel_id: 'channel2', + metadata: { + embeds: [{ + type: 'permalink', + data: { + post_id: 'linked_post', + channel_id: 'channel1', + post: {id: 'linked_post', message: 'secret message'}, + }, + }], + }, + }, + post2: {id: 'post2', channel_id: 'channel2'}, + }); + + const nextState = reducers.handlePosts(state, { + type: ChannelTypes.LEAVE_CHANNEL, + data: { + id: 'channel1', + viewArchivedChannels: false, + }, + }); + + expect(nextState).not.toBe(state); + expect(nextState.post2).toBe(state.post2); + expect(nextState.post1.metadata.embeds).toHaveLength(0); + }); + + it('MM-67130 should not modify posts with embeds referencing other channels', () => { + const state = deepFreeze({ + post1: { + id: 'post1', + channel_id: 'channel2', + metadata: { + embeds: [{ + type: 'permalink', + data: { + post_id: 'linked_post', + channel_id: 'channel3', + post: {id: 'linked_post', message: 'other message'}, + }, + }], + }, + }, + }); + + const nextState = reducers.handlePosts(state, { + type: ChannelTypes.LEAVE_CHANNEL, + data: { + id: 'channel1', + viewArchivedChannels: false, + }, + }); + + expect(nextState).toBe(state); + }); + + it('MM-67130 should handle posts with multiple embeds, only removing affected permalinks', () => { + const state = deepFreeze({ + post1: { + id: 'post1', + channel_id: 'channel2', + metadata: { + embeds: [ + { + type: 'opengraph', + url: 'https://example.com', + }, + { + type: 'permalink', + data: { + post_id: 'linked_post1', + channel_id: 'channel1', + post: {id: 'linked_post1', message: 'secret'}, + }, + }, + { + type: 'permalink', + data: { + post_id: 'linked_post2', + channel_id: 'channel3', + post: {id: 'linked_post2', message: 'keep this'}, + }, + }, + ], + }, + }, + }); + + const nextState = reducers.handlePosts(state, { + type: ChannelTypes.LEAVE_CHANNEL, + data: { + id: 'channel1', + viewArchivedChannels: false, + }, + }); + + expect(nextState).not.toBe(state); + expect(nextState.post1.metadata.embeds).toHaveLength(2); + expect(nextState.post1.metadata.embeds[0]).toBe(state.post1.metadata.embeds[0]); // opengraph preserved + expect(nextState.post1.metadata.embeds[1]).toBe(state.post1.metadata.embeds[2]); // channel3 permalink preserved + }); + }); + + describe(`leaving a team (${TeamTypes.LEAVE_TEAM})`, () => { + it('MM-67130 should remove posts from channels in the left team', () => { + // Team A: channel_teamA (user stays here) + // Team B: channel_teamB (user leaves this team) + const state = deepFreeze({ + post1: {id: 'post1', channel_id: 'channel_teamA'}, + post2: {id: 'post2', channel_id: 'channel_teamB'}, + post3: {id: 'post3', channel_id: 'channel_teamB'}, + }); + + const nextState = reducers.handlePosts(state, { + type: TeamTypes.LEAVE_TEAM, + data: { + id: 'teamB', + channelIds: ['channel_teamB'], + }, + }); + + expect(nextState).not.toBe(state); + expect(nextState.post1).toBe(state.post1); + expect(nextState.post2).toBeUndefined(); + expect(nextState.post3).toBeUndefined(); + }); + + it('MM-67130 should remove permalink embeds referencing channels in the left team', () => { + // Scenario: User is on Team A and Team B + // - Post in Team A's channel has a permalink to a post in Team B's channel + // - When user leaves Team B, the permalink embed should be removed + const state = deepFreeze({ + post_in_teamA: { + id: 'post_in_teamA', + channel_id: 'channel_teamA', + metadata: { + embeds: [{ + type: 'permalink', + data: { + post_id: 'secret_post', + channel_id: 'channel_teamB', + post: {id: 'secret_post', message: 'secret message from Team B'}, + }, + }], + }, + }, + other_post: {id: 'other_post', channel_id: 'channel_teamA'}, + }); + + const nextState = reducers.handlePosts(state, { + type: TeamTypes.LEAVE_TEAM, + data: { + id: 'teamB', + channelIds: ['channel_teamB'], + }, + }); + + expect(nextState).not.toBe(state); + expect(nextState.other_post).toBe(state.other_post); + expect(nextState.post_in_teamA.metadata.embeds).toHaveLength(0); + }); + + it('MM-67130 should handle leaving team with multiple channels', () => { + // Team B has multiple channels, all should be cleaned up + const state = deepFreeze({ + post_in_teamA: { + id: 'post_in_teamA', + channel_id: 'channel_teamA', + metadata: { + embeds: [ + { + type: 'permalink', + data: { + post_id: 'post1', + channel_id: 'channel_teamB_1', + post: {id: 'post1', message: 'secret 1'}, + }, + }, + { + type: 'permalink', + data: { + post_id: 'post2', + channel_id: 'channel_teamB_2', + post: {id: 'post2', message: 'secret 2'}, + }, + }, + { + type: 'permalink', + data: { + post_id: 'post3', + channel_id: 'channel_teamA', + post: {id: 'post3', message: 'keep this'}, + }, + }, + ], + }, + }, + post_teamB_1: {id: 'post_teamB_1', channel_id: 'channel_teamB_1'}, + post_teamB_2: {id: 'post_teamB_2', channel_id: 'channel_teamB_2'}, + }); + + const nextState = reducers.handlePosts(state, { + type: TeamTypes.LEAVE_TEAM, + data: { + id: 'teamB', + channelIds: ['channel_teamB_1', 'channel_teamB_2'], + }, + }); + + expect(nextState).not.toBe(state); + + // Posts from Team B channels should be removed + expect(nextState.post_teamB_1).toBeUndefined(); + expect(nextState.post_teamB_2).toBeUndefined(); + + // Only the permalink to Team A's channel should remain + expect(nextState.post_in_teamA.metadata.embeds).toHaveLength(1); + expect(nextState.post_in_teamA.metadata.embeds[0].data.channel_id).toBe('channel_teamA'); + }); + + it('MM-67130 should handle LEAVE_TEAM with no channelIds (no-op)', () => { + const state = deepFreeze({ + post1: {id: 'post1', channel_id: 'channel1'}, + }); + + // channelIds not provided (e.g., from other dispatch sites) + const nextState = reducers.handlePosts(state, { + type: TeamTypes.LEAVE_TEAM, + data: { + id: 'teamB', + }, + }); + + expect(nextState).toBe(state); + }); + + it('MM-67130 should handle LEAVE_TEAM with empty channelIds array', () => { + const state = deepFreeze({ + post1: {id: 'post1', channel_id: 'channel1'}, + }); + + const nextState = reducers.handlePosts(state, { + type: TeamTypes.LEAVE_TEAM, + data: { + id: 'teamB', + channelIds: [], + }, + }); + + expect(nextState).toBe(state); + }); + }); + describe(`follow a post/thread (${ThreadTypes.FOLLOW_CHANGED_THREAD})`, () => { test.each([[true], [false]])('should set is_following to %s', (following) => { const state = deepFreeze({ diff --git a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/posts.ts b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/posts.ts index f4f2587a7c..8b5f876966 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/posts.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/posts.ts @@ -20,7 +20,7 @@ import type { } from '@mattermost/types/utilities'; import type {MMReduxAction} from 'mattermost-redux/action_types'; -import {ChannelTypes, PostTypes, UserTypes, ThreadTypes, CloudTypes} from 'mattermost-redux/action_types'; +import {ChannelTypes, PostTypes, UserTypes, ThreadTypes, CloudTypes, TeamTypes} from 'mattermost-redux/action_types'; import {Posts} from 'mattermost-redux/constants'; import {comparePosts, isPermalink, shouldUpdatePost} from 'mattermost-redux/utils/post_utils'; @@ -156,6 +156,52 @@ export function nextPostsReplies(state: {[x in Post['id']]: number} = {}, action } } +// Helper function to remove posts and permalink embeds for a set of channel IDs. +function removePostsAndEmbedsForChannels(state: IDMappedObjects, channelIds: Set): IDMappedObjects { + let postModified = false; + const nextState = {...state}; + + for (const post of Object.values(state)) { + // Remove posts from the channels + if (channelIds.has(post.channel_id)) { + Reflect.deleteProperty(nextState, post.id); + postModified = true; + continue; + } + + // Remove permalink embeds referencing those channels (matches server behavior) + if (post.metadata?.embeds?.length) { + const newEmbeds: PostEmbed[] = []; + let embedRemoved = false; + + for (const embed of post.metadata.embeds) { + if (embed.type === 'permalink' && embed.data && channelIds.has((embed.data as PostPreviewMetadata).channel_id)) { + embedRemoved = true; + } else { + newEmbeds.push(embed); + } + } + + if (embedRemoved) { + nextState[post.id] = { + ...nextState[post.id], + metadata: { + ...nextState[post.id].metadata, + embeds: newEmbeds, + }, + }; + postModified = true; + } + } + } + + if (!postModified) { + return state; + } + + return nextState; +} + export function handlePosts(state: IDMappedObjects = {}, action: MMReduxAction) { switch (action.type) { case PostTypes.RECEIVED_POST: @@ -288,24 +334,15 @@ export function handlePosts(state: IDMappedObjects = {}, action: MMReduxAc } const channelId = action.data.id; + return removePostsAndEmbedsForChannels(state, new Set([channelId])); + } - let postDeleted = false; - - // Remove any posts in the deleted channel - const nextState = {...state}; - for (const post of Object.values(state)) { - if (post.channel_id === channelId) { - Reflect.deleteProperty(nextState, post.id); - postDeleted = true; - } - } - - if (!postDeleted) { - // Nothing changed + case TeamTypes.LEAVE_TEAM: { + const channelIds: string[] = action.data.channelIds || []; + if (channelIds.length === 0) { return state; } - - return nextState; + return removePostsAndEmbedsForChannels(state, new Set(channelIds)); } case ThreadTypes.FOLLOW_CHANGED_THREAD: {