bugfix: canonicalize emails for users

This allows us to reliably search out user emails for login, and should
reduce some of the oddities of logins.

Fixes #133

Gemfile

@@ -25,6 +25,7 @@ gem "aws-sdk-s3"           # hugo packet is big, let s3 handle the downloads
 gem "bootsnap"             # boot large ruby/rails apps faster
 gem "bundler-audit"        # checks for insecure gems
 gem "devise"               # authentication solution for Rails with Warden
+gem "email_address"        # canonicalize email addresses
 gem "gem-licenses"         # print libraries depended on by this project, grouped by licence
 gem "httparty"             # high level abstraction for rest integrations
 gem "jbuilder"             # Build JSON APIs with ease. Read more: https://github.com/rails/jbuilder

Gemfile.lock

@@ -135,6 +135,8 @@ GEM
     diff-lcs (1.4.4)
     docile (1.4.0)
     e2mmap (0.1.0)
+    email_address (0.2.2)
+      simpleidn
     erubi (1.10.0)
     et-orbi (1.2.6)
       tzinfo
@@ -374,6 +376,8 @@ GEM
       simplecov_json_formatter (~> 0.1)
     simplecov-html (0.12.3)
     simplecov_json_formatter (0.1.3)
+    simpleidn (0.2.1)
+      unf (~> 0.1.4)
     solargraph (0.44.1)
       backport (~> 1.2)
       benchmark
@@ -418,6 +422,9 @@ GEM
     tiny_tds (2.1.5)
     tzinfo (2.0.4)
       concurrent-ruby (~> 1.0)
+    unf (0.1.4)
+      unf_ext
+    unf_ext (0.0.8)
     unicode-display_width (2.1.0)
     warden (1.2.9)
       rack (>= 2.0.9)
@@ -454,6 +461,7 @@ DEPENDENCIES
   database_cleaner-active_record
   database_plumber!
   devise
+  email_address
   factory_bot_rails
   faker
   gem-licenses

app/models/user.rb

@@ -33,11 +33,17 @@ class User < ApplicationRecord
   has_many :reservations, through: :active_claims
   has_many :carts
 
-  # See Cart's validations, one active, pending cart and one active, processing cart at a time. 
-  has_one  :active_pending_cart, -> () { active_pending }, class_name: "Cart"
-  has_one  :active_processing_cart, -> () { active_processing }, class_name: "Cart"
+  # See Cart's validations, one active, pending cart and one active, processing cart at a time.
+  has_one  :active_pending_cart, -> { active_pending }, class_name: "Cart"
+  has_one  :active_processing_cart, -> { active_processing }, class_name: "Cart"
 
+  attribute :email, :canonical_email_address
+  attribute :user_provided_email, :string
 
+  def email=(email_address)
+    self[:user_provided_email] = email_address
+    self[:email] = email_address
+  end
   validates :email, presence: true, uniqueness: true
   validates :hugo_download_counter, presence: true
 
@@ -55,12 +61,8 @@ def in_stripe?
   def email_address_format_valid
     return if email.nil? # covered by presence: true
 
-    if !email.match(Devise.email_regexp)
-      errors.add(:email, "is an unsupported format")
-    end
+    errors.add(:email, "is an unsupported format") unless email.match(Devise.email_regexp)
 
-    if email.include?("/")
-      errors.add(:email, "slashes are unsupported")
-    end
+    errors.add(:email, "slashes are unsupported") if email.include?("/")
   end
 end

changelog.d/133.bugfix

@@ -0,0 +1 @@
+Canonicalize user emails for login

config/initializers/email_address.rb

@@ -0,0 +1,3 @@
+ActiveRecord::Type.register(:email_address, EmailAddress::EmailAddressType)
+ActiveRecord::Type.register(:canonical_email_address,
+                            EmailAddress::CanonicalEmailAddressType)

db/migrate/20220130232402_canonicalize_email.rb

@@ -0,0 +1,17 @@
+class CanonicalizeEmail < ActiveRecord::Migration[6.1]
+  def up
+    add_column :users, :user_provided_email, :string
+    execute "UPDATE users SET user_provided_email = email"
+
+    User.all.each do |user|
+      curr_email = user.user_provided_email
+      user.email = curr_email
+      user.save
+    end
+  end
+
+  def down
+    execute "UPDATE users SET email = user_provided_email"
+    remove_column :users, :user_provided_email
+  end
+end

db/structure.sql

@@ -791,7 +791,8 @@ CREATE TABLE public.users (
     current_sign_in_ip inet,
     last_sign_in_ip inet,
     stripe_id character varying,
-    hugo_download_counter integer DEFAULT 0 NOT NULL
+    hugo_download_counter integer DEFAULT 0 NOT NULL,
+    user_provided_email character varying
 );
 
 
@@ -1547,6 +1548,7 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20210916230829'),
 ('20211110022812'),
 ('20211120232041'),
-('20220116010218');
+('20220116010218'),
+('20220130232402');
 
 

spec/controllers/transfers_controller_spec.rb

@@ -32,7 +32,7 @@
   let(:show_update_params) do
     {
       id: new_user.email,
-      reservation_id: reservation.id,
+      reservation_id: reservation.id
     }
   end
 
@@ -73,7 +73,7 @@
   describe "#create" do
     subject(:post_create) do
       post :create, params: { email: new_user.email,
-      reservation_id: reservation.id }
+                              reservation_id: reservation.id }
     end
 
     it "bounces you if you're not logged in" do
@@ -114,6 +114,23 @@
           .to(new_user)
       end
 
+      context "the user email case is different" do
+        let(:show_update_params) do
+          {
+            id: new_user.email.upcase,
+            reservation_id: reservation.id
+          }
+        end
+        it "transfers between users" do
+          puts "After setting up the user, before setting up the transfer to #{new_user.email} using #{show_update_params[:id]}"
+
+          expect { update_reservation_transfer }
+            .to change { reservation.reload.user }
+            .from(old_user)
+            .to(new_user)
+        end
+      end
+
       it "doesn't copy contact" do
         expect { update_reservation_transfer }.to_not change { old_user.reload.claims.last.conzealand_contact }
         expect(new_user.reload.claims.last.conzealand_contact).to be_nil
@@ -125,7 +142,7 @@
             id: new_user.email,
             reservation_id: reservation.id,
             plan_transfer: {
-              copy_contact: "1",
+              copy_contact: "1"
             }
           }
         end
@@ -142,7 +159,7 @@
         expect(MembershipMailer).to_not receive(:transfer)
         patch :update, params: {
           id: "invalid email",
-          reservation_id: reservation.id,
+          reservation_id: reservation.id
         }
       end
 

spec/models/user_spec.rb

@@ -25,6 +25,14 @@
     expect(build(:user, email: "harry/potter@hogwarts.net")).to_not be_valid
   end
 
+  it "should canonicalize the provided email" do
+    expect(build(:user, email: "Manual@Mail.com").email).to eq("manual@mail.com")
+  end
+
+  it "should retain the user provided email" do
+    expect(build(:user, email: "Manual@Mail.com").user_provided_email).to eq("Manual@Mail.com")
+  end
+
   # I felt the need to do this because factory code gets quite hairy, especially with factories calling factories from
   # the :with_reservation trait.
   describe "user factory links" do