FEATURE: enforce_canonical_emails site setting

The new `enforce_canonical_emails` site setting ensures that emails in the
canonical form are unique.

This mean that if `s.a.m+1@gmail.com` is registered `sam@gmail.com` will
not be allowed.

The commit contains a blanket "tag strip" (stripping everything after +)
it also contains special handling of a "dot strip" for googlemail and gmail.

The setting only impacts new registrations after `enforce_canonical_emails`

The setting is default false so it will not impact any existing installs.
This commit is contained in:
Sam Saffron 2020-04-14 14:16:30 +10:00
parent e2284cf739
commit 6f9177e2ed
No known key found for this signature in database
GPG Key ID: B9606168D2FFD9F5
5 changed files with 70 additions and 8 deletions

View File

@ -15,10 +15,27 @@ class UserEmail < ActiveRecord::Base
validate :user_id_not_changed, if: :primary validate :user_id_not_changed, if: :primary
validate :unique_email validate :unique_email
before_save :save_canonical
scope :secondary, -> { where(primary: false) } scope :secondary, -> { where(primary: false) }
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
private private
def save_canonical
if SiteSetting.enforce_canonical_emails && self.will_save_change_to_email?
self.canonical_email = UserEmail.canonical(self.email)
end
end
def strip_downcase_email def strip_downcase_email
if self.email if self.email
self.email = self.email.strip self.email = self.email.strip
@ -32,10 +49,16 @@ class UserEmail < ActiveRecord::Base
end end
def unique_email def unique_email
if self.will_save_change_to_email? && self.class.where("lower(email) = ?", email).exists? if self.will_save_change_to_email?
exists = self.class.where("lower(email) = ?", email).exists?
exists ||= SiteSetting.enforce_canonical_emails &&
self.class.where("canonical_email = ?", UserEmail.canonical(email)).exists?
if exists
self.errors.add(:email, :taken) self.errors.add(:email, :taken)
end end
end end
end
def user_id_not_changed def user_id_not_changed
if self.will_save_change_to_user_id? && self.persisted? if self.will_save_change_to_user_id? && self.persisted?
@ -56,9 +79,11 @@ end
# primary :boolean default(FALSE), not null # primary :boolean default(FALSE), not null
# created_at :datetime not null # created_at :datetime not null
# updated_at :datetime not null # updated_at :datetime not null
# canonical_email :string
# #
# Indexes # Indexes
# #
# index_user_emails_on_canonical_email (canonical_email) WHERE (canonical_email IS NOT NULL)
# index_user_emails_on_email (lower((email)::text)) UNIQUE # index_user_emails_on_email (lower((email)::text)) UNIQUE
# index_user_emails_on_user_id (user_id) # index_user_emails_on_user_id (user_id)
# index_user_emails_on_user_id_and_primary (user_id,primary) UNIQUE WHERE "primary" # index_user_emails_on_user_id_and_primary (user_id,primary) UNIQUE WHERE "primary"

View File

@ -1582,6 +1582,7 @@ en:
allow_index_in_robots_txt: "Specify in robots.txt that this site is allowed to be indexed by web search engines. In exceptional cases you can permanently <a href='%{base_path}/admin/customize/robots'>override robots.txt</a>." allow_index_in_robots_txt: "Specify in robots.txt that this site is allowed to be indexed by web search engines. In exceptional cases you can permanently <a href='%{base_path}/admin/customize/robots'>override robots.txt</a>."
email_domains_blacklist: "A pipe-delimited list of email domains that users are not allowed to register accounts with. Example: mailinator.com|trashmail.net" email_domains_blacklist: "A pipe-delimited list of email domains that users are not allowed to register accounts with. Example: mailinator.com|trashmail.net"
email_domains_whitelist: "A pipe-delimited list of email domains that users MUST register accounts with. WARNING: Users with email domains other than those listed will not be allowed!" email_domains_whitelist: "A pipe-delimited list of email domains that users MUST register accounts with. WARNING: Users with email domains other than those listed will not be allowed!"
enforce_canonical_emails: "Prior to creating users strip email to canonical form ensuring uniqueness. Setting will only take effect on new account registrations. When set user+1@domain.com will not be allowed to register if user+2@domain.com is already registered."
auto_approve_email_domains: "Users with email addresses from this list of domains will be automatically approved." auto_approve_email_domains: "Users with email addresses from this list of domains will be automatically approved."
hide_email_address_taken: "Don't inform users that an account exists with a given email address during signup and from the forgot password form." hide_email_address_taken: "Don't inform users that an account exists with a given email address during signup and from the forgot password form."
log_out_strict: "When logging out, log out ALL sessions for the user on all devices" log_out_strict: "When logging out, log out ALL sessions for the user on all devices"

View File

@ -459,6 +459,7 @@ login:
email_domains_whitelist: email_domains_whitelist:
default: "" default: ""
type: list type: list
enforce_canonical_emails: false
auto_approve_email_domains: auto_approve_email_domains:
default: "" default: ""
type: list type: list

View File

@ -0,0 +1,7 @@
# frozen_string_literal: true
class AddCanonicalEmailToUserEmail < ActiveRecord::Migration[6.0]
def change
add_column :user_emails, :canonical_email, :string, length: 513
add_index :user_emails, :canonical_email, where: 'canonical_email IS NOT NULL'
end
end

View File

@ -672,4 +672,32 @@ describe Jobs::UserEmail do
end end
end end
context "canonical emails" do
it "correctly creates canonical emails" do
expect(UserEmail.canonical('s.a.m+1@gmail.com')).to eq('sam@gmail.com')
expect(UserEmail.canonical('sa.m+1@googlemail.com')).to eq('sam@googlemail.com')
expect(UserEmail.canonical('sa.m+1722@sam.com')).to eq('sa.m@sam.com')
expect(UserEmail.canonical('sa.m@sam.com')).to eq('sa.m@sam.com')
end
it "correctly bans non canonical emails" do
email = UserEmail.create!(email: 'sam@sam.com', user_id: user.id)
expect(email.canonical_email).to eq(nil)
email = UserEmail.create!(email: 'sam+1@sam.com', user_id: user.id)
expect(email.canonical_email).to eq(nil)
SiteSetting.enforce_canonical_emails = true
email = UserEmail.create!(email: 'Sam+5@sam.com', user_id: user.id)
expect(email.canonical_email).to eq('sam@sam.com')
expect do
UserEmail.create!(email: 'saM+3@sam.com', user_id: user.id)
end.to raise_error(ActiveRecord::RecordInvalid)
end
end
end end