Merge branch '411832-remove-validation-exception' into 'master'

Remove remaining 'skip validation' exceptions for Organization presence validation on Namespace

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/165533



Merged-by: Gary Holtz <gholtz@gitlab.com>
Approved-by: Gary Holtz <gholtz@gitlab.com>
Co-authored-by: Rutger Wessels <rwessels@gitlab.com>

app/services/groups/create_service.rb

@@ -17,7 +17,7 @@ def execute
       @group.name ||= @group.path.dup
       create_chat_team
-      Namespace.with_disabled_organization_validation { create_group }
+      create_group
       return error_response unless @group.persisted?

app/services/groups/update_service.rb

@@ -39,7 +39,7 @@ def execute
       group.assign_attributes(params)
       begin
-        success = Namespace.with_disabled_organization_validation { group.save }
+        success = group.save
         after_update if success

app/services/users/create_service.rb

@@ -13,9 +13,7 @@ def execute
       user = build_class.new(current_user, params).execute
       reset_token = user.generate_reset_token if user.recently_sent_password_reset?
-      Namespace.with_disabled_organization_validation do
-        after_create_hook(user, reset_token) if user.save
-      end
+      after_create_hook(user, reset_token) if user.save
       user
     end

ee/spec/controllers/ee/registrations_controller_spec.rb

@@ -124,10 +124,6 @@
     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?

ee/spec/features/groups_spec.rb

@@ -2,7 +2,7 @@
 require 'spec_helper'
-RSpec.describe 'Group', feature_category: :groups_and_projects do
+RSpec.describe 'Group', :with_current_organization, feature_category: :groups_and_projects do
   include NamespaceStorageHelpers
   include FreeUserCapHelpers
@@ -187,7 +187,12 @@
   end
   describe 'identity verification', :js, :saas do
-    let_it_be_with_reload(:user) { create(:user, :identity_verification_eligible, :with_sign_ins) }
+    let_it_be_with_reload(:user) do
+      create(
+        :user, :identity_verification_eligible, :with_sign_ins,
+        organizations: [current_organization]
+      )
+    end
     before do
       sign_in(user)
@@ -239,7 +244,7 @@
       end
       context 'when creating a subgroup' do
-        let_it_be(:group) { create(:group, path: 'parent', owners: user) }
+        let_it_be(:group) { create(:group, path: 'parent', organization: current_organization, owners: user) }
         before do
           visit new_group_path(parent_id: group.id, anchor: 'create-group-pane')

ee/spec/features/registrations/combined_registration_spec.rb

@@ -2,10 +2,10 @@
 require 'spec_helper'
-RSpec.describe 'Registration group and project creation flow', :js, feature_category: :onboarding do
+RSpec.describe 'Registration group and project creation flow', :with_current_organization, :js, feature_category: :onboarding do
   include SaasRegistrationHelpers
-  let_it_be(:user) { create(:user, onboarding_in_progress: true) }
+  let_it_be(:user) { create(:user, onboarding_in_progress: true, organizations: [current_organization]) }
   before do
     # https://gitlab.com/gitlab-org/gitlab/-/issues/340302

ee/spec/features/trials/saas/creation_with_no_existing_namespace_flow_spec.rb

@@ -2,8 +2,8 @@
 require 'spec_helper'
-RSpec.describe 'Trial lead submission, group and trial creation', :saas_trial, :js, feature_category: :acquisition do
-  let_it_be(:user) { create(:user) } # rubocop:disable Gitlab/RSpec/AvoidSetup
+RSpec.describe 'Trial lead submission, group and trial creation', :with_current_organization, :saas_trial, :js, feature_category: :acquisition do
+  let_it_be(:user) { create(:user, organizations: [current_organization]) } # rubocop:disable Gitlab/RSpec/AvoidSetup -- We need to ensure user is member of current organization
   context 'when creating lead, group and applying trial is successful' do
     it 'fills out form, testing validations, submits and lands on the group page' do

ee/spec/lib/gitlab/auth/oidc/user_spec.rb

@@ -30,10 +30,6 @@
       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)

ee/spec/requests/api/service_accounts_spec.rb

@@ -2,9 +2,9 @@
 require 'spec_helper'
-RSpec.describe API::ServiceAccounts, :aggregate_failures, feature_category: :user_management do
-  let(:user)  { create(:user) }
-  let(:admin) { create(:admin) }
+RSpec.describe API::ServiceAccounts, :with_current_organization, :aggregate_failures, feature_category: :user_management do
+  let(:user)  { create(:user, organizations: [current_organization]) }
+  let(:admin) { create(:admin, organizations: [current_organization]) }
   let(:license) { create(:license, plan: License::ULTIMATE_PLAN) }
   describe "POST /service_accounts" do

ee/spec/services/ee/groups/create_service_spec.rb

@@ -4,12 +4,14 @@
 RSpec.describe Groups::CreateService, '#execute', feature_category: :groups_and_projects do
   let_it_be(:user) { create(:user) }
+  let_it_be(:organization) { create(:organization, users: [user]) }
   let(:current_user) { user }
   let(:group_params) do
     {
       name: 'GitLab',
       path: 'group_path',
-      visibility_level: Gitlab::VisibilityLevel::PUBLIC
+      visibility_level: Gitlab::VisibilityLevel::PUBLIC,
+      organization_id: organization.id
     }.merge(extra_params)
   end
@@ -46,7 +48,7 @@
   end
   context 'when created group is a sub-group' do
-    let_it_be(:group) { create(:group, owners: user) }
+    let_it_be(:group) { create(:group, organization: organization, owners: user) }
     let(:extra_params) { { parent_id: group.id } }
     include_examples 'sends streaming audit event'

ee/spec/services/ee/users/create_service_spec.rb

@@ -4,13 +4,15 @@
 RSpec.describe Users::CreateService, feature_category: :user_management do
   let_it_be(:current_user) { create(:admin) }
+  let_it_be(:organization) { create(:organization) }
   let(:params) do
     {
       name: 'John Doe',
       username: 'jduser',
       email: 'jd@example.com',
-      password: User.random_password
+      password: User.random_password,
+      organization_id: organization.id
     }
   end

spec/controllers/groups_controller_spec.rb

@@ -2,11 +2,11 @@
 require 'spec_helper'
-RSpec.describe GroupsController, factory_default: :keep, feature_category: :code_review_workflow do
+RSpec.describe GroupsController, :with_current_organization, factory_default: :keep, feature_category: :code_review_workflow do
   include ExternalAuthorizationServiceHelpers
   include AdminModeHelper
-  let_it_be(:group_organization) { create(:organization) }
+  let_it_be(:group_organization) { current_organization }
   let_it_be_with_refind(:group) { create_default(:group, :public, organization: group_organization) }
   let_it_be_with_refind(:project) { create(:project, namespace: group) }
   let_it_be(:user) { create(:user) }
@@ -18,6 +18,10 @@
   let_it_be(:developer) { group.add_developer(create(:user)).user }
   let_it_be(:guest) { group.add_guest(create(:user)).user }
+  before_all do
+    group_organization.users = User.all
+  end
+
   before do
     enable_admin_mode!(admin_with_admin_mode)
   end
@@ -240,7 +244,7 @@
     context 'authorization' do
       it 'allows an admin to create a group' do
-        sign_in(create(:admin))
+        sign_in(admin_without_admin_mode)
         expect do
           post :create, params: { group: { name: 'new_group', path: 'new_group' } }
@@ -301,10 +305,9 @@
       end
     end
-    context 'when creating a top level group', :with_current_organization do
+    context 'when creating a top level group' do
       before do
         sign_in(developer)
-        Current.organization.users << developer
       end
       context 'and can_create_group is enabled' do

spec/features/dashboard/group_spec.rb

@@ -2,10 +2,14 @@
 require 'spec_helper'
-RSpec.describe 'Dashboard Group', :js, feature_category: :groups_and_projects do
+RSpec.describe 'Dashboard Group', :with_current_organization, :js, feature_category: :groups_and_projects do
   let(:user) { create(:user) }
   let(:group) { create(:group) }
+  before do
+    current_organization.users << user
+  end
+
   context 'when user has no groups' do
     before do
       sign_in(user)

spec/features/file_uploads/group_import_spec.rb

@@ -2,10 +2,10 @@
 require 'spec_helper'
-RSpec.describe 'Upload a group export archive', :api, :js, feature_category: :groups_and_projects do
+RSpec.describe 'Upload a group export archive', :with_current_organization, :api, :js, feature_category: :groups_and_projects do
   include_context 'file upload requests helpers'
-  let_it_be(:user) { create(:user, :admin) }
+  let_it_be(:user) { create(:user, :admin, organizations: [current_organization]) }
   let_it_be(:personal_access_token) { create(:personal_access_token, user: user) }
   let(:api_path) { '/groups/import' }

spec/lib/gitlab/auth/ldap/user_spec.rb

@@ -52,7 +52,7 @@
   describe '#valid_sign_in?' do
     before do
-      Namespace.with_disabled_organization_validation { gl_user.save! }
+      gl_user.save!
     end
     it 'returns true' do

spec/lib/gitlab/auth/o_auth/user_spec.rb

@@ -31,10 +31,6 @@
   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' }

spec/services/groups/create_service_spec.rb

@@ -4,8 +4,13 @@
 RSpec.describe Groups::CreateService, '#execute', feature_category: :groups_and_projects do
   let_it_be(:user, reload: true) { create(:user) }
+  let_it_be(:organization) { create(:organization, users: [user]) }
   let(:current_user) { user }
-  let(:group_params) { { path: 'group_path', visibility_level: Gitlab::VisibilityLevel::PUBLIC }.merge(extra_params) }
+  let(:group_params) do
+    { path: 'group_path', visibility_level: Gitlab::VisibilityLevel::PUBLIC,
+      organization_id: organization.id }.merge(extra_params)
+  end
+
   let(:extra_params) { {} }
   let(:created_group) { response[:group] }
@@ -220,11 +225,11 @@
       end
     end
-    context 'when organization is not set by params', :with_current_organization do
+    context 'when organization is not set by params' do
       context 'and the parent of the group has an organization' do
         let_it_be(:parent_group) { create(:group, organization: other_organization) }
-        let(:extra_params) { { parent_id: parent_group.id } }
+        let(:group_params) { { path: 'with-parent', parent_id: parent_group.id } }
         it 'creates group with the parent group organization' do
           expect(created_group.organization).to eq(other_organization)
@@ -232,26 +237,18 @@
       end
     end
-    context 'when organization_id is set to nil' do
+    context 'when organization_id is not specified' do
       let_it_be(:default_organization) { create(:organization, :default) }
-      let(:extra_params) { { organization_id: nil } }
+      let(:group_params) { { path: 'group_path' } }
       it 'creates group in default organization' do
         expect(created_group.organization).to eq(default_organization)
       end
     end
-
-    context 'when organization is not set at all' do
-      it 'creates group without an organization' do
-        expect(created_group.organization).to eq(nil)
-        # let db default happen even if the organization record itself doesn't exist
-        expect(created_group.organization_id).not_to be_nil
-      end
-    end
   end
   context 'for a subgroup' do
-    let_it_be(:group) { create(:group) }
+    let_it_be(:group) { create(:group, organization: organization) }
     let(:extra_params) { { parent_id: group.id } }
     context 'as group owner' do
@@ -337,7 +334,7 @@
     context 'when there is a group-level exclusion' do
       let(:extra_params) { { parent_id: group.id } }
-      let_it_be(:group) { create(:group) { |g| g.add_owner(user) } }
+      let_it_be(:group) { create(:group, organization: organization) { |g| g.add_owner(user) } }
       let_it_be(:group_integration) do
         create(:beyond_identity_integration, group: group, instance: false, active: false)
       end
@@ -363,7 +360,7 @@
     context 'with an active group-level integration' do
       let(:extra_params) { { parent_id: group.id } }
-      let_it_be(:group) { create(:group) { |g| g.add_owner(user) } }
+      let_it_be(:group) { create(:group, organization: organization) { |g| g.add_owner(user) } }
       let_it_be(:group_integration) do
         create(:prometheus_integration, :group, group: group, api_url: 'https://prometheus.group.com/')
       end

spec/services/users/create_service_spec.rb

@@ -4,17 +4,19 @@
 RSpec.describe Users::CreateService, feature_category: :user_management do
   describe '#execute' do
+    let_it_be(:organization) { create(:organization) }
     let(:password) { User.random_password }
     let(:admin_user) { create(:admin) }
+    let(:email) { 'jd@example.com' }
+    let(:base_params) do
+      { name: 'John Doe', username: 'jduser', email: email, password: password, organization_id: organization.id }
+    end
     context 'with an admin user' do
       let(:service) { described_class.new(admin_user, params) }
-      let(:email) { 'jd@example.com' }
       context 'when required parameters are provided' do
-        let(:params) do
-          { name: 'John Doe', username: 'jduser', email: email, password: password }
-        end
+        let(:params) { base_params }
         it 'returns a persisted user' do
           expect(service.execute).to be_persisted
@@ -88,9 +90,7 @@
       end
       context 'when force_random_password parameter is true' do
-        let(:params) do
-          { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: password, force_random_password: true }
-        end
+        let(:params) { base_params.merge(force_random_password: true) }
         it 'generates random password' do
           user = service.execute
@@ -101,15 +101,7 @@
       end
       context 'when password_automatically_set parameter is true' do
-        let(:params) do
-          {
-            name: 'John Doe',
-            username: 'jduser',
-            email: 'jd@example.com',
-            password: password,
-            password_automatically_set: true
-          }
-        end
+        let(:params) { base_params.merge(password_automatically_set: true) }
         it 'persists the given attributes' do
           user = service.execute
@@ -127,9 +119,7 @@
       end
       context 'when skip_confirmation parameter is true' do
-        let(:params) do
-          { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: password, skip_confirmation: true }
-        end
+        let(:params) { base_params.merge(skip_confirmation: true) }
         it 'confirms the user' do
           expect(service.execute).to be_confirmed
@@ -137,9 +127,7 @@
       end
       context 'when reset_password parameter is true' do
-        let(:params) do
-          { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: password, reset_password: true }
-        end
+        let(:params) { base_params.merge(reset_password: true) }
         it 'resets password even if a password parameter is given' do
           expect(service.execute).to be_recently_sent_password_reset
@@ -158,9 +146,7 @@
     end
     context 'with nil user' do
-      let(:params) do
-        { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: password, skip_confirmation: true }
-      end
+      let(:params) { base_params.merge(skip_confirmation: true) }
       let(:service) { described_class.new(nil, params) }

spec/support/helpers/disable_namespace_organization_validation.yml

@@ -23,6 +23,4 @@
 - spec/lib/gitlab/auth/saml/user_spec.rb
 - spec/models/hooks/system_hook_spec.rb
 - spec/requests/api/groups_spec.rb
-- spec/services/users/create_service_spec.rb
-- spec/services/groups/create_service_spec.rb
 - spec/services/resource_access_tokens/create_service_spec.rb