Merge remote-tracking branch 'parent/main' into upstream-20240118

app/models/user.rb

@@ -151,6 +151,10 @@ class User < ApplicationRecord
     end
   end
 
+  def self.skip_mx_check?
+    Rails.env.local?
+  end
+
   def role
     if role_id.nil?
       UserRole.everyone
@@ -188,37 +192,16 @@ class User < ApplicationRecord
   end
 
   def confirm
-    new_user      = !confirmed?
-    self.approved = true if open_registrations? && !sign_up_from_ip_requires_approval?
-
-    super
-
-    if new_user
-      # Avoid extremely unlikely race condition when approving and confirming
-      # the user at the same time
-      reload unless approved?
-
-      if approved?
-        prepare_new_user!
-      else
-        notify_staff_about_pending_account!
-      end
+    wrap_email_confirmation do
+      super
     end
   end
 
-  def confirm!
-    new_user      = !confirmed?
-    self.approved = true if open_registrations?
-
-    skip_confirmation!
-    save!
-
-    if new_user
-      # Avoid extremely unlikely race condition when approving and confirming
-      # the user at the same time
-      reload unless approved?
-
-      prepare_new_user! if approved?
+  # Mark current email as confirmed, bypassing Devise
+  def mark_email_as_confirmed!
+    wrap_email_confirmation do
+      skip_confirmation!
+      save!
     end
   end
 
@@ -428,14 +411,53 @@ class User < ApplicationRecord
     end
   end
 
+  def grant_approval_on_confirmation?
+    # Re-check approval on confirmation if the server has switched to open registrations
+    open_registrations? && !sign_up_from_ip_requires_approval? && !sign_up_email_requires_approval?
+  end
+
+  def wrap_email_confirmation
+    new_user      = !confirmed?
+    self.approved = true if grant_approval_on_confirmation?
+
+    yield
+
+    if new_user
+      # Avoid extremely unlikely race condition when approving and confirming
+      # the user at the same time
+      reload unless approved?
+
+      if approved?
+        prepare_new_user!
+      else
+        notify_staff_about_pending_account!
+      end
+    end
+  end
+
   def sign_up_from_ip_requires_approval?
     !sign_up_ip.nil? && IpBlock.where(severity: :sign_up_requires_approval).where('ip >>= ?', sign_up_ip.to_s).exists?
   end
 
   def sign_up_email_requires_approval?
-    return false unless email.present? || unconfirmed_email.present?
+    return false if email.blank?
 
-    EmailDomainBlock.requires_approval?(email.presence || unconfirmed_email, attempt_ip: sign_up_ip)
+    _, domain = email.split('@', 2)
+    return false if domain.blank?
+
+    records = []
+
+    # Doing this conditionally is not very satisfying, but this is consistent
+    # with the MX records validations we do and keeps the specs tractable.
+    unless self.class.skip_mx_check?
+      Resolv::DNS.open do |dns|
+        dns.timeouts = 5
+
+        records = dns.getresources(domain, Resolv::DNS::Resource::IN::MX).to_a.map { |e| e.exchange.to_s }.compact_blank
+      end
+    end
+
+    EmailDomainBlock.requires_approval?(records + [domain], attempt_ip: sign_up_ip)
   end
 
   def open_registrations?
@@ -486,7 +508,7 @@ class User < ApplicationRecord
   end
 
   def validate_email_dns?
-    email_changed? && !external? && !Rails.env.local?
+    email_changed? && !external? && !self.class.skip_mx_check?
   end
 
   def validate_role_elevation