Merge integration into deployment-portage For Upcoming Release

CHANGELOG.md

@@ -1,9 +1,33 @@
 # Changelog
 
+## [Unreleased]
+
+### Added
+
+ - Add rake task for openid_connect / CILogon cleanup [#944](https://github.com/portagenetwork/roadmap/pull/944)
+
+### Changed
+
+ - Email Confirmation Changes [#923](https://github.com/portagenetwork/roadmap/pull/923)
+
+ - Disable Updating of User Emails [#917](https://github.com/portagenetwork/roadmap/pull/917)
+
+ - Apply `translation:sync` to `yaml` Files and Remove Unused `locale/` + `locales/` Files [#937](https://github.com/portagenetwork/roadmap/pull/937)
+
+### Fixed
+
+ - Fix User Lookup Via SSO Email: Make Query Case-Insensitive [#924](https://github.com/portagenetwork/roadmap/pull/924)
+
+ - Fixes to CILogon / `openid_connect` Tests [#922](https://github.com/portagenetwork/roadmap/pull/922)
+
+ - Fix Paginating, Sorting, and Searching Issues Within "Research Outputs" Tab [#938](https://github.com/portagenetwork/roadmap/pull/938)
+
 ## [4.1.1+portage-4.2.2] - 2024-09-18
 
 ### Changed
 
+ - Prevent Uploading of DB Fields to translation.io Within Test, Development, and UAT Environments [#945](https://github.com/portagenetwork/roadmap/pull/945)
+
  - Update Handling of SSO Linking [#907](https://github.com/portagenetwork/roadmap/pull/907)
 
  - Updated SSO button string [portagenetwork/roadmap#903](https://github.com/portagenetwork/roadmap/issues/903)

app/controllers/concerns/email_confirmation_handler.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+# EmailConfirmationHandler
+
+# Some users in our db are both unconfirmed AND have no outstanding confirmation_token
+# This is true for those users due to the following:
+#   - We haven't always used Devise's :confirmable module (it generates a confirmation_token when a user is created)
+#   - We have set `confirmed_at` and `confirmation_token` to nil via Rake tasks
+# This concern is meant to improve the confirmation process for those users
+module EmailConfirmationHandler
+  extend ActiveSupport::Concern
+
+  def confirmation_instructions_missing_and_handled?(user)
+    #  A user's "confirmation instructions are missing" if they're both unconfirmed and have no confirmation_token
+    return false if user_confirmed_or_has_confirmation_token?(user)
+
+    handle_missing_confirmation_instructions(user)
+    true
+  end
+
+  private
+
+  def user_confirmed_or_has_confirmation_token?(user)
+    user.confirmed? || user.confirmation_token.present?
+  end
+
+  def handle_missing_confirmation_instructions(user)
+    user.send_confirmation_instructions
+    redirect_to root_path, notice: I18n.t('devise.registrations.signed_up_but_unconfirmed')
+  end
+end

app/controllers/paginable/research_outputs_controller.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+module Paginable
+  # Controller for paginating/sorting/searching the research_outputs table
+  class ResearchOutputsController < ApplicationController
+    include Paginable
+
+    after_action :verify_authorized
+
+    # GET /paginable/plans/:plan_id/research_outputs
+    def index
+      @plan = Plan.find_by(id: params[:plan_id])
+      # Same assignment as app/controllers/research_outputs_controller.rb
+      research_outputs = ResearchOutput.includes(:repositories).where(plan_id: @plan.id)
+      # Same authorize handling as app/controllers/research_outputs_controller.rb
+      # `|| ResearchOutput.new(plan_id: @plan.id)` prevents Pundit::NotDefinedError when a direct
+      # GET /paginable/plans/:id/research_outputs request is made on a plan with 0 research_outputs
+      authorize(research_outputs.first || ResearchOutput.new(plan_id: @plan.id))
+      paginable_renderise(
+        partial: 'index',
+        scope: research_outputs,
+        query_params: { sort_field: 'research_outputs.title' },
+        format: :json
+      )
+    end
+  end
+end

app/controllers/registrations_controller.rb

@@ -144,7 +144,7 @@ def update
       if params[:skip_personal_details] == 'true'
         do_update_password(current_user, update_params)
       else
-        do_update(needs_password?(current_user))
+        do_update
       end
     else
       render(file: File.join(Rails.root, 'public/403.html'), status: 403, layout: false)
@@ -154,27 +154,15 @@ def update
 
   private
 
-  # check if we need password to update user data
-  # ie if password or email was changed
-  # extend this as needed
-  def needs_password?(user)
-    user.email != update_params[:email] || update_params[:password].present?
-  end
-
   # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
   # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
   # rubocop:disable Style/OptionalBooleanParameter
-  def do_update(require_password = true, confirm = false)
+  def do_update(confirm = false)
     restrict_orgs = Rails.configuration.x.application.restrict_orgs
     mandatory_params = true
     # added to by below, overwritten otherwise
     message = _('Save Unsuccessful. ')
     # ensure that the required fields are present
-
-    if update_params[:email].blank?
-      message += _('Please enter an email address. ')
-      mandatory_params &&= false
-    end
     if update_params[:firstname].blank?
       message += _('Please enter a First name. ')
       mandatory_params &&= false
@@ -194,39 +182,11 @@ def do_update(require_password = true, confirm = false)
       attrs = update_params
       attrs = handle_org(attrs: attrs)
 
-      # user is changing email or password
-      if require_password
-        # if user is changing email
-        if current_user.email == attrs[:email]
-          # remove the current_password because its not actuallyt part of the User record
-          attrs.delete(:current_password)
-
-          # This case is never reached since this method when called with
-          # require_password = true is because the email changed.
-          # The case for password changed goes to do_update_password instead
-          successfully_updated = current_user.update_without_password(attrs)
-        elsif attrs[:password].blank?
-          # password needs to be present
-          message = _('Please enter your password to change email address.')
-          successfully_updated = false
-        elsif current_user.valid_password?(attrs[:current_password])
-          successfully_updated = current_user.update_with_password(attrs)
-          # rubocop:disable Metrics/BlockNesting
-          unless successfully_updated
-            message = _("Save unsuccessful. \
-                That email address is already registered. \
-                You must enter a unique email address.")
-          end
-          # rubocop:enable Metrics/BlockNesting
-        else
-          message = _('Invalid password')
-        end
-      else
-        # password not required
-        # remove the current_password because its not actuallyt part of the User record
-        attrs.delete(:current_password)
-        successfully_updated = current_user.update_without_password(attrs)
-      end
+      # password not required
+      # remove the current_password because its not actuallyt part of the User record
+      attrs.delete(:current_password)
+      successfully_updated = current_user.update_without_password(attrs)
+
     else
       successfully_updated = false
     end
@@ -296,7 +256,7 @@ def sign_up_params
   end
 
   def update_params
-    params.require(:user).permit(:email, :firstname, :org_id, :language_id,
+    params.require(:user).permit(:firstname, :org_id, :language_id,
                                  :current_password, :password, :password_confirmation,
                                  :surname, :department_id, :org_id,
                                  :org_name, :org_crosswalk)

app/controllers/sessions_controller.rb

@@ -2,6 +2,8 @@
 
 # Controller that handles user login and logout
 class SessionsController < Devise::SessionsController
+  include EmailConfirmationHandler
+
   def new
     redirect_to(root_path)
   end
@@ -13,6 +15,9 @@ def create
     existing_user = User.find_by(email: params[:user][:email])
     unless existing_user.nil?
 
+      # See app/controllers/concerns/email_confirmation_handler.rb
+      return if confirmation_instructions_missing_and_handled?(existing_user)
+
       # Until ORCID login is supported
       unless session['devise.shibboleth_data'].nil?
         args = {

app/controllers/super_admin/users_controller.rb

@@ -119,8 +119,7 @@ def archive
     private
 
     def user_params
-      params.require(:user).permit(:email,
-                                   :firstname,
+      params.require(:user).permit(:firstname,
                                    :surname,
                                    :org_id, :org_name, :org_crosswalk,
                                    :department_id,

app/controllers/users/omniauth_callbacks_controller.rb

@@ -3,9 +3,10 @@
 module Users
   # Controller that handles callbacks from OmniAuth integrations (e.g. Shibboleth and ORCID)
   class OmniauthCallbacksController < Devise::OmniauthCallbacksController
-    # This is for the OpenidConnect CILogon
+    include EmailConfirmationHandler
 
-    # rubocop:disable Metrics/AbcSize, Metrics/MethodLength, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
+    # This is for the OpenidConnect CILogon
+    # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
     def openid_connect
       # First or create
       auth = request.env['omniauth.auth']
@@ -23,23 +24,7 @@ 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
-        sign_in_and_redirect user, event: :authentication
+        handle_openid_connect_for_signed_out_user(user, auth, identifier_scheme)
       elsif user.nil?
         # we need to link
         current_user.identifiers << Identifier.create(identifier_scheme: identifier_scheme,
@@ -58,7 +43,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 +125,32 @@ def handle_omniauth(scheme)
     def failure
       redirect_to root_path
     end
+
+    private
+
+    def handle_openid_connect_for_signed_out_user(user, auth, identifier_scheme)
+      # 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?
+      # See app/controllers/concerns/email_confirmation_handler.rb
+      return if confirmation_instructions_missing_and_handled?(user)
+
+      sign_in_and_redirect user, event: :authentication
+    end
+
+    # 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/research_output.rb

@@ -70,6 +70,15 @@ class ResearchOutput < ApplicationRecord
   # Ensure presence of the :output_type_description if the user selected 'other'
   validates_presence_of :output_type_description, if: -> { other? }, message: PRESENCE_MESSAGE
 
+  # ==========
+  # = Scopes =
+  # ==========
+
+  scope :search, lambda { |term|
+                   search_pattern = "%#{term}%"
+                   where('lower(title) LIKE lower(?)', search_pattern)
+                 }
+
   # ====================
   # = Instance methods =
   # ====================

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,19 +185,21 @@ def self.from_omniauth(auth)
   ##
   # Handle user creation from provider
   # rubocop:disable Metrics/AbcSize
-  def self.create_from_provider_data(provider_data)
-    user = User.find_or_initialize_by(email: provider_data.info.email)
+  def self.find_or_create_from_provider_data(provider_data)
+    user = User.find_or_initialize_by(email: provider_data.info.email.downcase)
 
-    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'),
-      surname: provider_data.info&.last_name.present? ? provider_data.info.last_name : _('Last name'),
-      email: provider_data.info.email,
+      firstname: provider_data.info&.first_name.presence || _('First name'),
+      surname: provider_data.info&.last_name.presence || _('Last name'),
       # 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.utc` and bypass Devise's email confirmation step
+      confirmed_at: Time.now.utc
     )
     user
   end

app/views/devise/registrations/_personal_details.html.erb

@@ -1,17 +1,17 @@
 <%= form_for(resource, namespace: current_user.id, as: resource_name, url: registration_path(resource_name), html: {method: :put, id: 'personal_details_registration_form' }) do |f| %>
-  <p class="form-control-static">
-    <%= sanitize _("Please note that your email address is used as your username. If you change this, remember to use your new email address on sign in.") %>
-  </p>
-
-  <p class="form-control-static"><%= _('You can edit any of the details below.') %></p>
-  <%= hidden_field_tag :unlink_flag, "false", id: 'unlink_flag' %>
 
   <div class="form-group col-xs-8">
     <%= f.label(:email, _('Email'), class: 'control-label') %>
-    <%= f.email_field(:email, class: "form-control", "aria-required": true, value: @user.email) %>
+    <%= f.email_field(:email, class: "form-control", "aria-required": true, value: @user.email, "disabled": true) %>
     <%= hidden_field_tag :original_email, @user.email %>
   </div>
 
+  <div class="form-group col-xs-12">
+    <p class="form-control-static"><%= _('You can edit any of the details below.') %></p>
+  </div>
+
+  <%= hidden_field_tag :unlink_flag, "false", id: 'unlink_flag' %>
+
   <div class="form-group col-xs-8">
     <%= f.label(:firstname, _('First name'), class: 'control-label') %>
     <%= f.text_field(:firstname, class: "form-control", "aria-required": true, value: @user.firstname) %>

app/views/super_admin/users/edit.html.erb

@@ -13,7 +13,7 @@
     <%= form_for(@user, namespace: :superadmin, as: :user, url: super_admin_user_path(@user), html: {method: :put, id: 'super_admin_user_edit' }) do |f| %>
       <div class="form-group col-xs-12">
         <%= f.label(:email, _('Email'), class: 'control-label') %>
-        <%= f.email_field(:email, class: "form-control", "aria-required": true) %>
+        <%= f.email_field(:email, class: "form-control", "aria-required": true, "disabled": true) %>
       </div>
 
       <div class="form-group col-xs-12">

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