Merge pull request #710 from ZogStriP/email-is-case-sensitive

better consistency around email case sensitivity

app/assets/javascripts/discourse/views/modal/forgot_password_view.js

@@ -18,7 +18,7 @@ Discourse.ForgotPasswordView = Discourse.ModalBodyView.extend({
   submit: function() {
 
     Discourse.ajax(Discourse.getURL("/session/forgot_password"), {
-      data: { username: this.get('accountEmailOrUsername') },
+      data: { login: this.get('accountEmailOrUsername') },
       type: 'POST'
     });
 

app/controllers/session_controller.rb

@@ -3,13 +3,13 @@ class SessionController < ApplicationController
   def create
     requires_parameter(:login, :password)
 
-    login = params[:login].downcase
+    login = params[:login]
     login = login[1..-1] if login[0] == "@"
 
     if login =~ /@/
-      @user = User.where(email: login).first
+      @user = User.where(email: Email.downcase(login)).first
     else
-      @user = User.where(username_lower: login).first
+      @user = User.where(username_lower: login.downcase).first
     end
 
     if @user.present?
@@ -37,9 +37,9 @@ class SessionController < ApplicationController
   end
 
   def forgot_password
-    requires_parameter(:username)
+    requires_parameter(:login)
 
-    user = User.where('username_lower = :username or email = :username', username: params[:username].downcase).first
+    user = User.where('username_lower = :username or email = :email', username: params[:login].downcase, email: Email.downcase(params[:login])).first
     if user.present?
       email_token = user.email_tokens.create(email: user.email)
       Jobs.enqueue(:user_email, type: :forgot_password, user_id: user.id, email_token: email_token.token)

app/controllers/users_controller.rb

@@ -265,16 +265,17 @@ class UsersController < ApplicationController
     requires_parameter(:email)
     user = fetch_user_from_params
     guardian.ensure_can_edit!(user)
+    lower_email = Email.downcase(params[:email])
 
     # Raise an error if the email is already in use
-    if User.where("lower(email) = ?", params[:email].downcase).exists?
+    if User.where("email = ?", lower_email).exists?
       raise Discourse::InvalidParameters.new(:email)
     end
 
-    email_token = user.email_tokens.create(email: params[:email])
+    email_token = user.email_tokens.create(email: lower_email)
     Jobs.enqueue(
       :user_email,
-      to_address: params[:email],
+      to_address: lower_email,
       type: :authorize_email,
       user_id: user.id,
       email_token: email_token.token

app/models/invite.rb

@@ -11,13 +11,12 @@ class Invite < ActiveRecord::Base
 
   acts_as_paranoid
 
-
   before_create do
     self.invite_key ||= SecureRandom.hex
   end
 
   before_save do
-    self.email = email.downcase
+    self.email = Email.downcase(email)
   end
 
   validate :user_doesnt_already_exist
@@ -26,7 +25,7 @@ class Invite < ActiveRecord::Base
   def user_doesnt_already_exist
     @email_already_exists = false
     return if email.blank?
-    if User.where("lower(email) = ?", email.downcase).exists?
+    if User.where("email = ?", Email.downcase(email)).exists?
       @email_already_exists = true
       errors.add(:email)
     end

app/models/topic.rb

@@ -398,11 +398,9 @@ class Topic < ActiveRecord::Base
   # Invite a user by email and return the invite. Return the previously existing invite
   # if already exists. Returns nil if the invite can't be created.
   def invite_by_email(invited_by, email)
-    lower_email = email.downcase
-
+    lower_email = Email.downcase(email)
     invite = Invite.with_deleted.where('invited_by_id = ? and email = ?', invited_by.id, lower_email).first
 
-
     if invite.blank?
       invite = Invite.create(invited_by: invited_by, email: lower_email)
       unless invite.valid?

app/models/user.rb

@@ -193,7 +193,9 @@ class User < ActiveRecord::Base
   end
 
   def self.find_by_username_or_email(username_or_email)
-    where("username_lower = :user or lower(username) = :user or lower(email) = :user or lower(name) = :user", user: username_or_email.downcase)
+    lower_user = username_or_email.downcase
+    lower_email = Email.downcase(username_or_email)
+    where("username_lower = :user or lower(username) = :user or email = :email or lower(name) = :user", user: lower_user, email: lower_email)
   end
 
   # tricky, we need our bus to be subscribed from the right spot

lib/email.rb

@@ -1,8 +1,8 @@
 require 'mail'
+
 module Email
 
   def self.is_valid?(email)
-
     parser = Mail::RFC2822Parser.new
     parser.root = :addr_spec
     result = parser.parse(email)
@@ -12,4 +12,9 @@ module Email
     result && result.respond_to?(:domain) && result.domain.dot_atom_text.elements.size > 1
   end
 
+  def self.downcase(email)
+    return email unless Email.is_valid?(email)
+    email.gsub(/^(.+@{1})([^@]+)$/) { $1 + $2.downcase }
+  end
+
 end

spec/components/email_spec.rb

@@ -3,20 +3,39 @@ require 'email'
 
 describe Email do
 
+  describe "is_valid?" do
+
+    it 'treats a good email as valid' do
+      Email.is_valid?('sam@sam.com').should be_true
+    end
+
+    it 'treats a bad email as invalid' do
+      Email.is_valid?('sam@sam').should be_false
+    end
+
+    it 'allows museum tld' do
+      Email.is_valid?('sam@nic.museum').should be_true
+    end
+
+    it 'does not think a word is an email' do
+      Email.is_valid?('sam').should be_false
+    end
 
-  it 'should treat a good email as valid' do
-    Email.is_valid?('sam@sam.com').should be_true
   end
 
-  it 'should treat a bad email as invalid' do
-    Email.is_valid?('sam@sam').should be_false
+  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'
+    end
+
+    it 'leaves invalid emails untouched' do
+      Email.downcase('SAM@GMAILCOM').should == 'SAM@GMAILCOM'
+      Email.downcase('samGMAIL.COM').should == 'samGMAIL.COM'
+      Email.downcase('sam@GM@AIL.COM').should == 'sam@GM@AIL.COM'
+    end
+
   end
 
-  it 'should allow museum tld' do
-    Email.is_valid?('sam@nic.museum').should be_true
-  end
-
-  it 'should not think a word is an email' do
-    Email.is_valid?('sam').should be_false
-  end
 end

spec/controllers/session_controller_spec.rb

@@ -119,12 +119,12 @@ describe SessionController do
 
     context 'for a non existant username' do
       it "doesn't generate a new token for a made up username" do
-        lambda { xhr :post, :forgot_password, username: 'made_up'}.should_not change(EmailToken, :count)
+        lambda { xhr :post, :forgot_password, login: 'made_up'}.should_not change(EmailToken, :count)
       end
 
       it "doesn't enqueue an email" do
         Jobs.expects(:enqueue).with(:user_mail, anything).never
-        xhr :post, :forgot_password, username: 'made_up'
+        xhr :post, :forgot_password, login: 'made_up'
       end
     end
 
@@ -132,12 +132,12 @@ describe SessionController do
       let(:user) { Fabricate(:user) }
 
       it "generates a new token for a made up username" do
-        lambda { xhr :post, :forgot_password, username: user.username}.should change(EmailToken, :count)
+        lambda { xhr :post, :forgot_password, login: user.username}.should change(EmailToken, :count)
       end
 
       it "enqueues an email" do
         Jobs.expects(:enqueue).with(:user_email, has_entries(type: :forgot_password, user_id: user.id))
-        xhr :post, :forgot_password, username: user.username
+        xhr :post, :forgot_password, login: user.username
       end
     end
 

spec/models/invite_spec.rb

@@ -36,7 +36,6 @@ describe Invite do
       end
     end
 
-
     context 'to a topic' do
       let!(:topic) { Fabricate(:topic) }
       let(:inviter) { topic.user }
@@ -97,8 +96,12 @@ describe Invite do
             topic.invite_by_email(inviter, 'iceking@adventuretime.ooo').should == @invite
           end
 
-          it 'matches case insensitively' do
-            topic.invite_by_email(inviter, 'ICEKING@adventuretime.ooo').should == @invite
+          it 'matches case insensitively for the domain part' do
+            topic.invite_by_email(inviter, 'iceking@ADVENTURETIME.ooo').should == @invite
+          end
+
+          it 'matches case sensitively for the local part' do
+            topic.invite_by_email(inviter, 'ICEKING@adventuretime.ooo').should_not == @invite
           end
         end