From 0412a4d03e3e075b8b4090774ebe5db4f95412de Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 24 Aug 2022 19:00:55 +0200 Subject: [PATCH] Change e-mail domain blocks to match subdomains of blocked domains (#18979) --- app/models/email_domain_block.rb | 64 ++++++++++++++++++-------- spec/models/email_domain_block_spec.rb | 27 ++++++++--- 2 files changed, 64 insertions(+), 27 deletions(-) diff --git a/app/models/email_domain_block.rb b/app/models/email_domain_block.rb index 0e1e663c10..f9d74332b8 100644 --- 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 567a32c328..e23116888c 100644 --- 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