Change e-mail domain blocks to match subdomains of blocked domains (#18979)

diff --git a/app/models/email_domain_block.rb b/app/models/email_domain_block.rb
index 0e1e663c1092552949aa40c6847c0b6c61abd6b8..f9d74332b88d29fc44c863308ef9467db9abfef7 100644 (file)
--- a/app/models/email_domain_block.rb
+++ b/app/models/email_domain_block.rb
@@ -30,32 +30,56 @@ class EmailDomainBlock < ApplicationRecord
     @history ||= Trends::History.new('email_domain_blocks', id)
   end
 
-  def self.block?(domain_or_domains, attempt_ip: nil)
-    domains = Array(domain_or_domains).map do |str|
-      domain = begin
-        if str.include?('@')
-          str.split('@', 2).last
-        else
-          str
-        end
-      end
+  class Matcher
+    def initialize(domain_or_domains, attempt_ip: nil)
+      @uris       = extract_uris(domain_or_domains)
+      @attempt_ip = attempt_ip
+    end
 
-      TagManager.instance.normalize_domain(domain) if domain.present?
-    rescue Addressable::URI::InvalidURIError
-      nil
+    def match?
+      blocking? || invalid_uri?
     end
 
-    # If some of the inputs passed in are invalid, we definitely want to
-    # block the attempt, but we also want to register hits against any
-    # other valid matches
+    private
 
-    blocked = domains.any?(&:nil?)
+    def invalid_uri?
+      @uris.any?(&:nil?)
+    end
 
-    where(domain: domains).find_each do |block|
-      blocked = true
-      block.history.add(attempt_ip) if attempt_ip.present?
+    def blocking?
+      blocks = EmailDomainBlock.where(domain: domains_with_variants).order(Arel.sql('char_length(domain) desc'))
+      blocks.each { |block| block.history.add(@attempt_ip) } if @attempt_ip.present?
+      blocks.any?
     end
 
-    blocked
+    def domains_with_variants
+      @uris.flat_map do |uri|
+        next if uri.nil?
+
+        segments = uri.normalized_host.split('.')
+
+        segments.map.with_index { |_, i| segments[i..-1].join('.') }
+      end
+    end
+
+    def extract_uris(domain_or_domains)
+      Array(domain_or_domains).map do |str|
+        domain = begin
+          if str.include?('@')
+            str.split('@', 2).last
+          else
+            str
+          end
+        end
+
+        Addressable::URI.new.tap { |u| u.host = domain.strip } if domain.present?
+      rescue Addressable::URI::InvalidURIError, IDN::Idna::IdnaError
+        nil
+      end
+    end
+  end
+
+  def self.block?(domain_or_domains, attempt_ip: nil)
+    Matcher.new(domain_or_domains, attempt_ip: attempt_ip).match?
   end
 end

diff --git a/spec/models/email_domain_block_spec.rb b/spec/models/email_domain_block_spec.rb
index 567a32c32807842e72f1abcf6dcb7905aa67ca02..e23116888c7aedac19cb814da82c379fa02fc7c7 100644 (file)
--- a/spec/models/email_domain_block_spec.rb
+++ b/spec/models/email_domain_block_spec.rb
@@ -12,16 +12,29 @@ RSpec.describe EmailDomainBlock, type: :model do
     let(:input) { nil }
 
     context 'given an e-mail address' do
-      let(:input) { 'nyarn@example.com' }
+      let(:input) { "foo@#{domain}" }
 
-      it 'returns true if the domain is blocked' do
-        Fabricate(:email_domain_block, domain: 'example.com')
-        expect(EmailDomainBlock.block?(input)).to be true
+      context do
+        let(:domain) { 'example.com' }
+
+        it 'returns true if the domain is blocked' do
+          Fabricate(:email_domain_block, domain: 'example.com')
+          expect(EmailDomainBlock.block?(input)).to be true
+        end
+
+        it 'returns false if the domain is not blocked' do
+          Fabricate(:email_domain_block, domain: 'other-example.com')
+          expect(EmailDomainBlock.block?(input)).to be false
+        end
       end
 
-      it 'returns false if the domain is not blocked' do
-        Fabricate(:email_domain_block, domain: 'other-example.com')
-        expect(EmailDomainBlock.block?(input)).to be false
+      context do
+        let(:domain) { 'mail.example.com' }
+
+        it 'returns true if it is a subdomain of a blocked domain' do
+          Fabricate(:email_domain_block, domain: 'example.com')
+          expect(described_class.block?(input)).to be true
+        end
       end
     end