diff --git a/app/assets/javascripts/api/groups_api.js b/app/assets/javascripts/api/groups_api.js index 58b423f65b36f23b4c172addb533879dd75e7ec7..01f86fb4dae0c45470fb129f44a28bf1842a2732 100644 --- a/app/assets/javascripts/api/groups_api.js +++ b/app/assets/javascripts/api/groups_api.js @@ -97,3 +97,6 @@ export const getSharedGroups = (groupId, params = {}) => { return axios.get(url, { params: { ...defaultParams, ...params } }); }; + +// no-op: See EE code for implementation +export const fetchGroupEnterpriseUsers = () => {}; diff --git a/app/assets/javascripts/members/index.js b/app/assets/javascripts/members/index.js index cb19c51ee6b7ac05a8933ee22c5c5f24eee00df6..ae69b2e9627ef4ccd26b9e31735349628101ce85 100644 --- a/app/assets/javascripts/members/index.js +++ b/app/assets/javascripts/members/index.js @@ -40,6 +40,7 @@ export const initMembersApp = (el, context, options) => { namespaceUserLimit, availableRoles, reassignmentCsvPath, + restrictReassignmentToEnterprise, allowInactivePlaceholderReassignment, ...vuexStoreAttributes } = parseDataAttributes(el); @@ -86,6 +87,7 @@ export const initMembersApp = (el, context, options) => { availableRoles, context, reassignmentCsvPath, + restrictReassignmentToEnterprise, allowInactivePlaceholderReassignment: parseBoolean(allowInactivePlaceholderReassignment), group: { id: isGroup ? sourceId : null, diff --git a/app/assets/javascripts/members/placeholders/components/placeholder_actions.vue b/app/assets/javascripts/members/placeholders/components/placeholder_actions.vue index 53670d30f734650e66d3a24cb4520ad4003db7e9..125b1be0524d6e65da84e918b3ae18fcba99b642 100644 --- a/app/assets/javascripts/members/placeholders/components/placeholder_actions.vue +++ b/app/assets/javascripts/members/placeholders/components/placeholder_actions.vue @@ -2,11 +2,15 @@ import produce from 'immer'; import { debounce, isEmpty, isNull } from 'lodash'; import { GlAvatarLabeled, GlButton, GlCollapsibleListbox } from '@gitlab/ui'; +import { + getFirstPropertyValue, + normalizeHeaders, + parseIntPagination, +} from '~/lib/utils/common_utils'; import { __, s__ } from '~/locale'; import { createAlert } from '~/alert'; -import { getFirstPropertyValue } from '~/lib/utils/common_utils'; - import searchUsersQuery from '~/graphql_shared/queries/users_search_all_paginated.query.graphql'; +import { fetchGroupEnterpriseUsers } from 'ee_else_ce/api/groups_api'; import { DEFAULT_DEBOUNCE_AND_THROTTLE_MS } from '~/lib/utils/constants'; import { PLACEHOLDER_STATUS_AWAITING_APPROVAL, @@ -34,6 +38,12 @@ export default { GlCollapsibleListbox, }, inject: { + group: { + default: {}, + }, + restrictReassignmentToEnterprise: { + default: false, + }, allowInactivePlaceholderReassignment: { default: false, }, @@ -51,11 +61,17 @@ export default { isConfirmLoading: false, isCancelLoading: false, isNotifyLoading: false, - isLoadingInitial: true, - isLoadingMore: false, + apolloIsLoadingInitial: true, + apolloIsLoadingMore: false, isValidated: false, search: '', selectedUserToReassign: null, + enterpriseUsers: [], + enterpriseUsersPageInfo: { + nextPage: null, + }, + enterpriseUsersIsLoadingInitial: false, + enterpriseUsersIsLoadingMore: false, }; }, @@ -69,7 +85,10 @@ export default { }; }, result() { - this.isLoadingInitial = false; + this.apolloIsLoadingInitial = false; + }, + skip() { + return this.restrictReassignmentToEnterprise; }, error() { this.onError(); @@ -90,11 +109,35 @@ export default { }, hasNextPage() { + if (this.restrictReassignmentToEnterprise) { + return Boolean(this.enterpriseUsersPageInfo.nextPage); + } + return this.users?.pageInfo?.hasNextPage; }, isLoading() { - return this.$apollo.queries.users.loading && !this.isLoadingMore; + if (this.restrictReassignmentToEnterprise) { + return this.enterpriseUsersIsLoadingInitial; + } + + return this.$apollo.queries.users.loading && !this.apolloIsLoadingMore; + }, + + isLoadingMore() { + if (this.restrictReassignmentToEnterprise) { + return this.enterpriseUsersIsLoadingMore; + } + + return this.apolloIsLoadingMore; + }, + + isLoadingInitial() { + if (this.restrictReassignmentToEnterprise) { + return this.enterpriseUsersIsLoadingInitial; + } + + return this.apolloIsLoadingInitial; }, userSelectInvalid() { @@ -102,6 +145,10 @@ export default { }, userItems() { + if (this.restrictReassignmentToEnterprise) { + return this.enterpriseUsers?.map((user) => this.createUserObjectFromEnterprise(user)); + } + return this.users?.nodes?.map((user) => createUserObject(user)); }, @@ -146,10 +193,45 @@ export default { }, methods: { + async fetchEnterpriseUsers(page) { + try { + const { data, headers } = await fetchGroupEnterpriseUsers(this.group.id, { + page, + per_page: USERS_PER_PAGE, + search: this.search, + }); + + this.enterpriseUsersPageInfo = parseIntPagination(normalizeHeaders(headers)); + this.enterpriseUsers.push(...data); + } catch (error) { + this.onError(); + } + }, + async loadInitialEnterpriseUsers() { + if (!this.restrictReassignmentToEnterprise || this.enterpriseUsers.length > 0) { + return; + } + + this.enterpriseUsersIsLoadingInitial = true; + await this.fetchEnterpriseUsers(1); + this.enterpriseUsersIsLoadingInitial = false; + }, + + async loadMoreEnterpriseUsers() { + this.enterpriseUsersIsLoadingMore = true; + await this.fetchEnterpriseUsers(this.enterpriseUsersPageInfo.nextPage); + this.enterpriseUsersIsLoadingMore = false; + }, + async loadMoreUsers() { if (!this.hasNextPage) return; - this.isLoadingMore = true; + if (this.restrictReassignmentToEnterprise) { + this.loadMoreEnterpriseUsers(); + return; + } + + this.apolloIsLoadingMore = true; try { await this.$apollo.queries.users.fetchMore({ @@ -166,7 +248,7 @@ export default { } catch (error) { this.onError(); } finally { - this.isLoadingMore = false; + this.apolloIsLoadingMore = false; } }, @@ -178,6 +260,32 @@ export default { setSearch(searchTerm) { this.search = searchTerm; + + if (this.restrictReassignmentToEnterprise) { + this.enterpriseUsers = []; + this.loadInitialEnterpriseUsers(); + } + }, + + createUserObjectFromEnterprise({ + id, + username, + web_url: webUrl, + web_path: webPath, + avatar_url: avatarUrl, + name, + }) { + const gid = `gid://gitlab/User/${id}`; + + return { + username, + webUrl, + webPath, + avatarUrl, + id: gid, + text: name, + value: gid, + }; }, onSelect(value) { @@ -301,6 +409,7 @@ export default { :searching="isLoading" infinite-scroll :infinite-scroll-loading="isLoadingMore" + @shown="loadInitialEnterpriseUsers" @search="debouncedSetSearch" @select="onSelect" @bottom-reached="loadMoreUsers" diff --git a/doc/user/group/import/direct_transfer_migrations.md b/doc/user/group/import/direct_transfer_migrations.md index f65c2264be0dd4c4476d1fbd7ce2f74edddb9f16..bc50c77ea2268e7c4d954bb2b322d1c9c3a5134f 100644 --- a/doc/user/group/import/direct_transfer_migrations.md +++ b/doc/user/group/import/direct_transfer_migrations.md @@ -122,6 +122,10 @@ has an existing membership in the destination namespace with a [higher role](../ the one being mapped, the membership is mapped as a direct membership instead. This ensures the member does not get elevated permissions. +[In GitLab 18.0 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/510673), +for top-level groups with at least one enterprise user, you can map +contributions and memberships only to [enterprise users](../../enterprise_user/_index.md). + {{< alert type="note" >}} There is a [known issue](_index.md#known-issues) affecting the mapping of shared memberships. diff --git a/ee/app/assets/javascripts/api/groups_api.js b/ee/app/assets/javascripts/api/groups_api.js index 6cfacfc990466efe3ae334716a01555b36f71b0e..51d7dbb9c2d3ad277c50f1df1879dc224f7f6628 100644 --- a/ee/app/assets/javascripts/api/groups_api.js +++ b/ee/app/assets/javascripts/api/groups_api.js @@ -10,6 +10,7 @@ const GROUPS_BILLABLE_MEMBERS_SINGLE_MEMBERSHIPS_PATH = '/api/:version/groups/:group_id/billable_members/:member_id/memberships'; const GROUPS_BILLABLE_MEMBERS_SINGLE_INDIRECT_MEMBERSHIPS_PATH = '/api/:version/groups/:group_id/billable_members/:member_id/indirect'; +const GROUPS_ENTERPRISE_USERS_PATH = '/api/:version/groups/:id/enterprise_users'; export const fetchBillableGroupMembersList = (namespaceId, options = {}) => { const url = buildApiUrl(GROUPS_BILLABLE_MEMBERS_PATH).replace(':id', namespaceId); @@ -50,6 +51,21 @@ export const removeBillableMemberFromGroup = (groupId, memberId) => { return axios.delete(url); }; +export const fetchGroupEnterpriseUsers = (groupId, options) => { + const url = buildApiUrl(GROUPS_ENTERPRISE_USERS_PATH).replace(':id', groupId); + const defaults = { + per_page: DEFAULT_PER_PAGE, + page: 1, + }; + + return axios.get(url, { + params: { + ...defaults, + ...options, + }, + }); +}; + export const updateGroupSettings = (id, settings) => { const url = buildApiUrl(GROUP_PATH).replace(':id', id); diff --git a/ee/app/helpers/ee/groups/group_members_helper.rb b/ee/app/helpers/ee/groups/group_members_helper.rb index a8e69018ea2ec312a59704652b0a99a9db17a96e..2ee025301714fb78e4678310fa44c17ba55ed387 100644 --- a/ee/app/helpers/ee/groups/group_members_helper.rb +++ b/ee/app/helpers/ee/groups/group_members_helper.rb @@ -32,7 +32,8 @@ def group_members_app_data( manage_member_roles_path: manage_member_roles_path(group), promotion_request: { enabled: member_promotion_management_enabled?, total_items: pending_members_count }, can_approve_access_requests: !::Namespaces::FreeUserCap::Enforcement.new(group.root_ancestor).reached_limit?, - namespace_user_limit: ::Namespaces::FreeUserCap.dashboard_limit + namespace_user_limit: ::Namespaces::FreeUserCap.dashboard_limit, + restrict_reassignment_to_enterprise: group.any_enterprise_users? }) end # rubocop:enable Metrics/ParameterLists diff --git a/ee/app/models/ee/namespace.rb b/ee/app/models/ee/namespace.rb index 2794a4bb7fbff981064f39af8012be33c2caa137..fff845981e3a58e57f67994929462cbf7ce5dda2 100644 --- a/ee/app/models/ee/namespace.rb +++ b/ee/app/models/ee/namespace.rb @@ -611,6 +611,10 @@ def domain_verification_available? ::Gitlab.com? && root? && licensed_feature_available?(:domain_verification) end + def any_enterprise_users? + domain_verification_available? && enterprise_users.any? + end + def enforce_ssh_certificates? root? && namespace_settings&.enforce_ssh_certificates? end diff --git a/ee/app/services/ee/import/source_users/reassign_service.rb b/ee/app/services/ee/import/source_users/reassign_service.rb index 83f9cbae0a5d6bb277004a0900f34019e1ebb55e..3620df17950754d911536b66226b21e0be96370b 100644 --- a/ee/app/services/ee/import/source_users/reassign_service.rb +++ b/ee/app/services/ee/import/source_users/reassign_service.rb @@ -14,7 +14,11 @@ def run_validations return validation_error if validation_error - error_invalid_assignee_due_to_sso_enforcement unless valid_assignee_if_sso_enforcement_is_applicable? + return error_invalid_assignee_due_to_sso_enforcement unless valid_assignee_if_sso_enforcement_is_applicable? + + return if valid_assignee_if_should_check_enterprise_users? + + error_invalid_assignee_due_to_enterprise_users_check end def valid_assignee_if_sso_enforcement_is_applicable? @@ -29,6 +33,24 @@ def error_invalid_assignee_due_to_sso_enforcement ) end + def valid_assignee_if_should_check_enterprise_users? + return true unless root_namespace.any_enterprise_users? + + assignee_user.managed_by_group?(root_namespace) + end + + def error_invalid_assignee_due_to_enterprise_users_check + ServiceResponse.error( + message: invalid_assignee_due_to_enterprise_users_check_message, + reason: :invalid_assignee, + payload: import_source_user + ) + end + + def invalid_assignee_due_to_enterprise_users_check_message + s_("UserMapping|You can assign only enterprise users in the top-level group you're importing to.") + end + def invalid_assignee_due_to_sso_enforcement_message s_('UserMapping|You can assign only users with linked SAML and SCIM identities. ' \ 'Ensure the user has signed into GitLab through your SAML SSO provider and has an ' \ diff --git a/ee/spec/frontend/members/placeholders/components/placeholder_actions_spec.js b/ee/spec/frontend/members/placeholders/components/placeholder_actions_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..549025dec96769f2c30da67559fcdb337660f013 --- /dev/null +++ b/ee/spec/frontend/members/placeholders/components/placeholder_actions_spec.js @@ -0,0 +1,156 @@ +import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import { GlCollapsibleListbox } from '@gitlab/ui'; +import { fetchGroupEnterpriseUsers } from 'ee_else_ce/api/groups_api'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import waitForPromises from 'helpers/wait_for_promises'; + +import PlaceholderActions from '~/members/placeholders/components/placeholder_actions.vue'; + +import { mockSourceUsers } from 'jest/members/placeholders/mock_data'; + +import { + mockEnterpriseUser1, + mockEnterpriseUser2, + mockEnterpriseUsersQueryResponse, + mockEnterpriseUsersWithPaginationQueryResponse, +} from '../mock_data'; + +Vue.use(VueApollo); +jest.mock('~/alert'); +jest.mock('ee_component/api/groups_api'); + +describe('PlaceholderActions', () => { + let wrapper; + + const defaultProps = { + sourceUser: mockSourceUsers[0], + }; + + const $toast = { + show: jest.fn(), + }; + + const createComponent = ({ provide = {}, props = {} } = {}) => { + wrapper = shallowMountExtended(PlaceholderActions, { + apolloProvider: createMockApollo(), + propsData: { + ...defaultProps, + ...props, + }, + provide: { + ...provide, + }, + mocks: { $toast }, + }); + }; + + const findListbox = () => wrapper.findComponent(GlCollapsibleListbox); + + afterEach(() => { + fetchGroupEnterpriseUsers.mockClear(); + }); + + describe('when restrictReassignmentToEnterprise is false', () => { + beforeEach(() => { + createComponent({ + provide: { restrictReassignmentToEnterprise: false }, + }); + }); + + it('does not call fetchGroupEnterpriseUsers on listbox `shown` event', async () => { + await findListbox().vm.$emit('shown'); + await waitForPromises(); + + expect(fetchGroupEnterpriseUsers).not.toHaveBeenCalled(); + }); + }); + + describe('when restrictReassignmentToEnterprise is true', () => { + beforeEach(() => { + createComponent({ + provide: { restrictReassignmentToEnterprise: true }, + }); + }); + + it('calls fetchGroupEnterpriseUsers on listbox `shown` event', async () => { + fetchGroupEnterpriseUsers.mockResolvedValueOnce( + mockEnterpriseUsersWithPaginationQueryResponse, + ); + + await findListbox().vm.$emit('shown'); + + expect(findListbox().props('loading')).toBe(true); + expect(fetchGroupEnterpriseUsers).toHaveBeenCalledTimes(1); + + await waitForPromises(); + + expect(findListbox().props('loading')).toBe(false); + expect(findListbox().props('items')).toHaveLength(1); + }); + + describe('when the component is opened multiple times', () => { + beforeEach(() => { + fetchGroupEnterpriseUsers.mockResolvedValueOnce( + mockEnterpriseUsersWithPaginationQueryResponse, + ); + }); + + it('does not call fetchGroupEnterpriseUsers again', async () => { + await findListbox().vm.$emit('shown'); + await waitForPromises(); + + await findListbox().vm.$emit('shown'); + await waitForPromises(); + + expect(fetchGroupEnterpriseUsers).toHaveBeenCalledTimes(1); + }); + }); + + describe('when users query succeeds and has pagination', () => { + beforeEach(async () => { + fetchGroupEnterpriseUsers.mockResolvedValueOnce( + mockEnterpriseUsersWithPaginationQueryResponse, + ); + fetchGroupEnterpriseUsers.mockResolvedValueOnce(mockEnterpriseUsersQueryResponse()); + + await findListbox().vm.$emit('shown'); + await waitForPromises(); + }); + + describe('when "bottom-reached" event is emitted', () => { + beforeEach(async () => { + await findListbox().vm.$emit('bottom-reached'); + }); + + it('calls fetchGroupEnterpriseUsers again to get next page', () => { + expect(fetchGroupEnterpriseUsers).toHaveBeenCalledTimes(2); + }); + + it('appends query results to "enterpriseUsers"', () => { + const allUsers = [mockEnterpriseUser2, mockEnterpriseUser1]; + + expect(findListbox().props('items')).toHaveLength(allUsers.length); + }); + }); + }); + + describe('when users query succeeds and does not have pagination', () => { + beforeEach(async () => { + await findListbox().vm.$emit('shown'); + await waitForPromises(); + }); + + describe('when "bottom-reached" event is emitted', () => { + beforeEach(async () => { + await findListbox().vm.$emit('bottom-reached'); + }); + + it('does not call fetchGroupEnterpriseUsers again', () => { + expect(fetchGroupEnterpriseUsers).toHaveBeenCalledTimes(1); + }); + }); + }); + }); +}); diff --git a/ee/spec/frontend/members/placeholders/mock_data.js b/ee/spec/frontend/members/placeholders/mock_data.js new file mode 100644 index 0000000000000000000000000000000000000000..1949ed121d2981b25f92aa6c5d5fcb76389335ff --- /dev/null +++ b/ee/spec/frontend/members/placeholders/mock_data.js @@ -0,0 +1,29 @@ +export const mockEnterpriseUser1 = { + username: 'Administrator', + web_url: '/root', + web_path: '/root', + avatar_url: '/avatar1', + id: 1, + name: 'Admin', +}; + +export const mockEnterpriseUser2 = { + username: 'Rookie', + web_url: '/rookie', + web_path: '/rookie', + avatar_url: '/avatar2', + id: 2, + name: 'Rookie', +}; + +export const mockEnterpriseUsersQueryResponse = ({ + enterpriseUser = mockEnterpriseUser1, +} = {}) => ({ + data: [enterpriseUser], + headers: { 'X-Next-Page': null }, +}); + +export const mockEnterpriseUsersWithPaginationQueryResponse = { + data: [mockEnterpriseUser2], + headers: { 'X-Next-Page': 2 }, +}; diff --git a/ee/spec/helpers/ee/groups/group_members_helper_spec.rb b/ee/spec/helpers/ee/groups/group_members_helper_spec.rb index 86e8fd7af7ff9755a0f2fa362fa6f77d01352972..15407734c554d7c808dea31ec18e44324e7c334e 100644 --- a/ee/spec/helpers/ee/groups/group_members_helper_spec.rb +++ b/ee/spec/helpers/ee/groups/group_members_helper_spec.rb @@ -55,6 +55,25 @@ expect(subject[:manage_member_roles_path]).to eq(admin_application_settings_roles_and_permissions_path) end + context 'when enterprise users exist for the group' do + let_it_be(:enterprise_user) { create(:user, enterprise_group: group) } + + before do + stub_licensed_features(domain_verification: true) + allow(::Gitlab).to receive(:com?).and_return(true) + end + + it 'sets the restrict_reassignment_to_enterprise flag to true' do + expect(subject[:restrict_reassignment_to_enterprise]).to be(true) + end + end + + context 'when enterprise users do not exist for the group' do + it 'sets the restrict_reassignment_to_enterprise flag to true' do + expect(subject[:restrict_reassignment_to_enterprise]).to be(false) + end + end + context 'adds `can_approve_access_requests`' do before do stub_ee_application_setting(dashboard_limit_enabled: true) diff --git a/ee/spec/models/ee/namespace_spec.rb b/ee/spec/models/ee/namespace_spec.rb index fab14c1993d668a833b460561961533c02439c27..c9abd4cb0ddfcbca2ef381b6833086cafca725b4 100644 --- a/ee/spec/models/ee/namespace_spec.rb +++ b/ee/spec/models/ee/namespace_spec.rb @@ -2303,6 +2303,42 @@ end end + describe '#any_enterprise_users?' do + let_it_be(:group) { create(:group) } + + subject { group.any_enterprise_users? } + + before do + stub_licensed_features(domain_verification: true) + allow(::Gitlab).to receive(:com?).and_return(true) + end + + context 'when there are enterprise users for the group' do + let_it_be(:enterprise_user) { create(:user, enterprise_group: group) } + + it { is_expected.to be(true) } + + context 'when domain_verification_available? is false' do + before do + stub_licensed_features(domain_verification: false) + end + + it { is_expected.to be(false) } + end + end + + context 'when there are enterprise users for another group' do + let_it_be(:other_group) { create(:group) } + let_it_be(:enterprise_user) { create(:user, enterprise_group: other_group) } + + it { is_expected.to be(false) } + end + + context 'when there are no enterprise users' do + it { is_expected.to be(false) } + end + end + describe '#enforce_ssh_certificates?' do let(:namespace) { create(:group) } diff --git a/ee/spec/services/ee/import/source_users/reassign_service_spec.rb b/ee/spec/services/ee/import/source_users/reassign_service_spec.rb index 7f6cc099b7c5fd9678176b53552568c5ff20b307..fc6aa17f02c1943f9495c7df0729d8336c1f31a7 100644 --- a/ee/spec/services/ee/import/source_users/reassign_service_spec.rb +++ b/ee/spec/services/ee/import/source_users/reassign_service_spec.rb @@ -88,5 +88,34 @@ end end end + + context 'for enterprise users check' do + before do + # Needed for managed_by_group? + stub_licensed_features(domain_verification: true) + allow(::Gitlab).to receive(:com?).and_return(true) + end + + context 'when there are enterprise users' do + let_it_be(:enterprise_user) { create(:user, enterprise_group: group) } + + context 'when the user is part of the enterprise group' do + let_it_be(:assignee_user) { create(:user, enterprise_group: group) } + + it_behaves_like 'success response' + end + + context 'when the user is not part of the enterprise group' do + let_it_be(:assignee_user) { create(:user, enterprise_group: nil) } + + it_behaves_like 'an error response', + error: "You can assign only enterprise users in the top-level group you're importing to." + end + end + + context 'when there are no enterprise users' do + it_behaves_like 'success response' + end + end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5def4e6796b42f77c659c2c702fabb0aec661bc3..960153f05eed1e5914b44845424c45cae9bcb13c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -65880,6 +65880,9 @@ msgstr "" msgid "UserMapping|You can assign active users with regular, auditor, or administrator access only." msgstr "" +msgid "UserMapping|You can assign only enterprise users in the top-level group you're importing to." +msgstr "" + msgid "UserMapping|You can assign only users with linked SAML and SCIM identities. Ensure the user has signed into GitLab through your SAML SSO provider and has an active SCIM identity for this group." msgstr ""