From 2ca7e8b7d83e0304e652fdfd4a76798541742a7b Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 12 Nov 2024 10:08:02 -0500 Subject: [PATCH] Extract `User::Approval` concern --- app/models/concerns/user/approval.rb | 38 ++++++++++++++++++++++++++++ app/models/user.rb | 35 ------------------------- 2 files changed, 38 insertions(+), 35 deletions(-) diff --git a/app/models/concerns/user/approval.rb b/app/models/concerns/user/approval.rb index 28f3c2a825..22999348b1 100644 --- a/app/models/concerns/user/approval.rb +++ b/app/models/concerns/user/approval.rb @@ -6,6 +6,8 @@ module User::Approval included do scope :approved, -> { where(approved: true) } scope :pending, -> { where(approved: false) } + + before_create :set_approved end def pending? @@ -21,4 +23,40 @@ module User::Approval reload unless confirmed? prepare_new_user! if confirmed? end + + private + + def set_approved + self.approved = begin + if sign_up_from_ip_requires_approval? || sign_up_email_requires_approval? + false + else + open_registrations? || valid_invitation? || external? + end + 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 sign_up_from_ip_requires_approval? + sign_up_ip.present? && IpBlock.severity_sign_up_requires_approval.containing(sign_up_ip.to_s).exists? + end + + def sign_up_email_requires_approval? + return false if email.blank? + + _, 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. + records = DomainResource.new(domain).mx unless self.class.skip_mx_check? + + EmailDomainBlock.requires_approval?(records + [domain], attempt_ip: sign_up_ip) + end end diff --git a/app/models/user.rb b/app/models/user.rb index 02b30e4fc5..e9aa3bec77 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -127,7 +127,6 @@ class User < ApplicationRecord scope :matches_ip, ->(value) { left_joins(:ips).merge(IpBlock.contained_by(value)).group('users.id') } before_validation :sanitize_role - before_create :set_approved after_commit :send_pending_devise_notifications after_create_commit :trigger_webhooks @@ -393,21 +392,6 @@ class User < ApplicationRecord devise_mailer.send(notification, self, *, **).deliver_later end - def set_approved - self.approved = begin - if sign_up_from_ip_requires_approval? || sign_up_email_requires_approval? - false - else - open_registrations? || valid_invitation? || external? - end - 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? @@ -427,25 +411,6 @@ class User < ApplicationRecord end end - def sign_up_from_ip_requires_approval? - sign_up_ip.present? && IpBlock.severity_sign_up_requires_approval.containing(sign_up_ip.to_s).exists? - end - - def sign_up_email_requires_approval? - return false if email.blank? - - _, 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. - records = DomainResource.new(domain).mx unless self.class.skip_mx_check? - - EmailDomainBlock.requires_approval?(records + [domain], attempt_ip: sign_up_ip) - end - def open_registrations? Setting.registrations_mode == 'open' end