From 715097cc76510a3d78ba83e8544ee7c956ed26e9 Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Wed, 17 Oct 2018 11:24:12 -0400 Subject: MM-12234: configurable limit to user autocomplete and search matches (#9499) * unit test cleanup * allow limiting user search results * clean up test users before starting * model UserSearchOptions to simplify parameters --- api4/apitestlib.go | 7 +++++-- api4/user.go | 59 ++++++++++++++++++++++++++++++++---------------------- api4/user_test.go | 44 ++++++++++++++++++++-------------------- 3 files changed, 62 insertions(+), 48 deletions(-) (limited to 'api4') diff --git a/api4/apitestlib.go b/api4/apitestlib.go index 37dbcad25..9976932a9 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -184,8 +184,11 @@ func (me *TestHelper) TearDown() { go func() { defer wg.Done() - options := map[string]bool{} - options[store.USER_SEARCH_OPTION_NAMES_ONLY_NO_FULL_NAME] = true + options := &model.UserSearchOptions{ + AllowEmails: false, + AllowFullNames: false, + Limit: model.USER_SEARCH_MAX_LIMIT, + } if result := <-me.App.Srv.Store.User().Search("", "fakeuser", options); result.Err != nil { mlog.Error("Error tearing down test users") } else { diff --git a/api4/user.go b/api4/user.go index c97e90e89..b827857a7 100644 --- a/api4/user.go +++ b/api4/user.go @@ -14,7 +14,6 @@ import ( "github.com/mattermost/mattermost-server/app" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" - "github.com/mattermost/mattermost-server/store" ) func (api *API) InitUser() { @@ -547,23 +546,26 @@ func searchUsers(c *Context, w http.ResponseWriter, r *http.Request) { return } - searchOptions := map[string]bool{} - searchOptions[store.USER_SEARCH_OPTION_ALLOW_INACTIVE] = props.AllowInactive + if props.Limit <= 0 || props.Limit > model.USER_SEARCH_MAX_LIMIT { + c.SetInvalidParam("limit") + return + } - if !c.App.SessionHasPermissionTo(c.Session, model.PERMISSION_MANAGE_SYSTEM) { - hideFullName := !c.App.Config().PrivacySettings.ShowFullName - hideEmail := !c.App.Config().PrivacySettings.ShowEmailAddress - - if hideFullName && hideEmail { - searchOptions[store.USER_SEARCH_OPTION_NAMES_ONLY_NO_FULL_NAME] = true - } else if hideFullName { - searchOptions[store.USER_SEARCH_OPTION_ALL_NO_FULL_NAME] = true - } else if hideEmail { - searchOptions[store.USER_SEARCH_OPTION_NAMES_ONLY] = true - } + options := &model.UserSearchOptions{ + IsAdmin: c.IsSystemAdmin(), + AllowInactive: props.AllowInactive, + Limit: props.Limit, } - profiles, err := c.App.SearchUsers(props, searchOptions, c.IsSystemAdmin()) + if c.App.SessionHasPermissionTo(c.Session, model.PERMISSION_MANAGE_SYSTEM) { + options.AllowEmails = true + options.AllowFullNames = true + } else { + options.AllowEmails = c.App.Config().PrivacySettings.ShowEmailAddress + options.AllowFullNames = c.App.Config().PrivacySettings.ShowFullName + } + + profiles, err := c.App.SearchUsers(props, options) if err != nil { c.Err = err return @@ -576,17 +578,26 @@ func autocompleteUsers(c *Context, w http.ResponseWriter, r *http.Request) { channelId := r.URL.Query().Get("in_channel") teamId := r.URL.Query().Get("in_team") name := r.URL.Query().Get("name") + limitStr := r.URL.Query().Get("limit") + limit, _ := strconv.Atoi(limitStr) + if limitStr == "" { + limit = model.USER_SEARCH_DEFAULT_LIMIT + } autocomplete := new(model.UserAutocomplete) var err *model.AppError - searchOptions := map[string]bool{} + options := &model.UserSearchOptions{ + IsAdmin: c.IsSystemAdmin(), + // Never autocomplete on emails. + AllowEmails: false, + Limit: limit, + } - hideFullName := !c.App.Config().PrivacySettings.ShowFullName - if hideFullName && !c.App.SessionHasPermissionTo(c.Session, model.PERMISSION_MANAGE_SYSTEM) { - searchOptions[store.USER_SEARCH_OPTION_NAMES_ONLY_NO_FULL_NAME] = true + if c.App.SessionHasPermissionTo(c.Session, model.PERMISSION_MANAGE_SYSTEM) { + options.AllowFullNames = true } else { - searchOptions[store.USER_SEARCH_OPTION_NAMES_ONLY] = true + options.AllowFullNames = c.App.Config().PrivacySettings.ShowFullName } if len(channelId) > 0 { @@ -606,8 +617,8 @@ func autocompleteUsers(c *Context, w http.ResponseWriter, r *http.Request) { if len(channelId) > 0 { // Applying the provided teamId here is useful for DMs and GMs which don't belong // to a team. Applying it when the channel does belong to a team makes less sense, - //t but the permissions are checked above regardless. - result, err := c.App.AutocompleteUsersInChannel(teamId, channelId, name, searchOptions, c.IsSystemAdmin()) + // but the permissions are checked above regardless. + result, err := c.App.AutocompleteUsersInChannel(teamId, channelId, name, options) if err != nil { c.Err = err return @@ -616,7 +627,7 @@ func autocompleteUsers(c *Context, w http.ResponseWriter, r *http.Request) { autocomplete.Users = result.InChannel autocomplete.OutOfChannel = result.OutOfChannel } else if len(teamId) > 0 { - result, err := c.App.AutocompleteUsersInTeam(teamId, name, searchOptions, c.IsSystemAdmin()) + result, err := c.App.AutocompleteUsersInTeam(teamId, name, options) if err != nil { c.Err = err return @@ -625,7 +636,7 @@ func autocompleteUsers(c *Context, w http.ResponseWriter, r *http.Request) { autocomplete.Users = result.InTeam } else { // No permission check required - result, err := c.App.SearchUsersInTeam("", name, searchOptions, c.IsSystemAdmin()) + result, err := c.App.SearchUsersInTeam("", name, options) if err != nil { c.Err = err return diff --git a/api4/user_test.go b/api4/user_test.go index b99011aeb..9df062a2a 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -759,92 +759,92 @@ func TestAutocompleteUsers(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { cfg.PrivacySettings.ShowFullName = showFullName }) }() - rusers, resp := Client.AutocompleteUsersInChannel(teamId, channelId, username, "") + rusers, resp := Client.AutocompleteUsersInChannel(teamId, channelId, username, model.USER_SEARCH_DEFAULT_LIMIT, "") CheckNoError(t, resp) if len(rusers.Users) != 1 { t.Fatal("should have returned 1 user") } - rusers, resp = Client.AutocompleteUsersInChannel(teamId, channelId, "amazonses", "") + rusers, resp = Client.AutocompleteUsersInChannel(teamId, channelId, "amazonses", model.USER_SEARCH_DEFAULT_LIMIT, "") CheckNoError(t, resp) if len(rusers.Users) != 0 { t.Fatal("should have returned 0 users") } - rusers, resp = Client.AutocompleteUsersInChannel(teamId, channelId, "", "") + rusers, resp = Client.AutocompleteUsersInChannel(teamId, channelId, "", model.USER_SEARCH_DEFAULT_LIMIT, "") CheckNoError(t, resp) if len(rusers.Users) < 2 { t.Fatal("should have many users") } - rusers, resp = Client.AutocompleteUsersInChannel("", channelId, "", "") + rusers, resp = Client.AutocompleteUsersInChannel("", channelId, "", model.USER_SEARCH_DEFAULT_LIMIT, "") CheckNoError(t, resp) if len(rusers.Users) < 2 { t.Fatal("should have many users") } - rusers, resp = Client.AutocompleteUsersInTeam(teamId, username, "") + rusers, resp = Client.AutocompleteUsersInTeam(teamId, username, model.USER_SEARCH_DEFAULT_LIMIT, "") CheckNoError(t, resp) if len(rusers.Users) != 1 { t.Fatal("should have returned 1 user") } - rusers, resp = Client.AutocompleteUsers(username, "") + rusers, resp = Client.AutocompleteUsers(username, model.USER_SEARCH_DEFAULT_LIMIT, "") CheckNoError(t, resp) if len(rusers.Users) != 1 { t.Fatal("should have returned 1 users") } - rusers, resp = Client.AutocompleteUsers("", "") + rusers, resp = Client.AutocompleteUsers("", model.USER_SEARCH_DEFAULT_LIMIT, "") CheckNoError(t, resp) if len(rusers.Users) < 2 { t.Fatal("should have returned many users") } - rusers, resp = Client.AutocompleteUsersInTeam(teamId, "amazonses", "") + rusers, resp = Client.AutocompleteUsersInTeam(teamId, "amazonses", model.USER_SEARCH_DEFAULT_LIMIT, "") CheckNoError(t, resp) if len(rusers.Users) != 0 { t.Fatal("should have returned 0 users") } - rusers, resp = Client.AutocompleteUsersInTeam(teamId, "", "") + rusers, resp = Client.AutocompleteUsersInTeam(teamId, "", model.USER_SEARCH_DEFAULT_LIMIT, "") CheckNoError(t, resp) if len(rusers.Users) < 2 { t.Fatal("should have many users") } Client.Logout() - _, resp = Client.AutocompleteUsersInChannel(teamId, channelId, username, "") + _, resp = Client.AutocompleteUsersInChannel(teamId, channelId, username, model.USER_SEARCH_DEFAULT_LIMIT, "") CheckUnauthorizedStatus(t, resp) - _, resp = Client.AutocompleteUsersInTeam(teamId, username, "") + _, resp = Client.AutocompleteUsersInTeam(teamId, username, model.USER_SEARCH_DEFAULT_LIMIT, "") CheckUnauthorizedStatus(t, resp) - _, resp = Client.AutocompleteUsers(username, "") + _, resp = Client.AutocompleteUsers(username, model.USER_SEARCH_DEFAULT_LIMIT, "") CheckUnauthorizedStatus(t, resp) user := th.CreateUser() Client.Login(user.Email, user.Password) - _, resp = Client.AutocompleteUsersInChannel(teamId, channelId, username, "") + _, resp = Client.AutocompleteUsersInChannel(teamId, channelId, username, model.USER_SEARCH_DEFAULT_LIMIT, "") CheckForbiddenStatus(t, resp) - _, resp = Client.AutocompleteUsersInTeam(teamId, username, "") + _, resp = Client.AutocompleteUsersInTeam(teamId, username, model.USER_SEARCH_DEFAULT_LIMIT, "") CheckForbiddenStatus(t, resp) - _, resp = Client.AutocompleteUsers(username, "") + _, resp = Client.AutocompleteUsers(username, model.USER_SEARCH_DEFAULT_LIMIT, "") CheckNoError(t, resp) - _, resp = th.SystemAdminClient.AutocompleteUsersInChannel(teamId, channelId, username, "") + _, resp = th.SystemAdminClient.AutocompleteUsersInChannel(teamId, channelId, username, model.USER_SEARCH_DEFAULT_LIMIT, "") CheckNoError(t, resp) - _, resp = th.SystemAdminClient.AutocompleteUsersInTeam(teamId, username, "") + _, resp = th.SystemAdminClient.AutocompleteUsersInTeam(teamId, username, model.USER_SEARCH_DEFAULT_LIMIT, "") CheckNoError(t, resp) - _, resp = th.SystemAdminClient.AutocompleteUsers(username, "") + _, resp = th.SystemAdminClient.AutocompleteUsers(username, model.USER_SEARCH_DEFAULT_LIMIT, "") CheckNoError(t, resp) // Check against privacy config settings @@ -852,21 +852,21 @@ func TestAutocompleteUsers(t *testing.T) { th.LoginBasic() - rusers, resp = Client.AutocompleteUsers(username, "") + rusers, resp = Client.AutocompleteUsers(username, model.USER_SEARCH_DEFAULT_LIMIT, "") CheckNoError(t, resp) if rusers.Users[0].FirstName != "" || rusers.Users[0].LastName != "" { t.Fatal("should not show first/last name") } - rusers, resp = Client.AutocompleteUsersInChannel(teamId, channelId, username, "") + rusers, resp = Client.AutocompleteUsersInChannel(teamId, channelId, username, model.USER_SEARCH_DEFAULT_LIMIT, "") CheckNoError(t, resp) if rusers.Users[0].FirstName != "" || rusers.Users[0].LastName != "" { t.Fatal("should not show first/last name") } - rusers, resp = Client.AutocompleteUsersInTeam(teamId, username, "") + rusers, resp = Client.AutocompleteUsersInTeam(teamId, username, model.USER_SEARCH_DEFAULT_LIMIT, "") CheckNoError(t, resp) if rusers.Users[0].FirstName != "" || rusers.Users[0].LastName != "" { @@ -874,7 +874,7 @@ func TestAutocompleteUsers(t *testing.T) { } t.Run("user must have access to team id, especially when it does not match channel's team id", func(t *testing.T) { - rusers, resp = Client.AutocompleteUsersInChannel("otherTeamId", channelId, username, "") + rusers, resp = Client.AutocompleteUsersInChannel("otherTeamId", channelId, username, model.USER_SEARCH_DEFAULT_LIMIT, "") CheckErrorMessage(t, resp, "api.context.permissions.app_error") }) } -- cgit v1.2.3-1-g7c22