From b61bb1ae8eae7464addedee5c38ba3880403c4c5 Mon Sep 17 00:00:00 2001 From: Oiza Date: Wed, 23 Apr 2025 14:23:29 -0400 Subject: [PATCH 01/17] Create column and setting for confirmation bypass Adds columns to NamespaceSettings table. Creates the placeholder_user_confirmation_bypass feature for EE users. Changelog: added EE: true --- ...lder_confirmation_to_namespace_settings.rb | 20 +++++++++++++++++++ ee/app/models/ee/namespace_setting.rb | 11 ++++++++++ .../models/gitlab_subscriptions/features.rb | 1 + 3 files changed, 32 insertions(+) create mode 100644 db/migrate/20250421224751_add_allow_bypass_placeholder_confirmation_to_namespace_settings.rb diff --git a/db/migrate/20250421224751_add_allow_bypass_placeholder_confirmation_to_namespace_settings.rb b/db/migrate/20250421224751_add_allow_bypass_placeholder_confirmation_to_namespace_settings.rb new file mode 100644 index 00000000000000..958cd0bbbef31f --- /dev/null +++ b/db/migrate/20250421224751_add_allow_bypass_placeholder_confirmation_to_namespace_settings.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddAllowBypassPlaceholderConfirmationToNamespaceSettings < Gitlab::Database::Migration[2.2] + milestone '18.0' + disable_ddl_transaction! + + def up + add_column :namespace_settings, :allow_bypass_placeholder_confirmation, :boolean, default: false, null: false + add_column :namespace_settings, :allow_bypass_placeholder_confirmation_expires_at, :datetime_with_timezone, + null: true + end + + def down + remove_column :namespace_settings, :allow_bypass_placeholder_confirmation + remove_column :namespace_settings, :allow_bypass_placeholder_confirmation_expires_at + end +end diff --git a/ee/app/models/ee/namespace_setting.rb b/ee/app/models/ee/namespace_setting.rb index d4af2581ff1ca3..7fc0241c0a7fd6 100644 --- a/ee/app/models/ee/namespace_setting.rb +++ b/ee/app/models/ee/namespace_setting.rb @@ -227,6 +227,15 @@ def experiment_features_allowed errors.add(:experiment_features_enabled, _("Experiment features' settings not allowed.")) end + + def allow_bypass_placeholder_confirmation? + return false unless Feature.enabled?(:importer_user_mapping_allow_bypass_of_confirmation, namespace) + return false unless namespace.licensed_feature_available?(:placeholder_user_confirmation_bypass) + + allow_bypass_placeholder_confirmation && + (allow_bypass_placeholder_confirmation_expires_at.nil? || + allow_bypass_placeholder_confirmation_expires_at.future?) + end end class_methods do @@ -249,6 +258,8 @@ def experiment_features_allowed duo_features_enabled lock_duo_features_enabled enterprise_users_extensions_marketplace_opt_in_status + allow_bypass_placeholder_confirmation + allow_bypass_placeholder_confirmation_expires_at ].freeze override :allowed_namespace_settings_params diff --git a/ee/app/models/gitlab_subscriptions/features.rb b/ee/app/models/gitlab_subscriptions/features.rb index b9f8987a7d0215..a7f588cdd97706 100644 --- a/ee/app/models/gitlab_subscriptions/features.rb +++ b/ee/app/models/gitlab_subscriptions/features.rb @@ -199,6 +199,7 @@ class Features review_merge_request board_status_lists disable_invite_members + placeholder_user_confirmation_bypass ].freeze ULTIMATE_FEATURES = %i[ -- GitLab From 414fd8387e40eeab7a1f57e456ef7980ff545622 Mon Sep 17 00:00:00 2001 From: Oiza Date: Wed, 23 Apr 2025 22:47:59 -0400 Subject: [PATCH 02/17] Update structure.sql file --- db/structure.sql | 9 ++++++--- ee/app/models/ee/namespace.rb | 3 +++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/db/structure.sql b/db/structure.sql index 728968be951c35..bcb6f3977a0828 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -4864,7 +4864,7 @@ PARTITION BY LIST (partition_id); CREATE TABLE p_ci_finished_build_ch_sync_events ( build_id bigint NOT NULL, - partition bigint DEFAULT 1 NOT NULL, + partition bigint DEFAULT 2 NOT NULL, build_finished_at timestamp without time zone NOT NULL, processed boolean DEFAULT false NOT NULL, project_id bigint NOT NULL @@ -4874,7 +4874,7 @@ PARTITION BY LIST (partition); CREATE TABLE p_ci_finished_pipeline_ch_sync_events ( pipeline_id bigint NOT NULL, project_namespace_id bigint NOT NULL, - partition bigint DEFAULT 1 NOT NULL, + partition bigint DEFAULT 2 NOT NULL, pipeline_finished_at timestamp without time zone NOT NULL, processed boolean DEFAULT false NOT NULL ) @@ -10563,6 +10563,7 @@ CREATE TABLE bulk_import_entities ( has_failures boolean DEFAULT false, migrate_memberships boolean DEFAULT true NOT NULL, organization_id bigint, + migrate_banned_contributions boolean DEFAULT false NOT NULL, CONSTRAINT check_13f279f7da CHECK ((char_length(source_full_path) <= 255)), CONSTRAINT check_469f9235c5 CHECK ((num_nonnulls(namespace_id, organization_id, project_id) = 1)), CONSTRAINT check_715d725ea2 CHECK ((char_length(destination_name) <= 255)), @@ -18095,6 +18096,8 @@ CREATE TABLE namespace_settings ( disable_invite_members boolean DEFAULT false NOT NULL, web_based_commit_signing_enabled boolean, lock_web_based_commit_signing_enabled boolean DEFAULT false NOT NULL, + allow_bypass_placeholder_confirmation boolean DEFAULT false NOT NULL, + allow_bypass_placeholder_confirmation_expires_at timestamp with time zone, CONSTRAINT check_0ba93c78c7 CHECK ((char_length(default_branch_name) <= 255)), CONSTRAINT check_namespace_settings_security_policies_is_hash CHECK ((jsonb_typeof(security_policies) = 'object'::text)), CONSTRAINT namespace_settings_unique_project_download_limit_alertlist_size CHECK ((cardinality(unique_project_download_limit_alertlist) <= 100)), @@ -24503,8 +24506,8 @@ CREATE TABLE user_preferences ( dpop_enabled boolean DEFAULT false NOT NULL, use_work_items_view boolean DEFAULT false NOT NULL, text_editor_type smallint DEFAULT 0 NOT NULL, - merge_request_dashboard_list_type smallint DEFAULT 0 NOT NULL, extensions_marketplace_opt_in_url text, + merge_request_dashboard_list_type smallint DEFAULT 0 NOT NULL, CONSTRAINT check_1d670edc68 CHECK ((time_display_relative IS NOT NULL)), CONSTRAINT check_89bf269f41 CHECK ((char_length(diffs_deletion_color) <= 7)), CONSTRAINT check_9b50d9f942 CHECK ((char_length(extensions_marketplace_opt_in_url) <= 512)), diff --git a/ee/app/models/ee/namespace.rb b/ee/app/models/ee/namespace.rb index 6b62763fd2d527..97a0f4abce38cb 100644 --- a/ee/app/models/ee/namespace.rb +++ b/ee/app/models/ee/namespace.rb @@ -209,6 +209,9 @@ module Namespace delegate :security_policy_management_project, to: :security_orchestration_policy_configuration, allow_nil: true + delegate :allow_bypass_placeholder_confirmation, :allow_bypass_placeholder_confirmation_expires_at, + to: :namespace_settings + before_create :sync_membership_lock_with_parent # Changing the plan or other details may invalidate this cache -- GitLab From 643840f0f568bb908534fc3328dd9dd79b686a0d Mon Sep 17 00:00:00 2001 From: Oiza Date: Wed, 23 Apr 2025 22:51:16 -0400 Subject: [PATCH 03/17] Clean up migration files --- ...ow_bypass_placeholder_confirmation_to_namespace_settings.rb | 3 --- db/schema_migrations/20250421224751 | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) create mode 100644 db/schema_migrations/20250421224751 diff --git a/db/migrate/20250421224751_add_allow_bypass_placeholder_confirmation_to_namespace_settings.rb b/db/migrate/20250421224751_add_allow_bypass_placeholder_confirmation_to_namespace_settings.rb index 958cd0bbbef31f..0945620bd6d2bb 100644 --- a/db/migrate/20250421224751_add_allow_bypass_placeholder_confirmation_to_namespace_settings.rb +++ b/db/migrate/20250421224751_add_allow_bypass_placeholder_confirmation_to_namespace_settings.rb @@ -1,8 +1,5 @@ # frozen_string_literal: true -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - class AddAllowBypassPlaceholderConfirmationToNamespaceSettings < Gitlab::Database::Migration[2.2] milestone '18.0' disable_ddl_transaction! diff --git a/db/schema_migrations/20250421224751 b/db/schema_migrations/20250421224751 new file mode 100644 index 00000000000000..b596565f348ee3 --- /dev/null +++ b/db/schema_migrations/20250421224751 @@ -0,0 +1 @@ +b0f92ce5d13897123b53be2d31fb27a0a2913303d6921de629598e1db91781e8 \ No newline at end of file -- GitLab From 7819ed9a4249b60ddf1599f6e75e41704afb280c Mon Sep 17 00:00:00 2001 From: Oiza Date: Thu, 24 Apr 2025 15:42:48 -0400 Subject: [PATCH 04/17] Test change to model --- ee/app/models/ee/namespace_setting.rb | 18 +++--- ee/spec/models/ee/namespace_spec.rb | 2 + ee/spec/models/namespace_setting_spec.rb | 77 ++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 9 deletions(-) diff --git a/ee/app/models/ee/namespace_setting.rb b/ee/app/models/ee/namespace_setting.rb index 7fc0241c0a7fd6..3cbc1749273d06 100644 --- a/ee/app/models/ee/namespace_setting.rb +++ b/ee/app/models/ee/namespace_setting.rb @@ -173,6 +173,15 @@ def duo_core_features_enabled=(value) self.duo_nano_features_enabled = value end + def allow_bypass_placeholder_confirmation? + return false unless ::Feature.enabled?(:importer_user_mapping_allow_bypass_of_confirmation, namespace) + return false unless namespace.licensed_feature_available?(:placeholder_user_confirmation_bypass) + + allow_bypass_placeholder_confirmation && + (allow_bypass_placeholder_confirmation_expires_at.nil? || + allow_bypass_placeholder_confirmation_expires_at.future?) + end + private def trigger_todo_creation @@ -227,15 +236,6 @@ def experiment_features_allowed errors.add(:experiment_features_enabled, _("Experiment features' settings not allowed.")) end - - def allow_bypass_placeholder_confirmation? - return false unless Feature.enabled?(:importer_user_mapping_allow_bypass_of_confirmation, namespace) - return false unless namespace.licensed_feature_available?(:placeholder_user_confirmation_bypass) - - allow_bypass_placeholder_confirmation && - (allow_bypass_placeholder_confirmation_expires_at.nil? || - allow_bypass_placeholder_confirmation_expires_at.future?) - end end class_methods do diff --git a/ee/spec/models/ee/namespace_spec.rb b/ee/spec/models/ee/namespace_spec.rb index 9aee54e0be9df1..8bd1161e8e4674 100644 --- a/ee/spec/models/ee/namespace_spec.rb +++ b/ee/spec/models/ee/namespace_spec.rb @@ -47,6 +47,8 @@ it { is_expected.to delegate_method(:lock_duo_features_enabled).to(:namespace_settings) } it { is_expected.to delegate_method(:duo_availability).to(:namespace_settings) } it { is_expected.to delegate_method(:security_policy_management_project).to(:security_orchestration_policy_configuration) } + it { is_expected.to delegate_method(:allow_bypass_placeholder_confirmation).to(:namespace_settings) } + it { is_expected.to delegate_method(:allow_bypass_placeholder_confirmation_expires_at).to(:namespace_settings) } shared_examples 'plan helper' do |namespace_plan| let(:namespace) { create(:namespace_with_plan, plan: "#{plan_name}_plan") } diff --git a/ee/spec/models/namespace_setting_spec.rb b/ee/spec/models/namespace_setting_spec.rb index feccde4aa479eb..ba0e0d828ddaeb 100644 --- a/ee/spec/models/namespace_setting_spec.rb +++ b/ee/spec/models/namespace_setting_spec.rb @@ -847,4 +847,81 @@ it { is_expected.to eq(expected_opt_in_status) } end end + + describe '#allow_bypass_placeholder_confirmation' do + context 'when feature flag is disabled' do + before do + stub_feature_flags(importer_user_mapping_allow_bypass_of_confirmation: false) + end + + it 'returns false' do + setting.allow_bypass_placeholder_confirmation = true + + expect(setting.allow_bypass_placeholder_confirmation?).to be_falsey + end + end + + context 'when license does not include the feature' do + before do + stub_feature_flags(importer_user_mapping_allow_bypass_of_confirmation: true) + allow(group).to receive(:licensed_feature_available?).with(:placeholder_user_confirmation_bypass).and_return(false) + end + + it 'returns false' do + setting.allow_bypass_placeholder_confirmation = true + + expect(setting.allow_bypass_placeholder_confirmation?).to be_falsey + end + end + + context 'when feature flag and license are available' do + before do + stub_feature_flags(importer_user_mapping_allow_bypass_of_confirmation: true) + allow(group).to receive(:licensed_feature_available?).with(:placeholder_user_confirmation_bypass).and_return(true) + end + + context 'when setting is disabled' do + before do + setting.allow_bypass_placeholder_confirmation = false + end + + it 'returns false' do + expect(setting.allow_bypass_placeholder_confirmation?).to be_falsey + end + end + + context 'when setting is enabled without expiry' do + before do + setting.allow_bypass_placeholder_confirmation = true + setting.allow_bypass_placeholder_confirmation_expires_at = nil + end + + it 'returns true' do + expect(setting.allow_bypass_placeholder_confirmation?).to be_truthy + end + end + + context 'when setting is enabled with future expiry' do + before do + setting.allow_bypass_placeholder_confirmation = true + setting.allow_bypass_placeholder_confirmation_expires_at = 1.day.from_now + end + + it 'returns true' do + expect(setting.allow_bypass_placeholder_confirmation?).to be_truthy + end + end + + context 'when setting is enabled with past expiry' do + before do + setting.allow_bypass_placeholder_confirmation = true + setting.allow_bypass_placeholder_confirmation_expires_at = 1.day.ago + end + + it 'returns false' do + expect(setting.allow_bypass_placeholder_confirmation?).to be_falsey + end + end + end + end end -- GitLab From c05059029db9785465371842a3e415078a8f1c43 Mon Sep 17 00:00:00 2001 From: Oiza Date: Fri, 25 Apr 2025 00:15:10 -0400 Subject: [PATCH 05/17] Ensure namespace_settings bypass params are accepted --- ee/app/controllers/concerns/ee/groups/params.rb | 5 +++++ ee/app/models/ee/group.rb | 1 + 2 files changed, 6 insertions(+) diff --git a/ee/app/controllers/concerns/ee/groups/params.rb b/ee/app/controllers/concerns/ee/groups/params.rb index ba2f8572bbedbc..58be96b5ea9f28 100644 --- a/ee/app/controllers/concerns/ee/groups/params.rb +++ b/ee/app/controllers/concerns/ee/groups/params.rb @@ -94,6 +94,11 @@ def group_params_ee can?(current_user, :owner_access, current_group) params_ee << :disable_invite_members end + + if ::Feature.enabled?(:importer_user_mapping_allow_bypass_of_confirmation, current_group) && + current_group&.licensed_feature_available?(:placeholder_user_confirmation_bypass) + params_ee << { namespace_settings_attributes: [:id, :allow_bypass_placeholder_confirmation] } + end end end # rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/AbcSize diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index 2bab0d1b69795f..57bcf0d863f8b6 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -42,6 +42,7 @@ module Group has_one :analytics_dashboards_pointer, class_name: 'Analytics::DashboardsPointer', foreign_key: :namespace_id accepts_nested_attributes_for :analytics_dashboards_pointer, allow_destroy: true accepts_nested_attributes_for :value_stream_dashboard_aggregation, update_only: true + accepts_nested_attributes_for :namespace_settings has_one :analytics_dashboards_configuration_project, through: :analytics_dashboards_pointer, source: :target_project # rubocop:disable Cop/ActiveRecordDependent -- legacy usage has_one :index_status, class_name: 'Elastic::GroupIndexStatus', foreign_key: :namespace_id, dependent: :destroy -- GitLab From 6c34bf98bd414dea2c9ca2ae1a13384123ff9fa0 Mon Sep 17 00:00:00 2001 From: Oiza Date: Sun, 27 Apr 2025 17:36:33 -0400 Subject: [PATCH 06/17] Modify ee/groups/params to include expiry date --- ee/app/controllers/concerns/ee/groups/params.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ee/app/controllers/concerns/ee/groups/params.rb b/ee/app/controllers/concerns/ee/groups/params.rb index 58be96b5ea9f28..42f3c468831503 100644 --- a/ee/app/controllers/concerns/ee/groups/params.rb +++ b/ee/app/controllers/concerns/ee/groups/params.rb @@ -97,7 +97,8 @@ def group_params_ee if ::Feature.enabled?(:importer_user_mapping_allow_bypass_of_confirmation, current_group) && current_group&.licensed_feature_available?(:placeholder_user_confirmation_bypass) - params_ee << { namespace_settings_attributes: [:id, :allow_bypass_placeholder_confirmation] } + params_ee << { namespace_settings_attributes: [:id, :allow_bypass_placeholder_confirmation, + :allow_bypass_placeholder_confirmation_expires_at] } end end end -- GitLab From ca3e76cc1466e7e4293af8340907b365d0478b2a Mon Sep 17 00:00:00 2001 From: Oiza Date: Sun, 27 Apr 2025 18:49:54 -0400 Subject: [PATCH 07/17] Update groups docs, address migration error --- doc/api/groups.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/api/groups.md b/doc/api/groups.md index 311ba45ee8346d..e565051f55f0cc 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -1602,6 +1602,8 @@ PUT /groups/:id | `duo_features_enabled` | boolean | no | Indicates whether GitLab Duo features are enabled for this group. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144931) in GitLab 16.10. GitLab Self-Managed, Premium and Ultimate only. | | `lock_duo_features_enabled` | boolean | no | Indicates whether the GitLab Duo features enabled setting is enforced for all subgroups. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144931) in GitLab 16.10. GitLab Self-Managed, Premium and Ultimate only. | | `max_artifacts_size` | integer | No | The maximum file size in megabytes for individual job artifacts. | +| `allow_bypass_placeholder_confirmation` | boolean | no | Indicates if user confirmation can be skipped while reassigning contributions from placeholder users. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/188810) in GitLab 18.0. GitLab Premium and Ultimate only. | +| `allow_bypass_placeholder_confirmation_expires_at` | string | no | Optional expiry date after which `allow_bypass_placeholder_confirmation` if `true` becomes `false`. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/188810) in GitLab 18.0. GitLab Premium and Ultimate only. | {{< alert type="note" >}} -- GitLab From 4df85beada9a878addba30aef3229c01def246d5 Mon Sep 17 00:00:00 2001 From: Oiza Date: Mon, 28 Apr 2025 18:49:04 -0400 Subject: [PATCH 08/17] Fixes db migration and column naming issues - Renames column to specify enterprise feature - Removed expiry column - Addresses redundant nested namespace_attributes --- ...lder_confirmation_to_namespace_settings.rb | 17 ------- ...on_for_enterprise_to_namespace_settings.rb | 10 ++++ db/schema_migrations/20250421224751 | 1 - db/schema_migrations/20250506035156 | 1 + db/structure.sql | 10 ++-- doc/api/groups.md | 3 +- .../controllers/concerns/ee/groups/params.rb | 5 +- ee/app/models/ee/group.rb | 1 - ee/app/models/ee/namespace.rb | 3 +- ee/app/models/ee/namespace_setting.rb | 11 ++-- .../models/gitlab_subscriptions/features.rb | 2 +- ee/app/services/ee/groups/update_service.rb | 3 +- ee/spec/models/ee/namespace_spec.rb | 3 +- ee/spec/models/namespace_setting_spec.rb | 51 ++++--------------- 14 files changed, 36 insertions(+), 85 deletions(-) delete mode 100644 db/migrate/20250421224751_add_allow_bypass_placeholder_confirmation_to_namespace_settings.rb create mode 100644 db/migrate/20250506035156_add_allow_bypass_placeholder_confirmation_for_enterprise_to_namespace_settings.rb delete mode 100644 db/schema_migrations/20250421224751 create mode 100644 db/schema_migrations/20250506035156 diff --git a/db/migrate/20250421224751_add_allow_bypass_placeholder_confirmation_to_namespace_settings.rb b/db/migrate/20250421224751_add_allow_bypass_placeholder_confirmation_to_namespace_settings.rb deleted file mode 100644 index 0945620bd6d2bb..00000000000000 --- a/db/migrate/20250421224751_add_allow_bypass_placeholder_confirmation_to_namespace_settings.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -class AddAllowBypassPlaceholderConfirmationToNamespaceSettings < Gitlab::Database::Migration[2.2] - milestone '18.0' - disable_ddl_transaction! - - def up - add_column :namespace_settings, :allow_bypass_placeholder_confirmation, :boolean, default: false, null: false - add_column :namespace_settings, :allow_bypass_placeholder_confirmation_expires_at, :datetime_with_timezone, - null: true - end - - def down - remove_column :namespace_settings, :allow_bypass_placeholder_confirmation - remove_column :namespace_settings, :allow_bypass_placeholder_confirmation_expires_at - end -end diff --git a/db/migrate/20250506035156_add_allow_bypass_placeholder_confirmation_for_enterprise_to_namespace_settings.rb b/db/migrate/20250506035156_add_allow_bypass_placeholder_confirmation_for_enterprise_to_namespace_settings.rb new file mode 100644 index 00000000000000..7a3c6b69ce3e7c --- /dev/null +++ b/db/migrate/20250506035156_add_allow_bypass_placeholder_confirmation_for_enterprise_to_namespace_settings.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class AddAllowBypassPlaceholderConfirmationForEnterpriseToNamespaceSettings < Gitlab::Database::Migration[2.3] + milestone '18.0' + + def change + add_column :namespace_settings, :allow_enterprise_bypass_placeholder_confirmation, :boolean, + default: false, null: false + end +end diff --git a/db/schema_migrations/20250421224751 b/db/schema_migrations/20250421224751 deleted file mode 100644 index b596565f348ee3..00000000000000 --- a/db/schema_migrations/20250421224751 +++ /dev/null @@ -1 +0,0 @@ -b0f92ce5d13897123b53be2d31fb27a0a2913303d6921de629598e1db91781e8 \ No newline at end of file diff --git a/db/schema_migrations/20250506035156 b/db/schema_migrations/20250506035156 new file mode 100644 index 00000000000000..e13a4fdcf789fe --- /dev/null +++ b/db/schema_migrations/20250506035156 @@ -0,0 +1 @@ +2244181d0870519e816e9f8735a44976b3740cc8a7d1a97f709b63d2acef0d7b \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index bcb6f3977a0828..ae78c012863793 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -4864,7 +4864,7 @@ PARTITION BY LIST (partition_id); CREATE TABLE p_ci_finished_build_ch_sync_events ( build_id bigint NOT NULL, - partition bigint DEFAULT 2 NOT NULL, + partition bigint DEFAULT 1 NOT NULL, build_finished_at timestamp without time zone NOT NULL, processed boolean DEFAULT false NOT NULL, project_id bigint NOT NULL @@ -4874,7 +4874,7 @@ PARTITION BY LIST (partition); CREATE TABLE p_ci_finished_pipeline_ch_sync_events ( pipeline_id bigint NOT NULL, project_namespace_id bigint NOT NULL, - partition bigint DEFAULT 2 NOT NULL, + partition bigint DEFAULT 1 NOT NULL, pipeline_finished_at timestamp without time zone NOT NULL, processed boolean DEFAULT false NOT NULL ) @@ -10563,7 +10563,6 @@ CREATE TABLE bulk_import_entities ( has_failures boolean DEFAULT false, migrate_memberships boolean DEFAULT true NOT NULL, organization_id bigint, - migrate_banned_contributions boolean DEFAULT false NOT NULL, CONSTRAINT check_13f279f7da CHECK ((char_length(source_full_path) <= 255)), CONSTRAINT check_469f9235c5 CHECK ((num_nonnulls(namespace_id, organization_id, project_id) = 1)), CONSTRAINT check_715d725ea2 CHECK ((char_length(destination_name) <= 255)), @@ -18096,8 +18095,7 @@ CREATE TABLE namespace_settings ( disable_invite_members boolean DEFAULT false NOT NULL, web_based_commit_signing_enabled boolean, lock_web_based_commit_signing_enabled boolean DEFAULT false NOT NULL, - allow_bypass_placeholder_confirmation boolean DEFAULT false NOT NULL, - allow_bypass_placeholder_confirmation_expires_at timestamp with time zone, + allow_enterprise_bypass_placeholder_confirmation boolean DEFAULT false NOT NULL, CONSTRAINT check_0ba93c78c7 CHECK ((char_length(default_branch_name) <= 255)), CONSTRAINT check_namespace_settings_security_policies_is_hash CHECK ((jsonb_typeof(security_policies) = 'object'::text)), CONSTRAINT namespace_settings_unique_project_download_limit_alertlist_size CHECK ((cardinality(unique_project_download_limit_alertlist) <= 100)), @@ -24506,8 +24504,8 @@ CREATE TABLE user_preferences ( dpop_enabled boolean DEFAULT false NOT NULL, use_work_items_view boolean DEFAULT false NOT NULL, text_editor_type smallint DEFAULT 0 NOT NULL, - extensions_marketplace_opt_in_url text, merge_request_dashboard_list_type smallint DEFAULT 0 NOT NULL, + extensions_marketplace_opt_in_url text, CONSTRAINT check_1d670edc68 CHECK ((time_display_relative IS NOT NULL)), CONSTRAINT check_89bf269f41 CHECK ((char_length(diffs_deletion_color) <= 7)), CONSTRAINT check_9b50d9f942 CHECK ((char_length(extensions_marketplace_opt_in_url) <= 512)), diff --git a/doc/api/groups.md b/doc/api/groups.md index e565051f55f0cc..eccc7d36710c35 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -1602,8 +1602,7 @@ PUT /groups/:id | `duo_features_enabled` | boolean | no | Indicates whether GitLab Duo features are enabled for this group. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144931) in GitLab 16.10. GitLab Self-Managed, Premium and Ultimate only. | | `lock_duo_features_enabled` | boolean | no | Indicates whether the GitLab Duo features enabled setting is enforced for all subgroups. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144931) in GitLab 16.10. GitLab Self-Managed, Premium and Ultimate only. | | `max_artifacts_size` | integer | No | The maximum file size in megabytes for individual job artifacts. | -| `allow_bypass_placeholder_confirmation` | boolean | no | Indicates if user confirmation can be skipped while reassigning contributions from placeholder users. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/188810) in GitLab 18.0. GitLab Premium and Ultimate only. | -| `allow_bypass_placeholder_confirmation_expires_at` | string | no | Optional expiry date after which `allow_bypass_placeholder_confirmation` if `true` becomes `false`. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/188810) in GitLab 18.0. GitLab Premium and Ultimate only. | +| `allow_enterprise_bypass_placeholder_confirmation` | boolean | no | Indicates if user confirmation can be skipped while reassigning contributions from placeholder users. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/188810) in GitLab 18.0. GitLab Enterprise Users only. | {{< alert type="note" >}} diff --git a/ee/app/controllers/concerns/ee/groups/params.rb b/ee/app/controllers/concerns/ee/groups/params.rb index 42f3c468831503..c6fba1ea9b7a19 100644 --- a/ee/app/controllers/concerns/ee/groups/params.rb +++ b/ee/app/controllers/concerns/ee/groups/params.rb @@ -96,9 +96,8 @@ def group_params_ee end if ::Feature.enabled?(:importer_user_mapping_allow_bypass_of_confirmation, current_group) && - current_group&.licensed_feature_available?(:placeholder_user_confirmation_bypass) - params_ee << { namespace_settings_attributes: [:id, :allow_bypass_placeholder_confirmation, - :allow_bypass_placeholder_confirmation_expires_at] } + current_group&.licensed_feature_available?(:placeholder_user_confirmation_bypass_for_enterprise) + params_ee + [:allow_enterprise_bypass_placeholder_confirmation] end end end diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index 57bcf0d863f8b6..2bab0d1b69795f 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -42,7 +42,6 @@ module Group has_one :analytics_dashboards_pointer, class_name: 'Analytics::DashboardsPointer', foreign_key: :namespace_id accepts_nested_attributes_for :analytics_dashboards_pointer, allow_destroy: true accepts_nested_attributes_for :value_stream_dashboard_aggregation, update_only: true - accepts_nested_attributes_for :namespace_settings has_one :analytics_dashboards_configuration_project, through: :analytics_dashboards_pointer, source: :target_project # rubocop:disable Cop/ActiveRecordDependent -- legacy usage has_one :index_status, class_name: 'Elastic::GroupIndexStatus', foreign_key: :namespace_id, dependent: :destroy diff --git a/ee/app/models/ee/namespace.rb b/ee/app/models/ee/namespace.rb index 97a0f4abce38cb..83c9871e194e58 100644 --- a/ee/app/models/ee/namespace.rb +++ b/ee/app/models/ee/namespace.rb @@ -209,8 +209,7 @@ module Namespace delegate :security_policy_management_project, to: :security_orchestration_policy_configuration, allow_nil: true - delegate :allow_bypass_placeholder_confirmation, :allow_bypass_placeholder_confirmation_expires_at, - to: :namespace_settings + delegate :allow_enterprise_bypass_placeholder_confirmation, to: :namespace_settings before_create :sync_membership_lock_with_parent diff --git a/ee/app/models/ee/namespace_setting.rb b/ee/app/models/ee/namespace_setting.rb index 3cbc1749273d06..2609b548b79e6e 100644 --- a/ee/app/models/ee/namespace_setting.rb +++ b/ee/app/models/ee/namespace_setting.rb @@ -173,13 +173,11 @@ def duo_core_features_enabled=(value) self.duo_nano_features_enabled = value end - def allow_bypass_placeholder_confirmation? + def allow_enterprise_bypass_placeholder_confirmation? return false unless ::Feature.enabled?(:importer_user_mapping_allow_bypass_of_confirmation, namespace) - return false unless namespace.licensed_feature_available?(:placeholder_user_confirmation_bypass) + return false unless namespace.licensed_feature_available?(:placeholder_user_confirmation_bypass_for_enterprise) - allow_bypass_placeholder_confirmation && - (allow_bypass_placeholder_confirmation_expires_at.nil? || - allow_bypass_placeholder_confirmation_expires_at.future?) + allow_enterprise_bypass_placeholder_confirmation end private @@ -258,8 +256,7 @@ def experiment_features_allowed duo_features_enabled lock_duo_features_enabled enterprise_users_extensions_marketplace_opt_in_status - allow_bypass_placeholder_confirmation - allow_bypass_placeholder_confirmation_expires_at + allow_enterprise_bypass_placeholder_confirmation ].freeze override :allowed_namespace_settings_params diff --git a/ee/app/models/gitlab_subscriptions/features.rb b/ee/app/models/gitlab_subscriptions/features.rb index a7f588cdd97706..d97dfeb1eaec18 100644 --- a/ee/app/models/gitlab_subscriptions/features.rb +++ b/ee/app/models/gitlab_subscriptions/features.rb @@ -199,7 +199,7 @@ class Features review_merge_request board_status_lists disable_invite_members - placeholder_user_confirmation_bypass + placeholder_user_confirmation_bypass_for_enterprise ].freeze ULTIMATE_FEATURES = %i[ diff --git a/ee/app/services/ee/groups/update_service.rb b/ee/app/services/ee/groups/update_service.rb index cb2ac7dbc53c1a..8103f70d647bff 100644 --- a/ee/app/services/ee/groups/update_service.rb +++ b/ee/app/services/ee/groups/update_service.rb @@ -7,7 +7,8 @@ module UpdateService EE_SETTINGS_PARAMS = [ :prevent_forking_outside_group, :remove_dormant_members, - :remove_dormant_members_period + :remove_dormant_members_period, + :allow_enterprise_bypass_placeholder_confirmation ].freeze override :execute diff --git a/ee/spec/models/ee/namespace_spec.rb b/ee/spec/models/ee/namespace_spec.rb index 8bd1161e8e4674..108b103c3a19ef 100644 --- a/ee/spec/models/ee/namespace_spec.rb +++ b/ee/spec/models/ee/namespace_spec.rb @@ -47,8 +47,7 @@ it { is_expected.to delegate_method(:lock_duo_features_enabled).to(:namespace_settings) } it { is_expected.to delegate_method(:duo_availability).to(:namespace_settings) } it { is_expected.to delegate_method(:security_policy_management_project).to(:security_orchestration_policy_configuration) } - it { is_expected.to delegate_method(:allow_bypass_placeholder_confirmation).to(:namespace_settings) } - it { is_expected.to delegate_method(:allow_bypass_placeholder_confirmation_expires_at).to(:namespace_settings) } + it { is_expected.to delegate_method(:allow_enterprise_bypass_placeholder_confirmation).to(:namespace_settings) } shared_examples 'plan helper' do |namespace_plan| let(:namespace) { create(:namespace_with_plan, plan: "#{plan_name}_plan") } diff --git a/ee/spec/models/namespace_setting_spec.rb b/ee/spec/models/namespace_setting_spec.rb index ba0e0d828ddaeb..cbfdea87c0ae14 100644 --- a/ee/spec/models/namespace_setting_spec.rb +++ b/ee/spec/models/namespace_setting_spec.rb @@ -848,78 +848,45 @@ end end - describe '#allow_bypass_placeholder_confirmation' do + describe '#allow_enterprise_bypass_placeholder_confirmation' do context 'when feature flag is disabled' do before do stub_feature_flags(importer_user_mapping_allow_bypass_of_confirmation: false) end it 'returns false' do - setting.allow_bypass_placeholder_confirmation = true + setting.allow_enterprise_bypass_placeholder_confirmation = true - expect(setting.allow_bypass_placeholder_confirmation?).to be_falsey + expect(setting.allow_enterprise_bypass_placeholder_confirmation?).to be_falsey end end context 'when license does not include the feature' do before do stub_feature_flags(importer_user_mapping_allow_bypass_of_confirmation: true) - allow(group).to receive(:licensed_feature_available?).with(:placeholder_user_confirmation_bypass).and_return(false) + allow(group).to receive(:licensed_feature_available?).with(:placeholder_user_confirmation_bypass_for_enterprise).and_return(false) end it 'returns false' do - setting.allow_bypass_placeholder_confirmation = true + setting.allow_enterprise_bypass_placeholder_confirmation = true - expect(setting.allow_bypass_placeholder_confirmation?).to be_falsey + expect(setting.allow_enterprise_bypass_placeholder_confirmation?).to be_falsey end end context 'when feature flag and license are available' do before do stub_feature_flags(importer_user_mapping_allow_bypass_of_confirmation: true) - allow(group).to receive(:licensed_feature_available?).with(:placeholder_user_confirmation_bypass).and_return(true) + allow(group).to receive(:licensed_feature_available?).with(:placeholder_user_confirmation_bypass_for_enterprise).and_return(true) end context 'when setting is disabled' do before do - setting.allow_bypass_placeholder_confirmation = false + setting.allow_enterprise_bypass_placeholder_confirmation = false end it 'returns false' do - expect(setting.allow_bypass_placeholder_confirmation?).to be_falsey - end - end - - context 'when setting is enabled without expiry' do - before do - setting.allow_bypass_placeholder_confirmation = true - setting.allow_bypass_placeholder_confirmation_expires_at = nil - end - - it 'returns true' do - expect(setting.allow_bypass_placeholder_confirmation?).to be_truthy - end - end - - context 'when setting is enabled with future expiry' do - before do - setting.allow_bypass_placeholder_confirmation = true - setting.allow_bypass_placeholder_confirmation_expires_at = 1.day.from_now - end - - it 'returns true' do - expect(setting.allow_bypass_placeholder_confirmation?).to be_truthy - end - end - - context 'when setting is enabled with past expiry' do - before do - setting.allow_bypass_placeholder_confirmation = true - setting.allow_bypass_placeholder_confirmation_expires_at = 1.day.ago - end - - it 'returns false' do - expect(setting.allow_bypass_placeholder_confirmation?).to be_falsey + expect(setting.allow_enterprise_bypass_placeholder_confirmation?).to be_falsey end end end -- GitLab From 368bdbaa4233723e77efe85d3034db98bf8b6f36 Mon Sep 17 00:00:00 2001 From: Oiza Date: Wed, 7 May 2025 01:19:16 -0400 Subject: [PATCH 09/17] Remove license feature check This feature will be available to enterprise users however the mode of validation needs further determination. The MR is simplified to just add the column and setting. --- .../controllers/concerns/ee/groups/params.rb | 3 +-- ee/app/models/ee/namespace_setting.rb | 2 +- .../models/gitlab_subscriptions/features.rb | 1 - ee/spec/models/namespace_setting_spec.rb | 26 ++++++++----------- 4 files changed, 13 insertions(+), 19 deletions(-) diff --git a/ee/app/controllers/concerns/ee/groups/params.rb b/ee/app/controllers/concerns/ee/groups/params.rb index c6fba1ea9b7a19..33618d205f76cd 100644 --- a/ee/app/controllers/concerns/ee/groups/params.rb +++ b/ee/app/controllers/concerns/ee/groups/params.rb @@ -95,8 +95,7 @@ def group_params_ee params_ee << :disable_invite_members end - if ::Feature.enabled?(:importer_user_mapping_allow_bypass_of_confirmation, current_group) && - current_group&.licensed_feature_available?(:placeholder_user_confirmation_bypass_for_enterprise) + if ::Feature.enabled?(:importer_user_mapping_allow_bypass_of_confirmation, current_group) params_ee + [:allow_enterprise_bypass_placeholder_confirmation] end end diff --git a/ee/app/models/ee/namespace_setting.rb b/ee/app/models/ee/namespace_setting.rb index 2609b548b79e6e..7fb16fba9194d4 100644 --- a/ee/app/models/ee/namespace_setting.rb +++ b/ee/app/models/ee/namespace_setting.rb @@ -56,6 +56,7 @@ module NamespaceSetting validates :remove_dormant_members, inclusion: { in: [false] }, if: :subgroup? validates :remove_dormant_members_period, numericality: { only_integer: true, greater_than_or_equal_to: 90, less_than_or_equal_to: 1827 } # 90d - ~5 years + validates :allow_enterprise_bypass_placeholder_confirmation, inclusion: { in: [true, false] } enum :enterprise_users_extensions_marketplace_opt_in_status, ::Enums::WebIde::ExtensionsMarketplaceOptInStatus.statuses, prefix: :enterprise_users_extensions_marketplace @@ -175,7 +176,6 @@ def duo_core_features_enabled=(value) def allow_enterprise_bypass_placeholder_confirmation? return false unless ::Feature.enabled?(:importer_user_mapping_allow_bypass_of_confirmation, namespace) - return false unless namespace.licensed_feature_available?(:placeholder_user_confirmation_bypass_for_enterprise) allow_enterprise_bypass_placeholder_confirmation end diff --git a/ee/app/models/gitlab_subscriptions/features.rb b/ee/app/models/gitlab_subscriptions/features.rb index d97dfeb1eaec18..b9f8987a7d0215 100644 --- a/ee/app/models/gitlab_subscriptions/features.rb +++ b/ee/app/models/gitlab_subscriptions/features.rb @@ -199,7 +199,6 @@ class Features review_merge_request board_status_lists disable_invite_members - placeholder_user_confirmation_bypass_for_enterprise ].freeze ULTIMATE_FEATURES = %i[ diff --git a/ee/spec/models/namespace_setting_spec.rb b/ee/spec/models/namespace_setting_spec.rb index cbfdea87c0ae14..4a982af4f5f06d 100644 --- a/ee/spec/models/namespace_setting_spec.rb +++ b/ee/spec/models/namespace_setting_spec.rb @@ -861,23 +861,9 @@ end end - context 'when license does not include the feature' do + context 'when feature flag is enabled' do before do stub_feature_flags(importer_user_mapping_allow_bypass_of_confirmation: true) - allow(group).to receive(:licensed_feature_available?).with(:placeholder_user_confirmation_bypass_for_enterprise).and_return(false) - end - - it 'returns false' do - setting.allow_enterprise_bypass_placeholder_confirmation = true - - expect(setting.allow_enterprise_bypass_placeholder_confirmation?).to be_falsey - end - end - - context 'when feature flag and license are available' do - before do - stub_feature_flags(importer_user_mapping_allow_bypass_of_confirmation: true) - allow(group).to receive(:licensed_feature_available?).with(:placeholder_user_confirmation_bypass_for_enterprise).and_return(true) end context 'when setting is disabled' do @@ -889,6 +875,16 @@ expect(setting.allow_enterprise_bypass_placeholder_confirmation?).to be_falsey end end + + context 'when setting is enabled' do + before do + setting.allow_enterprise_bypass_placeholder_confirmation = true + end + + it 'returns false' do + expect(setting.allow_enterprise_bypass_placeholder_confirmation?).to be_truthy + end + end end end end -- GitLab From c56495f03d41531a3c67d6ffbb70ddff72de7ba2 Mon Sep 17 00:00:00 2001 From: Oiza Date: Mon, 12 May 2025 12:10:40 -0400 Subject: [PATCH 10/17] Clarfy namespace_setting tests for FF enablement --- ee/spec/models/namespace_setting_spec.rb | 28 ++++++++++-------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/ee/spec/models/namespace_setting_spec.rb b/ee/spec/models/namespace_setting_spec.rb index 4a982af4f5f06d..5efb346f9270af 100644 --- a/ee/spec/models/namespace_setting_spec.rb +++ b/ee/spec/models/namespace_setting_spec.rb @@ -861,29 +861,23 @@ end end - context 'when feature flag is enabled' do + context 'when setting is disabled' do before do - stub_feature_flags(importer_user_mapping_allow_bypass_of_confirmation: true) + setting.allow_enterprise_bypass_placeholder_confirmation = false end - context 'when setting is disabled' do - before do - setting.allow_enterprise_bypass_placeholder_confirmation = false - end - - it 'returns false' do - expect(setting.allow_enterprise_bypass_placeholder_confirmation?).to be_falsey - end + it 'returns false' do + expect(setting.allow_enterprise_bypass_placeholder_confirmation?).to be_falsey end + end - context 'when setting is enabled' do - before do - setting.allow_enterprise_bypass_placeholder_confirmation = true - end + context 'when setting is enabled' do + before do + setting.allow_enterprise_bypass_placeholder_confirmation = true + end - it 'returns false' do - expect(setting.allow_enterprise_bypass_placeholder_confirmation?).to be_truthy - end + it 'returns true' do + expect(setting.allow_enterprise_bypass_placeholder_confirmation?).to be_truthy end end end -- GitLab From dc780a91d41cf1748115862bb9e59c9508c015c1 Mon Sep 17 00:00:00 2001 From: Oiza Date: Mon, 12 May 2025 19:43:29 -0400 Subject: [PATCH 11/17] Clean up permissions checks Change bypass to only be permitted if the group is a top-level group, and domain verification is available. --- .../controllers/concerns/ee/groups/params.rb | 10 ++-- ee/app/models/ee/namespace_setting.rb | 6 --- ee/spec/models/namespace_setting_spec.rb | 34 ------------- ee/spec/requests/groups_controller_spec.rb | 50 +++++++++++++++++++ 4 files changed, 57 insertions(+), 43 deletions(-) diff --git a/ee/app/controllers/concerns/ee/groups/params.rb b/ee/app/controllers/concerns/ee/groups/params.rb index 33618d205f76cd..9743d717f2df61 100644 --- a/ee/app/controllers/concerns/ee/groups/params.rb +++ b/ee/app/controllers/concerns/ee/groups/params.rb @@ -95,9 +95,7 @@ def group_params_ee params_ee << :disable_invite_members end - if ::Feature.enabled?(:importer_user_mapping_allow_bypass_of_confirmation, current_group) - params_ee + [:allow_enterprise_bypass_placeholder_confirmation] - end + params_ee << :allow_enterprise_bypass_placeholder_confirmation if enterprise_bypass_placeholders_allowed? end end # rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/AbcSize @@ -123,6 +121,12 @@ def licensed_ai_features_available? def current_group @group end + + def enterprise_bypass_placeholders_allowed? + ::Feature.enabled?(:importer_user_mapping_allow_bypass_of_confirmation, current_group) && + current_group&.root? && + current_group.domain_verification_available? + end end end end diff --git a/ee/app/models/ee/namespace_setting.rb b/ee/app/models/ee/namespace_setting.rb index 7fb16fba9194d4..f16cb6cd7ac3e2 100644 --- a/ee/app/models/ee/namespace_setting.rb +++ b/ee/app/models/ee/namespace_setting.rb @@ -174,12 +174,6 @@ def duo_core_features_enabled=(value) self.duo_nano_features_enabled = value end - def allow_enterprise_bypass_placeholder_confirmation? - return false unless ::Feature.enabled?(:importer_user_mapping_allow_bypass_of_confirmation, namespace) - - allow_enterprise_bypass_placeholder_confirmation - end - private def trigger_todo_creation diff --git a/ee/spec/models/namespace_setting_spec.rb b/ee/spec/models/namespace_setting_spec.rb index 5efb346f9270af..feccde4aa479eb 100644 --- a/ee/spec/models/namespace_setting_spec.rb +++ b/ee/spec/models/namespace_setting_spec.rb @@ -847,38 +847,4 @@ it { is_expected.to eq(expected_opt_in_status) } end end - - describe '#allow_enterprise_bypass_placeholder_confirmation' do - context 'when feature flag is disabled' do - before do - stub_feature_flags(importer_user_mapping_allow_bypass_of_confirmation: false) - end - - it 'returns false' do - setting.allow_enterprise_bypass_placeholder_confirmation = true - - expect(setting.allow_enterprise_bypass_placeholder_confirmation?).to be_falsey - end - end - - context 'when setting is disabled' do - before do - setting.allow_enterprise_bypass_placeholder_confirmation = false - end - - it 'returns false' do - expect(setting.allow_enterprise_bypass_placeholder_confirmation?).to be_falsey - end - end - - context 'when setting is enabled' do - before do - setting.allow_enterprise_bypass_placeholder_confirmation = true - end - - it 'returns true' do - expect(setting.allow_enterprise_bypass_placeholder_confirmation?).to be_truthy - end - end - end end diff --git a/ee/spec/requests/groups_controller_spec.rb b/ee/spec/requests/groups_controller_spec.rb index 695db7ea48ec68..e3bcf96ba4c888 100644 --- a/ee/spec/requests/groups_controller_spec.rb +++ b/ee/spec/requests/groups_controller_spec.rb @@ -644,6 +644,56 @@ end end end + + context 'when setting allow_enterprise_bypass_placeholder_confirmation' do + let(:params) { { group: { allow_enterprise_bypass_placeholder_confirmation: true } } } + + context 'when group is a top level group' do + context 'when user is a group owner' do + before do + group.add_owner(user) + allow_any_instance_of(Group).to receive(:domain_verification_available?).and_return(true) + stub_feature_flags(importer_user_mapping_allow_bypass_of_confirmation: group) + end + + it 'successfully changes the column' do + expect { request }.to change { + group.reload.namespace_settings.allow_enterprise_bypass_placeholder_confirmation? + }.from(false).to(true) + expect(response).to have_gitlab_http_status(:found) + end + end + + context 'when user is not group owner' do + before do + group.owners.delete(user) + group.add_maintainer(user) + allow_any_instance_of(Group).to receive(:domain_verification_available?).and_return(true) + end + + it 'does not change the column and returns not found' do + expect { request }.not_to change { + group.reload.namespace_settings.allow_enterprise_bypass_placeholder_confirmation? + } + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when group is not a top level group' do + let(:group) { create(:group, :nested) } + + it 'does not change the column and returns not found' do + expect(group).not_to receive(:domain_verification_available?) + expect(group.namespace_settings.allow_enterprise_bypass_placeholder_confirmation?).to be(false) + + request + + expect(response).to have_gitlab_http_status(:found) + expect(group.namespace_settings.allow_enterprise_bypass_placeholder_confirmation?).to be(false) + end + end + end end describe 'PUT #transfer', :saas do -- GitLab From a7ed090d48fe9a4789b03c960797160c69ce18f9 Mon Sep 17 00:00:00 2001 From: Oiza Date: Mon, 12 May 2025 19:52:30 -0400 Subject: [PATCH 12/17] Remove api doc update --- doc/api/groups.md | 1 - 1 file changed, 1 deletion(-) diff --git a/doc/api/groups.md b/doc/api/groups.md index eccc7d36710c35..311ba45ee8346d 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -1602,7 +1602,6 @@ PUT /groups/:id | `duo_features_enabled` | boolean | no | Indicates whether GitLab Duo features are enabled for this group. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144931) in GitLab 16.10. GitLab Self-Managed, Premium and Ultimate only. | | `lock_duo_features_enabled` | boolean | no | Indicates whether the GitLab Duo features enabled setting is enforced for all subgroups. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144931) in GitLab 16.10. GitLab Self-Managed, Premium and Ultimate only. | | `max_artifacts_size` | integer | No | The maximum file size in megabytes for individual job artifacts. | -| `allow_enterprise_bypass_placeholder_confirmation` | boolean | no | Indicates if user confirmation can be skipped while reassigning contributions from placeholder users. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/188810) in GitLab 18.0. GitLab Enterprise Users only. | {{< alert type="note" >}} -- GitLab From 59e6346b02e461b62fcb3e60143ee83870827bbe Mon Sep 17 00:00:00 2001 From: Oiza Date: Mon, 12 May 2025 21:29:37 -0400 Subject: [PATCH 13/17] Add column to namespace_setting_auditor denylist --- .../lib/namespaces/namespace_setting_changes_auditor_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/lib/namespaces/namespace_setting_changes_auditor_spec.rb b/ee/spec/lib/namespaces/namespace_setting_changes_auditor_spec.rb index 98ccf9be2a811e..8e9b94ce9a76ea 100644 --- a/ee/spec/lib/namespaces/namespace_setting_changes_auditor_spec.rb +++ b/ee/spec/lib/namespaces/namespace_setting_changes_auditor_spec.rb @@ -127,7 +127,7 @@ jwt_ci_cd_job_token_enabled jwt_ci_cd_job_token_opted_out require_dpop_for_manage_api_endpoints disable_invite_members job_token_policies_enabled security_policies duo_nano_features_enabled lock_model_prompt_cache_enabled model_prompt_cache_enabled lock_web_based_commit_signing_enabled - web_based_commit_signing_enabled] + web_based_commit_signing_enabled allow_enterprise_bypass_placeholder_confirmation] columns_to_audit = Namespaces::NamespaceSettingChangesAuditor::EVENT_NAME_PER_COLUMN.keys.map(&:to_s) -- GitLab From 59613d9ffbd39ee441a0ac56241c8b47d758ab32 Mon Sep 17 00:00:00 2001 From: Sam Word Date: Fri, 16 May 2025 15:26:21 +0000 Subject: [PATCH 14/17] Refactored and added specs, updated delegation for consistency --- ee/app/models/ee/namespace.rb | 4 +- ee/spec/models/ee/namespace_spec.rb | 3 +- ee/spec/requests/groups_controller_spec.rb | 83 ++++++++++++++-------- 3 files changed, 58 insertions(+), 32 deletions(-) diff --git a/ee/app/models/ee/namespace.rb b/ee/app/models/ee/namespace.rb index 83c9871e194e58..a2d478daebb25f 100644 --- a/ee/app/models/ee/namespace.rb +++ b/ee/app/models/ee/namespace.rb @@ -209,7 +209,9 @@ module Namespace delegate :security_policy_management_project, to: :security_orchestration_policy_configuration, allow_nil: true - delegate :allow_enterprise_bypass_placeholder_confirmation, to: :namespace_settings + delegate :allow_enterprise_bypass_placeholder_confirmation, + :allow_enterprise_bypass_placeholder_confirmation=, + to: :namespace_settings, allow_nil: true before_create :sync_membership_lock_with_parent diff --git a/ee/spec/models/ee/namespace_spec.rb b/ee/spec/models/ee/namespace_spec.rb index 108b103c3a19ef..071309f59b9551 100644 --- a/ee/spec/models/ee/namespace_spec.rb +++ b/ee/spec/models/ee/namespace_spec.rb @@ -47,7 +47,8 @@ it { is_expected.to delegate_method(:lock_duo_features_enabled).to(:namespace_settings) } it { is_expected.to delegate_method(:duo_availability).to(:namespace_settings) } it { is_expected.to delegate_method(:security_policy_management_project).to(:security_orchestration_policy_configuration) } - it { is_expected.to delegate_method(:allow_enterprise_bypass_placeholder_confirmation).to(:namespace_settings) } + it { is_expected.to delegate_method(:allow_enterprise_bypass_placeholder_confirmation).to(:namespace_settings).allow_nil } + it { is_expected.to delegate_method(:allow_enterprise_bypass_placeholder_confirmation=).to(:namespace_settings).with_arguments(:args) } shared_examples 'plan helper' do |namespace_plan| let(:namespace) { create(:namespace_with_plan, plan: "#{plan_name}_plan") } diff --git a/ee/spec/requests/groups_controller_spec.rb b/ee/spec/requests/groups_controller_spec.rb index e3bcf96ba4c888..748fdd8c6d1fa8 100644 --- a/ee/spec/requests/groups_controller_spec.rb +++ b/ee/spec/requests/groups_controller_spec.rb @@ -648,49 +648,72 @@ context 'when setting allow_enterprise_bypass_placeholder_confirmation' do let(:params) { { group: { allow_enterprise_bypass_placeholder_confirmation: true } } } - context 'when group is a top level group' do - context 'when user is a group owner' do - before do - group.add_owner(user) - allow_any_instance_of(Group).to receive(:domain_verification_available?).and_return(true) - stub_feature_flags(importer_user_mapping_allow_bypass_of_confirmation: group) - end + before do + group.add_owner(user) + allow(Group).to receive(:find_by_full_path).and_return(group) + allow(group).to receive(:domain_verification_available?).and_return(true) + end - it 'successfully changes the column' do - expect { request }.to change { - group.reload.namespace_settings.allow_enterprise_bypass_placeholder_confirmation? - }.from(false).to(true) - expect(response).to have_gitlab_http_status(:found) - end + it 'successfully updates the setting for top-level group owners' do + expect { request }.to change { + group.reload.namespace_settings.allow_enterprise_bypass_placeholder_confirmation? + }.from(false).to(true) + + expect(response).to have_gitlab_http_status(:found) + end + + context 'and user is not a group owner' do + before do + group.owners.delete(user) + group.add_maintainer(user) end - context 'when user is not group owner' do - before do - group.owners.delete(user) - group.add_maintainer(user) - allow_any_instance_of(Group).to receive(:domain_verification_available?).and_return(true) - end + it 'does not change the setting and returns not found' do + expect { request }.not_to change { + group.reload.namespace_settings.allow_enterprise_bypass_placeholder_confirmation? + }.from(false) - it 'does not change the column and returns not found' do - expect { request }.not_to change { - group.reload.namespace_settings.allow_enterprise_bypass_placeholder_confirmation? - } - expect(response).to have_gitlab_http_status(:not_found) - end + expect(response).to have_gitlab_http_status(:not_found) end end context 'when group is not a top level group' do let(:group) { create(:group, :nested) } - it 'does not change the column and returns not found' do - expect(group).not_to receive(:domain_verification_available?) - expect(group.namespace_settings.allow_enterprise_bypass_placeholder_confirmation?).to be(false) + it 'does not change the setting' do + expect { request }.not_to change { + group.reload.namespace_settings.allow_enterprise_bypass_placeholder_confirmation? + }.from(false) - request + expect(response).to have_gitlab_http_status(:found) + end + end + + context 'when domain verification is not enabled' do + before do + allow(group).to receive(:domain_verification_available?).and_return(false) + end + + it 'does not change the setting' do + expect { request }.not_to change { + group.reload.namespace_settings.allow_enterprise_bypass_placeholder_confirmation? + }.from(false) + + expect(response).to have_gitlab_http_status(:found) + end + end + + context 'when the importer_user_mapping_allow_bypass_of_confirmation feature flag is disabled' do + before do + stub_feature_flags(importer_user_mapping_allow_bypass_of_confirmation: false) + end + + it 'does not change the setting' do + expect { request }.not_to change { + group.reload.namespace_settings.allow_enterprise_bypass_placeholder_confirmation? + }.from(false) expect(response).to have_gitlab_http_status(:found) - expect(group.namespace_settings.allow_enterprise_bypass_placeholder_confirmation?).to be(false) end end end -- GitLab From d8fbc7dec3892b76b5b5a3570607363cd755a9dc Mon Sep 17 00:00:00 2001 From: Sam Word Date: Thu, 22 May 2025 16:32:38 -0400 Subject: [PATCH 15/17] Removed .root? check b/c .domain_verification_available? includes it --- ee/app/controllers/concerns/ee/groups/params.rb | 1 - ee/spec/requests/groups_controller_spec.rb | 12 ------------ 2 files changed, 13 deletions(-) diff --git a/ee/app/controllers/concerns/ee/groups/params.rb b/ee/app/controllers/concerns/ee/groups/params.rb index 9743d717f2df61..dbcdf4c196c3d1 100644 --- a/ee/app/controllers/concerns/ee/groups/params.rb +++ b/ee/app/controllers/concerns/ee/groups/params.rb @@ -124,7 +124,6 @@ def current_group def enterprise_bypass_placeholders_allowed? ::Feature.enabled?(:importer_user_mapping_allow_bypass_of_confirmation, current_group) && - current_group&.root? && current_group.domain_verification_available? end end diff --git a/ee/spec/requests/groups_controller_spec.rb b/ee/spec/requests/groups_controller_spec.rb index 748fdd8c6d1fa8..44a212d73aed57 100644 --- a/ee/spec/requests/groups_controller_spec.rb +++ b/ee/spec/requests/groups_controller_spec.rb @@ -677,18 +677,6 @@ end end - context 'when group is not a top level group' do - let(:group) { create(:group, :nested) } - - it 'does not change the setting' do - expect { request }.not_to change { - group.reload.namespace_settings.allow_enterprise_bypass_placeholder_confirmation? - }.from(false) - - expect(response).to have_gitlab_http_status(:found) - end - end - context 'when domain verification is not enabled' do before do allow(group).to receive(:domain_verification_available?).and_return(false) -- GitLab From 245b352041304c35db1ae19693f5f2013f38aa04 Mon Sep 17 00:00:00 2001 From: Sam Word Date: Fri, 23 May 2025 14:48:24 -0400 Subject: [PATCH 16/17] Fixed specs --- ee/app/controllers/concerns/ee/groups/params.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/controllers/concerns/ee/groups/params.rb b/ee/app/controllers/concerns/ee/groups/params.rb index dbcdf4c196c3d1..f366b8f3d2841c 100644 --- a/ee/app/controllers/concerns/ee/groups/params.rb +++ b/ee/app/controllers/concerns/ee/groups/params.rb @@ -124,7 +124,7 @@ def current_group def enterprise_bypass_placeholders_allowed? ::Feature.enabled?(:importer_user_mapping_allow_bypass_of_confirmation, current_group) && - current_group.domain_verification_available? + current_group&.domain_verification_available? end end end -- GitLab From 2b19f5c781d0ca85943ecbf3cc7a94c730bb5430 Mon Sep 17 00:00:00 2001 From: James Nutt Date: Tue, 27 May 2025 12:35:53 +0100 Subject: [PATCH 17/17] Apply 1 suggestion(s) to 1 file(s) --- ...eholder_confirmation_for_enterprise_to_namespace_settings.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/migrate/20250506035156_add_allow_bypass_placeholder_confirmation_for_enterprise_to_namespace_settings.rb b/db/migrate/20250506035156_add_allow_bypass_placeholder_confirmation_for_enterprise_to_namespace_settings.rb index 7a3c6b69ce3e7c..411419b16fa8d3 100644 --- a/db/migrate/20250506035156_add_allow_bypass_placeholder_confirmation_for_enterprise_to_namespace_settings.rb +++ b/db/migrate/20250506035156_add_allow_bypass_placeholder_confirmation_for_enterprise_to_namespace_settings.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class AddAllowBypassPlaceholderConfirmationForEnterpriseToNamespaceSettings < Gitlab::Database::Migration[2.3] - milestone '18.0' + milestone '18.1' def change add_column :namespace_settings, :allow_enterprise_bypass_placeholder_confirmation, :boolean, -- GitLab