Merge branch 'user-change-email-non-allowlist-domain' into 'master'

Prevent user from bypassing domain signup restrictions

See merge request gitlab-org/gitlab!76272

app/models/user.rb

@@ -251,7 +251,7 @@ class User < ApplicationRecord
   validate :notification_email_verified, if: :notification_email_changed?
   validate :public_email_verified, if: :public_email_changed?
   validate :commit_email_verified, if: :commit_email_changed?
-  validate :signup_email_valid?, on: :create, if: ->(user) { !user.created_by_id }
+  validate :email_allowed_by_restrictions?, if: ->(user) { user.new_record? ? !user.created_by_id : user.email_changed? }
   validate :check_username_format, if: :username_changed?
 
   validates :theme_id, allow_nil: true, inclusion: { in: Gitlab::Themes.valid_ids,
@@ -2145,14 +2145,14 @@ class User < ApplicationRecord
     end
   end
 
-  def signup_email_valid?
+  def email_allowed_by_restrictions?
     error = validate_admin_signup_restrictions(email)
 
     errors.add(:email, error) if error
   end
 
   def signup_email_invalid_message
-    _('is not allowed for sign-up.')
+    self.new_record? ? _('is not allowed for sign-up.') : _('is not allowed.')
   end
 
   def check_username_format

locale/gitlab.pot

@@ -42286,6 +42286,9 @@ msgstr ""
 msgid "is not allowed since the group is not top-level group."
 msgstr ""
 
+msgid "is not allowed."
+msgstr ""
+
 msgid "is not allowed. We do not currently support project-level iterations"
 msgstr ""
 

spec/models/user_spec.rb

@@ -542,6 +542,13 @@ RSpec.describe User do
           expect(user).to be_invalid
           expect(user.errors.messages[:email].first).to eq(expected_error)
         end
+
+        it 'does not allow user to update email to a non-allowlisted domain' do
+          user = create(:user, email: "info@test.example.com")
+
+          expect { user.update!(email: "test@notexample.com") }
+            .to raise_error(StandardError, 'Validation failed: Email is not allowed. Check with your administrator.')
+        end
       end
 
       context 'when a signup domain is allowed and subdomains are not allowed' do
@@ -608,6 +615,13 @@ RSpec.describe User do
             user = build(:user, email: 'info@example.com', created_by_id: 1)
             expect(user).to be_valid
           end
+
+          it 'does not allow user to update email to a denied domain' do
+            user = create(:user, email: 'info@test.com')
+
+            expect { user.update!(email: 'info@example.com') }
+              .to raise_error(StandardError, 'Validation failed: Email is not allowed. Check with your administrator.')
+          end
         end
 
         context 'when a signup domain is denied but a wildcard subdomain is allowed' do
@@ -679,6 +693,13 @@ RSpec.describe User do
             expect(user.errors.messages[:email].first).to eq(expected_error)
           end
 
+          it 'does not allow user to update email to a restricted domain' do
+            user = create(:user, email: 'info@test.com')
+
+            expect { user.update!(email: 'info@gitlab.com') }
+              .to raise_error(StandardError, 'Validation failed: Email is not allowed. Check with your administrator.')
+          end
+
           it 'does accept a valid email address' do
             user = build(:user, email: 'info@test.com')