diff --git a/app/models/current.rb b/app/models/current.rb
new file mode 100644
index 0000000000000000000000000000000000000000..b442b3576fb0a22dbcae42eaebfafd2a67b4f80f
--- /dev/null
+++ b/app/models/current.rb
@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+
+class Current < ActiveSupport::CurrentAttributes # rubocop:disable Gitlab/NamespacedClass -- We want this to be top level due to scope of use and no namespace due to ease of calling
+ # watch background jobs need to reset on each job if using
+ attribute :organization
+end
diff --git a/app/models/organizations/organization.rb b/app/models/organizations/organization.rb
index d300f2f225208c10d30fed20f9349eb331e50014..0875b57e79b251e6b764d02661a1df21ebae4ead 100644
--- a/app/models/organizations/organization.rb
+++ b/app/models/organizations/organization.rb
@@ -39,12 +39,16 @@ def self.default_organization
find_by(id: DEFAULT_ORGANIZATION_ID)
end
+ def self.default?(id)
+ id == DEFAULT_ORGANIZATION_ID
+ end
+
def organization_detail
super.presence || build_organization_detail
end
def default?
- id == DEFAULT_ORGANIZATION_ID
+ self.class.default?(id)
end
def to_param
diff --git a/app/models/organizations/organization_setting.rb b/app/models/organizations/organization_setting.rb
index fb141c164d30fe32d27973882bf4adc7a70e9601..7ddf2eb62785a3fee574311afec0ec2d2cfa73a9 100644
--- a/app/models/organizations/organization_setting.rb
+++ b/app/models/organizations/organization_setting.rb
@@ -2,8 +2,6 @@
module Organizations
class OrganizationSetting < ApplicationRecord
- extend ::Organization::CurrentOrganization
-
belongs_to :organization
validates :settings, json_schema: { filename: "organization_settings" }
@@ -20,9 +18,9 @@ class OrganizationSetting < ApplicationRecord
end
def self.for_current_organization
- return unless current_organization
+ return unless Current.organization
- current_organization.settings || current_organization.build_settings
+ Current.organization.settings || Current.organization.build_settings
end
end
end
diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb
index 0fdc73ef2b3b3e0469a8b081bce1cfafd4e16cf4..b99fcad785c55cecf0cfb7c491e948f8b434ad95 100644
--- a/app/services/groups/create_service.rb
+++ b/app/services/groups/create_service.rb
@@ -2,8 +2,6 @@
module Groups
class CreateService < Groups::BaseService
- include Organization::CurrentOrganization
-
def initialize(user, params = {})
@current_user = user
@params = params.dup
@@ -17,7 +15,7 @@ def execute
@group = Group.new(params.except(*::NamespaceSetting.allowed_namespace_settings_params))
- set_organization unless @params[:organization_id]
+ set_organization
@group.build_namespace_settings
handle_namespace_settings
@@ -136,7 +134,11 @@ def organization_setting_valid?
# 2. We shouldn't need to check if this is allowed if the user didn't try to set it themselves. i.e.
# provided in the params
return true if params[:organization_id].blank?
- return true if @group.organization.blank?
+ # There is a chance the organization is still blank(if not default organization), but that is the only case
+ # where we should allow this to not actually be a record in the database.
+ # Otherwise it isn't valid to set this to a non-existent record id and we'll check that in the lines after
+ # this code.
+ return true if @group.organization.blank? && Organizations::Organization.default?(params[:organization_id])
can_create_group_in_organization? && matches_parent_organization?
end
@@ -164,10 +166,16 @@ def inherit_group_shared_runners_settings
end
def set_organization
- if @group.parent_id
+ if params[:organization_id]
+ nil # nothing to do, already assigned from params
+ elsif @group.parent_id
@group.organization = @group.parent.organization
- elsif current_organization
- @group.organization = current_organization
+ # Rely on middleware setting of the organization,
+ # but since we are guarding with feature flag, we need to check it first
+ # Consider replacing elsif with else in cleanup of current_organization_middleware feature flag in
+ # https://gitlab.com/gitlab-org/gitlab/-/issues/441121
+ elsif Current.organization
+ @group.organization = Current.organization
end
end
end
diff --git a/config/feature_flags/gitlab_com_derisk/current_organization_middleware.yml b/config/feature_flags/gitlab_com_derisk/current_organization_middleware.yml
new file mode 100644
index 0000000000000000000000000000000000000000..fc391f89762830fbb91dd94c86f20401be641d19
--- /dev/null
+++ b/config/feature_flags/gitlab_com_derisk/current_organization_middleware.yml
@@ -0,0 +1,9 @@
+---
+name: current_organization_middleware
+feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/437541
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/143978
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/441121
+milestone: '16.10'
+group: group::tenant scale
+type: gitlab_com_derisk
+default_enabled: false
diff --git a/config/initializers/current_organization.rb b/config/initializers/current_organization.rb
new file mode 100644
index 0000000000000000000000000000000000000000..9c9e9ac09d14d64ce4f40a63a65b3b15ac6989f3
--- /dev/null
+++ b/config/initializers/current_organization.rb
@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+Rails.application.configure do |config|
+ config.middleware.use(Gitlab::Middleware::Organizations::Current)
+end
diff --git a/ee/spec/requests/api/graphql/vulnerabilities/issue_links_spec.rb b/ee/spec/requests/api/graphql/vulnerabilities/issue_links_spec.rb
index 8ce9216f2ac50727516732696ece8309bfe06223..fcef65b569e536a9572c6dc64ecd24976e17175e 100644
--- a/ee/spec/requests/api/graphql/vulnerabilities/issue_links_spec.rb
+++ b/ee/spec/requests/api/graphql/vulnerabilities/issue_links_spec.rb
@@ -103,7 +103,8 @@
# 26) Loading the project authorizations
# 27) Loading the namespace
# 28) Loading the user
- expect { query_issue_links }.not_to exceed_query_limit(28)
+ # 29) Loading the organization
+ expect { query_issue_links }.not_to exceed_query_limit(29)
end
end
diff --git a/lib/gitlab/middleware/organizations/current.rb b/lib/gitlab/middleware/organizations/current.rb
new file mode 100644
index 0000000000000000000000000000000000000000..5baf75058615c4ec298c5a4682a63a4105540efc
--- /dev/null
+++ b/lib/gitlab/middleware/organizations/current.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Middleware
+ module Organizations
+ class Current
+ def initialize(app)
+ @app = app
+ end
+
+ def call(env)
+ # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/437541 to enhance the finder:
+ # - Separate logged in vs not logged in user(perhaps using session)
+ # - Authenticated:
+ # - Request header
+ # - Rails session value to drive the finder
+ # - First organization current user is a user of
+ # - Unauthenticated:
+ # - default organization
+ if Feature.enabled?(:current_organization_middleware, type: :gitlab_com_derisk)
+ ::Current.organization = ::Organizations::Organization.default_organization
+ end
+
+ @app.call(env)
+ end
+ end
+ end
+ end
+end
diff --git a/lib/organization/current_organization.rb b/lib/organization/current_organization.rb
deleted file mode 100644
index d1f50aba7a1825a452bcd3739c1055cb82bf931e..0000000000000000000000000000000000000000
--- a/lib/organization/current_organization.rb
+++ /dev/null
@@ -1,23 +0,0 @@
-# frozen_string_literal: true
-
-module Organization
- module CurrentOrganization
- CURRENT_ORGANIZATION_THREAD_VAR = :current_organization
-
- def current_organization
- Thread.current[CURRENT_ORGANIZATION_THREAD_VAR]
- end
-
- def current_organization=(organization)
- Thread.current[CURRENT_ORGANIZATION_THREAD_VAR] = organization
- end
-
- def with_current_organization(organization, &_blk)
- previous_organization = current_organization
- self.current_organization = organization
- yield
- ensure
- self.current_organization = previous_organization
- end
- end
-end
diff --git a/spec/features/groups_spec.rb b/spec/features/groups_spec.rb
index aac2e3c4e46870f0eb2860ca724bb5eafbfd357a..1c6394d3e56196901ed36d87b2b5fafb568ff574 100644
--- a/spec/features/groups_spec.rb
+++ b/spec/features/groups_spec.rb
@@ -35,6 +35,20 @@
expect(page).to have_current_path(group_path(group), ignore_query: true)
expect(page).to have_selector '.visibility-icon [data-testid="earth-icon"]'
end
+
+ context 'with current organization setting in middleware' do
+ it 'sets the organization to the default organization' do
+ default_organization = create(:organization, :default)
+
+ fill_in 'Group name', with: 'test-group'
+ click_button 'Create group'
+
+ group = Group.find_by(name: 'test-group')
+
+ expect(group.organization).to eq(default_organization)
+ expect(page).to have_current_path(group_path(group), ignore_query: true)
+ end
+ end
end
describe 'with expected fields' do
diff --git a/spec/lib/gitlab/middleware/organizations/current_spec.rb b/spec/lib/gitlab/middleware/organizations/current_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..2c84c2975b0c4b101d0e46da7c6be5e7bae6edd0
--- /dev/null
+++ b/spec/lib/gitlab/middleware/organizations/current_spec.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Middleware::Organizations::Current, feature_category: :cell do
+ subject(:perform_request) do
+ path = '/'
+ app = ->(env) { [200, env, 'app'] }
+ middleware = described_class.new(app)
+ Rack::MockRequest.new(middleware).get(path)
+ end
+
+ context 'with an existing default organization' do
+ let_it_be(:organization) { create(:organization, :default) }
+
+ before_all do
+ create(:organization) # prove we are really being selective for the default org
+ end
+
+ after do
+ Current.reset
+ end
+
+ it 'loads the current organization' do
+ perform_request
+
+ expect(Current.organization).to eq(organization)
+ end
+
+ context 'when current_organization_middleware feature flag is disabled' do
+ before do
+ stub_feature_flags(current_organization_middleware: false)
+ end
+
+ it 'does not set the organization' do
+ perform_request
+
+ expect(Current.organization).to eq(nil)
+ end
+ end
+ end
+
+ context 'without an existing default organization' do
+ it 'sets the current organization to nil' do
+ perform_request
+
+ expect(Current.organization).to eq(nil)
+ end
+ end
+end
diff --git a/spec/lib/organization/current_organization_spec.rb b/spec/lib/organization/current_organization_spec.rb
deleted file mode 100644
index ffd37ac4de9eea71cf72f8fc8e6d6f40963f0745..0000000000000000000000000000000000000000
--- a/spec/lib/organization/current_organization_spec.rb
+++ /dev/null
@@ -1,66 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Organization::CurrentOrganization, feature_category: :organization do
- include described_class
-
- after do
- # Wipe thread variables between specs.
- Thread.current[described_class::CURRENT_ORGANIZATION_THREAD_VAR] = nil
- end
-
- describe '.current_organization' do
- subject { current_organization }
-
- context 'when current organization is set' do
- let(:some_organization) { create(:organization) }
-
- before do
- self.current_organization = some_organization
- end
-
- it { is_expected.to eq some_organization }
- end
-
- context 'when organization is not set' do
- it { is_expected.to be_nil }
- end
- end
-
- describe '.current_organization=' do
- subject(:setter) { self.current_organization = some_organization }
-
- let(:some_organization) { create(:organization) }
-
- it 'sets current organization' do
- expect { setter }.to change { current_organization }.from(nil).to(some_organization)
- end
- end
-
- describe '.with_current_organization' do
- let(:some_organization) { create(:organization) }
-
- it 'sets current organization within block' do
- expect(current_organization).to be_nil
- with_current_organization(some_organization) do
- expect(current_organization).to eq some_organization
- end
- expect(current_organization).to be_nil
- end
-
- context 'when an error is raised' do
- it 'resets current organization' do
- begin
- with_current_organization(some_organization) do
- raise StandardError
- end
- rescue StandardError
- nil
- end
-
- expect(current_organization).to be_nil
- end
- end
- end
-end
diff --git a/spec/models/organizations/organization_setting_spec.rb b/spec/models/organizations/organization_setting_spec.rb
index dd90a4d0f58f40b4fbb9ae9597cce30f90ce1f30..b38db1f9a650e896ff26fc90d75c3f44e7716cca 100644
--- a/spec/models/organizations/organization_setting_spec.rb
+++ b/spec/models/organizations/organization_setting_spec.rb
@@ -57,36 +57,26 @@
end
describe '.for_current_organization' do
- let(:dummy_class) { Class.new { extend Organization::CurrentOrganization } }
-
subject(:settings) { described_class.for_current_organization }
context 'when there is no current organization' do
it { is_expected.to be_nil }
end
- context 'when there is a current organization' do
- before do
- dummy_class.current_organization = organization
- end
-
- after do
- dummy_class.current_organization = nil
- end
+ context 'when there is a current organization', :with_current_organization do
+ context 'when current organization has settings' do
+ let_it_be(:organization_setting) { create(:organization_setting, organization: current_organization) }
- it 'returns current organization' do
- expect(settings).to eq(organization_setting)
+ it 'returns current organization' do
+ expect(settings).to eq(organization_setting)
+ end
end
context 'when current organization does not have settings' do
- before do
- allow(organization).to receive(:settings).and_return(nil)
- end
-
it 'returns new settings record' do
new_settings = settings
- expect(new_settings.organization).to eq(organization)
+ expect(new_settings.organization).to eq(current_organization)
expect(new_settings.new_record?).to eq(true)
end
end
diff --git a/spec/models/organizations/organization_spec.rb b/spec/models/organizations/organization_spec.rb
index 4fabf6ec4dcfcd2369ae5c548a3a62962fc64328..d09403512cc38fbfe6b3b3aa3b33ac764e7dfb77 100644
--- a/spec/models/organizations/organization_spec.rb
+++ b/spec/models/organizations/organization_spec.rb
@@ -88,6 +88,20 @@
end
end
+ describe '.default?' do
+ context 'when organization is default' do
+ it 'returns true' do
+ expect(described_class.default?(default_organization.id)).to eq(true)
+ end
+ end
+
+ context 'when organization is not default' do
+ it 'returns false' do
+ expect(described_class.default?(organization.id)).to eq(false)
+ end
+ end
+ end
+
describe '#id' do
context 'when organization is default' do
it 'has id 1' do
diff --git a/spec/requests/api/graphql/mutations/merge_requests/set_assignees_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/set_assignees_spec.rb
index 1bd239ecd87c97717cb270e150e5460c875b8e12..bbac48bccb28d80b28c0d20971385365e581e83a 100644
--- a/spec/requests/api/graphql/mutations/merge_requests/set_assignees_spec.rb
+++ b/spec/requests/api/graphql/mutations/merge_requests/set_assignees_spec.rb
@@ -127,7 +127,7 @@ def run_mutation!
context 'when passing append as true' do
let(:mode) { Types::MutationOperationModeEnum.enum[:append] }
let(:input) { { assignee_usernames: [assignee2.username], operation_mode: mode } }
- let(:db_query_limit) { 25 }
+ let(:db_query_limit) { 26 }
before do
# In CE, APPEND is a NOOP as you can't have multiple assignees
@@ -147,7 +147,7 @@ def run_mutation!
end
context 'when passing remove as true' do
- let(:db_query_limit) { 33 }
+ let(:db_query_limit) { 34 }
let(:mode) { Types::MutationOperationModeEnum.enum[:remove] }
let(:input) { { assignee_usernames: [assignee.username], operation_mode: mode } }
let(:expected_result) { [] }
diff --git a/spec/requests/user_avatar_spec.rb b/spec/requests/user_avatar_spec.rb
index 0a9f3784833e400bb452ec28885fc1341952ec60..f84f59a63fb9057fc2965bea9a8e82bf71df6d92 100644
--- a/spec/requests/user_avatar_spec.rb
+++ b/spec/requests/user_avatar_spec.rb
@@ -20,7 +20,7 @@
get user.avatar_url # Skip queries on first application load
expect(response).to have_gitlab_http_status(:ok)
- expect { get user.avatar_url }.not_to exceed_query_limit(3)
+ expect { get user.avatar_url }.not_to exceed_query_limit(4)
end
end
@@ -30,7 +30,7 @@
get user.avatar_url # Skip queries on first application load
expect(response).to have_gitlab_http_status(:ok)
- expect { get user.avatar_url }.not_to exceed_query_limit(2)
+ expect { get user.avatar_url }.not_to exceed_query_limit(3)
end
end
end
diff --git a/spec/services/groups/create_service_spec.rb b/spec/services/groups/create_service_spec.rb
index a4227b3b28bcd185c68871dfc85216a3f6ae5498..0a65ed4ab165cc0797a7ec46908eaa4c13f48b1e 100644
--- a/spec/services/groups/create_service_spec.rb
+++ b/spec/services/groups/create_service_spec.rb
@@ -153,6 +153,18 @@
it_behaves_like 'creating a group'
end
+ context 'when organization_id is not a valid id' do
+ let(:extra_params) { { organization_id: non_existing_record_id } }
+
+ it_behaves_like 'does not create a group'
+
+ it 'returns an error and does not set organization_id', :aggregate_failures do
+ expect(created_group.errors[:organization_id].first)
+ .to eq(s_("CreateGroup|You don't have permission to create a group in the provided organization."))
+ expect(created_group.organization_id).to be_nil
+ end
+ end
+
context 'when user is an admin', :enable_admin_mode do
let(:current_user) { create(:admin) }
@@ -189,9 +201,8 @@
end
end
- context 'when organization is not set by params' do
+ context 'when organization is not set by params', :with_current_organization do
let_it_be(:default_organization) { create(:organization, :default) }
- let_it_be(:current_organization) { create(:organization, name: 'Current Organization') }
context 'and the parent of the group has an organization' do
let_it_be(:parent_group) { create(:group, organization: other_organization) }
@@ -203,22 +214,18 @@
end
end
- context 'and current_organization is known' do
- before do
- allow_next_instance_of(Groups::CreateService) do |instance|
- allow(instance).to receive(:current_organization).and_return(current_organization)
- end
- end
-
+ context 'and has no parent group' do
it 'creates group with the current organization' do
expect(created_group.organization).to eq(current_organization)
end
end
+ end
- context 'and no group can be found' do
- it 'creates group with the default organization' do
- expect(created_group.organization).to eq(default_organization)
- 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
diff --git a/spec/support/shared_contexts/current_organization_context.rb b/spec/support/shared_contexts/current_organization_context.rb
new file mode 100644
index 0000000000000000000000000000000000000000..d9d4685a4ab6a69878ce5e2b8c57cf4ee08366bf
--- /dev/null
+++ b/spec/support/shared_contexts/current_organization_context.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+# goal of this context: provide an easy process for setting and using the current organization that is set
+# in the middleware for non-feature spec level specs.
+RSpec.shared_context 'with current_organization setting', shared_context: :metadata do # rubocop:disable RSpec/SharedGroupsMetadata -- We are actually using this for easy metadata setting
+ let_it_be(:current_organization, reload: true) { create(:organization, name: 'Current Organization') }
+
+ around do |example|
+ Current.set(organization: current_organization) { example.run }
+ end
+end
+
+RSpec.configure do |rspec|
+ rspec.include_context 'with current_organization setting', with_current_organization: true
+end