diff --git a/app/models/organizations/organization_user.rb b/app/models/organizations/organization_user.rb
index 9e06870dcc6628b47d78c6343f53fdb7d00a4601..5916f8c950fd9979467ff0fbf611d1404c8509f9 100644
--- a/app/models/organizations/organization_user.rb
+++ b/app/models/organizations/organization_user.rb
@@ -16,5 +16,16 @@ class OrganizationUser < ApplicationRecord
}
scope :owners, -> { where(access_level: Gitlab::Access::OWNER) }
+
+ def self.create_default_organization_record_for(user_id, access_level)
+ Organizations::OrganizationUser.upsert(
+ {
+ organization_id: Organizations::Organization::DEFAULT_ORGANIZATION_ID,
+ user_id: user_id,
+ access_level: access_level
+ },
+ unique_by: [:organization_id, :user_id]
+ )
+ end
end
end
diff --git a/app/models/user.rb b/app/models/user.rb
index c9873975cc932135a25bb836221bfa28f8768b49..120ba0a34cf10330f1918c354491f6f451779eb3 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -366,6 +366,8 @@ def update_tracked_fields!(request)
update_invalid_gpg_signatures if previous_changes.key?('email')
end
+ after_create_commit :create_default_organization_user
+
# User's Layout preference
enum layout: { fixed: 0, fluid: 1 }
@@ -2578,6 +2580,19 @@ def prefix_for_feed_token
FEED_TOKEN_PREFIX
end
+ def create_default_organization_user
+ return unless Feature.enabled?(:update_default_organization_users, self, type: :gitlab_com_derisk)
+
+ organization_access_level = if admin?
+ :owner
+ else
+ :default
+ end
+
+ Organizations::OrganizationUser
+ .create_default_organization_record_for(id, organization_access_level)
+ end
+
# method overriden in EE
def audit_lock_access(reason: nil); end
diff --git a/config/feature_flags/gitlab_com_derisk/update_default_organization_users.yml b/config/feature_flags/gitlab_com_derisk/update_default_organization_users.yml
new file mode 100644
index 0000000000000000000000000000000000000000..d4112a9a352eba8283e16a178b92f02c379ee5e0
--- /dev/null
+++ b/config/feature_flags/gitlab_com_derisk/update_default_organization_users.yml
@@ -0,0 +1,9 @@
+---
+name: update_default_organization_users
+feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/437891
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/141500
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/437896
+milestone: '16.9'
+group: group::tenant scale
+type: gitlab_com_derisk
+default_enabled: false
diff --git a/ee/spec/features/registrations/saas/invite_flow_spec.rb b/ee/spec/features/registrations/saas/invite_flow_spec.rb
index 3a0996e49410b53239b19ba543993f0151466bfa..c0832f38d25ba28ccfeb4b2468febfc585cde7cd 100644
--- a/ee/spec/features/registrations/saas/invite_flow_spec.rb
+++ b/ee/spec/features/registrations/saas/invite_flow_spec.rb
@@ -55,7 +55,7 @@ def registers_from_invite(user:, group:)
visit invite_path(invitation.raw_invite_token, invite_type: Emails::Members::INITIAL_INVITE)
# TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/438017
- allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(102)
+ allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(103)
fill_in_sign_up_form(user, invite: true)
end
diff --git a/spec/factories/users.rb b/spec/factories/users.rb
index 7ade859dcf2df56b65dfa2f09cc311d3f928b865..6bad16f81765c3233cc70c5aa56a7b80142a60c7 100644
--- a/spec/factories/users.rb
+++ b/spec/factories/users.rb
@@ -23,6 +23,16 @@
user.assign_personal_namespace if assign_ns
end
+ trait :without_default_org do
+ after(:build) do
+ User.skip_callback(:commit, :after, :create_default_organization_user)
+ end
+
+ after(:create) do
+ User.set_callback(:commit, :after, :create_default_organization_user)
+ end
+ end
+
trait :with_namespace do
namespace { assign_personal_namespace }
end
diff --git a/spec/migrations/fix_broken_user_achievements_awarded_spec.rb b/spec/migrations/fix_broken_user_achievements_awarded_spec.rb
index cb31ca44a9f9d7fb5f1d8b55904b5047d26a211e..8f9d2d30c48ac92ede5e69f581cbea4ebaf68c97 100644
--- a/spec/migrations/fix_broken_user_achievements_awarded_spec.rb
+++ b/spec/migrations/fix_broken_user_achievements_awarded_spec.rb
@@ -32,15 +32,27 @@
awarding_user.delete
end
- it 'migrates the invalid user achievement' do
- expect { migrate! }
- .to change { user_achievement_invalid.reload.awarded_by_user_id }
- .from(nil).to(Users::Internal.ghost.id)
+ context 'when ghost user exists' do
+ let!(:ghost_user) do
+ users_table.create!(username: generate(:username), email: 'ghost@example.com', projects_limit: 0, user_type: 5)
+ end
+
+ it 'migrates the invalid user achievement' do
+ expect { migrate! }
+ .to change { user_achievement_invalid.reload.awarded_by_user_id }
+ .from(nil).to(ghost_user.id)
+ end
+
+ it 'does not migrate the valid user achievement' do
+ expect { migrate! }
+ .not_to change { user_achievement_valid.reload.awarded_by_user_id }
+ end
end
- it 'does not migrate the valid user achievement' do
- expect { migrate! }
- .not_to change { user_achievement_valid.reload.awarded_by_user_id }
+ context 'when ghost user does not exist' do
+ it 'does not migrate the invalid user achievement' do
+ expect { migrate! }.not_to change { user_achievement_invalid.reload.awarded_by_user_id }
+ end
end
end
end
diff --git a/spec/migrations/fix_broken_user_achievements_revoked_spec.rb b/spec/migrations/fix_broken_user_achievements_revoked_spec.rb
index 34517ae67b4594cd4c758d0869c17c389ff7740f..dd8c4b75c7aa5f1de04e12760eb82ca2d8d25d55 100644
--- a/spec/migrations/fix_broken_user_achievements_revoked_spec.rb
+++ b/spec/migrations/fix_broken_user_achievements_revoked_spec.rb
@@ -25,20 +25,32 @@
let(:not_revoked) { user_achievements_table.create!(user_id: user.id, achievement_id: achievement.id) }
describe '#up' do
- it 'migrates the invalid user achievement' do
- expect { migrate! }
- .to change { revoked_invalid.reload.revoked_by_user_id }
- .from(nil).to(Users::Internal.ghost.id)
+ context 'when ghost user exists' do
+ let!(:ghost_user) do
+ users_table.create!(username: generate(:username), email: 'ghost@example.com', projects_limit: 0, user_type: 5)
+ end
+
+ it 'migrates the invalid user achievement' do
+ expect { migrate! }
+ .to change { revoked_invalid.reload.revoked_by_user_id }
+ .from(nil).to(ghost_user.id)
+ end
+
+ it 'does not migrate valid revoked user achievement' do
+ expect { migrate! }
+ .not_to change { revoked_valid.reload.revoked_by_user_id }
+ end
+
+ it 'does not migrate the not revoked user achievement' do
+ expect { migrate! }
+ .not_to change { not_revoked.reload.revoked_by_user_id }
+ end
end
- it 'does not migrate valid revoked user achievement' do
- expect { migrate! }
- .not_to change { revoked_valid.reload.revoked_by_user_id }
- end
-
- it 'does not migrate the not revoked user achievement' do
- expect { migrate! }
- .not_to change { not_revoked.reload.revoked_by_user_id }
+ context 'when ghost user does not exist' do
+ it 'does not migrate the invalid user achievement' do
+ expect { migrate! }.not_to change { revoked_invalid.reload.revoked_by_user_id }
+ end
end
end
end
diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index 3c15992bab207825e77478ceeeadff26e2d288b5..eb227f22b8ae79a76d4c1003cbda22f4b68e8d54 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -1086,6 +1086,8 @@
context 'for updating organization_users' do
let_it_be(:group) { create(:group, :with_organization) }
+ let_it_be(:user) { create(:user) }
+ let(:member) { create(:group_member, source: group, user: user) }
let(:update_organization_users_enabled) { true }
subject(:commit_member) { member }
@@ -1102,8 +1104,6 @@
end
context 'when creating' do
- let(:member) { create(:group_member, source: group) }
-
context 'when update_organization_users is enabled' do
it 'inserts new record on member creation' do
expect { member }.to change { Organizations::OrganizationUser.count }.by(1)
@@ -1175,13 +1175,13 @@
end
context 'when member is an invite' do
- let(:member) { create(:group_member, :invited, source: group) }
+ let(:member) { create(:group_member, :invited, source: group, user: nil) }
it_behaves_like 'does not create an organization_user entry'
end
context 'when organization does not exist' do
- let(:member) { create(:group_member) }
+ let(:member) { create(:group_member, user: user) }
it_behaves_like 'does not create an organization_user entry'
end
diff --git a/spec/models/organizations/organization_user_spec.rb b/spec/models/organizations/organization_user_spec.rb
index c3416c93ec96c188683fb03ba2b17b6eaf68c692..da337c04bc54ce881c8dc8342496073806993a33 100644
--- a/spec/models/organizations/organization_user_spec.rb
+++ b/spec/models/organizations/organization_user_spec.rb
@@ -44,4 +44,65 @@
end
it_behaves_like 'having unique enum values'
+
+ describe '.create_default_organization_record_for' do
+ let_it_be(:default_organization) { create(:organization, :default) }
+ let_it_be(:user) { create(:user, :without_default_org) }
+ let(:access_level) { :default }
+ let(:user_id) { user.id }
+
+ subject(:create_entry) { described_class.create_default_organization_record_for(user_id, access_level) }
+
+ context 'when creating as as default user' do
+ it 'creates record with correct attributes' do
+ expect { create_entry }.to change { described_class.count }.by(1)
+ expect(default_organization.user?(user)).to be(true)
+ end
+ end
+
+ context 'when creating as an owner' do
+ let(:access_level) { :owner }
+
+ it 'creates record with correct attributes' do
+ expect { create_entry }.to change { described_class.count }.by(1)
+ expect(default_organization.owner?(user)).to be(true)
+ end
+ end
+
+ context 'when entry already exists' do
+ let_it_be(:organization_user) { create(:organization_user, user: user, organization: default_organization) }
+
+ it 'does not create or update existing record' do
+ expect { create_entry }.not_to change { described_class.count }
+ end
+
+ context 'when access_level changes' do
+ let(:access_level) { :owner }
+
+ it 'changes access_level on the existing record' do
+ expect(default_organization.owner?(user)).to be(false)
+
+ expect { create_entry }.not_to change { described_class.count }
+
+ expect(default_organization.owner?(user)).to be(true)
+ end
+ end
+ end
+
+ context 'when creating with invalid access_level' do
+ let(:access_level) { :foo }
+
+ it 'raises and error' do
+ expect { create_entry }.to raise_error(ActiveRecord::NotNullViolation)
+ end
+ end
+
+ context 'when creating with invalid user_id' do
+ let(:user_id) { nil }
+
+ it 'raises and error' do
+ expect { create_entry }.to raise_error(ActiveRecord::NotNullViolation)
+ end
+ end
+ end
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 7014c9e685f159b4dc9c7a61fb1015a57eb0a8c3..1a069117f0f0ab98fb3658b932e9fb1ca8f4d6e9 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1751,6 +1751,41 @@
end
end
+ context 'when after_create_commit :create_default_organization_user on default organization' do
+ let_it_be(:default_organization) { create(:organization, :default) }
+ let(:user) { create(:user) }
+
+ subject(:create_user) { user }
+
+ context 'when user is created as an instance admin' do
+ let(:user) { create(:admin) }
+
+ it 'adds user to organization_users as an owner of default organization' do
+ expect { create_user }.to change { Organizations::OrganizationUser.count }.by(1)
+ expect(default_organization.owner?(user)).to be(true)
+ end
+ end
+
+ context 'when user is created as a regular user' do
+ it 'adds user to organization_users as a regular user of default organization' do
+ expect { create_user }.to change { Organizations::OrganizationUser.count }.by(1)
+ expect(default_organization.owner?(user)).to be(false)
+ expect(default_organization.user?(user)).to be(true)
+ end
+ end
+
+ context 'when feature flag update_default_organization_users is disabled' do
+ before do
+ stub_feature_flags(update_default_organization_users: false)
+ end
+
+ it 'adds user to organization_users as a regular user of default organization' do
+ expect { create_user }.not_to change { Organizations::OrganizationUser.count }
+ expect(default_organization.user?(user)).to be(false)
+ end
+ end
+ end
+
describe 'name getters' do
let(:user) { create(:user, name: 'Kane Martin William') }