Improve email address validation (#29838)
This commit is contained in:
		
							parent
							
								
									1f11aa5f04
								
							
						
					
					
						commit
						38b9d31f63
					
				
							
								
								
									
										2
									
								
								Gemfile
									
									
									
									
									
								
							
							
						
						
									
										2
									
								
								Gemfile
									
									
									
									
									
								
							@ -206,3 +206,5 @@ gem 'net-http', '~> 0.4.0'
 | 
				
			|||||||
gem 'rubyzip', '~> 2.3'
 | 
					gem 'rubyzip', '~> 2.3'
 | 
				
			||||||
 | 
					
 | 
				
			||||||
gem 'hcaptcha', '~> 7.1'
 | 
					gem 'hcaptcha', '~> 7.1'
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					gem 'mail', '~> 2.8'
 | 
				
			||||||
 | 
				
			|||||||
@ -880,6 +880,7 @@ DEPENDENCIES
 | 
				
			|||||||
  letter_opener_web (~> 2.0)
 | 
					  letter_opener_web (~> 2.0)
 | 
				
			||||||
  link_header (~> 0.0)
 | 
					  link_header (~> 0.0)
 | 
				
			||||||
  lograge (~> 0.12)
 | 
					  lograge (~> 0.12)
 | 
				
			||||||
 | 
					  mail (~> 2.8)
 | 
				
			||||||
  mario-redis-lock (~> 1.2)
 | 
					  mario-redis-lock (~> 1.2)
 | 
				
			||||||
  md-paperclip-azure (~> 2.2)
 | 
					  md-paperclip-azure (~> 2.2)
 | 
				
			||||||
  memory_profiler
 | 
					  memory_profiler
 | 
				
			||||||
 | 
				
			|||||||
@ -95,6 +95,8 @@ class User < ApplicationRecord
 | 
				
			|||||||
  accepts_nested_attributes_for :invite_request, reject_if: ->(attributes) { attributes['text'].blank? && !Setting.require_invite_text }
 | 
					  accepts_nested_attributes_for :invite_request, reject_if: ->(attributes) { attributes['text'].blank? && !Setting.require_invite_text }
 | 
				
			||||||
  validates :invite_request, presence: true, on: :create, if: :invite_text_required?
 | 
					  validates :invite_request, presence: true, on: :create, if: :invite_text_required?
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  validates :email, presence: true, email_address: true
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  validates_with BlacklistedEmailValidator, if: -> { ENV['EMAIL_DOMAIN_LISTS_APPLY_AFTER_CONFIRMATION'] == 'true' || !confirmed? }
 | 
					  validates_with BlacklistedEmailValidator, if: -> { ENV['EMAIL_DOMAIN_LISTS_APPLY_AFTER_CONFIRMATION'] == 'true' || !confirmed? }
 | 
				
			||||||
  validates_with EmailMxValidator, if: :validate_email_dns?
 | 
					  validates_with EmailMxValidator, if: :validate_email_dns?
 | 
				
			||||||
  validates :agreement, acceptance: { allow_nil: false, accept: [true, 'true', '1'] }, on: :create
 | 
					  validates :agreement, acceptance: { allow_nil: false, accept: [true, 'true', '1'] }, on: :create
 | 
				
			||||||
 | 
				
			|||||||
							
								
								
									
										18
									
								
								app/validators/email_address_validator.rb
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										18
									
								
								app/validators/email_address_validator.rb
									
									
									
									
									
										Normal file
									
								
							@ -0,0 +1,18 @@
 | 
				
			|||||||
 | 
					# frozen_string_literal: true
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					# NOTE: I initially wrote this as `EmailValidator` but it ended up clashing
 | 
				
			||||||
 | 
					# with an indirect dependency of ours, `validate_email`, which, turns out,
 | 
				
			||||||
 | 
					# has the same approach as we do, but with an extra check disallowing
 | 
				
			||||||
 | 
					# single-label domains. Decided to not switch to `validate_email` because
 | 
				
			||||||
 | 
					# we do want to allow at least `localhost`.
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					class EmailAddressValidator < ActiveModel::EachValidator
 | 
				
			||||||
 | 
					  def validate_each(record, attribute, value)
 | 
				
			||||||
 | 
					    value = value.strip
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    address = Mail::Address.new(value)
 | 
				
			||||||
 | 
					    record.errors.add(attribute, :invalid) if address.address != value
 | 
				
			||||||
 | 
					  rescue Mail::Field::FieldError
 | 
				
			||||||
 | 
					    record.errors.add(attribute, :invalid)
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
 | 
					end
 | 
				
			||||||
@ -38,6 +38,12 @@ RSpec.describe User do
 | 
				
			|||||||
      user.save(validate: false)
 | 
					      user.save(validate: false)
 | 
				
			||||||
      expect(user.valid?).to be true
 | 
					      expect(user.valid?).to be true
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    it 'is valid with a localhost e-mail address' do
 | 
				
			||||||
 | 
					      user = Fabricate.build(:user, email: 'admin@localhost')
 | 
				
			||||||
 | 
					      user.valid?
 | 
				
			||||||
 | 
					      expect(user.valid?).to be true
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  describe 'Normalizations' do
 | 
					  describe 'Normalizations' do
 | 
				
			||||||
 | 
				
			|||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user