Prevent user from changing email to non allowlist

Currently, a user can bypass the domain allowlist
by creating an account with an email that is on the list,
then changing their email to a domain that is not
on the list.

Changelog: fixed

Fix conditional order

Remove accidental changes

Fix failing specs

Remove bad spec

Allow admin to add user not on allowlist

Validate if not new record

Remove unnecessary spec changes

Validate domain update

Use existing methods in restricted signup concern
to validate domain update

Add specs for deny and restricted

Add specs for updating domain
to denied or restricted domain

Undo unintentional changes

Remove unneeded spec

Fix bad spec

Reuse signup restrictions method

Change method name back

Was breaking other specs

Check if not new record

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

@@ -42268,6 +42268,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')