CLDC-1425 Send notification to old email address when changing it (#1820)

* Send notification to old email address when changing it

* send email changed notification to old email

* Add specs

* send both emails

* Only send required confirmation emails

* Show flash notice

* Use reconfirmable template only if not confirmed

* Don't commit .env.development

* new email flow also when updating own email

.gitignore

@@ -42,6 +42,7 @@ yarn-debug.log*
 .yarn-integrity
 
 .env
+.env.development
 
 # Code coverage results
 /coverage

app/controllers/users_controller.rb

@@ -53,16 +53,21 @@ class UsersController < ApplicationController
       if @user == current_user
         bypass_sign_in @user
         flash[:notice] = I18n.t("devise.passwords.updated") if user_params.key?("password")
+        if user_params.key?("email") && FeatureToggle.new_email_journey?
+          flash[:notice] = I18n.t("devise.email.updated", email: @user.unconfirmed_email)
+        end
+
         redirect_to account_path
       else
         user_name = @user.name&.possessive || @user.email.possessive
-        case user_params[:active]
+        if user_params[:active] == "false"
-        when "false"
           @user.update!(confirmed_at: nil, sign_in_count: 0, initial_confirmation_sent: false)
           flash[:notice] = I18n.t("devise.activation.deactivated", user_name:)
-        when "true"
+        elsif user_params[:active] == "true"
           @user.send_confirmation_instructions
           flash[:notice] = I18n.t("devise.activation.reactivated", user_name:)
+        elsif user_params.key?("email") && FeatureToggle.new_email_journey?
+          flash[:notice] = I18n.t("devise.email.updated", email: @user.unconfirmed_email)
         end
         redirect_to user_path(@user)
       end

app/mailers/devise_notify_mailer.rb

@@ -35,12 +35,17 @@ class DeviseNotifyMailer < Devise::Mailer
   end
 
   def confirmation_instructions(record, token, _opts = {})
-    username = record.email
+    if email_changed?(record)
-    if email_changed(record)
+      if new_email_journey?
-      username = record.unconfirmed_email
+        send_email_changed_to_old_email(record)
-      send_confirmation_email(record.unconfirmed_email, record, token, username)
+        send_email_changed_to_new_email(record, token)
+      else
+        send_confirmation_email(record.unconfirmed_email, record, token, record.unconfirmed_email)
+        send_confirmation_email(record.email, record, token, record.unconfirmed_email)
+      end
+    else
+      send_confirmation_email(record.email, record, token, record.email)
     end
-    send_confirmation_email(record.email, record, token, username)
   end
 
   def intercept_send?(email)
@@ -54,10 +59,48 @@ class DeviseNotifyMailer < Devise::Mailer
     Rails.application.credentials[:email_allowlist] || []
   end
 
-private
+  def send_email_changed_to_old_email(record)
+    return true if intercept_send?(record.email)
+
+    send_email(
+      record.email,
+      User::FOR_OLD_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID,
+      {
+        new_email: record.unconfirmed_email,
+        old_email: record.email,
+      },
+    )
+  end
+
+  def send_email_changed_to_new_email(record, token)
+    return true if intercept_send?(record.email)
+
+    link = "#{user_confirmation_url}?confirmation_token=#{token}"
+
+    send_email(
+      record.unconfirmed_email,
+      User::FOR_NEW_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID,
+      {
+        new_email: record.unconfirmed_email,
+        old_email: record.email,
+        link:,
+      },
+    )
+  end
+
+  def email_changed?(record)
+    (
+      record.confirmable_template == User::CONFIRMABLE_TEMPLATE_ID && (
+        record.unconfirmed_email.present? && record.unconfirmed_email != record.email)
+    ) || (
+      new_email_journey? &&
+        record.versions.last.changeset.key?("unconfirmed_email") &&
+        record.confirmed?
+    )
+  end
 
-  def email_changed(record)
+  def new_email_journey?
-    record.confirmable_template == User::CONFIRMABLE_TEMPLATE_ID && (record.unconfirmed_email.present? && record.unconfirmed_email != record.email)
+    FeatureToggle.new_email_journey?
   end
 
   def send_confirmation_email(email, record, token, username)

app/models/user.rb

@@ -121,6 +121,8 @@ class User < ApplicationRecord
   RECONFIRMABLE_TEMPLATE_ID = "bcdec787-f0a7-46e9-8d63-b3e0a06ee455".freeze
   BETA_ONBOARDING_TEMPLATE_ID = "b48bc2cd-5887-4611-8296-d0ab3ed0e7fd".freeze
   USER_REACTIVATED_TEMPLATE_ID = "ac45a899-490e-4f59-ae8d-1256fc0001f9".freeze
+  FOR_OLD_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID = "3eb80517-1051-4dfc-b4cc-cb18228a3829".freeze
+  FOR_NEW_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID = "0cdd0be1-7fa5-4808-8225-ae4c5a002352".freeze
 
   def reset_password_notify_template
     RESET_PASSWORD_TEMPLATE_ID
@@ -131,7 +133,7 @@ class User < ApplicationRecord
       USER_REACTIVATED_TEMPLATE_ID
     elsif was_migrated_from_softwire? && last_sign_in_at.blank?
       BETA_ONBOARDING_TEMPLATE_ID
-    elsif initial_confirmation_sent
+    elsif initial_confirmation_sent && !confirmed?
       RECONFIRMABLE_TEMPLATE_ID
     else
       CONFIRMABLE_TEMPLATE_ID

app/services/feature_toggle.rb

@@ -33,4 +33,8 @@ class FeatureToggle
   def self.deduplication_flow_enabled?
     !Rails.env.production? && !Rails.env.staging?
   end
+
+  def self.new_email_journey?
+    !Rails.env.production?
+  end
 end

app/views/users/show.html.erb

@@ -117,6 +117,5 @@
         <% end %>
       <% end %>
     </div>
-
   </div>
 </div>

config/locales/en.yml

@@ -684,6 +684,8 @@ Make sure these answers are correct."
       hint_text: "This is more than 5 times the income, which is higher than we would expect."
 
   devise:
+    email:
+      updated: An email has been sent to %{email} to confirm this change.
     two_factor_authentication:
       success: "Two-factor authentication successful"
       attempt_failed: "Attempt failed"

spec/features/user_spec.rb

@@ -529,7 +529,7 @@ RSpec.describe "User Features" do
       end
 
       before do
-        other_user.update!(initial_confirmation_sent: true)
+        other_user.update!(initial_confirmation_sent: true, confirmed_at: nil)
         allow(user).to receive(:need_two_factor_authentication?).and_return(false)
         sign_in(user)
         visit(user_path(user.id))

spec/mailers/resend_invitation_mailer_spec.rb

@@ -57,9 +57,31 @@ RSpec.describe ResendInvitationMailer do
       end
 
       it "sends a reinvitation" do
-        expect(notify_client).to receive(:send_email).with(email_address: "active_user@example.com", template_id: User::RECONFIRMABLE_TEMPLATE_ID, personalisation:).once
+        expect(notify_client).to receive(:send_email).with(email_address: "active_user@example.com", template_id: User::CONFIRMABLE_TEMPLATE_ID, personalisation:).once
         described_class.new.resend_invitation_email(active_user)
       end
     end
+
+    context "with unconfirmed user after the initial invitation has been sent" do
+      let!(:unconfirmed_user) { create(:user, organisation:, confirmation_token: "dluch", initial_confirmation_sent: true, old_user_id: "234", sign_in_count: 0, confirmed_at: nil) }
+
+      let(:personalisation) do
+        {
+          name: unconfirmed_user.name,
+          email: unconfirmed_user.email,
+          organisation: unconfirmed_user.organisation.name,
+          link: include("/account/confirmation?confirmation_token=#{unconfirmed_user.confirmation_token}"),
+        }
+      end
+
+      before do
+        LegacyUser.destroy_all
+      end
+
+      it "sends a reinvitation" do
+        expect(notify_client).to receive(:send_email).with(email_address: unconfirmed_user.email, template_id: User::RECONFIRMABLE_TEMPLATE_ID, personalisation:).once
+        described_class.new.resend_invitation_email(unconfirmed_user)
+      end
+    end
   end
 end

spec/requests/users_controller_spec.rb

@@ -1538,7 +1538,7 @@ RSpec.describe UsersController, type: :request do
           expect(whodunnit_actor.id).to eq(user.id)
         end
 
-        context "when user changes email, dpo and key contact" do
+        context "when user changes email, dpo and key contact", :aggregate_failures do
           let(:params) { { id: user.id, user: { name: new_name, email: new_email, is_dpo: "true", is_key_contact: "true" } } }
           let(:personalisation) do
             {
@@ -1551,6 +1551,8 @@ RSpec.describe UsersController, type: :request do
 
           before do
             user.legacy_users.destroy_all
+
+            allow(FeatureToggle).to receive(:new_email_journey?).and_return(false)
           end
 
           it "allows changing email and dpo" do
@@ -1566,6 +1568,43 @@ RSpec.describe UsersController, type: :request do
             expect(notify_client).to receive(:send_email).with(email_address: user.email, template_id: User::CONFIRMABLE_TEMPLATE_ID, personalisation:).once
             request
           end
+
+          context "with new email journy enabled" do
+            before do
+              allow(FeatureToggle).to receive(:new_email_journey?).and_return(true)
+            end
+
+            it "shows flash notice" do
+              patch("/users/#{other_user.id}", headers:, params:)
+
+              expect(flash[:notice]).to eq("An email has been sent to #{new_email} to confirm this change.")
+            end
+
+            it "sends new flow emails" do
+              expect(notify_client).to receive(:send_email).with(
+                email_address: other_user.email,
+                template_id: User::FOR_OLD_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID,
+                personalisation: {
+                  new_email:,
+                  old_email: other_user.email,
+                },
+              ).once
+
+              expect(notify_client).to receive(:send_email).with(
+                email_address: new_email,
+                template_id: User::FOR_NEW_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID,
+                personalisation: {
+                  new_email:,
+                  old_email: other_user.email,
+                  link: include("/account/confirmation?confirmation_token="),
+                },
+              ).once
+
+              expect(notify_client).not_to receive(:send_email)
+
+              patch "/users/#{other_user.id}", headers:, params:
+            end
+          end
         end
 
         context "when we update the user password" do
@@ -1679,13 +1718,28 @@ RSpec.describe UsersController, type: :request do
               expect(page).to have_content(other_user.reload.email.to_s)
             end
 
-            context "when the support user tries to update the user’s password" do
+            context "when the support user tries to update the user’s password", :aggregate_failures do
               let(:params) do
                 {
-                  id: user.id, user: { password: new_name, password_confirmation: new_name, name: "new name" }
+                  id: user.id, user: { password: new_name, password_confirmation: new_name, name: "new name", email: new_email }
+                }
+              end
+
+              let(:personalisation) do
+                {
+                  name: params[:user][:name],
+                  email: new_email,
+                  organisation: other_user.organisation.name,
+                  link: include("/account/confirmation?confirmation_token="),
                 }
               end
 
+              before do
+                other_user.legacy_users.destroy_all
+
+                allow(FeatureToggle).to receive(:new_email_journey?).and_return(false)
+              end
+
               it "does not update the password" do
                 expect { patch "/users/#{other_user.id}", headers:, params: }
                   .not_to change(other_user, :encrypted_password)
@@ -1695,6 +1749,57 @@ RSpec.describe UsersController, type: :request do
                 expect { patch "/users/#{other_user.id}", headers:, params: }
                   .to change { other_user.reload.name }.from("Danny Rojas").to("new name")
               end
+
+              it "allows changing email" do
+                expect { patch "/users/#{other_user.id}", headers:, params: }
+                  .to change { other_user.reload.unconfirmed_email }.from(nil).to(new_email)
+              end
+
+              it "sends a confirmation email to both emails" do
+                expect(notify_client).to receive(:send_email).with(email_address: other_user.email, template_id: User::CONFIRMABLE_TEMPLATE_ID, personalisation:).once
+                expect(notify_client).to receive(:send_email).with(email_address: new_email, template_id: User::CONFIRMABLE_TEMPLATE_ID, personalisation:).once
+
+                expect(notify_client).not_to receive(:send_email)
+
+                patch "/users/#{other_user.id}", headers:, params:
+              end
+
+              context "with new user email flow enabled" do
+                before do
+                  allow(FeatureToggle).to receive(:new_email_journey?).and_return(true)
+                end
+
+                it "shows flash notice" do
+                  patch("/users/#{other_user.id}", headers:, params:)
+
+                  expect(flash[:notice]).to eq("An email has been sent to #{new_email} to confirm this change.")
+                end
+
+                it "sends new flow emails" do
+                  expect(notify_client).to receive(:send_email).with(
+                    email_address: other_user.email,
+                    template_id: User::FOR_OLD_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID,
+                    personalisation: {
+                      new_email:,
+                      old_email: other_user.email,
+                    },
+                  ).once
+
+                  expect(notify_client).to receive(:send_email).with(
+                    email_address: new_email,
+                    template_id: User::FOR_NEW_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID,
+                    personalisation: {
+                      new_email:,
+                      old_email: other_user.email,
+                      link: include("/account/confirmation?confirmation_token="),
+                    },
+                  ).once
+
+                  expect(notify_client).not_to receive(:send_email)
+
+                  patch "/users/#{other_user.id}", headers:, params:
+                end
+              end
             end
           end
         end