From 15e13fcdac2e7c069e46307d2813881f1003b64b Mon Sep 17 00:00:00 2001 From: Paul Esch-Laurent Date: Fri, 9 Sep 2022 20:56:44 -0500 Subject: [PATCH] fix: filter out bots for sharing, @mention-ing, and Person property (#3762) * fix: filter out bots for sharing and @mention-ing * feat: add `?exclude_bots` to `getTeamUsers` API * chore: `make swagger` * chore: `make generate` * fix: plugin store test function implementation Co-authored-by: Mattermod --- Makefile | 2 +- server/api/teams.go | 8 +++- server/app/user.go | 4 +- server/integrationtests/pluginteststore.go | 5 +- .../mattermostauthlayer.go | 7 ++- server/services/store/mockstore/mockstore.go | 8 ++-- .../services/store/sqlstore/public_methods.go | 4 +- server/services/store/sqlstore/user.go | 2 +- server/services/store/store.go | 2 +- server/swagger/docs/html/index.html | 48 +++++++++++++++---- server/swagger/swagger.yml | 4 ++ .../markdownEditorInput.tsx | 3 +- .../src/components/shareBoard/shareBoard.tsx | 3 +- webapp/src/octoClient.ts | 12 +++-- webapp/src/properties/person/person.tsx | 3 +- 15 files changed, 85 insertions(+), 30 deletions(-) diff --git a/Makefile b/Makefile index f33ca83d0..4f5231220 100644 --- a/Makefile +++ b/Makefile @@ -100,7 +100,7 @@ server-linux-package-docker: rm -rf package generate: ## Install and run code generators. - cd server; go get github.com/golang/mock/mockgen + cd server; go install github.com/golang/mock/mockgen@v1.6.0 cd server; go generate ./... server-lint: templates-archive ## Run linters on server code. diff --git a/server/api/teams.go b/server/api/teams.go index b0b138954..8e377f78c 100644 --- a/server/api/teams.go +++ b/server/api/teams.go @@ -198,6 +198,11 @@ func (a *API) handleGetTeamUsers(w http.ResponseWriter, r *http.Request) { // description: string to filter users list // required: false // type: string + // - name: exclude_bots + // in: query + // description: exclude bot users + // required: false + // type: boolean // security: // - BearerAuth: [] // responses: @@ -217,6 +222,7 @@ func (a *API) handleGetTeamUsers(w http.ResponseWriter, r *http.Request) { userID := getUserID(r) query := r.URL.Query() searchQuery := query.Get("search") + excludeBots := r.URL.Query().Get("exclude_bots") == True if !a.permissions.HasPermissionToTeam(userID, teamID, model.PermissionViewTeam) { a.errorResponse(w, r.URL.Path, http.StatusForbidden, "Access denied to team", PermissionError{"access denied to team"}) @@ -236,7 +242,7 @@ func (a *API) handleGetTeamUsers(w http.ResponseWriter, r *http.Request) { asGuestUser = userID } - users, err := a.app.SearchTeamUsers(teamID, searchQuery, asGuestUser) + users, err := a.app.SearchTeamUsers(teamID, searchQuery, asGuestUser, excludeBots) if err != nil { a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "searchQuery="+searchQuery, err) return diff --git a/server/app/user.go b/server/app/user.go index 99a5e0b62..c61dbbdef 100644 --- a/server/app/user.go +++ b/server/app/user.go @@ -9,8 +9,8 @@ func (a *App) GetTeamUsers(teamID string, asGuestID string) ([]*model.User, erro return a.store.GetUsersByTeam(teamID, asGuestID) } -func (a *App) SearchTeamUsers(teamID string, searchQuery string, asGuestID string) ([]*model.User, error) { - return a.store.SearchUsersByTeam(teamID, searchQuery, asGuestID) +func (a *App) SearchTeamUsers(teamID string, searchQuery string, asGuestID string, excludeBots bool) ([]*model.User, error) { + return a.store.SearchUsersByTeam(teamID, searchQuery, asGuestID, excludeBots) } func (a *App) UpdateUserConfig(userID string, patch model.UserPropPatch) ([]mmModel.Preference, error) { diff --git a/server/integrationtests/pluginteststore.go b/server/integrationtests/pluginteststore.go index 0625329b3..232e14fd6 100644 --- a/server/integrationtests/pluginteststore.go +++ b/server/integrationtests/pluginteststore.go @@ -219,7 +219,7 @@ func (s *PluginTestStore) GetUsersByTeam(teamID string, asGuestID string) ([]*mo return nil, errTestStore } -func (s *PluginTestStore) SearchUsersByTeam(teamID string, searchQuery string, asGuestID string) ([]*model.User, error) { +func (s *PluginTestStore) SearchUsersByTeam(teamID string, searchQuery string, asGuestID string, excludeBots bool) ([]*model.User, error) { users := []*model.User{} teamUsers, err := s.GetUsersByTeam(teamID, asGuestID) if err != nil { @@ -227,6 +227,9 @@ func (s *PluginTestStore) SearchUsersByTeam(teamID string, searchQuery string, a } for _, user := range teamUsers { + if excludeBots && user.IsBot { + continue + } if strings.Contains(user.Username, searchQuery) { users = append(users, user) } diff --git a/server/services/store/mattermostauthlayer/mattermostauthlayer.go b/server/services/store/mattermostauthlayer/mattermostauthlayer.go index 74a820e90..4bec9b169 100644 --- a/server/services/store/mattermostauthlayer/mattermostauthlayer.go +++ b/server/services/store/mattermostauthlayer/mattermostauthlayer.go @@ -351,7 +351,7 @@ func (s *MattermostAuthLayer) GetUsersList(userIDs []string) ([]*model.User, err return users, nil } -func (s *MattermostAuthLayer) SearchUsersByTeam(teamID string, searchQuery string, asGuestID string) ([]*model.User, error) { +func (s *MattermostAuthLayer) SearchUsersByTeam(teamID string, searchQuery string, asGuestID string, excludeBots bool) ([]*model.User, error) { query := s.getQueryBuilder(). Select("u.id", "u.username", "u.email", "u.nickname", "u.firstname", "u.lastname", "u.props", "u.CreateAt as create_at", "u.UpdateAt as update_at", "u.DeleteAt as delete_at", "b.UserId IS NOT NULL AS is_bot, u.roles = 'system_guest' as is_guest"). @@ -367,6 +367,11 @@ func (s *MattermostAuthLayer) SearchUsersByTeam(teamID string, searchQuery strin OrderBy("u.username"). Limit(10) + if excludeBots { + query = query. + Where(sq.Eq{"b.UserId IS NOT NULL": false}) + } + if asGuestID == "" { query = query. Join("TeamMembers as tm ON tm.UserID = u.id"). diff --git a/server/services/store/mockstore/mockstore.go b/server/services/store/mockstore/mockstore.go index 9bf7e5d4f..3e18c0723 100644 --- a/server/services/store/mockstore/mockstore.go +++ b/server/services/store/mockstore/mockstore.go @@ -1413,18 +1413,18 @@ func (mr *MockStoreMockRecorder) SearchUserChannels(arg0, arg1, arg2 interface{} } // SearchUsersByTeam mocks base method. -func (m *MockStore) SearchUsersByTeam(arg0, arg1, arg2 string) ([]*model.User, error) { +func (m *MockStore) SearchUsersByTeam(arg0, arg1, arg2 string, arg3 bool) ([]*model.User, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SearchUsersByTeam", arg0, arg1, arg2) + ret := m.ctrl.Call(m, "SearchUsersByTeam", arg0, arg1, arg2, arg3) ret0, _ := ret[0].([]*model.User) ret1, _ := ret[1].(error) return ret0, ret1 } // SearchUsersByTeam indicates an expected call of SearchUsersByTeam. -func (mr *MockStoreMockRecorder) SearchUsersByTeam(arg0, arg1, arg2 interface{}) *gomock.Call { +func (mr *MockStoreMockRecorder) SearchUsersByTeam(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SearchUsersByTeam", reflect.TypeOf((*MockStore)(nil).SearchUsersByTeam), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SearchUsersByTeam", reflect.TypeOf((*MockStore)(nil).SearchUsersByTeam), arg0, arg1, arg2, arg3) } // SendMessage mocks base method. diff --git a/server/services/store/sqlstore/public_methods.go b/server/services/store/sqlstore/public_methods.go index c2f22b62d..c8dfd8ff0 100644 --- a/server/services/store/sqlstore/public_methods.go +++ b/server/services/store/sqlstore/public_methods.go @@ -786,8 +786,8 @@ func (s *SQLStore) SearchUserChannels(teamID string, userID string, query string } -func (s *SQLStore) SearchUsersByTeam(teamID string, searchQuery string, asGuestID string) ([]*model.User, error) { - return s.searchUsersByTeam(s.db, teamID, searchQuery, asGuestID) +func (s *SQLStore) SearchUsersByTeam(teamID string, searchQuery string, asGuestID string, excludeBots bool) ([]*model.User, error) { + return s.searchUsersByTeam(s.db, teamID, searchQuery, asGuestID, excludeBots) } diff --git a/server/services/store/sqlstore/user.go b/server/services/store/sqlstore/user.go index 4c7634b3f..90b248ef9 100644 --- a/server/services/store/sqlstore/user.go +++ b/server/services/store/sqlstore/user.go @@ -218,7 +218,7 @@ func (s *SQLStore) getUsersByTeam(db sq.BaseRunner, _ string, _ string) ([]*mode return s.getUsersByCondition(db, nil, 0) } -func (s *SQLStore) searchUsersByTeam(db sq.BaseRunner, _ string, searchQuery string, _ string) ([]*model.User, error) { +func (s *SQLStore) searchUsersByTeam(db sq.BaseRunner, _ string, searchQuery string, _ string, _ bool) ([]*model.User, error) { return s.getUsersByCondition(db, &sq.Like{"username": "%" + searchQuery + "%"}, 10) } diff --git a/server/services/store/store.go b/server/services/store/store.go index 61de662f3..634b3f3ca 100644 --- a/server/services/store/store.go +++ b/server/services/store/store.go @@ -63,7 +63,7 @@ type Store interface { UpdateUserPassword(username, password string) error UpdateUserPasswordByID(userID, password string) error GetUsersByTeam(teamID string, asGuestID string) ([]*model.User, error) - SearchUsersByTeam(teamID string, searchQuery string, asGuestID string) ([]*model.User, error) + SearchUsersByTeam(teamID string, searchQuery string, asGuestID string, excludeBots bool) ([]*model.User, error) PatchUserProps(userID string, patch model.UserPropPatch) error GetUserPreferences(userID string) (mmModel.Preferences, error) diff --git a/server/swagger/docs/html/index.html b/server/swagger/docs/html/index.html index 6a2e8ff31..81ff75c86 100644 --- a/server/swagger/docs/html/index.html +++ b/server/swagger/docs/html/index.html @@ -14909,7 +14909,7 @@ Team ID 
curl -X GET \
 -H "Authorization: [[apiKey]]" \
  -H "Accept: application/json" \
- "http://localhost/api/v2/teams/{teamID}/users?search=search_example"
+ "http://localhost/api/v2/teams/{teamID}/users?search=search_example&exclude_bots=true"
@@ -14935,9 +14935,10 @@ public class DefaultApiExample { DefaultApi apiInstance = new DefaultApi(); String teamID = teamID_example; // String | Team ID String search = search_example; // String | string to filter users list + Boolean excludeBots = true; // Boolean | exclude bot users try { - array[Object] result = apiInstance.getTeamUsers(teamID, search); + array[Object] result = apiInstance.getTeamUsers(teamID, search, excludeBots); System.out.println(result); } catch (ApiException e) { System.err.println("Exception when calling DefaultApi#getTeamUsers"); @@ -14956,9 +14957,10 @@ public class DefaultApiExample { DefaultApi apiInstance = new DefaultApi(); String teamID = teamID_example; // String | Team ID String search = search_example; // String | string to filter users list + Boolean excludeBots = true; // Boolean | exclude bot users try { - array[Object] result = apiInstance.getTeamUsers(teamID, search); + array[Object] result = apiInstance.getTeamUsers(teamID, search, excludeBots); System.out.println(result); } catch (ApiException e) { System.err.println("Exception when calling DefaultApi#getTeamUsers"); @@ -14984,9 +14986,11 @@ public class DefaultApiExample { DefaultApi *apiInstance = [[DefaultApi alloc] init]; String *teamID = teamID_example; // Team ID (default to null) String *search = search_example; // string to filter users list (optional) (default to null) +Boolean *excludeBots = true; // exclude bot users (optional) (default to null) [apiInstance getTeamUsersWith:teamID search:search + excludeBots:excludeBots completionHandler: ^(array[Object] output, NSError* error) { if (output) { NSLog(@"%@", output); @@ -15012,7 +15016,8 @@ BearerAuth.apiKey = "YOUR API KEY"; var api = new FocalboardServer.DefaultApi() var teamID = teamID_example; // {String} Team ID var opts = { - 'search': search_example // {String} string to filter users list + 'search': search_example, // {String} string to filter users list + 'excludeBots': true // {Boolean} exclude bot users }; var callback = function(error, data, response) { @@ -15051,9 +15056,10 @@ namespace Example var apiInstance = new DefaultApi(); var teamID = teamID_example; // String | Team ID (default to null) var search = search_example; // String | string to filter users list (optional) (default to null) + var excludeBots = true; // Boolean | exclude bot users (optional) (default to null) try { - array[Object] result = apiInstance.getTeamUsers(teamID, search); + array[Object] result = apiInstance.getTeamUsers(teamID, search, excludeBots); Debug.WriteLine(result); } catch (Exception e) { Debug.Print("Exception when calling DefaultApi.getTeamUsers: " + e.Message ); @@ -15077,9 +15083,10 @@ OpenAPITools\Client\Configuration::getDefaultConfiguration()->setApiKey('Authori $api_instance = new OpenAPITools\Client\Api\DefaultApi(); $teamID = teamID_example; // String | Team ID $search = search_example; // String | string to filter users list +$excludeBots = true; // Boolean | exclude bot users try { - $result = $api_instance->getTeamUsers($teamID, $search); + $result = $api_instance->getTeamUsers($teamID, $search, $excludeBots); print_r($result); } catch (Exception $e) { echo 'Exception when calling DefaultApi->getTeamUsers: ', $e->getMessage(), PHP_EOL; @@ -15101,9 +15108,10 @@ $WWW::OPenAPIClient::Configuration::api_key->{'Authorization'} = 'YOUR_API_KEY'; my $api_instance = WWW::OPenAPIClient::DefaultApi->new(); my $teamID = teamID_example; # String | Team ID my $search = search_example; # String | string to filter users list +my $excludeBots = true; # Boolean | exclude bot users eval { - my $result = $api_instance->getTeamUsers(teamID => $teamID, search => $search); + my $result = $api_instance->getTeamUsers(teamID => $teamID, search => $search, excludeBots => $excludeBots); print Dumper($result); }; if ($@) { @@ -15127,9 +15135,10 @@ openapi_client.configuration.api_key['Authorization'] = 'YOUR_API_KEY' api_instance = openapi_client.DefaultApi() teamID = teamID_example # String | Team ID (default to null) search = search_example # String | string to filter users list (optional) (default to null) +excludeBots = true # Boolean | exclude bot users (optional) (default to null) try: - api_response = api_instance.get_team_users(teamID, search=search) + api_response = api_instance.get_team_users(teamID, search=search, excludeBots=excludeBots) pprint(api_response) except ApiException as e: print("Exception when calling DefaultApi->getTeamUsers: %s\n" % e) @@ -15141,9 +15150,10 @@ except ApiException as e: pub fn main() { let teamID = teamID_example; // String let search = search_example; // String + let excludeBots = true; // Boolean let mut context = DefaultApi::Context::default(); - let result = client.getTeamUsers(teamID, search, &context).wait(); + let result = client.getTeamUsers(teamID, search, excludeBots, &context).wait(); println!("{:?}", result); } @@ -15216,6 +15226,26 @@ string to filter users list
+ + + exclude_bots + + + +
+
+
+ + Boolean + + +
+exclude bot users +
+
+
+
+ diff --git a/server/swagger/swagger.yml b/server/swagger/swagger.yml index 0226e3b36..ea9c8c961 100644 --- a/server/swagger/swagger.yml +++ b/server/swagger/swagger.yml @@ -1637,6 +1637,10 @@ paths: in: query name: search type: string + - description: exclude bot users + in: query + name: exclude_bots + type: boolean produces: - application/json responses: diff --git a/webapp/src/components/markdownEditorInput/markdownEditorInput.tsx b/webapp/src/components/markdownEditorInput/markdownEditorInput.tsx index 6995e731b..929118e2b 100644 --- a/webapp/src/components/markdownEditorInput/markdownEditorInput.tsx +++ b/webapp/src/components/markdownEditorInput/markdownEditorInput.tsx @@ -74,7 +74,8 @@ const MarkdownEditorInput = (props: Props): ReactElement => { let users: Array if (!me?.is_guest && (allowAddUsers || (board && board.type === BoardTypeOpen))) { - users = await octoClient.searchTeamUsers(term) + const excludeBots = true + users = await octoClient.searchTeamUsers(term, excludeBots) } else { users = boardUsers .filter(user => { diff --git a/webapp/src/components/shareBoard/shareBoard.tsx b/webapp/src/components/shareBoard/shareBoard.tsx index 2110ad9d5..36f5d9ca8 100644 --- a/webapp/src/components/shareBoard/shareBoard.tsx +++ b/webapp/src/components/shareBoard/shareBoard.tsx @@ -364,7 +364,8 @@ export default function ShareBoardDialog(props: Props): JSX.Element { loadOptions={async (inputValue: string) => { const result = [] if (Utils.isFocalboardPlugin()) { - const users = await client.searchTeamUsers(inputValue) + const excludeBots = true + const users = await client.searchTeamUsers(inputValue, excludeBots) if (users) { result.push({label: intl.formatMessage({id: 'shareBoard.members-select-group', defaultMessage: 'Members'}), options: users || []}) } diff --git a/webapp/src/octoClient.ts b/webapp/src/octoClient.ts index 8297c72df..5b144ee44 100644 --- a/webapp/src/octoClient.ts +++ b/webapp/src/octoClient.ts @@ -629,8 +629,10 @@ class OctoClient { return this.getJson>(response, []) } - async getTeamUsers(): Promise { - const path = this.teamPath() + '/users' + async getTeamUsers(excludeBots?: boolean): Promise { + let path = this.teamPath() + '/users' + if (excludeBots) + path += '?exclude_bots=true' const response = await fetch(this.getBaseURL() + path, {headers: this.headers()}) if (response.status !== 200) { return [] @@ -638,8 +640,10 @@ class OctoClient { return (await this.getJson(response, [])) as IUser[] } - async searchTeamUsers(searchQuery: string): Promise { - const path = this.teamPath() + `/users?search=${searchQuery}` + async searchTeamUsers(searchQuery: string, excludeBots?: boolean): Promise { + let path = this.teamPath() + `/users?search=${searchQuery}` + if (excludeBots) + path += '&exclude_bots=true' const response = await fetch(this.getBaseURL() + path, {headers: this.headers()}) if (response.status !== 200) { return [] diff --git a/webapp/src/properties/person/person.tsx b/webapp/src/properties/person/person.tsx index 958ed4e60..4ac4d2fe3 100644 --- a/webapp/src/properties/person/person.tsx +++ b/webapp/src/properties/person/person.tsx @@ -127,7 +127,8 @@ const Person = (props: PropertyProps): JSX.Element => { if (!allowAddUsers) { return boardUsers.filter((u) => u.username.toLowerCase().includes(value.toLowerCase())) } - const allUsers = await client.searchTeamUsers(value) + const excludeBots = true + const allUsers = await client.searchTeamUsers(value, excludeBots) const usersInsideBoard: IUser[] = [] const usersOutsideBoard: IUser[] = [] for (const u of allUsers) {