From d163ecc9671d547e3449d9542c1621692324758a Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Fri, 7 Feb 2025 15:17:28 -0700 Subject: [PATCH 1/9] Add logic allowing superusers to confirm emails This change addresses an issue where some users reported not receiving the confirmation instructions email. Rather than having to manually update the users email confirmation status within the Rails console, this change allows users to confirm/unconfirm user emails within the app. --- .../super_admin/users_controller.rb | 19 ++++++++++++++++++- .../users/_email_confirmation_status.html.erb | 7 +++++++ app/views/super_admin/users/edit.html.erb | 2 ++ 3 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 app/views/super_admin/users/_email_confirmation_status.html.erb diff --git a/app/controllers/super_admin/users_controller.rb b/app/controllers/super_admin/users_controller.rb index c4b96141b7..36a95d5d5f 100644 --- a/app/controllers/super_admin/users_controller.rb +++ b/app/controllers/super_admin/users_controller.rb @@ -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 @@ -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 @@ -136,5 +138,20 @@ def merge_accounts flash.now[:alert] = failure_message(@user, _('merge')) end end + + def handle_confirmed_at_param(attrs) + # 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 + elsif @user.confirmed? && attrs[:confirmed_at] == '0' + 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 diff --git a/app/views/super_admin/users/_email_confirmation_status.html.erb b/app/views/super_admin/users/_email_confirmation_status.html.erb new file mode 100644 index 0000000000..21670c2e91 --- /dev/null +++ b/app/views/super_admin/users/_email_confirmation_status.html.erb @@ -0,0 +1,7 @@ +
+ <%= f.label(:confirmed_at, _('Email Confirmation Status'), class: 'control-label') %> +
+ <%= f.check_box(:confirmed_at, checked: @user.confirmed? ) %> + <%= @user.confirmed? ? _("Confirmed.") : _("Unconfirmed.") %> + <%= _("(Use checkbox to change status.)") %> +
diff --git a/app/views/super_admin/users/edit.html.erb b/app/views/super_admin/users/edit.html.erb index d4dc76902b..e58da75449 100644 --- a/app/views/super_admin/users/edit.html.erb +++ b/app/views/super_admin/users/edit.html.erb @@ -57,6 +57,8 @@ <% end %> + <%= render 'email_confirmation_status', f: f %> +
<%= f.button(_('Save'), class: 'btn btn-secondary', type: "submit", id: "personal_details_registration_form_submit") %> From 8dd4180919f29c753ba76e10ebb558f112a06408 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 10 Feb 2025 09:50:04 -0700 Subject: [PATCH 2/9] Add users_controller_spec + confirmation tests --- .../super_admin/users_controller_spec.rb | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 spec/controllers/super_admin/users_controller_spec.rb diff --git a/spec/controllers/super_admin/users_controller_spec.rb b/spec/controllers/super_admin/users_controller_spec.rb new file mode 100644 index 0000000000..85bd2eb372 --- /dev/null +++ b/spec/controllers/super_admin/users_controller_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe SuperAdmin::UsersController, type: :controller do + let(:super_admin) { create(:user, :super_admin) } + let(:user) { create(:user, confirmed_at: nil) } + + before do + sign_in super_admin + end + + describe 'PUT #update' do + context 'when confirming an unconfirmed user' do + it 'sets confirmed_at to the current time' do + put :update, params: { id: user.id, user: { confirmed_at: '1' } } + user.reload + expect(user.confirmed_at).to be_a(Time) + end + end + + context 'when unconfirming a confirmed user' do + before do + user.update(confirmed_at: Time.current) + end + + it 'sets confirmed_at to nil' do + put :update, params: { id: user.id, user: { confirmed_at: '0' } } + user.reload + expect(user.confirmed_at).to be_nil + end + end + + context 'when update will not affect confirmation status' do + it 'does not update confirmed_at value for an already confirmed user' do + # (usec: 0) removes mircoseconds to better enable comparison + user.update(confirmed_at: Time.current.change(usec: 0)) + original_confirmed_at = user.confirmed_at + patch :update, params: { id: user.id, user: { firstname: 'NewName', confirmed_at: '1' } } + user.reload + expect(user.confirmed_at).to eq(original_confirmed_at) + end + end + end +end From b9231819ddd078d23181adf3970e769127eb2617 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 10 Feb 2025 10:44:28 -0700 Subject: [PATCH 3/9] Fix email confirmation checkbox's alignment --- .../super_admin/users/_email_confirmation_status.html.erb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/views/super_admin/users/_email_confirmation_status.html.erb b/app/views/super_admin/users/_email_confirmation_status.html.erb index 21670c2e91..fe7b09f705 100644 --- a/app/views/super_admin/users/_email_confirmation_status.html.erb +++ b/app/views/super_admin/users/_email_confirmation_status.html.erb @@ -1,7 +1,9 @@
<%= f.label(:confirmed_at, _('Email Confirmation Status'), class: 'control-label') %>
- <%= f.check_box(:confirmed_at, checked: @user.confirmed? ) %> + <%= f.check_box(:confirmed_at, { checked: @user.confirmed?, + style: 'vertical-align: middle; + margin-top: -2px;' }) %> <%= @user.confirmed? ? _("Confirmed.") : _("Unconfirmed.") %> <%= _("(Use checkbox to change status.)") %>
From 6a695385c07e9ae08ebbed8129e0784b3cc54d45 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 10 Feb 2025 12:15:29 -0700 Subject: [PATCH 4/9] Prevent unconfirming of super admin emails --- .../super_admin/users/_email_confirmation_status.html.erb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/views/super_admin/users/_email_confirmation_status.html.erb b/app/views/super_admin/users/_email_confirmation_status.html.erb index fe7b09f705..7528238d91 100644 --- a/app/views/super_admin/users/_email_confirmation_status.html.erb +++ b/app/views/super_admin/users/_email_confirmation_status.html.erb @@ -1,9 +1,11 @@
<%= f.label(:confirmed_at, _('Email Confirmation Status'), class: 'control-label') %>
- <%= f.check_box(:confirmed_at, { checked: @user.confirmed?, + <% 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;' }) %> <%= @user.confirmed? ? _("Confirmed.") : _("Unconfirmed.") %> - <%= _("(Use checkbox to change status.)") %> + <%= content_tag(:small, _("(Use checkbox to change status.)")) unless is_checkbox_disabled %>
From fff120457dc6a860c17d950fea2e85505d02ac83 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 10 Feb 2025 12:56:42 -0700 Subject: [PATCH 5/9] Prevent superadmin un-confirmation if UI bypassed `spec/controllers/super_admin/users_controller_spec.rb`: - Added test to prevent un-confirming of any super admin email within app `app/controllers/super_admin/users_controller.rb`: - Added !@user.can_super_admin? check to enable the aforementioned test to pass - Also, updated a `patch` statement to `put` --- app/controllers/super_admin/users_controller.rb | 4 ++-- spec/controllers/super_admin/users_controller_spec.rb | 10 +++++++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/app/controllers/super_admin/users_controller.rb b/app/controllers/super_admin/users_controller.rb index 36a95d5d5f..d701e82019 100644 --- a/app/controllers/super_admin/users_controller.rb +++ b/app/controllers/super_admin/users_controller.rb @@ -143,8 +143,8 @@ def handle_confirmed_at_param(attrs) # 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 - elsif @user.confirmed? && attrs[:confirmed_at] == '0' + # 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 diff --git a/spec/controllers/super_admin/users_controller_spec.rb b/spec/controllers/super_admin/users_controller_spec.rb index 85bd2eb372..5d3232eb76 100644 --- a/spec/controllers/super_admin/users_controller_spec.rb +++ b/spec/controllers/super_admin/users_controller_spec.rb @@ -36,10 +36,18 @@ # (usec: 0) removes mircoseconds to better enable comparison user.update(confirmed_at: Time.current.change(usec: 0)) original_confirmed_at = user.confirmed_at - patch :update, params: { id: user.id, user: { firstname: 'NewName', confirmed_at: '1' } } + put :update, params: { id: user.id, user: { firstname: 'NewName', confirmed_at: '1' } } user.reload expect(user.confirmed_at).to eq(original_confirmed_at) end end + + context 'when attempting to set a super_admin to unconfirmed' do + it 'does not update confirmed_at value to nil' do + put :update, params: { id: super_admin.id, user: { confirmed_at: '0' } } + super_admin.reload + expect(super_admin.confirmed_at).not_to be_nil + end + end end end From 6a4071700601525fce27dfe882e403531aa09a15 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 10 Feb 2025 13:10:19 -0700 Subject: [PATCH 6/9] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe1f3f3280..c62924decc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +- Add Functionality Allowing Super Admins to Confirm/Unconfirm User Emails [#1009](https://github.com/portagenetwork/roadmap/pull/1009) - 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) From b00f75b97fcea9673f7a682c80988b32421cfa01 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Tue, 27 May 2025 17:59:04 -0600 Subject: [PATCH 7/9] Add comment to `def handle_confirmed_at_param` --- app/controllers/super_admin/users_controller.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/controllers/super_admin/users_controller.rb b/app/controllers/super_admin/users_controller.rb index d701e82019..3b98ce978c 100644 --- a/app/controllers/super_admin/users_controller.rb +++ b/app/controllers/super_admin/users_controller.rb @@ -140,6 +140,11 @@ def merge_accounts 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 From 8b4cdd35798e27d7010f0ebc9ab93493bb3c53d2 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Wed, 28 May 2025 11:57:15 -0600 Subject: [PATCH 8/9] Clarify email confirmation checkbox instructions Improves UX and reduces confusion when updating a user's email confirmation status. - Moved confirmation status inline with the "Email status:" heading - Updated checkbox helper text to reflect current @user.confirmed? state: - Shows "(Check to confirm email)" if unconfirmed - Shows "(Uncheck to unconfirm email)" if confirmed --- .../super_admin/users/_email_confirmation_status.html.erb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/views/super_admin/users/_email_confirmation_status.html.erb b/app/views/super_admin/users/_email_confirmation_status.html.erb index 7528238d91..2641962229 100644 --- a/app/views/super_admin/users/_email_confirmation_status.html.erb +++ b/app/views/super_admin/users/_email_confirmation_status.html.erb @@ -1,11 +1,12 @@
- <%= f.label(:confirmed_at, _('Email Confirmation Status'), class: 'control-label') %> + <%= f.label(:confirmed_at, _('Email status: '), class: 'control-label') %> + <%= @user.confirmed? ? _("Confirmed.") : _("Unconfirmed.") %>
<% 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;' }) %> - <%= @user.confirmed? ? _("Confirmed.") : _("Unconfirmed.") %> - <%= content_tag(:small, _("(Use checkbox to change status.)")) unless is_checkbox_disabled %> + <% checkbox_helper_text = @user.confirmed? ? _('(Uncheck to unconfirm email)') : _('(Check to confirm email)') %> + <%= content_tag(:small, checkbox_helper_text) unless is_checkbox_disabled %>
From fbc38ec537575c119d3ea8d8bebdecc8394fd05f Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Fri, 4 Jul 2025 09:56:13 -0600 Subject: [PATCH 9/9] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c62924decc..022ca96f02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -- Add Functionality Allowing Super Admins to Confirm/Unconfirm User Emails [#1009](https://github.com/portagenetwork/roadmap/pull/1009) +- 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)