From 04ba5009bea7e3c85e629691f897983add1fa3d4 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Fri, 7 Feb 2025 15:17:28 -0700 Subject: [PATCH 1/5] 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 a27f528adb..f56a1fb39e 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 8d1a3ab7ee..d9f1da15f6 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 5076b174fdf3991e0665a6de81f3a451ee17a8f1 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 10 Feb 2025 09:50:04 -0700 Subject: [PATCH 2/5] 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 8b7d177694b5e96c767a681be61dd23255bc1f8a Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 10 Feb 2025 10:44:28 -0700 Subject: [PATCH 3/5] 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 b6bea33662132b8d90b34f2c099f0394f94292e2 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 10 Feb 2025 12:15:29 -0700 Subject: [PATCH 4/5] 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 9b2528d0d788552babfa01f72efa6441cb49e463 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 10 Feb 2025 12:56:42 -0700 Subject: [PATCH 5/5] 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 f56a1fb39e..93be8fcec7 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