diff --git a/app/assets/javascripts/discourse/controllers/preferences/email.js.es6 b/app/assets/javascripts/discourse/controllers/preferences/email.js.es6 index 48c925fb0..393b1801d 100644 --- a/app/assets/javascripts/discourse/controllers/preferences/email.js.es6 +++ b/app/assets/javascripts/discourse/controllers/preferences/email.js.es6 @@ -15,7 +15,11 @@ export default Discourse.ObjectController.extend({ newEmailEmpty: Em.computed.empty('newEmail'), saveDisabled: Em.computed.or('saving', 'newEmailEmpty', 'taken', 'unchanged'), - unchanged: Discourse.computed.propertyEqual('newEmail', 'email'), + unchanged: Discourse.computed.propertyEqual('newEmailLower', 'email'), + + newEmailLower: function() { + return this.get('newEmail').toLowerCase(); + }.property('newEmail'), saveButtonText: function() { if (this.get('saving')) return I18n.t("saving"); diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 5a078075b..abc0b5319 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -251,7 +251,7 @@ class UsersController < ApplicationController lower_email = Email.downcase(params[:email]).strip # Raise an error if the email is already in use - if User.where("email = ?", lower_email).exists? + if User.find_by_email(lower_email) raise Discourse::InvalidParameters.new(:email) end diff --git a/app/models/category.rb b/app/models/category.rb index 76d1710cf..3adaf1fec 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -28,6 +28,7 @@ class Category < ActiveRecord::Base before_validation :ensure_slug before_save :apply_permissions + before_save :downcase_email after_create :create_category_definition after_create :publish_categories_list after_destroy :publish_categories_list @@ -245,6 +246,10 @@ SQL end end + def downcase_email + self.email_in = email_in.downcase if self.email_in + end + def secure_group_ids if self.read_restricted? groups.pluck("groups.id") diff --git a/app/models/discourse_single_sign_on.rb b/app/models/discourse_single_sign_on.rb index e277b057c..52eff1357 100644 --- a/app/models/discourse_single_sign_on.rb +++ b/app/models/discourse_single_sign_on.rb @@ -76,7 +76,7 @@ class DiscourseSingleSignOn < SingleSignOn private def match_email_or_create_user - user = User.find_by(email: Email.downcase(email)) + user = User.find_by_email(email) try_name = name.blank? ? nil : name try_username = username.blank? ? nil : username diff --git a/app/models/email_token.rb b/app/models/email_token.rb index 0d02e0d16..809a2368c 100644 --- a/app/models/email_token.rb +++ b/app/models/email_token.rb @@ -7,6 +7,7 @@ class EmailToken < ActiveRecord::Base before_validation(on: :create) do self.token = EmailToken.generate_token + self.email = self.email.downcase if self.email end after_create do diff --git a/app/models/screened_email.rb b/app/models/screened_email.rb index a7bb72f3e..05f0f9422 100644 --- a/app/models/screened_email.rb +++ b/app/models/screened_email.rb @@ -12,15 +12,21 @@ class ScreenedEmail < ActiveRecord::Base validates :email, presence: true, uniqueness: true + before_save :downcase_email + + def downcase_email + self.email = email.downcase + end + def self.block(email, opts={}) - find_by_email(email) || create(opts.slice(:action_type, :ip_address).merge({email: email})) + find_by_email(Email.downcase(email)) || create(opts.slice(:action_type, :ip_address).merge({email: email})) end def self.should_block?(email) screened_emails = ScreenedEmail.order(created_at: :desc).limit(100) distances = {} - screened_emails.each { |se| distances[se.email] = levenshtein(se.email, email) } + screened_emails.each { |se| distances[se.email] = levenshtein(se.email.downcase, email.downcase) } max_distance = SiteSetting.levenshtein_distance_spammer_emails screened_email = screened_emails.select { |se| distances[se.email] <= max_distance } diff --git a/app/models/user.rb b/app/models/user.rb index e3973089c..856354fe8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -58,6 +58,8 @@ class User < ActiveRecord::Base delegate :last_sent_email_address, :to => :email_logs + before_validation :downcase_email + validates_presence_of :username validate :username_validator validates :email, presence: true, uniqueness: true @@ -687,6 +689,10 @@ class User < ActiveRecord::Base self.username_lower = username.downcase end + def downcase_email + self.email = self.email.downcase if self.email + end + def username_validator username_format_validator || begin lower = username.downcase diff --git a/db/migrate/20140715160720_update_users_case_insensitive_emails.rb b/db/migrate/20140715160720_update_users_case_insensitive_emails.rb new file mode 100644 index 000000000..13c1aebef --- /dev/null +++ b/db/migrate/20140715160720_update_users_case_insensitive_emails.rb @@ -0,0 +1,26 @@ +class UpdateUsersCaseInsensitiveEmails < ActiveRecord::Migration + def up + execute "DROP INDEX index_users_on_email" + + # Find duplicate emails. + results = execute < 1 +SQL + + results.each do |row| + execute "UPDATE users SET email = '#{row['email'].downcase}#{row['count']}' WHERE id = #{row['id']}" + end + + execute "UPDATE users SET email = lower(email)" + execute "CREATE UNIQUE INDEX index_users_on_email ON users ((lower(email)));" + end + + def down + execute "DROP INDEX index_users_on_email" + execute "CREATE UNIQUE INDEX index_users_on_email ON users (email);" + end +end diff --git a/lib/auth/facebook_authenticator.rb b/lib/auth/facebook_authenticator.rb index f2f85c70d..3b5ddf503 100644 --- a/lib/auth/facebook_authenticator.rb +++ b/lib/auth/facebook_authenticator.rb @@ -20,7 +20,7 @@ class Auth::FacebookAuthenticator < Auth::Authenticator user_info = FacebookUserInfo.find_by(facebook_user_id: facebook_hash[:facebook_user_id]) result.user = user_info.try(:user) - if !result.user && !email.blank? && result.user = User.find_by(email: Email.downcase(email)) + if !result.user && !email.blank? && result.user = User.find_by_email(email) FacebookUserInfo.create({user_id: result.user.id}.merge(facebook_hash)) end diff --git a/lib/auth/google_oauth2_authenticator.rb b/lib/auth/google_oauth2_authenticator.rb index 63158e4ce..86af469e2 100644 --- a/lib/auth/google_oauth2_authenticator.rb +++ b/lib/auth/google_oauth2_authenticator.rb @@ -18,7 +18,7 @@ class Auth::GoogleOAuth2Authenticator < Auth::Authenticator user_info = GoogleUserInfo.find_by(google_user_id: google_hash[:google_user_id]) result.user = user_info.try(:user) - if !result.user && !result.email.blank? && result.user = User.find_by(email: Email.downcase(result.email)) + if !result.user && !result.email.blank? && result.user = User.find_by_email(result.email) GoogleUserInfo.create({user_id: result.user.id}.merge(google_hash)) end diff --git a/lib/email.rb b/lib/email.rb index 0075b2a90..07da49eb9 100644 --- a/lib/email.rb +++ b/lib/email.rb @@ -18,7 +18,7 @@ module Email def self.downcase(email) return email unless Email.is_valid?(email) - email.gsub(/^(.+@{1})([^@]+)$/) { $1 + $2.downcase } + email.downcase end end diff --git a/spec/components/email/email_spec.rb b/spec/components/email/email_spec.rb index ae93e6b72..47c2e050e 100644 --- a/spec/components/email/email_spec.rb +++ b/spec/components/email/email_spec.rb @@ -25,9 +25,9 @@ describe Email do describe "downcase" do - it 'downcases only the host part' do - Email.downcase('SAM@GMAIL.COM').should == 'SAM@gmail.com' - Email.downcase('sam@GMAIL.COM').should_not == 'SAM@gmail.com' + it 'downcases local and host part' do + Email.downcase('SAM@GMAIL.COM').should == 'sam@gmail.com' + Email.downcase('sam@GMAIL.COM').should == 'sam@gmail.com' end it 'leaves invalid emails untouched' do diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 9d4ea00db..7c618a2ef 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -203,6 +203,14 @@ describe UsersController do end end + context 'when new email is different case of existing email' do + let!(:other_user) { Fabricate(:user, email: 'case.insensitive@gmail.com')} + + it 'raises an error' do + lambda { xhr :put, :change_email, username: user.username, email: other_user.email.upcase }.should raise_error(Discourse::InvalidParameters) + end + end + context 'success' do it 'has an email token' do diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb index 422dbbb41..f29c38331 100644 --- a/spec/models/category_spec.rb +++ b/spec/models/category_spec.rb @@ -423,4 +423,16 @@ describe Category do end + describe "find_by_email" do + it "is case insensitive" do + c1 = Fabricate(:category, email_in: 'lower@example.com') + c2 = Fabricate(:category, email_in: 'UPPER@EXAMPLE.COM') + c3 = Fabricate(:category, email_in: 'Mixed.Case@Example.COM') + Category.find_by_email('LOWER@EXAMPLE.COM').should == c1 + Category.find_by_email('upper@example.com').should == c2 + Category.find_by_email('mixed.case@example.com').should == c3 + Category.find_by_email('MIXED.CASE@EXAMPLE.COM').should == c3 + end + end + end diff --git a/spec/models/email_token_spec.rb b/spec/models/email_token_spec.rb index fb9835325..db7df7cd6 100644 --- a/spec/models/email_token_spec.rb +++ b/spec/models/email_token_spec.rb @@ -16,6 +16,11 @@ describe EmailToken do email_token.should be_present end + it 'should downcase the email' do + token = user.email_tokens.create(email: "UpperCaseSoWoW@GMail.com") + token.email.should == "uppercasesowow@gmail.com" + end + it 'is valid' do email_token.should be_valid end diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index ac1b86a76..49e6f5161 100644 --- a/spec/models/invite_spec.rb +++ b/spec/models/invite_spec.rb @@ -79,7 +79,7 @@ describe Invite do it 'returns the original invite' do topic.invite_by_email(inviter, 'iceking@adventuretime.ooo').should == @invite topic.invite_by_email(inviter, 'iceking@ADVENTURETIME.ooo').should == @invite - topic.invite_by_email(inviter, 'ICEKING@adventuretime.ooo').should_not == @invite + topic.invite_by_email(inviter, 'ICEKING@adventuretime.ooo').should == @invite end it 'returns a new invite if the other has expired' do diff --git a/spec/models/screened_email_spec.rb b/spec/models/screened_email_spec.rb index e607a7a0c..5feaf2e00 100644 --- a/spec/models/screened_email_spec.rb +++ b/spec/models/screened_email_spec.rb @@ -15,6 +15,11 @@ describe ScreenedEmail do # have ever been blocked by looking at last_match_at. ScreenedEmail.create(email: email).last_match_at.should be_nil end + + it "downcases the email" do + s = ScreenedEmail.create(email: 'SPAMZ@EXAMPLE.COM') + s.email.should == 'spamz@example.com' + end end describe '#block' do @@ -66,6 +71,11 @@ describe ScreenedEmail do ScreenedEmail.should_block?(email).should be_true end + it "returns true when it's same email, but all caps" do + ScreenedEmail.create(email: email).save + ScreenedEmail.should_block?(email.upcase).should be_true + end + shared_examples "when a ScreenedEmail record matches" do it "updates statistics" do Timecop.freeze(Time.zone.now) do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e7208f515..b39ce620c 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -242,6 +242,12 @@ describe User do its(:email_tokens) { should be_present } end + + it "downcases email addresses" do + user = Fabricate.build(:user, email: 'Fancy.Caps.4.U@gmail.com') + user.save + user.reload.email.should == 'fancy.caps.4.u@gmail.com' + end end describe 'ip address validation' do @@ -755,7 +761,7 @@ describe User do expect(found_user).to eq bob found_user = User.find_by_username_or_email('Bob@Example.com') - expect(found_user).to be_nil + expect(found_user).to eq bob found_user = User.find_by_username_or_email('bob1') expect(found_user).to be_nil @@ -763,6 +769,9 @@ describe User do found_user = User.find_by_email('bob@Example.com') expect(found_user).to eq bob + found_user = User.find_by_email('BOB@Example.com') + expect(found_user).to eq bob + found_user = User.find_by_email('bob') expect(found_user).to be_nil