Revert "FEATURE: enforce_canonical_emails site setting"

This reverts commit 6f9177e2ed.

We decided on a completely different approach to the problem.

Instead we will let blocked emails be treated as canonical.
This commit is contained in:
Sam Saffron 2020-04-24 13:25:35 +10:00
parent 6df22638e1
commit 6a18c9aa0b
No known key found for this signature in database
GPG Key ID: B9606168D2FFD9F5
6 changed files with 22 additions and 69 deletions

View File

@ -15,27 +15,12 @@ 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) self.ignored_columns = ['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
@ -49,16 +34,10 @@ class UserEmail < ActiveRecord::Base
end end
def unique_email def unique_email
if self.will_save_change_to_email? if self.will_save_change_to_email? && self.class.where("lower(email) = ?", email).exists?
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?
@ -79,11 +58,9 @@ 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

@ -1586,7 +1586,6 @@ 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,7 +459,6 @@ 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

@ -1,7 +0,0 @@
# 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

@ -0,0 +1,13 @@
# frozen_string_literal: true
class RemoveCanonicalEmailFromUserEmails < ActiveRecord::Migration[6.0]
def up
execute <<~SQL
ALTER TABLE user_emails
DROP COLUMN IF EXISTS canonical_email
SQL
end
def down
# nothing to do, we already nuke the migrations
end
end

View File

@ -672,32 +672,4 @@ 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