diff --git a/api4/channel.go b/api4/channel.go index c4c5da5a37..46ac40de74 100644 --- a/api4/channel.go +++ b/api4/channel.go @@ -746,12 +746,20 @@ func searchChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Request) { return } - if !c.App.SessionHasPermissionToTeam(c.App.Session, c.Params.TeamId, model.PERMISSION_LIST_TEAM_CHANNELS) { - c.SetPermissionError(model.PERMISSION_LIST_TEAM_CHANNELS) - return + var channels *model.ChannelList + var err *model.AppError + if c.App.SessionHasPermissionToTeam(c.App.Session, c.Params.TeamId, model.PERMISSION_LIST_TEAM_CHANNELS) { + channels, err = c.App.SearchChannels(c.Params.TeamId, props.Term) + } else { + // If the user is not a team member, return a 404 + if _, err = c.App.GetTeamMember(c.Params.TeamId, c.App.Session.UserId); err != nil { + c.Err = err + return + } + + channels, err = c.App.SearchChannelsForUser(c.App.Session.UserId, c.Params.TeamId, props.Term) } - channels, err := c.App.SearchChannels(c.Params.TeamId, props.Term) if err != nil { c.Err = err return diff --git a/api4/channel_test.go b/api4/channel_test.go index 61f46817cf..1f6933bd23 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -909,13 +909,48 @@ func TestSearchChannels(t *testing.T) { search.Term = th.BasicChannel.Name _, resp = Client.SearchChannels(model.NewId(), search) - CheckForbiddenStatus(t, resp) + CheckNotFoundStatus(t, resp) _, resp = Client.SearchChannels("junk", search) CheckBadRequestStatus(t, resp) _, resp = th.SystemAdminClient.SearchChannels(th.BasicTeam.Id, search) CheckNoError(t, resp) + + // Check the appropriate permissions are enforced. + defaultRolePermissions := th.SaveDefaultRolePermissions() + defer func() { + th.RestoreDefaultRolePermissions(defaultRolePermissions) + }() + + // Remove list channels permission from the user + th.RemovePermissionFromRole(model.PERMISSION_LIST_TEAM_CHANNELS.Id, model.TEAM_USER_ROLE_ID) + + t.Run("Search for a BasicChannel, which the user is a member of", func(t *testing.T) { + search.Term = th.BasicChannel.Name + channelList, resp := Client.SearchChannels(th.BasicTeam.Id, search) + CheckNoError(t, resp) + + channelNames := []string{} + for _, c := range channelList { + channelNames = append(channelNames, c.Name) + } + require.Contains(t, channelNames, th.BasicChannel.Name) + }) + + t.Run("Remove the user from BasicChannel and search again, should not be returned", func(t *testing.T) { + th.App.RemoveUserFromChannel(th.BasicUser.Id, th.BasicUser.Id, th.BasicChannel) + + search.Term = th.BasicChannel.Name + channelList, resp := Client.SearchChannels(th.BasicTeam.Id, search) + CheckNoError(t, resp) + + channelNames := []string{} + for _, c := range channelList { + channelNames = append(channelNames, c.Name) + } + require.NotContains(t, channelNames, th.BasicChannel.Name) + }) } func TestSearchAllChannels(t *testing.T) { diff --git a/app/channel.go b/app/channel.go index b5ccfab098..7e4956d741 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1823,6 +1823,14 @@ func (a *App) SearchChannels(teamId string, term string) (*model.ChannelList, *m return a.Srv.Store.Channel().SearchInTeam(teamId, term, includeDeleted) } +func (a *App) SearchChannelsForUser(userId, teamId, term string) (*model.ChannelList, *model.AppError) { + includeDeleted := *a.Config().TeamSettings.ExperimentalViewArchivedChannels + + term = strings.TrimSpace(term) + + return a.Srv.Store.Channel().SearchForUserInTeam(userId, teamId, term, includeDeleted) +} + func (a *App) SearchGroupChannels(userId, term string) (*model.ChannelList, *model.AppError) { if term == "" { return &model.ChannelList{}, nil diff --git a/app/channel_test.go b/app/channel_test.go index 7414eb05fa..99eed39727 100644 --- a/app/channel_test.go +++ b/app/channel_test.go @@ -988,3 +988,56 @@ func TestDefaultChannelNames(t *testing.T) { expect = []string{"town-square", "foo", "bar"} require.ElementsMatch(t, expect, actual) } + +func TestSearchChannelsForUser(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + c1, err := th.App.CreateChannel(&model.Channel{DisplayName: "test-dev-1", Name: "test-dev-1", Type: model.CHANNEL_OPEN, TeamId: th.BasicTeam.Id}, false) + require.Nil(t, err) + + c2, err := th.App.CreateChannel(&model.Channel{DisplayName: "test-dev-2", Name: "test-dev-2", Type: model.CHANNEL_OPEN, TeamId: th.BasicTeam.Id}, false) + require.Nil(t, err) + + c3, err := th.App.CreateChannel(&model.Channel{DisplayName: "dev-3", Name: "dev-3", Type: model.CHANNEL_OPEN, TeamId: th.BasicTeam.Id}, false) + require.Nil(t, err) + + defer func() { + th.App.PermanentDeleteChannel(c1) + th.App.PermanentDeleteChannel(c2) + th.App.PermanentDeleteChannel(c3) + }() + + // add user to test-dev-1 and dev3 + _, err = th.App.AddUserToChannel(th.BasicUser, c1) + require.Nil(t, err) + _, err = th.App.AddUserToChannel(th.BasicUser, c3) + require.Nil(t, err) + + searchAndCheck := func(t *testing.T, term string, expectedDisplayNames []string) { + res, searchErr := th.App.SearchChannelsForUser(th.BasicUser.Id, th.BasicTeam.Id, term) + require.Nil(t, searchErr) + require.Len(t, *res, len(expectedDisplayNames)) + + resultDisplayNames := []string{} + for _, c := range *res { + resultDisplayNames = append(resultDisplayNames, c.Name) + } + require.ElementsMatch(t, expectedDisplayNames, resultDisplayNames) + } + + t.Run("Search for test, only test-dev-1 should be returned", func(t *testing.T) { + searchAndCheck(t, "test", []string{"test-dev-1"}) + }) + + t.Run("Search for dev, both test-dev-1 and dev-3 should be returned", func(t *testing.T) { + searchAndCheck(t, "dev", []string{"test-dev-1", "dev-3"}) + }) + + t.Run("After adding user to test-dev-2, search for dev, the three channels should be returned", func(t *testing.T) { + _, err = th.App.AddUserToChannel(th.BasicUser, c2) + require.Nil(t, err) + + searchAndCheck(t, "dev", []string{"test-dev-1", "test-dev-2", "dev-3"}) + }) +} diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index e6ac025d16..e8bb20be58 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -2121,6 +2121,35 @@ func (s SqlChannelStore) SearchInTeam(teamId string, term string, includeDeleted }) } +func (s SqlChannelStore) SearchForUserInTeam(userId string, teamId string, term string, includeDeleted bool) (*model.ChannelList, *model.AppError) { + deleteFilter := "AND c.DeleteAt = 0" + if includeDeleted { + deleteFilter = "" + } + + return s.performSearch(` + SELECT + Channels.* + FROM + Channels + JOIN + PublicChannels c ON (c.Id = Channels.Id) + JOIN + ChannelMembers cm ON (c.Id = cm.ChannelId) + WHERE + c.TeamId = :TeamId + AND + cm.UserId = :UserId + `+deleteFilter+` + SEARCH_CLAUSE + ORDER BY c.DisplayName + LIMIT 100 + `, term, map[string]interface{}{ + "TeamId": teamId, + "UserId": userId, + }) +} + func (s SqlChannelStore) SearchAllChannels(term string, opts store.ChannelSearchOpts) (*model.ChannelListWithTeamData, *model.AppError) { query := s.getQueryBuilder(). Select("c.*, t.DisplayName AS TeamDisplayName, t.Name AS TeamName, t.UpdateAt as TeamUpdateAt"). diff --git a/store/store.go b/store/store.go index 1e68eb6b09..de16f15853 100644 --- a/store/store.go +++ b/store/store.go @@ -188,6 +188,7 @@ type ChannelStore interface { AutocompleteInTeamForSearch(teamId string, userId string, term string, includeDeleted bool) (*model.ChannelList, *model.AppError) SearchAllChannels(term string, opts ChannelSearchOpts) (*model.ChannelListWithTeamData, *model.AppError) SearchInTeam(teamId string, term string, includeDeleted bool) (*model.ChannelList, *model.AppError) + SearchForUserInTeam(userId string, teamId string, term string, includeDeleted bool) (*model.ChannelList, *model.AppError) SearchMore(userId string, teamId string, term string) (*model.ChannelList, *model.AppError) SearchGroupChannels(userId, term string) (*model.ChannelList, *model.AppError) GetMembersByIds(channelId string, userIds []string) (*model.ChannelMembers, *model.AppError) diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index f7761026ae..891046f6dc 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -68,6 +68,7 @@ func TestChannelStore(t *testing.T, ss store.Store, s SqlSupplier) { t.Run("GetGuestCount", func(t *testing.T) { testGetGuestCount(t, ss) }) t.Run("SearchMore", func(t *testing.T) { testChannelStoreSearchMore(t, ss) }) t.Run("SearchInTeam", func(t *testing.T) { testChannelStoreSearchInTeam(t, ss) }) + t.Run("SearchForUserInTeam", func(t *testing.T) { testChannelStoreSearchForUserInTeam(t, ss) }) t.Run("SearchAllChannels", func(t *testing.T) { testChannelStoreSearchAllChannels(t, ss) }) t.Run("AutocompleteInTeamForSearch", func(t *testing.T) { testChannelStoreAutocompleteInTeamForSearch(t, ss, s) }) t.Run("GetMembersByIds", func(t *testing.T) { testChannelStoreGetMembersByIds(t, ss) }) @@ -2568,6 +2569,112 @@ func testChannelStoreSearchInTeam(t *testing.T, ss store.Store) { } } +func testChannelStoreSearchForUserInTeam(t *testing.T, ss store.Store) { + userId := model.NewId() + teamId := model.NewId() + otherTeamId := model.NewId() + + // create 4 channels for the same team and one for other team + o1 := model.Channel{ + TeamId: teamId, + DisplayName: "test-dev-1", + Name: "zz" + model.NewId() + "b", + Type: model.CHANNEL_OPEN, + } + _, err := ss.Channel().Save(&o1, -1) + require.Nil(t, err) + + o2 := model.Channel{ + TeamId: teamId, + DisplayName: "test-dev-2", + Name: "zz" + model.NewId() + "b", + Type: model.CHANNEL_OPEN, + } + _, err = ss.Channel().Save(&o2, -1) + require.Nil(t, err) + + o3 := model.Channel{ + TeamId: teamId, + DisplayName: "dev-3", + Name: "zz" + model.NewId() + "b", + Type: model.CHANNEL_OPEN, + } + _, err = ss.Channel().Save(&o3, -1) + require.Nil(t, err) + + o4 := model.Channel{ + TeamId: teamId, + DisplayName: "dev-4", + Name: "zz" + model.NewId() + "b", + Type: model.CHANNEL_OPEN, + } + _, err = ss.Channel().Save(&o4, -1) + require.Nil(t, err) + + o5 := model.Channel{ + TeamId: otherTeamId, + DisplayName: "other-team-dev-5", + Name: "zz" + model.NewId() + "b", + Type: model.CHANNEL_OPEN, + } + _, err = ss.Channel().Save(&o5, -1) + require.Nil(t, err) + + // add the user to the first 3 channels and the other team channel + for _, c := range []model.Channel{o1, o2, o3, o5} { + _, err = ss.Channel().SaveMember(&model.ChannelMember{ + ChannelId: c.Id, + UserId: userId, + NotifyProps: model.GetDefaultChannelNotifyProps(), + }) + require.Nil(t, err) + } + + searchAndCheck := func(t *testing.T, term string, includeDeleted bool, expectedDisplayNames []string) { + res, searchErr := ss.Channel().SearchForUserInTeam(userId, teamId, term, includeDeleted) + require.Nil(t, searchErr) + require.Len(t, *res, len(expectedDisplayNames)) + + resultDisplayNames := []string{} + for _, c := range *res { + resultDisplayNames = append(resultDisplayNames, c.DisplayName) + } + require.ElementsMatch(t, expectedDisplayNames, resultDisplayNames) + } + + t.Run("Search for test, get channels 1 and 2", func(t *testing.T) { + searchAndCheck(t, "test", false, []string{o1.DisplayName, o2.DisplayName}) + }) + + t.Run("Search for dev, get channels 1, 2 and 3", func(t *testing.T) { + searchAndCheck(t, "dev", false, []string{o1.DisplayName, o2.DisplayName, o3.DisplayName}) + }) + + t.Run("After adding user to channel 4, search for dev, get channels 1, 2, 3 and 4", func(t *testing.T) { + _, err = ss.Channel().SaveMember(&model.ChannelMember{ + ChannelId: o4.Id, + UserId: userId, + NotifyProps: model.GetDefaultChannelNotifyProps(), + }) + require.Nil(t, err) + + searchAndCheck(t, "dev", false, []string{o1.DisplayName, o2.DisplayName, o3.DisplayName, o4.DisplayName}) + }) + + t.Run("Mark channel 1 as deleted, search for dev, get channels 2, 3 and 4", func(t *testing.T) { + o1.DeleteAt = model.GetMillis() + o1.UpdateAt = o1.DeleteAt + err = ss.Channel().Delete(o1.Id, o1.DeleteAt) + require.Nil(t, err) + + searchAndCheck(t, "dev", false, []string{o2.DisplayName, o3.DisplayName, o4.DisplayName}) + }) + + t.Run("With includeDeleted, search for dev, get channels 1, 2, 3 and 4", func(t *testing.T) { + searchAndCheck(t, "dev", true, []string{o1.DisplayName, o2.DisplayName, o3.DisplayName, o4.DisplayName}) + }) +} + func testChannelStoreSearchAllChannels(t *testing.T, ss store.Store) { cleanupChannels(t, ss) diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index c61206b534..e8dc3fd5cb 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -1420,6 +1420,31 @@ func (_m *ChannelStore) SearchAllChannels(term string, opts store.ChannelSearchO return r0, r1 } +// SearchForUserInTeam provides a mock function with given fields: userId, teamId, term, includeDeleted +func (_m *ChannelStore) SearchForUserInTeam(userId string, teamId string, term string, includeDeleted bool) (*model.ChannelList, *model.AppError) { + ret := _m.Called(userId, teamId, term, includeDeleted) + + var r0 *model.ChannelList + if rf, ok := ret.Get(0).(func(string, string, string, bool) *model.ChannelList); ok { + r0 = rf(userId, teamId, term, includeDeleted) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.ChannelList) + } + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string, string, string, bool) *model.AppError); ok { + r1 = rf(userId, teamId, term, includeDeleted) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 +} + // SearchGroupChannels provides a mock function with given fields: userId, term func (_m *ChannelStore) SearchGroupChannels(userId string, term string) (*model.ChannelList, *model.AppError) { ret := _m.Called(userId, term)