From f6e907de1db1bb2a798fe62069419b5250c1a9ff Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Tue, 3 Sep 2024 14:33:34 +0200 Subject: [PATCH 1/3] Explicitly pass organization_id to ProvisionService objects --- .../gitlab/scim/group/provisioning_service.rb | 5 ++--- ee/lib/ee/gitlab/scim/provisioning_service.rb | 4 +--- .../scim/group/provisioning_service_spec.rb | 1 + .../gitlab/scim/provisioning_service_spec.rb | 20 +++++++++++++------ ...able_namespace_organization_validation.yml | 2 -- 5 files changed, 18 insertions(+), 14 deletions(-) diff --git a/ee/lib/ee/gitlab/scim/group/provisioning_service.rb b/ee/lib/ee/gitlab/scim/group/provisioning_service.rb index 91ae3a7e37ee24..6f1a83c6ef2e7b 100644 --- a/ee/lib/ee/gitlab/scim/group/provisioning_service.rb +++ b/ee/lib/ee/gitlab/scim/group/provisioning_service.rb @@ -72,6 +72,7 @@ def group_user_params hash[:saml_provider_id] = @group.saml_provider.id hash[:group_id] = @group&.id hash[:provider] = ::Users::BuildService::GROUP_SCIM_PROVIDER + hash[:organization_id] = @group.organization_id end end @@ -82,9 +83,7 @@ def create_identity_and_member end def create_user_and_member - saved = ::Namespace.with_disabled_organization_validation do - user.save && member.errors.empty? - end + saved = user.save && member.errors.empty? return success_response if saved diff --git a/ee/lib/ee/gitlab/scim/provisioning_service.rb b/ee/lib/ee/gitlab/scim/provisioning_service.rb index d7af5b01ee9370..33b18702906a49 100644 --- a/ee/lib/ee/gitlab/scim/provisioning_service.rb +++ b/ee/lib/ee/gitlab/scim/provisioning_service.rb @@ -53,9 +53,7 @@ def existing_user? end def create_identity_and_user - saved = ::Namespace.with_disabled_organization_validation do - user.save && identity.save - end + saved = user.save && identity.save return success_response if saved diff --git a/ee/spec/lib/ee/gitlab/scim/group/provisioning_service_spec.rb b/ee/spec/lib/ee/gitlab/scim/group/provisioning_service_spec.rb index b557e357d3527f..7d5291355db94f 100644 --- a/ee/spec/lib/ee/gitlab/scim/group/provisioning_service_spec.rb +++ b/ee/spec/lib/ee/gitlab/scim/group/provisioning_service_spec.rb @@ -78,6 +78,7 @@ def user service.execute expect(user).to be_a(User) + expect(user.namespace.organization_id).to eq(group.organization_id) end context 'when access level is given for created group member' do diff --git a/ee/spec/lib/ee/gitlab/scim/provisioning_service_spec.rb b/ee/spec/lib/ee/gitlab/scim/provisioning_service_spec.rb index 464063fd7903c9..19aacd97d4526d 100644 --- a/ee/spec/lib/ee/gitlab/scim/provisioning_service_spec.rb +++ b/ee/spec/lib/ee/gitlab/scim/provisioning_service_spec.rb @@ -6,13 +6,16 @@ include LoginHelpers describe '#execute' do + let_it_be(:organization) { create(:organization) } + let(:service) { described_class.new(service_params) } let_it_be(:service_params) do { email: 'work@example.com', name: 'Test Name', extern_uid: 'test_uid', - username: 'username' + username: 'username', + organization_id: organization.id } end @@ -40,7 +43,8 @@ email: 'work@example.com', name: 'Test Name', extern_uid: 'test_uid', - username: 'username' + username: 'username', + organization_id: organization.id } end @@ -101,7 +105,8 @@ def user email: 'work@example.com', name: 'Test Name', extern_uid: 'test_uid', - username: ' --ricky.^#!__the._raccoon--' + username: ' --ricky.^#!__the._raccoon--', + organization_id: organization.id } end @@ -128,7 +133,8 @@ def user { email: 'work@example.com', name: 'Test Name', - extern_uid: 'test_uid' + extern_uid: 'test_uid', + organization_id: organization.id } end @@ -146,7 +152,8 @@ def user email: 'work@example.com', name: 'Test Name', extern_uid: '', - username: '' + username: '', + organization_id: organization.id } end @@ -193,7 +200,8 @@ def user email: 'work@example.com', name: 'Test Name', extern_uid: '', - username: 'username' + username: 'username', + organization_id: organization.id } end diff --git a/spec/support/helpers/disable_namespace_organization_validation.yml b/spec/support/helpers/disable_namespace_organization_validation.yml index 5073522243d3cb..04fe4bd51f89a4 100644 --- a/spec/support/helpers/disable_namespace_organization_validation.yml +++ b/spec/support/helpers/disable_namespace_organization_validation.yml @@ -21,8 +21,6 @@ - ee/spec/features/trials/saas/creation_with_no_existing_namespace_flow_spec.rb - ee/spec/lib/gitlab/auth/group_saml/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/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 -- GitLab From 8102456897de8976e7f84cdda7b83b9481dc91e0 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Tue, 3 Sep 2024 15:29:49 +0200 Subject: [PATCH 2/3] Pass Current.organization_id to ProvisioningService --- ee/lib/api/scim/instance_scim.rb | 4 +++- ee/spec/requests/api/scim/instance_scim_spec.rb | 6 +++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/ee/lib/api/scim/instance_scim.rb b/ee/lib/api/scim/instance_scim.rb index 8fadff2682f542..5f71d726192392 100644 --- a/ee/lib/api/scim/instance_scim.rb +++ b/ee/lib/api/scim/instance_scim.rb @@ -105,7 +105,9 @@ def reprovision(identity) post do check_access! parser = ::EE::Gitlab::Scim::ParamsParser.new(params) - result = ::EE::Gitlab::Scim::ProvisioningService.new(parser.post_params).execute + result = ::EE::Gitlab::Scim::ProvisioningService.new( + parser.post_params.merge(organization_id: ::Current.organization_id) + ).execute case result.status when :success diff --git a/ee/spec/requests/api/scim/instance_scim_spec.rb b/ee/spec/requests/api/scim/instance_scim_spec.rb index b195608b6cdac8..03e75f8d8fdb50 100644 --- a/ee/spec/requests/api/scim/instance_scim_spec.rb +++ b/ee/spec/requests/api/scim/instance_scim_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::Scim::InstanceScim, feature_category: :system_access do +RSpec.describe API::Scim::InstanceScim, :with_default_organization, feature_category: :system_access do include LoginHelpers let(:user) { create(:user) } @@ -231,6 +231,10 @@ it 'responds with 201 and the scim user attributes' do create(:user, email: email) + expect(::EE::Gitlab::Scim::ProvisioningService).to receive(:new).with( + hash_including(organization_id: Organizations::Organization.default_organization.id) + ).and_call_original + api_request expect(response).to have_gitlab_http_status(:created) -- GitLab From 1af1feaed23e0a4dbae2869af5ad8e7a26c43146 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Fri, 6 Sep 2024 13:28:24 +0200 Subject: [PATCH 3/3] Remove redundant assignment --- ee/lib/ee/gitlab/scim/group/provisioning_service.rb | 4 +--- ee/lib/ee/gitlab/scim/provisioning_service.rb | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/ee/lib/ee/gitlab/scim/group/provisioning_service.rb b/ee/lib/ee/gitlab/scim/group/provisioning_service.rb index 6f1a83c6ef2e7b..fa8fdfa9cc1d65 100644 --- a/ee/lib/ee/gitlab/scim/group/provisioning_service.rb +++ b/ee/lib/ee/gitlab/scim/group/provisioning_service.rb @@ -83,9 +83,7 @@ def create_identity_and_member end def create_user_and_member - saved = user.save && member.errors.empty? - - return success_response if saved + return success_response if user.save && member.errors.empty? 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 33b18702906a49..6bdb3e102c6c57 100644 --- a/ee/lib/ee/gitlab/scim/provisioning_service.rb +++ b/ee/lib/ee/gitlab/scim/provisioning_service.rb @@ -53,9 +53,7 @@ def existing_user? end def create_identity_and_user - saved = user.save && identity.save - - return success_response if saved + return success_response if user.save && identity.save error_response(objects: [identity, user]) end -- GitLab