Remove IP matching from e-mail domain blocks (#18190)

Clear out e-mail domain blocks created from automatically resolved DNS records

diff --git a/app/models/email_domain_block.rb b/app/models/email_domain_block.rb
index 36e7e62ab83ab10b9f224429d942d0b12e54dfb7..0e1e663c1092552949aa40c6847c0b6c61abd6b8 100644 (file)
--- a/app/models/email_domain_block.rb
+++ b/app/models/email_domain_block.rb
@@ -3,16 +3,19 @@
 #
 # Table name: email_domain_blocks
 #
-#  id              :bigint(8)        not null, primary key
-#  domain          :string           default(""), not null
-#  created_at      :datetime         not null
-#  updated_at      :datetime         not null
-#  parent_id       :bigint(8)
-#  ips             :inet             is an Array
-#  last_refresh_at :datetime
+#  id         :bigint(8)        not null, primary key
+#  domain     :string           default(""), not null
+#  created_at :datetime         not null
+#  updated_at :datetime         not null
+#  parent_id  :bigint(8)
 #
 
 class EmailDomainBlock < ApplicationRecord
+  self.ignored_columns = %w(
+    ips
+    last_refresh_at
+  )
+
   include DomainNormalizable
 
   belongs_to :parent, class_name: 'EmailDomainBlock', optional: true
@@ -27,7 +30,7 @@ class EmailDomainBlock < ApplicationRecord
     @history ||= Trends::History.new('email_domain_blocks', id)
   end
 
-  def self.block?(domain_or_domains, ips: [], attempt_ip: nil)
+  def self.block?(domain_or_domains, attempt_ip: nil)
     domains = Array(domain_or_domains).map do |str|
       domain = begin
         if str.include?('@')
@@ -48,10 +51,7 @@ class EmailDomainBlock < ApplicationRecord
 
     blocked = domains.any?(&:nil?)
 
-    scope = where(domain: domains)
-    scope = scope.or(where('ips && ARRAY[?]::inet[]', ips)) if ips.any?
-
-    scope.find_each do |block|
+    where(domain: domains).find_each do |block|
       blocked = true
       block.history.add(attempt_ip) if attempt_ip.present?
     end

diff --git a/app/validators/email_mx_validator.rb b/app/validators/email_mx_validator.rb
index 237ca4c7bc8613ae2a2a277d7c8acd0931903e33..20f2fd37c64c8de97a43190e3a7ba7c2fe250f62 100644 (file)
--- a/app/validators/email_mx_validator.rb
+++ b/app/validators/email_mx_validator.rb
@@ -15,7 +15,7 @@ class EmailMxValidator < ActiveModel::Validator
 
       if resolved_ips.empty?
         user.errors.add(:email, :unreachable)
-      elsif on_blacklist?(resolved_domains, resolved_ips, user.sign_up_ip)
+      elsif on_blacklist?(resolved_domains, user.sign_up_ip)
         user.errors.add(:email, :blocked)
       end
     end
@@ -57,7 +57,7 @@ class EmailMxValidator < ActiveModel::Validator
     [ips, records]
   end
 
-  def on_blacklist?(domains, resolved_ips, attempt_ip)
-    EmailDomainBlock.block?(domains, ips: resolved_ips, attempt_ip: attempt_ip)
+  def on_blacklist?(domains, attempt_ip)
+    EmailDomainBlock.block?(domains, attempt_ip: attempt_ip)
   end
 end

diff --git a/app/workers/scheduler/email_domain_block_refresh_scheduler.rb b/app/workers/scheduler/email_domain_block_refresh_scheduler.rb
deleted file mode 100644 (file)
index e0ad898..0000000
--- a/app/workers/scheduler/email_domain_block_refresh_scheduler.rb
+++ /dev/null
@@ -1,31 +0,0 @@
-# frozen_string_literal: true
-
-class Scheduler::EmailDomainBlockRefreshScheduler
-  include Sidekiq::Worker
-  include Redisable
-
-  sidekiq_options retry: 0
-
-  def perform
-    Resolv::DNS.open do |dns|
-      dns.timeouts = 5
-
-      EmailDomainBlock.find_each do |email_domain_block|
-        ips = begin
-          if ip?(email_domain_block.domain)
-            [email_domain_block.domain]
-          else
-            resources = dns.getresources(email_domain_block.domain, Resolv::DNS::Resource::IN::A).to_a + dns.getresources(email_domain_block.domain, Resolv::DNS::Resource::IN::AAAA).to_a
-            resources.map { |resource| resource.address.to_s }
-          end
-        end
-
-        email_domain_block.update(ips: ips, last_refresh_at: Time.now.utc)
-      end
-    end
-  end
-
-  def ip?(str)
-    str =~ Regexp.union([Resolv::IPv4::Regex, Resolv::IPv6::Regex])
-  end
-end

diff --git a/config/sidekiq.yml b/config/sidekiq.yml
index f2ae9279bcb945040f90f6a9cd437081c2e964fa..26be263265963bb090a7bc4df663433374a177ec 100644 (file)
--- a/config/sidekiq.yml
+++ b/config/sidekiq.yml
@@ -17,10 +17,6 @@
     every: '5m'
     class: Scheduler::Trends::RefreshScheduler
     queue: scheduler
-  email_domain_block_refresh_scheduler:
-    every: '1h'
-    class: Scheduler::EmailDomainBlockRefreshScheduler
-    queue: scheduler
   trends_review_notifications_scheduler:
     every: '6h'
     class: Scheduler::Trends::ReviewNotificationsScheduler

diff --git a/db/migrate/20180615122121_add_autofollow_to_invites.rb b/db/migrate/20180615122121_add_autofollow_to_invites.rb
index 850b1d693e107782b6fed343ee55c88d08fc77de..8c5fb7410536037ffcd6c56ed3cb5f4b2265d733 100644 (file)
--- a/db/migrate/20180615122121_add_autofollow_to_invites.rb
+++ b/db/migrate/20180615122121_add_autofollow_to_invites.rb
@@ -5,7 +5,7 @@ class AddAutofollowToInvites < ActiveRecord::Migration[5.2]
 
   disable_ddl_transaction!
 
-  def change
+  def up
     safety_assured do
       add_column_with_default :invites, :autofollow, :bool, default: false, allow_null: false
     end

diff --git a/db/post_migrate/20220429101025_remove_ips_from_email_domain_blocks.rb b/db/post_migrate/20220429101025_remove_ips_from_email_domain_blocks.rb
new file mode 100644 (file)
index 0000000..fbb74d9
--- /dev/null
+++ b/db/post_migrate/20220429101025_remove_ips_from_email_domain_blocks.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+class RemoveIpsFromEmailDomainBlocks < ActiveRecord::Migration[5.2]
+  disable_ddl_transaction!
+
+  def change
+    safety_assured do
+      remove_column :email_domain_blocks, :ips, :inet, array: true
+      remove_column :email_domain_blocks, :last_refresh_at, :datetime
+    end
+  end
+end

diff --git a/db/post_migrate/20220429101850_clear_email_domain_blocks.rb b/db/post_migrate/20220429101850_clear_email_domain_blocks.rb
new file mode 100644 (file)
index 0000000..ff525b6
--- /dev/null
+++ b/db/post_migrate/20220429101850_clear_email_domain_blocks.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+class ClearEmailDomainBlocks < ActiveRecord::Migration[5.2]
+  disable_ddl_transaction!
+
+  class EmailDomainBlock < ApplicationRecord
+  end
+
+  def up
+    EmailDomainBlock.where.not(parent_id: nil).in_batches.delete_all
+  end
+
+  def down; end
+end

diff --git a/db/schema.rb b/db/schema.rb
index 55c5ea74c3469b8b5f4f3ef85ba0f3d09bc82d25..4552828d6da44f758dfbb188e268d3d6b3f6b04f 100644 (file)
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2022_04_28_114902) do
+ActiveRecord::Schema.define(version: 2022_04_29_101850) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -389,8 +389,6 @@ ActiveRecord::Schema.define(version: 2022_04_28_114902) do
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
     t.bigint "parent_id"
-    t.inet "ips", array: true
-    t.datetime "last_refresh_at"
     t.index ["domain"], name: "index_email_domain_blocks_on_domain", unique: true
   end
 

diff --git a/spec/validators/email_mx_validator_spec.rb b/spec/validators/email_mx_validator_spec.rb
index af0eb98f5ced7cc6774570015afb8850171bcd01..4feedd0c7f6e8b2622b5e7a6eb58839dda12b30d 100644 (file)
--- a/spec/validators/email_mx_validator_spec.rb
+++ b/spec/validators/email_mx_validator_spec.rb
@@ -56,66 +56,6 @@ describe EmailMxValidator do
       expect(user.errors).to have_received(:add)
     end
 
-    it 'adds an error if the A record is blacklisted' do
-      EmailDomainBlock.create!(domain: 'alternate-example.com', ips: ['1.2.3.4'])
-      resolver = double
-
-      allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([])
-      allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::A).and_return([double(address: '1.2.3.4')])
-      allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::AAAA).and_return([])
-      allow(resolver).to receive(:timeouts=).and_return(nil)
-      allow(Resolv::DNS).to receive(:open).and_yield(resolver)
-
-      subject.validate(user)
-      expect(user.errors).to have_received(:add)
-    end
-
-    it 'adds an error if the AAAA record is blacklisted' do
-      EmailDomainBlock.create!(domain: 'alternate-example.com', ips: ['fd00::1'])
-      resolver = double
-
-      allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([])
-      allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::A).and_return([])
-      allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::AAAA).and_return([double(address: 'fd00::1')])
-      allow(resolver).to receive(:timeouts=).and_return(nil)
-      allow(Resolv::DNS).to receive(:open).and_yield(resolver)
-
-      subject.validate(user)
-      expect(user.errors).to have_received(:add)
-    end
-
-    it 'adds an error if the A record of the MX record is blacklisted' do
-      EmailDomainBlock.create!(domain: 'mail.other-domain.com', ips: ['2.3.4.5'])
-      resolver = double
-
-      allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([double(exchange: 'mail.example.com')])
-      allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::A).and_return([])
-      allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::AAAA).and_return([])
-      allow(resolver).to receive(:getresources).with('mail.example.com', Resolv::DNS::Resource::IN::A).and_return([double(address: '2.3.4.5')])
-      allow(resolver).to receive(:getresources).with('mail.example.com', Resolv::DNS::Resource::IN::AAAA).and_return([])
-      allow(resolver).to receive(:timeouts=).and_return(nil)
-      allow(Resolv::DNS).to receive(:open).and_yield(resolver)
-
-      subject.validate(user)
-      expect(user.errors).to have_received(:add)
-    end
-
-    it 'adds an error if the AAAA record of the MX record is blacklisted' do
-      EmailDomainBlock.create!(domain: 'mail.other-domain.com', ips: ['fd00::2'])
-      resolver = double
-
-      allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([double(exchange: 'mail.example.com')])
-      allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::A).and_return([])
-      allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::AAAA).and_return([])
-      allow(resolver).to receive(:getresources).with('mail.example.com', Resolv::DNS::Resource::IN::A).and_return([])
-      allow(resolver).to receive(:getresources).with('mail.example.com', Resolv::DNS::Resource::IN::AAAA).and_return([double(address: 'fd00::2')])
-      allow(resolver).to receive(:timeouts=).and_return(nil)
-      allow(Resolv::DNS).to receive(:open).and_yield(resolver)
-
-      subject.validate(user)
-      expect(user.errors).to have_received(:add)
-    end
-
     it 'adds an error if the MX record is blacklisted' do
       EmailDomainBlock.create!(domain: 'mail.example.com')
       resolver = double