From 02df1b4e4a1690c934b0e4fb9aa32b832d6bc2a1 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 13 Aug 2024 03:37:32 -0400 Subject: [PATCH] Finish email allow/deny list naming migration (#30530) --- .rubocop_todo.yml | 1 - app/models/user.rb | 2 +- app/validators/email_mx_validator.rb | 8 ++-- ...l_validator.rb => user_email_validator.rb} | 10 ++--- .../auto_close_registrations_scheduler.rb | 2 +- config/initializers/blacklists.rb | 6 --- config/initializers/doorkeeper.rb | 2 +- config/initializers/email_domains_lists.rb | 6 +++ spec/models/user_spec.rb | 40 +++++++++---------- spec/validators/email_mx_validator_spec.rb | 8 ++-- ...r_spec.rb => user_email_validator_spec.rb} | 2 +- 11 files changed, 43 insertions(+), 44 deletions(-) rename app/validators/{blacklisted_email_validator.rb => user_email_validator.rb} (74%) delete mode 100644 config/initializers/blacklists.rb create mode 100644 config/initializers/email_domains_lists.rb rename spec/validators/{blacklisted_email_validator_spec.rb => user_email_validator_spec.rb} (96%) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 357ed995450..09acb795bf2 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -40,7 +40,6 @@ Style/FetchEnvVar: - 'config/environments/production.rb' - 'config/initializers/2_limited_federation_mode.rb' - 'config/initializers/3_omniauth.rb' - - 'config/initializers/blacklists.rb' - 'config/initializers/cache_buster.rb' - 'config/initializers/devise.rb' - 'config/initializers/paperclip.rb' diff --git a/app/models/user.rb b/app/models/user.rb index 72854569260..f22b7ec684c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -100,7 +100,7 @@ class User < ApplicationRecord validates :email, presence: true, email_address: true - validates_with BlacklistedEmailValidator, if: -> { ENV['EMAIL_DOMAIN_LISTS_APPLY_AFTER_CONFIRMATION'] == 'true' || !confirmed? } + validates_with UserEmailValidator, if: -> { ENV['EMAIL_DOMAIN_LISTS_APPLY_AFTER_CONFIRMATION'] == 'true' || !confirmed? } validates_with EmailMxValidator, if: :validate_email_dns? validates :agreement, acceptance: { allow_nil: false, accept: [true, 'true', '1'] }, on: :create diff --git a/app/validators/email_mx_validator.rb b/app/validators/email_mx_validator.rb index 79437789435..f78b98d7dd7 100644 --- 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, user.sign_up_ip) + elsif email_domain_blocked?(resolved_domains, user.sign_up_ip) user.errors.add(:email, :blocked) end end @@ -34,9 +34,9 @@ class EmailMxValidator < ActiveModel::Validator end def on_allowlist?(domain) - return false if Rails.configuration.x.email_domains_whitelist.blank? + return false if Rails.configuration.x.email_domains_allowlist.blank? - Rails.configuration.x.email_domains_whitelist.include?(domain) + Rails.configuration.x.email_domains_allowlist.include?(domain) end def resolve_mx(domain) @@ -58,7 +58,7 @@ class EmailMxValidator < ActiveModel::Validator [ips, records] end - def on_blacklist?(domains, attempt_ip) + def email_domain_blocked?(domains, attempt_ip) EmailDomainBlock.block?(domains, attempt_ip: attempt_ip) end end diff --git a/app/validators/blacklisted_email_validator.rb b/app/validators/user_email_validator.rb similarity index 74% rename from app/validators/blacklisted_email_validator.rb rename to app/validators/user_email_validator.rb index 9b3f2e33e55..21b22794ef6 100644 --- a/app/validators/blacklisted_email_validator.rb +++ b/app/validators/user_email_validator.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class BlacklistedEmailValidator < ActiveModel::Validator +class UserEmailValidator < ActiveModel::Validator def validate(user) return if user.valid_invitation? || user.email.blank? @@ -23,18 +23,18 @@ class BlacklistedEmailValidator < ActiveModel::Validator end def not_allowed_through_configuration?(email) - return false if Rails.configuration.x.email_domains_whitelist.blank? + return false if Rails.configuration.x.email_domains_allowlist.blank? - domains = Rails.configuration.x.email_domains_whitelist.gsub('.', '\.') + domains = Rails.configuration.x.email_domains_allowlist.gsub('.', '\.') regexp = Regexp.new("@(.+\\.)?(#{domains})$", true) email !~ regexp end def disallowed_through_configuration?(email) - return false if Rails.configuration.x.email_domains_blacklist.blank? + return false if Rails.configuration.x.email_domains_denylist.blank? - domains = Rails.configuration.x.email_domains_blacklist.gsub('.', '\.') + domains = Rails.configuration.x.email_domains_denylist.gsub('.', '\.') regexp = Regexp.new("@(.+\\.)?(#{domains})", true) regexp.match?(email) diff --git a/app/workers/scheduler/auto_close_registrations_scheduler.rb b/app/workers/scheduler/auto_close_registrations_scheduler.rb index 68745029150..6226e6ea1fe 100644 --- a/app/workers/scheduler/auto_close_registrations_scheduler.rb +++ b/app/workers/scheduler/auto_close_registrations_scheduler.rb @@ -11,7 +11,7 @@ class Scheduler::AutoCloseRegistrationsScheduler OPEN_REGISTRATIONS_MODERATOR_THRESHOLD = 1.week + UserTrackingConcern::SIGN_IN_UPDATE_FREQUENCY def perform - return if Rails.configuration.x.email_domains_whitelist.present? || ENV['DISABLE_AUTOMATIC_SWITCHING_TO_APPROVED_REGISTRATIONS'] == 'true' + return if Rails.configuration.x.email_domains_allowlist.present? || ENV['DISABLE_AUTOMATIC_SWITCHING_TO_APPROVED_REGISTRATIONS'] == 'true' return unless Setting.registrations_mode == 'open' switch_to_approval_mode! unless active_moderators? diff --git a/config/initializers/blacklists.rb b/config/initializers/blacklists.rb deleted file mode 100644 index 0e3339c98e6..00000000000 --- a/config/initializers/blacklists.rb +++ /dev/null @@ -1,6 +0,0 @@ -# frozen_string_literal: true - -Rails.application.configure do - config.x.email_domains_blacklist = (ENV['EMAIL_DOMAIN_DENYLIST'] || ENV['EMAIL_DOMAIN_BLACKLIST']) || '' - config.x.email_domains_whitelist = (ENV['EMAIL_DOMAIN_ALLOWLIST'] || ENV['EMAIL_DOMAIN_WHITELIST']) || '' -end diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 83100b1cf5f..86fde3cacf7 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -147,7 +147,7 @@ Doorkeeper.configure do force_ssl_in_redirect_uri false # Specify what redirect URI's you want to block during Application creation. - # Any redirect URI is whitelisted by default. + # Any redirect URI is allowed by default. # # You can use this option in order to forbid URI's with 'javascript' scheme # for example. diff --git a/config/initializers/email_domains_lists.rb b/config/initializers/email_domains_lists.rb new file mode 100644 index 00000000000..1361b1935ad --- /dev/null +++ b/config/initializers/email_domains_lists.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +Rails.application.configure do + config.x.email_domains_denylist = ENV.fetch('EMAIL_DOMAIN_DENYLIST', nil) || ENV.fetch('EMAIL_DOMAIN_BLACKLIST', '') + config.x.email_domains_allowlist = ENV.fetch('EMAIL_DOMAIN_ALLOWLIST', nil) || ENV.fetch('EMAIL_DOMAIN_WHITELIST', '') +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 4755500fc4e..0f3e25576ee 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -182,30 +182,30 @@ RSpec.describe User do end end - describe 'blacklist' do + describe 'email domains denylist integration' do around do |example| - old_blacklist = Rails.configuration.x.email_blacklist + original = Rails.configuration.x.email_domains_denylist - Rails.configuration.x.email_domains_blacklist = 'mvrht.com' + Rails.configuration.x.email_domains_denylist = 'mvrht.com' example.run - Rails.configuration.x.email_domains_blacklist = old_blacklist + Rails.configuration.x.email_domains_denylist = original end - it 'allows a non-blacklisted user to be created' do + it 'allows a user with an email domain that is not on the denylist to be created' do user = described_class.new(email: 'foo@example.com', account: account, password: password, agreement: true) expect(user).to be_valid end - it 'does not allow a blacklisted user to be created' do + it 'does not allow a user with an email domain on the deylist to be created' do user = described_class.new(email: 'foo@mvrht.com', account: account, password: password, agreement: true) expect(user).to_not be_valid end - it 'does not allow a subdomain blacklisted user to be created' do + it 'does not allow a user with an email where the subdomain is on the denylist to be created' do user = described_class.new(email: 'foo@mvrht.com.topdomain.tld', account: account, password: password, agreement: true) expect(user).to_not be_valid @@ -374,43 +374,43 @@ RSpec.describe User do end end - describe 'whitelist' do + describe 'allowlist integration' do around do |example| - old_whitelist = Rails.configuration.x.email_domains_whitelist + original = Rails.configuration.x.email_domains_allowlist - Rails.configuration.x.email_domains_whitelist = 'mastodon.space' + Rails.configuration.x.email_domains_allowlist = 'mastodon.space' example.run - Rails.configuration.x.email_domains_whitelist = old_whitelist + Rails.configuration.x.email_domains_allowlist = original end - it 'does not allow a user to be created unless they are whitelisted' do + it 'does not allow a user to be created when their email is not on the allowlist' do user = described_class.new(email: 'foo@example.com', account: account, password: password, agreement: true) expect(user).to_not be_valid end - it 'allows a user to be created if they are whitelisted' do + it 'allows a user to be created when their email is on the allowlist' do user = described_class.new(email: 'foo@mastodon.space', account: account, password: password, agreement: true) expect(user).to be_valid end - it 'does not allow a user with a whitelisted top domain as subdomain in their email address to be created' do + it 'does not allow a user with an email subdomain included on the top level domain allowlist to be created' do user = described_class.new(email: 'foo@mastodon.space.userdomain.com', account: account, password: password, agreement: true) expect(user).to_not be_valid end - context 'with a blacklisted subdomain' do + context 'with a subdomain on the denylist' do around do |example| - old_blacklist = Rails.configuration.x.email_blacklist + original = Rails.configuration.x.email_domains_denylist example.run - Rails.configuration.x.email_domains_blacklist = old_blacklist + Rails.configuration.x.email_domains_denylist = original end - it 'does not allow a user to be created with a specific blacklisted subdomain even if the top domain is whitelisted' do - Rails.configuration.x.email_domains_blacklist = 'blacklisted.mastodon.space' + it 'does not allow a user to be created with an email subdomain on the denylist even if the top domain is on the allowlist' do + Rails.configuration.x.email_domains_denylist = 'denylisted.mastodon.space' - user = described_class.new(email: 'foo@blacklisted.mastodon.space', account: account, password: password) + user = described_class.new(email: 'foo@denylisted.mastodon.space', account: account, password: password) expect(user).to_not be_valid end end diff --git a/spec/validators/email_mx_validator_spec.rb b/spec/validators/email_mx_validator_spec.rb index bc26be87295..23a5f768efc 100644 --- a/spec/validators/email_mx_validator_spec.rb +++ b/spec/validators/email_mx_validator_spec.rb @@ -9,10 +9,10 @@ describe EmailMxValidator do context 'with 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' + tmp = Rails.configuration.x.email_domains_allowlist + Rails.configuration.x.email_domains_allowlist = 'example.com' block.call - Rails.configuration.x.email_domains_whitelist = tmp + Rails.configuration.x.email_domains_allowlist = tmp end it 'does not add errors if there are no DNS records' do @@ -69,7 +69,7 @@ describe EmailMxValidator do expect(user.errors).to have_received(:add) end - it 'adds an error if the MX record is blacklisted' do + it 'adds an error if the MX record has an email domain block' do EmailDomainBlock.create!(domain: 'mail.example.com') configure_resolver( diff --git a/spec/validators/blacklisted_email_validator_spec.rb b/spec/validators/user_email_validator_spec.rb similarity index 96% rename from spec/validators/blacklisted_email_validator_spec.rb rename to spec/validators/user_email_validator_spec.rb index 86760df2e7f..92da04ea395 100644 --- a/spec/validators/blacklisted_email_validator_spec.rb +++ b/spec/validators/user_email_validator_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' -RSpec.describe BlacklistedEmailValidator do +RSpec.describe UserEmailValidator do describe '#validate' do subject { described_class.new.validate(user) }