From d467e9be96a694b3f55f73b6694aff68cd4d0177 Mon Sep 17 00:00:00 2001
From: Kyle Drake <kyle@kyledrake.net>
Date: Fri, 8 Dec 2017 22:13:27 -0800
Subject: [PATCH] Force case insensitivity for new emails, existing.

There is a legacy bug I just caught, where many accounts would have the
same email but then have different casing. In extreme scenarios, this
would lead to them creating a new user with the same email, or having
issues with password reset and username lookup.

This doesn't merge in the existing duplicates, but makes sure to only
allow insensitive lowercase emails from here on out. It also will check
for emails in a case insensitive way for such things as resets and
logins if the sensitive lookup doesn't work.

The implementation was not wrong per se - email is supposed to be case sensitive
for usernames. But of course, nobody (nor do most/all email servers) treat
them that way, leading to confusion situations where the user sometimes
camelcases their email and then switches to lowercase later.
---
 app/index.rb                     |  2 +-
 app/password_reset.rb            |  2 +-
 models/site.rb                   | 19 ++++++++++++++-----
 tests/acceptance/signin_tests.rb | 22 ++++++++++++++++++++++
 tests/acceptance/signup_tests.rb | 14 ++++++++++++++
 5 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/app/index.rb b/app/index.rb
index 6bd5d27e..d5483bae 100644
--- a/app/index.rb
+++ b/app/index.rb
@@ -139,7 +139,7 @@ post '/forgot_username' do
     redirect '/forgot_username'
   end
 
-  sites = Site.where(email: params[:email]).all
+  sites = Site.get_recovery_sites_with_email params[:email]
 
   sites.each do |site|
     body = <<-EOT
diff --git a/app/password_reset.rb b/app/password_reset.rb
index 81383a9e..04033465 100644
--- a/app/password_reset.rb
+++ b/app/password_reset.rb
@@ -9,7 +9,7 @@ post '/send_password_reset' do
     redirect '/password_reset'
   end
 
-  sites = Site.filter(email: params[:email]).all
+  sites = Site.get_recovery_sites_with_email params[:email]
 
   if sites.length > 0
     token = SecureRandom.uuid.gsub('-', '')
diff --git a/models/site.rb b/models/site.rb
index c66fb90e..df83e7cd 100644
--- a/models/site.rb
+++ b/models/site.rb
@@ -295,14 +295,23 @@ class Site < Sequel::Model
 
     def get_with_identifier(username_or_email)
       if username_or_email =~ /@/
-        site = self.where(email: username_or_email).where(parent_site_id: nil).first
+        site = get_with_email username_or_email
       else
-        site = self[username: username_or_email]
+        site = self[username: username_or_email.downcase]
       end
       return nil if site.nil? || site.is_banned || site.owner.is_banned
       site
     end
 
+    def get_recovery_sites_with_email(email)
+      self.where('lower(email) = ?', email.downcase).all
+    end
+
+    def get_with_email(email)
+      query = self.where(parent_site_id: nil)
+      query.where(email: email).first || query.where('lower(email) = ?', email.downcase).first
+    end
+
     def ip_create_limit?(ip)
       Site.where('created_at > ?', Date.today.to_time).where(ip: ip).count > IP_CREATE_LIMIT ||
       Site.where(ip: ip).count > TOTAL_IP_CREATE_LIMIT
@@ -844,7 +853,7 @@ class Site < Sequel::Model
 
   def email=(email)
     @original_email = values[:email] unless new?
-    super
+    super(email.nil? ? nil : email.downcase)
   end
 
   def can_email?(col=nil)
@@ -972,12 +981,12 @@ class Site < Sequel::Model
 
     # Check for existing email if new or changing email.
     if new? || @original_email
-      email_check = self.class.select(:id).filter(email: values[:email])
+      email_check = self.class.select(:id).filter('lower(email)=?', values[:email])
       email_check = email_check.exclude(id: self.id) unless new?
       email_check = email_check.first
 
       if parent? && email_check && email_check.id != self.id
-        errors.add :email, 'This email address already exists on Neocities, please use your existing account instead of creating a new one.'
+        errors.add :email, 'This email address already exists on Neocities.'
       end
     end
 
diff --git a/tests/acceptance/signin_tests.rb b/tests/acceptance/signin_tests.rb
index abe40ed3..c3676dd7 100644
--- a/tests/acceptance/signin_tests.rb
+++ b/tests/acceptance/signin_tests.rb
@@ -60,6 +60,17 @@ describe 'signin' do
     page.must_have_content 'Your Feed'
   end
 
+  it 'signs in with invalid case username' do
+    pass = SecureRandom.hex
+    @site = Fabricate :site, password: pass
+    visit '/'
+    click_link 'Sign In'
+    fill_in 'username', with: @site.username.upcase
+    fill_in 'password', with: pass
+    click_button 'Sign In'
+    page.must_have_content 'Your Feed'
+  end
+
   it 'signs in with email' do
     pass = SecureRandom.hex
     @site = Fabricate :site, password: pass
@@ -70,4 +81,15 @@ describe 'signin' do
     click_button 'Sign In'
     page.must_have_content 'Your Feed'
   end
+
+  it 'signs in with invalid case email' do
+    pass = SecureRandom.hex
+    @site = Fabricate :site, password: pass
+    visit '/'
+    click_link 'Sign In'
+    fill_in 'username', with: @site.email.upcase
+    fill_in 'password', with: pass
+    click_button 'Sign In'
+    page.must_have_content 'Your Feed'
+  end
 end
diff --git a/tests/acceptance/signup_tests.rb b/tests/acceptance/signup_tests.rb
index d43bf794..a42b93db 100644
--- a/tests/acceptance/signup_tests.rb
+++ b/tests/acceptance/signup_tests.rb
@@ -140,6 +140,20 @@ describe 'signup' do
     page.must_have_content /email.+exists/
   end
 
+  it 'fails with existing email even if case sensitive' do
+    email = Fabricate.attributes_for(:site)[:email]
+    fill_in_valid
+    fill_in 'email', with: email
+    click_signup_button
+    site_created?.must_equal true
+    Capybara.reset_sessions!
+    visit_signup
+    fill_in_valid
+    fill_in 'email', with: email.upcase
+    click_signup_button
+    page.must_have_content /email.+exists/
+  end
+
   it 'succeeds with no tags' do
     fill_in_valid
     fill_in 'new_tags_string', with: ''