Change domain blocks to automatically support subdomains (#11138)

* Change domain blocks to automatically support subdomains

If a more authoritative domain is blocked (example.com), then the
same block will be applied to a subdomain (foo.example.com)

* Match subdomains of existing accounts when blocking/unblocking domains

* Improve code style

diff --git a/app/controllers/admin/domain_blocks_controller.rb b/app/controllers/admin/domain_blocks_controller.rb
index 71597763bf7b295d6ea84253e9cd41957e667cd1..377cac8adcfe61b7f65ce8c192237ddda976236f 100644 (file)
--- a/app/controllers/admin/domain_blocks_controller.rb
+++ b/app/controllers/admin/domain_blocks_controller.rb
@@ -13,7 +13,7 @@ module Admin
       authorize :domain_block, :create?
 
       @domain_block = DomainBlock.new(resource_params)
-      existing_domain_block = resource_params[:domain].present? ? DomainBlock.find_by(domain: resource_params[:domain]) : nil
+      existing_domain_block = resource_params[:domain].present? ? DomainBlock.rule_for(resource_params[:domain]) : nil
 
       if existing_domain_block.present? && !@domain_block.stricter_than?(existing_domain_block)
         @domain_block.save

diff --git a/app/controllers/admin/instances_controller.rb b/app/controllers/admin/instances_controller.rb
index 6dd659a304a99f4e40e77785961fa23673c3628b..7888e844fb5abc0a2840ca7795e9462870e28650 100644 (file)
--- a/app/controllers/admin/instances_controller.rb
+++ b/app/controllers/admin/instances_controller.rb
@@ -18,7 +18,7 @@ module Admin
       @blocks_count    = Block.where(target_account: Account.where(domain: params[:id])).count
       @available       = DeliveryFailureTracker.available?(Account.select(:shared_inbox_url).where(domain: params[:id]).first&.shared_inbox_url)
       @media_storage   = MediaAttachment.where(account: Account.where(domain: params[:id])).sum(:file_file_size)
-      @domain_block    = DomainBlock.find_by(domain: params[:id])
+      @domain_block    = DomainBlock.rule_for(params[:id])
     end
 
     private

diff --git a/app/controllers/media_proxy_controller.rb b/app/controllers/media_proxy_controller.rb
index 950cf6d09f2673ff132f092a31f681a997aa1ec9..8fc18dd0602a9067d16e25efa454578170038148 100644 (file)
--- a/app/controllers/media_proxy_controller.rb
+++ b/app/controllers/media_proxy_controller.rb
@@ -39,6 +39,6 @@ class MediaProxyController < ApplicationController
   end
 
   def reject_media?
-    DomainBlock.find_by(domain: @media_attachment.account.domain)&.reject_media?
+    DomainBlock.reject_media?(@media_attachment.account.domain)
   end
 end

diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb
index f55dd35b2c7f8580f1eca87652bd1a3b3259cbb4..487e8e91e68a764366c8a12a63e7569463603f91 100644 (file)
--- a/app/lib/activitypub/activity/create.rb
+++ b/app/lib/activitypub/activity/create.rb
@@ -380,7 +380,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
 
   def skip_download?
     return @skip_download if defined?(@skip_download)
-    @skip_download ||= DomainBlock.find_by(domain: @account.domain)&.reject_media?
+    @skip_download ||= DomainBlock.reject_media?(@account.domain)
   end
 
   def reply_to_local?

diff --git a/app/lib/activitypub/activity/flag.rb b/app/lib/activitypub/activity/flag.rb
index f73b93058f59fa79600b5922fd75d42e458b9f06..1659bc61fe52676ed66977ed93121a8a671282f6 100644 (file)
--- a/app/lib/activitypub/activity/flag.rb
+++ b/app/lib/activitypub/activity/flag.rb
@@ -23,7 +23,7 @@ class ActivityPub::Activity::Flag < ActivityPub::Activity
   private
 
   def skip_reports?
-    DomainBlock.find_by(domain: @account.domain)&.reject_reports?
+    DomainBlock.reject_reports?(@account.domain)
   end
 
   def object_uris

diff --git a/app/lib/ostatus/activity/creation.rb b/app/lib/ostatus/activity/creation.rb
index 3840c8fbfed5b6507419ca5d8d4f07ae902ad66f..60de712db8adec04c6f63be0a7ce11e243e5b3df 100644 (file)
--- a/app/lib/ostatus/activity/creation.rb
+++ b/app/lib/ostatus/activity/creation.rb
@@ -148,7 +148,7 @@ class OStatus::Activity::Creation < OStatus::Activity::Base
   end
 
   def save_media
-    do_not_download = DomainBlock.find_by(domain: @account.domain)&.reject_media?
+    do_not_download = DomainBlock.reject_media?(@account.domain)
     media_attachments = []
 
     @xml.xpath('./xmlns:link[@rel="enclosure"]', xmlns: OStatus::TagManager::XMLNS).each do |link|
@@ -176,7 +176,7 @@ class OStatus::Activity::Creation < OStatus::Activity::Base
   end
 
   def save_emojis(parent)
-    do_not_download = DomainBlock.find_by(domain: parent.account.domain)&.reject_media?
+    do_not_download = DomainBlock.reject_media?(parent.account.domain)
 
     return if do_not_download
 

diff --git a/app/models/account.rb b/app/models/account.rb
index 9276aa9271274f33856bd849a60a531b068ff969..c588451fce0286dce224f78b07b3ef628017fdcb 100644 (file)
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -98,6 +98,7 @@ class Account < ApplicationRecord
   scope :tagged_with, ->(tag) { joins(:accounts_tags).where(accounts_tags: { tag_id: tag }) }
   scope :by_recent_status, -> { order(Arel.sql('(case when account_stats.last_status_at is null then 1 else 0 end) asc, account_stats.last_status_at desc')) }
   scope :popular, -> { order('account_stats.followers_count desc') }
+  scope :by_domain_and_subdomains, ->(domain) { where(domain: domain).or(where(arel_table[:domain].matches('%.' + domain))) }
 
   delegate :email,
            :unconfirmed_email,

diff --git a/app/models/custom_emoji.rb b/app/models/custom_emoji.rb
index d3cc7050492ea5523ccde7596eebcfba777d4f3c..e73cd9bd2753e0d037afccbcc000d8e7a3ab1000 100644 (file)
--- a/app/models/custom_emoji.rb
+++ b/app/models/custom_emoji.rb
@@ -39,6 +39,7 @@ class CustomEmoji < ApplicationRecord
   scope :local,      -> { where(domain: nil) }
   scope :remote,     -> { where.not(domain: nil) }
   scope :alphabetic, -> { order(domain: :asc, shortcode: :asc) }
+  scope :by_domain_and_subdomains, ->(domain) { where(domain: domain).or(where(arel_table[:domain].matches('%.' + domain))) }
 
   remotable_attachment :image, LIMIT
 

diff --git a/app/models/domain_block.rb b/app/models/domain_block.rb
index 84c08c15879b4250ff9a81752d6e679e2a6fc87b..25d3b87ef51d9e3acb75e6a9c8e66368804fd9b3 100644 (file)
--- a/app/models/domain_block.rb
+++ b/app/models/domain_block.rb
@@ -24,14 +24,41 @@ class DomainBlock < ApplicationRecord
 
   scope :matches_domain, ->(value) { where(arel_table[:domain].matches("%#{value}%")) }
 
-  def self.blocked?(domain)
-    where(domain: domain, severity: :suspend).exists?
+  class << self
+    def suspend?(domain)
+      !!rule_for(domain)&.suspend?
+    end
+
+    def silence?(domain)
+      !!rule_for(domain)&.silence?
+    end
+
+    def reject_media?(domain)
+      !!rule_for(domain)&.reject_media?
+    end
+
+    def reject_reports?(domain)
+      !!rule_for(domain)&.reject_reports?
+    end
+
+    alias blocked? suspend?
+
+    def rule_for(domain)
+      return if domain.blank?
+
+      uri      = Addressable::URI.new.tap { |u| u.host = domain.gsub(/[\/]/, '') }
+      segments = uri.normalized_host.split('.')
+      variants = segments.map.with_index { |_, i| segments[i..-1].join('.') }
+
+      where(domain: variants[0..-2]).order(Arel.sql('char_length(domain) desc')).first
+    end
   end
 
   def stricter_than?(other_block)
-    return true if suspend?
+    return true  if suspend?
     return false if other_block.suspend? && (silence? || noop?)
     return false if other_block.silence? && noop?
+
     (reject_media || !other_block.reject_media) && (reject_reports || !other_block.reject_reports)
   end
 

diff --git a/app/models/instance.rb b/app/models/instance.rb
index 7bf000d40480ba701a4f367c232eee675e413c17..a01db1212402dd7bb7f9617161d29d78ad3c4742 100644 (file)
--- a/app/models/instance.rb
+++ b/app/models/instance.rb
@@ -8,7 +8,7 @@ class Instance
   def initialize(resource)
     @domain         = resource.domain
     @accounts_count = resource.is_a?(DomainBlock) ? nil : resource.accounts_count
-    @domain_block   = resource.is_a?(DomainBlock) ? resource : DomainBlock.find_by(domain: domain)
+    @domain_block   = resource.is_a?(DomainBlock) ? resource : DomainBlock.rule_for(domain)
   end
 
   def cached_sample_accounts

diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb
index ad22d37fe07be29cfe9a1ad15b997444fd2f6358..05c017bdf5a41e40d47b5de41c7c65d2f4779dab 100644 (file)
--- a/app/services/activitypub/process_account_service.rb
+++ b/app/services/activitypub/process_account_service.rb
@@ -205,7 +205,7 @@ class ActivityPub::ProcessAccountService < BaseService
 
   def domain_block
     return @domain_block if defined?(@domain_block)
-    @domain_block = DomainBlock.find_by(domain: @domain)
+    @domain_block = DomainBlock.rule_for(@domain)
   end
 
   def key_changed?

diff --git a/app/services/block_domain_service.rb b/app/services/block_domain_service.rb
index 497f0394bf8aa3e00adcff1c2e0245b63d2265e1..c6eef04d420fe75a7ee8d4e79e965a16ecf719f1 100644 (file)
--- a/app/services/block_domain_service.rb
+++ b/app/services/block_domain_service.rb
@@ -76,7 +76,7 @@ class BlockDomainService < BaseService
   end
 
   def blocked_domain_accounts
-    Account.where(domain: blocked_domain)
+    Account.by_domain_and_subdomains(blocked_domain)
   end
 
   def media_from_blocked_domain
@@ -84,6 +84,6 @@ class BlockDomainService < BaseService
   end
 
   def emojis_from_blocked_domains
-    CustomEmoji.where(domain: blocked_domain)
+    CustomEmoji.by_domain_and_subdomains(blocked_domain)
   end
 end

diff --git a/app/services/resolve_account_service.rb b/app/services/resolve_account_service.rb
index 11e33a83ade2462d077ed521c602e281049e21d0..57c9ccfe1b8c503fe5bbe44e9f47a87222b2c870 100644 (file)
--- a/app/services/resolve_account_service.rb
+++ b/app/services/resolve_account_service.rb
@@ -146,7 +146,7 @@ class ResolveAccountService < BaseService
 
   def domain_block
     return @domain_block if defined?(@domain_block)
-    @domain_block = DomainBlock.find_by(domain: @domain)
+    @domain_block = DomainBlock.rule_for(@domain)
   end
 
   def atom_url

diff --git a/app/services/unblock_domain_service.rb b/app/services/unblock_domain_service.rb
index 9b8526fbe5f7bd9f59fb6868363ff45c0c92d04c..fc262a50ada223034602d078824ff55dba62ceb4 100644 (file)
--- a/app/services/unblock_domain_service.rb
+++ b/app/services/unblock_domain_service.rb
@@ -14,7 +14,8 @@ class UnblockDomainService < BaseService
   end
 
   def blocked_accounts
-    scope = Account.where(domain: domain_block.domain)
+    scope = Account.by_domain_and_subdomains(domain_block.domain)
+
     if domain_block.silence?
       scope.where(silenced_at: @domain_block.created_at)
     else

diff --git a/app/services/update_remote_profile_service.rb b/app/services/update_remote_profile_service.rb
index 68d36addfcaf12442568d3f8210768cb72c1e336..403395a0ddb1b0f620f61048f65b5ff17d99a71d 100644 (file)
--- a/app/services/update_remote_profile_service.rb
+++ b/app/services/update_remote_profile_service.rb
@@ -26,7 +26,7 @@ class UpdateRemoteProfileService < BaseService
     account.note         = remote_profile.note         || ''
     account.locked       = remote_profile.locked?
 
-    if !account.suspended? && !DomainBlock.find_by(domain: account.domain)&.reject_media?
+    if !account.suspended? && !DomainBlock.reject_media?(account.domain)
       if remote_profile.avatar.present?
         account.avatar_remote_url = remote_profile.avatar
       else
@@ -46,7 +46,7 @@ class UpdateRemoteProfileService < BaseService
   end
 
   def save_emojis
-    do_not_download = DomainBlock.find_by(domain: account.domain)&.reject_media?
+    do_not_download = DomainBlock.reject_media?(account.domain)
 
     return if do_not_download
 

diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb
index 379872316b257d090ad4c03d80147957ea9884ce..ce9ea250d1329602110666e4be933c51c7380b5f 100644 (file)
--- a/spec/models/account_spec.rb
+++ b/spec/models/account_spec.rb
@@ -687,6 +687,23 @@ RSpec.describe Account, type: :model do
       end
     end
 
+    describe 'by_domain_and_subdomains' do
+      it 'returns exact domain matches' do
+        account = Fabricate(:account, domain: 'example.com')
+        expect(Account.by_domain_and_subdomains('example.com')).to eq [account]
+      end
+
+      it 'returns subdomains' do
+        account = Fabricate(:account, domain: 'foo.example.com')
+        expect(Account.by_domain_and_subdomains('example.com')).to eq [account]
+      end
+
+      it 'does not return partially matching domains' do
+        account = Fabricate(:account, domain: 'grexample.com')
+        expect(Account.by_domain_and_subdomains('example.com')).to_not eq [account]
+      end
+    end
+
     describe 'expiring' do
       it 'returns remote accounts with followers whose subscription expiration date is past or not given' do
         local = Fabricate(:account, domain: nil)

diff --git a/spec/models/domain_block_spec.rb b/spec/models/domain_block_spec.rb
index 0035fd0ffa0fbd7c54711302c3448d10dd8520cd..d98c5e11863030c11bbf694496b2d14db93335d5 100644 (file)
--- a/spec/models/domain_block_spec.rb
+++ b/spec/models/domain_block_spec.rb
@@ -21,23 +21,40 @@ RSpec.describe DomainBlock, type: :model do
     end
   end
 
-  describe 'blocked?' do
+  describe '.blocked?' do
     it 'returns true if the domain is suspended' do
-      Fabricate(:domain_block, domain: 'domain', severity: :suspend)
-      expect(DomainBlock.blocked?('domain')).to eq true
+      Fabricate(:domain_block, domain: 'example.com', severity: :suspend)
+      expect(DomainBlock.blocked?('example.com')).to eq true
     end
 
     it 'returns false even if the domain is silenced' do
-      Fabricate(:domain_block, domain: 'domain', severity: :silence)
-      expect(DomainBlock.blocked?('domain')).to eq false
+      Fabricate(:domain_block, domain: 'example.com', severity: :silence)
+      expect(DomainBlock.blocked?('example.com')).to eq false
     end
 
     it 'returns false if the domain is not suspended nor silenced' do
-      expect(DomainBlock.blocked?('domain')).to eq false
+      expect(DomainBlock.blocked?('example.com')).to eq false
     end
   end
 
-  describe 'stricter_than?' do
+  describe '.rule_for' do
+    it 'returns rule matching a blocked domain' do
+      block = Fabricate(:domain_block, domain: 'example.com')
+      expect(DomainBlock.rule_for('example.com')).to eq block
+    end
+
+    it 'returns a rule matching a subdomain of a blocked domain' do
+      block = Fabricate(:domain_block, domain: 'example.com')
+      expect(DomainBlock.rule_for('sub.example.com')).to eq block
+    end
+
+    it 'returns a rule matching a blocked subdomain' do
+      block = Fabricate(:domain_block, domain: 'sub.example.com')
+      expect(DomainBlock.rule_for('sub.example.com')).to eq block
+    end
+  end
+
+  describe '#stricter_than?' do
     it 'returns true if the new block has suspend severity while the old has lower severity' do
       suspend = DomainBlock.new(domain: 'domain', severity: :suspend)
       silence = DomainBlock.new(domain: 'domain', severity: :silence)