diff --git a/app/views/admin/application_settings/_account_and_limit.html.haml b/app/views/admin/application_settings/_account_and_limit.html.haml index 1aa1b2b9bfc67689c968dfdabfdba496b00048f8..6a06f581e2d17371022bc091d50d23c60698a372 100644 --- a/app/views/admin/application_settings/_account_and_limit.html.haml +++ b/app/views/admin/application_settings/_account_and_limit.html.haml @@ -32,6 +32,7 @@ = render 'admin/application_settings/require_personal_access_token_expiry', form: f = render_if_exists 'admin/application_settings/personal_access_token_expiration_policy', form: f = render_if_exists 'admin/application_settings/service_access_tokens_expiration_enforced', form: f + = render_if_exists 'admin/application_settings/allow_top_level_group_owners_to_create_service_accounts', form: f = render_if_exists 'admin/application_settings/ssh_key_expiration_policy', form: f .form-group diff --git a/db/migrate/20240816061320_allow_top_level_group_owners_to_create_service_accounts.rb b/db/migrate/20240816061320_allow_top_level_group_owners_to_create_service_accounts.rb new file mode 100644 index 0000000000000000000000000000000000000000..d5a060e97de9810c61b9cc3e628a43ccfc743274 --- /dev/null +++ b/db/migrate/20240816061320_allow_top_level_group_owners_to_create_service_accounts.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AllowTopLevelGroupOwnersToCreateServiceAccounts < Gitlab::Database::Migration[2.2] + milestone '17.5' + + enable_lock_retries! + + def change + add_column :application_settings, + :allow_top_level_group_owners_to_create_service_accounts, + :boolean, + default: false, + null: false + end +end diff --git a/db/schema_migrations/20240816061320 b/db/schema_migrations/20240816061320 new file mode 100644 index 0000000000000000000000000000000000000000..5383373a2b8f8054e3f26baf552fb81856ef16ff --- /dev/null +++ b/db/schema_migrations/20240816061320 @@ -0,0 +1 @@ +c6a5125a7c2f4642da7d6fa79d6b837e4f98902660194e48ca236ff6c1b9d8e8 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 671b11dd8d345726b16fe08e39d1306fc3884f25..82f309f6204c288cdecdb2f934e5bccf7a8a8d78 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -6379,6 +6379,7 @@ CREATE TABLE application_settings ( lock_spp_repository_pipeline_access boolean DEFAULT false NOT NULL, required_instance_ci_template text, enforce_ci_inbound_job_token_scope_enabled boolean DEFAULT false NOT NULL, + allow_top_level_group_owners_to_create_service_accounts boolean DEFAULT false NOT NULL, CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)), CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)), CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)), diff --git a/doc/administration/settings/account_and_limit_settings.md b/doc/administration/settings/account_and_limit_settings.md index 59b8cd5c7f0baa8a3e4b3d2b6a165364983cfe42..74792630eb8c3aa5c17bb064cd487025f6728690 100644 --- a/doc/administration/settings/account_and_limit_settings.md +++ b/doc/administration/settings/account_and_limit_settings.md @@ -238,6 +238,25 @@ When you require expiration dates for new access tokens: - Users must set an expiration date that does not exceed the allowed lifetime for new access tokens. - To control the maximum access token lifetime, use the [**Limit the lifetime of access tokens** setting](#limit-the-lifetime-of-access-tokens). +## Allow top-level group Owners to create service accounts + +DETAILS: +**Tier:** Premium, Ultimate +**Offering:** Self-managed + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/163726) in GitLab 17.5 [with a feature flag](../feature_flags.md) named `allow_top_level_group_owners_to_create_service_accounts` for GitLab self-managed. Disabled by default. + +FLAG: +On GitLab self-managed, by default this feature is not available. To make it available, an administrator can [enable the feature flag](../feature_flags.md) named `allow_top_level_group_owners_to_create_service_accounts`. On GitLab.com, this feature is available. + +By default, in GitLab self-managed, top-level group Owners can not create service accounts. GitLab administrators can allow top-level group Owners to create service accounts. + +1. On the left sidebar, at the bottom, select **Admin**. +1. Select **Settings > General**. +1. Expand **Account and limit**. +1. Under **Service account creation**, select the **Allow top-level group owners to create Service accounts** checkbox. +1. Select **Save changes**. + ## Limit the lifetime of SSH keys DETAILS: diff --git a/doc/security/hardening_nist_800_53.md b/doc/security/hardening_nist_800_53.md index 31e222a988948573a447d9e9713ce8e771ef6014..ff5b3441d815e0b9260da0d1f9c17b63a95e6e43 100644 --- a/doc/security/hardening_nist_800_53.md +++ b/doc/security/hardening_nist_800_53.md @@ -136,7 +136,7 @@ GitLab also supports the following user types for unique use cases: to projects by default and assisting administrators in identifying which users are not employed by the organization. -- [Service Accounts](../user/profile/service_accounts.md#self-managed-gitlab) - +- [Service Accounts](../user/profile/service_accounts.md#administrators-in-gitlab-self-managed) - Service accounts may be added to accommodate automated tasks. Service accounts do not use a seat under the license. diff --git a/doc/user/profile/service_accounts.md b/doc/user/profile/service_accounts.md index 88cad23e763af737034eb4d3930a7717c5adb0d4..7670b00e851767569f223f48b67f8df0355182c0 100644 --- a/doc/user/profile/service_accounts.md +++ b/doc/user/profile/service_accounts.md @@ -46,13 +46,23 @@ accounts allowed under your license: - On GitLab Premium, you can create one service account for every paid seat you have. - On GitLab Ultimate, you can create an unlimited number of service accounts. -How you create an account differs depending on whether you are on GitLab.com or self-managed. +How you create an account differs depending on whether you are a: -### GitLab.com +- Top-level group Owner. +- In GitLab self-managed, an administrator. + +### Top-level group Owners + +> - Introduced for GitLab.com in GitLab 16.3 +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/163726) in GitLab 17.5 [with a feature flag](../../administration/feature_flags.md) named `allow_top_level_group_owners_to_create_service_accounts` for GitLab Self-Managed. Disabled by default. + +FLAG: +On GitLab self-managed, by default this feature is not available. To make it available, an administrator can [enable the feature flag](../../administration/feature_flags.md) named `allow_top_level_group_owners_to_create_service_accounts`. On GitLab.com, this feature is available. Prerequisites: - You must have the Owner role in a top-level group. +- For GitLab self-managed, top-level group Owners must be [allowed to create service accounts](../../administration/settings/account_and_limit_settings.md#allow-top-level-group-owners-to-create-service-accounts). 1. [Create a service account](../../api/group_service_accounts.md#create-a-service-account-user). @@ -72,7 +82,10 @@ Prerequisites: 1. Make this service account a group or project member by [manually adding the service account user to the group or project](#add-a-service-account-to-subgroup-or-project). 1. Use the returned personal access token value to authenticate as the service account user. -### Self-managed GitLab +### Administrators in GitLab self-managed + +DETAILS: +**Offering:** Self-managed Prerequisites: @@ -112,7 +125,7 @@ There is no limit to the number of service accounts you can add to a project or A service account: - Can have different roles across multiple subgroups and projects of the same top level group. -- On GitLab.com, only belongs to one top-level group. +- When created by a top-level group owner, only belongs to one top-level group. ### Add to a subgroup or project @@ -141,8 +154,8 @@ For more information on the attributes, see the [API documentation on editing a Prerequisites: -- For GitLab.com, you must have the Owner role in a top-level group. -- For self-managed GitLab, you must be an administrator for your self-managed instance. +- For service accounts created by top-level group Owners, you must have the Owner role in the top-level group or be an administrator. +- For service accounts created by administrators, you must be an administrator for your self-managed instance. Use the groups API to [rotate the personal access token](../../api/group_service_accounts.md#rotate-a-personal-access-token-for-a-service-account-user) for a service account user. @@ -159,7 +172,7 @@ To revoke a personal access token, use the [personal access tokens API](../../ap ### Delete a service account -#### GitLab.com +#### Top-Level Group Owners Prerequisites: @@ -167,7 +180,7 @@ Prerequisites: To delete a service account, [use the service accounts API to delete the service account user](../../api/group_service_accounts.md#delete-a-service-account-user). -#### Self-managed GitLab +#### Administrators in GitLab self-managed Prerequisites: diff --git a/ee/app/controllers/ee/admin/application_settings_controller.rb b/ee/app/controllers/ee/admin/application_settings_controller.rb index 7df60b9521b3864998ef5d7df8a86a96f6f0bb0f..813c830b8494edcc5770c7b4b75640de5806512e 100644 --- a/ee/app/controllers/ee/admin/application_settings_controller.rb +++ b/ee/app/controllers/ee/admin/application_settings_controller.rb @@ -132,7 +132,8 @@ def visible_application_setting_attributes :lock_maven_package_requests_forwarding], default_branch_protection_restriction_in_groups: :group_owners_can_manage_default_branch_protection, group_ip_restriction: :globally_allowed_ips, - service_accounts: :service_access_tokens_expiration_enforced, + service_accounts: [:service_access_tokens_expiration_enforced, + :allow_top_level_group_owners_to_create_service_accounts], disable_personal_access_tokens: :disable_personal_access_tokens }.each do |license_feature, attribute_names| if License.feature_available?(license_feature) diff --git a/ee/app/helpers/ee/application_settings_helper.rb b/ee/app/helpers/ee/application_settings_helper.rb index c47bee24bc42c14908738b416a093cae96745fdb..b39e6db0de8e7b7d47fa398cb1c80addffc2b7ac 100644 --- a/ee/app/helpers/ee/application_settings_helper.rb +++ b/ee/app/helpers/ee/application_settings_helper.rb @@ -144,6 +144,7 @@ def self.possible_licensed_attributes globally_allowed_ips service_access_tokens_expiration_enforced disabled_direct_code_suggestions + allow_top_level_group_owners_to_create_service_accounts ] end diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb index edbad712bf554f1e4ef7069aa29a9f3864d3e5b1..9f1ea79e99948985ea550bcdd50df60754d0f7cb 100644 --- a/ee/app/policies/ee/group_policy.rb +++ b/ee/app/policies/ee/group_policy.rb @@ -218,6 +218,14 @@ module GroupPolicy ::Gitlab::Saas.feature_available?(:google_cloud_support) end + condition(:allow_top_level_group_owners_to_create_service_accounts, scope: :global) do + if ::Feature.enabled?(:allow_top_level_group_owners_to_create_service_accounts, @user) + ::Gitlab::CurrentSettings.current_application_settings.allow_top_level_group_owners_to_create_service_accounts? + else + is_gitlab_com? + end + end + MemberRole.all_customizable_group_permissions.each do |ability| desc "Custom role on group that enables #{ability.to_s.tr('_', ' ')}" condition("custom_role_enables_#{ability}".to_sym) do @@ -481,7 +489,7 @@ module GroupPolicy prevent :admin_group_member end - rule { service_accounts_available & ~has_parent & (admin | (is_gitlab_com & owner)) }.policy do + rule { service_accounts_available & ~has_parent & (admin | (allow_top_level_group_owners_to_create_service_accounts & owner)) }.policy do enable :create_service_account enable :delete_service_account end diff --git a/ee/app/views/admin/application_settings/_allow_top_level_group_owners_to_create_service_accounts.html.haml b/ee/app/views/admin/application_settings/_allow_top_level_group_owners_to_create_service_accounts.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..93a27b7aaf461a72fdf5f421c223687b29a5fa7a --- /dev/null +++ b/ee/app/views/admin/application_settings/_allow_top_level_group_owners_to_create_service_accounts.html.haml @@ -0,0 +1,6 @@ +- return unless License.feature_available?(:service_accounts) +- form = local_assigns.fetch(:form) + +.form-group + = form.label :allow_top_level_group_owners_to_create_service_accounts, _('Service account creation'), class: 'label-bold' + = form.gitlab_ui_checkbox_component :allow_top_level_group_owners_to_create_service_accounts, _('Allow top-level group owners to create Service accounts.') diff --git a/ee/config/feature_flags/gitlab_com_derisk/allow_top_level_group_owners_to_create_service_accounts.yml b/ee/config/feature_flags/gitlab_com_derisk/allow_top_level_group_owners_to_create_service_accounts.yml new file mode 100644 index 0000000000000000000000000000000000000000..e079ffd813fd3e5e1561dc96e549125bfb779329 --- /dev/null +++ b/ee/config/feature_flags/gitlab_com_derisk/allow_top_level_group_owners_to_create_service_accounts.yml @@ -0,0 +1,9 @@ +--- +name: allow_top_level_group_owners_to_create_service_accounts +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/468806 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/163726 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/482400 +milestone: '17.5' +group: group::authentication +type: gitlab_com_derisk +default_enabled: false diff --git a/ee/lib/ee/api/entities/application_setting.rb b/ee/lib/ee/api/entities/application_setting.rb index 41e486f54e68f620bec3d79906c269f908f37caf..0390dd85b2bc8bb5c65ed34e2811c01358b2452a 100644 --- a/ee/lib/ee/api/entities/application_setting.rb +++ b/ee/lib/ee/api/entities/application_setting.rb @@ -42,6 +42,7 @@ module ApplicationSetting expose :duo_features_enabled, if: ->(_instance, _opts) { ::License.ai_features_available? } expose :lock_duo_features_enabled, if: ->(_instance, _opts) { ::License.ai_features_available? } expose :disabled_direct_code_suggestions, if: ->(_instance, _opts) { CloudConnector::AvailableServices.find_by_name(:code_suggestions)&.purchased? } + expose :allow_top_level_group_owners_to_create_service_accounts, if: ->(_instance, _opts) { ::License.feature_available?(:service_accounts) } end end end diff --git a/ee/spec/controllers/admin/application_settings_controller_spec.rb b/ee/spec/controllers/admin/application_settings_controller_spec.rb index 4d918bda0e3cedf8a68d58902f77c1afeb7fe874..0b29342e8530dfb2b14a54929bc41a7b4c39e669 100644 --- a/ee/spec/controllers/admin/application_settings_controller_spec.rb +++ b/ee/spec/controllers/admin/application_settings_controller_spec.rb @@ -209,6 +209,13 @@ it_behaves_like 'settings for licensed features' end + context 'updating allow_top_level_group_owners_to_create_service_accounts setting' do + let(:settings) { { allow_top_level_group_owners_to_create_service_accounts: false } } + let(:feature) { :service_accounts } + + it_behaves_like 'settings for licensed features' + end + context 'updating `git_two_factor_session_expiry` setting' do before do stub_feature_flags(two_factor_for_cli: true) diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index e18f2e44bfd13854b90c93bae10268ec8bf29587..5cc83e5bc361dbd5861fb59f9637e0909f3584fe 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -3180,9 +3180,22 @@ def expect_private_group_permissions_as_if_non_member end describe 'admin_service_accounts' do - context 'when self-managed' do - context 'when the feature is not enabled' do - let(:current_user) { owner } + context 'when the feature is not enabled' do + let(:current_user) { owner } + + it { is_expected.to be_disallowed(:admin_service_accounts) } + it { is_expected.to be_disallowed(:admin_service_account_member) } + it { is_expected.to be_disallowed(:create_service_account) } + it { is_expected.to be_disallowed(:delete_service_account) } + end + + context 'when feature is enabled' do + before do + stub_licensed_features(service_accounts: true) + end + + context 'when the user is a maintainer' do + let(:current_user) { maintainer } it { is_expected.to be_disallowed(:admin_service_accounts) } it { is_expected.to be_disallowed(:admin_service_account_member) } @@ -3190,60 +3203,78 @@ def expect_private_group_permissions_as_if_non_member it { is_expected.to be_disallowed(:delete_service_account) } end - context 'when feature is enabled' do - before do - stub_licensed_features(service_accounts: true) - end - - context 'when the user is a maintainer' do - let(:current_user) { maintainer } - - it { is_expected.to be_disallowed(:admin_service_accounts) } - it { is_expected.to be_disallowed(:admin_service_account_member) } - it { is_expected.to be_disallowed(:create_service_account) } - it { is_expected.to be_disallowed(:delete_service_account) } - end + context 'when the user is an owner' do + let(:current_user) { owner } - context 'when the user is an owner' do - let(:current_user) { owner } + context 'when allow_top_level_group_owners_to_create_service_accounts is disabled' do + before do + stub_feature_flags(allow_top_level_group_owners_to_create_service_accounts: false) + end it { is_expected.to be_allowed(:admin_service_accounts) } it { is_expected.to be_allowed(:admin_service_account_member) } it { is_expected.to be_disallowed(:create_service_account) } it { is_expected.to be_disallowed(:delete_service_account) } - context 'when a trial is active' do - before do - allow(group).to receive_messages(gitlab_subscription: nil) - end - + context 'when saas', :saas do it { is_expected.to be_allowed(:admin_service_accounts) } it { is_expected.to be_allowed(:admin_service_account_member) } - it { is_expected.to be_disallowed(:create_service_account) } - it { is_expected.to be_disallowed(:delete_service_account) } + it { is_expected.to be_allowed(:create_service_account) } + it { is_expected.to be_allowed(:delete_service_account) } end + end - context 'for subgroup' do - let_it_be(:subgroup) { create(:group, :private, parent: group) } + context 'when allow_top_level_group_owners_to_create_service_accounts FF is enabled' do + before do + stub_feature_flags(allow_top_level_group_owners_to_create_service_accounts: true) + end - subject { described_class.new(current_user, subgroup) } + context 'when application setting is disabled' do + before do + stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: false) + end it { is_expected.to be_allowed(:admin_service_accounts) } it { is_expected.to be_allowed(:admin_service_account_member) } it { is_expected.to be_disallowed(:create_service_account) } it { is_expected.to be_disallowed(:delete_service_account) } + + context 'when saas', :saas do + it { is_expected.to be_allowed(:admin_service_accounts) } + it { is_expected.to be_allowed(:admin_service_account_member) } + it { is_expected.to be_disallowed(:create_service_account) } + it { is_expected.to be_disallowed(:delete_service_account) } + end end - end - context 'when the user is an instance admin' do - let(:current_user) { admin } + context 'when application setting is enabled ' do + before do + stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: true) + end - context 'when admin mode is enabled', :enable_admin_mode do it { is_expected.to be_allowed(:admin_service_accounts) } it { is_expected.to be_allowed(:admin_service_account_member) } it { is_expected.to be_allowed(:create_service_account) } it { is_expected.to be_allowed(:delete_service_account) } + context 'when saas', :saas do + it { is_expected.to be_allowed(:admin_service_accounts) } + it { is_expected.to be_allowed(:admin_service_account_member) } + it { is_expected.to be_allowed(:create_service_account) } + it { is_expected.to be_allowed(:delete_service_account) } + + context 'when trial is active' do + before do + allow(group).to receive_messages(trial_active?: true) + end + + it { is_expected.to be_disallowed(:admin_service_accounts) } + it { is_expected.to be_disallowed(:admin_service_account_member) } + it { is_expected.to be_disallowed(:create_service_account) } + it { is_expected.to be_disallowed(:delete_service_account) } + end + end + context 'when a trial is active' do before do allow(group).to receive_messages(gitlab_subscription: nil) @@ -3266,8 +3297,23 @@ def expect_private_group_permissions_as_if_non_member it { is_expected.to be_disallowed(:delete_service_account) } end end + end + + context 'for subgroup' do + let_it_be(:subgroup) { create(:group, :private, parent: group) } + + subject { described_class.new(current_user, subgroup) } + + it { is_expected.to be_allowed(:admin_service_accounts) } + it { is_expected.to be_allowed(:admin_service_account_member) } + it { is_expected.to be_disallowed(:create_service_account) } + it { is_expected.to be_disallowed(:delete_service_account) } + + context 'when a trial is active in GitLab.com', :saas do + before do + allow(subgroup.root_ancestor).to receive_messages(trial_active?: true) + end - context 'when admin mode is not enabled' do it { is_expected.to be_disallowed(:admin_service_accounts) } it { is_expected.to be_disallowed(:admin_service_account_member) } it { is_expected.to be_disallowed(:create_service_account) } @@ -3275,35 +3321,11 @@ def expect_private_group_permissions_as_if_non_member end end end - end - context 'when on GitLab.com', :saas do - context 'when the feature is not enabled' do - let(:current_user) { owner } - - it { is_expected.to be_disallowed(:admin_service_accounts) } - it { is_expected.to be_disallowed(:admin_service_account_member) } - it { is_expected.to be_disallowed(:create_service_account) } - it { is_expected.to be_disallowed(:delete_service_account) } - end - - context 'when feature is enabled' do - before do - stub_licensed_features(service_accounts: true) - end - - context 'when the user is a maintainer' do - let(:current_user) { maintainer } - - it { is_expected.to be_disallowed(:admin_service_accounts) } - it { is_expected.to be_disallowed(:admin_service_account_member) } - it { is_expected.to be_disallowed(:create_service_account) } - it { is_expected.to be_disallowed(:delete_service_account) } - end - - context 'when the user is an owner' do - let(:current_user) { owner } + context 'when the user is an instance admin' do + let(:current_user) { admin } + context 'when admin mode is enabled', :enable_admin_mode do it { is_expected.to be_allowed(:admin_service_accounts) } it { is_expected.to be_allowed(:admin_service_account_member) } it { is_expected.to be_allowed(:create_service_account) } @@ -3311,13 +3333,13 @@ def expect_private_group_permissions_as_if_non_member context 'when a trial is active' do before do - allow(group).to receive_messages(trial_active?: true) + allow(group).to receive_messages(gitlab_subscription: nil) end - it { is_expected.to be_disallowed(:admin_service_accounts) } - it { is_expected.to be_disallowed(:admin_service_account_member) } - it { is_expected.to be_disallowed(:create_service_account) } - it { is_expected.to be_disallowed(:delete_service_account) } + it { is_expected.to be_allowed(:admin_service_accounts) } + it { is_expected.to be_allowed(:admin_service_account_member) } + it { is_expected.to be_allowed(:create_service_account) } + it { is_expected.to be_allowed(:delete_service_account) } end context 'for subgroup' do @@ -3329,69 +3351,14 @@ def expect_private_group_permissions_as_if_non_member it { is_expected.to be_allowed(:admin_service_account_member) } it { is_expected.to be_disallowed(:create_service_account) } it { is_expected.to be_disallowed(:delete_service_account) } - - context 'when a trial is active' do - before do - allow(subgroup.root_ancestor).to receive_messages(trial_active?: true) - end - - it { is_expected.to be_disallowed(:admin_service_accounts) } - it { is_expected.to be_disallowed(:admin_service_account_member) } - it { is_expected.to be_disallowed(:create_service_account) } - it { is_expected.to be_disallowed(:delete_service_account) } - end end end - context 'when the user is an instance admin' do - let(:current_user) { admin } - - context 'when admin mode is enabled', :enable_admin_mode do - it { is_expected.to be_allowed(:admin_service_accounts) } - it { is_expected.to be_allowed(:admin_service_account_member) } - it { is_expected.to be_allowed(:create_service_account) } - it { is_expected.to be_allowed(:delete_service_account) } - - context 'when a trial is active' do - before do - allow(group).to receive_messages(trial_active?: true) - end - - it { is_expected.to be_disallowed(:admin_service_accounts) } - it { is_expected.to be_disallowed(:admin_service_account_member) } - it { is_expected.to be_disallowed(:create_service_account) } - it { is_expected.to be_disallowed(:delete_service_account) } - end - - context 'for subgroup' do - let_it_be(:subgroup) { create(:group, :private, parent: group) } - - subject { described_class.new(current_user, subgroup) } - - it { is_expected.to be_allowed(:admin_service_accounts) } - it { is_expected.to be_allowed(:admin_service_account_member) } - it { is_expected.to be_disallowed(:create_service_account) } - it { is_expected.to be_disallowed(:delete_service_account) } - - context 'when a trial is active' do - before do - allow(subgroup.root_ancestor).to receive_messages(trial_active?: true) - end - - it { is_expected.to be_disallowed(:admin_service_accounts) } - it { is_expected.to be_disallowed(:admin_service_account_member) } - it { is_expected.to be_disallowed(:create_service_account) } - it { is_expected.to be_disallowed(:delete_service_account) } - end - end - end - - context 'when admin mode is not enabled' do - it { is_expected.to be_disallowed(:admin_service_accounts) } - it { is_expected.to be_disallowed(:admin_service_account_member) } - it { is_expected.to be_disallowed(:create_service_account) } - it { is_expected.to be_disallowed(:delete_service_account) } - end + context 'when admin mode is not enabled' do + it { is_expected.to be_disallowed(:admin_service_accounts) } + it { is_expected.to be_disallowed(:admin_service_account_member) } + it { is_expected.to be_disallowed(:create_service_account) } + it { is_expected.to be_disallowed(:delete_service_account) } end end end diff --git a/ee/spec/requests/api/group_service_accounts_spec.rb b/ee/spec/requests/api/group_service_accounts_spec.rb index 84038db103fdb4bbabee874f2b7a3518c6648e5b..ef5554e393a5a27dfab098fdd1674eaf0bcc187b 100644 --- a/ee/spec/requests/api/group_service_accounts_spec.rb +++ b/ee/spec/requests/api/group_service_accounts_spec.rb @@ -121,206 +121,201 @@ end end - describe "POST /groups/:id/service_accounts" do - subject(:perform_request) { post api("/groups/#{group_id}/service_accounts", user), params: params } - - let_it_be(:params) { {} } - - context 'when Self-managed' do - before do - stub_licensed_features(service_accounts: true) - allow(License).to receive(:current).and_return(license) - end - - context 'when the feature is licensed' do - let(:license) { create(:license, plan: License::ULTIMATE_PLAN) } - - context 'when current user is an admin', :enable_admin_mode do - let_it_be(:user) { create(:admin) } + RSpec.shared_examples "service account user creation" do + context 'when the group exists' do + let(:group_id) { group.id } - context 'when the group exists' do - let(:group_id) { group.id } + it "creates user with service_account user type" do + perform_request - it "creates user with service_account user type" do - perform_request + expect(response).to have_gitlab_http_status(:created) + expect(json_response.keys).to match_array(%w[id name username]) - expect(response).to have_gitlab_http_status(:created) - expect(json_response.keys).to match_array(%w[id name username]) + user = User.find(json_response['id']) - user = User.find(json_response['id']) + expect(user.username).to start_with("service_account_group_#{group_id}") + expect(user.namespace.organization).to eq(organization) + expect(user.user_type).to eq('service_account') + end - expect(user.username).to start_with("service_account_group_#{group_id}") - expect(user.namespace.organization).to eq(organization) - expect(user.user_type).to eq('service_account') - end + context 'when params are provided' do + let_it_be(:params) do + { + name: 'John Doe', + username: 'test' + } + end - context 'for subgroup' do - let(:group_id) { subgroup.id } + it "creates user with provided details" do + perform_request - it 'returns error' do - perform_request + expect(response).to have_gitlab_http_status(:created) + expect(json_response['username']).to eq(params[:username]) + expect(json_response['name']).to eq(params[:name]) + expect(json_response.keys).to match_array(%w[id name username]) + end - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to include( - s_('ServiceAccount|User does not have permission to create a service account in this namespace.') - ) - end - end + context 'when user with the username already exists' do + before do + post api("/groups/#{group_id}/service_accounts", user), params: params end - context 'when the group does not exist' do - let(:group_id) { non_existing_record_id } - - it "returns error" do - perform_request + it 'returns error' do + perform_request - expect(response).to have_gitlab_http_status(:not_found) - end + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to include('Username has already been taken') end end - end - end - context 'when on GitLab.com', :saas do - before do - stub_licensed_features(service_accounts: true) - create(:gitlab_subscription, namespace: group, hosted_plan: hosted_plan) - stub_application_setting(check_namespace_plan: true) - end + context 'when the group does not exist' do + let(:group_id) { non_existing_record_id } - context 'when the feature is licensed' do - let(:hosted_plan) { create(:ultimate_plan) } + it "returns error" do + perform_request - context 'when current user is an owner' do - before do - group.add_owner(user) + expect(response).to have_gitlab_http_status(:not_found) end + end + end - context 'when the group exists' do - let(:group_id) { group.id } + it "returns bad request when service returns bad request" do + allow_next_instance_of(::Namespaces::ServiceAccounts::CreateService) do |service| + allow(service).to receive(:execute).and_return( + ServiceResponse.error(message: message, reason: :bad_request) + ) + end - it "creates user with service_account user type" do - perform_request + perform_request - expect(response).to have_gitlab_http_status(:created) - expect(json_response.keys).to match_array(%w[id name username]) + expect(response).to have_gitlab_http_status(:bad_request) + end - user = User.find(json_response['id']) + context 'for subgroup' do + let(:group_id) { subgroup.id } - expect(user.username).to start_with("service_account_group_#{group_id}") - expect(user.namespace.organization).to eq(organization) - expect(user.user_type).to eq('service_account') - end + it 'returns error' do + perform_request - context 'when params are provided' do - let_it_be(:params) do - { - name: 'John Doe', - username: 'test' - } - end + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to include( + s_('ServiceAccount|User does not have permission to create a service account in this namespace.') + ) + end + end + end - it "creates user with provided details" do - perform_request + context 'when the group does not exist' do + let(:group_id) { non_existing_record_id } - expect(response).to have_gitlab_http_status(:created) - expect(json_response['username']).to eq(params[:username]) - expect(json_response['name']).to eq(params[:name]) - expect(json_response.keys).to match_array(%w[id name username]) - end + it "returns error" do + perform_request - context 'when user with the username already exists' do - before do - post api("/groups/#{group_id}/service_accounts", user), params: params - end + expect(response).to have_gitlab_http_status(:not_found) + end + end + end - it 'returns error' do - perform_request + describe "POST /groups/:id/service_accounts" do + subject(:perform_request) { post api("/groups/#{group_id}/service_accounts", user), params: params } - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to include('Username has already been taken') - end - end - end + let_it_be(:params) { {} } - it "returns bad request when service returns bad request" do - allow_next_instance_of(::Namespaces::ServiceAccounts::CreateService) do |service| - allow(service).to receive(:execute).and_return( - ServiceResponse.error(message: message, reason: :bad_request) - ) - end + before do + stub_licensed_features(service_accounts: true) + allow(License).to receive(:current).and_return(license) + end - perform_request + context 'when the feature is licensed' do + let(:license) { create(:license, plan: License::ULTIMATE_PLAN) } - expect(response).to have_gitlab_http_status(:bad_request) - end + context 'when current user is an admin', :enable_admin_mode do + let_it_be(:user) { create(:admin) } - context 'for subgroup' do - let(:group_id) { subgroup.id } + it_behaves_like "service account user creation" + end - it 'returns error' do - perform_request + context 'when current user is an owner and feature is activated and allow top level setting activated' do + let_it_be(:user) { create(:user) } - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to include( - s_('ServiceAccount|User does not have permission to create a service account in this namespace.') - ) - end - end - end + before do + group.add_owner(user) + stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: true) + stub_feature_flags(allow_top_level_group_owners_to_create_service_accounts: true) + end - context 'when the group does not exist' do - let(:group_id) { non_existing_record_id } + it_behaves_like "service account user creation" + end - it "returns error" do - perform_request + context 'when current user is an owner and feature is deactivated in GitLab.com', :saas do + let_it_be(:user) { create(:user) } + let(:hosted_plan) { create(:ultimate_plan) } - expect(response).to have_gitlab_http_status(:not_found) - end - end + before do + group.add_owner(user) + stub_feature_flags(allow_top_level_group_owners_to_create_service_accounts: false) + create(:gitlab_subscription, namespace: group, hosted_plan: hosted_plan) + stub_application_setting(check_namespace_plan: true) end - context 'when user is not an owner' do - let(:group_id) { group.id } - - before do - group.add_maintainer(user) - end + it_behaves_like "service account user creation" + end - it "returns error" do - perform_request + context 'when current user is an owner and feature is deactivated and allow top level setting activated' do + let_it_be(:user) { create(:user) } + let(:group_id) { group.id } - expect(response).to have_gitlab_http_status(:forbidden) - end + before do + group.add_owner(user) + stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: true) + stub_feature_flags(allow_top_level_group_owners_to_create_service_accounts: false) end - context 'without authentication' do - let(:group_id) { group.id } - - it "returns error" do - post api("/groups/#{group_id}/service_accounts") + it 'returns error' do + perform_request - expect(response).to have_gitlab_http_status(:unauthorized) - end + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to include( + s_('ServiceAccount|User does not have permission to create a service account in this namespace.') + ) end end - context 'when the feature is not licensed' do - let(:hosted_plan) { create(:free_plan) } - + context 'when current user is an owner and allow top level setting deactivated' do + let_it_be(:user) { create(:user) } let(:group_id) { group.id } before do group.add_owner(user) + stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: false) end - it "returns error" do + it 'returns error' do perform_request - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to include( + s_('ServiceAccount|User does not have permission to create a service account in this namespace.') + ) end end end + + context 'when the feature is not licensed' do + let(:license) { nil } + let(:group_id) { group.id } + + before do + stub_licensed_features(service_accounts: false) + group.add_owner(user) + end + + it "returns error" do + perform_request + + expect(response).to have_gitlab_http_status(:forbidden) + end + end end describe "GET /groups/:id/service_accounts" do @@ -452,62 +447,70 @@ it_behaves_like 'DELETE request permissions for admin mode' - context 'when self-managed' do - it_behaves_like "service account user deletion" + it_behaves_like "service account user deletion" - it "is not available for non admin users" do - perform_enqueued_jobs { delete api(path, user) } + it "is not available for group owners when allow top level group owners feature is disabled on Self Managed", + :sidekiq_inline do + group.add_owner(user) - expect(response).to have_gitlab_http_status(:forbidden) - end + stub_feature_flags(allow_top_level_group_owners_to_create_service_accounts: false) + perform_enqueued_jobs { delete api(path, user) } + expect(response).to have_gitlab_http_status(:no_content) + expect(Users::GhostUserMigration.where(user: service_account_user, initiator_user: user)).not_to exist + end - context 'when feature is not licensed' do - let(:group_id) { group.id } + it "is not available for group owners when allow top level group owners feature is disabled", :sidekiq_inline do + group.add_owner(user) - before do - stub_licensed_features(service_accounts: false) - group.add_owner(user) - end + stub_feature_flags(allow_top_level_group_owners_to_create_service_accounts: true) + stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: false) + perform_enqueued_jobs { delete api(path, user) } + expect(response).to have_gitlab_http_status(:no_content) + expect(Users::GhostUserMigration.where(user: service_account_user, initiator_user: user)).not_to exist + end - it 'returns error' do - perform_enqueued_jobs { delete api(path, service_account_user) } + it "is available for group owners when allow top level group owners feature is disabled on GitLab.com", + :sidekiq_inline, :saas do + group.add_owner(user) - expect(response).to have_gitlab_http_status(:forbidden) - end - end + stub_feature_flags(allow_top_level_group_owners_to_create_service_accounts: false) + perform_enqueued_jobs { delete api(path, user) } + expect(response).to have_gitlab_http_status(:no_content) + expect(Users::GhostUserMigration.where(user: service_account_user, initiator_user: user)).to exist end - context 'when .com', :saas do - it_behaves_like "service account user deletion" + it "is available for group owners when allow top level group owners application setting is enabled", + :sidekiq_inline, :saas do + group.add_owner(user) - it "is available for group owners", :sidekiq_inline do - group.add_owner(user) - - perform_enqueued_jobs { delete api(path, user) } - expect(response).to have_gitlab_http_status(:no_content) - expect(Users::GhostUserMigration.where(user: service_account_user, initiator_user: user)).to exist - end + stub_feature_flags(allow_top_level_group_owners_to_create_service_accounts: true) + stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: true) + perform_enqueued_jobs { delete api(path, user) } + expect(response).to have_gitlab_http_status(:no_content) + expect(Users::GhostUserMigration.where(user: service_account_user, initiator_user: user)).to exist + end - it "is not available to non group owners" do - group.add_maintainer(user) + it "is not available to non group owners" do + group.add_maintainer(user) + stub_feature_flags(allow_top_level_group_owners_to_create_service_accounts: true) + stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: true) - perform_enqueued_jobs { delete api(path, user) } - expect(response).to have_gitlab_http_status(:forbidden) - end + perform_enqueued_jobs { delete api(path, user) } + expect(response).to have_gitlab_http_status(:forbidden) + end - context 'when feature is not licensed' do - let(:group_id) { group.id } + context 'when feature is not licensed' do + let(:group_id) { group.id } - before do - stub_licensed_features(service_accounts: false) - group.add_owner(user) - end + before do + stub_licensed_features(service_accounts: false) + group.add_owner(user) + end - it 'returns error' do - perform_enqueued_jobs { delete api(path, service_account_user) } + it 'returns error' do + perform_enqueued_jobs { delete api(path, service_account_user) } - expect(response).to have_gitlab_http_status(:forbidden) - end + expect(response).to have_gitlab_http_status(:forbidden) end end end @@ -526,6 +529,7 @@ context 'when the feature is licensed' do let(:group_id) { group.id } + let(:hosted_plan) { create(:ultimate_plan) } before do stub_licensed_features(service_accounts: true) diff --git a/ee/spec/requests/api/settings_spec.rb b/ee/spec/requests/api/settings_spec.rb index 3985953ad4e4359a1de837f5bf3e446963cd67cd..e3c28e4004c9ed25ca6ce3d9c854882f0ae83e04 100644 --- a/ee/spec/requests/api/settings_spec.rb +++ b/ee/spec/requests/api/settings_spec.rb @@ -78,6 +78,34 @@ end end + context 'allow top_level group owners to create service accounts' do + let(:params) { { file_template_project_id: project.id, allow_top_level_group_owners_to_create_service_accounts: true } } + + it_behaves_like 'PUT request permissions for admin mode' + + subject(:api_request) do + put api(path, admin, admin_mode: true), params: params + end + + it 'sets setting when licensed feature is available' do + stub_licensed_features(service_accounts: true) + + api_request + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['allow_top_level_group_owners_to_create_service_accounts']).to eq(true) + end + + it 'does not set the value when licensed feature is not available' do + stub_licensed_features(service_accounts: false) + + api_request + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['allow_top_level_group_owners_to_create_service_accounts']).to eq(nil) + end + end + context 'with duo_features_enabled settings' do let(:params) { { duo_features_enabled: false, lock_duo_features_enabled: true } } diff --git a/ee/spec/services/namespaces/service_accounts/create_service_spec.rb b/ee/spec/services/namespaces/service_accounts/create_service_spec.rb index ef850699bdbc9798dc0e4f12af450ca5c4bb40b4..bc82bcc5e81fc62073b8b8377bf81e8f022ac0ac 100644 --- a/ee/spec/services/namespaces/service_accounts/create_service_spec.rb +++ b/ee/spec/services/namespaces/service_accounts/create_service_spec.rb @@ -129,119 +129,64 @@ context 'when group owner' do let_it_be(:current_user) { create(:user, owner_of: group) } - it_behaves_like 'service account creation failure' - end - end - end - - context 'when saas', :saas do - before do - stub_licensed_features(service_accounts: true) - create(:gitlab_subscription, namespace: group, hosted_plan: hosted_plan) - stub_application_setting(check_namespace_plan: true) - end - - shared_examples 'creates service accounts as per subscription' do - context 'when subscription is of free plan' do - let(:hosted_plan) { create(:free_plan) } - - it_behaves_like 'service account creation failure' - end - - context 'when subscription is ultimate tier' do - let(:hosted_plan) { create(:ultimate_plan) } - - it_behaves_like 'service account creation success' do - let(:username_prefix) { "service_account_group_#{group.id}" } - end - - it 'sets provisioned by group' do - result = service.execute - expect(result.payload.provisioned_by_group_id).to eq(group.id) - end - - context 'when the group is invalid' do - let(:namespace_id) { non_existing_record_id } - - it_behaves_like 'service account creation failure' - end - - context 'when the group is subgroup' do - let(:namespace_id) { subgroup.id } - - it_behaves_like 'service account creation failure' - end - end - - context 'when subscription is of premium tier' do - let_it_be(:hosted_plan) { create(:premium_plan) } - let_it_be(:service_account1) { create(:user, :service_account, provisioned_by_group_id: group.id) } - let_it_be(:service_account2) { create(:user, :service_account, provisioned_by_group_id: group.id) } - - context 'when premium seats are not available' do + context 'when allow_top_level_group_owners_to_create_service_accounts FF is disabled' do before do - group.gitlab_subscription.update!(seats: 1) + stub_feature_flags(allow_top_level_group_owners_to_create_service_accounts: false) end - it 'raises error' do - result = service.execute + it_behaves_like 'service account creation failure' - expect(result.status).to eq(:error) - expect(result.message).to include( - s_('ServiceAccount|No more seats are available to create Service Account User') - ) + context 'when saas', :saas do + it_behaves_like 'service account creation success' do + let(:username_prefix) { "service_account_group_#{group.id}" } + end end end - context 'when premium seats are available' do + context 'when allow_top_level_group_owners_to_create_service_accounts FF is enabled' do before do - group.gitlab_subscription.update!(seats: 4) - end - - it_behaves_like 'service account creation success' do - let(:username_prefix) { "service_account_group_#{group.id}" } - end - - it 'sets provisioned by group' do - result = service.execute - - expect(result.payload.provisioned_by_group_id).to eq(group.id) + stub_feature_flags(allow_top_level_group_owners_to_create_service_accounts: true) end - context 'when the group is invalid' do - let(:namespace_id) { non_existing_record_id } + context 'when application setting is disabled' do + before do + stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: false) + end it_behaves_like 'service account creation failure' - end - context 'when the group is subgroup' do - let(:namespace_id) { subgroup.id } - - it_behaves_like 'service account creation failure' + context 'when saas', :saas do + it_behaves_like 'service account creation failure' + end end - end - end - end - context 'when current user is an admin', :enable_admin_mode do - let_it_be(:current_user) { create(:admin) } + context 'when application setting is enabled' do + before do + stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: true) + end - it_behaves_like "creates service accounts as per subscription" - end + it_behaves_like 'service account creation success' do + let(:username_prefix) { "service_account_group_#{group.id}" } + end - context 'when current user is not an admin' do - let(:hosted_plan) { create(:ultimate_plan) } + context 'when saas', :saas do + it_behaves_like 'service account creation success' do + let(:username_prefix) { "service_account_group_#{group.id}" } + end + end - context 'when not a group owner' do - let_it_be(:current_user) { create(:user, maintainer_of: group) } + # setting is only applicable for top level group + context 'when the group is subgroup' do + let(:namespace_id) { subgroup.id } - it_behaves_like 'service account creation failure' - end + it_behaves_like 'service account creation failure' - context 'when group owner' do - let_it_be(:current_user) { create(:user, owner_of: group) } - - it_behaves_like "creates service accounts as per subscription" + context 'when saas', :saas do + it_behaves_like 'service account creation failure' + end + end + end + end end end end diff --git a/ee/spec/services/namespaces/service_accounts/delete_service_spec.rb b/ee/spec/services/namespaces/service_accounts/delete_service_spec.rb index 086a9294049997351a155b951edc08cfdbb6055d..a573ebe3055696b2673421f19ce67fc42b0bde19 100644 --- a/ee/spec/services/namespaces/service_accounts/delete_service_spec.rb +++ b/ee/spec/services/namespaces/service_accounts/delete_service_spec.rb @@ -63,55 +63,65 @@ context 'when group owner' do let_it_be(:current_user) { create(:user, owner_of: group) } - it_behaves_like 'service account deletion failure' - end - end - end - - context 'when saas', :saas do - before do - stub_licensed_features(service_accounts: true) - end - - context 'when current user is an admin' do - let_it_be(:current_user) { create(:admin) } + context 'when allow_top_level_group_owners_to_create_service_accounts FF is disabled' do + before do + stub_feature_flags(allow_top_level_group_owners_to_create_service_accounts: false) + end - context 'when admin mode is enabled', :enable_admin_mode do - it_behaves_like 'service account deletion is success' + it_behaves_like 'service account deletion failure' - context 'when user to be deleted is not of type service account' do - let_it_be(:delete_user) { create(:user) } + context 'when saas', :saas do + it_behaves_like 'service account deletion is success' + end + end - before_all do - group.add_maintainer(delete_user) + context 'when allow_top_level_group_owners_to_create_service_accounts is enabled' do + before do + stub_feature_flags(allow_top_level_group_owners_to_create_service_accounts: true) end - it_behaves_like 'service account deletion failure' - end - end - end + context 'when setting is off' do + before do + stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: false) + end - context 'when current user is not an admin' do - context 'when not a group owner' do - let_it_be(:current_user) { create(:user, maintainer_of: group) } + it_behaves_like 'service account deletion failure' - it_behaves_like 'service account deletion failure' - end + context 'when saas', :saas do + it_behaves_like 'service account deletion failure' + end + end - context 'when group owner' do - let_it_be(:current_user) { create(:user, owner_of: group) } + context 'when setting is on' do + before do + stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: true) + end + + it_behaves_like 'service account deletion is success' - it_behaves_like 'service account deletion is success' + context 'when saas', :saas do + it_behaves_like 'service account deletion is success' + end - context 'when user to be deleted is not of type service account' do - let_it_be(:delete_user) { create(:user) } + context 'when its a subgroup' do + let_it_be(:group) { create(:group, :private, parent: create(:group)) } + let_it_be(:delete_user) { create(:service_account, provisioned_by_group: group) } - before_all do - group.add_maintainer(delete_user) + it_behaves_like 'service account deletion failure' + end end + end + end - it_behaves_like 'service account deletion failure' + context 'when user to be deleted is not of type service account' do + let_it_be(:delete_user) { create(:user) } + let_it_be(:current_user) { create(:user, owner_of: group) } + + before_all do + group.add_maintainer(delete_user) end + + it_behaves_like 'service account deletion failure' end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b9a992fbaf8feece414fcf2f7e9756858d8c2805..f68f4ac60db92b36ffef08a3fe13613689d9089f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -5742,6 +5742,9 @@ msgstr "" msgid "Allow this key to push to this repository" msgstr "" +msgid "Allow top-level group owners to create Service accounts." +msgstr "" + msgid "Allow use of licensed EE features" msgstr "" @@ -51025,6 +51028,9 @@ msgstr "" msgid "Service account" msgstr "" +msgid "Service account creation" +msgstr "" + msgid "Service account generated successfully" msgstr ""