From b3ceb3dcc4df62803aa967d7aecee686973a8996 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 17 Apr 2021 03:14:25 +0200 Subject: [PATCH] Add canonical e-mail blocks for suspended accounts (#16049) Prevent new accounts from being created using the same underlying e-mail as a suspended account using extensions and period permutations. Stores e-mails as a SHA256 hash --- app/helpers/email_helper.rb | 18 +++++++ app/models/account.rb | 14 ++++++ app/models/canonical_email_block.rb | 27 +++++++++++ app/validators/blacklisted_email_validator.rb | 32 ++++++++----- ...416200740_create_canonical_email_blocks.rb | 10 ++++ db/schema.rb | 12 ++++- .../canonical_email_block_fabricator.rb | 4 ++ spec/models/canonical_email_block_spec.rb | 47 +++++++++++++++++++ .../blacklisted_email_validator_spec.rb | 29 ++++++++---- 9 files changed, 172 insertions(+), 21 deletions(-) create mode 100644 app/helpers/email_helper.rb create mode 100644 app/models/canonical_email_block.rb create mode 100644 db/migrate/20210416200740_create_canonical_email_blocks.rb create mode 100644 spec/fabricators/canonical_email_block_fabricator.rb create mode 100644 spec/models/canonical_email_block_spec.rb diff --git a/app/helpers/email_helper.rb b/app/helpers/email_helper.rb new file mode 100644 index 000000000..360783c62 --- /dev/null +++ b/app/helpers/email_helper.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module EmailHelper + def self.included(base) + base.extend(self) + end + + def email_to_canonical_email(str) + username, domain = str.downcase.split('@', 2) + username, = username.gsub('.', '').split('+', 2) + + "#{username}@#{domain}" + end + + def email_to_canonical_email_hash(str) + Digest::SHA2.new(256).hexdigest(email_to_canonical_email(str)) + end +end diff --git a/app/models/account.rb b/app/models/account.rb index 80689d4aa..a573365de 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -235,6 +235,7 @@ class Account < ApplicationRecord transaction do create_deletion_request! update!(suspended_at: date, suspension_origin: origin) + create_canonical_email_block! end end @@ -242,6 +243,7 @@ class Account < ApplicationRecord transaction do deletion_request&.destroy! update!(suspended_at: nil, suspension_origin: nil) + destroy_canonical_email_block! end end @@ -569,4 +571,16 @@ class Account < ApplicationRecord def clean_feed_manager FeedManager.instance.clean_feeds!(:home, [id]) end + + def create_canonical_email_block! + return unless local? && user_email.present? + + CanonicalEmailBlock.create(reference_account: self, email: user_email) + end + + def destroy_canonical_email_block! + return unless local? + + CanonicalEmailBlock.where(reference_account: self).delete_all + end end diff --git a/app/models/canonical_email_block.rb b/app/models/canonical_email_block.rb new file mode 100644 index 000000000..a8546d65a --- /dev/null +++ b/app/models/canonical_email_block.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true +# == Schema Information +# +# Table name: canonical_email_blocks +# +# id :bigint(8) not null, primary key +# canonical_email_hash :string default(""), not null +# reference_account_id :bigint(8) not null +# created_at :datetime not null +# updated_at :datetime not null +# + +class CanonicalEmailBlock < ApplicationRecord + include EmailHelper + + belongs_to :reference_account, class_name: 'Account' + + validates :canonical_email_hash, presence: true + + def email=(email) + self.canonical_email_hash = email_to_canonical_email_hash(email) + end + + def self.block?(email) + where(canonical_email_hash: email_to_canonical_email_hash(email)).exists? + end +end diff --git a/app/validators/blacklisted_email_validator.rb b/app/validators/blacklisted_email_validator.rb index 1ca73fdcc..eb66ad93d 100644 --- a/app/validators/blacklisted_email_validator.rb +++ b/app/validators/blacklisted_email_validator.rb @@ -6,26 +6,25 @@ class BlacklistedEmailValidator < ActiveModel::Validator @email = user.email - user.errors.add(:email, :blocked) if blocked_email? + user.errors.add(:email, :blocked) if blocked_email_provider? + user.errors.add(:email, :taken) if blocked_canonical_email? end private - def blocked_email? - on_blacklist? || not_on_whitelist? + def blocked_email_provider? + disallowed_through_email_domain_block? || disallowed_through_configuration? || not_allowed_through_configuration? end - def on_blacklist? - return true if EmailDomainBlock.block?(@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) + def blocked_canonical_email? + CanonicalEmailBlock.block?(@email) end - def not_on_whitelist? + def disallowed_through_email_domain_block? + EmailDomainBlock.block?(@email) + end + + def not_allowed_through_configuration? return false if Rails.configuration.x.email_domains_whitelist.blank? domains = Rails.configuration.x.email_domains_whitelist.gsub('.', '\.') @@ -33,4 +32,13 @@ class BlacklistedEmailValidator < ActiveModel::Validator @email !~ regexp end + + def disallowed_through_configuration? + 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) + end end diff --git a/db/migrate/20210416200740_create_canonical_email_blocks.rb b/db/migrate/20210416200740_create_canonical_email_blocks.rb new file mode 100644 index 000000000..a1f1660bf --- /dev/null +++ b/db/migrate/20210416200740_create_canonical_email_blocks.rb @@ -0,0 +1,10 @@ +class CreateCanonicalEmailBlocks < ActiveRecord::Migration[6.1] + def change + create_table :canonical_email_blocks do |t| + t.string :canonical_email_hash, null: false, default: '', index: { unique: true } + t.belongs_to :reference_account, null: false, foreign_key: { on_cascade: :delete, to_table: 'accounts' } + + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 599cd873b..dcbbf4aea 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: 2021_03_24_171613) do +ActiveRecord::Schema.define(version: 2021_04_16_200740) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -280,6 +280,15 @@ ActiveRecord::Schema.define(version: 2021_03_24_171613) do t.index ["status_id"], name: "index_bookmarks_on_status_id" end + create_table "canonical_email_blocks", force: :cascade do |t| + t.string "canonical_email_hash", default: "", null: false + t.bigint "reference_account_id", null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["canonical_email_hash"], name: "index_canonical_email_blocks_on_canonical_email_hash", unique: true + t.index ["reference_account_id"], name: "index_canonical_email_blocks_on_reference_account_id" + end + create_table "conversation_mutes", force: :cascade do |t| t.bigint "conversation_id", null: false t.bigint "account_id", null: false @@ -991,6 +1000,7 @@ ActiveRecord::Schema.define(version: 2021_03_24_171613) do add_foreign_key "blocks", "accounts", name: "fk_4269e03e65", on_delete: :cascade add_foreign_key "bookmarks", "accounts", on_delete: :cascade add_foreign_key "bookmarks", "statuses", on_delete: :cascade + add_foreign_key "canonical_email_blocks", "accounts", column: "reference_account_id" add_foreign_key "conversation_mutes", "accounts", name: "fk_225b4212bb", on_delete: :cascade add_foreign_key "conversation_mutes", "conversations", on_delete: :cascade add_foreign_key "custom_filters", "accounts", on_delete: :cascade diff --git a/spec/fabricators/canonical_email_block_fabricator.rb b/spec/fabricators/canonical_email_block_fabricator.rb new file mode 100644 index 000000000..a0b6e0d22 --- /dev/null +++ b/spec/fabricators/canonical_email_block_fabricator.rb @@ -0,0 +1,4 @@ +Fabricator(:canonical_email_block) do + email "test@example.com" + reference_account { Fabricate(:account) } +end diff --git a/spec/models/canonical_email_block_spec.rb b/spec/models/canonical_email_block_spec.rb new file mode 100644 index 000000000..8e0050d65 --- /dev/null +++ b/spec/models/canonical_email_block_spec.rb @@ -0,0 +1,47 @@ +require 'rails_helper' + +RSpec.describe CanonicalEmailBlock, type: :model do + describe '#email=' do + let(:target_hash) { '973dfe463ec85785f5f95af5ba3906eedb2d931c24e69824a89ea65dba4e813b' } + + it 'sets canonical_email_hash' do + subject.email = 'test@example.com' + expect(subject.canonical_email_hash).to eq target_hash + end + + it 'sets the same hash even with dot permutations' do + subject.email = 't.e.s.t@example.com' + expect(subject.canonical_email_hash).to eq target_hash + end + + it 'sets the same hash even with extensions' do + subject.email = 'test+mastodon1@example.com' + expect(subject.canonical_email_hash).to eq target_hash + end + + it 'sets the same hash with different casing' do + subject.email = 'Test@EXAMPLE.com' + expect(subject.canonical_email_hash).to eq target_hash + end + end + + describe '.block?' do + let!(:canonical_email_block) { Fabricate(:canonical_email_block, email: 'foo@bar.com') } + + it 'returns true for the same email' do + expect(described_class.block?('foo@bar.com')).to be true + end + + it 'returns true for the same email with dots' do + expect(described_class.block?('f.oo@bar.com')).to be true + end + + it 'returns true for the same email with extensions' do + expect(described_class.block?('foo+spam@bar.com')).to be true + end + + it 'returns false for different email' do + expect(described_class.block?('hoge@bar.com')).to be false + end + end +end diff --git a/spec/validators/blacklisted_email_validator_spec.rb b/spec/validators/blacklisted_email_validator_spec.rb index 53b355a57..f7d5e01bc 100644 --- a/spec/validators/blacklisted_email_validator_spec.rb +++ b/spec/validators/blacklisted_email_validator_spec.rb @@ -9,23 +9,36 @@ RSpec.describe BlacklistedEmailValidator, type: :validator do before do allow(user).to receive(:valid_invitation?) { false } - allow_any_instance_of(described_class).to receive(:blocked_email?) { blocked_email } - described_class.new.validate(user) + allow_any_instance_of(described_class).to receive(:blocked_email_provider?) { blocked_email } end - context 'blocked_email?' do + subject { described_class.new.validate(user); errors } + + context 'when e-mail provider is blocked' do let(:blocked_email) { true } - it 'calls errors.add' do - expect(errors).to have_received(:add).with(:email, :blocked) + it 'adds error' do + expect(subject).to have_received(:add).with(:email, :blocked) end end - context '!blocked_email?' do + context 'when e-mail provider is not blocked' do let(:blocked_email) { false } - it 'not calls errors.add' do - expect(errors).not_to have_received(:add).with(:email, :blocked) + it 'does not add errors' do + expect(subject).not_to have_received(:add).with(:email, :blocked) + end + + context 'when canonical e-mail is blocked' do + let(:other_user) { Fabricate(:user, email: 'i.n.f.o@mail.com') } + + before do + other_user.account.suspend! + end + + it 'adds error' do + expect(subject).to have_received(:add).with(:email, :taken) + end end end end