diff --git a/app/models/email.rb b/app/models/email.rb
index 5c74dce73083003828c623e5e84a114585d09bde..c2e6919322358d950a0bf416ad8617c468b25fbe 100644
--- a/app/models/email.rb
+++ b/app/models/email.rb
@@ -11,6 +11,12 @@ class Email < ApplicationRecord
validate :unique_email, if: ->(email) { email.email_changed? }
+ scope :users_by_detumbled_email_count, ->(email) do
+ normalized_email = ::Gitlab::Utils::Email.normalize_email(email)
+
+ where(detumbled_email: normalized_email).distinct.count(:user_id)
+ end
+
scope :confirmed, -> { where.not(confirmed_at: nil) }
scope :unconfirmed, -> { where(confirmed_at: nil) }
scope :unconfirmed_and_created_before, ->(created_cut_off) { unconfirmed.where('created_at < ?', created_cut_off) }
diff --git a/app/models/user.rb b/app/models/user.rb
index 866fc089b297c58fff6325f9072b721816d2b26b..c98b72bf1c8e1052151ad03644e1cf69dc60f234 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -341,6 +341,7 @@ def update_tracked_fields!(request)
validate :namespace_move_dir_allowed, if: :username_changed?, unless: :new_record?
validate :unique_email, if: :email_changed?
+ validates_with AntiAbuse::UniqueDetumbledEmailValidator, if: :email_changed?
validate :notification_email_verified, if: :notification_email_changed?
validate :public_email_verified, if: :public_email_changed?
validate :commit_email_verified, if: :commit_email_changed?
@@ -1253,15 +1254,6 @@ def unique_email
errors.add(:email, _('is linked to an account pending deletion.'), help_page_url: help_page_url)
end
-
- banned_user_email_reuse_check unless errors.include?(:email)
- end
-
- def banned_user_email_reuse_check
- return unless ::Feature.enabled?(:block_banned_user_normalized_email_reuse, ::Feature.current_request)
- return unless ::Users::BannedUser.by_detumbled_email(email).exists?
-
- errors.add(:email, _('is not allowed. Please enter a different email address and try again.'))
end
def commit_email_or_default
diff --git a/app/validators/anti_abuse/unique_detumbled_email_validator.rb b/app/validators/anti_abuse/unique_detumbled_email_validator.rb
new file mode 100644
index 0000000000000000000000000000000000000000..e471b49b4b899be8cc8cbb5d1606357a70f17b91
--- /dev/null
+++ b/app/validators/anti_abuse/unique_detumbled_email_validator.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+module AntiAbuse
+ class UniqueDetumbledEmailValidator < ActiveModel::Validator
+ NORMALIZED_EMAIL_ACCOUNT_LIMIT = 5
+
+ def validate(record)
+ return if record.errors.include?(:email)
+
+ email = record.email
+
+ return unless prevent_banned_user_email_reuse?(email) || limit_normalized_email_reuse?(email)
+
+ record.errors.add(:email, _('is not allowed. Please enter a different email address and try again.'))
+ end
+
+ private
+
+ def prevent_banned_user_email_reuse?(email)
+ return false unless ::Feature.enabled?(:block_banned_user_normalized_email_reuse, ::Feature.current_request)
+
+ ::Users::BannedUser.by_detumbled_email(email).exists?
+ end
+
+ def limit_normalized_email_reuse?(email)
+ return false unless ::Feature.enabled?(:limit_normalized_email_reuse, ::Feature.current_request)
+
+ Email.users_by_detumbled_email_count(email) >= NORMALIZED_EMAIL_ACCOUNT_LIMIT
+ end
+ end
+end
+
+AntiAbuse::UniqueDetumbledEmailValidator.prepend_mod
diff --git a/config/feature_flags/gitlab_com_derisk/limit_normalized_email_reuse.yml b/config/feature_flags/gitlab_com_derisk/limit_normalized_email_reuse.yml
new file mode 100644
index 0000000000000000000000000000000000000000..d9001d6c10f0fe34e43f3cd15017542c025e7c32
--- /dev/null
+++ b/config/feature_flags/gitlab_com_derisk/limit_normalized_email_reuse.yml
@@ -0,0 +1,9 @@
+---
+name: limit_normalized_email_reuse
+feature_issue_url: https://gitlab.com/gitlab-org/modelops/anti-abuse/team-tasks/-/issues/812
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/167357
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/495124
+milestone: '17.6'
+group: group::anti-abuse
+type: gitlab_com_derisk
+default_enabled: false
diff --git a/ee/app/validators/ee/anti_abuse/unique_detumbled_email_validator.rb b/ee/app/validators/ee/anti_abuse/unique_detumbled_email_validator.rb
new file mode 100644
index 0000000000000000000000000000000000000000..d63a5e79e4469c3beac527dcf1e0ab1ba5735ce0
--- /dev/null
+++ b/ee/app/validators/ee/anti_abuse/unique_detumbled_email_validator.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+module EE
+ module AntiAbuse
+ module UniqueDetumbledEmailValidator
+ extend ::Gitlab::Utils::Override
+
+ private
+
+ override :limit_normalized_email_reuse?
+ def limit_normalized_email_reuse?(email)
+ super && !paid_verified_domain?(email)
+ end
+
+ def paid_verified_domain?(email)
+ return false unless ::Gitlab::Saas.feature_available?(:limit_normalized_email_reuse)
+
+ email_domain = Mail::Address.new(email).domain.downcase
+
+ return true if email_domain == ::Gitlab::Saas.root_domain
+
+ root_group = ::PagesDomain.verified.find_by_domain_case_insensitive(email_domain)&.root_group
+
+ !!root_group&.paid?
+ end
+ end
+ end
+end
diff --git a/ee/config/saas_features/limit_normalized_email_reuse.yml b/ee/config/saas_features/limit_normalized_email_reuse.yml
new file mode 100644
index 0000000000000000000000000000000000000000..5159a0df42ee129800baeb19fdbba10ca6c04b93
--- /dev/null
+++ b/ee/config/saas_features/limit_normalized_email_reuse.yml
@@ -0,0 +1,5 @@
+---
+name: limit_normalized_email_reuse
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/167357
+milestone: '17.6'
+group: group::anti-abuse
diff --git a/ee/lib/ee/gitlab/saas.rb b/ee/lib/ee/gitlab/saas.rb
index d86fcbf795ce89521cb84788be9cff56d9fe297c..8f9928d18e71165e575dc2f0bc55217967e15908 100644
--- a/ee/lib/ee/gitlab/saas.rb
+++ b/ee/lib/ee/gitlab/saas.rb
@@ -32,6 +32,7 @@ module Saas
gitlab_duo_saas_only
pipl_compliance
ci_runners_allowed_plans
+ limit_normalized_email_reuse
].freeze
CONFIG_FILE_ROOT = 'ee/config/saas_features'
diff --git a/ee/spec/models/ee/user_spec.rb b/ee/spec/models/ee/user_spec.rb
index 481f13b88ca14231e3e99f7d57bb1ae9dba7495b..c2aa78f2768786333d4b8472de7f5e0c4d2ccb93 100644
--- a/ee/spec/models/ee/user_spec.rb
+++ b/ee/spec/models/ee/user_spec.rb
@@ -3643,4 +3643,78 @@
end
end
end
+
+ context 'normalized email reuse check' do
+ let(:error_message) { 'Email is not allowed. Please enter a different email address and try again.' }
+ let(:tumbled_email) { 'user+inbox1@test.com' }
+ let(:normalized_email) { 'user@test.com' }
+
+ subject(:new_user) { build(:user, email: tumbled_email).tap(&:valid?) }
+
+ before do
+ stub_const("::AntiAbuse::UniqueDetumbledEmailValidator::NORMALIZED_EMAIL_ACCOUNT_LIMIT", 1)
+ create(:user, email: normalized_email)
+ end
+
+ context 'when the user has a gitlab.com email address' do
+ let(:tumbled_email) { 'user+inbox1@gitlab.com' }
+ let(:normalized_email) { 'user@gitlab.com' }
+
+ context 'when running in saas', :saas do
+ it 'does not add an error' do
+ expect(new_user.errors).to be_empty
+ end
+ end
+
+ context 'when not running in saas' do
+ it 'adds a validation error' do
+ expect(new_user.errors.full_messages).to include(error_message)
+ end
+ end
+ end
+
+ context 'when a saas user has a an email associated with a verified domain', :saas do
+ let(:verified_domain) { 'verified.com' }
+ let(:tumbled_email) { "user+inbox1@#{verified_domain}" }
+ let(:normalized_email) { "user@#{verified_domain}" }
+
+ context 'when the group is paid' do
+ let_it_be(:ultimate_group) { create(:group_with_plan, plan: :ultimate_plan) }
+ let_it_be(:ultimate_project) { create(:project, group: ultimate_group) }
+
+ it 'does not add an error' do
+ create(:pages_domain, domain: verified_domain, project: ultimate_project)
+
+ expect(new_user.errors).to be_empty
+ end
+ end
+
+ context 'when the root group is not paid' do
+ let_it_be(:free_group) { create(:group) }
+ let_it_be(:free_project) { create(:project, group: free_group) }
+
+ it 'adds a validation error' do
+ create(:pages_domain, domain: verified_domain, project: free_project)
+
+ expect(new_user.errors.full_messages).to include(error_message)
+ end
+ end
+
+ context 'when the root group does not exist' do
+ let_it_be(:project) { create(:project) }
+
+ it 'adds a validation error' do
+ create(:pages_domain, domain: verified_domain, project: project)
+
+ expect(new_user.errors.full_messages).to include(error_message)
+ end
+ end
+ end
+
+ context 'when user is on a self-managed instance' do
+ it 'does not check domain verification' do
+ expect(::PagesDomain).not_to receive(:verified)
+ end
+ end
+ end
end
diff --git a/lib/gitlab/saas.rb b/lib/gitlab/saas.rb
index 754eed4f25375c5945e89d4cc27c88706dcf5365..847ac5c36de12efe0bce1da5c41b624da628e61d 100644
--- a/lib/gitlab/saas.rb
+++ b/lib/gitlab/saas.rb
@@ -5,6 +5,10 @@
module Gitlab
module Saas
+ def self.root_domain
+ 'gitlab.com'
+ end
+
def self.com_url
'https://gitlab.com'
end
diff --git a/spec/lib/gitlab/saas_spec.rb b/spec/lib/gitlab/saas_spec.rb
index 3be0a6c7bf08ec863b29e58bd483f08fafb1819c..e9132b7a2290d832cdf94529975879f0af940322 100644
--- a/spec/lib/gitlab/saas_spec.rb
+++ b/spec/lib/gitlab/saas_spec.rb
@@ -6,6 +6,12 @@
RSpec.describe Gitlab::Saas, feature_category: :shared do
include SaasTestHelper
+ describe '.root_domain' do
+ subject { described_class.root_domain }
+
+ it { is_expected.to eq('gitlab.com') }
+ end
+
describe '.canary_toggle_com_url' do
subject { described_class.canary_toggle_com_url }
diff --git a/spec/models/email_spec.rb b/spec/models/email_spec.rb
index c21612ae8277f5d7847c80beab4eb6e02b6d0d7e..bdc4945bc729e0c2c24982863d1cc7a573135170 100644
--- a/spec/models/email_spec.rb
+++ b/spec/models/email_spec.rb
@@ -59,6 +59,22 @@
let_it_be(:unconfirmed_secondary_email) { create(:email, user: confirmed_user) }
let_it_be(:confirmed_secondary_email) { create(:email, :confirmed, user: confirmed_user) }
+ describe '.users_by_detumbled_email_count' do
+ before do
+ # Create users with primary and secondary emails that detumble to the same email: user@example.com.
+ user = create(:user, email: 'user+A@example.com') # New user with a matching primary email
+ create(:user, email: 'user+B@example.com') # New user with a matching primary email
+ create(:email, user: user, email: 'user+C@example.com') # Duplicate user with a matching secondary email
+ create(:email, email: 'user+D@example.com') # New user with a matching secondary email
+ end
+
+ # We created 4 emails but this method should only return a count of 3 since one user
+ # has a primary and secondary email that detumble to the same email address.
+ it 'return the count of unique users with the same detumbled email address' do
+ expect(described_class.users_by_detumbled_email_count('user@example.com')).to eq(3)
+ end
+ end
+
describe '.confirmed' do
it 'returns confirmed emails' do
expect(described_class.confirmed).to contain_exactly(
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 062321a1aa11e9cf2104c4f21a9b2829e368b7f5..f7ae0289e7e024e136ad575361ec7ad4ef271a50 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -8834,51 +8834,148 @@ def owner_class_attribute_default; end
end
end
- context 'banned user normalized email reuse check' do
- let_it_be(:existing_user) { create(:user) }
+ context 'normalized email reuse check' do
+ let(:error_message) { 'Email is not allowed. Please enter a different email address and try again.' }
+
+ subject(:new_user) { build(:user, email: tumbled_email).tap(&:valid?) }
- shared_examples 'does not perform the check' do
+ shared_examples 'adds a validation error' do
specify do
+ subject
+
+ expect(new_user.errors.full_messages).to include(error_message)
+ end
+ end
+
+ shared_examples 'checking normalized email reuse limit' do
+ before do
+ stub_const("AntiAbuse::UniqueDetumbledEmailValidator::NORMALIZED_EMAIL_ACCOUNT_LIMIT", 2)
+ end
+
+ context 'when the normalized email limit has been reached by unique users' do
+ before do
+ create(:user, email: tumbled_email.split('@').join('1@'))
+ end
+
+ it_behaves_like 'adds a validation error'
+
+ it 'performs the normalized email limit check' do
+ expect(Email).to receive(:users_by_detumbled_email_count).and_call_original
+
+ subject
+ end
+ end
+
+ context 'when the normalized email limit has been reached by non-unique users' do
+ before do
+ user = described_class.find_by(email: normalized_email)
+ create(:email, user: user, email: tumbled_email.split('@').join('1@'))
+ end
+
+ it 'does not add an error' do
+ expect(new_user.errors).to be_empty
+ end
+ end
+
+ context 'when the normalized email limit has not been reached' do
+ it 'does not add an error' do
+ expect(new_user.errors).to be_empty
+ end
+ end
+
+ context 'when the feature flag is disabled' do
+ before do
+ stub_feature_flags(limit_normalized_email_reuse: false)
+ end
+
+ it 'does not perform the check' do
+ expect(Email).not_to receive(:users_by_detumbled_email_count)
+
+ subject
+ end
+ end
+ end
+
+ context 'when email has other validation errors' do
+ subject(:new_user) { build(:user, email: 'invalid-email').tap(&:valid?) }
+
+ it 'does not perform the normalized email checks' do
expect(::Users::BannedUser).not_to receive(:by_detumbled_email)
+ expect(Email).not_to receive(:users_by_detumbled_email_count)
subject
end
end
- context 'when email has other validation errors' do
- subject(:new_user) { build(:user, email: existing_user.email).tap(&:valid?) }
+ context 'when the email has not changed' do
+ it 'does not perform the normalized email checks' do
+ user = create(:user)
- it_behaves_like 'does not perform the check'
+ expect(::Users::BannedUser).not_to receive(:by_detumbled_email)
+ expect(Email).not_to receive(:users_by_detumbled_email_count)
+
+ user.valid?
+ end
end
context 'when email has no other validation errors' do
- let(:error_message) { 'Email is not allowed. Please enter a different email address and try again.' }
- let(:tumbled_email) { 'person+inbox1@test.com' }
- let(:normalized_email) { 'person@test.com' }
- let!(:banned_user) { create(:user, :banned, email: normalized_email) }
+ context 'when the email is associated with a banned user' do
+ let(:tumbled_email) { 'banned+inbox1@test.com' }
+ let(:normalized_email) { 'banned@test.com' }
- subject(:new_user) { build(:user, email: tumbled_email).tap(&:valid?) }
+ before do
+ create(:user, :banned, email: normalized_email)
+ end
- it 'performs the check and adds an error' do
- subject
+ it_behaves_like 'adds a validation error'
- expect(new_user.errors.full_messages).to include(error_message)
- end
+ it 'performs the banned user check' do
+ expect(::Users::BannedUser).to receive(:by_detumbled_email).and_call_original
- context 'and does not match normalized email of a banned user' do
- let(:tumbled_email) { 'unique+tumbled@email.com' }
+ subject
+ end
- it 'does not add an error' do
- expect(new_user.errors.full_messages).not_to include(error_message)
+ it 'does not perform the normalized email limit check' do
+ expect(Email).not_to receive(:users_by_detumbled_email_count)
+
+ subject
+ end
+
+ context 'and does not match normalized email of a banned user' do
+ let(:tumbled_email) { 'unique+tumbled@email.com' }
+
+ it 'does not add an error' do
+ expect(new_user.errors).to be_empty
+ end
+ end
+
+ context 'when feature flag is disabled' do
+ before do
+ stub_feature_flags(block_banned_user_normalized_email_reuse: false)
+ end
+
+ it 'does not perform the check' do
+ expect(::Users::BannedUser).not_to receive(:by_detumbled_email)
+ end
+
+ it_behaves_like 'checking normalized email reuse limit'
end
end
- context 'when feature flag is disabled' do
+ context 'when the email is not associated with a banned user' do
+ let(:tumbled_email) { 'active+inbox1@test.com' }
+ let(:normalized_email) { 'active@test.com' }
+
before do
- stub_feature_flags(block_banned_user_normalized_email_reuse: false)
+ create(:user, email: normalized_email)
+ end
+
+ it 'performs the check and does not add an error' do
+ expect(::Users::BannedUser).to receive(:by_detumbled_email).and_call_original
+ expect(new_user.errors).to be_empty
end
- it_behaves_like 'does not perform the check'
+ it_behaves_like 'checking normalized email reuse limit'
end
end
end