Skip to content

Commit

Permalink
Improve email address validation (mastodon#29838)
Browse files Browse the repository at this point in the history
  • Loading branch information
ClearlyClaire authored and noellabo committed Jun 1, 2024
1 parent 53fc1bf commit 5910fe3
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 2 deletions.
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,5 @@ gem 'concurrent-ruby', require: false
gem 'connection_pool', require: false

gem 'xorcist', '~> 1.1'
gem 'cocoon', '~> 1.2'
gem 'mail', '~> 2.8'
3 changes: 2 additions & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,8 @@ DEPENDENCIES
letter_opener (~> 1.7)
letter_opener_web (~> 1.4)
link_header (~> 0.0)
lograge (~> 0.11)
lograge (~> 0.12)
mail (~> 2.8)
makara (~> 0.5)
mario-redis-lock (~> 1.2)
memory_profiler
Expand Down
5 changes: 4 additions & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,10 @@ class User < ApplicationRecord
validates :invite_request, presence: true, on: :create, if: :invite_text_required?

validates :locale, inclusion: I18n.available_locales.map(&:to_s), if: :locale?
validates_with BlacklistedEmailValidator, if: -> { !confirmed? }

validates :email, presence: true, email_address: true

validates_with BlacklistedEmailValidator, if: -> { ENV['EMAIL_DOMAIN_LISTS_APPLY_AFTER_CONFIRMATION'] == 'true' || !confirmed? }
validates_with EmailMxValidator, if: :validate_email_dns?
validates :agreement, acceptance: { allow_nil: false, accept: [true, 'true', '1'] }, on: :create
validates :setting_max_frequently_used_emojis, numericality: { greater_than_or_equal_to: 1, less_than_or_equal_to: CustomEmoji::FREQUENTLY_USED_EMOJIS_LIMIT }
Expand Down
18 changes: 18 additions & 0 deletions app/validators/email_address_validator.rb
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@
expect(user.valid?).to be true
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

it 'cleans out empty string from languages' do
user = Fabricate.build(:user, chosen_languages: [''])
user.valid?
Expand Down

0 comments on commit 5910fe3

Please sign in to comment.