From f57085aeaf8ee29517331c05eaa59026d01db3dc Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Thu, 6 Jun 2024 17:26:29 +0200 Subject: [PATCH 01/18] Validate presence of Organization on Namespace model Changelog: changed --- app/models/namespace.rb | 5 +++++ spec/factories/namespaces.rb | 2 ++ spec/models/group_spec.rb | 2 +- spec/models/namespace_spec.rb | 1 + spec/models/namespaces/project_namespace_spec.rb | 2 +- 5 files changed, 10 insertions(+), 2 deletions(-) diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 406f7a16df6818..1f884c17afde41 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -115,6 +115,7 @@ class Namespace < ApplicationRecord has_one :import_user, class_name: 'User', through: :namespace_import_user, foreign_key: :user_id validates :owner, presence: true, if: ->(n) { n.owner_required? } + validates :organization, presence: true, if: :require_organization? validates :name, presence: true, length: { maximum: 255 } @@ -711,6 +712,10 @@ def all_projects_with_pages :active_pages_deployments) end + def require_organization? + true + end + private def cross_namespace_reference?(from) diff --git a/spec/factories/namespaces.rb b/spec/factories/namespaces.rb index 9940d932cfa6da..52ffe645d28aea 100644 --- a/spec/factories/namespaces.rb +++ b/spec/factories/namespaces.rb @@ -11,6 +11,8 @@ owner { association(:user, strategy: :build, namespace: instance, username: path) } + association :organization + after(:create) do |namespace, evaluator| # simulating ::Namespaces::ProcessSyncEventsWorker because most tests don't run Sidekiq inline # Note: we need to get refreshed `traversal_ids` it is updated via SQL query diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 13c2d087424f70..ebb9d2d65b37a5 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -271,7 +271,7 @@ it 'does not allow a subgroup to have the same name as an existing subgroup' do sub_group1 = create(:group, parent: group, name: "SG", path: 'api') - sub_group2 = described_class.new(parent: group, name: "SG", path: 'api2') + sub_group2 = described_class.new(parent: group, name: "SG", path: 'api2', organization: sub_group1.organization) expect(sub_group1).to be_valid expect(sub_group2).not_to be_valid diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index f404db2697c134..f8566ad65f3353 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -90,6 +90,7 @@ it { is_expected.to validate_presence_of(:path) } it { is_expected.to validate_length_of(:path).is_at_most(255) } it { is_expected.to validate_presence_of(:owner) } + it { is_expected.to validate_presence_of(:organization) } it { is_expected.to validate_numericality_of(:max_artifacts_size).only_integer.is_greater_than(0) } context 'validating the parent of a namespace' do diff --git a/spec/models/namespaces/project_namespace_spec.rb b/spec/models/namespaces/project_namespace_spec.rb index 84b67375f1b4ee..d2706e9292552f 100644 --- a/spec/models/namespaces/project_namespace_spec.rb +++ b/spec/models/namespaces/project_namespace_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Namespaces::ProjectNamespace, type: :model do +RSpec.describe Namespaces::ProjectNamespace, type: :model, feature_category: :groups_and_projects do let_it_be(:organization) { create(:organization) } describe 'relationships' do -- GitLab From fef362e8aef7cf8af49ba09ed6d2f9110400319f Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Fri, 7 Jun 2024 16:54:52 +0200 Subject: [PATCH 02/18] Pass organization to Group creation --- spec/models/concerns/routable_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index 326bc461e26e8b..b1c9156f6aa3cc 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -314,7 +314,7 @@ it 'skips route creation for the resource' do expect do - described_class.create!(project: nil, parent: group, visibility_level: Gitlab::VisibilityLevel::PUBLIC, path: 'foo', name: 'foo') + described_class.create!(project: nil, organization: group.organization, parent: group, visibility_level: Gitlab::VisibilityLevel::PUBLIC, path: 'foo', name: 'foo') end.not_to change { Route.count } end end -- GitLab From 32c1275426520eee05389453ccd3b7a7aec3ad54 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Tue, 11 Jun 2024 13:49:20 +0200 Subject: [PATCH 03/18] Add with_disabled_organization_validation helper --- app/models/namespace.rb | 9 ++- spec/factories/projects.rb | 3 +- spec/models/namespace_spec.rb | 10 +++ ...able_namespace_organization_validation.yml | 66 +++++++++++++++++++ ...amespace_organization_validation_helper.rb | 32 +++++++++ 5 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 spec/support/helpers/disable_namespace_organization_validation.yml create mode 100644 spec/support/helpers/disable_namespace_organization_validation_helper.rb diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 1f884c17afde41..2ac38921354fd6 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -351,6 +351,13 @@ def sum_project_statistics_column(column) coalesce = Arel::Nodes::NamedFunction.new('COALESCE', [sum, 0]) coalesce.as(column.to_s) end + + def with_disabled_organization_validation + Gitlab::SafeRequestStore[:require_organization] = false + yield + ensure + Gitlab::SafeRequestStore[:require_organization] = true + end end def to_reference_base(from = nil, full: false, absolute_path: false) @@ -713,7 +720,7 @@ def all_projects_with_pages end def require_organization? - true + Gitlab::SafeRequestStore.fetch(:require_organization) { true } # rubocop:disable Style/RedundantFetchBlock -- This fetch has a different interface end private diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 5e55ad239b7dbb..58705e7547f46b 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -16,8 +16,7 @@ has_external_wiki { false } # Associations - organization { namespace&.organization } - namespace + organization { association(:organization, strategy: :create) } creator { group ? association(:user) : namespace&.owner } transient do diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index f8566ad65f3353..e21e2b796379a0 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -695,6 +695,16 @@ it { is_expected.to include_module(Namespaces::Traversal::LinearScopes) } end + describe '.with_disabled_organization_validation', :request_store do + it 'does not require organization' do + namespace.organization = nil + + Namespace.with_disabled_organization_validation do + expect(namespace.valid?).to eq(true) + end + end + end + describe '#traversal_ids' do let(:namespace) { build(:group) } diff --git a/spec/support/helpers/disable_namespace_organization_validation.yml b/spec/support/helpers/disable_namespace_organization_validation.yml new file mode 100644 index 00000000000000..02f22d8458270a --- /dev/null +++ b/spec/support/helpers/disable_namespace_organization_validation.yml @@ -0,0 +1,66 @@ +--- +- ee/spec/controllers/ee/groups_controller_spec.rb +- ee/spec/controllers/registrations/groups_controller_spec.rb +- ee/spec/controllers/subscriptions_controller_spec.rb +- ee/spec/elastic/migrate/20240107132344_remove_issue_documents_based_on_schema_version_spec.rb +- ee/spec/features/groups_spec.rb +- ee/spec/features/projects/settings/merge_requests_settings_spec.rb +- ee/spec/features/registrations/combined_registration_spec.rb +- ee/spec/features/registrations/saas/sso_signin_standard_flow_company_creating_project_spec.rb +- ee/spec/features/registrations/saas/standard_flow_company_creating_project_spec.rb +- ee/spec/features/registrations/saas/standard_flow_just_me_creating_project_spec.rb +- ee/spec/features/registrations/saas/standard_flow_just_me_importing_project_spec.rb +- ee/spec/features/registrations/saas/subscription_flow_company_paid_plan_spec.rb +- ee/spec/features/registrations/saas/subscription_flow_just_me_paid_plan_spec.rb +- ee/spec/features/registrations/saas/trial_flow_company_creating_project_spec.rb +- ee/spec/features/registrations/saas/trial_flow_company_importing_project_spec.rb +- ee/spec/features/registrations/saas/trial_flow_just_me_creating_project_spec.rb +- ee/spec/features/registrations/saas/trial_flow_just_me_importing_project_spec.rb +- ee/spec/features/registrations/sign_up_with_trial_from_external_site_without_confirmation_spec.rb +- ee/spec/features/registrations/start_trial_from_external_site_without_confirmation_spec.rb +- ee/spec/features/subscriptions/subscription_flow_for_existing_user_with_eligible_group_spec.rb +- ee/spec/features/trials/saas/creation_with_multiple_existing_namespace_flow_spec.rb +- ee/spec/features/trials/saas/creation_with_no_existing_namespace_flow_spec.rb +- ee/spec/features/trials/saas/creation_with_one_existing_namespace_flow_spec.rb +- ee/spec/finders/approval_rules/group_finder_spec.rb +- ee/spec/finders/epics_finder_spec.rb +- ee/spec/lib/gitlab/duo/developments/setup_spec.rb +- ee/spec/lib/gitlab/llm/chain/utils/chat_authorizer_spec.rb +- ee/spec/requests/api/graphql/current_user/groups_query_spec.rb +- ee/spec/requests/api/graphql/gitlab_subscriptions/add_on_eligible_users_spec.rb +- ee/spec/requests/api/graphql/gitlab_subscriptions/user_add_on_assignments/create_spec.rb +- ee/spec/requests/api/graphql/gitlab_subscriptions/user_add_on_assignments/remove_spec.rb +- ee/spec/requests/api/graphql/namespace/compliance_frameworks_spec.rb +- ee/spec/requests/api/groups_spec.rb +- ee/spec/services/ee/groups/create_service_spec.rb +- ee/spec/services/epics/update_dates_service_spec.rb +- ee/spec/services/gitlab_subscriptions/trials/create_service_spec.rb +- ee/spec/services/registrations/import_namespace_create_service_spec.rb +- ee/spec/services/registrations/standard_namespace_create_service_spec.rb +- spec/controllers/admin/groups_controller_spec.rb +- spec/controllers/groups_controller_spec.rb +- spec/controllers/import/bitbucket_controller_spec.rb +- spec/features/admin/admin_groups_spec.rb +- spec/features/dashboard/group_spec.rb +- spec/features/file_uploads/group_import_spec.rb +- spec/features/groups/import_export/import_file_spec.rb +- spec/features/groups_spec.rb +- spec/finders/concerns/finder_with_group_hierarchy_spec.rb +- spec/frontend/fixtures/groups.rb +- spec/graphql/resolvers/group_labels_resolver_spec.rb +- spec/graphql/types/group_type_spec.rb +- spec/graphql/types/project_type_spec.rb +- spec/lib/gitlab/seeders/ci/runner/runner_fleet_seeder_spec.rb +- spec/models/member_spec.rb +- spec/requests/api/graphql/current_user/groups_query_spec.rb +- spec/requests/api/group_import_spec.rb +- spec/requests/api/groups_spec.rb +- spec/requests/api/import_bitbucket_server_spec.rb +- spec/requests/import/gitlab_groups_controller_spec.rb +- spec/services/ci/create_web_ide_terminal_service_spec.rb +- spec/services/ci/refs/enqueue_pipelines_to_unlock_service_spec.rb +- spec/services/groups/create_service_spec.rb +- spec/services/groups/nested_create_service_spec.rb +- spec/services/resource_access_tokens/create_service_spec.rb +- spec/tasks/gitlab/seed/runner_fleet_rake_spec.rb +- spec/tasks/gitlab/update_templates_rake_spec.rb diff --git a/spec/support/helpers/disable_namespace_organization_validation_helper.rb b/spec/support/helpers/disable_namespace_organization_validation_helper.rb new file mode 100644 index 00000000000000..dd756c5b195a8e --- /dev/null +++ b/spec/support/helpers/disable_namespace_organization_validation_helper.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module DisableNamespaceOrganizationValidationHelper + extend ActiveSupport::Concern + + SPECS_FOR_CODE_TO_FIX = File.join(__dir__, 'disable_namespace_organization_validation.yml') + + class << self + include Gitlab::Utils::StrongMemoize + + def todo_list + YAML.load_file(SPECS_FOR_CODE_TO_FIX).filter_map { |path| full_path(path) } || [] + end + strong_memoize_attr :todo_list + + def full_path(path) + return unless File.exist?(path) + + Pathname.new(path).realpath.to_s + end + end + + included do + spec_file = Pathname.new(example.metadata[:example_group][:file_path]).realpath.to_s + + if spec_file.in?(DisableNamespaceOrganizationValidationHelper.todo_list) + around do |example| + ::Gitlab::SafeRequestStore.ensure_request_store { example.run } + end + end + end +end -- GitLab From b3856fc4d50af93e4ae35fb9391fffa14928c1df Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Wed, 12 Jun 2024 14:17:27 +0200 Subject: [PATCH 04/18] Disable validation in case the specs fail --- .../import/gitlab_groups_controller.rb | 5 ++- .../ldap/omniauth_callbacks_controller.rb | 4 +- .../omniauth_callbacks_controller.rb | 4 +- app/controllers/registrations_controller.rb | 13 ++++--- .../ci/runners/create_runner_service.rb | 4 +- app/services/groups/create_service.rb | 2 +- app/services/groups/update_service.rb | 2 +- app/services/users/create_service.rb | 4 +- .../registrations/groups_controller.rb | 4 +- .../trials/create_service.rb | 4 +- .../standard_namespace_create_service.rb | 4 +- .../gitlab/scim/group/provisioning_service.rb | 6 ++- ee/lib/ee/gitlab/scim/provisioning_service.rb | 6 ++- .../ee/registrations_controller_spec.rb | 4 ++ .../omniauth_callbacks_controller_spec.rb | 4 ++ ee/spec/lib/gitlab/auth/ldap/user_spec.rb | 4 ++ ee/spec/lib/gitlab/auth/oidc/user_spec.rb | 4 ++ .../llm/chain/utils/chat_authorizer_spec.rb | 1 + .../developments/setup_spec.rb | 3 +- lib/gitlab/auth/o_auth/user.rb | 4 +- lib/gitlab/import/placeholder_user_creator.rb | 4 +- .../group/relation_tree_restorer.rb | 2 +- .../seeders/ci/runner/runner_fleet_seeder.rb | 8 +++- lib/tasks/gitlab/update_templates.rake | 6 ++- spec/graphql/types/group_type_spec.rb | 8 ---- spec/graphql/types/project_type_spec.rb | 8 ---- spec/lib/gitlab/auth/ldap/user_spec.rb | 2 +- spec/lib/gitlab/auth/o_auth/user_spec.rb | 4 ++ .../create_service_spec.rb | 2 +- .../users/registrations_build_service_spec.rb | 2 +- spec/spec_helper.rb | 1 + ...able_namespace_organization_validation.yml | 37 +++++++++++++------ 32 files changed, 114 insertions(+), 56 deletions(-) diff --git a/app/controllers/import/gitlab_groups_controller.rb b/app/controllers/import/gitlab_groups_controller.rb index 1fd3434a82a952..d0c684a7fb3d4d 100644 --- a/app/controllers/import/gitlab_groups_controller.rb +++ b/app/controllers/import/gitlab_groups_controller.rb @@ -21,7 +21,10 @@ def create ) .with_defaults(organization_id: Current.organization_id) - response = ::Groups::CreateService.new(current_user, group_data).execute + response = Namespace.with_disabled_organization_validation do + ::Groups::CreateService.new(current_user, group_data).execute + end + group = response[:group] if response.success? diff --git a/app/controllers/ldap/omniauth_callbacks_controller.rb b/app/controllers/ldap/omniauth_callbacks_controller.rb index 1c79bd3a668e12..4c9439de780436 100644 --- a/app/controllers/ldap/omniauth_callbacks_controller.rb +++ b/app/controllers/ldap/omniauth_callbacks_controller.rb @@ -22,7 +22,9 @@ def ldap return admin_mode_flow(Gitlab::Auth::Ldap::User) if current_user_mode.admin_mode_requested? end - sign_in_user_flow(Gitlab::Auth::Ldap::User) + Namespace.with_disabled_organization_validation do + sign_in_user_flow(Gitlab::Auth::Ldap::User) + end end define_providers! diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 5f99b9c988be88..db99cda2a18011 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -165,7 +165,9 @@ def omniauth_flow(auth_module, identity_linker: nil) redirect_identity_exists end else - sign_in_user_flow(auth_module::User) + Namespace.with_disabled_organization_validation do + sign_in_user_flow(auth_module::User) + end end end diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index e4b21408b2170b..36306f2ef12273 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -38,14 +38,15 @@ def new def create set_resource_fields - super do |new_user| - if new_user.persisted? - after_successful_create_hook(new_user) - else - track_error(new_user) + Namespace.with_disabled_organization_validation do + super do |new_user| + if new_user.persisted? + after_successful_create_hook(new_user) + else + track_error(new_user) + end end end - # Devise sets a flash message on both successful & failed signups, # but we only want to show a message if the resource is blocked by a pending approval. flash[:notice] = nil unless allow_flash_content?(resource) diff --git a/app/services/ci/runners/create_runner_service.rb b/app/services/ci/runners/create_runner_service.rb index ff4a33e431be2c..4427881e761f36 100644 --- a/app/services/ci/runners/create_runner_service.rb +++ b/app/services/ci/runners/create_runner_service.rb @@ -27,7 +27,9 @@ def execute runner = ::Ci::Runner.new(params) - return ServiceResponse.success(payload: { runner: runner }) if runner.save + if Namespace.with_disabled_organization_validation { runner.save } + return ServiceResponse.success(payload: { runner: runner }) + end ServiceResponse.error(message: runner.errors.full_messages, reason: :save_error) end diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb index 723261da36b428..643b083d6f4319 100644 --- a/app/services/groups/create_service.rb +++ b/app/services/groups/create_service.rb @@ -17,7 +17,7 @@ def execute @group.name ||= @group.path.dup create_chat_team - create_group + Namespace.with_disabled_organization_validation { create_group } return error_response unless @group.persisted? diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index 6d11660c3093ed..8072baa5f2e645 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -39,7 +39,7 @@ def execute group.assign_attributes(params) begin - success = group.save + success = Namespace.with_disabled_organization_validation { group.save } after_update if success diff --git a/app/services/users/create_service.rb b/app/services/users/create_service.rb index 591d88b275efc3..bcae6e8ce3ff90 100644 --- a/app/services/users/create_service.rb +++ b/app/services/users/create_service.rb @@ -13,7 +13,9 @@ def execute user = build_class.new(current_user, params).execute reset_token = user.generate_reset_token if user.recently_sent_password_reset? - after_create_hook(user, reset_token) if user.save + Namespace.with_disabled_organization_validation do + after_create_hook(user, reset_token) if user.save + end user end diff --git a/ee/app/controllers/registrations/groups_controller.rb b/ee/app/controllers/registrations/groups_controller.rb index 558b3995c49ba5..b780c38abdd583 100644 --- a/ee/app/controllers/registrations/groups_controller.rb +++ b/ee/app/controllers/registrations/groups_controller.rb @@ -41,7 +41,9 @@ def create end service_params = params.with_defaults(organization_id: Current.organization_id) - result = service_class.new(current_user, service_params).execute + result = Namespace.with_disabled_organization_validation do + service_class.new(current_user, service_params).execute + end if result.success? actions_after_success(result.payload) diff --git a/ee/app/services/gitlab_subscriptions/trials/create_service.rb b/ee/app/services/gitlab_subscriptions/trials/create_service.rb index d9ad3afb0fd882..fb37fc79b45fb6 100644 --- a/ee/app/services/gitlab_subscriptions/trials/create_service.rb +++ b/ee/app/services/gitlab_subscriptions/trials/create_service.rb @@ -28,7 +28,9 @@ def create_group_flow name = ActionController::Base.helpers.sanitize(trial_params[:new_group_name]) path = Namespace.clean_path(name.parameterize) create_service_params = { name: name, path: path, organization_id: trial_params[:organization_id] } - response = Groups::CreateService.new(user, create_service_params).execute + response = Namespace.with_disabled_organization_validation do + Groups::CreateService.new(user, create_service_params).execute + end @namespace = response[:group] if response.success? diff --git a/ee/app/services/registrations/standard_namespace_create_service.rb b/ee/app/services/registrations/standard_namespace_create_service.rb index b207247028638d..06ebae6f4fac0f 100644 --- a/ee/app/services/registrations/standard_namespace_create_service.rb +++ b/ee/app/services/registrations/standard_namespace_create_service.rb @@ -63,7 +63,9 @@ def project_params_attributes end def create_project_flow - @project = ::Projects::CreateService.new(user, create_project_params).execute + @project = Namespace.with_disabled_organization_validation do + ::Projects::CreateService.new(user, create_project_params).execute + end if project.persisted? Gitlab::Tracking.event(self.class.name, 'create_project', namespace: project.namespace, user: user) diff --git a/ee/lib/ee/gitlab/scim/group/provisioning_service.rb b/ee/lib/ee/gitlab/scim/group/provisioning_service.rb index 0439b3e23b3644..91ae3a7e37ee24 100644 --- a/ee/lib/ee/gitlab/scim/group/provisioning_service.rb +++ b/ee/lib/ee/gitlab/scim/group/provisioning_service.rb @@ -82,7 +82,11 @@ def create_identity_and_member end def create_user_and_member - return success_response if user.save && member.errors.empty? + saved = ::Namespace.with_disabled_organization_validation do + user.save && member.errors.empty? + end + + return success_response if saved error_response(objects: [user, identity, member]) end diff --git a/ee/lib/ee/gitlab/scim/provisioning_service.rb b/ee/lib/ee/gitlab/scim/provisioning_service.rb index 6bdb3e102c6c57..d7af5b01ee9370 100644 --- a/ee/lib/ee/gitlab/scim/provisioning_service.rb +++ b/ee/lib/ee/gitlab/scim/provisioning_service.rb @@ -53,7 +53,11 @@ def existing_user? end def create_identity_and_user - return success_response if user.save && identity.save + saved = ::Namespace.with_disabled_organization_validation do + user.save && identity.save + end + + return success_response if saved error_response(objects: [identity, user]) end diff --git a/ee/spec/controllers/ee/registrations_controller_spec.rb b/ee/spec/controllers/ee/registrations_controller_spec.rb index dc7e2e6dfbe9da..4416b5061002c0 100644 --- a/ee/spec/controllers/ee/registrations_controller_spec.rb +++ b/ee/spec/controllers/ee/registrations_controller_spec.rb @@ -124,6 +124,10 @@ let(:params) { {} } let(:session) { {} } + around do |example| + Namespace.with_disabled_organization_validation { example.run } + end + subject(:post_create) { post :create, params: params.merge(user_params), session: session } def identity_verification_exempt_for_user? diff --git a/ee/spec/controllers/groups/omniauth_callbacks_controller_spec.rb b/ee/spec/controllers/groups/omniauth_callbacks_controller_spec.rb index badaab0e73e1ce..a7570bbcf9ce90 100644 --- a/ee/spec/controllers/groups/omniauth_callbacks_controller_spec.rb +++ b/ee/spec/controllers/groups/omniauth_callbacks_controller_spec.rb @@ -329,6 +329,10 @@ def stub_last_request_id(id) it "links the identity" do post provider, params: { group_id: group } + # This line should be removed if this file is removed from the exception file: + # spec/support/helpers/disable_namespace_organization_validation.yml + Gitlab::SafeRequestStore.clear! + expect(group).to be_member(user) end diff --git a/ee/spec/lib/gitlab/auth/ldap/user_spec.rb b/ee/spec/lib/gitlab/auth/ldap/user_spec.rb index ce9e635ce1ea79..be239e2bfa6be6 100644 --- a/ee/spec/lib/gitlab/auth/ldap/user_spec.rb +++ b/ee/spec/lib/gitlab/auth/ldap/user_spec.rb @@ -24,6 +24,10 @@ let(:external_groups) { [] } let!(:fake_proxy) { fake_ldap_sync_proxy(auth_hash.provider) } + around do |example| + Namespace.with_disabled_organization_validation { example.run } + end + before do allow(fake_proxy).to receive(:dns_for_group_cn).with(group_cn).and_return(group_member_dns) stub_ldap_config(external_groups: external_groups) diff --git a/ee/spec/lib/gitlab/auth/oidc/user_spec.rb b/ee/spec/lib/gitlab/auth/oidc/user_spec.rb index a9ed5be2ee1b2a..86d0cc1c1dd333 100644 --- a/ee/spec/lib/gitlab/auth/oidc/user_spec.rb +++ b/ee/spec/lib/gitlab/auth/oidc/user_spec.rb @@ -29,6 +29,10 @@ extra: extra_hash) end + around do |example| + Namespace.with_disabled_organization_validation { example.run } + end + before do allow(Gitlab.config.omniauth).to receive_messages(block_auto_created_users: false) diff --git a/ee/spec/lib/gitlab/llm/chain/utils/chat_authorizer_spec.rb b/ee/spec/lib/gitlab/llm/chain/utils/chat_authorizer_spec.rb index 8596f5b94a19d3..b6bd420605ae7e 100644 --- a/ee/spec/lib/gitlab/llm/chain/utils/chat_authorizer_spec.rb +++ b/ee/spec/lib/gitlab/llm/chain/utils/chat_authorizer_spec.rb @@ -211,6 +211,7 @@ shared_examples 'container authorizer' do before do allow(user).to receive(:can?).with(:access_duo_features, container).and_return(duo_features_enabled) + allow(user).to receive(:can?).with(any_args).and_call_original end context 'when container has duo_features enabled' do diff --git a/ee/spec/lib/gitlab/product_analytics/developments/setup_spec.rb b/ee/spec/lib/gitlab/product_analytics/developments/setup_spec.rb index a3c5aa81986ac1..55ad38dbd22e8e 100644 --- a/ee/spec/lib/gitlab/product_analytics/developments/setup_spec.rb +++ b/ee/spec/lib/gitlab/product_analytics/developments/setup_spec.rb @@ -5,7 +5,8 @@ RSpec.describe Gitlab::ProductAnalytics::Developments::Setup, :saas, feature_category: :product_analytics_data_management do include RakeHelpers - let_it_be(:group) { create(:group, path: 'test-group') } + let_it_be(:organization) { create(:organization) } + let_it_be(:group) { create(:group, path: 'test-group', organization: organization) } let_it_be(:project) { create(:project, group: group) } let_it_be(:user) { create(:user) } diff --git a/lib/gitlab/auth/o_auth/user.rb b/lib/gitlab/auth/o_auth/user.rb index ca2730f855d5ca..8a693b72b6734f 100644 --- a/lib/gitlab/auth/o_auth/user.rb +++ b/lib/gitlab/auth/o_auth/user.rb @@ -57,7 +57,9 @@ def save(provider = protocol_name) block_after_save = needs_blocking? - Users::UpdateService.new(gl_user, user: gl_user).execute! + Namespace.with_disabled_organization_validation do + Users::UpdateService.new(gl_user, user: gl_user).execute! + end gl_user.block_pending_approval if block_after_save activate_user_if_user_cap_not_reached diff --git a/lib/gitlab/import/placeholder_user_creator.rb b/lib/gitlab/import/placeholder_user_creator.rb index 033c2b713e4ada..6ff5396eaaab12 100644 --- a/lib/gitlab/import/placeholder_user_creator.rb +++ b/lib/gitlab/import/placeholder_user_creator.rb @@ -19,7 +19,9 @@ def execute ) user.assign_personal_namespace(Organizations::Organization.default_organization) - user.save! + Namespace.with_disabled_organization_validation do + user.save! + end user end diff --git a/lib/gitlab/import_export/group/relation_tree_restorer.rb b/lib/gitlab/import_export/group/relation_tree_restorer.rb index 3ae9f67e9017ce..da6af1e1c9b4fb 100644 --- a/lib/gitlab/import_export/group/relation_tree_restorer.rb +++ b/lib/gitlab/import_export/group/relation_tree_restorer.rb @@ -178,7 +178,7 @@ def update_params! modify_attributes - @importable.save!(touch: false) + Namespace.with_disabled_organization_validation { @importable.save!(touch: false) } end def filter_attributes(params) diff --git a/lib/gitlab/seeders/ci/runner/runner_fleet_seeder.rb b/lib/gitlab/seeders/ci/runner/runner_fleet_seeder.rb index a1f40818ec706d..ed96e1cf18e661 100644 --- a/lib/gitlab/seeders/ci/runner/runner_fleet_seeder.rb +++ b/lib/gitlab/seeders/ci/runner/runner_fleet_seeder.rb @@ -178,7 +178,9 @@ def generate_name(name) def create_group(**args) logger.info(message: 'Creating group', **args) - ensure_success(::Groups::CreateService.new(@user, **args).execute[:group]) + Namespace.with_disabled_organization_validation do + ensure_success(::Groups::CreateService.new(@user, **args).execute[:group]) + end end def ensure_project(name:, namespace_id:, **args) @@ -194,7 +196,9 @@ def ensure_project(name:, namespace_id:, **args) def create_project(**args) logger.info(message: 'Creating project', **args) - ensure_success(::Projects::CreateService.new(@user, **args).execute) + Namespace.with_disabled_organization_validation do + ensure_success(::Projects::CreateService.new(@user, **args).execute) + end end def register_record(record, records) diff --git a/lib/tasks/gitlab/update_templates.rake b/lib/tasks/gitlab/update_templates.rake index 3c21aef3dd8039..8a87f565e168f7 100644 --- a/lib/tasks/gitlab/update_templates.rake +++ b/lib/tasks/gitlab/update_templates.rake @@ -37,7 +37,9 @@ namespace :gitlab do tmp_namespace_path = "tmp-project-import-#{Time.now.to_i}" puts "Creating temporary namespace #{tmp_namespace_path}" - tmp_namespace = Namespace.create!(owner: admin, name: tmp_namespace_path, path: tmp_namespace_path, type: Namespaces::UserNamespace.sti_name) + tmp_namespace = Namespace.with_disabled_organization_validation do + Namespace.create!(owner: admin, name: tmp_namespace_path, path: tmp_namespace_path, type: Namespaces::UserNamespace.sti_name) + end templates = if template_names.empty? Gitlab::ProjectTemplate.all @@ -54,7 +56,7 @@ namespace :gitlab do } puts "Creating project for #{template.title}" - project = Projects::CreateService.new(admin, params).execute + project = Namespace.with_disabled_organization_validation { Projects::CreateService.new(admin, params).execute } unless project.persisted? raise "Failed to create project: #{project.errors.messages}" diff --git a/spec/graphql/types/group_type_spec.rb b/spec/graphql/types/group_type_spec.rb index 2f2e699b96559b..21b6f732163099 100644 --- a/spec/graphql/types/group_type_spec.rb +++ b/spec/graphql/types/group_type_spec.rb @@ -266,13 +266,5 @@ def clean_state_query ) end end - - context 'when group does not have an organization associated with it' do - let_it_be(:group) { create(:group, :public, organization: nil) } - - it 'returns nil' do - expect(organization_edit_path).to be_nil - end - end end end diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb index 6dfa88680e8b66..a55ae28b08cef7 100644 --- a/spec/graphql/types/project_type_spec.rb +++ b/spec/graphql/types/project_type_spec.rb @@ -1305,13 +1305,5 @@ ) end end - - context 'when project does not have an organization associated with it' do - let_it_be(:project) { create(:project, :public, organization: nil) } - - it 'returns nil' do - expect(organization_edit_path).to be_nil - end - end end end diff --git a/spec/lib/gitlab/auth/ldap/user_spec.rb b/spec/lib/gitlab/auth/ldap/user_spec.rb index 5771b1cd609761..d635cfe0907646 100644 --- a/spec/lib/gitlab/auth/ldap/user_spec.rb +++ b/spec/lib/gitlab/auth/ldap/user_spec.rb @@ -51,7 +51,7 @@ describe '#valid_sign_in?' do before do - gl_user.save! + Namespace.with_disabled_organization_validation { gl_user.save! } end it 'returns true' do diff --git a/spec/lib/gitlab/auth/o_auth/user_spec.rb b/spec/lib/gitlab/auth/o_auth/user_spec.rb index 20b25f5b08a45e..fd22300605a662 100644 --- a/spec/lib/gitlab/auth/o_auth/user_spec.rb +++ b/spec/lib/gitlab/auth/o_auth/user_spec.rb @@ -30,6 +30,10 @@ let(:ldap_user) { Gitlab::Auth::Ldap::Person.new(Net::LDAP::Entry.new, 'ldapmain') } let(:ldap_user_2) { Gitlab::Auth::Ldap::Person.new(Net::LDAP::Entry.new, 'ldapmain') } + around do |example| + Namespace.with_disabled_organization_validation { example.run } + end + describe '.find_by_uid_and_provider' do let(:provider) { 'provider' } let(:uid) { 'uid' } diff --git a/spec/services/resource_access_tokens/create_service_spec.rb b/spec/services/resource_access_tokens/create_service_spec.rb index eefd67ab2a6b2c..bd8b936c52fbe7 100644 --- a/spec/services/resource_access_tokens/create_service_spec.rb +++ b/spec/services/resource_access_tokens/create_service_spec.rb @@ -393,7 +393,7 @@ end context 'when resource organization is not set', :enable_admin_mode do - let_it_be(:resource) { create(:project, :private, organization: nil) } + let_it_be(:resource) { create(:project, :private, organization_id: nil) } let_it_be(:default_organization) { Organizations::Organization.default_organization } let(:user) { create(:admin) } diff --git a/spec/services/users/registrations_build_service_spec.rb b/spec/services/users/registrations_build_service_spec.rb index ec9617991e753f..f8e0b83a896a7b 100644 --- a/spec/services/users/registrations_build_service_spec.rb +++ b/spec/services/users/registrations_build_service_spec.rb @@ -18,7 +18,7 @@ it 'creates the user_detail record' do user = service.execute - expect { user.save! }.to change { UserDetail.count }.by(1) + expect { Namespace.with_disabled_organization_validation { user.save! } }.to change { UserDetail.count }.by(1) end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 89f0b53ddb5854..b300a34f0ed746 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -205,6 +205,7 @@ config.include UserWithNamespaceShim config.include OrphanFinalArtifactsCleanupHelpers, :orphan_final_artifacts_cleanup config.include ClickHouseHelpers, :click_house + config.include DisableNamespaceOrganizationValidationHelper config.include_context 'when rendered has no HTML escapes', type: :view diff --git a/spec/support/helpers/disable_namespace_organization_validation.yml b/spec/support/helpers/disable_namespace_organization_validation.yml index 02f22d8458270a..b2e8c54a02fbd7 100644 --- a/spec/support/helpers/disable_namespace_organization_validation.yml +++ b/spec/support/helpers/disable_namespace_organization_validation.yml @@ -1,8 +1,11 @@ --- - ee/spec/controllers/ee/groups_controller_spec.rb +- ee/spec/controllers/ee/registrations_controller_spec.rb +- ee/spec/controllers/ee/omniauth_callbacks_controller_spec.rb +- ee/spec/controllers/groups/omniauth_callbacks_controller_spec.rb +- ee/spec/controllers/ldap/omniauth_callbacks_controller_spec.rb - ee/spec/controllers/registrations/groups_controller_spec.rb - ee/spec/controllers/subscriptions_controller_spec.rb -- ee/spec/elastic/migrate/20240107132344_remove_issue_documents_based_on_schema_version_spec.rb - ee/spec/features/groups_spec.rb - ee/spec/features/projects/settings/merge_requests_settings_spec.rb - ee/spec/features/registrations/combined_registration_spec.rb @@ -23,44 +26,56 @@ - ee/spec/features/trials/saas/creation_with_no_existing_namespace_flow_spec.rb - ee/spec/features/trials/saas/creation_with_one_existing_namespace_flow_spec.rb - ee/spec/finders/approval_rules/group_finder_spec.rb -- ee/spec/finders/epics_finder_spec.rb -- ee/spec/lib/gitlab/duo/developments/setup_spec.rb +- ee/spec/lib/gitlab/auth/group_saml/user_spec.rb +- ee/spec/lib/gitlab/auth/ldap/user_spec.rb +- ee/spec/lib/gitlab/auth/oidc/user_spec.rb +- ee/spec/lib/gitlab/auth/saml/user_spec.rb - ee/spec/lib/gitlab/llm/chain/utils/chat_authorizer_spec.rb -- ee/spec/requests/api/graphql/current_user/groups_query_spec.rb -- ee/spec/requests/api/graphql/gitlab_subscriptions/add_on_eligible_users_spec.rb -- ee/spec/requests/api/graphql/gitlab_subscriptions/user_add_on_assignments/create_spec.rb -- ee/spec/requests/api/graphql/gitlab_subscriptions/user_add_on_assignments/remove_spec.rb -- ee/spec/requests/api/graphql/namespace/compliance_frameworks_spec.rb +- ee/spec/lib/ee/gitlab/scim/provisioning_service_spec.rb +- ee/spec/lib/ee/gitlab/scim/group/provisioning_service_spec.rb - ee/spec/requests/api/groups_spec.rb - ee/spec/services/ee/groups/create_service_spec.rb +- ee/spec/services/ee/users/create_service_spec.rb +- ee/spec/services/users/service_accounts/create_service_spec.rb - ee/spec/services/epics/update_dates_service_spec.rb - ee/spec/services/gitlab_subscriptions/trials/create_service_spec.rb - ee/spec/services/registrations/import_namespace_create_service_spec.rb - ee/spec/services/registrations/standard_namespace_create_service_spec.rb - spec/controllers/admin/groups_controller_spec.rb +- spec/controllers/admin/users_controller_spec.rb - spec/controllers/groups_controller_spec.rb - spec/controllers/import/bitbucket_controller_spec.rb +- spec/controllers/omniauth_callbacks_controller_spec.rb +- spec/controllers/registrations_controller_spec.rb - spec/features/admin/admin_groups_spec.rb - spec/features/dashboard/group_spec.rb - spec/features/file_uploads/group_import_spec.rb - spec/features/groups/import_export/import_file_spec.rb - spec/features/groups_spec.rb -- spec/finders/concerns/finder_with_group_hierarchy_spec.rb - spec/frontend/fixtures/groups.rb -- spec/graphql/resolvers/group_labels_resolver_spec.rb - spec/graphql/types/group_type_spec.rb - spec/graphql/types/project_type_spec.rb +- spec/lib/gitlab/auth/atlassian/user_spec.rb +- spec/lib/gitlab/auth/ldap/user_spec.rb +- spec/lib/gitlab/auth/o_auth/user_spec.rb +- spec/lib/gitlab/auth/saml/user_spec.rb +- spec/lib/gitlab/import/placeholder_user_creator_spec.rb +- spec/lib/gitlab/import/source_user_mapper_spec.rb - spec/lib/gitlab/seeders/ci/runner/runner_fleet_seeder_spec.rb +- spec/models/analytics/cycle_analytics/stage_spec.rb - spec/models/member_spec.rb -- spec/requests/api/graphql/current_user/groups_query_spec.rb +- spec/models/hooks/system_hook_spec.rb - spec/requests/api/group_import_spec.rb - spec/requests/api/groups_spec.rb - spec/requests/api/import_bitbucket_server_spec.rb - spec/requests/import/gitlab_groups_controller_spec.rb - spec/services/ci/create_web_ide_terminal_service_spec.rb - spec/services/ci/refs/enqueue_pipelines_to_unlock_service_spec.rb +- spec/services/users/create_service_spec.rb +- spec/services/users/registrations_build_service_spec.rb - spec/services/groups/create_service_spec.rb - spec/services/groups/nested_create_service_spec.rb - spec/services/resource_access_tokens/create_service_spec.rb +- spec/services/users/registrations_build_service_spec.rb - spec/tasks/gitlab/seed/runner_fleet_rake_spec.rb - spec/tasks/gitlab/update_templates_rake_spec.rb -- GitLab From cd977f4299747104877e526565cc58d0e08b35bb Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Wed, 12 Jun 2024 15:03:53 +0200 Subject: [PATCH 05/18] Ensure namespace organization on projects is created --- .../features/projects/settings/merge_requests_settings_spec.rb | 2 +- spec/factories/projects.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ee/spec/features/projects/settings/merge_requests_settings_spec.rb b/ee/spec/features/projects/settings/merge_requests_settings_spec.rb index 799d9c5dd6dc20..ce2af66d1955dc 100644 --- a/ee/spec/features/projects/settings/merge_requests_settings_spec.rb +++ b/ee/spec/features/projects/settings/merge_requests_settings_spec.rb @@ -6,7 +6,7 @@ let_it_be(:user) { create(:user, :with_namespace) } let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project, :public, namespace: user.namespace, path: 'gitlab', name: 'sample', group: group) } + let_it_be(:project) { create(:project, :public, path: 'gitlab', name: 'sample', group: group) } let_it_be(:group_member) { create(:user) } before do diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 58705e7547f46b..5e55ad239b7dbb 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -16,7 +16,8 @@ has_external_wiki { false } # Associations - organization { association(:organization, strategy: :create) } + organization { namespace&.organization } + namespace creator { group ? association(:user) : namespace&.owner } transient do -- GitLab From 7f421efdcf280e8240188dd42569245538bc215a Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Thu, 13 Jun 2024 16:52:33 +0200 Subject: [PATCH 06/18] Fix specs by setting current_organization --- .../controllers/subscriptions/groups_controller_spec.rb | 3 ++- ee/spec/controllers/subscriptions_controller_spec.rb | 2 +- spec/models/member_spec.rb | 8 +------- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/ee/spec/controllers/subscriptions/groups_controller_spec.rb b/ee/spec/controllers/subscriptions/groups_controller_spec.rb index 0ca2b8572725bc..788b312f84f8f0 100644 --- a/ee/spec/controllers/subscriptions/groups_controller_spec.rb +++ b/ee/spec/controllers/subscriptions/groups_controller_spec.rb @@ -105,7 +105,7 @@ end end - describe 'POST #create' do + describe 'POST #create', :with_current_organization do context 'with an unauthenticated user' do subject { post :create, params: { group: { name: 'Test Group', plan_id: 'plan-id' } } } @@ -116,6 +116,7 @@ context 'with an authenticated user' do before do sign_in(user) + current_organization.users << user end context 'with valid params' do diff --git a/ee/spec/controllers/subscriptions_controller_spec.rb b/ee/spec/controllers/subscriptions_controller_spec.rb index 4531d09c66be28..cff94623d853fe 100644 --- a/ee/spec/controllers/subscriptions_controller_spec.rb +++ b/ee/spec/controllers/subscriptions_controller_spec.rb @@ -786,7 +786,7 @@ end context 'when an error occurs creating a group' do - let(:group) { Group.new(path: 'foo') } + let(:group) { Group.new(path: 'foo', organization: current_organization) } it 'returns the errors in json format' do group.valid? diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 38a9f12ca04704..563b806db89369 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -1277,7 +1277,7 @@ end context 'for updating organization_users' do - let_it_be(:group) { create(:group, :with_organization) } + let_it_be(:group) { create(:group) } let_it_be(:user) { create(:user) } let(:member) { create(:group_member, source: group, user: user) } @@ -1362,12 +1362,6 @@ it_behaves_like 'does not create an organization_user entry' end - - context 'when organization does not exist' do - let(:member) { create(:group_member, user: user) } - - it_behaves_like 'does not create an organization_user entry' - end end context 'when updating' do -- GitLab From c95421661223eaf99ca5510e56cb5760af807752 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Fri, 21 Jun 2024 14:48:21 +0200 Subject: [PATCH 07/18] Fix specs: ensure organizations are the same for the created groups --- ee/spec/models/vulnerabilities/export_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/spec/models/vulnerabilities/export_spec.rb b/ee/spec/models/vulnerabilities/export_spec.rb index 66a2c423c5ee24..6846b96bf7c745 100644 --- a/ee/spec/models/vulnerabilities/export_spec.rb +++ b/ee/spec/models/vulnerabilities/export_spec.rb @@ -152,7 +152,7 @@ subject(:set_exportable) { vulnerability_export.exportable = exportable } context 'when the exportable is a Project' do - let(:exportable) { build(:project) } + let(:exportable) { create(:project) } it 'changes the exportable of the export to given project' do expect { set_exportable }.to change { vulnerability_export.exportable }.to(exportable) @@ -165,7 +165,7 @@ end context 'when the exportable is a Group' do - let(:exportable) { build(:group) } + let(:exportable) { create(:group) } it 'changes the exportable of the export to given group' do expect { set_exportable }.to change { vulnerability_export.exportable }.to(exportable) -- GitLab From cfe4c454314a6534b492c26196690b8fd4c90071 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Wed, 26 Jun 2024 15:35:46 +0200 Subject: [PATCH 08/18] Prevent additional organizatoin query Because we now require organization, the PolicyCheck for organization owner is now being used. This caused the test to fail, but the query is legit --- spec/models/preloaders/project_policy_preloader_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/preloaders/project_policy_preloader_spec.rb b/spec/models/preloaders/project_policy_preloader_spec.rb index dd7f8e15d5d9a2..362717bb84ebcb 100644 --- a/spec/models/preloaders/project_policy_preloader_spec.rb +++ b/spec/models/preloaders/project_policy_preloader_spec.rb @@ -28,7 +28,7 @@ control = ActiveRecord::QueryRecorder.new { authorize_all_projects(user) } new_project1 = create(:project, :private, maintainers: user) - new_project2 = create(:project, :private, namespace: root_parent) + new_project2 = create(:project, :private, namespace: root_parent, maintainers: user) another_root = create(:group, :private, name: 'root-3', path: 'root-3') new_project3 = create(:project, :private, namespace: another_root, maintainers: user) -- GitLab From cae07fb2280b9697a696e99c3ef14152afe37049 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Wed, 26 Jun 2024 16:54:08 +0200 Subject: [PATCH 09/18] Increase query count because of organization validation --- spec/requests/api/project_import_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/api/project_import_spec.rb b/spec/requests/api/project_import_spec.rb index 4c2adadd3600c5..7ccca5e031e024 100644 --- a/spec/requests/api/project_import_spec.rb +++ b/spec/requests/api/project_import_spec.rb @@ -64,7 +64,7 @@ it 'executes a limited number of queries', :use_clean_rails_redis_caching do control = ActiveRecord::QueryRecorder.new { perform_archive_upload } - expect(control.count).to be <= 114 + expect(control.count).to be <= 115 end it 'schedules an import using a namespace' do -- GitLab From 202dcc4725e7224488a74c0040040d32b5369973 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Thu, 27 Jun 2024 14:24:34 +0200 Subject: [PATCH 10/18] Increase query limit for merge request graphql test --- app/controllers/graphql_controller.rb | 11 ++++++++--- .../api/graphql/project/merge_requests_spec.rb | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index 06a36348baa3b8..def0d9ec50be84 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -187,10 +187,15 @@ def mutation?(query_string, operation_name = permitted_params[:operationName]) def disable_query_limiting return unless Gitlab::QueryLimiting.enabled_for_env? - disable_issue = request.headers[DISABLE_SQL_QUERY_LIMIT_HEADER] - return unless disable_issue + disable_reference = request.headers[DISABLE_SQL_QUERY_LIMIT_HEADER] + return unless disable_reference - Gitlab::QueryLimiting.disable!(disable_issue) + disable_issue, new_threshold = disable_reference.split(';') + if new_threshold + Gitlab::QueryLimiting.disable!(disable_issue, new_threshold: new_threshold&.to_i) + else + Gitlab::QueryLimiting.disable!(disable_issue) + end end def set_user_last_activity diff --git a/spec/requests/api/graphql/project/merge_requests_spec.rb b/spec/requests/api/graphql/project/merge_requests_spec.rb index 176a02df0be7d1..0798772b9bd734 100644 --- a/spec/requests/api/graphql/project/merge_requests_spec.rb +++ b/spec/requests/api/graphql/project/merge_requests_spec.rb @@ -59,7 +59,7 @@ def query_merge_requests(fields) # We cannot disable SQL query limiting here, since the transaction does not # begin until we enter the controller. headers = { - 'X-GITLAB-DISABLE-SQL-QUERY-LIMIT' => 'https://gitlab.com/gitlab-org/gitlab/-/issues/322979' + 'X-GITLAB-DISABLE-SQL-QUERY-LIMIT' => 'https://gitlab.com/gitlab-org/gitlab/-/issues/469250;205' } post_graphql(query, current_user: current_user, headers: headers) -- GitLab From 51b496ba2e959c6d6c5472c23142b0126a0d951c Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Thu, 27 Jun 2024 17:04:02 +0200 Subject: [PATCH 11/18] Ensure projects are using the same organization --- ee/spec/services/ci/runners/assign_runner_service_spec.rb | 2 +- spec/models/analytics/cycle_analytics/stage_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ee/spec/services/ci/runners/assign_runner_service_spec.rb b/ee/spec/services/ci/runners/assign_runner_service_spec.rb index a46adfb9aff1e6..3b4fbb06af37cd 100644 --- a/ee/spec/services/ci/runners/assign_runner_service_spec.rb +++ b/ee/spec/services/ci/runners/assign_runner_service_spec.rb @@ -4,7 +4,7 @@ RSpec.describe ::Ci::Runners::AssignRunnerService, '#execute', feature_category: :runner do let_it_be(:owner_project) { create(:project) } - let_it_be(:new_project) { create(:project) } + let_it_be(:new_project) { create(:project, organization: owner_project.organization) } let_it_be(:project_runner) { create(:ci_runner, :project, projects: [owner_project]) } let(:params) { {} } diff --git a/spec/models/analytics/cycle_analytics/stage_spec.rb b/spec/models/analytics/cycle_analytics/stage_spec.rb index 1703a425a45f6e..40f087803313a4 100644 --- a/spec/models/analytics/cycle_analytics/stage_spec.rb +++ b/spec/models/analytics/cycle_analytics/stage_spec.rb @@ -42,8 +42,8 @@ describe '.distinct_stages_within_hierarchy' do let_it_be(:group) { create(:group) } - let_it_be(:sub_group) { create(:group, parent: group) } - let_it_be(:project) { create(:project, group: sub_group).reload } + let_it_be(:sub_group) { create(:group, organization: group.organization, parent: group) } + let_it_be(:project) { create(:project, organization: group.organization, group: sub_group).reload } before do # event identifiers are the same -- GitLab From 670886385c6a6e3b23e4a4eaf5d53c56ccc9c33f Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Thu, 4 Jul 2024 15:40:48 +0200 Subject: [PATCH 12/18] Add derisk feature flag for enabling organization validation --- app/models/namespace.rb | 2 ++ .../require_organization.yml | 9 +++++++ spec/models/namespace_spec.rb | 26 +++++++++++++++++-- 3 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 config/feature_flags/gitlab_com_derisk/require_organization.yml diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 2ac38921354fd6..004bc99e85ed18 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -720,6 +720,8 @@ def all_projects_with_pages end def require_organization? + return false unless Feature.enabled?(:require_organization, Feature.current_request) + Gitlab::SafeRequestStore.fetch(:require_organization) { true } # rubocop:disable Style/RedundantFetchBlock -- This fetch has a different interface end diff --git a/config/feature_flags/gitlab_com_derisk/require_organization.yml b/config/feature_flags/gitlab_com_derisk/require_organization.yml new file mode 100644 index 00000000000000..452ea242073baf --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/require_organization.yml @@ -0,0 +1,9 @@ +--- +name: require_organization +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/411832 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/155732 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/470839 +milestone: '17.2' +group: group::tenant scale +type: gitlab_com_derisk +default_enabled: false diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index e21e2b796379a0..9771f5d2eb5cc2 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -695,12 +695,34 @@ it { is_expected.to include_module(Namespaces::Traversal::LinearScopes) } end - describe '.with_disabled_organization_validation', :request_store do + context 'when feature flag require_organization is disabled' do + before do + stub_feature_flags(require_organization: false) + end + it 'does not require organization' do namespace.organization = nil + expect(namespace.valid?).to eq(true) + end + end + + context 'when feature flag require_organization is enabled' do + it 'does require organization' do + namespace.organization = nil + Namespace.with_disabled_organization_validation do - expect(namespace.valid?).to eq(true) + expect(namespace.valid?).to eq(false) + end + end + + describe '.with_disabled_organization_validation', :request_store do + it 'does not require organization' do + namespace.organization = nil + + Namespace.with_disabled_organization_validation do + expect(namespace.valid?).to eq(true) + end end end end -- GitLab From 748c2ced7e5938d5ed52393088b0d8c4ad8a403d Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Thu, 4 Jul 2024 16:22:57 +0200 Subject: [PATCH 13/18] Only reset RequestStore if it was changed from true --- app/models/namespace.rb | 4 +++- spec/models/namespace_spec.rb | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 004bc99e85ed18..f28ba454b8738b 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -353,10 +353,12 @@ def sum_project_statistics_column(column) end def with_disabled_organization_validation + current_value = Gitlab::SafeRequestStore[:require_organization] Gitlab::SafeRequestStore[:require_organization] = false + yield ensure - Gitlab::SafeRequestStore[:require_organization] = true + Gitlab::SafeRequestStore[:require_organization] = current_value end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 9771f5d2eb5cc2..60ec701a8deeb7 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -724,6 +724,22 @@ expect(namespace.valid?).to eq(true) end end + + context 'with nested calls' do + it 'only last call will enable the validation' do + result = [] + Namespace.with_disabled_organization_validation do + result << described_class.new.require_organization? + Namespace.with_disabled_organization_validation do + result << described_class.new.require_organization? + end + result << described_class.new.require_organization? + end + + expect(result.any?(true)).to be false + expect(described_class.new.require_organization?).to be true + end + end end end -- GitLab From 85c22983233048c3b0ef998efce67333b473dbab Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Fri, 12 Jul 2024 10:39:14 +0200 Subject: [PATCH 14/18] Fix specs for ChatAuthorizer --- ee/spec/lib/gitlab/llm/chain/utils/chat_authorizer_spec.rb | 3 +-- .../helpers/disable_namespace_organization_validation.yml | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/ee/spec/lib/gitlab/llm/chain/utils/chat_authorizer_spec.rb b/ee/spec/lib/gitlab/llm/chain/utils/chat_authorizer_spec.rb index b6bd420605ae7e..41ee408d0a8860 100644 --- a/ee/spec/lib/gitlab/llm/chain/utils/chat_authorizer_spec.rb +++ b/ee/spec/lib/gitlab/llm/chain/utils/chat_authorizer_spec.rb @@ -211,7 +211,6 @@ shared_examples 'container authorizer' do before do allow(user).to receive(:can?).with(:access_duo_features, container).and_return(duo_features_enabled) - allow(user).to receive(:can?).with(any_args).and_call_original end context 'when container has duo_features enabled' do @@ -239,7 +238,7 @@ let(:container) { create(:group) } before do - allow(user).to receive(:can?).with(:admin_organization, nil).and_call_original + allow(user).to receive(:can?).with(:admin_organization, container.organization).and_call_original allow(user).to receive(:can?).with(:admin_all_resources).and_call_original end diff --git a/spec/support/helpers/disable_namespace_organization_validation.yml b/spec/support/helpers/disable_namespace_organization_validation.yml index b2e8c54a02fbd7..6b2da7321665cf 100644 --- a/spec/support/helpers/disable_namespace_organization_validation.yml +++ b/spec/support/helpers/disable_namespace_organization_validation.yml @@ -30,7 +30,6 @@ - ee/spec/lib/gitlab/auth/ldap/user_spec.rb - ee/spec/lib/gitlab/auth/oidc/user_spec.rb - ee/spec/lib/gitlab/auth/saml/user_spec.rb -- ee/spec/lib/gitlab/llm/chain/utils/chat_authorizer_spec.rb - ee/spec/lib/ee/gitlab/scim/provisioning_service_spec.rb - ee/spec/lib/ee/gitlab/scim/group/provisioning_service_spec.rb - ee/spec/requests/api/groups_spec.rb -- GitLab From 816d8dbe81d82873a1d1bbbb11972270ee889250 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Fri, 12 Jul 2024 11:29:39 +0200 Subject: [PATCH 15/18] Remove exceptions --- .../disable_namespace_organization_validation.yml | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/spec/support/helpers/disable_namespace_organization_validation.yml b/spec/support/helpers/disable_namespace_organization_validation.yml index 6b2da7321665cf..0751e8597a06b6 100644 --- a/spec/support/helpers/disable_namespace_organization_validation.yml +++ b/spec/support/helpers/disable_namespace_organization_validation.yml @@ -5,7 +5,6 @@ - ee/spec/controllers/groups/omniauth_callbacks_controller_spec.rb - ee/spec/controllers/ldap/omniauth_callbacks_controller_spec.rb - ee/spec/controllers/registrations/groups_controller_spec.rb -- ee/spec/controllers/subscriptions_controller_spec.rb - ee/spec/features/groups_spec.rb - ee/spec/features/projects/settings/merge_requests_settings_spec.rb - ee/spec/features/registrations/combined_registration_spec.rb @@ -25,14 +24,12 @@ - ee/spec/features/trials/saas/creation_with_multiple_existing_namespace_flow_spec.rb - ee/spec/features/trials/saas/creation_with_no_existing_namespace_flow_spec.rb - ee/spec/features/trials/saas/creation_with_one_existing_namespace_flow_spec.rb -- ee/spec/finders/approval_rules/group_finder_spec.rb - ee/spec/lib/gitlab/auth/group_saml/user_spec.rb - ee/spec/lib/gitlab/auth/ldap/user_spec.rb - ee/spec/lib/gitlab/auth/oidc/user_spec.rb - ee/spec/lib/gitlab/auth/saml/user_spec.rb - ee/spec/lib/ee/gitlab/scim/provisioning_service_spec.rb - ee/spec/lib/ee/gitlab/scim/group/provisioning_service_spec.rb -- ee/spec/requests/api/groups_spec.rb - ee/spec/services/ee/groups/create_service_spec.rb - ee/spec/services/ee/users/create_service_spec.rb - ee/spec/services/users/service_accounts/create_service_spec.rb @@ -61,20 +58,13 @@ - spec/lib/gitlab/import/placeholder_user_creator_spec.rb - spec/lib/gitlab/import/source_user_mapper_spec.rb - spec/lib/gitlab/seeders/ci/runner/runner_fleet_seeder_spec.rb -- spec/models/analytics/cycle_analytics/stage_spec.rb -- spec/models/member_spec.rb - spec/models/hooks/system_hook_spec.rb -- spec/requests/api/group_import_spec.rb - spec/requests/api/groups_spec.rb -- spec/requests/api/import_bitbucket_server_spec.rb - spec/requests/import/gitlab_groups_controller_spec.rb -- spec/services/ci/create_web_ide_terminal_service_spec.rb -- spec/services/ci/refs/enqueue_pipelines_to_unlock_service_spec.rb - spec/services/users/create_service_spec.rb - spec/services/users/registrations_build_service_spec.rb - spec/services/groups/create_service_spec.rb - spec/services/groups/nested_create_service_spec.rb - spec/services/resource_access_tokens/create_service_spec.rb -- spec/services/users/registrations_build_service_spec.rb - spec/tasks/gitlab/seed/runner_fleet_rake_spec.rb - spec/tasks/gitlab/update_templates_rake_spec.rb -- GitLab From 65b5f9b63b85b18ae3ac2cc9990f97acc74846f7 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Mon, 15 Jul 2024 17:35:01 +0200 Subject: [PATCH 16/18] Use let_it_be for project creating --- ee/spec/models/vulnerabilities/export_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/models/vulnerabilities/export_spec.rb b/ee/spec/models/vulnerabilities/export_spec.rb index 6846b96bf7c745..58e3049dbc4474 100644 --- a/ee/spec/models/vulnerabilities/export_spec.rb +++ b/ee/spec/models/vulnerabilities/export_spec.rb @@ -152,7 +152,7 @@ subject(:set_exportable) { vulnerability_export.exportable = exportable } context 'when the exportable is a Project' do - let(:exportable) { create(:project) } + let_it_be(:exportable) { create(:project) } it 'changes the exportable of the export to given project' do expect { set_exportable }.to change { vulnerability_export.exportable }.to(exportable) -- GitLab From 2de437da4907310df5ea03df03d3e21ec9572ed7 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Thu, 18 Jul 2024 16:05:14 +0200 Subject: [PATCH 17/18] Update milestone for feature flag yaml file --- config/feature_flags/gitlab_com_derisk/require_organization.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/feature_flags/gitlab_com_derisk/require_organization.yml b/config/feature_flags/gitlab_com_derisk/require_organization.yml index 452ea242073baf..5c28edea1971cc 100644 --- a/config/feature_flags/gitlab_com_derisk/require_organization.yml +++ b/config/feature_flags/gitlab_com_derisk/require_organization.yml @@ -3,7 +3,7 @@ name: require_organization feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/411832 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/155732 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/470839 -milestone: '17.2' +milestone: '17.3' group: group::tenant scale type: gitlab_com_derisk default_enabled: false -- GitLab From 35d38b77309d507e12ab6696389a90a0c441982f Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Mon, 22 Jul 2024 13:48:25 +0200 Subject: [PATCH 18/18] Fix spec: Use project namespace organization --- ee/spec/services/sbom/ingestion/tasks/ingest_sources_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/services/sbom/ingestion/tasks/ingest_sources_spec.rb b/ee/spec/services/sbom/ingestion/tasks/ingest_sources_spec.rb index 5358249a5b56ff..20c3cd6b720917 100644 --- a/ee/spec/services/sbom/ingestion/tasks/ingest_sources_spec.rb +++ b/ee/spec/services/sbom/ingestion/tasks/ingest_sources_spec.rb @@ -6,7 +6,7 @@ describe '#execute' do let_it_be(:pipeline) { build_stubbed(:ci_pipeline) } - let!(:organization) { create(:organization, :default) } + let(:organization) { pipeline.project.namespace.organization } let(:report_source) { create(:ci_reports_sbom_source) } let(:occurrence_maps) { create_list(:sbom_occurrence_map, 4, report_source: report_source) } -- GitLab