diff --git a/app/assets/javascripts/api/user_api.js b/app/assets/javascripts/api/user_api.js index c056b42b5b6df4fa321ecce2c40167195ee685e4..302de9760800c11c55180295c7d1a7274202282b 100644 --- a/app/assets/javascripts/api/user_api.js +++ b/app/assets/javascripts/api/user_api.js @@ -4,6 +4,7 @@ import { buildApiUrl } from './api_utils'; const USER_COUNTS_PATH = '/api/:version/user_counts'; const USERS_PATH = '/api/:version/users.json'; +const USERS_SAML_PATH = '/api/:version/groups/:id/users.json'; const USER_PATH = '/api/:version/users/:id'; const USER_STATUS_PATH = '/api/:version/users/:id/status'; const USER_PROJECTS_PATH = '/api/:version/users/:id/projects'; @@ -25,6 +26,25 @@ export function getUsers(query, options) { }); } +/** + * Returns a list of SAML users and service accounts that contains the query string. + * If the query string is less than 3 characters it returns an empty list. + * + * @param {string} query - query string to search + * @param {string} groupId -- top-level group id + * @param {object} options + */ +export function getGroupUsers(query, groupId, options) { + const url = buildApiUrl(USERS_SAML_PATH).replace(':id', groupId); + return axios.get(url, { + params: { + search: query, + per_page: DEFAULT_PER_PAGE, + ...options, + }, + }); +} + export function getUser(id, options) { const url = buildApiUrl(USER_PATH).replace(':id', encodeURIComponent(id)); return axios.get(url, { 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 bcf594a7b1ce98d5da25f4fcdeefebee7148893c..dead90eeb71ff09e8d9ab78da579a5f3bcfcc68a 100644 --- a/app/assets/javascripts/invite_members/components/invite_members_modal.vue +++ b/app/assets/javascripts/invite_members/components/invite_members_modal.vue @@ -516,6 +516,7 @@ export default { :exception-state="exceptionState" :users-filter="usersFilter" :filter-id="filterId" + :root-group-id="rootId" :invalid-members="invalidMembers" @clear="clearValidation" @token-remove="removeToken" diff --git a/app/assets/javascripts/invite_members/components/members_token_select.vue b/app/assets/javascripts/invite_members/components/members_token_select.vue index b29755545f23bf5466c9b8cc1a0085cc4e360747..0e3f2890b29898949c76e6b4b62d89266e9f73c4 100644 --- a/app/assets/javascripts/invite_members/components/members_token_select.vue +++ b/app/assets/javascripts/invite_members/components/members_token_select.vue @@ -2,7 +2,9 @@ import { GlTokenSelector, GlAvatar, GlAvatarLabeled, GlIcon, GlSprintf } from '@gitlab/ui'; import { debounce, isEmpty } from 'lodash'; import { __ } from '~/locale'; -import { getUsers } from '~/rest_api'; +import { getUsers, getGroupUsers } from '~/rest_api'; +import * as Sentry from '~/sentry/sentry_browser_wrapper'; +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { memberName } from '../utils/member_utils'; import { SEARCH_DELAY, @@ -20,6 +22,7 @@ export default { GlIcon, GlSprintf, }, + mixins: [glFeatureFlagsMixin()], props: { canUseEmailToken: { type: Boolean, @@ -59,6 +62,10 @@ export default { required: false, default: '', }, + rootGroupId: { + type: String, + required: true, + }, }, data() { return { @@ -87,9 +94,16 @@ export default { }, queryOptions() { if (this.usersFilter === USERS_FILTER_SAML_PROVIDER_ID) { + if (!this.glFeatures.groupUserSaml) { + return { + saml_provider_id: this.filterId, + ...this.$options.defaultQueryOptions, + }; + } return { - saml_provider_id: this.filterId, - ...this.$options.defaultQueryOptions, + active: true, + include_saml_users: true, + include_service_accounts: true, }; } return this.$options.defaultQueryOptions; @@ -133,19 +147,27 @@ export default { class: this.tokenClass(token), })); }, - retrieveUsers: debounce(function debouncedRetrieveUsers() { - return getUsers(this.query, this.queryOptions) - .then((response) => { - this.users = response.data.map((token) => ({ - id: token.id, - name: token.name, - username: token.username, - avatar_url: token.avatar_url, - })); - }) - .finally(() => { - this.loading = false; - }); + retrieveUsersRequest() { + if (this.usersFilter === USERS_FILTER_SAML_PROVIDER_ID && this.glFeatures.groupUserSaml) { + return getGroupUsers(this.query, this.rootGroupId, this.queryOptions); + } + + return getUsers(this.query, this.queryOptions); + }, + retrieveUsers: debounce(async function debouncedRetrieveUsers() { + try { + const { data } = await this.retrieveUsersRequest(); + this.users = data.map((token) => ({ + id: token.id, + name: token.name, + username: token.username, + avatar_url: token.avatar_url, + })); + } catch (error) { + Sentry.captureException(error); + } + + this.loading = false; }, SEARCH_DELAY), tokenClass(token) { if (this.hasError(token)) { diff --git a/config/feature_flags/development/group_user_saml.yml b/config/feature_flags/development/group_user_saml.yml new file mode 100644 index 0000000000000000000000000000000000000000..f3a03aad18f77d30ca5648d1edfa52263fe341bb --- /dev/null +++ b/config/feature_flags/development/group_user_saml.yml @@ -0,0 +1,8 @@ +--- +name: group_user_saml +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/138075 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/434464 +milestone: '16.7' +type: development +group: group::authentication +default_enabled: false diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index b630eb13a606a6effe40892def83f11e0d68af97..0a49a2c750553bcf0ece7acc1d0c57e0c826a754 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -82,6 +82,7 @@ def add_gon_variables push_frontend_feature_flag(:remove_monitor_metrics) push_frontend_feature_flag(:custom_emoji) push_frontend_feature_flag(:encoding_logs_tree) + push_frontend_feature_flag(:group_user_saml) end # Exposes the state of a feature flag to the frontend code. diff --git a/spec/frontend/api/user_api_spec.js b/spec/frontend/api/user_api_spec.js index a6e08e1cf4b5a4d7f2023b815a6ec7fc1c987757..aeddf6b9ae135f0b9628f07f775f9ca9694b4e1b 100644 --- a/spec/frontend/api/user_api_spec.js +++ b/spec/frontend/api/user_api_spec.js @@ -4,6 +4,8 @@ import projects from 'test_fixtures/api/users/projects/get.json'; import followers from 'test_fixtures/api/users/followers/get.json'; import following from 'test_fixtures/api/users/following/get.json'; import { + getUsers, + getGroupUsers, followUser, unfollowUser, associationsCount, @@ -36,6 +38,32 @@ describe('~/api/user_api', () => { axiosMock.resetHistory(); }); + describe('getUsers', () => { + it('calls correct URL with expected query parameters', async () => { + const expectedUrl = '/api/v4/users.json'; + axiosMock.onGet(expectedUrl).replyOnce(HTTP_STATUS_OK); + + await getUsers('den', { without_project_bots: true }); + + const { url, params } = axiosMock.history.get[0]; + expect(url).toBe(expectedUrl); + expect(params).toMatchObject({ search: 'den', without_project_bots: true }); + }); + }); + + describe('getSAMLUsers', () => { + it('calls correct URL with expected query parameters', async () => { + const expectedUrl = '/api/v4/groups/34/users.json'; + axiosMock.onGet(expectedUrl).replyOnce(HTTP_STATUS_OK); + + await getGroupUsers('den', '34', { include_service_accounts: true }); + + const { url, params } = axiosMock.history.get[0]; + expect(url).toBe(expectedUrl); + expect(params).toMatchObject({ search: 'den', include_service_accounts: true }); + }); + }); + describe('followUser', () => { it('calls correct URL and returns expected response', async () => { const expectedUrl = '/api/v4/users/1/follow'; 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 5e36cfe915ad668311ce15978196bdb4355b515c..a2b21367388884e75a1b8abdb65b4d03d2edcc06 100644 --- a/spec/frontend/invite_members/components/members_token_select_spec.js +++ b/spec/frontend/invite_members/components/members_token_select_spec.js @@ -6,22 +6,26 @@ import waitForPromises from 'helpers/wait_for_promises'; import * as UserApi from '~/api/user_api'; import MembersTokenSelect from '~/invite_members/components/members_token_select.vue'; import { VALID_TOKEN_BACKGROUND, INVALID_TOKEN_BACKGROUND } from '~/invite_members/constants'; +import * as Sentry from '~/sentry/sentry_browser_wrapper'; const label = 'testgroup'; const placeholder = 'Search for a member'; +const rootGroupId = '31'; 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 handleEnterSpy = jest.fn(); -const createComponent = (props) => { +const createComponent = (props = {}, glFeatures = {}) => { return shallowMount(MembersTokenSelect, { propsData: { ariaLabelledby: label, invalidMembers: {}, placeholder, + rootGroupId, ...props, }, + provide: { glFeatures }, stubs: { GlTokenSelector: stubComponent(GlTokenSelector, { methods: { @@ -167,6 +171,21 @@ describe('MembersTokenSelect', () => { }); }); + describe('when API search fails', () => { + beforeEach(() => { + jest.spyOn(Sentry, 'captureException'); + jest.spyOn(UserApi, 'getUsers').mockRejectedValue('error'); + }); + + it('reports to sentry', async () => { + tokenSelector.vm.$emit('text-input', 'Den'); + + await waitForPromises(); + + expect(Sentry.captureException).toHaveBeenCalledWith('error'); + }); + }); + it('allows tab to function as enter', () => { tokenSelector.vm.$emit('text-input', 'username'); @@ -216,31 +235,45 @@ describe('MembersTokenSelect', () => { }); }); - describe('when component is mounted for a group using a saml provider', () => { + 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; - }), - ); + jest.spyOn(UserApi, 'getGroupUsers').mockResolvedValue({ data: allUsers }); - wrapper = createComponent({ filterId: samlProviderId, usersFilter: 'saml_provider_id' }); + wrapper = createComponent({ usersFilter: 'saml_provider_id' }, { groupUserSaml: true }); findTokenSelector().vm.$emit('text-input', searchParam); }); - it('calls the API with the saml provider ID param', () => { - resolveApiRequest({ data: allUsers }); - - expect(UserApi.getUsers).toHaveBeenCalledWith(searchParam, { + it('calls the group API with correct parameters', () => { + expect(UserApi.getGroupUsers).toHaveBeenCalledWith(searchParam, rootGroupId, { active: true, - without_project_bots: true, - saml_provider_id: samlProviderId, + include_saml_users: true, + include_service_accounts: true, + }); + }); + }); + + describe('when group_user_saml feature flag is disabled', () => { + describe('when component is mounted for a group using a SAML provider', () => { + const searchParam = 'name'; + const samlProviderId = 123; + + beforeEach(() => { + jest.spyOn(UserApi, 'getUsers').mockResolvedValue({ data: allUsers }); + + wrapper = createComponent({ filterId: samlProviderId, usersFilter: 'saml_provider_id' }); + + findTokenSelector().vm.$emit('text-input', searchParam); + }); + + it('calls the API with the saml provider ID param', () => { + expect(UserApi.getUsers).toHaveBeenCalledWith(searchParam, { + active: true, + without_project_bots: true, + saml_provider_id: samlProviderId, + }); }); }); });