Merge pull request #4069 from betagouv/improve-email-change

Amélioration du changement d'adresse email

app/assets/stylesheets/new_design/card.scss

@@ -21,6 +21,14 @@
     }
   }
 
+  &.warning {
+    border-top: 8px solid $orange;
+
+    .card-title {
+      color: $orange;
+    }
+  }
+
   &.feedback {
     max-width: 600px;
     margin: 30px auto;

app/assets/stylesheets/new_design/profil.scss

@@ -12,4 +12,8 @@
   p {
     margin: 16px auto;
   }
+
+  .email-address {
+    font-weight: bold;
+  }
 }

app/controllers/users/profil_controller.rb

@@ -12,8 +12,9 @@ module Users
     def update_email
       if @current_user.update(update_email_params)
         flash.notice = t('devise.registrations.update_needs_confirmation')
-      # to avoid leaking who has signed in
       elsif @current_user.errors&.details&.dig(:email)&.any? { |e| e[:error] == :taken }
+        UserMailer.account_already_taken(@current_user, requested_email).deliver_later
+        # avoid leaking information about whether an account with this email exists or not
         flash.notice = t('devise.registrations.update_needs_confirmation')
       else
         flash.alert = @current_user.errors.full_messages
@@ -27,5 +28,9 @@ module Users
     def update_email_params
       params.require(:user).permit(:email)
     end
+
+    def requested_email
+      update_email_params[:email]
+    end
   end
 end

app/mailers/user_mailer.rb

@@ -8,4 +8,12 @@ class UserMailer < ApplicationMailer
 
     mail(to: user.email, subject: @subject)
   end
+
+  def account_already_taken(user, requested_email)
+    @user = user
+    @requested_email = requested_email
+    @subject = "Changement d’adresse email"
+
+    mail(to: requested_email, subject: @subject)
+  end
 end

app/views/layouts/_account_dropdown.haml

@@ -26,11 +26,11 @@
           = link_to admin_procedures_path, class: "menu-item menu-link" do
             = image_tag "icons/switch-profile.svg"
             Passer en administrateur
-      %li
-        = link_to profil_path, class: "menu-item menu-link" do
-          = image_tag "icons/switch-profile.svg"
-          Voir mon profil
 
+    %li
+      = link_to profil_path, class: "menu-item menu-link" do
+        = image_tag "icons/switch-profile.svg"
+        Voir mon profil
     %li
       = link_to destroy_user_session_path, method: :delete, class: "menu-item menu-link" do
         = image_tag "icons/sign-out.svg"

app/views/root/patron.html.haml

@@ -157,6 +157,11 @@
         Titre de la carte mise en avant
       %p Et voici le contenu de la carte
 
+    .card.warning
+      .card-title
+        Titre de la carte d’avertissement
+      %p Et voici le contenu de la carte
+
     .card.feedback
       .card-title
         Titre de la carte pour demander un avis

app/views/user_mailer/account_already_taken.haml

@@ -0,0 +1,20 @@
+- content_for(:title, @subject)
+
+%p
+  Bonjour,
+
+%p
+  L’utilisateur « #{@user.email} » a demandé le changement de son adresse vers « #{@requested_email} ».
+
+%p
+  Malheureusement, votre compte « #{@requested_email} » existe déjà. Nous ne pouvons pas fusionner automatiquement vos comptes.
+
+%p
+  %strong Nous ne pouvons donc pas effectuer le changement d’adresse email.
+
+%p
+  Si vous n'êtes pas à l’origine de cette demande, vous pouvez ignorer ce message. Et si vous avez besoin d’assistance, n’hésitez pas à nous contacter à
+  = succeed '.' do
+    = mail_to CONTACT_EMAIL
+
+= render partial: "layouts/mailers/signature"

app/views/users/profil/show.html.haml

@@ -7,12 +7,16 @@
 
   .card
     .card-title Coordonnées
-    %p Votre email est actuellement #{current_user.email}
+    %p
+      Votre email est actuellement
+      %span.email-address= current_user.email
     - if current_user.unconfirmed_email.present?
-      %p
+      .card.warning
-        Un email a été envoyé à #{current_user.unconfirmed_email}.
+        .card-title
-        %br
+          Changement en attente :
-        Merci de vérifier vos emails et de cliquer sur le lien d’activation pour finaliser la validation de votre nouvelle adresse.
+          %span.email-address= current_user.unconfirmed_email
+        %p
+          Pour finaliser votre changement d’adresse, vérifiez vos emails et cliquez sur le lien de confirmation.
 
     = form_for @current_user, url: update_email_path, method: :patch, html: { class: 'form' } do |f|
       = f.email_field :email, value: nil, placeholder: 'Nouvelle adresse email', required: true

config/locales/devise.fr.yml

@@ -42,7 +42,7 @@ fr:
       signed_up_but_inactive: "Vous êtes bien enregistré. Vous ne pouvez cependant pas vous connecter car votre compte n'est pas encore activé."
       signed_up_but_locked: "Vous êtes bien enregistré. Vous ne pouvez cependant pas vous connecter car votre compte est verrouillé."
       signed_up_but_unconfirmed: "Nous vous avons envoyé un email contenant un lien d'activation. Ouvrez ce lien pour activer votre compte."
-      update_needs_confirmation: "Votre compte a bien été mis à jour mais nous devons vérifier votre nouvelle adresse email. Merci de vérifier vos emails et de cliquer sur le lien d’activation pour finaliser la validation de votre nouvelle adresse."
+      update_needs_confirmation: "Vous devez confirmer votre nouvelle adresse email. Vérifiez vos emails, et cliquez sur le lien de confirmation pour confirmer votre changement d’adresse."
       updated: "Votre compte a été modifié avec succès."
     sessions:
       signed_in: "Connecté."

spec/controllers/users/profil_controller_spec.rb

@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 describe Users::ProfilController, type: :controller do
+  include ActiveJob::TestHelper
+
   let(:user) { create(:user) }
 
   before { sign_in(user) }
@@ -34,13 +36,17 @@ describe Users::ProfilController, type: :controller do
     end
 
     context 'when the mail is already taken' do
-      let!(:user2) { create(:user) }
+      let(:existing_user) { create(:user) }
 
       before do
-        patch :update_email, params: { user: { email: user2.email } }
+        perform_enqueued_jobs do
+          patch :update_email, params: { user: { email: existing_user.email } }
+        end
         user.reload
       end
 
+      it { expect(user.unconfirmed_email).to be_nil }
+      it { expect(ActionMailer::Base.deliveries.last.to).to eq([existing_user.email]) }
       it { expect(response).to redirect_to(profil_path) }
       it { expect(flash.notice).to eq(I18n.t('devise.registrations.update_needs_confirmation')) }
     end

spec/features/users/change_email_spec.rb

@@ -19,14 +19,15 @@ feature 'Changing an email' do
       click_button 'Changer mon adresse'
     end
 
-    user.reload
+    expect(page).to have_content(I18n.t('devise.registrations.update_needs_confirmation'))
-    expect(user.email).to eq(old_email)
+    expect(page).to have_content(old_email)
-    expect(user.unconfirmed_email).to eq(new_email)
+    expect(page).to have_content(new_email)
 
     click_confirmation_link_for(new_email)
 
-    user.reload
+    expect(page).to have_content(I18n.t('devise.confirmations.confirmed'))
-    expect(user.email).to eq(new_email)
+    expect(page).not_to have_content(old_email)
-    expect(user.unconfirmed_email).to be_nil
+    expect(page).to have_content(new_email)
+    expect(user.reload.email).to eq(new_email)
   end
 end

spec/mailers/previews/user_mailer_preview.rb

@@ -3,6 +3,10 @@ class UserMailerPreview < ActionMailer::Preview
     UserMailer.new_account_warning(user)
   end
 
+  def account_already_taken
+    UserMailer.account_already_taken(user, 'dircab@territoires.gouv.fr')
+  end
+
   private
 
   def user

spec/mailers/user_mailer_spec.rb

@@ -0,0 +1,21 @@
+require "rails_helper"
+
+RSpec.describe UserMailer, type: :mailer do
+  let(:user) { build(:user) }
+
+  describe '.new_account_warning' do
+    subject { described_class.new_account_warning(user) }
+
+    it { expect(subject.to).to eq([user.email]) }
+    it { expect(subject.body).to include(user.email) }
+  end
+
+  describe '.account_already_taken' do
+    let(:requested_email) { 'new@exemple.fr' }
+
+    subject { described_class.account_already_taken(user, requested_email) }
+
+    it { expect(subject.to).to eq([requested_email]) }
+    it { expect(subject.body).to include(requested_email) }
+  end
+end

spec/models/dossier_spec.rb

@@ -1,4 +1,4 @@
-require 'spec_helper'
+require 'rails_helper'
 
 describe Dossier do
   include ActiveJob::TestHelper
@@ -420,7 +420,6 @@ describe Dossier do
 
     it "send an email when the dossier is created for the very first time" do
       dossier = nil
-      ActiveJob::Base.queue_adapter = :test
       expect do
         perform_enqueued_jobs do
           dossier = Dossier.create(procedure: procedure, state: Dossier.states.fetch(:brouillon), user: user)