Prompt Team Admin before joining private channel (#29955)

* Prompt Team Admin before joining private channel

* fix and implement prompt when team admin joins via channel link

* update premission check and unit tests

* remove comment

* clean up some code

* update check to fix E2E tests

server/channels/api4/channel.go

@@ -1406,7 +1406,9 @@ func getChannelByName(c *Context, w http.ResponseWriter, r *http.Request) {
 			return
 		}
 	} else {
-		if !c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), channel.Id, model.PermissionReadChannel) {
+		// allows team admins to access private channel
+		if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), channel.TeamId, model.PermissionManageTeam) &&
+			!c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), channel.Id, model.PermissionReadChannel) {
 			c.Err = model.NewAppError("getChannelByName", "app.channel.get_by_name.missing.app_error", nil, "teamId="+channel.TeamId+", "+"name="+channel.Name+"", http.StatusNotFound)
 			return
 		}
@@ -1436,17 +1438,19 @@ func getChannelByNameForTeamName(c *Context, w http.ResponseWriter, r *http.Requ
 		return
 	}
 
-	teamOk := c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), channel.TeamId, model.PermissionReadPublicChannel)
 	channelOk := c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), channel.Id, model.PermissionReadChannel)
-
 	if channel.Type == model.ChannelTypeOpen {
+		teamOk := c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), channel.TeamId, model.PermissionReadPublicChannel)
 		if !teamOk && !channelOk {
 			c.SetPermissionError(model.PermissionReadPublicChannel)
 			return
 		}
 	} else if !channelOk {
-		c.Err = model.NewAppError("getChannelByNameForTeamName", "app.channel.get_by_name.missing.app_error", nil, "teamId="+channel.TeamId+", "+"name="+channel.Name+"", http.StatusNotFound)
+		// allows team admins to access private channel
-		return
+		if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), channel.TeamId, model.PermissionManageTeam) {
+			c.Err = model.NewAppError("getChannelByNameForTeamName", "app.channel.get_by_name.missing.app_error", nil, "teamId="+channel.TeamId+", "+"name="+channel.Name+"", http.StatusNotFound)
+			return
+		}
 	}
 
 	appErr = c.App.FillInChannelProps(c.AppContext, channel)

server/channels/api4/channel_test.go

@@ -2487,6 +2487,13 @@ func TestGetChannelByName(t *testing.T) {
 		_, _, err = client.GetChannelByName(context.Background(), th.BasicChannel.Name, th.BasicTeam.Id, "")
 		require.NoError(t, err)
 	})
+
+	th.SystemAdminClient.RemoveUserFromChannel(context.Background(), th.BasicPrivateChannel.Id, th.TeamAdminUser.Id)
+	TeamAdminClient := th.CreateClient()
+	th.LoginTeamAdminWithClient(TeamAdminClient)
+	channel, _, err = TeamAdminClient.GetChannelByName(context.Background(), th.BasicPrivateChannel.Name, th.BasicTeam.Id, "")
+	require.NoError(t, err)
+	require.Equal(t, th.BasicPrivateChannel.Name, channel.Name, "names did not match")
 }
 
 func TestGetChannelByNameForTeamName(t *testing.T) {
@@ -2498,7 +2505,14 @@ func TestGetChannelByNameForTeamName(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, th.BasicChannel.Name, channel.Name, "names did not match")
 
-	_, _, err = client.GetChannelByNameForTeamName(context.Background(), th.BasicChannel.Name, th.BasicTeam.Name, "")
+	th.SystemAdminClient.RemoveUserFromChannel(context.Background(), th.BasicPrivateChannel.Id, th.TeamAdminUser.Id)
+	TeamAdminClient := th.CreateClient()
+	th.LoginTeamAdminWithClient(TeamAdminClient)
+	channel, _, err = TeamAdminClient.GetChannelByNameForTeamName(context.Background(), th.BasicPrivateChannel.Name, th.BasicTeam.Name, "")
+	require.NoError(t, err)
+	require.Equal(t, th.BasicPrivateChannel.Name, channel.Name, "names did not match")
+
+	channel, _, err = client.GetChannelByNameForTeamName(context.Background(), th.BasicChannel.Name, th.BasicTeam.Name, "")
 	require.NoError(t, err)
 	require.Equal(t, th.BasicChannel.Name, channel.Name, "names did not match")
 

webapp/channels/src/components/channel_layout/channel_identifier_router/actions.test.ts

@@ -73,6 +73,11 @@ describe('Actions', () => {
                         name: 'team2',
                     },
                 },
+                myMembers: {
+                    team_id1: {
+                        scheme_user: true,
+                    },
+                },
             },
             users: {
                 currentUserId: 'current_user_id',
@@ -150,10 +155,21 @@ describe('Actions', () => {
             const testStore = await mockStore(initialState);
 
             await testStore.dispatch((goToChannelByChannelName({params: {team: 'team1', identifier: 'achannel3', path: '/'}, url: ''}, {} as any) as any));
-            expect(joinChannel).toHaveBeenCalledWith('current_user_id', 'team_id1', '', 'achannel3');
+            expect(joinChannel).toHaveBeenCalledWith('current_user_id', 'team_id1', 'channel_id3', 'achannel3');
             expect(emitChannelClickEvent).toHaveBeenCalledWith(channel3);
         });
 
+        test('switch to public channel we don\'t have locally and need to join', async () => {
+            const testStore = await mockStore(initialState);
+
+            const channel = {id: 'channel_id3a', name: 'achannel3a', team_id: 'team_id1', type: 'O'};
+            (joinChannel as jest.Mock).mockReturnValueOnce({type: '', data: {channel}});
+            (getChannelByNameAndTeamName as jest.Mock).mockReturnValueOnce({type: '', data: channel});
+            await testStore.dispatch((goToChannelByChannelName({params: {team: 'team1', identifier: channel.name, path: '/'}, url: ''}, {} as any) as any));
+            expect(joinChannel).toHaveBeenCalledWith('current_user_id', 'team_id1', 'channel_id3a', 'achannel3a');
+            expect(emitChannelClickEvent).toHaveBeenCalledWith(channel);
+        });
+
         test('switch to private channel we don\'t have locally and get prompted if super user and then join', async () => {
             const testStore = await mockStore({
                 ...initialState,
@@ -177,7 +193,49 @@ describe('Actions', () => {
             expect(getChannelByNameAndTeamName).toHaveBeenCalledWith('team1', channel.name, true);
             expect(getChannelMember).toHaveBeenCalledWith(channel.id, 'current_user_id');
             expect(joinPrivateChannelPrompt).toHaveBeenCalled();
-            expect(joinChannel).toHaveBeenCalledWith('current_user_id', 'team_id1', '', channel.name);
+            expect(joinChannel).toHaveBeenCalledWith('current_user_id', 'team_id1', channel.id, channel.name);
+        });
+
+        test('switch to private channel we don\'t have locally and get prompted if team Admin user and then join', async () => {
+            const testStore = await mockStore({
+                ...initialState,
+                entities: {
+                    ...initialState.entities,
+                    users: {
+                        ...initialState.entities.users,
+                        profiles: {
+                            ...initialState.entities.users.profiles,
+                            current_user_id: {
+                                roles: 'system_user',
+                            },
+                        },
+                    },
+                    channels: {
+                        ...initialState.entities.channels,
+                        myMembers: {
+                            privatechannelid: {channel_id: 'privatechannelid', user_id: 'current_user_id'},
+                        },
+                    },
+                    teams: {
+                        ...initialState.entities.teams,
+                        myMembers: {
+                            team_id1: {
+                                scheme_user: true,
+                                scheme_admin: true,
+                            },
+                        },
+                    },
+                },
+            });
+
+            const channel = {id: 'channel_id6', name: 'achannel6', team_id: 'team_id1', type: 'P'};
+            (joinChannel as jest.Mock).mockReturnValueOnce({type: '', data: {channel}});
+            (getChannelByNameAndTeamName as jest.Mock).mockReturnValueOnce({type: '', data: channel});
+            await testStore.dispatch((goToChannelByChannelName({params: {team: 'team1', identifier: channel.name, path: '/'}, url: ''}, {} as any) as any));
+            expect(getChannelByNameAndTeamName).toHaveBeenCalledWith('team1', channel.name, true);
+            expect(getChannelMember).toHaveBeenCalledWith(channel.id, 'current_user_id');
+            expect(joinPrivateChannelPrompt).toHaveBeenCalled();
+            expect(joinChannel).toHaveBeenCalledWith('current_user_id', 'team_id1', channel.id, channel.name);
         });
     });
 

webapp/channels/src/components/channel_layout/channel_identifier_router/actions.ts

@@ -10,7 +10,7 @@ import {joinChannel, getChannelByNameAndTeamName, getChannelMember, markGroupCha
 import {getUser, getUserByUsername, getUserByEmail} from 'mattermost-redux/actions/users';
 import {Client4} from 'mattermost-redux/client';
 import {getChannelByName, getOtherChannels, getChannel, getChannelsNameMapInTeam, getRedirectChannelNameForTeam} from 'mattermost-redux/selectors/entities/channels';
-import {getTeamByName} from 'mattermost-redux/selectors/entities/teams';
+import {getTeamByName, getMyTeamMember} from 'mattermost-redux/selectors/entities/teams';
 import {getCurrentUser, getCurrentUserId, getUserByUsername as selectUserByUsername, getUser as selectUser, getUserByEmail as selectUserByEmail} from 'mattermost-redux/selectors/entities/users';
 import * as UserUtils from 'mattermost-redux/utils/user_utils';
 
@@ -186,11 +186,18 @@ export function goToChannelByChannelName(match: Match, history: History): Action
         }
 
         if (!channel || !member) {
-            // Prompt system admin before joining the private channel
+            if (channel?.type === Constants.PRIVATE_CHANNEL) {
-            const user = getCurrentUser(getState());
+                // Prompt system admins and team admins before joining the private channel
-            const isSystemAdmin = UserUtils.isSystemAdmin(user?.roles);
+                const user = getCurrentUser(getState());
-            if (isSystemAdmin) {
+                const isSystemAdmin = UserUtils.isSystemAdmin(user?.roles);
-                if (channel?.type === Constants.PRIVATE_CHANNEL) {
+                let prompt = false;
+                if (isSystemAdmin) {
+                    prompt = true;
+                } else {
+                    const teamMember = getMyTeamMember(state, teamObj.id);
+                    prompt = Boolean(teamMember && teamMember.scheme_admin);
+                }
+                if (prompt) {
                     const joinPromptResult = await dispatch(joinPrivateChannelPrompt(teamObj, channel.display_name));
                     if ('data' in joinPromptResult && !joinPromptResult.data!.join) {
                         return {data: undefined};
@@ -198,7 +205,7 @@ export function goToChannelByChannelName(match: Match, history: History): Action
                 }
             }
 
-            const joinChannelDispatchResult = await dispatch(joinChannel(getCurrentUserId(state), teamObj!.id, '', channelName));
+            const joinChannelDispatchResult = await dispatch(joinChannel(getCurrentUserId(state), teamObj!.id, channel?.id || '', channelName));
             if ('error' in joinChannelDispatchResult) {
                 if (!channel) {
                     const getChannelDispatchResult = await dispatch(getChannelByNameAndTeamName(team, channelName, true));

webapp/channels/src/components/permalink_view/actions.ts

@@ -10,7 +10,7 @@ import {getMissingProfilesByIds} from 'mattermost-redux/actions/users';
 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 {getCurrentTeam, getTeam, getMyTeamMember} from 'mattermost-redux/selectors/entities/teams';
 import {getCurrentUser} from 'mattermost-redux/selectors/entities/users';
 import {getUserIdFromChannelName} from 'mattermost-redux/utils/channel_utils';
 import {isSystemAdmin} from 'mattermost-redux/utils/user_utils';
@@ -104,9 +104,16 @@ export function focusPost(postId: string, returnTo = '', currentUserId: string,
         }
 
         if (!postInfo.has_joined_channel) {
-            // Prompt system admin before joining the private channel
+            // Prompt system admins and team admins before joining the private channel
             const user = getCurrentUser(state);
+            let prompt = false;
             if (postInfo.channel_type === Constants.PRIVATE_CHANNEL && isSystemAdmin(user.roles)) {
+                prompt = true;
+            } else {
+                const teamMember = getMyTeamMember(state, currentTeam.id);
+                prompt = Boolean(teamMember && teamMember.scheme_admin);
+            }
+            if (prompt) {
                 privateChannelJoinPromptVisible = true;
                 const joinPromptResult = await dispatch(joinPrivateChannelPrompt(currentTeam, postInfo.channel_display_name));
                 privateChannelJoinPromptVisible = false;

webapp/channels/src/components/permalink_view/permalink_view.test.tsx

@@ -44,6 +44,7 @@ jest.mock('mattermost-redux/actions/posts', () => ({
         const replyPost1 = {id: 'replypostid1', message: 'some message', channel_id: 'channelid1', root_id: 'postid1'};
         const dmPost = {id: 'dmpostid1', message: 'some message', channel_id: 'dmchannelid'};
         const gmPost = {id: 'gmpostid1', message: 'some message', channel_id: 'gmchannelid'};
+        const privatePost = {id: 'privatepostid1', message: 'some message', channel_id: 'privatechannelid'};
 
         switch (postId) {
         case 'postid1':
@@ -56,6 +57,8 @@ jest.mock('mattermost-redux/actions/posts', () => ({
             return {type: 'MOCK_GET_POST_THREAD', data: {posts: {gmpostid1: gmPost}, order: [gmPost.id]}};
         case 'replypostid1':
             return {type: 'MOCK_GET_POST_THREAD', data: {posts: {replypostid1: replyPost1, postid1: post}, order: [post.id, replyPost1.id]}};
+        case 'privatepostid1':
+            return {type: 'MOCK_GET_POST_THREAD', data: {posts: {privatepostid1: privatePost}, order: [privatePost.id]}};
         default:
             return {type: 'MOCK_GET_POST_THREAD'};
         }
@@ -80,6 +83,14 @@ jest.mock('mattermost-redux/actions/channels', () => ({
     }),
 }));
 
+jest.mock('utils/channel_utils', () => ({
+    joinPrivateChannelPrompt: jest.fn(() => {
+        return async () => {
+            return {data: {join: false}};
+        };
+    }),
+}));
+
 describe('components/PermalinkView', () => {
     const baseProps: ComponentProps<typeof PermalinkView> = {
         channelId: 'channel_id',
@@ -154,6 +165,7 @@ describe('components/PermalinkView', () => {
                 channels: {
                     channels: {
                         channelid1: TestHelper.getChannelMock({id: 'channelid1', name: 'channel1', type: 'O', team_id: 'current_team_id'}),
+                        privatechannelid: TestHelper.getChannelMock({id: 'privatechannelid', name: 'private_channel', type: 'P', team_id: 'current_team_id'}),
                         dmchannelid: TestHelper.getChannelMock({id: 'dmchannelid', name: 'dmchannel__current_user_id', type: 'D', team_id: ''}),
                         gmchannelid: TestHelper.getChannelMock({id: 'gmchannelid', name: 'gmchannel', type: 'G', team_id: ''}),
                     },
@@ -379,6 +391,147 @@ describe('components/PermalinkView', () => {
                 ]);
                 expect(getHistory().replace).not.toBeCalled();
             });
+
+            describe('focusPost - with prompt', () => {
+                function nockInfoForPrivatePost(postId: string) {
+                    nock(Client4.getPostRoute(postId)).
+                        get('/info').
+                        reply(200, {
+                            channel_type: 'P',
+                            has_joined_channel: false,
+                        });
+                }
+                test('should prompt admin user before redirect to private channel link', async () => {
+                    const testState = {
+                        ...initialState,
+                        entities: {
+                            ...initialState.entities,
+                            users: {
+                                ...initialState.entities.users,
+                                profiles: {
+                                    ...initialState.entities.users.profiles,
+                                    current_user_id: {
+                                        roles: 'system_admin',
+                                    },
+                                },
+                            },
+                        },
+                    };
+
+                    const postId = 'privatepostid1';
+                    nockInfoForPrivatePost(postId);
+
+                    const testStore = await mockStore(testState);
+                    await testStore.dispatch(focusPost(postId, undefined, baseProps.currentUserId));
+
+                    expect(getPostThread).not.toHaveBeenCalled();
+                    expect(testStore.getActions()).toEqual([]);
+                });
+
+                test('should prompt team admin before redirect to private channel link', async () => {
+                    const testState = {
+                        ...initialState,
+                        entities: {
+                            ...initialState.entities,
+                            users: {
+                                ...initialState.entities.users,
+                                profiles: {
+                                    ...initialState.entities.users.profiles,
+                                    current_user_id: {
+                                        roles: 'system_user',
+                                    },
+                                },
+                            },
+                            teams: {
+                                ...initialState.entities.teams,
+                                myMembers: {
+                                    current_team_id: {
+                                        scheme_user: true,
+                                        scheme_admin: true,
+                                    },
+                                },
+                            },
+                        },
+                    };
+                    const postId = 'privatepostid1';
+                    nockInfoForPrivatePost(postId);
+                    const testStore = await mockStore(testState);
+                    await testStore.dispatch(focusPost(postId, undefined, baseProps.currentUserId));
+                    expect(getPostThread).not.toHaveBeenCalled();
+                    expect(testStore.getActions()).toEqual([]);
+                });
+
+                test('should allow redirect to private channel link if prompt response true', async () => {
+                    const testState = {
+                        ...initialState,
+                        entities: {
+                            ...initialState.entities,
+                            users: {
+                                ...initialState.entities.users,
+                                profiles: {
+                                    ...initialState.entities.users.profiles,
+                                    current_user_id: {
+                                        roles: 'system_user',
+                                    },
+                                },
+                            },
+                            channels: {
+                                ...initialState.entities.channels,
+                                myMembers: {
+                                    privatechannelid: {channel_id: 'privatechannelid', user_id: 'current_user_id'},
+                                },
+                            },
+                            teams: {
+                                ...initialState.entities.teams,
+                                myMembers: {
+                                    current_team_id: {
+                                        scheme_user: true,
+                                    },
+                                },
+                            },
+                        },
+                    };
+
+                    jest.mock('utils/channel_utils', () => ({
+                        joinPrivateChannelPrompt: jest.fn(() => {
+                            return async () => {
+                                return {data: {join: true}};
+                            };
+                        }),
+                    }));
+
+                    const postId = 'privatepostid1';
+                    nockInfoForPrivatePost(postId);
+
+                    const testStore = await mockStore(testState);
+                    await testStore.dispatch(focusPost(postId, undefined, baseProps.currentUserId));
+
+                    expect(getPostThread).toHaveBeenCalledWith(postId);
+                    expect(testStore.getActions()).toEqual([
+                        {
+                            type: 'MOCK_JOIN_CHANNEL',
+                            args: [
+                                'current_user',
+                                '',
+                                undefined,
+                            ],
+                        },
+                        {
+                            type: 'MOCK_GET_POST_THREAD',
+                            data: {
+                                posts: {
+                                    privatepostid1: {id: 'privatepostid1', message: 'some message', channel_id: 'privatechannelid'},
+                                },
+                                order: ['privatepostid1'],
+                            },
+                        },
+                        {type: 'MOCK_SELECT_CHANNEL', args: ['privatechannelid']},
+                        {type: 'RECEIVED_FOCUSED_POST', channelId: 'privatechannelid', data: postId},
+                        {type: 'MOCK_LOAD_CHANNELS_FOR_CURRENT_USER'},
+                        {type: 'MOCK_GET_CHANNEL_STATS', args: ['privatechannelid']},
+                    ]);
+                });
+            });
         });
     });
 });