diff options
20 files changed, 325 insertions, 160 deletions
diff --git a/app/controllers/admin/email_domain_blocks_controller.rb b/app/controllers/admin/email_domain_blocks_controller.rb index f7bdfb0c5..33ee079f3 100644 --- a/app/controllers/admin/email_domain_blocks_controller.rb +++ b/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 diff --git a/app/models/email_domain_block.rb b/app/models/email_domain_block.rb index f50fa46ba..36e7e62ab 100644 --- a/app/models/email_domain_block.rb +++ b/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 diff --git a/app/models/form/email_domain_block_batch.rb b/app/models/form/email_domain_block_batch.rb new file mode 100644 index 000000000..df120182b --- /dev/null +++ b/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 diff --git a/app/models/status.rb b/app/models/status.rb index 2e3df98a1..96e41b1d3 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -24,6 +24,7 @@ # poll_id :bigint(8) # deleted_at :datetime # edited_at :datetime +# trendable :boolean # class Status < ApplicationRecord diff --git a/app/validators/blacklisted_email_validator.rb b/app/validators/blacklisted_email_validator.rb index eb66ad93d..9b3f2e33e 100644 --- a/app/validators/blacklisted_email_validator.rb +++ b/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 diff --git a/app/validators/email_mx_validator.rb b/app/validators/email_mx_validator.rb index dceef5029..237ca4c7b 100644 --- a/app/validators/email_mx_validator.rb +++ b/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 diff --git a/app/views/admin/email_domain_blocks/_email_domain_block.html.haml b/app/views/admin/email_domain_blocks/_email_domain_block.html.haml index 41ab8c171..c5a55bc27 100644 --- a/app/views/admin/email_domain_blocks/_email_domain_block.html.haml +++ b/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 }) diff --git a/app/views/admin/email_domain_blocks/index.html.haml b/app/views/admin/email_domain_blocks/index.html.haml index fa5d86b67..b073e8716 100644 --- a/app/views/admin/email_domain_blocks/index.html.haml +++ b/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 diff --git a/app/views/admin/email_domain_blocks/new.html.haml b/app/views/admin/email_domain_blocks/new.html.haml index 4a346f240..524b69968 100644 --- a/app/views/admin/email_domain_blocks/new.html.haml +++ b/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 diff --git a/app/workers/scheduler/email_domain_block_refresh_scheduler.rb b/app/workers/scheduler/email_domain_block_refresh_scheduler.rb new file mode 100644 index 000000000..c67be6843 --- /dev/null +++ b/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 diff --git a/config/locales/en.yml b/config/locales/en.yml index c206c893b..f045174a9 100644 --- a/config/locales/en.yml +++ b/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." diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml index 03eefd0d5..c5e75b408 100644 --- a/config/locales/simple_form.en.yml +++ b/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:' diff --git a/config/routes.rb b/config/routes.rb index f59f2a5bb..176438e45 100644 --- a/config/routes.rb +++ b/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] diff --git a/config/sidekiq.yml b/config/sidekiq.yml index 9dde5a053..c8b1a20dd 100644 --- a/config/sidekiq.yml +++ b/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 diff --git a/db/migrate/20220224010024_add_ips_to_email_domain_blocks.rb b/db/migrate/20220224010024_add_ips_to_email_domain_blocks.rb new file mode 100644 index 000000000..1b19a2aa1 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 8842dcd8c..0e9b6e619 100644 --- 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_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 diff --git a/spec/controllers/admin/email_domain_blocks_controller_spec.rb b/spec/controllers/admin/email_domain_blocks_controller_spec.rb index 52db56f4e..cf194579d 100644 --- a/spec/controllers/admin/email_domain_blocks_controller_spec.rb +++ b/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 diff --git a/spec/models/email_domain_block_spec.rb b/spec/models/email_domain_block_spec.rb index efd2853a9..567a32c32 100644 --- a/spec/models/email_domain_block_spec.rb +++ b/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 diff --git a/spec/validators/blacklisted_email_validator_spec.rb b/spec/validators/blacklisted_email_validator_spec.rb index f7d5e01bc..351de0707 100644 --- a/spec/validators/blacklisted_email_validator_spec.rb +++ b/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 diff --git a/spec/validators/email_mx_validator_spec.rb b/spec/validators/email_mx_validator_spec.rb index 550e91996..af0eb98f5 100644 --- a/spec/validators/email_mx_validator_spec.rb +++ b/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 |