Email Confirmation Changes

app/controllers/sessions_controller.rb

@@ -8,11 +8,16 @@ def new
 
   # Capture the user's shibboleth id if they're coming in from an IDP
   # ---------------------------------------------------------------------
-  # rubocop:disable Metrics/AbcSize
+  # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
   def create
     existing_user = User.find_by(email: params[:user][:email])
     unless existing_user.nil?
 
+      unless existing_user.confirmation_instructions_handled?
+        handle_confirmation_instructions(existing_user)
+        return
+      end
+
       # Until ORCID login is supported
       unless session['devise.shibboleth_data'].nil?
         args = {
@@ -36,7 +41,7 @@ def create
       end
     end
   end
-  # rubocop:enable Metrics/AbcSize
+  # rubocop:enable Metrics/AbcSize, Metrics/MethodLength
 
   def destroy
     super
@@ -45,3 +50,11 @@ def destroy
     set_locale
   end
 end
+
+private
+
+def handle_confirmation_instructions(user)
+  user.send_confirmation_instructions
+  flash[:notice] = I18n.t('devise.registrations.signed_up_but_unconfirmed')
+  redirect_to root_path
+end

app/controllers/users/omniauth_callbacks_controller.rb

@@ -5,7 +5,7 @@ module Users
   class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     # This is for the OpenidConnect CILogon
 
-    # rubocop:disable Metrics/AbcSize, Metrics/MethodLength, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
+    # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
     def openid_connect
       # First or create
       auth = request.env['omniauth.auth']
@@ -23,22 +23,8 @@ def openid_connect
       identifier_scheme = IdentifierScheme.find_by(name: auth.provider)
 
       if current_user.nil? # if user is not signed in (They clicked the SSO sign in button)
-        if user.nil? # If an entry does not exist in the identifiers table for the chosen SSO account
-          user = User.create_from_provider_data(auth)
-          if user.nil? # if a user was NOT created (a match was found for User.find_by(email: auth.info.email)
-            # Do not link SSO credentials for the signed out, existing user
-            flash[:alert] = _('That email appears to be associated with an existing account.<br>' \
-                              'Sign into your existing account, and you can link that ' \
-                              "account with SSO from the 'Edit Profile' page.")
-            redirect_to root_path
-            return
-          end
-          # A new user was created, link the SSO credentials (we can do this for a newly created user)
-          user.identifiers << Identifier.create(identifier_scheme: identifier_scheme,
-                                                value: auth.uid,
-                                                attrs: auth,
-                                                identifiable: user)
-        end
+        # user.nil? is true if the chosen CILogon email is not currently linked to an existing user account
+        user = handle_new_sso_email_for_signed_out_user(auth, identifier_scheme) if user.nil?
         sign_in_and_redirect user, event: :authentication
       elsif user.nil?
         # we need to link
@@ -58,7 +44,7 @@ def openid_connect
         redirect_to edit_user_registration_path
       end
     end
-    # rubocop:enable Metrics/AbcSize, Metrics/MethodLength, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
+    # rubocop:enable Metrics/AbcSize, Metrics/MethodLength
 
     def orcid
       handle_omniauth(IdentifierScheme.for_authentication.find_by(name: 'orcid'))
@@ -140,5 +126,22 @@ def handle_omniauth(scheme)
     def failure
       redirect_to root_path
     end
+
+    private
+
+    # This method is executed when a user performs the following two steps:
+    # 1) clicks "Sign in with institutional or social ID"
+    # 2) Within CILogon, selects an email that is not currently linked to an existing user account
+    def handle_new_sso_email_for_signed_out_user(auth, identifier_scheme)
+      # Find or create the user with user.email == email selected via SSO
+      user = User.find_or_create_from_provider_data(auth)
+      if user.confirmed? # Only link the SSO email if user.email is confirmed
+        user.identifiers << Identifier.create(identifier_scheme: identifier_scheme,
+                                              value: auth.uid,
+                                              attrs: auth,
+                                              identifiable: user)
+      end
+      user
+    end
   end
 end

app/models/user.rb

@@ -65,7 +65,7 @@ class User < ApplicationRecord
   #   :token_authenticatable, :confirmable,
   #   :lockable, :timeoutable and :omniauthable
   devise :invitable, :database_authenticatable, :registerable, :recoverable,
-         :rememberable, :trackable, :validatable, :omniauthable,
+         :rememberable, :trackable, :validatable, :omniauthable, :confirmable,
          omniauth_providers: %i[shibboleth orcid openid_connect]
 
   ##
@@ -185,10 +185,10 @@ def self.from_omniauth(auth)
   ##
   # Handle user creation from provider
   # rubocop:disable Metrics/AbcSize
-  def self.create_from_provider_data(provider_data)
+  def self.find_or_create_from_provider_data(provider_data)
     user = User.find_or_initialize_by(email: provider_data.info.email)
 
-    return unless user.new_record?
+    return user unless user.new_record?
 
     user.update!(
       firstname: provider_data.info&.first_name.present? ? provider_data.info.first_name : _('First name'),
@@ -197,7 +197,10 @@ def self.create_from_provider_data(provider_data)
       # We don't know which organization to setup so we will use other
       org: Org.find_by(is_other: true),
       accept_terms: true,
-      password: Devise.friendly_token[0, 20]
+      password: Devise.friendly_token[0, 20],
+      # provider_data.info.email comes from CILogon sign-in, which requires email confirmation
+      # It follows that we can set `confirmed_at: Time.now` and bypass Devise's email confirmation step
+      confirmed_at: Time.now
     )
     user
   end
@@ -407,6 +410,14 @@ def deliver_invitation(options = {})
     )
   end
 
+  # If this method returns false, then one of the following two scenarios must be true:
+  # 1) The user was created while the app wasn't using Devise :confirmable (a confirmation_token was never generated)
+  # 2) An outdated confirmation_token existed, but was set to nil via `rake dmp_assistant_upgrade:v4_2_3`
+  #    (see `def handle_email_confirmations_for_existing_users`` in `lib/tasks/dmp_assistant_upgrade.rake`)
+  def confirmation_instructions_handled?
+    confirmed? || confirmation_token.present?
+  end
+
   # Case insensitive search over User model
   #
   # field - The name of the field being queried

config/environments/test.rb

@@ -45,6 +45,7 @@
   # The :test delivery method accumulates sent emails in the
   # ActionMailer::Base.deliveries array.
   config.action_mailer.delivery_method = :test
+  config.action_mailer.default_options = { from: '[email protected]' }
 
   # Print deprecation notices to the stderr.
   config.active_support.deprecation = :stderr

config/locales/translation.en-CA.yml

@@ -228,11 +228,13 @@ en-CA:
       not_found_in_database: Invalid email or password.
       timeout: Your session expired, please sign in again to continue.
       unauthenticated: You need to sign in or sign up before continuing.
-      unconfirmed: You have to confirm your account before continuing.
+      unconfirmed: |
+        You have to confirm your account before continuing.
+        <a href="/users/confirmation/new" class="a-orange">(Click to request a new confirmation email)</a>
       invited: You have a pending invitation, accept it to finish creating your account.
     mailer:
       confirmation_instructions:
-        subject: Confirm your DMPonline account
+        subject: Confirm your DMP Assistant account
       reset_password_instructions:
         subject: Reset password instructions
       unlock_instructions:

config/locales/translation.fr-CA.yml

@@ -242,7 +242,9 @@ fr-CA:
       not_found_in_database: Courriel ou mot de passe incorrect.
       timeout: Votre session est expirée. Veuillez vous reconnecter pour continuer.
       unauthenticated: Vous devez vous connecter ou vous inscrire pour continuer.
-      unconfirmed: Vous devez confirmer votre compte pour continuer.
+      unconfirmed: |
+        Vous devez confirmer votre compte pour continuer.
+        <a href="/users/confirmation/new" class="a-orange">(Cliquez pour demander un nouvel e-mail de confirmation)</a>
       invited: Vous avez une invitation en attente. Acceptez-la pour terminer la création
         de votre compte.
     mailer:

lib/tasks/dmp_assistant_upgrade.rake

@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+# File for DMP Assistant upgrade tasks, beginning with release 4.1.1+portage-4.2.3
+
+# rubocop:disable Naming/VariableNumber
+namespace :dmp_assistant_upgrade do
+  desc 'Upgrade to DMP Assistant 4.1.1+portage-4.2.3'
+  task v4_2_3: :environment do
+    p '------------------------------------------------------------------------'
+    p 'Executing upgrade tasks for DMP Assistant 4.1.1+portage-4.2.3'
+    p 'Beginning task: Handle email confirmations for existing users'
+    p '------------------------------------------------------------------------'
+    handle_email_confirmations_for_existing_users
+    p 'Task completed: Handle email confirmations for existing users'
+    p 'All tasks completed successfully'
+  end
+  # rubocop:enable Naming/VariableNumber
+
+  private
+
+  def handle_email_confirmations_for_existing_users
+    p 'Updating :confirmable columns to nil for all users'
+    p '(i.e. Setting confirmed_at, confirmation_token, and confirmation_sent_at to nil for all users)'
+    p '------------------------------------------------------------------------'
+    set_confirmable_cols_to_nil_for_all_users
+    p '------------------------------------------------------------------------'
+    p 'Updating superusers so that they are not required to confirm their email addresses'
+    p '(i.e. Setting `confirmed_at = Time.now`` for superusers)'
+    p '------------------------------------------------------------------------'
+    confirm_superusers
+  end
+
+  # Setting `confirmed_at` to nil will require users to confirm their email addresses when using :confirmable
+  # Setting `confirmation_token` to nil will improve the email confirmation-related UX flow for existing users
+  # (For more information regarding this improved UX flow, refer to the following:
+  # `def handle_confirmation_instructions(user)` in `app/controllers/sessions_controller.rb`)
+  def set_confirmable_cols_to_nil_for_all_users
+    count = User.update_all(confirmed_at: nil, confirmation_token: nil, confirmation_sent_at: nil)
+    p ":confirmable columns updated to nil for #{count} users"
+  end
+
+  # Sets `confirmed_at` to `Time.now` for all superusers
+  def confirm_superusers
+    confirmed_at = Time.now
+    count = User.joins(:perms).where(perms: { id: super_admin_perm_ids })
+                .distinct
+                .update_all(confirmed_at: confirmed_at)
+    p "Updated confirmed_at = #{confirmed_at} for #{count} superuser(s)"
+  end
+
+  # Returns an array of all perm ids that are considered super admin perms
+  # (Based off of `def can_super_admin?`` in `app/models/user.rb`
+  # i.e. `can_add_orgs? || can_grant_api_to_orgs? || can_change_org?` )
+  def super_admin_perm_ids
+    [Perm.add_orgs.id, Perm.grant_api.id, Perm.change_affiliation.id]
+  end
+end

spec/factories/users.rb

@@ -60,6 +60,7 @@
     email        { Faker::Internet.unique.email }
     password     { 'password' }
     accept_terms { true }
+    confirmed_at { Time.now }
 
     trait :org_admin do
       after(:create) do |user, _evaluator|

spec/features/registrations_spec.rb

@@ -26,9 +26,9 @@
     click_button 'Create account'
 
     # Expectations
-    expect(current_path).to eql(plans_path)
-    expect(page).to have_text(user_attributes[:firstname])
-    expect(page).to have_text(user_attributes[:surname])
+    expect(current_path).to eql(root_path)
+    expect(page).to have_text('A message with a confirmation link has been sent to your email address.')
+    expect(User.count).to eq(1)
   end
 
   scenario 'User attempts to create a new acccount with invalid atts', :js do

spec/integration/openid_connect_sso_spec.rb

@@ -39,18 +39,6 @@
       expect(page).to have_content('John Doe')
     end
 
-    it 'does not create SSO link when user is signed out and SSO email is an existing account email' do
-      # Hardcode user email to what we are mocking via OmniAuth.config.mock_auth[:openid_connect]
-      user = create(:user, :org_admin, org: @org, email: '[email protected]', firstname: 'DMP Name',
-                                       surname: 'DMP Lastname')
-      expect(user.identifiers.count).to eql(0)
-      visit root_path
-      click_link 'Sign in with institutional or social ID'
-      error_message = 'That email appears to be associated with an existing account'
-      expect(page).to have_content(error_message)
-      expect(user.identifiers.count).to eql(0)
-    end
-
     xit 'links account from external credentails' do
       # Create existing user
       user = create(:user, :org_admin, org: @org, email: '[email protected]', firstname: 'DMP Name',