FEATURE: when blocking emails prefer blocking canonical

Previously we relied entirely on levenshtein_distance_spammer_emails site
setting to handle "similar looking" emails.

This commit improves the situation by always preferring to block (and check)
canonical emails.

This means that if:

`samevil+test@domain.com` is blocked the system will block `samevil@domain.com`

This means that `samevil+2@domain.com` (ad infinitum) will be blocked
This commit is contained in:
Sam Saffron 2020-04-24 14:09:51 +10:00
parent 6a18c9aa0b
commit cbceadf48b
No known key found for this signature in database
GPG Key ID: B9606168D2FFD9F5
2 changed files with 23 additions and 1 deletions

View File

@ -18,11 +18,24 @@ class ScreenedEmail < ActiveRecord::Base
self.email = email.downcase
end
def self.canonical(email)
name, domain = email.split('@', 2)
name = name.gsub(/\+.*/, '')
if ['gmail.com', 'googlemail.com'].include?(domain.downcase)
name = name.gsub('.', '')
end
"#{name}@#{domain}".downcase
end
def self.block(email, opts = {})
find_by_email(Email.downcase(email)) || create(opts.slice(:action_type, :ip_address).merge(email: email))
email = canonical(email)
find_by_email(email) || create!(opts.slice(:action_type, :ip_address).merge(email: email))
end
def self.should_block?(email)
email = canonical(email)
screened_emails = ScreenedEmail.order(created_at: :desc).limit(100)
distances = {}

View File

@ -26,6 +26,7 @@ describe ScreenedEmail do
describe '#block' do
context 'email is not being blocked' do
it 'creates a new record with default action of :block' do
record = ScreenedEmail.block(email)
expect(record).not_to be_new_record
@ -57,6 +58,14 @@ describe ScreenedEmail do
describe '#should_block?' do
subject { ScreenedEmail.should_block?(email) }
it "automatically blocks via email canonicalization" do
SiteSetting.levenshtein_distance_spammer_emails = 0
ScreenedEmail.block('bad.acTor+1@gmail.com')
ScreenedEmail.block('bad.actOr+2@gmail.com')
expect(ScreenedEmail.should_block?('b.a.dactor@gmail.com')).to eq(true)
end
it "returns false if a record with the email doesn't exist" do
expect(subject).to eq(false)
end