diff --git a/app/assets/javascripts/invite_members/components/invite_members_modal.vue b/app/assets/javascripts/invite_members/components/invite_members_modal.vue index 40eccefd7e0e0a889bc36dd65e8cf542c09d6284..ab42e8cdfebeee367f48747b849331625c259ef2 100644 --- a/app/assets/javascripts/invite_members/components/invite_members_modal.vue +++ b/app/assets/javascripts/invite_members/components/invite_members_modal.vue @@ -16,7 +16,12 @@ import Api from '~/api'; import ExperimentTracking from '~/experimentation/experiment_tracking'; import { BV_SHOW_MODAL } from '~/lib/utils/constants'; import { s__, sprintf } from '~/locale'; -import { INVITE_MEMBERS_IN_COMMENT, GROUP_FILTERS, MEMBER_AREAS_OF_FOCUS } from '../constants'; +import { + INVITE_MEMBERS_IN_COMMENT, + GROUP_FILTERS, + USERS_FILTER_ALL, + MEMBER_AREAS_OF_FOCUS, +} from '../constants'; import eventHub from '../event_hub'; import { responseMessageFromError, @@ -72,6 +77,16 @@ export default { required: false, default: null, }, + usersFilter: { + type: String, + required: false, + default: USERS_FILTER_ALL, + }, + filterId: { + type: Number, + required: false, + default: null, + }, helpLink: { type: String, required: true, @@ -384,6 +399,8 @@ export default { class="gl-mb-2" :validation-state="validationState" :aria-labelledby="$options.membersTokenSelectLabelId" + :users-filter="usersFilter" + :filter-id="filterId" @clear="handleMembersTokenSelectClear" /> { this.users = response.data.map((token) => ({ id: token.id, @@ -98,7 +117,7 @@ export default { this.$emit('clear'); }, }, - queryOptions: { exclude_internal: true, active: true }, + defaultQueryOptions: { exclude_internal: true, active: true }, i18n: { inviteTextMessage: __('Invite "%{email}" by email'), }, diff --git a/app/assets/javascripts/invite_members/constants.js b/app/assets/javascripts/invite_members/constants.js index 01b35f2a656acc099e3eb09c303ada02a9be0bcf..d7daf83e26b0446f7717e3df662f35fc79d60175 100644 --- a/app/assets/javascripts/invite_members/constants.js +++ b/app/assets/javascripts/invite_members/constants.js @@ -17,3 +17,5 @@ export const GROUP_FILTERS = { export const API_MESSAGES = { EMAIL_ALREADY_INVITED: __('Invite email has already been taken'), }; +export const USERS_FILTER_ALL = 'all'; +export const USERS_FILTER_SAML_PROVIDER_ID = 'saml_provider_id'; diff --git a/app/assets/javascripts/invite_members/init_invite_members_modal.js b/app/assets/javascripts/invite_members/init_invite_members_modal.js index db7e2ca4a71918ccafb940937c72e32b547a80f9..c1dfaa25dc7e675514620052a9d1207d14557889 100644 --- a/app/assets/javascripts/invite_members/init_invite_members_modal.js +++ b/app/assets/javascripts/invite_members/init_invite_members_modal.js @@ -25,6 +25,8 @@ export default function initInviteMembersModal() { groupSelectParentId: parseInt(el.dataset.parentId, 10), areasOfFocusOptions: JSON.parse(el.dataset.areasOfFocusOptions), noSelectionAreasOfFocus: JSON.parse(el.dataset.noSelectionAreasOfFocus), + usersFilter: el.dataset.usersFilter, + filterId: parseInt(el.dataset.filterId, 10), }, }), }); diff --git a/app/helpers/invite_members_helper.rb b/app/helpers/invite_members_helper.rb index 3274ea15b8b5562bd74d12159464c4cd7be15b3e..e9e5f8b3a0be9c57383f0d18d362867c58834f6b 100644 --- a/app/helpers/invite_members_helper.rb +++ b/app/helpers/invite_members_helper.rb @@ -78,4 +78,9 @@ def member_areas_of_focus_options } ] end + + # Overridden in EE + def users_filter_data(group) + {} + end end diff --git a/app/views/groups/_invite_members_modal.html.haml b/app/views/groups/_invite_members_modal.html.haml index 8801ad98b8cdee63d8670e1873ffe3e4435006c5..760efb16d40f395229869e94338b6322debb5136 100644 --- a/app/views/groups/_invite_members_modal.html.haml +++ b/app/views/groups/_invite_members_modal.html.haml @@ -2,4 +2,5 @@ .js-invite-members-modal{ data: { is_project: 'false', access_levels: GroupMember.access_level_roles.to_json, - help_link: help_page_url('user/permissions') }.merge(group_select_data(group)).merge(common_invite_modal_dataset(group)) } + default_access_level: Gitlab::Access::GUEST, + help_link: help_page_url('user/permissions') }.merge(group_select_data(group)).merge(common_invite_modal_dataset(group)).merge(users_filter_data(group)) } diff --git a/app/views/projects/_invite_members_modal.html.haml b/app/views/projects/_invite_members_modal.html.haml index 16964d2154a305dafe4f761f679090a355a54c53..f2fffeb23c5a87a1a33374139ef97aa0e7ced054 100644 --- a/app/views/projects/_invite_members_modal.html.haml +++ b/app/views/projects/_invite_members_modal.html.haml @@ -2,4 +2,4 @@ .js-invite-members-modal{ data: { is_project: 'true', access_levels: ProjectMember.access_level_roles.to_json, - help_link: help_page_url('user/permissions') }.merge(common_invite_modal_dataset(project)) } + help_link: help_page_url('user/permissions') }.merge(common_invite_modal_dataset(project)).merge(users_filter_data(project.group)) } diff --git a/ee/app/helpers/ee/invite_members_helper.rb b/ee/app/helpers/ee/invite_members_helper.rb new file mode 100644 index 0000000000000000000000000000000000000000..5de9f9ada9b115ca2a62ef961a68398ef4ee5a49 --- /dev/null +++ b/ee/app/helpers/ee/invite_members_helper.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module EE + module InviteMembersHelper + def users_filter_data(group) + root_group = group&.root_ancestor + + return {} unless root_group&.enforced_sso? && root_group.saml_provider&.id + + { users_filter: 'saml_provider_id', filter_id: root_group.saml_provider.id } + end + end +end diff --git a/ee/spec/features/groups/members/list_members_spec.rb b/ee/spec/features/groups/members/list_members_spec.rb index 646092c76c821ce703547fef05fd89bd2a80349a..e39847ca9d959acf780d3283447c39d752f1cbf3 100644 --- a/ee/spec/features/groups/members/list_members_spec.rb +++ b/ee/spec/features/groups/members/list_members_spec.rb @@ -4,21 +4,20 @@ RSpec.describe 'Groups > Members > List members' do include Spec::Support::Helpers::Features::MembersHelpers - let(:user1) { create(:user, name: 'John Doe') } - let(:user2) { create(:user, name: 'Mary Jane') } - let(:group) { create(:group) } + let_it_be(:user1) { create(:user, name: 'John Doe') } + let_it_be(:user2) { create(:user, name: 'Mary Jane') } + let_it_be(:group) { create(:group) } context 'with Group SAML identity linked for a user' do - let(:saml_provider) { create(:saml_provider) } + let_it_be(:saml_provider) { create(:saml_provider) } + let(:group) { saml_provider.group } before do sign_in(user1) group.add_developer(user1) group.add_guest(user2) - user2.identities.create!(provider: :group_saml, - saml_provider: saml_provider, - extern_uid: 'user2@example.com') + create(:identity, saml_provider: saml_provider, user: user2) end it 'shows user with SSO status badge', :js do @@ -44,45 +43,40 @@ end context 'with SAML and enforced SSO' do - let(:saml_provider) { create(:saml_provider, group: group, enabled: true, enforced_sso: true) } - let(:user3) { create(:user, name: 'Amy with different SAML provider') } - let(:user4) { create(:user, name: 'Bob without SAML') } - let(:session) { { active_group_sso_sign_ins: { saml_provider.id => DateTime.now } } } + let_it_be(:saml_provider) { create(:saml_provider, group: group, enabled: true, enforced_sso: true) } + let_it_be(:user3) { create(:user, name: 'Amy with different SAML provider') } + let_it_be(:user4) { create(:user, name: 'Bob without SAML') } + let_it_be(:session) { { active_group_sso_sign_ins: { saml_provider.id => DateTime.now } } } before do stub_licensed_features(group_saml: true) allow(Gitlab::Session).to receive(:current).and_return(session) - - create(:identity, saml_provider: saml_provider, user: user1) - - group.add_owner(user1) sign_in(user1) end - it 'returns only users with SAML in autocomplete', :js do - create(:identity, saml_provider: saml_provider, user: user2) + before_all do + create(:identity, provider: 'group_saml1', saml_provider_id: saml_provider.id, user: user1) + create(:identity, provider: 'group_saml1', saml_provider_id: saml_provider.id, user: user2) create(:identity, user: user3) + group.add_owner(user1) + end + it 'returns only users with SAML in autocomplete', :js do visit group_group_members_path(group) - wait_for_requests - click_on 'Invite members' page.within '#invite-members-modal' do - [user1, user2].each do |user_with_saml| - find('[data-testid="members-token-select-input"]').set(user_with_saml.name) - wait_for_requests - - expect(page).to have_content(user_with_saml.name) - end + field = find('[data-testid="members-token-select-input"]') + field.native.send_keys :tab + field.click - [user3, user4].each do |user_without_saml| - find('[data-testid="members-token-select-input"]').set(user_without_saml.name) - wait_for_requests + wait_for_requests - expect(page).not_to have_content(user_without_saml.name) - end + expect(page).to have_content(user1.name) + expect(page).to have_content(user2.name) + expect(page).not_to have_content(user3.name) + expect(page).not_to have_content(user4.name) end end @@ -92,9 +86,6 @@ end it 'returns only users with SAML in autocomplete', :js do - create(:identity, saml_provider: saml_provider, user: user2) - create(:identity, user: user3) - visit group_group_members_path(group) wait_for_requests diff --git a/ee/spec/helpers/ee/invite_members_helper_spec.rb b/ee/spec/helpers/ee/invite_members_helper_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..fd2724259e121168314bc165bab79c6472817d8d --- /dev/null +++ b/ee/spec/helpers/ee/invite_members_helper_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe EE::InviteMembersHelper do + describe '.users_filter_data' do + let_it_be(:group) { create(:group) } + let_it_be(:saml_provider) { create(:saml_provider, group: group) } + + let!(:group2) { create(:group) } + + context 'when the group has enforced sso' do + before do + allow(group).to receive(:enforced_sso?).and_return(true) + end + + context 'when there is a group with a saml provider' do + it 'returns user filter data' do + expected = { users_filter: 'saml_provider_id', filter_id: saml_provider.id } + + expect(helper.users_filter_data(group)).to eq expected + end + end + + context 'when there is a group without a saml provider' do + it 'does not return user filter data' do + expect(helper.users_filter_data(group2)).to eq({}) + end + end + end + + context 'when group has enforced sso disabled' do + before do + allow(group).to receive(:enforced_sso?).and_return(false) + end + + context 'when there is a group with a saml provider' do + it 'does not return user filter data' do + expect(helper.users_filter_data(group)).to eq({}) + end + end + + context 'when there is a group without a saml provider' do + it 'does not return user filter data' do + expect(helper.users_filter_data(group2)).to eq({}) + end + end + end + end +end diff --git a/spec/frontend/invite_members/components/members_token_select_spec.js b/spec/frontend/invite_members/components/members_token_select_spec.js index 12db7e42464629d744d7a0812738ebf421c9f2ca..196a716d08cc2a9b389160ebf82020a1167d4dec 100644 --- a/spec/frontend/invite_members/components/members_token_select_spec.js +++ b/spec/frontend/invite_members/components/members_token_select_spec.js @@ -12,11 +12,12 @@ const user1 = { id: 1, name: 'John Smith', username: 'one_1', avatar_url: '' }; const user2 = { id: 2, name: 'Jane Doe', username: 'two_2', avatar_url: '' }; const allUsers = [user1, user2]; -const createComponent = () => { +const createComponent = (props) => { return shallowMount(MembersTokenSelect, { propsData: { ariaLabelledby: label, placeholder, + ...props, }, stubs: { GlTokenSelector: stubComponent(GlTokenSelector), @@ -27,11 +28,6 @@ const createComponent = () => { describe('MembersTokenSelect', () => { let wrapper; - beforeEach(() => { - jest.spyOn(UserApi, 'getUsers').mockResolvedValue({ data: allUsers }); - wrapper = createComponent(); - }); - afterEach(() => { wrapper.destroy(); wrapper = null; @@ -41,6 +37,8 @@ describe('MembersTokenSelect', () => { describe('rendering the token-selector component', () => { it('renders with the correct props', () => { + wrapper = createComponent(); + const expectedProps = { ariaLabelledby: label, placeholder, @@ -51,6 +49,11 @@ describe('MembersTokenSelect', () => { }); describe('users', () => { + beforeEach(() => { + jest.spyOn(UserApi, 'getUsers').mockResolvedValue({ data: allUsers }); + wrapper = createComponent(); + }); + describe('when input is focused for the first time (modal auto-focus)', () => { it('does not call the API', async () => { findTokenSelector().vm.$emit('focus'); @@ -90,10 +93,10 @@ describe('MembersTokenSelect', () => { await waitForPromises(); - expect(UserApi.getUsers).toHaveBeenCalledWith( - searchParam, - wrapper.vm.$options.queryOptions, - ); + expect(UserApi.getUsers).toHaveBeenCalledWith(searchParam, { + active: true, + exclude_internal: true, + }); expect(tokenSelector.props('hideDropdownWithNoItems')).toBe(false); }); @@ -134,6 +137,8 @@ describe('MembersTokenSelect', () => { describe('when text input is blurred', () => { it('clears text input', async () => { + wrapper = createComponent(); + const tokenSelector = findTokenSelector(); tokenSelector.vm.$emit('blur'); @@ -143,4 +148,33 @@ describe('MembersTokenSelect', () => { expect(tokenSelector.props('hideDropdownWithNoItems')).toBe(false); }); }); + + describe('when component is mounted for a group using a saml provider', () => { + const searchParam = 'name'; + const samlProviderId = 123; + let resolveApiRequest; + + beforeEach(() => { + jest.spyOn(UserApi, 'getUsers').mockImplementation( + () => + new Promise((resolve) => { + resolveApiRequest = resolve; + }), + ); + + wrapper = createComponent({ filterId: samlProviderId, usersFilter: 'saml_provider_id' }); + + findTokenSelector().vm.$emit('text-input', searchParam); + }); + + it('calls the API with the saml provider ID param', () => { + resolveApiRequest({ data: allUsers }); + + expect(UserApi.getUsers).toHaveBeenCalledWith(searchParam, { + active: true, + exclude_internal: true, + saml_provider_id: samlProviderId, + }); + }); + }); });