Skip to content

Commit

Permalink
Store email using exact casing as provided by user.
Browse files Browse the repository at this point in the history
* Fixes #1763
* Index on lower(email)
* Enforce uniqueness on lower(email)
* Use Case insensitive email to login
  • Loading branch information
anuragaryan authored and simi committed Nov 11, 2023
1 parent 0b40270 commit b12e92c
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 5 deletions.
13 changes: 9 additions & 4 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ class User < ApplicationRecord
has_many :oidc_id_tokens, through: :oidc_api_key_roles, class_name: "OIDC::IdToken", inverse_of: :user, source: :id_tokens
has_many :oidc_providers, through: :oidc_api_key_roles, class_name: "OIDC::Provider", inverse_of: :users, source: :providers

validates :email, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }, format: { with: URI::MailTo::EMAIL_REGEXP }, presence: true
validates :email, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }, format: { with: URI::MailTo::EMAIL_REGEXP }, presence: true,
uniqueness: { case_sensitive: false }
validates :unconfirmed_email, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }, format: { with: URI::MailTo::EMAIL_REGEXP }, allow_blank: true

validates :handle, uniqueness: { case_sensitive: false }, allow_nil: true, if: :handle_changed?
Expand Down Expand Up @@ -72,12 +73,16 @@ class User < ApplicationRecord
validate :toxic_email_domain, on: :create

def self.authenticate(who, password)
user = find_by(email: who.downcase) || find_by(handle: who)
user = find_by_email(who) || find_by(handle: who)
user if user&.authenticated?(password)
rescue BCrypt::Errors::InvalidHash
nil
end

def self.find_by_email(email)
find_by("lower(email) = ?", email.downcase)
end

def self.find_by_slug!(slug)
raise ActiveRecord::RecordNotFound if slug.blank?
find_by(id: slug) || find_by!(handle: slug)
Expand All @@ -90,7 +95,7 @@ def self.find_by_slug(slug)

def self.find_by_name(name)
return if name.blank?
find_by(email: name) || find_by(handle: name)
find_by_email(name) || find_by(handle: name)
end

def self.find_by_blocked(slug)
Expand All @@ -111,7 +116,7 @@ def self.ownership_request_notifiable_owners
end

def self.normalize_email(email)
super
email.to_s.gsub(/\s+/, "")
rescue ArgumentError => e
Rails.error.report(e, handled: true)
""
Expand Down
9 changes: 9 additions & 0 deletions db/migrate/20181128191130_add_index_to_lowercase_email.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class AddIndexToLowercaseEmail < ActiveRecord::Migration[5.2]
def up
add_index "users", "lower(email) varchar_pattern_ops", name: "index_users_on_lower_email"
end

def down
remove_index "users", name: "index_users_on_lower_email"
end
end
1 change: 1 addition & 0 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@
t.string "totp_seed"
t.string "mfa_hashed_recovery_codes", default: [], array: true
t.boolean "public_email", default: false, null: false
t.index "lower((email)::text) varchar_pattern_ops", name: "index_users_on_lower_email"
t.index ["email"], name: "index_users_on_email"
t.index ["handle"], name: "index_users_on_handle"
t.index ["id", "confirmation_token"], name: "index_users_on_id_and_confirmation_token"
Expand Down
13 changes: 13 additions & 0 deletions test/integration/sign_up_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,19 @@ class SignUpTest < SystemTest
assert page.has_selector? "#flash_notice", text: "A confirmation mail has been sent to your email address."
end

test "sign up stores original email casing" do
visit sign_up_path

fill_in "Email", with: "Email@person.com"
fill_in "Username", with: "nick"
fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD
click_button "Sign up"

assert page.has_selector? "#flash_notice", text: "A confirmation mail has been sent to your email address."

assert_equal "Email@person.com", User.last.email
end

test "sign up with no handle" do
visit sign_up_path

Expand Down
2 changes: 1 addition & 1 deletion test/models/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -852,7 +852,7 @@ class UserTest < ActiveSupport::TestCase

context ".normalize_email" do
should "return the normalized email" do
assert_equal "user@example.com", User.normalize_email(:"UsEr@ example . COM")
assert_equal "UsEr@example.COM", User.normalize_email(:"UsEr@ example . COM")
end

should "return an empty string on invalid inputs" do
Expand Down

0 comments on commit b12e92c

Please sign in to comment.