From cff7de9e68402f544dedc229c170e8f363c8a547 Mon Sep 17 00:00:00 2001 From: Mario de Frutos Dieguez Date: Mon, 20 Apr 2020 11:21:51 +0200 Subject: [PATCH] [MM-24065] Guest users should be able to search by channel (#14302) * Remove unnecessary check for PERMISSION_LIST_TEAM_CHANNELS In the autocompleteChannelsForTeamForSearch method we're checking for the PERMISSION_LIST_TEAM_CHANNELS permission in order to avoid filtering channels in the autocomplete search but this check is not necessary. Now we're going directly to the database to search for those channels in this specific method and we're filtering by channel membership and team so there is no chance that we are going to filter undesired channels to the user. [Here](https://github.com/mattermost/mattermost-server/blob/v5.22.0/store/sqlstore/channel_store.go#L2014) is the query where you can see the filtering we're making --- api4/channel.go | 7 --- api4/channel_test.go | 140 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 138 insertions(+), 9 deletions(-) diff --git a/api4/channel.go b/api4/channel.go index dd653efb47..fe5a4f368b 100644 --- a/api4/channel.go +++ b/api4/channel.go @@ -865,11 +865,6 @@ func autocompleteChannelsForTeamForSearch(c *Context, w http.ResponseWriter, r * return } - if !c.App.SessionHasPermissionToTeam(*c.App.Session(), c.Params.TeamId, model.PERMISSION_LIST_TEAM_CHANNELS) { - c.SetPermissionError(model.PERMISSION_LIST_TEAM_CHANNELS) - return - } - name := r.URL.Query().Get("name") channels, err := c.App.AutocompleteChannelsForSearch(c.Params.TeamId, c.App.Session().UserId, name) @@ -878,8 +873,6 @@ func autocompleteChannelsForTeamForSearch(c *Context, w http.ResponseWriter, r * return } - // Don't fill in channels props, since unused by client and potentially expensive. - w.Write([]byte(channels.ToJson())) } diff --git a/api4/channel_test.go b/api4/channel_test.go index d54d605346..5dfc6459c3 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -2746,9 +2746,13 @@ func TestAutocompleteChannelsForSearch(t *testing.T) { th.LoginBasicWithClient(th.Client) u1 := th.CreateUserWithClient(th.SystemAdminClient) + defer th.App.PermanentDeleteUser(u1) u2 := th.CreateUserWithClient(th.SystemAdminClient) + defer th.App.PermanentDeleteUser(u2) u3 := th.CreateUserWithClient(th.SystemAdminClient) + defer th.App.PermanentDeleteUser(u3) u4 := th.CreateUserWithClient(th.SystemAdminClient) + defer th.App.PermanentDeleteUser(u4) // A private channel to make sure private channels are not used utils.DisableDebugLogForTest() @@ -2798,7 +2802,7 @@ func TestAutocompleteChannelsForSearch(t *testing.T) { for _, tc := range []struct { description string - teamId string + teamID string fragment string expectedIncludes []string expectedExcludes []string @@ -2833,7 +2837,139 @@ func TestAutocompleteChannelsForSearch(t *testing.T) { }, } { t.Run(tc.description, func(t *testing.T) { - channels, resp := th.Client.AutocompleteChannelsForTeamForSearch(tc.teamId, tc.fragment) + channels, resp := th.Client.AutocompleteChannelsForTeamForSearch(tc.teamID, tc.fragment) + require.Nil(t, resp.Error) + names := make([]string, len(*channels)) + for i, c := range *channels { + names[i] = c.Name + } + for _, name := range tc.expectedIncludes { + require.Contains(t, names, name, "channel not included") + } + for _, name := range tc.expectedExcludes { + require.NotContains(t, names, name, "channel not excluded") + } + }) + } +} + +func TestAutocompleteChannelsForSearchGuestUsers(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + u1 := th.CreateUserWithClient(th.SystemAdminClient) + defer th.App.PermanentDeleteUser(u1) + + enableGuestAccounts := *th.App.Config().GuestAccountsSettings.Enable + defer func() { + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = enableGuestAccounts }) + th.App.RemoveLicense() + }() + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = true }) + th.App.SetLicense(model.NewTestLicense()) + + id := model.NewId() + guest := &model.User{ + Email: "success+" + id + "@simulator.amazonses.com", + Username: "un_" + id, + Nickname: "nn_" + id, + Password: "Password1", + EmailVerified: true, + } + guest, err := th.App.CreateGuest(guest) + require.Nil(t, err) + + th.LoginSystemAdminWithClient(th.SystemAdminClient) + + _, resp := th.SystemAdminClient.AddTeamMember(th.BasicTeam.Id, guest.Id) + CheckNoError(t, resp) + + // A private channel to make sure private channels are not used + utils.DisableDebugLogForTest() + town, _ := th.SystemAdminClient.CreateChannel(&model.Channel{ + DisplayName: "Town", + Name: "town", + Type: model.CHANNEL_OPEN, + TeamId: th.BasicTeam.Id, + }) + defer func() { + th.SystemAdminClient.DeleteChannel(town.Id) + }() + _, resp = th.SystemAdminClient.AddChannelMember(town.Id, guest.Id) + CheckNoError(t, resp) + + mypriv, _ := th.SystemAdminClient.CreateChannel(&model.Channel{ + DisplayName: "My private town", + Name: "townpriv", + Type: model.CHANNEL_PRIVATE, + TeamId: th.BasicTeam.Id, + }) + defer func() { + th.SystemAdminClient.DeleteChannel(mypriv.Id) + }() + _, resp = th.SystemAdminClient.AddChannelMember(mypriv.Id, guest.Id) + CheckNoError(t, resp) + + utils.EnableDebugLogForTest() + + dc1, resp := th.SystemAdminClient.CreateDirectChannel(th.BasicUser.Id, guest.Id) + CheckNoError(t, resp) + defer func() { + th.SystemAdminClient.DeleteChannel(dc1.Id) + }() + + dc2, resp := th.SystemAdminClient.CreateDirectChannel(th.BasicUser.Id, th.BasicUser2.Id) + CheckNoError(t, resp) + defer func() { + th.SystemAdminClient.DeleteChannel(dc2.Id) + }() + + gc1, resp := th.SystemAdminClient.CreateGroupChannel([]string{th.BasicUser.Id, th.BasicUser2.Id, guest.Id}) + CheckNoError(t, resp) + defer func() { + th.SystemAdminClient.DeleteChannel(gc1.Id) + }() + + gc2, resp := th.SystemAdminClient.CreateGroupChannel([]string{th.BasicUser.Id, th.BasicUser2.Id, u1.Id}) + CheckNoError(t, resp) + defer func() { + th.SystemAdminClient.DeleteChannel(gc2.Id) + }() + + _, resp = th.Client.Login(guest.Username, "Password1") + CheckNoError(t, resp) + + for _, tc := range []struct { + description string + teamID string + fragment string + expectedIncludes []string + expectedExcludes []string + }{ + { + "Should return those channel where is member", + th.BasicTeam.Id, + "town", + []string{"town", "townpriv"}, + []string{"town-square", "off-topic"}, + }, + { + "Should return empty if not member of the searched channels", + th.BasicTeam.Id, + "off-to", + []string{}, + []string{"off-topic", "town-square", "town", "townpriv"}, + }, + { + "Should return direct and group messages", + th.BasicTeam.Id, + "fakeuser", + []string{dc1.Name, gc1.Name}, + []string{dc2.Name, gc2.Name}, + }, + } { + t.Run(tc.description, func(t *testing.T) { + channels, resp := th.Client.AutocompleteChannelsForTeamForSearch(tc.teamID, tc.fragment) require.Nil(t, resp.Error) names := make([]string, len(*channels)) for i, c := range *channels {