diff --git a/doc/administration/settings/account_and_limit_settings.md b/doc/administration/settings/account_and_limit_settings.md index cbd6b0f6a4bdcab80c0eca4430988b3c8cc7b092..f4daddc58ed73e331bb1c136bec0d6d441203546 100644 --- a/doc/administration/settings/account_and_limit_settings.md +++ b/doc/administration/settings/account_and_limit_settings.md @@ -248,6 +248,7 @@ DETAILS: **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. +> - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/125835) in GitLab 17.6. Feature flag `allow_top_level_group_owners_to_create_service_accounts` removed. 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. diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb index a5e5f29b9455624afc6959a2e0f5cc0c8ec4b8b5..b9e6b5a892c58ec9fe79dcfb199ff4b54c17d9ee 100644 --- a/ee/app/policies/ee/group_policy.rb +++ b/ee/app/policies/ee/group_policy.rb @@ -221,11 +221,7 @@ module GroupPolicy 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 + ::Gitlab::CurrentSettings.current_application_settings.allow_top_level_group_owners_to_create_service_accounts? end MemberRole.all_customizable_group_permissions.each do |ability| 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 deleted file mode 100644 index e079ffd813fd3e5e1561dc96e549125bfb779329..0000000000000000000000000000000000000000 --- a/ee/config/feature_flags/gitlab_com_derisk/allow_top_level_group_owners_to_create_service_accounts.yml +++ /dev/null @@ -1,9 +0,0 @@ ---- -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/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index 3581ef5502de74efad08ac32ff8b6a821d99e644..43764daf501a5dadcf618d1ab6d1d1c7b0076f49 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -3328,9 +3328,9 @@ def expect_private_group_permissions_as_if_non_member 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 + context 'when application setting 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) + stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: false) end it { is_expected.to be_allowed(:admin_service_accounts) } @@ -3341,83 +3341,59 @@ def expect_private_group_permissions_as_if_non_member 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) } + it { is_expected.to be_disallowed(:create_service_account) } + it { is_expected.to be_disallowed(:delete_service_account) } end end - context 'when allow_top_level_group_owners_to_create_service_accounts FF is enabled' do + context 'when application setting 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) + stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: true) end - 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_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_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) } - context 'when saas', :saas do - it { is_expected.to be_allowed(:admin_service_accounts) } - it { is_expected.to be_allowed(:admin_service_account_member) } + 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 application setting is enabled ' do + context 'when a trial is active' do before do - stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: true) + allow(group).to receive_messages(gitlab_subscription: nil) 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_allowed(:create_service_account) } it { is_expected.to be_allowed(:delete_service_account) } + 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_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) - 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_allowed(:create_service_account) } - it { is_expected.to be_allowed(:delete_service_account) } - end - - context 'for subgroup' do - let_it_be(:subgroup) { create(:group, :private, parent: group) } + context 'for subgroup' do + let_it_be(:subgroup) { create(:group, :private, parent: group) } - subject { described_class.new(current_user, subgroup) } + 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) } - 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) } 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 ef5554e393a5a27dfab098fdd1674eaf0bcc187b..7ba8fcef2a18f2c5e7ccc4a0fa8d79a36c0115dd 100644 --- a/ee/spec/requests/api/group_service_accounts_spec.rb +++ b/ee/spec/requests/api/group_service_accounts_spec.rb @@ -235,68 +235,47 @@ it_behaves_like "service account user creation" end - context 'when current user is an owner and feature is activated and allow top level setting activated' do + context 'when current user is an owner' do let_it_be(:user) { create(:user) } 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 - it_behaves_like "service account user creation" - end - - 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) } - - 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 - - it_behaves_like "service account user creation" - end + context 'when allow top level setting is activated' do + before do + stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: true) + end - 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 } + it_behaves_like "service account user creation" - 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 'when in GitLab.com', :saas do + let(:hosted_plan) { create(:ultimate_plan) } - it 'returns error' do - perform_request + before do + create(:gitlab_subscription, namespace: group, hosted_plan: hosted_plan) + stub_application_setting(check_namespace_plan: true) + 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.') - ) + it_behaves_like "service account user creation" + end end - end - 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 } + context 'when allow top level setting is deactivated' do + 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 + before do + stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: false) + end - it 'returns error' do - perform_request + it 'returns error' do + perform_request - 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.') - ) + 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 end @@ -449,41 +428,10 @@ it_behaves_like "service account user deletion" - 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) - - 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 - - it "is not available for group owners when allow top level group owners feature is disabled", :sidekiq_inline do - group.add_owner(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: 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 "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) - - 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 - it "is available for group owners when allow top level group owners application setting is enabled", :sidekiq_inline, :saas do group.add_owner(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(:no_content) @@ -492,7 +440,6 @@ 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) } 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 bc82bcc5e81fc62073b8b8377bf81e8f022ac0ac..79a4fa145167971b952bde173bd9dd46496f2772 100644 --- a/ee/spec/services/namespaces/service_accounts/create_service_spec.rb +++ b/ee/spec/services/namespaces/service_accounts/create_service_spec.rb @@ -129,61 +129,41 @@ context 'when group owner' do let_it_be(:current_user) { create(:user, owner_of: group) } - context 'when allow_top_level_group_owners_to_create_service_accounts FF is disabled' do + context 'when application setting is disabled' do before do - stub_feature_flags(allow_top_level_group_owners_to_create_service_accounts: false) + stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: false) end it_behaves_like 'service account creation failure' context 'when saas', :saas do - it_behaves_like 'service account creation success' do - let(:username_prefix) { "service_account_group_#{group.id}" } - end + it_behaves_like 'service account creation failure' end end - context 'when allow_top_level_group_owners_to_create_service_accounts FF is enabled' do + context 'when application setting is enabled' do before do - 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) end - 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' - - context 'when saas', :saas do - it_behaves_like 'service account creation failure' - end + it_behaves_like 'service account creation success' do + let(:username_prefix) { "service_account_group_#{group.id}" } end - 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 saas', :saas do it_behaves_like 'service account creation success' do let(:username_prefix) { "service_account_group_#{group.id}" } end + end - context 'when saas', :saas do - it_behaves_like 'service account creation success' do - let(:username_prefix) { "service_account_group_#{group.id}" } - end - end + # setting is only applicable for top level group + context 'when the group is subgroup' do + let(:namespace_id) { subgroup.id } - # 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' + context 'when saas', :saas do it_behaves_like 'service account creation failure' - - context 'when saas', :saas do - it_behaves_like 'service account creation failure' - 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 a573ebe3055696b2673421f19ce67fc42b0bde19..972fb4ead32c94e2f91bfcdecff83428b05d5fda 100644 --- a/ee/spec/services/namespaces/service_accounts/delete_service_spec.rb +++ b/ee/spec/services/namespaces/service_accounts/delete_service_spec.rb @@ -63,52 +63,34 @@ context 'when group owner' do let_it_be(:current_user) { create(:user, owner_of: group) } - context 'when allow_top_level_group_owners_to_create_service_accounts FF is disabled' do + context 'when setting is off' do before do - stub_feature_flags(allow_top_level_group_owners_to_create_service_accounts: false) + stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: false) end it_behaves_like 'service account deletion failure' context 'when saas', :saas do - it_behaves_like 'service account deletion is success' + it_behaves_like 'service account deletion failure' end end - context 'when allow_top_level_group_owners_to_create_service_accounts is enabled' do + context 'when setting is on' do before do - 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) end - context 'when setting is off' do - before do - stub_ee_application_setting(allow_top_level_group_owners_to_create_service_accounts: false) - end - - it_behaves_like 'service account deletion failure' - - context 'when saas', :saas do - it_behaves_like 'service account deletion failure' - end - end - - 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' + context 'when saas', :saas do it_behaves_like 'service account deletion is success' + end - context 'when saas', :saas do - it_behaves_like 'service account deletion is success' - end - - 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) } + 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) } - it_behaves_like 'service account deletion failure' - end + it_behaves_like 'service account deletion failure' end end end diff --git a/spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb b/spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb index bccddaa50a0d4b8c58a499eb539c29accefbe2a9..9fd57f80cbf6046703a7cabf6e64ae5ed3a6a71e 100644 --- a/spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb +++ b/spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb @@ -86,7 +86,7 @@ with_them do before do allow(request).to receive(:session).and_return(session) - stub_feature_flags(bypass_two_factor: false) + stub_feature_flags(by_pass_two_factor_for_current_session: false) stub_application_setting(require_two_factor_authentication: instance_level_enabled) allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(group_level_enabled) session[:provider_2FA] = provider_2FA