From a1b853d1dc47abc44acf6259ab7067ee95704d7f Mon Sep 17 00:00:00 2001 From: Doug Lauder Date: Mon, 20 Sep 2021 15:00:47 -0400 Subject: [PATCH] MM-38611 getSharedChannels: only return channels user is member of (#18417) * getSharedChannels: only return channels user is member of --- api4/shared_channel.go | 11 ++++ api4/shared_channel_test.go | 16 +++++- model/shared_channel.go | 1 + store/sqlstore/shared_channel_store.go | 5 ++ store/storetest/shared_channel_store.go | 73 +++++++++++++++++++++++-- 5 files changed, 100 insertions(+), 6 deletions(-) diff --git a/api4/shared_channel.go b/api4/shared_channel.go index a3ef245a98..b2fc386bce 100644 --- a/api4/shared_channel.go +++ b/api4/shared_channel.go @@ -27,10 +27,21 @@ func getSharedChannels(c *Context, w http.ResponseWriter, r *http.Request) { return } + // make sure user has access to the team. + if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), c.Params.TeamId, model.PermissionViewTeam) { + c.SetPermissionError(model.PermissionViewTeam) + return + } + opts := model.SharedChannelFilterOpts{ TeamId: c.Params.TeamId, } + // only return channels the user is a member of, unless they are a shared channels manager. + if !c.App.HasPermissionTo(c.AppContext.Session().UserId, model.PermissionManageSharedChannels) { + opts.MemberId = c.AppContext.Session().UserId + } + channels, appErr := c.App.GetSharedChannels(c.Params.Page, c.Params.PerPage, opts) if appErr != nil { c.Err = appErr diff --git a/api4/shared_channel_test.go b/api4/shared_channel_test.go index 63e0f4bf53..7aa65872fc 100644 --- a/api4/shared_channel_test.go +++ b/api4/shared_channel_test.go @@ -64,9 +64,21 @@ func TestGetAllSharedChannels(t *testing.T) { }) t.Run("get shared channels for invalid team", func(t *testing.T) { - channels, _, err := th.Client.GetAllSharedChannels(model.NewId(), 0, 100) + _, _, err := th.Client.GetAllSharedChannels(model.NewId(), 0, 100) + require.Error(t, err) + }) + + t.Run("get shared channels, user not member of team", func(t *testing.T) { + team := &model.Team{ + DisplayName: "tteam", + Name: GenerateTestTeamName(), + Type: model.TeamOpen, + } + team, _, err := th.SystemAdminClient.CreateTeam(team) require.NoError(t, err) - assert.Empty(t, channels) + + _, _, err = th.Client.GetAllSharedChannels(team.Id, 0, 100) + require.Error(t, err) }) } diff --git a/model/shared_channel.go b/model/shared_channel.go index 08a2929249..ed069b28f5 100644 --- a/model/shared_channel.go +++ b/model/shared_channel.go @@ -238,6 +238,7 @@ func (scf *SharedChannelAttachment) IsValid() *AppError { type SharedChannelFilterOpts struct { TeamId string CreatorId string + MemberId string ExcludeHome bool ExcludeRemote bool } diff --git a/store/sqlstore/shared_channel_store.go b/store/sqlstore/shared_channel_store.go index 7fcae61809..c14f45243f 100644 --- a/store/sqlstore/shared_channel_store.go +++ b/store/sqlstore/shared_channel_store.go @@ -216,6 +216,11 @@ func (s SqlSharedChannelStore) getSharedChannelsQuery(opts model.SharedChannelFi Select(selectStr). From("SharedChannels AS sc") + if opts.MemberId != "" { + query = query.Join("ChannelMembers AS cm ON cm.ChannelId = sc.ChannelId"). + Where(sq.Eq{"cm.UserId": opts.MemberId}) + } + if opts.TeamId != "" { query = query.Where(sq.Eq{"sc.TeamId": opts.TeamId}) } diff --git a/store/storetest/shared_channel_store.go b/store/storetest/shared_channel_store.go index ead3c51ffa..a79d0d8538 100644 --- a/store/storetest/shared_channel_store.go +++ b/store/storetest/shared_channel_store.go @@ -184,6 +184,8 @@ func testHasSharedChannel(t *testing.T, ss store.Store) { func testGetSharedChannels(t *testing.T, ss store.Store) { require.NoError(t, clearSharedChannels(ss)) + user, err := createTestUser(ss, "gary.goodspeed") + require.NoError(t, err) creator := model.NewId() team1 := model.NewId() @@ -203,7 +205,7 @@ func testGetSharedChannels(t *testing.T, ss store.Store) { } for i, sc := range data { - channel, err := createTestChannel(ss, "test_get2_"+strconv.Itoa(i)) + channel, err := createTestChannelWithUser(ss, "test_get2_"+strconv.Itoa(i), user) require.NoError(t, err) sc.ChannelId = channel.Id @@ -287,6 +289,41 @@ func testGetSharedChannels(t *testing.T, ss store.Store) { _, err = ss.SharedChannel().GetAll(0, -100, opts) require.Error(t, err) }) + + t.Run("Get shared channels for member", func(t *testing.T) { + opts := model.SharedChannelFilterOpts{ + TeamId: team1, + MemberId: user.Id, + } + + count, err := ss.SharedChannel().GetAllCount(opts) + require.NoError(t, err, "error getting shared channels count") + + remotes, err := ss.SharedChannel().GetAll(0, 100, opts) + require.NoError(t, err, "error getting shared channels") + + require.Equal(t, int(count), len(remotes)) + require.Len(t, remotes, 4, "should be 4 matching channels") + for _, sc := range remotes { + require.Equal(t, team1, sc.TeamId) + } + }) + + t.Run("Get shared channels for non-member", func(t *testing.T) { + opts := model.SharedChannelFilterOpts{ + TeamId: team1, + MemberId: model.NewId(), + } + + count, err := ss.SharedChannel().GetAllCount(opts) + require.NoError(t, err, "error getting shared channels count") + + remotes, err := ss.SharedChannel().GetAll(0, 100, opts) + require.NoError(t, err, "error getting shared channels") + + require.Equal(t, int(count), len(remotes)) + require.Len(t, remotes, 0, "should be 0 matching channels") + }) } func testUpdateSharedChannel(t *testing.T, ss store.Store) { @@ -661,7 +698,7 @@ func testHasRemote(t *testing.T, ss store.Store) { func testGetRemoteForUser(t *testing.T, ss store.Store) { // add remotes, and users to simulated shared channels. teamId := model.NewId() - channel, err := createSharedTestChannel(ss, "share_test_channel", true) + channel, err := createSharedTestChannel(ss, "share_test_channel", true, nil) require.NoError(t, err) remotes := []*model.RemoteCluster{ {RemoteId: model.NewId(), SiteURL: model.NewId(), CreatorId: model.NewId(), RemoteTeamId: teamId, Name: "Test_Remote_1"}, @@ -781,12 +818,25 @@ func testDeleteSharedChannelRemote(t *testing.T, ss store.Store) { }) } +func createTestUser(ss store.Store, username string) (*model.User, error) { + user := &model.User{ + Username: username, + Email: "gary@example.com", + } + return ss.User().Save(user) +} + func createTestChannel(ss store.Store, name string) (*model.Channel, error) { - channel, err := createSharedTestChannel(ss, name, false) + channel, err := createSharedTestChannel(ss, name, false, nil) return channel, err } -func createSharedTestChannel(ss store.Store, name string, shared bool) (*model.Channel, error) { +func createTestChannelWithUser(ss store.Store, name string, member *model.User) (*model.Channel, error) { + channel, err := createSharedTestChannel(ss, name, false, member) + return channel, err +} + +func createSharedTestChannel(ss store.Store, name string, shared bool, member *model.User) (*model.Channel, error) { channel := &model.Channel{ TeamId: model.NewId(), Type: model.ChannelTypeOpen, @@ -802,6 +852,21 @@ func createSharedTestChannel(ss store.Store, name string, shared bool) (*model.C return nil, err } + if member != nil { + newMember := &model.ChannelMember{ + ChannelId: channel.Id, + UserId: member.Id, + NotifyProps: model.GetDefaultChannelNotifyProps(), + SchemeGuest: member.IsGuest(), + SchemeUser: !member.IsGuest(), + } + + _, err = ss.Channel().SaveMember(newMember) + if err != nil { + return nil, err + } + } + if shared { sc := &model.SharedChannel{ ChannelId: channel.Id,