Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
cbb6dad
Merge pull request #3525 from DMPRoadmap/dstuckey-dmponline-update-se…
johnpinto1 May 12, 2025
895a4c8
Add `:confirmable` to included devise modules
aaronskiba Sep 25, 2024
90e6445
Streamline email confirmation for existing users
aaronskiba Apr 16, 2025
dcbc524
Refactor SessionsController#create
aaronskiba Apr 16, 2025
7587466
Refactor SessionsController#create
aaronskiba Apr 16, 2025
638e179
Adapt existing tests to `:confirmable` re-addition
aaronskiba Oct 7, 2024
1ceeec0
Add email confirmation .yml translations
aaronskiba Sep 26, 2024
6f0924e
Streamline email confirmation for SSO sign-in
aaronskiba Apr 17, 2025
e1cdee9
Create EmailConfirmationHandler concern & refactor
aaronskiba Apr 17, 2025
cdfe8fd
Refactor `def handle_omniauth`
aaronskiba Apr 17, 2025
ffdfd56
Small formatting cleanup for omniauth controller
aaronskiba Apr 17, 2025
f934172
Disable rubocop offences
aaronskiba Apr 17, 2025
c3075f1
Enable OmniAuth testing in RSpec tests
aaronskiba Apr 22, 2025
47233c7
Add tests for custom email confirmation UX flow
aaronskiba Apr 22, 2025
ad899ae
Fix failing shibboleth test
aaronskiba Apr 22, 2025
b13e4ff
Use `I18n.default_locale` in faker config
aaronskiba Apr 15, 2025
b7d16c6
Update CHANGELOG.md
aaronskiba Apr 24, 2025
ab61ae0
Fix comments in Shibboleth test
aaronskiba Jun 5, 2025
2535269
Merge pull request #3507 from DMPRoadmap/upstream/aaron/add-email-con…
aaronskiba Jun 5, 2025
d163ecc
Add logic allowing superusers to confirm emails
aaronskiba Feb 7, 2025
8dd4180
Add users_controller_spec + confirmation tests
aaronskiba Feb 10, 2025
b923181
Fix email confirmation checkbox's alignment
aaronskiba Feb 10, 2025
6a69538
Prevent unconfirming of super admin emails
aaronskiba Feb 10, 2025
fff1204
Prevent superadmin un-confirmation if UI bypassed
aaronskiba Feb 10, 2025
6a40717
Update CHANGELOG.md
aaronskiba Feb 10, 2025
b00f75b
Add comment to `def handle_confirmed_at_param`
aaronskiba May 27, 2025
8b4cdd3
Clarify email confirmation checkbox instructions
aaronskiba May 28, 2025
fbc38ec
Update CHANGELOG.md
aaronskiba Jul 4, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
# Changelog

- Add Functionality Allowing Super Admins to Confirm/Unconfirm User Emails [#3535](https://github.com/DMPRoadmap/roadmap/pull/3535)
- Updated seeds.rb file for identifier_schemes to include context value and removed logo_url and idenitifier_prefix for Shibboleth (as it was causing issues with SSO). [#3525](https://github.com/DMPRoadmap/roadmap/pull/3525)
- Adjustments to style of select tags and plan download layout [#3509](https://github.com/DMPRoadmap/roadmap/pull/3509)
- Fix failing eslint workflow / upgrade `actions/checkout` & `actions/setup-node` to v3 [#3503](https://github.com/DMPRoadmap/roadmap/pull/3503)
- Fix rendering of `confirm_merge` partial [#3515](https://github.com/DMPRoadmap/roadmap/pull/3515)
- Remove Auto-Generated TinyMCE Skins and Add `public/tinymce/skins/` to `.gitignore` [#3466](https://github.com/DMPRoadmap/roadmap/pull/3466)
- Re-implement email confirmation [#3507](https://github.com/DMPRoadmap/roadmap/pull/3507)
- Update `spec/support/faker.rb` to use `I18n.default_locale` [#3507](https://github.com/DMPRoadmap/roadmap/pull/3507)

## v5.0.0

Expand Down
34 changes: 21 additions & 13 deletions app/controllers/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

# Controller that handles user login and logout
class SessionsController < Devise::SessionsController
include EmailConfirmationHandler

def new
redirect_to(root_path)
end
Expand All @@ -11,28 +13,22 @@ def new
# rubocop:disable Metrics/AbcSize
def create
existing_user = User.find_by(email: params[:user][:email])
unless existing_user.nil?
if existing_user.present?

# (see app/models/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 = {
identifier_scheme: IdentifierScheme.find_by(name: 'shibboleth'),
value: session['devise.shibboleth_data']['uid'],
identifiable: existing_user,
attrs: session['devise.shibboleth_data']
}
@ui = Identifier.new(args)
end
@ui = create_shibboleth_identifier(existing_user) unless session['devise.shibboleth_data'].nil?
session[:locale] = existing_user.locale unless existing_user.locale.nil?
# Method defined at controllers/application_controller.rb
set_locale
end

super do
if !@ui.nil? && @ui.save
# rubocop:disable Layout/LineLength
flash[:notice] = _('Your account has been successfully linked to your institutional credentials. You will now be able to sign in with them.')
# rubocop:enable Layout/LineLength
flash[:notice] = _('Your account has been successfully linked to your institutional credentials. ' \
'You will now be able to sign in with them.')
end
end
end
Expand All @@ -45,3 +41,15 @@ def destroy
set_locale
end
end

private

def create_shibboleth_identifier(user)
args = {
identifier_scheme: IdentifierScheme.find_by(name: 'shibboleth'),
value: session['devise.shibboleth_data']['uid'],
identifiable: user,
attrs: session['devise.shibboleth_data']
}
Identifier.new(args)
end
24 changes: 23 additions & 1 deletion app/controllers/super_admin/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ def update

# Remove the extraneous Org Selector hidden fields
attrs = remove_org_selection_params(params_in: attrs)
attrs = handle_confirmed_at_param(attrs)

if @user.update(attrs)
# If its a new Org create it
Expand Down Expand Up @@ -125,7 +126,8 @@ def user_params
:org_id, :org_name, :org_crosswalk,
:department_id,
:language_id,
:other_organisation)
:other_organisation,
:confirmed_at)
end

def merge_accounts
Expand All @@ -136,5 +138,25 @@ def merge_accounts
flash.now[:alert] = failure_message(@user, _('merge'))
end
end

def handle_confirmed_at_param(attrs)
# NOTE: The :confirmed_at param is controlled by a check_box in the form
# `app/views/super_admin/users/_email_confirmation_status.html.erb`.
# When the checkbox is checked, Rails submits the string '1' (indicating "confirmed").
# When unchecked, it submits the string '0' (indicating "unconfirmed").

# if an unconfirmed email is now being confirmed
if !@user.confirmed? && attrs[:confirmed_at] == '1'
attrs[:confirmed_at] = Time.current
# elsif a confirmed email is now being unconfirmed and the user is not a super admin
elsif @user.confirmed? && attrs[:confirmed_at] == '0' && !@user.can_super_admin?
attrs[:confirmed_at] = nil
else
# else delete the param
# (keeps value nil for unconfirmed user and maintains previous Time value for confirmed user)
attrs.delete(:confirmed_at)
end
attrs
end
end
end
96 changes: 54 additions & 42 deletions app/controllers/users/omniauth_callbacks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
module Users
# Controller that handles callbacks from OmniAuth integrations (e.g. Shibboleth and ORCID)
class OmniauthCallbacksController < Devise::OmniauthCallbacksController
include EmailConfirmationHandler
##
# Dynamically build a handler for each omniauth provider
# -------------------------------------------------------------
Expand All @@ -21,8 +22,6 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
#
# scheme - The IdentifierScheme for the provider
#
# rubocop:disable Metrics/AbcSize, Metrics/MethodLength
# rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
def handle_omniauth(scheme)
user = if request.env['omniauth.auth'].nil?
User.from_omniauth(request.env)
Expand All @@ -32,55 +31,68 @@ def handle_omniauth(scheme)

# If the user isn't logged in
if current_user.nil?
# If the uid didn't have a match in the system send them to register
if user.nil?
session["devise.#{scheme.name.downcase}_data"] = request.env['omniauth.auth']
redirect_to new_user_registration_url

# Otherwise sign them in
elsif scheme.name == 'shibboleth'
# Until ORCID becomes supported as a login method
set_flash_message(:notice, :success, kind: scheme.description) if is_navigational_format?
sign_in_and_redirect user, event: :authentication
else
flash[:notice] = _('Successfully signed in')
redirect_to new_user_registration_url
end

handle_omniauth_for_signed_out_user(user, scheme)
# The user is already logged in and just registering the uid with us
else
# If the user could not be found by that uid then attach it to their record
if user.nil?
if Identifier.create(identifier_scheme: scheme,
value: request.env['omniauth.auth'].uid,
attrs: request.env['omniauth.auth'],
identifiable: current_user)
flash[:notice] =
format(_('Your account has been successfully linked to %{scheme}.'),
scheme: scheme.description)
handle_omniauth_for_signed_in_user(user, scheme)
end
end

def failure
redirect_to root_path
end

else
flash[:alert] = format(_('Unable to link your account to %{scheme}.'),
scheme: scheme.description)
end
private

elsif user.id != current_user.id
# If a user was found but does NOT match the current user then the identifier has
# already been attached to another account (likely the user has 2 accounts)
# rubocop:disable Layout/LineLength
flash[:alert] = _("The current #{scheme.description} iD has been already linked to a user with email #{identifier.user.email}")
# rubocop:enable Layout/LineLength
# rubocop:disable Metrics/AbcSize
def handle_omniauth_for_signed_in_user(user, scheme)
# If the user could not be found by that uid then attach it to their record
if user.nil?
if Identifier.create(identifier_scheme: scheme,
value: request.env['omniauth.auth'].uid,
attrs: request.env['omniauth.auth'],
identifiable: current_user)
flash[:notice] = format(_('Your account has been successfully linked to %{scheme}.'),
scheme: scheme.description)

else
flash[:alert] = format(_('Unable to link your account to %{scheme}.'),
scheme: scheme.description)
end

# Redirect to the User Profile page
redirect_to edit_user_registration_path
elsif user.id != current_user.id
# If a user was found but does NOT match the current user then the identifier has
# already been attached to another account (likely the user has 2 accounts)
flash[:alert] = _("The current #{scheme.description} iD has been already linked " \
"to a user with email #{identifier.user.email}")
end

# Redirect to the User Profile page
redirect_to edit_user_registration_path
end
# rubocop:enable Metrics/AbcSize, Metrics/MethodLength
# rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
# rubocop:enable Metrics/AbcSize

def failure
redirect_to root_path
# rubocop:disable Metrics/AbcSize
def handle_omniauth_for_signed_out_user(user, scheme)
# If the uid didn't have a match in the system send them to register
if user.nil?
session["devise.#{scheme.name.downcase}_data"] = request.env['omniauth.auth']
redirect_to new_user_registration_url

# Otherwise sign them in
elsif scheme.name == 'shibboleth'
# Until ORCID becomes supported as a login method

# (see app/models/concerns/email_confirmation_handler.rb)
return if confirmation_instructions_missing_and_handled?(user)

set_flash_message(:notice, :success, kind: scheme.description) if is_navigational_format?
sign_in_and_redirect user, event: :authentication
else
flash[:notice] = _('Successfully signed in')
redirect_to new_user_registration_url
end
end
# rubocop:enable Metrics/AbcSize
end
end
29 changes: 29 additions & 0 deletions app/models/concerns/email_confirmation_handler.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# frozen_string_literal: true

# 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 (lib/tasks/email_confirmation.rake)
# This concern is meant to streamline 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
2 changes: 1 addition & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]

# default user language to the default language
Expand Down
12 changes: 12 additions & 0 deletions app/views/super_admin/users/_email_confirmation_status.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<div class="form-group col-xs-12">
<%= f.label(:confirmed_at, _('Email status: '), class: 'control-label') %>
<%= @user.confirmed? ? _("Confirmed.") : _("Unconfirmed.") %>
<br>
<% is_checkbox_disabled = @user.can_super_admin? && @user.confirmed_at.present? %>
<%= f.check_box(:confirmed_at, { checked: @user.confirmed?,
disabled: is_checkbox_disabled,
style: 'vertical-align: middle;
margin-top: -2px;' }) %>
<% checkbox_helper_text = @user.confirmed? ? _('(Uncheck to unconfirm email)') : _('(Check to confirm email)') %>
<%= content_tag(:small, checkbox_helper_text) unless is_checkbox_disabled %>
</div>
2 changes: 2 additions & 0 deletions app/views/super_admin/users/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@
</div>
<% end %>

<%= render 'email_confirmation_status', f: f %>

<div class="form-control mb-3 col-xs-12">
<%= f.button(_('Save'), class: 'btn btn-secondary', type: "submit", id: "personal_details_registration_form_submit") %>

Expand Down
1 change: 1 addition & 0 deletions config/environments/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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: 'noreply@example.org' }

# Print deprecation notices to the stderr.
config.active_support.deprecation = :stderr
Expand Down
3 changes: 2 additions & 1 deletion config/locales/translation.en-CA.yml
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,8 @@ en-CA:
not_found_in_database: Invalid %{authentication_keys} 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 email address before continuing.
unconfirmed: You need to confirm your account before continuing. <a href="/users/confirmation/new"
class="a-orange">(Click to request a new confirmation email)</a>
invited:
mailer:
confirmation_instructions:
Expand Down
3 changes: 2 additions & 1 deletion config/locales/translation.en-GB.yml
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,8 @@ en-GB:
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:
Expand Down
3 changes: 2 additions & 1 deletion config/locales/translation.fr-CA.yml
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,8 @@ fr-CA:
not_found_in_database: "%{authentication_keys} 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 adresse courriel pour continuer.
unconfirmed: Vous devez confirmer votre compte avant de continuer. <a href="/users/confirmation/new"
class="a-orange">(cliquez pour demander un nouveau courriel de confirmation)</a>
invited: Vous avez une invitation en attente. Acceptez-la pour terminer la création
de votre compte.
mailer:
Expand Down
47 changes: 47 additions & 0 deletions lib/tasks/email_confirmation.rake
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# frozen_string_literal: true

namespace :email_confirmation do
desc 'Reset confirmation status for all users, excluding superusers'
task clear_all: :environment do
p '------------------------------------------------------------------------'
p 'Beginning task: Unconfirming all users except superusers'
p '------------------------------------------------------------------------'
unconfirm_all_users_except_superusers
p 'Task completed: Unconfirmed all users except superusers'
end

private

def unconfirm_all_users_except_superusers
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.current` for superusers)'
p '------------------------------------------------------------------------'
confirm_superusers
end

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.current` for all superusers
def confirm_superusers
confirmed_at = Time.current
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
Loading
Loading