diff --git a/server/channels/api4/channel.go b/server/channels/api4/channel.go index b85b67647f..c7c0806995 100644 --- a/server/channels/api4/channel.go +++ b/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) - return + // allows team admins to access private channel + 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) diff --git a/server/channels/api4/channel_test.go b/server/channels/api4/channel_test.go index 302cf1ed3f..4b4987fa3a 100644 --- a/server/channels/api4/channel_test.go +++ b/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") diff --git a/webapp/channels/src/components/channel_layout/channel_identifier_router/actions.test.ts b/webapp/channels/src/components/channel_layout/channel_identifier_router/actions.test.ts index 939c8fb083..4307a8f019 100644 --- a/webapp/channels/src/components/channel_layout/channel_identifier_router/actions.test.ts +++ b/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); }); }); 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 2e6696e2c9..0925f67201 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 @@ -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 - const user = getCurrentUser(getState()); - const isSystemAdmin = UserUtils.isSystemAdmin(user?.roles); - if (isSystemAdmin) { - if (channel?.type === Constants.PRIVATE_CHANNEL) { + if (channel?.type === Constants.PRIVATE_CHANNEL) { + // Prompt system admins and team admins before joining the private channel + const user = getCurrentUser(getState()); + const isSystemAdmin = UserUtils.isSystemAdmin(user?.roles); + 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)); diff --git a/webapp/channels/src/components/permalink_view/actions.ts b/webapp/channels/src/components/permalink_view/actions.ts index c6d9deee27..79eb325d45 100644 --- a/webapp/channels/src/components/permalink_view/actions.ts +++ b/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; 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 394178abce..2b79fcb4d8 100644 --- a/webapp/channels/src/components/permalink_view/permalink_view.test.tsx +++ b/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 = { 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']}, + ]); + }); + }); }); }); });