mirror of
https://github.com/codeninjasllc/discourse.git
synced 2024-11-23 23:58:31 -05:00
Emails are case insensitive
This commit is contained in:
parent
0c8025d513
commit
01a68f8cc7
18 changed files with 105 additions and 13 deletions
|
@ -15,7 +15,11 @@ export default Discourse.ObjectController.extend({
|
||||||
|
|
||||||
newEmailEmpty: Em.computed.empty('newEmail'),
|
newEmailEmpty: Em.computed.empty('newEmail'),
|
||||||
saveDisabled: Em.computed.or('saving', 'newEmailEmpty', 'taken', 'unchanged'),
|
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() {
|
saveButtonText: function() {
|
||||||
if (this.get('saving')) return I18n.t("saving");
|
if (this.get('saving')) return I18n.t("saving");
|
||||||
|
|
|
@ -251,7 +251,7 @@ class UsersController < ApplicationController
|
||||||
lower_email = Email.downcase(params[:email]).strip
|
lower_email = Email.downcase(params[:email]).strip
|
||||||
|
|
||||||
# Raise an error if the email is already in use
|
# 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)
|
raise Discourse::InvalidParameters.new(:email)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -28,6 +28,7 @@ class Category < ActiveRecord::Base
|
||||||
|
|
||||||
before_validation :ensure_slug
|
before_validation :ensure_slug
|
||||||
before_save :apply_permissions
|
before_save :apply_permissions
|
||||||
|
before_save :downcase_email
|
||||||
after_create :create_category_definition
|
after_create :create_category_definition
|
||||||
after_create :publish_categories_list
|
after_create :publish_categories_list
|
||||||
after_destroy :publish_categories_list
|
after_destroy :publish_categories_list
|
||||||
|
@ -245,6 +246,10 @@ SQL
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def downcase_email
|
||||||
|
self.email_in = email_in.downcase if self.email_in
|
||||||
|
end
|
||||||
|
|
||||||
def secure_group_ids
|
def secure_group_ids
|
||||||
if self.read_restricted?
|
if self.read_restricted?
|
||||||
groups.pluck("groups.id")
|
groups.pluck("groups.id")
|
||||||
|
|
|
@ -76,7 +76,7 @@ class DiscourseSingleSignOn < SingleSignOn
|
||||||
private
|
private
|
||||||
|
|
||||||
def match_email_or_create_user
|
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_name = name.blank? ? nil : name
|
||||||
try_username = username.blank? ? nil : username
|
try_username = username.blank? ? nil : username
|
||||||
|
|
|
@ -7,6 +7,7 @@ class EmailToken < ActiveRecord::Base
|
||||||
|
|
||||||
before_validation(on: :create) do
|
before_validation(on: :create) do
|
||||||
self.token = EmailToken.generate_token
|
self.token = EmailToken.generate_token
|
||||||
|
self.email = self.email.downcase if self.email
|
||||||
end
|
end
|
||||||
|
|
||||||
after_create do
|
after_create do
|
||||||
|
|
|
@ -12,15 +12,21 @@ class ScreenedEmail < ActiveRecord::Base
|
||||||
|
|
||||||
validates :email, presence: true, uniqueness: true
|
validates :email, presence: true, uniqueness: true
|
||||||
|
|
||||||
|
before_save :downcase_email
|
||||||
|
|
||||||
|
def downcase_email
|
||||||
|
self.email = email.downcase
|
||||||
|
end
|
||||||
|
|
||||||
def self.block(email, opts={})
|
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
|
end
|
||||||
|
|
||||||
def self.should_block?(email)
|
def self.should_block?(email)
|
||||||
screened_emails = ScreenedEmail.order(created_at: :desc).limit(100)
|
screened_emails = ScreenedEmail.order(created_at: :desc).limit(100)
|
||||||
|
|
||||||
distances = {}
|
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
|
max_distance = SiteSetting.levenshtein_distance_spammer_emails
|
||||||
screened_email = screened_emails.select { |se| distances[se.email] <= max_distance }
|
screened_email = screened_emails.select { |se| distances[se.email] <= max_distance }
|
||||||
|
|
|
@ -58,6 +58,8 @@ class User < ActiveRecord::Base
|
||||||
|
|
||||||
delegate :last_sent_email_address, :to => :email_logs
|
delegate :last_sent_email_address, :to => :email_logs
|
||||||
|
|
||||||
|
before_validation :downcase_email
|
||||||
|
|
||||||
validates_presence_of :username
|
validates_presence_of :username
|
||||||
validate :username_validator
|
validate :username_validator
|
||||||
validates :email, presence: true, uniqueness: true
|
validates :email, presence: true, uniqueness: true
|
||||||
|
@ -687,6 +689,10 @@ class User < ActiveRecord::Base
|
||||||
self.username_lower = username.downcase
|
self.username_lower = username.downcase
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def downcase_email
|
||||||
|
self.email = self.email.downcase if self.email
|
||||||
|
end
|
||||||
|
|
||||||
def username_validator
|
def username_validator
|
||||||
username_format_validator || begin
|
username_format_validator || begin
|
||||||
lower = username.downcase
|
lower = username.downcase
|
||||||
|
|
|
@ -0,0 +1,26 @@
|
||||||
|
class UpdateUsersCaseInsensitiveEmails < ActiveRecord::Migration
|
||||||
|
def up
|
||||||
|
execute "DROP INDEX index_users_on_email"
|
||||||
|
|
||||||
|
# Find duplicate emails.
|
||||||
|
results = execute <<SQL
|
||||||
|
SELECT id, email, count
|
||||||
|
FROM (SELECT id, email,
|
||||||
|
row_number() OVER(PARTITION BY lower(email) ORDER BY id asc) AS count
|
||||||
|
FROM users) dups
|
||||||
|
WHERE dups.count > 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
|
|
@ -20,7 +20,7 @@ class Auth::FacebookAuthenticator < Auth::Authenticator
|
||||||
user_info = FacebookUserInfo.find_by(facebook_user_id: facebook_hash[:facebook_user_id])
|
user_info = FacebookUserInfo.find_by(facebook_user_id: facebook_hash[:facebook_user_id])
|
||||||
result.user = user_info.try(:user)
|
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))
|
FacebookUserInfo.create({user_id: result.user.id}.merge(facebook_hash))
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -18,7 +18,7 @@ class Auth::GoogleOAuth2Authenticator < Auth::Authenticator
|
||||||
user_info = GoogleUserInfo.find_by(google_user_id: google_hash[:google_user_id])
|
user_info = GoogleUserInfo.find_by(google_user_id: google_hash[:google_user_id])
|
||||||
result.user = user_info.try(:user)
|
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))
|
GoogleUserInfo.create({user_id: result.user.id}.merge(google_hash))
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -18,7 +18,7 @@ module Email
|
||||||
|
|
||||||
def self.downcase(email)
|
def self.downcase(email)
|
||||||
return email unless Email.is_valid?(email)
|
return email unless Email.is_valid?(email)
|
||||||
email.gsub(/^(.+@{1})([^@]+)$/) { $1 + $2.downcase }
|
email.downcase
|
||||||
end
|
end
|
||||||
|
|
||||||
end
|
end
|
||||||
|
|
|
@ -25,9 +25,9 @@ describe Email do
|
||||||
|
|
||||||
describe "downcase" do
|
describe "downcase" do
|
||||||
|
|
||||||
it 'downcases only the host part' do
|
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'
|
||||||
Email.downcase('sam@GMAIL.COM').should_not == 'SAM@gmail.com'
|
Email.downcase('sam@GMAIL.COM').should == 'sam@gmail.com'
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'leaves invalid emails untouched' do
|
it 'leaves invalid emails untouched' do
|
||||||
|
|
|
@ -203,6 +203,14 @@ describe UsersController do
|
||||||
end
|
end
|
||||||
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
|
context 'success' do
|
||||||
|
|
||||||
it 'has an email token' do
|
it 'has an email token' do
|
||||||
|
|
|
@ -423,4 +423,16 @@ describe Category do
|
||||||
|
|
||||||
end
|
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
|
end
|
||||||
|
|
|
@ -16,6 +16,11 @@ describe EmailToken do
|
||||||
email_token.should be_present
|
email_token.should be_present
|
||||||
end
|
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
|
it 'is valid' do
|
||||||
email_token.should be_valid
|
email_token.should be_valid
|
||||||
end
|
end
|
||||||
|
|
|
@ -79,7 +79,7 @@ describe Invite do
|
||||||
it 'returns the original 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 == @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
|
end
|
||||||
|
|
||||||
it 'returns a new invite if the other has expired' do
|
it 'returns a new invite if the other has expired' do
|
||||||
|
|
|
@ -15,6 +15,11 @@ describe ScreenedEmail do
|
||||||
# have ever been blocked by looking at last_match_at.
|
# have ever been blocked by looking at last_match_at.
|
||||||
ScreenedEmail.create(email: email).last_match_at.should be_nil
|
ScreenedEmail.create(email: email).last_match_at.should be_nil
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "downcases the email" do
|
||||||
|
s = ScreenedEmail.create(email: 'SPAMZ@EXAMPLE.COM')
|
||||||
|
s.email.should == 'spamz@example.com'
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#block' do
|
describe '#block' do
|
||||||
|
@ -66,6 +71,11 @@ describe ScreenedEmail do
|
||||||
ScreenedEmail.should_block?(email).should be_true
|
ScreenedEmail.should_block?(email).should be_true
|
||||||
end
|
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
|
shared_examples "when a ScreenedEmail record matches" do
|
||||||
it "updates statistics" do
|
it "updates statistics" do
|
||||||
Timecop.freeze(Time.zone.now) do
|
Timecop.freeze(Time.zone.now) do
|
||||||
|
|
|
@ -242,6 +242,12 @@ describe User do
|
||||||
|
|
||||||
its(:email_tokens) { should be_present }
|
its(:email_tokens) { should be_present }
|
||||||
end
|
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
|
end
|
||||||
|
|
||||||
describe 'ip address validation' do
|
describe 'ip address validation' do
|
||||||
|
@ -755,7 +761,7 @@ describe User do
|
||||||
expect(found_user).to eq bob
|
expect(found_user).to eq bob
|
||||||
|
|
||||||
found_user = User.find_by_username_or_email('Bob@Example.com')
|
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')
|
found_user = User.find_by_username_or_email('bob1')
|
||||||
expect(found_user).to be_nil
|
expect(found_user).to be_nil
|
||||||
|
@ -763,6 +769,9 @@ describe User do
|
||||||
found_user = User.find_by_email('bob@Example.com')
|
found_user = User.find_by_email('bob@Example.com')
|
||||||
expect(found_user).to eq bob
|
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')
|
found_user = User.find_by_email('bob')
|
||||||
expect(found_user).to be_nil
|
expect(found_user).to be_nil
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue