Explorar el Código

Change e-mail domain blocks to block IPs dynamically (#17635)

* Change e-mail domain blocks to block IPs dynamically

* Update app/workers/scheduler/email_domain_block_refresh_scheduler.rb

Co-authored-by: Yamagishi Kazutoshi <ykzts@desire.sh>

* Update app/workers/scheduler/email_domain_block_refresh_scheduler.rb

Co-authored-by: Yamagishi Kazutoshi <ykzts@desire.sh>

Co-authored-by: Yamagishi Kazutoshi <ykzts@desire.sh>
Eugen Rochko hace 2 años
padre
commit
a29a982eaa

+ 44 - 28
app/controllers/admin/email_domain_blocks_controller.rb

@@ -6,7 +6,20 @@ module Admin
 
     def index
       authorize :email_domain_block, :index?
+
       @email_domain_blocks = EmailDomainBlock.where(parent_id: nil).includes(:children).order(id: :desc).page(params[:page])
+      @form                = Form::EmailDomainBlockBatch.new
+    end
+
+    def batch
+      @form = Form::EmailDomainBlockBatch.new(form_email_domain_block_batch_params.merge(current_account: current_account, action: action_from_button))
+      @form.save
+    rescue ActionController::ParameterMissing
+      flash[:alert] = I18n.t('admin.email_domain_blocks.no_email_domain_block_selected')
+    rescue Mastodon::NotPermittedError
+      flash[:alert] = I18n.t('admin.custom_emojis.not_permitted')
+    ensure
+      redirect_to admin_email_domain_blocks_path
     end
 
     def new
@@ -19,41 +32,25 @@ module Admin
 
       @email_domain_block = EmailDomainBlock.new(resource_params)
 
-      if @email_domain_block.save
-        log_action :create, @email_domain_block
-
-        if @email_domain_block.with_dns_records?
-          hostnames = []
-          ips       = []
-
-          Resolv::DNS.open do |dns|
-            dns.timeouts = 5
+      if action_from_button == 'save'
+        EmailDomainBlock.transaction do
+          @email_domain_block.save!
+          log_action :create, @email_domain_block
 
-            hostnames = dns.getresources(@email_domain_block.domain, Resolv::DNS::Resource::IN::MX).to_a.map { |e| e.exchange.to_s }
-
-            ([@email_domain_block.domain] + hostnames).uniq.each do |hostname|
-              ips.concat(dns.getresources(hostname, Resolv::DNS::Resource::IN::A).to_a.map { |e| e.address.to_s })
-              ips.concat(dns.getresources(hostname, Resolv::DNS::Resource::IN::AAAA).to_a.map { |e| e.address.to_s })
-            end
-          end
-
-          (hostnames + ips).each do |hostname|
-            another_email_domain_block = EmailDomainBlock.new(domain: hostname, parent: @email_domain_block)
-            log_action :create, another_email_domain_block if another_email_domain_block.save
+          (@email_domain_block.other_domains || []).uniq.each do |domain|
+            other_email_domain_block = EmailDomainBlock.create!(domain: domain, parent: @email_domain_block)
+            log_action :create, other_email_domain_block
           end
         end
 
         redirect_to admin_email_domain_blocks_path, notice: I18n.t('admin.email_domain_blocks.created_msg')
       else
+        set_resolved_records
         render :new
       end
-    end
-
-    def destroy
-      authorize @email_domain_block, :destroy?
-      @email_domain_block.destroy!
-      log_action :destroy, @email_domain_block
-      redirect_to admin_email_domain_blocks_path, notice: I18n.t('admin.email_domain_blocks.destroyed_msg')
+    rescue ActiveRecord::RecordInvalid
+      set_resolved_records
+      render :new
     end
 
     private
@@ -62,8 +59,27 @@ module Admin
       @email_domain_block = EmailDomainBlock.find(params[:id])
     end
 
+    def set_resolved_records
+      Resolv::DNS.open do |dns|
+        dns.timeouts = 5
+        @resolved_records = dns.getresources(@email_domain_block.domain, Resolv::DNS::Resource::IN::MX).to_a
+      end
+    end
+
     def resource_params
-      params.require(:email_domain_block).permit(:domain, :with_dns_records)
+      params.require(:email_domain_block).permit(:domain, other_domains: [])
+    end
+
+    def form_email_domain_block_batch_params
+      params.require(:form_email_domain_block_batch).permit(email_domain_block_ids: [])
+    end
+
+    def action_from_button
+      if params[:delete]
+        'delete'
+      elsif params[:save]
+        'save'
+      end
     end
   end
 end

+ 36 - 19
app/models/email_domain_block.rb

@@ -3,11 +3,13 @@
 #
 # 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)
+#  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
 #
 
 class EmailDomainBlock < ApplicationRecord
@@ -18,27 +20,42 @@ class EmailDomainBlock < ApplicationRecord
 
   validates :domain, presence: true, uniqueness: true, domain: true
 
-  def with_dns_records=(val)
-    @with_dns_records = ActiveModel::Type::Boolean.new.cast(val)
-  end
+  # Used for adding multiple blocks at once
+  attr_accessor :other_domains
 
-  def with_dns_records?
-    @with_dns_records
+  def history
+    @history ||= Trends::History.new('email_domain_blocks', id)
   end
 
-  alias with_dns_records with_dns_records?
+  def self.block?(domain_or_domains, ips: [], attempt_ip: nil)
+    domains = Array(domain_or_domains).map do |str|
+      domain = begin
+        if str.include?('@')
+          str.split('@', 2).last
+        else
+          str
+        end
+      end
+
+      TagManager.instance.normalize_domain(domain) if domain.present?
+    rescue Addressable::URI::InvalidURIError
+      nil
+    end
 
-  def self.block?(email)
-    _, domain = email.split('@', 2)
+    # 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
 
-    return true if domain.nil?
+    blocked = domains.any?(&:nil?)
 
-    begin
-      domain = TagManager.instance.normalize_domain(domain)
-    rescue Addressable::URI::InvalidURIError
-      return true
+    scope = where(domain: domains)
+    scope = scope.or(where('ips && ARRAY[?]::inet[]', ips)) if ips.any?
+
+    scope.find_each do |block|
+      blocked = true
+      block.history.add(attempt_ip) if attempt_ip.present?
     end
 
-    where(domain: domain).exists?
+    blocked
   end
 end

+ 30 - 0
app/models/form/email_domain_block_batch.rb

@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+class Form::EmailDomainBlockBatch
+  include ActiveModel::Model
+  include Authorization
+  include AccountableConcern
+
+  attr_accessor :email_domain_block_ids, :action, :current_account
+
+  def save
+    case action
+    when 'delete'
+      delete!
+    end
+  end
+
+  private
+
+  def email_domain_blocks
+    @email_domain_blocks ||= EmailDomainBlock.where(id: email_domain_block_ids)
+  end
+
+  def delete!
+    email_domain_blocks.each do |email_domain_block|
+      authorize(email_domain_block, :destroy?)
+      email_domain_block.destroy!
+      log_action :destroy, email_domain_block
+    end
+  end
+end

+ 1 - 0
app/models/status.rb

@@ -24,6 +24,7 @@
 #  poll_id                :bigint(8)
 #  deleted_at             :datetime
 #  edited_at              :datetime
+#  trendable              :boolean
 #
 
 class Status < ApplicationRecord

+ 12 - 14
app/validators/blacklisted_email_validator.rb

@@ -4,41 +4,39 @@ class BlacklistedEmailValidator < ActiveModel::Validator
   def validate(user)
     return if user.valid_invitation? || user.email.blank?
 
-    @email = user.email
-
-    user.errors.add(:email, :blocked) if blocked_email_provider?
-    user.errors.add(:email, :taken) if blocked_canonical_email?
+    user.errors.add(:email, :blocked) if blocked_email_provider?(user.email, user.sign_up_ip)
+    user.errors.add(:email, :taken) if blocked_canonical_email?(user.email)
   end
 
   private
 
-  def blocked_email_provider?
-    disallowed_through_email_domain_block? || disallowed_through_configuration? || not_allowed_through_configuration?
+  def blocked_email_provider?(email, ip)
+    disallowed_through_email_domain_block?(email, ip) || disallowed_through_configuration?(email) || not_allowed_through_configuration?(email)
   end
 
-  def blocked_canonical_email?
-    CanonicalEmailBlock.block?(@email)
+  def blocked_canonical_email?(email)
+    CanonicalEmailBlock.block?(email)
   end
 
-  def disallowed_through_email_domain_block?
-    EmailDomainBlock.block?(@email)
+  def disallowed_through_email_domain_block?(email, ip)
+    EmailDomainBlock.block?(email, attempt_ip: ip)
   end
 
-  def not_allowed_through_configuration?
+  def not_allowed_through_configuration?(email)
     return false if Rails.configuration.x.email_domains_whitelist.blank?
 
     domains = Rails.configuration.x.email_domains_whitelist.gsub('.', '\.')
     regexp  = Regexp.new("@(.+\\.)?(#{domains})$", true)
 
-    @email !~ regexp
+    email !~ regexp
   end
 
-  def disallowed_through_configuration?
+  def disallowed_through_configuration?(email)
     return false if Rails.configuration.x.email_domains_blacklist.blank?
 
     domains = Rails.configuration.x.email_domains_blacklist.gsub('.', '\.')
     regexp  = Regexp.new("@(.+\\.)?(#{domains})", true)
 
-    regexp.match?(@email)
+    regexp.match?(email)
   end
 end

+ 10 - 10
app/validators/email_mx_validator.rb

@@ -11,11 +11,11 @@ class EmailMxValidator < ActiveModel::Validator
     if domain.blank?
       user.errors.add(:email, :invalid)
     elsif !on_allowlist?(domain)
-      ips, hostnames = resolve_mx(domain)
+      resolved_ips, resolved_domains = resolve_mx(domain)
 
-      if ips.empty?
+      if resolved_ips.empty?
         user.errors.add(:email, :unreachable)
-      elsif on_blacklist?(hostnames + ips)
+      elsif on_blacklist?(resolved_domains, resolved_ips, user.sign_up_ip)
         user.errors.add(:email, :blocked)
       end
     end
@@ -40,24 +40,24 @@ class EmailMxValidator < ActiveModel::Validator
   end
 
   def resolve_mx(domain)
-    hostnames = []
-    ips       = []
+    records = []
+    ips     = []
 
     Resolv::DNS.open do |dns|
       dns.timeouts = 5
 
-      hostnames = dns.getresources(domain, Resolv::DNS::Resource::IN::MX).to_a.map { |e| e.exchange.to_s }
+      records = dns.getresources(domain, Resolv::DNS::Resource::IN::MX).to_a.map { |e| e.exchange.to_s }
 
-      ([domain] + hostnames).uniq.each do |hostname|
+      ([domain] + records).uniq.each do |hostname|
         ips.concat(dns.getresources(hostname, Resolv::DNS::Resource::IN::A).to_a.map { |e| e.address.to_s })
         ips.concat(dns.getresources(hostname, Resolv::DNS::Resource::IN::AAAA).to_a.map { |e| e.address.to_s })
       end
     end
 
-    [ips, hostnames]
+    [ips, records]
   end
 
-  def on_blacklist?(values)
-    EmailDomainBlock.where(domain: values.uniq).any?
+  def on_blacklist?(domains, resolved_ips, attempt_ip)
+    EmailDomainBlock.block?(domains, ips: resolved_ips, attempt_ip: attempt_ip)
   end
 end

+ 13 - 14
app/views/admin/email_domain_blocks/_email_domain_block.html.haml

@@ -1,15 +1,14 @@
-%tr
-  %td
-    %samp= email_domain_block.domain
-  %td
-    = table_link_to 'trash', t('admin.email_domain_blocks.delete'), admin_email_domain_block_path(email_domain_block), method: :delete
+.batch-table__row
+  %label.batch-table__row__select.batch-table__row__select--aligned.batch-checkbox
+    = f.check_box :email_domain_block_ids, { multiple: true, include_hidden: false }, email_domain_block.id
+  .batch-table__row__content.pending-account
+    .pending-account__header
+      %samp= link_to email_domain_block.domain, admin_accounts_path(email: "%@#{email_domain_block.domain}")
 
-- email_domain_block.children.each do |child_email_domain_block|
-  %tr
-    %td
-      %samp= child_email_domain_block.domain
-      %span.muted-hint
-        = surround '(', ')' do
-          = t('admin.email_domain_blocks.from_html', domain: content_tag(:samp, email_domain_block.domain))
-    %td
-      = table_link_to 'trash', t('admin.email_domain_blocks.delete'), admin_email_domain_block_path(child_email_domain_block), method: :delete
+      %br/
+
+      - if email_domain_block.parent.present?
+        = t('admin.email_domain_blocks.resolved_through_html', domain: content_tag(:samp, email_domain_block.parent.domain))
+        •
+
+      = t('admin.email_domain_blocks.attempts_over_week', count: email_domain_block.history.reduce(0) { |sum, day| sum + day.accounts })

+ 17 - 11
app/views/admin/email_domain_blocks/index.html.haml

@@ -4,16 +4,22 @@
 - content_for :heading_actions do
   = link_to t('admin.email_domain_blocks.add_new'), new_admin_email_domain_block_path, class: 'button'
 
-- if @email_domain_blocks.empty?
-  %div.muted-hint.center-text=t 'admin.email_domain_blocks.empty'
-- else
-  .table-wrapper
-    %table.table
-      %thead
-        %tr
-          %th= t('admin.email_domain_blocks.domain')
-          %th
-      %tbody
-        = render partial: 'email_domain_block', collection: @email_domain_blocks
+- content_for :header_tags do
+  = javascript_pack_tag 'admin', async: true, crossorigin: 'anonymous'
+
+= form_for(@form, url: batch_admin_email_domain_blocks_path) do |f|
+  = hidden_field_tag :page, params[:page] || 1
+
+  .batch-table
+    .batch-table__toolbar
+      %label.batch-table__toolbar__select.batch-checkbox-all
+        = check_box_tag :batch_checkbox_all, nil, false
+      .batch-table__toolbar__actions
+        = f.button safe_join([fa_icon('times'), t('admin.email_domain_blocks.delete')]), name: :delete, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') }
+    .batch-table__body
+      - if @email_domain_blocks.empty?
+        = nothing_here 'nothing-here--under-tabs'
+      - else
+        = render partial: 'email_domain_block', collection: @email_domain_blocks.flat_map { |x| [x, x.children.to_a].flatten }, locals: { f: f }
 
 = paginate @email_domain_blocks

+ 28 - 4
app/views/admin/email_domain_blocks/new.html.haml

@@ -1,14 +1,38 @@
 - content_for :page_title do
   = t('.title')
 
+- content_for :header_tags do
+  = javascript_pack_tag 'admin', async: true, crossorigin: 'anonymous'
+
 = simple_form_for @email_domain_block, url: admin_email_domain_blocks_path do |f|
   = render 'shared/error_messages', object: @email_domain_block
 
   .fields-group
-    = f.input :domain, wrapper: :with_block_label, label: t('admin.email_domain_blocks.domain')
+    = f.input :domain, wrapper: :with_block_label, label: t('admin.email_domain_blocks.domain'), input_html: { readonly: defined?(@resolved_records) }
 
-  .fields-group
-    = f.input :with_dns_records, as: :boolean, wrapper: :with_label
+  - if defined?(@resolved_records)
+    %p.hint= t('admin.email_domain_blocks.resolved_dns_records_hint_html')
+
+    .batch-table
+      .batch-table__toolbar
+        %label.batch-table__toolbar__select.batch-checkbox-all
+          = check_box_tag :batch_checkbox_all, nil, false
+        .batch-table__toolbar__actions
+      .batch-table__body
+        - @resolved_records.each do |record|
+          .batch-table__row
+            %label.batch-table__row__select.batch-table__row__select--aligned.batch-checkbox
+              = f.input_field :other_domains, as: :boolean, checked_value: record.exchange.to_s, include_hidden: false, multiple: true
+            .batch-table__row__content.pending-account
+              .pending-account__header
+                %samp= record.exchange.to_s
+                %br
+                = t('admin.email_domain_blocks.dns.types.mx')
+
+    %hr.spacer/
 
   .actions
-    = f.button :button, t('.create'), type: :submit
+    - if defined?(@resolved_records)
+      = f.button :button, t('.create'), type: :submit, name: :save
+    - else
+      = f.button :button, t('.resolve'), type: :submit, name: :resolve

+ 30 - 0
app/workers/scheduler/email_domain_block_refresh_scheduler.rb

@@ -0,0 +1,30 @@
+# 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
+            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.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

+ 10 - 3
config/locales/en.yml

@@ -467,15 +467,22 @@ en:
       view: View domain block
     email_domain_blocks:
       add_new: Add new
+      attempts_over_week:
+        one: "%{count} attempt over the last week"
+        other: "%{count} sign-up attempts over the last week"
       created_msg: Successfully blocked e-mail domain
       delete: Delete
-      destroyed_msg: Successfully unblocked e-mail domain
+      dns:
+        types:
+          mx: MX record
       domain: Domain
-      empty: No e-mail domains currently blocked.
-      from_html: from %{domain}
       new:
         create: Add domain
+        resolve: Resolve domain
         title: Block new e-mail domain
+      no_email_domain_block_selected: No e-mail domain blocks were changed as none were selected
+      resolved_dns_records_hint_html: The domain name resolves to the following MX domains, which are ultimately responsible for accepting e-mail. Blocking an MX domain will block sign-ups from any e-mail address which uses the same MX domain, even if the visible domain name is different. <strong>Be careful not to block major e-mail providers.</strong>
+      resolved_through_html: Resolved through %{domain}
       title: Blocked e-mail domains
     follow_recommendations:
       description_html: "<strong>Follow recommendations help new users quickly find interesting content</strong>. When a user has not interacted with others enough to form personalized follow recommendations, these accounts are recommended instead. They are re-calculated on a daily basis from a mix of accounts with the highest recent engagements and highest local follower counts for a given language."

+ 1 - 1
config/locales/simple_form.en.yml

@@ -64,7 +64,7 @@ en:
       domain_allow:
         domain: This domain will be able to fetch data from this server and incoming data from it will be processed and stored
       email_domain_block:
-        domain: This can be the domain name that shows up in the e-mail address, the MX record that domain resolves to, or IP of the server that MX record resolves to. Those will be checked upon user sign-up and the sign-up will be rejected.
+        domain: This can be the domain name that shows up in the e-mail address or the MX record it uses. They will be checked upon sign-up.
         with_dns_records: An attempt to resolve the given domain's DNS records will be made and the results will also be blocked
       featured_tag:
         name: 'You might want to use one of these:'

+ 6 - 1
config/routes.rb

@@ -193,7 +193,12 @@ Rails.application.routes.draw do
     resources :domain_allows, only: [:new, :create, :show, :destroy]
     resources :domain_blocks, only: [:new, :create, :show, :destroy, :update, :edit]
 
-    resources :email_domain_blocks, only: [:index, :new, :create, :destroy]
+    resources :email_domain_blocks, only: [:index, :new, :create] do
+      collection do
+        post :batch
+      end
+    end
+
     resources :action_logs, only: [:index]
     resources :warning_presets, except: [:new]
 

+ 4 - 0
config/sidekiq.yml

@@ -17,6 +17,10 @@
     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: '2h'
     class: Scheduler::Trends::ReviewNotificationsScheduler

+ 6 - 0
db/migrate/20220224010024_add_ips_to_email_domain_blocks.rb

@@ -0,0 +1,6 @@
+class AddIpsToEmailDomainBlocks < ActiveRecord::Migration[6.1]
+  def change
+    add_column :email_domain_blocks, :ips, :inet, array: true
+    add_column :email_domain_blocks, :last_refresh_at, :datetime
+  end
+end

+ 4 - 1
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_02_10_153119) do
+ActiveRecord::Schema.define(version: 2022_02_24_010024) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -387,6 +387,9 @@ ActiveRecord::Schema.define(version: 2022_02_10_153119) 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
 

+ 21 - 21
spec/controllers/admin/email_domain_blocks_controller_spec.rb

@@ -17,43 +17,43 @@ RSpec.describe Admin::EmailDomainBlocksController, type: :controller do
       EmailDomainBlock.paginates_per default_per_page
     end
 
-    it 'renders email blacks' do
+    it 'returns http success' do
       2.times { Fabricate(:email_domain_block) }
-
       get :index, params: { page: 2 }
-
-      assigned = assigns(:email_domain_blocks)
-      expect(assigned.count).to eq 1
-      expect(assigned.klass).to be EmailDomainBlock
       expect(response).to have_http_status(200)
     end
   end
 
   describe 'GET #new' do
-    it 'assigns a new email black' do
+    it 'returns http success' do
       get :new
-
-      expect(assigns(:email_domain_block)).to be_instance_of(EmailDomainBlock)
       expect(response).to have_http_status(200)
     end
   end
 
   describe 'POST #create' do
-    it 'blocks the domain when succeeded to save' do
-      post :create, params: { email_domain_block: { domain: 'example.com' } }
-
-      expect(flash[:notice]).to eq I18n.t('admin.email_domain_blocks.created_msg')
-      expect(response).to redirect_to(admin_email_domain_blocks_path)
+    context 'when resolve button is pressed' do
+      before do
+        post :create, params: { email_domain_block: { domain: 'example.com' } }
+      end
+
+      it 'renders new template' do
+        expect(response).to render_template(:new)
+      end
     end
-  end
 
-  describe 'DELETE #destroy' do
-    it 'unblocks the domain' do
-      email_domain_block = Fabricate(:email_domain_block)
-      delete :destroy, params: { id: email_domain_block.id }
+    context 'when save button is pressed' do
+      before do
+        post :create, params: { email_domain_block: { domain: 'example.com' }, save: '' }
+      end
+
+      it 'blocks the domain' do
+        expect(EmailDomainBlock.find_by(domain: 'example.com')).to_not be_nil
+      end
 
-      expect(flash[:notice]).to eq I18n.t('admin.email_domain_blocks.destroyed_msg')
-      expect(response).to redirect_to(admin_email_domain_blocks_path)
+      it 'redirects to e-mail domain blocks' do
+        expect(response).to redirect_to(admin_email_domain_blocks_path)
+      end
     end
   end
 end

+ 21 - 6
spec/models/email_domain_block_spec.rb

@@ -9,14 +9,29 @@ RSpec.describe EmailDomainBlock, type: :model do
   end
 
   describe 'block?' do
-    it 'returns true if the domain is registed' do
-      Fabricate(:email_domain_block, domain: 'example.com')
-      expect(EmailDomainBlock.block?('nyarn@example.com')).to eq true
+    let(:input) { nil }
+
+    context 'given an e-mail address' do
+      let(:input) { 'nyarn@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 true if the domain is not registed' do
-      Fabricate(:email_domain_block, domain: 'example.com')
-      expect(EmailDomainBlock.block?('nyarn@example.net')).to eq false
+    context 'given an array of domains' do
+      let(:input) { %w(foo.com mail.foo.com) }
+
+      it 'returns true if the domain is blocked' do
+        Fabricate(:email_domain_block, domain: 'mail.foo.com')
+        expect(EmailDomainBlock.block?(input)).to be true
+      end
     end
   end
 end

+ 1 - 1
spec/validators/blacklisted_email_validator_spec.rb

@@ -4,7 +4,7 @@ require 'rails_helper'
 
 RSpec.describe BlacklistedEmailValidator, type: :validator do
   describe '#validate' do
-    let(:user)   { double(email: 'info@mail.com', errors: errors) }
+    let(:user)   { double(email: 'info@mail.com', sign_up_ip: '1.2.3.4', errors: errors) }
     let(:errors) { double(add: nil) }
 
     before do

+ 30 - 26
spec/validators/email_mx_validator_spec.rb

@@ -4,24 +4,28 @@ require 'rails_helper'
 
 describe EmailMxValidator do
   describe '#validate' do
-    let(:user) { double(email: 'foo@example.com', errors: double(add: nil)) }
-
-    it 'does not add errors if there are no DNS records for an e-mail domain that is explicitly allowed' do
-      old_whitelist = Rails.configuration.x.email_domains_whitelist
-      Rails.configuration.x.email_domains_whitelist = 'example.com'
-
-      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([])
-      allow(resolver).to receive(:timeouts=).and_return(nil)
-      allow(Resolv::DNS).to receive(:open).and_yield(resolver)
-
-      subject.validate(user)
-      expect(user.errors).to_not have_received(:add)
-
-      Rails.configuration.x.email_domains_whitelist = old_whitelist
+    let(:user) { double(email: 'foo@example.com', sign_up_ip: '1.2.3.4', errors: double(add: nil)) }
+
+    context 'for an e-mail domain that is explicitly allowed' do
+      around do |block|
+        tmp = Rails.configuration.x.email_domains_whitelist
+        Rails.configuration.x.email_domains_whitelist = 'example.com'
+        block.call
+        Rails.configuration.x.email_domains_whitelist = tmp
+      end
+
+      it 'does not add errors if there are no DNS records' do
+        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([])
+        allow(resolver).to receive(:timeouts=).and_return(nil)
+        allow(Resolv::DNS).to receive(:open).and_yield(resolver)
+
+        subject.validate(user)
+        expect(user.errors).to_not have_received(:add)
+      end
     end
 
     it 'adds an error if there are no DNS records for the e-mail domain' do
@@ -37,7 +41,7 @@ describe EmailMxValidator do
       expect(user.errors).to have_received(:add)
     end
 
-    it 'adds an error if a MX record exists but does not lead to an IP' do
+    it 'adds an error if a MX record does not lead to an IP' do
       resolver = double
 
       allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([double(exchange: 'mail.example.com')])
@@ -53,7 +57,7 @@ describe EmailMxValidator do
     end
 
     it 'adds an error if the A record is blacklisted' do
-      EmailDomainBlock.create!(domain: '1.2.3.4')
+      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([])
@@ -67,7 +71,7 @@ describe EmailMxValidator do
     end
 
     it 'adds an error if the AAAA record is blacklisted' do
-      EmailDomainBlock.create!(domain: 'fd00::1')
+      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([])
@@ -80,8 +84,8 @@ describe EmailMxValidator do
       expect(user.errors).to have_received(:add)
     end
 
-    it 'adds an error if the MX record is blacklisted' do
-      EmailDomainBlock.create!(domain: '2.3.4.5')
+    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')])
@@ -96,8 +100,8 @@ describe EmailMxValidator do
       expect(user.errors).to have_received(:add)
     end
 
-    it 'adds an error if the MX IPv6 record is blacklisted' do
-      EmailDomainBlock.create!(domain: 'fd00::2')
+    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')])
@@ -112,7 +116,7 @@ describe EmailMxValidator do
       expect(user.errors).to have_received(:add)
     end
 
-    it 'adds an error if the MX hostname is blacklisted' do
+    it 'adds an error if the MX record is blacklisted' do
       EmailDomainBlock.create!(domain: 'mail.example.com')
       resolver = double