Commit 7a9a10c8 authored by Doug Stull's avatar Doug Stull Committed by Patrick Bair

Filter out project bots from user results on invite members

- project bots shouldn't be managed through the invite modal
  process - we have project access tokens for that process.

Changelog: changed
parent 503a4b5b
...@@ -117,7 +117,7 @@ export default { ...@@ -117,7 +117,7 @@ export default {
this.$emit('clear'); this.$emit('clear');
}, },
}, },
defaultQueryOptions: { exclude_internal: true, active: true }, defaultQueryOptions: { without_project_bots: true, active: true },
i18n: { i18n: {
inviteTextMessage: __('Invite "%{email}" by email'), inviteTextMessage: __('Invite "%{email}" by email'),
}, },
......
...@@ -47,6 +47,7 @@ class UsersFinder ...@@ -47,6 +47,7 @@ class UsersFinder
users = by_without_projects(users) users = by_without_projects(users)
users = by_custom_attributes(users) users = by_custom_attributes(users)
users = by_non_internal(users) users = by_non_internal(users)
users = by_without_project_bots(users)
order(users) order(users)
end end
...@@ -138,6 +139,12 @@ class UsersFinder ...@@ -138,6 +139,12 @@ class UsersFinder
users.non_internal users.non_internal
end end
def by_without_project_bots(users)
return users unless params[:without_project_bots]
users.without_project_bot
end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def order(users) def order(users)
return users unless params[:sort] return users unless params[:sort]
......
...@@ -97,6 +97,14 @@ In addition, to exclude external users from the users' list, you can use the par ...@@ -97,6 +97,14 @@ In addition, to exclude external users from the users' list, you can use the par
GET /users?exclude_external=true GET /users?exclude_external=true
``` ```
To exclude [bot users for projects](../user/project/settings/project_access_tokens.md#bot-users-for-projects)
and [bot users for groups](../user/group/settings/group_access_tokens.md#bot-users-for-groups), you can use the
parameter `without_project_bots=true`.
```plaintext
GET /users?without_project_bots=true
```
### For admins ### For admins
> The `namespace_id` field in the response was [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82045) in GitLab 14.10. > The `namespace_id` field in the response was [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82045) in GitLab 14.10.
......
...@@ -3,6 +3,7 @@ require 'spec_helper' ...@@ -3,6 +3,7 @@ require 'spec_helper'
RSpec.describe 'Groups > Members > List members' do RSpec.describe 'Groups > Members > List members' do
include Spec::Support::Helpers::Features::MembersHelpers include Spec::Support::Helpers::Features::MembersHelpers
include Spec::Support::Helpers::Features::InviteMembersModalHelper
let_it_be(:user1) { create(:user, name: 'John Doe') } let_it_be(:user1) { create(:user, name: 'John Doe') }
let_it_be(:user2) { create(:user, name: 'Mary Jane') } let_it_be(:user2) { create(:user, name: 'Mary Jane') }
...@@ -66,8 +67,8 @@ RSpec.describe 'Groups > Members > List members' do ...@@ -66,8 +67,8 @@ RSpec.describe 'Groups > Members > List members' do
click_on 'Invite members' click_on 'Invite members'
page.within '[data-testid="invite-modal"]' do page.within invite_modal_selector do
field = find('[data-testid="members-token-select-input"]') field = find(member_dropdown_selector)
field.native.send_keys :tab field.native.send_keys :tab
field.click field.click
......
...@@ -58,7 +58,7 @@ RSpec.describe 'Groups > Members > Manage members', :saas, :js do ...@@ -58,7 +58,7 @@ RSpec.describe 'Groups > Members > Manage members', :saas, :js do
click_on 'Invite members' click_on 'Invite members'
page.within '[data-testid="invite-modal"]' do page.within invite_modal_selector do
add_user_to_input(user2.name) add_user_to_input(user2.name)
add_user_to_input(user3.name) add_user_to_input(user3.name)
...@@ -118,7 +118,7 @@ RSpec.describe 'Groups > Members > Manage members', :saas, :js do ...@@ -118,7 +118,7 @@ RSpec.describe 'Groups > Members > Manage members', :saas, :js do
click_on 'Invite members' click_on 'Invite members'
page.within '[data-testid="invite-modal"]' do page.within invite_modal_selector do
add_user_to_input(name) add_user_to_input(name)
choose_options(role, nil) choose_options(role, nil)
...@@ -127,7 +127,7 @@ RSpec.describe 'Groups > Members > Manage members', :saas, :js do ...@@ -127,7 +127,7 @@ RSpec.describe 'Groups > Members > Manage members', :saas, :js do
end end
def add_user_to_input(name) def add_user_to_input(name)
find('[data-testid="members-token-select-input"]').set(name) find(member_dropdown_selector).set(name)
wait_for_requests wait_for_requests
click_button name click_button name
......
...@@ -3,7 +3,6 @@ ...@@ -3,7 +3,6 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe 'Project > Members > Invite group and members' do RSpec.describe 'Project > Members > Invite group and members' do
include Select2Helper
include ActionView::Helpers::DateHelper include ActionView::Helpers::DateHelper
include Spec::Support::Helpers::Features::MembersHelpers include Spec::Support::Helpers::Features::MembersHelpers
......
...@@ -89,6 +89,7 @@ module API ...@@ -89,6 +89,7 @@ module API
optional :created_before, type: DateTime, desc: 'Return users created before the specified time' optional :created_before, type: DateTime, desc: 'Return users created before the specified time'
optional :without_projects, type: Boolean, default: false, desc: 'Filters only users without projects' optional :without_projects, type: Boolean, default: false, desc: 'Filters only users without projects'
optional :exclude_internal, as: :non_internal, type: Boolean, default: false, desc: 'Filters only non internal users' optional :exclude_internal, as: :non_internal, type: Boolean, default: false, desc: 'Filters only non internal users'
optional :without_project_bots, type: Boolean, default: false, desc: 'Filters users without project bots'
optional :admins, type: Boolean, default: false, desc: 'Filters only admin users' optional :admins, type: Boolean, default: false, desc: 'Filters only admin users'
all_or_none_of :extern_uid, :provider all_or_none_of :extern_uid, :provider
......
...@@ -63,25 +63,6 @@ RSpec.describe 'Groups > Members > Manage members' do ...@@ -63,25 +63,6 @@ RSpec.describe 'Groups > Members > Manage members' do
) )
end end
it 'do not disclose email addresses', :js do
group.add_owner(user1)
create(:user, email: 'undisclosed_email@gitlab.com', name: "Jane 'invisible' Doe")
visit group_group_members_path(group)
click_on 'Invite members'
find('[data-testid="members-token-select-input"]').set('@gitlab.com')
wait_for_requests
expect(page).to have_content('No matches found')
find('[data-testid="members-token-select-input"]').set('undisclosed_email@gitlab.com')
wait_for_requests
expect(page).to have_content('Invite "undisclosed_email@gitlab.com" by email')
end
it 'remove user from group', :js do it 'remove user from group', :js do
group.add_owner(user1) group.add_owner(user1)
group.add_developer(user2) group.add_developer(user2)
...@@ -169,4 +150,57 @@ RSpec.describe 'Groups > Members > Manage members' do ...@@ -169,4 +150,57 @@ RSpec.describe 'Groups > Members > Manage members' do
end end
end end
end end
describe 'member search results', :js do
before do
group.add_owner(user1)
end
it 'does not disclose email addresses' do
create(:user, email: 'undisclosed_email@gitlab.com', name: "Jane 'invisible' Doe")
visit group_group_members_path(group)
click_on 'Invite members'
find(member_dropdown_selector).set('@gitlab.com')
wait_for_requests
expect(page).to have_content('No matches found')
find(member_dropdown_selector).set('undisclosed_email@gitlab.com')
wait_for_requests
expect(page).to have_content('Invite "undisclosed_email@gitlab.com" by email')
end
it 'does not show project_bots', :aggregate_failures do
internal_project_bot = create(:user, :project_bot, name: '_internal_project_bot_')
project = create(:project, group: group)
project.add_maintainer(internal_project_bot)
external_group = create(:group)
external_project_bot = create(:user, :project_bot, name: '_external_project_bot_')
external_project = create(:project, group: external_group)
external_project.add_maintainer(external_project_bot)
external_project.add_maintainer(user1)
visit group_group_members_path(group)
click_on 'Invite members'
page.within invite_modal_selector do
field = find(member_dropdown_selector)
field.native.send_keys :tab
field.click
wait_for_requests
expect(page).to have_content(user1.name)
expect(page).to have_content(user2.name)
expect(page).not_to have_content(internal_project_bot.name)
expect(page).not_to have_content(external_project_bot.name)
end
end
end
end end
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe 'Project members list', :js do RSpec.describe 'Projects > Members > Manage members', :js do
include Spec::Support::Helpers::Features::MembersHelpers include Spec::Support::Helpers::Features::MembersHelpers
include Spec::Support::Helpers::Features::InviteMembersModalHelper include Spec::Support::Helpers::Features::InviteMembersModalHelper
include Spec::Support::Helpers::ModalHelpers include Spec::Support::Helpers::ModalHelpers
...@@ -124,6 +124,36 @@ RSpec.describe 'Project members list', :js do ...@@ -124,6 +124,36 @@ RSpec.describe 'Project members list', :js do
) )
end end
describe 'member search results' do
it 'does not show project_bots', :aggregate_failures do
internal_project_bot = create(:user, :project_bot, name: '_internal_project_bot_')
project.add_maintainer(internal_project_bot)
external_group = create(:group)
external_project_bot = create(:user, :project_bot, name: '_external_project_bot_')
external_project = create(:project, group: external_group)
external_project.add_maintainer(external_project_bot)
external_project.add_maintainer(user1)
visit_members_page
click_on 'Invite members'
page.within invite_modal_selector do
field = find(member_dropdown_selector)
field.native.send_keys :tab
field.click
wait_for_requests
expect(page).to have_content(user1.name)
expect(page).to have_content(user2.name)
expect(page).not_to have_content(internal_project_bot.name)
expect(page).not_to have_content(external_project_bot.name)
end
end
end
context 'as a signed out visitor viewing a public project' do context 'as a signed out visitor viewing a public project' do
let_it_be(:project) { create(:project, :public) } let_it_be(:project) { create(:project, :public) }
......
...@@ -6,13 +6,15 @@ RSpec.describe UsersFinder do ...@@ -6,13 +6,15 @@ RSpec.describe UsersFinder do
describe '#execute' do describe '#execute' do
include_context 'UsersFinder#execute filter by project context' include_context 'UsersFinder#execute filter by project context'
let_it_be(:project_bot) { create(:user, :project_bot) }
context 'with a normal user' do context 'with a normal user' do
let(:user) { create(:user) } let_it_be(:user) { create(:user) }
it 'returns all users' do it 'returns all users' do
users = described_class.new(user).execute users = described_class.new(user).execute
expect(users).to contain_exactly(user, normal_user, blocked_user, external_user, omniauth_user, internal_user, admin_user) expect(users).to contain_exactly(user, normal_user, blocked_user, external_user, omniauth_user, internal_user, admin_user, project_bot)
end end
it 'filters by username' do it 'filters by username' do
...@@ -54,7 +56,7 @@ RSpec.describe UsersFinder do ...@@ -54,7 +56,7 @@ RSpec.describe UsersFinder do
it 'filters by active users' do it 'filters by active users' do
users = described_class.new(user, active: true).execute users = described_class.new(user, active: true).execute
expect(users).to contain_exactly(user, normal_user, external_user, omniauth_user, admin_user) expect(users).to contain_exactly(user, normal_user, external_user, omniauth_user, admin_user, project_bot)
end end
it 'filters by external users' do it 'filters by external users' do
...@@ -66,7 +68,7 @@ RSpec.describe UsersFinder do ...@@ -66,7 +68,7 @@ RSpec.describe UsersFinder do
it 'filters by non external users' do it 'filters by non external users' do
users = described_class.new(user, non_external: true).execute users = described_class.new(user, non_external: true).execute
expect(users).to contain_exactly(user, normal_user, blocked_user, omniauth_user, internal_user, admin_user) expect(users).to contain_exactly(user, normal_user, blocked_user, omniauth_user, internal_user, admin_user, project_bot)
end end
it 'filters by created_at' do it 'filters by created_at' do
...@@ -83,7 +85,13 @@ RSpec.describe UsersFinder do ...@@ -83,7 +85,13 @@ RSpec.describe UsersFinder do
it 'filters by non internal users' do it 'filters by non internal users' do
users = described_class.new(user, non_internal: true).execute users = described_class.new(user, non_internal: true).execute
expect(users).to contain_exactly(user, normal_user, external_user, blocked_user, omniauth_user, admin_user) expect(users).to contain_exactly(user, normal_user, external_user, blocked_user, omniauth_user, admin_user, project_bot)
end
it 'filters by without project bots' do
users = described_class.new(user, without_project_bots: true).execute
expect(users).to contain_exactly(user, normal_user, external_user, blocked_user, omniauth_user, internal_user, admin_user)
end end
it 'does not filter by custom attributes' do it 'does not filter by custom attributes' do
...@@ -92,23 +100,23 @@ RSpec.describe UsersFinder do ...@@ -92,23 +100,23 @@ RSpec.describe UsersFinder do
custom_attributes: { foo: 'bar' } custom_attributes: { foo: 'bar' }
).execute ).execute
expect(users).to contain_exactly(user, normal_user, blocked_user, external_user, omniauth_user, internal_user, admin_user) expect(users).to contain_exactly(user, normal_user, blocked_user, external_user, omniauth_user, internal_user, admin_user, project_bot)
end end
it 'orders returned results' do it 'orders returned results' do
users = described_class.new(user, sort: 'id_asc').execute users = described_class.new(user, sort: 'id_asc').execute
expect(users).to eq([normal_user, admin_user, blocked_user, external_user, omniauth_user, internal_user, user]) expect(users).to eq([normal_user, admin_user, blocked_user, external_user, omniauth_user, internal_user, project_bot, user])
end end
it 'does not filter by admins' do it 'does not filter by admins' do
users = described_class.new(user, admins: true).execute users = described_class.new(user, admins: true).execute
expect(users).to contain_exactly(user, normal_user, external_user, admin_user, blocked_user, omniauth_user, internal_user) expect(users).to contain_exactly(user, normal_user, external_user, admin_user, blocked_user, omniauth_user, internal_user, project_bot)
end end
end end
context 'with an admin user', :enable_admin_mode do context 'with an admin user', :enable_admin_mode do
let(:admin) { create(:admin) } let_it_be(:admin) { create(:admin) }
it 'filters by external users' do it 'filters by external users' do
users = described_class.new(admin, external: true).execute users = described_class.new(admin, external: true).execute
...@@ -119,7 +127,7 @@ RSpec.describe UsersFinder do ...@@ -119,7 +127,7 @@ RSpec.describe UsersFinder do
it 'returns all users' do it 'returns all users' do
users = described_class.new(admin).execute users = described_class.new(admin).execute
expect(users).to contain_exactly(admin, normal_user, blocked_user, external_user, omniauth_user, internal_user, admin_user) expect(users).to contain_exactly(admin, normal_user, blocked_user, external_user, omniauth_user, internal_user, admin_user, project_bot)
end end
it 'returns only admins' do it 'returns only admins' do
......
...@@ -95,7 +95,7 @@ describe('MembersTokenSelect', () => { ...@@ -95,7 +95,7 @@ describe('MembersTokenSelect', () => {
expect(UserApi.getUsers).toHaveBeenCalledWith(searchParam, { expect(UserApi.getUsers).toHaveBeenCalledWith(searchParam, {
active: true, active: true,
exclude_internal: true, without_project_bots: true,
}); });
expect(tokenSelector.props('hideDropdownWithNoItems')).toBe(false); expect(tokenSelector.props('hideDropdownWithNoItems')).toBe(false);
}); });
...@@ -172,7 +172,7 @@ describe('MembersTokenSelect', () => { ...@@ -172,7 +172,7 @@ describe('MembersTokenSelect', () => {
expect(UserApi.getUsers).toHaveBeenCalledWith(searchParam, { expect(UserApi.getUsers).toHaveBeenCalledWith(searchParam, {
active: true, active: true,
exclude_internal: true, without_project_bots: true,
saml_provider_id: samlProviderId, saml_provider_id: samlProviderId,
}); });
}); });
......
...@@ -359,6 +359,26 @@ RSpec.describe API::Users do ...@@ -359,6 +359,26 @@ RSpec.describe API::Users do
end end
end end
context 'without_project_bots param' do
let_it_be(:project_bot) { create(:user, :project_bot) }
it 'returns all users when it is not set' do
get api("/users?without_project_bots=false", user)
expect(response).to match_response_schema('public_api/v4/user/basics')
expect(response).to include_pagination_headers
expect(json_response.map { |u| u['id'] }).to include(project_bot.id)
end
it 'returns all non project_bot users when it is set' do
get api("/users?without_project_bots=true", user)
expect(response).to match_response_schema('public_api/v4/user/basics')
expect(response).to include_pagination_headers
expect(json_response.map { |u| u['id'] }).not_to include(project_bot.id)
end
end
context 'admins param' do context 'admins param' do
it 'returns all users' do it 'returns all users' do
get api("/users?admins=true", user) get api("/users?admins=true", user)
......
...@@ -8,8 +8,8 @@ module Spec ...@@ -8,8 +8,8 @@ module Spec
def invite_member(name, role: 'Guest', expires_at: nil) def invite_member(name, role: 'Guest', expires_at: nil)
click_on 'Invite members' click_on 'Invite members'
page.within '[data-testid="invite-modal"]' do page.within invite_modal_selector do
find('[data-testid="members-token-select-input"]').set(name) find(member_dropdown_selector).set(name)
wait_for_requests wait_for_requests
click_button name click_button name
...@@ -53,6 +53,14 @@ module Spec ...@@ -53,6 +53,14 @@ module Spec
'[data-testid="group-select-dropdown"]' '[data-testid="group-select-dropdown"]'
end end
def member_dropdown_selector
'[data-testid="members-token-select-input"]'
end
def invite_modal_selector
'[data-testid="invite-modal"]'
end
def expect_to_have_group(group) def expect_to_have_group(group)
expect(page).to have_selector("[entity-id='#{group.id}']") expect(page).to have_selector("[entity-id='#{group.id}']")
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment