From 3d016c547a5af77625457f836e1cf6756d839553 Mon Sep 17 00:00:00 2001
From: Jason Goodman <jgoodman@gitlab.com>
Date: Wed, 31 Jan 2024 01:44:51 +0000
Subject: [PATCH] Allow new guest users exceeding instance user cap on ultimate
license
This is behind a feature flag.
When the feature flag is enabled, new non-billable users will be set
to active even if they exceed the instance user cap.
With the feature flag disabled, any new human user, billable or not,
will be blocked pending approval if the instance user cap is exceeded.
---
ee/app/models/ee/user.rb | 8 +++
ee/app/services/ee/users/build_service.rb | 15 +++++-
...nbillable_users_over_instance_user_cap.yml | 9 ++++
ee/spec/models/ee/user_spec.rb | 31 ++++++++++++
.../ee/registrations_controller_spec.rb | 23 +++++++++
.../services/ee/users/build_service_spec.rb | 50 +++++++++++++++++++
6 files changed, 135 insertions(+), 1 deletion(-)
create mode 100644 ee/config/feature_flags/wip/activate_nonbillable_users_over_instance_user_cap.yml
diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb
index 531e8ba670263..7dadf830de06a 100644
--- a/ee/app/models/ee/user.rb
+++ b/ee/app/models/ee/user.rb
@@ -252,6 +252,14 @@ def clear_group_with_ai_available_cache(ids)
end
end
+ def pending_billable_invitations
+ if ::License.current.exclude_guests_from_active_count?
+ pending_invitations.where('access_level > ?', ::Gitlab::Access::GUEST)
+ else
+ pending_invitations
+ end
+ end
+
def external?
return true if security_policy_bot?
diff --git a/ee/app/services/ee/users/build_service.rb b/ee/app/services/ee/users/build_service.rb
index 6bd9880b20e38..53129ecb336f2 100644
--- a/ee/app/services/ee/users/build_service.rb
+++ b/ee/app/services/ee/users/build_service.rb
@@ -117,10 +117,23 @@ def build_scim_identity
def set_pending_approval_state
return unless ::User.user_cap_reached?
- return unless user.human?
+
+ if ::Feature.enabled?(:activate_nonbillable_users_over_instance_user_cap, type: :wip)
+ return unless will_be_billable?(user)
+ else
+ return unless user.human?
+ end
user.state = ::User::BLOCKED_PENDING_APPROVAL_STATE
end
+
+ def will_be_billable?(user)
+ user.human? && (all_humans_are_billable? || user.pending_billable_invitations.any?)
+ end
+
+ def all_humans_are_billable?
+ !::License.current.exclude_guests_from_active_count?
+ end
end
end
end
diff --git a/ee/config/feature_flags/wip/activate_nonbillable_users_over_instance_user_cap.yml b/ee/config/feature_flags/wip/activate_nonbillable_users_over_instance_user_cap.yml
new file mode 100644
index 0000000000000..1233831da721e
--- /dev/null
+++ b/ee/config/feature_flags/wip/activate_nonbillable_users_over_instance_user_cap.yml
@@ -0,0 +1,9 @@
+---
+name: activate_nonbillable_users_over_instance_user_cap
+feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/361563
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/142468
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/439520
+milestone: '16.9'
+group: group::utilization
+type: wip
+default_enabled: false
diff --git a/ee/spec/models/ee/user_spec.rb b/ee/spec/models/ee/user_spec.rb
index a0d77b25d0bd9..948609daeb9f6 100644
--- a/ee/spec/models/ee/user_spec.rb
+++ b/ee/spec/models/ee/user_spec.rb
@@ -1296,6 +1296,37 @@
end
end
+ describe '#pending_billable_invitations' do
+ let_it_be(:user) { described_class.new(confirmed_at: Time.zone.now, email: 'test@example.com') }
+
+ it 'returns pending billable invitations for the user' do
+ invitation = create(:group_member, :guest, :invited, invite_email: user.email)
+
+ expect(user.pending_billable_invitations).to eq([invitation])
+ end
+
+ it 'returns both project and group invitations' do
+ project_invitation = create(:project_member, :maintainer, :invited, invite_email: user.email)
+ group_invitation = create(:group_member, :developer, :invited, invite_email: user.email)
+
+ expect(user.pending_billable_invitations).to contain_exactly(project_invitation, group_invitation)
+ end
+
+ context 'with an ultimate license' do
+ before do
+ license = create(:license, plan: License::ULTIMATE_PLAN)
+ allow(License).to receive(:current).and_return(license)
+ end
+
+ it 'excludes pending non-billable invitations for the user' do
+ create(:group_member, :guest, :invited, invite_email: user.email)
+ developer_invitation = create(:group_member, :developer, :invited, invite_email: user.email)
+
+ expect(user.pending_billable_invitations).to eq([developer_invitation])
+ end
+ end
+ end
+
describe '#group_managed_account?' do
subject { user.group_managed_account? }
diff --git a/ee/spec/requests/ee/registrations_controller_spec.rb b/ee/spec/requests/ee/registrations_controller_spec.rb
index a62e144959222..3e6ac0b398ca3 100644
--- a/ee/spec/requests/ee/registrations_controller_spec.rb
+++ b/ee/spec/requests/ee/registrations_controller_spec.rb
@@ -205,5 +205,28 @@
create_user
end
end
+
+ describe 'user signup cap' do
+ before do
+ stub_application_setting(require_admin_approval_after_user_signup: false)
+ end
+
+ context 'when user signup cap is exceeded on an ultimate license' do
+ before do
+ allow(Gitlab::CurrentSettings).to receive(:new_user_signups_cap).and_return(1)
+ create(:group_member, :developer)
+
+ license = create(:license, plan: License::ULTIMATE_PLAN)
+ allow(License).to receive(:current).and_return(license)
+ end
+
+ it 'sets a new non-billable user state to active' do
+ create_user
+
+ user = User.find_by(email: user_attrs[:email])
+ expect(user).to be_active
+ end
+ end
+ end
end
end
diff --git a/ee/spec/services/ee/users/build_service_spec.rb b/ee/spec/services/ee/users/build_service_spec.rb
index 9dcf3fa78ade0..3f038915a00d3 100644
--- a/ee/spec/services/ee/users/build_service_spec.rb
+++ b/ee/spec/services/ee/users/build_service_spec.rb
@@ -158,6 +158,56 @@
expect(user).to be_active
end
end
+
+ context 'with an ultimate license' do
+ let_it_be(:group) { create(:group) }
+ let_it_be(:billable_users) { create_list(:user, 3) }
+
+ before_all do
+ billable_users.each { |u| group.add_developer(u) }
+ end
+
+ before do
+ license = create(:license, plan: License::ULTIMATE_PLAN)
+ allow(License).to receive(:current).and_return(license)
+ end
+
+ it 'sets a new billable user state to blocked pending approval' do
+ member = create(:group_member, :developer, :invited)
+ params.merge!(email: member.invite_email, skip_confirmation: true)
+
+ user = service.execute
+
+ expect(user).to be_blocked_pending_approval
+ end
+
+ it 'sets a new non-billable user state to active' do
+ user = service.execute
+
+ expect(user).to be_active
+ end
+
+ context 'when the feature flag is disabled' do
+ before do
+ stub_feature_flags(activate_nonbillable_users_over_instance_user_cap: false)
+ end
+
+ it 'sets a new billable user state to blocked pending approval' do
+ member = create(:group_member, :developer, :invited)
+ params.merge!(email: member.invite_email, skip_confirmation: true)
+
+ user = service.execute
+
+ expect(user).to be_blocked_pending_approval
+ end
+
+ it 'sets a new non-billable user state to blocked pending approval' do
+ user = service.execute
+
+ expect(user).to be_blocked_pending_approval
+ end
+ end
+ end
end
context 'when user signup cap is not set' do
--
GitLab