diff --git a/Gemfile b/Gemfile index b2c7e5a37fb..dee8b7bdd59 100644 --- a/Gemfile +++ b/Gemfile @@ -76,7 +76,8 @@ gem 'stringex', require: false gem 'strong_migrations', '>= 0.4.2' gem 'subprocess', require: false gem 'terminal-table', require: false -gem 'valid_email', '>= 0.1.3' +# until a release includes https://github.com/hallelujah/valid_email/pull/126 +gem 'valid_email', '>= 0.1.3', github: 'hallelujah/valid_email', ref: '486b860' gem 'view_component', '~> 3.0' gem 'webauthn', '~> 2.5.2' gem 'xmldsig', '~> 0.6' diff --git a/Gemfile.lock b/Gemfile.lock index def9e3a4864..e912059a4eb 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -44,6 +44,16 @@ GIT nokogiri (>= 1.10.2) pkcs11 +GIT + remote: https://github.com/hallelujah/valid_email.git + revision: 486b860fb40281d1ba99ec7621505abc5c9ad5bb + ref: 486b860 + specs: + valid_email (0.2.0) + activemodel + mail (>= 2.6.1) + simpleidn + GIT remote: https://github.com/hashrocket/capybara-webmock.git revision: d3f3b7c8edbeca7b575e74b256ad22df80d2b420 @@ -216,7 +226,7 @@ GEM erubi (~> 1.4) parser (>= 2.4) smart_properties - bigdecimal (3.1.5) + bigdecimal (3.1.6) bindata (2.4.15) bootsnap (1.17.0) msgpack (~> 1.2) @@ -249,7 +259,7 @@ GEM coderay (1.1.3) coercible (1.0.0) descendants_tracker (~> 0.0.1) - concurrent-ruby (1.2.2) + concurrent-ruby (1.2.3) connection_pool (2.4.1) cose (1.3.0) cbor (~> 0.5.9) @@ -418,11 +428,11 @@ GEM mini_histogram (0.3.1) mini_mime (1.1.5) mini_portile2 (2.8.5) - minitest (5.20.0) + minitest (5.22.2) msgpack (1.7.2) multiset (0.5.3) mutex_m (0.2.0) - net-imap (0.4.6) + net-imap (0.4.10) date net-protocol net-pop (0.1.2) @@ -431,7 +441,7 @@ GEM timeout net-sftp (3.0.0) net-ssh (>= 5.0.0, < 7.0.0) - net-smtp (0.4.0) + net-smtp (0.4.0.1) net-protocol net-ssh (6.1.0) newrelic_rpm (9.7.0) @@ -683,13 +693,9 @@ GEM concurrent-ruby (~> 1.0) unf (0.1.4) unf_ext - unf_ext (0.0.8) + unf_ext (0.0.9.1) unicode-display_width (2.5.0) uniform_notifier (1.16.0) - valid_email (0.1.4) - activemodel - mail (>= 2.6.1) - simpleidn view_component (3.9.0) activesupport (>= 5.2.0, < 8.0) concurrent-ruby (~> 1.0) @@ -850,7 +856,7 @@ DEPENDENCIES subprocess tableparser terminal-table - valid_email (>= 0.1.3) + valid_email (>= 0.1.3)! view_component (~> 3.0) webauthn (~> 2.5.2) webmock diff --git a/app/models/disposable_email_domain.rb b/app/models/disposable_email_domain.rb index e8835698014..919224a8ec1 100644 --- a/app/models/disposable_email_domain.rb +++ b/app/models/disposable_email_domain.rb @@ -1,5 +1,17 @@ class DisposableEmailDomain < ApplicationRecord def self.disposable?(domain) - exists?(name: domain) + exists?(name: subdomains(domain)) + end + + # @return [Array] + # @example + # subdomains("foo.bar.baz.com") + # => ["foo.bar.baz.com", "bar.baz.com", "baz.com"] + def self.subdomains(domain) + parts = domain.split('.') + + parts[...-1].to_enum.with_index.map do |_part, index| + parts[index..].join('.') + end end end diff --git a/app/validators/form_add_email_validator.rb b/app/validators/form_add_email_validator.rb index 9882e837ba2..93f71232de2 100644 --- a/app/validators/form_add_email_validator.rb +++ b/app/validators/form_add_email_validator.rb @@ -12,6 +12,7 @@ module FormAddEmailValidator email: { mx_with_fallback: !ENV['RAILS_OFFLINE'], ban_disposable_email: true, + partial: true, } validate :validate_domain end diff --git a/app/validators/form_email_validator.rb b/app/validators/form_email_validator.rb index fc37679a33b..96ad1ab3011 100644 --- a/app/validators/form_email_validator.rb +++ b/app/validators/form_email_validator.rb @@ -11,6 +11,7 @@ module FormEmailValidator email: { mx_with_fallback: !ENV['RAILS_OFFLINE'], ban_disposable_email: true, + partial: true, } validate :validate_domain end diff --git a/spec/forms/add_user_email_form_spec.rb b/spec/forms/add_user_email_form_spec.rb index d1c0e150038..18a1b72b0fe 100644 --- a/spec/forms/add_user_email_form_spec.rb +++ b/spec/forms/add_user_email_form_spec.rb @@ -51,5 +51,33 @@ expect(response.success?).to eq(false) end end + + context 'banned domains' do + before do + expect(BanDisposableEmailValidator).to receive(:config).and_return(%w[spamdomain.com]) + end + + context 'with the banned domain' do + let(:new_email) { 'test@spamdomain.com' } + + it 'fails and does not send a confirmation email' do + expect(SendAddEmailConfirmation).to_not receive(:new) + + response = submit + expect(response.success?).to eq(false) + end + end + + context 'with a subdomain of the banned domain' do + let(:new_email) { 'test@abc.def.spamdomain.com' } + + it 'fails and does not send a confirmation email' do + expect(SendAddEmailConfirmation).to_not receive(:new) + + response = submit + expect(response.success?).to eq(false) + end + end + end end end diff --git a/spec/forms/password_reset_email_form_spec.rb b/spec/forms/password_reset_email_form_spec.rb index 37a7928afbe..a2fe5f10379 100644 --- a/spec/forms/password_reset_email_form_spec.rb +++ b/spec/forms/password_reset_email_form_spec.rb @@ -65,5 +65,23 @@ ) end end + + context 'disposable domains' do + before do + expect(BanDisposableEmailValidator).to receive(:config).and_return(%w[spamdomain.com]) + end + + it 'bans direct matches to disposable domains' do + form = PasswordResetEmailForm.new('foo@spamdomain.com') + + expect(form.submit).to_not be_success + end + + it 'bans subdomains of disposable domains' do + form = PasswordResetEmailForm.new('foo@sub.bar.spamdomain.com') + + expect(form.submit).to_not be_success + end + end end end diff --git a/spec/forms/register_user_email_form_spec.rb b/spec/forms/register_user_email_form_spec.rb index 495751d73a8..6bc9992f895 100644 --- a/spec/forms/register_user_email_form_spec.rb +++ b/spec/forms/register_user_email_form_spec.rb @@ -353,6 +353,25 @@ ) expect_delivered_email_count(0) end + + it 'returns false and adds errors when subdomain is blocked' do + blocked_domain = 'blocked.com' + blocked_email = 'test@sub.' + blocked_domain + + errors = { email: [t('valid_email.validations.email.invalid')] } + expect(BanDisposableEmailValidator).to receive(:config).and_return([blocked_domain]) + + expect(subject.submit(email: blocked_email, terms_accepted: '1').to_h).to include( + success: false, + errors: errors, + error_details: hash_including(*errors.keys), + email_already_exists: false, + rate_limited: false, + user_id: 'anonymous-uuid', + domain_name: 'sub.blocked.com', + ) + expect_delivered_email_count(0) + end end context 'when request_id is invalid' do diff --git a/spec/models/disposable_email_domain_spec.rb b/spec/models/disposable_email_domain_spec.rb index 68d161ad285..8aa5c47741e 100644 --- a/spec/models/disposable_email_domain_spec.rb +++ b/spec/models/disposable_email_domain_spec.rb @@ -9,9 +9,17 @@ end context 'when the domain exists' do - it 'returns true' do + it 'returns true for an exact domain match' do expect(DisposableEmailDomain.disposable?(domain)).to eq true end + + it 'returns true for an first subdomain match' do + expect(DisposableEmailDomain.disposable?("temp1.#{domain}")).to eq true + end + + it 'returns true for an sub-sub-subdomain match' do + expect(DisposableEmailDomain.disposable?("foo.bar.temp1.#{domain}")).to eq true + end end context 'when the domain does not exist' do @@ -20,4 +28,16 @@ end end end + + describe '.subdomains' do + it 'breaks a domain into subdomains' do + expect(DisposableEmailDomain.subdomains('foo.bar.baz.com')).to eq( + %w[ + foo.bar.baz.com + bar.baz.com + baz.com + ], + ) + end + end end diff --git a/spec/support/shared_examples_for_email_validation.rb b/spec/support/shared_examples_for_email_validation.rb index 493f0358234..e1b70535ff6 100644 --- a/spec/support/shared_examples_for_email_validation.rb +++ b/spec/support/shared_examples_for_email_validation.rb @@ -4,6 +4,6 @@ detect { |v| v.instance_of?(EmailValidator) } expect(email_validator.options). - to eq(mx_with_fallback: true, ban_disposable_email: true) + to eq(mx_with_fallback: true, ban_disposable_email: true, partial: true) end end