diff --git a/app/models/user_email.rb b/app/models/user_email.rb index 57032164c96..e4bfe5034af 100644 --- a/app/models/user_email.rb +++ b/app/models/user_email.rb @@ -15,27 +15,12 @@ class UserEmail < ActiveRecord::Base validate :user_id_not_changed, if: :primary validate :unique_email - before_save :save_canonical - 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 + self.ignored_columns = ['canonical_email'] 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 if self.email self.email = self.email.strip @@ -49,14 +34,8 @@ class UserEmail < ActiveRecord::Base end def unique_email - 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) - end + if self.will_save_change_to_email? && self.class.where("lower(email) = ?", email).exists? + self.errors.add(:email, :taken) end end @@ -73,17 +52,15 @@ end # # Table name: user_emails # -# id :integer not null, primary key -# user_id :integer not null -# email :string(513) not null -# primary :boolean default(FALSE), not null -# created_at :datetime not null -# updated_at :datetime not null -# canonical_email :string +# id :integer not null, primary key +# user_id :integer not null +# email :string(513) not null +# primary :boolean default(FALSE), not null +# created_at :datetime not null +# updated_at :datetime not null # # 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_user_id (user_id) # index_user_emails_on_user_id_and_primary (user_id,primary) UNIQUE WHERE "primary" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 004f511e03f..d38b0caeab9 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -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 override robots.txt." 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!" - 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." 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" diff --git a/config/site_settings.yml b/config/site_settings.yml index 986fc3ba38c..8037237c2a7 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -459,7 +459,6 @@ login: email_domains_whitelist: default: "" type: list - enforce_canonical_emails: false auto_approve_email_domains: default: "" type: list diff --git a/db/migrate/20200414034210_add_canonical_email_to_user_email.rb b/db/migrate/20200414034210_add_canonical_email_to_user_email.rb deleted file mode 100644 index 7033f55bb7e..00000000000 --- a/db/migrate/20200414034210_add_canonical_email_to_user_email.rb +++ /dev/null @@ -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 diff --git a/db/post_migrate/20200424032633_remove_canonical_email_from_user_emails.rb b/db/post_migrate/20200424032633_remove_canonical_email_from_user_emails.rb new file mode 100644 index 00000000000..8d5a0c9ff3f --- /dev/null +++ b/db/post_migrate/20200424032633_remove_canonical_email_from_user_emails.rb @@ -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 diff --git a/spec/jobs/user_email_spec.rb b/spec/jobs/user_email_spec.rb index 3cea6c30787..e6def350b2f 100644 --- a/spec/jobs/user_email_spec.rb +++ b/spec/jobs/user_email_spec.rb @@ -672,32 +672,4 @@ describe Jobs::UserEmail do 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