Email verification (#4268)

* Add confirmable db attrs

* Add confirmable to users, adjust callbacks

* Confirmable tests

* confirmed_at backfill task

* Redirect successful confirms to prod home page

* Allow unconfirmed access forever

* Check that correct emails are sent correctly

* On the Hound's Time

* Third party accounds still need to be confirmed

app/controllers/confirmations_controller.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class ConfirmationsController < Devise::ConfirmationsController
+  protected
+
+  def after_confirmation_path_for(resource_name, resource)
+    'https://www.zooniverse.org'
+  end
+end

app/models/user.rb

@@ -13,7 +13,7 @@ class User < ApplicationRecord
   attr_accessor :minor_age
 
   devise :database_authenticatable, :registerable,
-    :recoverable, :rememberable, :trackable, :validatable,
+    :recoverable, :rememberable, :trackable, :validatable, :confirmable,
     :omniauthable, omniauth_providers: [:facebook, :google_oauth2]
 
   has_many :classifications, dependent: :restrict_with_exception
@@ -56,15 +56,13 @@ class User < ApplicationRecord
   validates_with IdentityGroupNameValidator
 
   after_create :set_zooniverse_id
-  after_create :send_welcome_email, unless: :migrated
   before_create :set_ouroboros_api_key
 
   delegate :projects, to: :identity_group
   delegate :collections, to: :identity_group
   delegate :subjects, to: :identity_group
   delegate :owns?, to: :identity_group
 
-
   pg_search_scope :search_name,
     against: [:login],
     using: {
@@ -204,6 +202,10 @@ def self.find_by_lower_login(login)
     find_by("lower(login) = ?", login.downcase)
   end
 
+  def after_confirmation
+    send_welcome_email
+  end
+
   def subject_limit
     super || Panoptes.max_subjects
   end

config/initializers/devise.rb

@@ -8,7 +8,8 @@
   config.strip_whitespace_keys = [ :email ]
   config.skip_session_storage = [:http_auth]
   config.stretches = Rails.env.test? ? 1 : 10
-  config.reconfirmable = true
+  config.reconfirmable = false
+  config.allow_unconfirmed_access_for = nil
   config.password_length = 8..128
   config.reset_password_within = 6.hours
   config.paranoid = true

config/routes.rb

@@ -21,7 +21,7 @@
   end
 
   devise_for :users,
-    controllers: { omniauth_callbacks: 'omniauth_callbacks', passwords: 'passwords' },
+    controllers: { confirmations: 'confirmations', omniauth_callbacks: 'omniauth_callbacks', passwords: 'passwords' },
     skip: [ :sessions, :registrations ]
 
   as :user do

db/migrate/20231025200957_add_confirmable_to_devise.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+class AddConfirmableToDevise < ActiveRecord::Migration[6.1]
+  disable_ddl_transaction!
+
+  def change
+    add_column :users, :confirmation_token, :string
+    add_column :users, :confirmed_at, :datetime
+    add_column :users, :confirmation_sent_at, :datetime
+
+    add_index :users, :confirmation_token, unique: true, algorithm: :concurrently
+  end
+
+  def down
+    remove_column :users, :confirmation_token
+    remove_column :users, :confirmed_at
+    remove_column :users, :confirmation_sent_at
+
+    remove_index :users, :confirmation_token
+  end
+end

db/structure.sql

@@ -53,8 +53,6 @@ COMMENT ON EXTENSION pg_trgm IS 'text similarity measurement and index searching
 
 SET default_tablespace = '';
 
-SET default_with_oids = false;
-
 --
 -- Name: access_control_lists; Type: TABLE; Schema: public; Owner: -
 --
@@ -1802,7 +1800,10 @@ CREATE TABLE public.users (
     upload_whitelist boolean DEFAULT false NOT NULL,
     ux_testing_email_communication boolean DEFAULT false,
     intervention_notifications boolean DEFAULT true,
-    nasa_email_communication boolean DEFAULT false
+    nasa_email_communication boolean DEFAULT false,
+    confirmation_token character varying,
+    confirmed_at timestamp without time zone,
+    confirmation_sent_at timestamp without time zone
 );
 
 
@@ -3568,6 +3569,13 @@ CREATE INDEX index_users_on_activated_state ON public.users USING btree (activat
 CREATE INDEX index_users_on_beta_email_communication ON public.users USING btree (beta_email_communication) WHERE (beta_email_communication = true);
 
 
+--
+-- Name: index_users_on_confirmation_token; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE UNIQUE INDEX index_users_on_confirmation_token ON public.users USING btree (confirmation_token);
+
+
 --
 -- Name: index_users_on_display_name; Type: INDEX; Schema: public; Owner: -
 --
@@ -4576,6 +4584,7 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20211124175756'),
 ('20211201164326'),
 ('20221018032140'),
-('20230613165746');
+('20230613165746'),
+('20231025200957');
 
 

lib/tasks/user.rake

@@ -48,6 +48,18 @@ namespace :user do
     end
   end
 
+  desc 'Backfill confirmed_at in batches (restartable)'
+  task backfill_confirmed_at: :environment do
+    mass_confirmed_at = Time.current.to_s(:db)
+    User.select(:id).find_in_batches do |users|
+      null_confirmed_at_user_scope = User.where(
+        id: users.map(&:id),
+        confirmed_at: nil
+      )
+      null_confirmed_at_user_scope.update_all(confirmed_at: mass_confirmed_at)
+    end
+  end
+
   namespace :limit do
 
     class UpdateUserLimitArgsError < StandardError; end

spec/controllers/passwords_controller_spec.rb

@@ -72,8 +72,9 @@
           expect(response.status).to eq(200)
         end
 
-        it "should not send an email to the account email address" do
-          expect(ActionMailer::Base.deliveries).to be_empty
+        it 'should send confirmation instructions to the account email address' do
+          expect(ActionMailer::Base.deliveries).not_to be_empty
+          expect(ActionMailer::Base.deliveries.first.body).to include('confirm your account')
         end
       end
 

spec/controllers/registrations_controller_spec.rb

@@ -149,8 +149,8 @@
           post :create, params: { user: user_attributes }
         end
 
-        it "should queue a welcome worker to send an email" do
-          expect(UserWelcomeMailerWorker).to receive(:perform_async)
+        it "should send a confirmation instructions email" do
+          expect_any_instance_of(User).to receive(:send_confirmation_instructions).once
           post :create, params: { user: user_attributes }
         end
 
@@ -212,8 +212,8 @@
           end
         end
 
-        it "should not queue a welcome worker to send an email" do
-          expect(UserWelcomeMailerWorker).to_not receive(:perform_async)
+        it "should not send a confirmation instructions email" do
+          expect_any_instance_of(User).to_not receive(:send_confirmation_instructions)
           post :create, params: { user: user_attributes }
         end
       end
@@ -279,8 +279,8 @@
           post :create, params: { user: user_attributes }
         end
 
-        it "should queue a welcome worker to send an email" do
-          expect(UserWelcomeMailerWorker).to receive(:perform_async)
+        it "should send a confirmation instructions email" do
+          expect_any_instance_of(User).to receive(:send_confirmation_instructions).once
           post :create, params: { user: user_attributes }
         end
       end
@@ -313,8 +313,8 @@
           expect(User.where(login: user_attributes[:login])).to_not exist
         end
 
-        it "should not queue a welcome worker to send an email" do
-          expect(UserWelcomeMailerWorker).to_not receive(:perform_async)
+        it "should not send a confirmation instructions email" do
+          expect_any_instance_of(User).to_not receive(:send_confirmation_instructions)
           post :create, params: { user: user_attributes }
         end
       end

spec/factories/users.rb

@@ -8,6 +8,8 @@
     sequence(:email) {|n| "example#{n}@example.com"}
     password { 'password' }
     encrypted_password { User.new.send(:password_digest, 'password') }
+    confirmed_at { Time.current }
+    confirmation_sent_at { Time.current }
     credited_name { 'Dr User' }
     activated_state { :active }
     sequence(:login) { |n| "new_user_#{n}" }

spec/models/user_spec.rb

@@ -213,6 +213,17 @@ def dormant_user_ids(num_days_since_activity=5)
     end
   end
 
+  describe 'confirmation' do
+    context 'when a new user is saved' do
+      let(:user) { build(:user, confirmed_at: nil, confirmation_sent_at: nil) }
+
+      it 'sends a confirmation email' do
+        expect_any_instance_of(User).to receive(:send_confirmation_instructions).once
+        user.save!
+      end
+    end
+  end
+
   describe '::send_reset_password_instructions' do
     context 'when the user exists' do
       let(:user) { create(:user) }
@@ -872,42 +883,42 @@ def dormant_user_ids(num_days_since_activity=5)
     end
   end
 
-  describe "#send_welcome_email after_create callback" do
-    let(:user) { build(:user) }
+  describe "#send_welcome_email on confirm" do
+    let(:user) { create(:user, confirmed_at: nil) }
 
     it "should send the welcome email" do
       expect(user).to receive(:send_welcome_email).once
-      user.save!
+      user.confirm
     end
 
     it "should queue the worker with the user id" do
       allow(UserWelcomeMailerWorker).to receive :perform_async
-      user.save!
+      user.confirm
       expect(UserWelcomeMailerWorker).to have_received(:perform_async).with(user.id, nil).ordered
     end
 
     context "when redis is unreachable" do
       it "should not raise an error on save" do
         [ Timeout::Error, Redis::TimeoutError, Redis::CannotConnectError ].each do |redis_error|
           allow(UserWelcomeMailerWorker).to receive(:perform_async).and_raise(redis_error)
-          expect { user.save! }.not_to raise_error
+          expect { user.confirm }.not_to raise_error
         end
       end
 
       it "should send the welcome email in band" do
         allow(UserWelcomeMailerWorker).to receive(:perform_async).and_raise(Redis::CannotConnectError)
         expect_any_instance_of(UserWelcomeMailerWorker).to receive(:perform).with(Integer, nil)
-        user.save!
+        user.confirm
       end
     end
 
     context "when the user has a project id" do
       let(:project) { create :project }
-      let!(:user) { build(:user, project_id: project.id) }
+      let!(:user) { create(:user, project_id: project.id, confirmed_at: nil) }
 
       it "should queue the worker with the user id and project id" do
         allow(UserWelcomeMailerWorker).to receive :perform_async
-        user.save!
+        user.confirm
         expect(UserWelcomeMailerWorker).to have_received(:perform_async).with(user.id, project.id).ordered
       end
     end
@@ -917,7 +928,7 @@ def dormant_user_ids(num_days_since_activity=5)
 
       it "should not queue the worker with the user id" do
         expect(UserWelcomeMailerWorker).not_to receive(:perform_async)
-        user.save!
+        user.confirm
       end
     end
   end