modifications après dernière review

app/controllers/france_connect/particulier_controller.rb

@@ -4,7 +4,7 @@ class FranceConnect::ParticulierController < ApplicationController
   before_action :redirect_to_login_if_fc_aborted, only: [:callback]
   before_action :securely_retrieve_fci, only: [:merge, :merge_with_existing_account, :merge_with_new_account, :resend_and_renew_merge_confirmation, :associate_user]
   before_action :securely_retrieve_fci_from_email_merge_token, only: [:mail_merge_with_existing_account]
-  before_action :set_user_by_confirmation_token, only: [:resend_confirmation, :post_resend_confirmation, :confirm_email, :connect_france_connect_particulier_redirect]
+  before_action :set_user_by_confirmation_token, only: [:confirm_email]
 
   def login
     if FranceConnectService.enabled?
@@ -50,14 +50,15 @@ class FranceConnect::ParticulierController < ApplicationController
 
   def associate_user
     email = use_fc_email? ? @fci.email_france_connect : params[:alternative_email]
+
     @fci.associate_user!(email)
     user = @fci.user
 
+    @fci.send_custom_confirmation_instructions(user)
     @fci.delete_merge_token!
     sign_only(user)
 
-    destination_path = destination_path(user)
+    render :confirmation_sent, locals: { email:, destination_path: destination_path(user) }
-    render :confirmation_sent, locals: { email:, destination_path: }
   rescue ActiveRecord::RecordInvalid => e
     if e.record.errors.where(:email, :taken)
       redirect_to new_user_session_path, alert: t('errors.messages.france_connect.email_taken', reset_link: new_user_password_path)
@@ -134,28 +135,20 @@ class FranceConnect::ParticulierController < ApplicationController
   end
 
   def confirm_email
-    if @user.confirmation_sent_at && 2.days.ago < @user.confirmation_sent_at
+    if @user.confirmation_sent_at && @user.confirmation_sent_at > 2.days.ago
       @user.update(email_verified_at: Time.zone.now, confirmation_token: nil)
       @user.after_confirmation
-      redirect_to stored_location_for(@user) || root_path(@user), notice: 'Votre email est bien vérifié'
+      redirect_to destination_path(@user), notice: I18n.t('france_connect.particulier.flash.email_confirmed')
-    else
+      return
-      redirect_to france_connect_resend_confirmation_path(token: @user.confirmation_token), alert: "Lien de confirmation expiré. Un nouveau lien de confirmation a été envoyé."
     end
-  end
 
-  def resend_confirmation
+    fci = FranceConnectInformation.find_by(user: @user)
-    if @user.email_verified_at
-      redirect_to root_path, alert: 'Email déjà confirmé.'
-    end
-  end
 
-  def post_resend_confirmation
+    if fci
-    if !@user.email_verified_at
-      fci = FranceConnectInformation.find_by(user: @user)
       fci.send_custom_confirmation_instructions(@user)
-      redirect_to root_path(@user), notice: "Un nouveau lien de confirmation vous a été envoyé par mail."
+      redirect_to root_path, notice: I18n.t('france_connect.particulier.flash.confirmation_mail_resent')
     else
-      redirect_to root_path(@user), alert: "Adresse email non trouvée ou déjà confirmée."
+      redirect_to root_path, alert: I18n.t('france_connect.particulier.flash.confirmation_mail_resent_error')
     end
   end
 
@@ -165,12 +158,12 @@ class FranceConnect::ParticulierController < ApplicationController
     @user = User.find_by(confirmation_token: params[:token])
 
     if @user.nil?
-      redirect_to root_path, alert: 'Utilisateur non trouvé' and return
+      redirect_to root_path, alert: I18n.t('france_connect.particulier.flash.user_not_found') and return
     end
 
     if user_signed_in? && current_user != @user
       sign_out current_user
-      redirect_to new_user_session_path, alert: "Veuillez vous connecter avec le compte associé à ce lien de confirmation."
+      redirect_to new_user_session_path, alert: I18n.t('france_connect.particulier.flash.redirect_new_user_session')
     end
   end
 
@@ -187,7 +180,7 @@ class FranceConnect::ParticulierController < ApplicationController
     @fci = FranceConnectInformation.find_by(email_merge_token: email_merge_token_params)
 
     if @fci.nil? || !@fci.valid_for_email_merge?
-      flash.alert = t('france_connect.particulier.flash.merger_token_expired', application_name: Current.application_name)
+      flash.alert = I18n.t('france_connect.particulier.flash.merger_token_expired', application_name: Current.application_name)
 
       redirect_to root_path
     else
@@ -199,7 +192,7 @@ class FranceConnect::ParticulierController < ApplicationController
     @fci = FranceConnectInformation.find_by(merge_token: merge_token_params)
 
     if @fci.nil? || !@fci.valid_for_merge?
-      flash.alert = t('france_connect.particulier.flash.merger_token_expired', application_name: Current.application_name)
+      flash.alert = I18n.t('france_connect.particulier.flash.merger_token_expired', application_name: Current.application_name)
 
       redirect_to root_path
     end
@@ -220,7 +213,7 @@ class FranceConnect::ParticulierController < ApplicationController
 
     user.update_attribute('loged_in_with_france_connect', User.loged_in_with_france_connects.fetch(:particulier))
 
-    redirect_to stored_location_for(current_user) || root_path(current_user)
+    redirect_to destination_path(current_user)
   end
 
   def redirect_france_connect_error_connection

app/models/france_connect_information.rb

@@ -15,7 +15,6 @@ class FranceConnectInformation < ApplicationRecord
         password: Devise.friendly_token[0, 20],
         confirmed_at: Time.zone.now
       )
-      send_custom_confirmation_instructions(user)
     rescue ActiveRecord::RecordNotUnique
       # ignore this exception because we check before if user is nil.
       # exception can be raised in race conditions, when FranceConnect calls callback 2 times.

app/views/france_connect/particulier/resend_confirmation.html.haml

@@ -1,11 +0,0 @@
-.fr-container
-  %h1.text-center.mt-1 Renvoyer le lien de confirmation
-
-  %p Bonjour #{@user.email}
-
-  %p Cliquez sur le bouton ci-dessous pour recevoir un nouveau lien de confirmation à votre adresse email.
-
-  = form_with url: france_connect_post_resend_confirmation_path, method: :post, local: true do |form|
-    = form.hidden_field :token, value: @user.confirmation_token
-    .form-group
-      = form.submit "Renvoyer le lien de confirmation", class: 'fr-btn'

config/locales/en.yml

@@ -914,6 +914,11 @@ en:
         link_confirm_by_email: Confirm by receiving an email
       flash:
           confirmation_mail_sent: "An email with the confirmation link has been sent, please click on the link."
+          confirmation_mail_resent: "Confirmation link expired. A new link has been sent by email."
+          confirmation_mail_resent_error: "An unexpected error has occurred. Please contact support if the problem persists."
+          redirect_new_user_session: "You have been disconnected from your previous account. Please click on the confirmation link again."
+          email_confirmed: "Your email is confirmed"
+          user_not_found: "User not found"
           invalid_password: "The password is not correct."
           connection_done: "The accounts for FranceConnect and %{application_name} are now merged."
           connection_done_verify_email: "The FranceConnect and %{application_name} accounts are now merged. Please confirm your email by clicking on the link you received in your email."

config/locales/fr.yml

@@ -969,7 +969,12 @@ fr:
         link_confirm_by_email: Confirmer mon compte par email
       flash:
         confirmation_mail_sent: "Nous venons de vous envoyer le mail de confirmation, veuillez cliquer sur le lien contenu dans ce mail pour fusionner vos comptes"
-        invalid_password: "Mauvais mot de passe"
+        confirmation_mail_resent: "Le lien de confirmation a expiré. Un nouveau lien de confirmation vous a été envoyé par email."
+        confirmation_mail_resent_error: "Une erreur inattendue est survenue. Veuillez contacter le support si le problème persiste."
+        redirect_new_user_session: "Vous avez été déconnecté de votre précédent compte. Veuillez cliquer à nouveau sur le lien de confirmation."
+        email_confirmed: 'Votre email est bien vérifié'
+        user_not_found: 'Utilisateur non trouvé'
+        invalid_password: "Mot de passe incorrect"
         connection_done: "Les comptes FranceConnect et %{application_name} sont à présent fusionnés"
         connection_done_verify_email: "Les comptes FranceConnect et %{application_name} sont maintenant fusionnés. Veuillez confirmer votre e-mail en cliquant sur le lien que vous avez reçu par mail"
         merger_token_expired: "Le délai pour fusionner les comptes FranceConnect et %{application_name} est expiré. Veuillez recommencer la procédure pour vous fusionner les comptes."

config/routes.rb

@@ -191,12 +191,10 @@ Rails.application.routes.draw do
     get 'particulier/merge/:merge_token' => 'particulier#merge', as: :particulier_merge
     get 'particulier/mail_merge_with_existing_account/:email_merge_token' => 'particulier#mail_merge_with_existing_account', as: :particulier_mail_merge_with_existing_account
     get 'confirm_email/:token', to: 'particulier#confirm_email', as: :confirm_email
-    get 'resend_confirmation', to: 'particulier#resend_confirmation', as: 'resend_confirmation'
-    post 'post_resend_confirmation', to: 'particulier#post_resend_confirmation', as: 'post_resend_confirmation'
     post 'particulier/resend_and_renew_merge_confirmation' => 'particulier#resend_and_renew_merge_confirmation', as: :particulier_resend_and_renew_merge_confirmation
     post 'particulier/merge_with_existing_account' => 'particulier#merge_with_existing_account'
     post 'particulier/merge_with_new_account' => 'particulier#merge_with_new_account'
-    post 'particulier/associate_user', to: 'particulier#associate_user', as: :particulier_associate_user
+    post 'particulier/associate_user'
   end
 
   namespace :agent_connect do

spec/controllers/france_connect/particulier_controller_spec.rb

@@ -162,6 +162,7 @@ describe FranceConnect::ParticulierController, type: :controller do
         allow(fci).to receive(:associate_user!)
         allow(fci).to receive(:user).and_return(user)
         allow(fci).to receive(:delete_merge_token!)
+        allow(fci).to receive(:send_custom_confirmation_instructions)
         allow(controller).to receive(:use_fc_email?).and_return(true)
         allow(controller).to receive(:sign_only)
         allow(controller).to receive(:destination_path).and_return(destination_path)
@@ -169,18 +170,50 @@ describe FranceConnect::ParticulierController, type: :controller do
 
       subject { post :associate_user, params: { merge_token: merge_token, use_france_connect_email: true } }
 
-      it 'renders the confirmation_sent template' do
+      context 'when association is successful' do
-        subject
+        it 'renders the confirmation_sent template' do
-        expect(response).to render_template(:confirmation_sent)
+          subject
+          expect(response).to render_template(:confirmation_sent)
+        end
+
+        it 'performs all expected steps' do
+          expect(fci).to receive(:associate_user!).with(email)
+          expect(fci).to receive(:send_custom_confirmation_instructions).with(user)
+          expect(fci).to receive(:delete_merge_token!)
+          expect(controller).to receive(:sign_only).with(user)
+          expect(controller).to receive(:render).with(:confirmation_sent, locals: { email: email, destination_path: destination_path })
+
+          subject
+        end
       end
 
-      it 'performs all expected steps' do
+      context 'when association fails due to taken email' do
-        expect(fci).to receive(:associate_user!).with(email)
+        before do
-        expect(fci).to receive(:delete_merge_token!)
+          allow(fci).to receive(:associate_user!).and_raise(ActiveRecord::RecordInvalid.new(User.new))
-        expect(controller).to receive(:sign_only).with(user)
+          allow_any_instance_of(User).to receive_message_chain(:errors, :where).and_return(['Some error'])
-        expect(controller).to receive(:render).with(:confirmation_sent, locals: { email: email, destination_path: destination_path })
+        end
 
-        subject
+        it 'redirects to new user session path with taken email alert' do
+          subject
+          expect(response).to redirect_to(new_user_session_path)
+          expect(flash[:alert]).to eq(I18n.t('errors.messages.france_connect.email_taken', reset_link: new_user_password_path))
+        end
+      end
+
+      context 'when association fails due to unknown error' do
+        let(:user) { User.new }
+        let(:error) { ActiveRecord::RecordInvalid.new(user) }
+
+        before do
+          allow(fci).to receive(:associate_user!).and_raise(error)
+          allow(user.errors).to receive(:where).with(:email, :taken).and_return(nil)
+        end
+
+        it 'redirects to new user session path with unknown error alert' do
+          subject
+          expect(response).to redirect_to(new_user_session_path)
+          expect(flash[:alert]).to eq(I18n.t('errors.messages.france_connect.unknown_error'))
+        end
       end
     end
 
@@ -217,24 +250,34 @@ describe FranceConnect::ParticulierController, type: :controller do
       let(:email) { 'user@example.com' }
       let(:user) { instance_double('User', id: 1) }
       let(:destination_path) { '/' }
+      let(:merge_token) { 'sample_merge_token' }
 
       before do
-        allow(FranceConnectInformation).to receive(:find_by).with(merge_token: merge_token).and_return(fci)
+        allow(controller).to receive(:securely_retrieve_fci) do
-        allow(fci).to receive(:valid_for_merge?).and_return(true)
+          controller.instance_variable_set(:@fci, fci)
+        end
         allow(fci).to receive(:email_france_connect).and_return(email)
         allow(fci).to receive(:associate_user!)
         allow(fci).to receive(:user).and_return(user)
         allow(fci).to receive(:delete_merge_token!)
+        allow(fci).to receive(:send_custom_confirmation_instructions)
         allow(controller).to receive(:use_fc_email?).and_return(true)
         allow(controller).to receive(:sign_only)
         allow(controller).to receive(:destination_path).and_return(destination_path)
       end
 
+      subject { post :associate_user, params: { merge_token: merge_token, use_france_connect_email: true } }
+
       it 'calls associate_user! with the correct email' do
         expect(fci).to receive(:associate_user!).with(email)
         subject
       end
 
+      it 'sends custom confirmation instructions' do
+        expect(fci).to receive(:send_custom_confirmation_instructions).with(user)
+        subject
+      end
+
       it 'deletes the merge token' do
         expect(fci).to receive(:delete_merge_token!)
         subject
@@ -327,137 +370,98 @@ describe FranceConnect::ParticulierController, type: :controller do
     end
 
     context 'when the confirmation token is expired' do
+      let(:expired_user_confirmation) do
+        create(:user, confirmation_token: 'expired_token', confirmation_sent_at: 3.days.ago)
+      end
+
       before do
-        sign_in(expired_user_confirmation)
+        allow(User).to receive(:find_by).with(confirmation_token: 'expired_token').and_return(expired_user_confirmation)
-        get :confirm_email, params: { token: expired_user_confirmation.confirmation_token }
+        allow(controller).to receive(:user_signed_in?).and_return(false)
+        allow(FranceConnectInformation).to receive(:find_by).with(user: expired_user_confirmation).and_return(nil)
       end
 
-      it 'redirects to the resend confirmation path with an alert' do
+      it 'redirects to root path with an alert when FranceConnectInformation is not found' do
-        expect(response).to redirect_to(france_connect_resend_confirmation_path(token: expired_user_confirmation.confirmation_token))
+        get :confirm_email, params: { token: 'expired_token' }
-        expect(flash[:alert]).to eq('Lien de confirmation expiré. Un nouveau lien de confirmation a été envoyé.')
-      end
-    end
 
-    context 'GET #resend_confirmation' do
+        expect(response).to redirect_to(root_path)
-      let(:user) { create(:user, email: 'test@example.com', email_verified_at: nil, confirmation_token: 'valid_token') }
+        expect(flash[:alert]).to eq(I18n.t('france_connect.particulier.flash.confirmation_mail_resent_error'))
-
-      context 'when the token is valid' do
-        it 'assigns @user and renders the form' do
-          get :resend_confirmation, params: { token: user.confirmation_token }
-          expect(assigns(:user)).to eq(user)
-          expect(response).to render_template(:resend_confirmation)
-        end
       end
 
-      context 'when the email is already confirmed' do
+      context 'when FranceConnectInformation exists' do
-        let(:confirmed_user) { create(:user, email: 'confirmed@example.com', email_verified_at: Time.zone.now, confirmation_token: nil) }
+        let(:france_connect_information) { instance_double(FranceConnectInformation) }
 
         before do
-          allow(controller).to receive(:set_user_by_confirmation_token) do
+          allow(FranceConnectInformation).to receive(:find_by).with(user: expired_user_confirmation).and_return(france_connect_information)
-            controller.instance_variable_set(:@user, confirmed_user)
+          allow(france_connect_information).to receive(:send_custom_confirmation_instructions)
-          end
         end
 
-        it 'redirects to root path with an alert about already confirmed email' do
+        it 'resends the confirmation email and redirects to root path with a notice' do
-          get :resend_confirmation, params: { email: confirmed_user.email }
+          expect(france_connect_information).to receive(:send_custom_confirmation_instructions).with(expired_user_confirmation)
+
+          get :confirm_email, params: { token: 'expired_token' }
+
           expect(response).to redirect_to(root_path)
-          expect(flash[:alert]).to eq('Email déjà confirmé.')
+          expect(flash[:notice]).to eq(I18n.t('france_connect.particulier.flash.confirmation_mail_resent'))
-        end
-      end
-
-      context 'when the token is invalid' do
-        before do
-          get :resend_confirmation, params: { token: 'invalid_token' }
-        end
-
-        it 'sets @user to nil and redirects to root path with an alert' do
-          expect(assigns(:user)).to be_nil
-          expect(response).to redirect_to(root_path)
-          expect(flash[:alert]).to eq('Utilisateur non trouvé')
-        end
-      end
-
-      context 'when the user is not found' do
-        before do
-          allow(User).to receive(:find_by).with(confirmation_token: 'invalid_token').and_return(nil)
-        end
-
-        it 'redirects to root_path with an alert' do
-          get :resend_confirmation, params: { token: 'invalid_token' }
-          expect(response).to redirect_to(root_path)
-          expect(flash[:alert]).to eq('Utilisateur non trouvé')
-        end
-      end
-
-      context 'when a different user is signed in' do
-        let(:current_user) { create(:user) }
-        let(:target_user) { create(:user, confirmation_token: 'valid_token') }
-
-        before do
-          sign_in current_user
-          allow(User).to receive(:find_by).with(confirmation_token: 'valid_token').and_return(target_user)
-        end
-
-        it 'signs out the current user and redirects to new_user_session_path with an alert' do
-          get :resend_confirmation, params: { token: 'valid_token' }
-          expect(controller.current_user).to be_nil
-          expect(response).to redirect_to(new_user_session_path)
-          expect(flash[:alert]).to eq("Veuillez vous connecter avec le compte associé à ce lien de confirmation.")
         end
       end
     end
 
-    context 'POST #post_resend_confirmation' do
+    context 'when a different user is signed in' do
-      let(:user) { create(:user, email: 'test@example.com', email_verified_at: nil, confirmation_token: 'valid_token') }
+      let(:expired_user_confirmation) do
-      let(:fci) { create(:france_connect_information, user: user) }
+        create(:user, confirmation_token: 'expired_token', confirmation_sent_at: 3.days.ago)
+      end
+      let(:current_user) { create(:user) }
 
       before do
-        allow(FranceConnectInformation).to receive(:find_by).with(user: user).and_return(fci)
+        allow(User).to receive(:find_by).with(confirmation_token: 'expired_token').and_return(expired_user_confirmation)
-        allow(controller).to receive(:set_user_by_confirmation_token).and_call_original
+        allow(controller).to receive(:user_signed_in?).and_return(true)
+        allow(controller).to receive(:current_user).and_return(current_user)
       end
 
-      context 'when the user exists and email is not verified' do
+      it 'signs out the current user and redirects to sign in path' do
-        before do
+        expect(controller).to receive(:sign_out).with(current_user)
-          allow(controller).to receive(:set_user_by_confirmation_token) do
-            controller.instance_variable_set(:@user, user)
-          end
-        end
 
-        it 'sends custom confirmation instructions and redirects with notice' do
+        get :confirm_email, params: { token: 'expired_token' }
-          expect(fci).to receive(:send_custom_confirmation_instructions).with(user)
+
-          post :post_resend_confirmation, params: { token: user.confirmation_token }
+        expect(response).to redirect_to(new_user_session_path)
-          expect(response).to redirect_to(root_path)
+        expect(flash[:alert]).to eq(I18n.t('france_connect.particulier.flash.redirect_new_user_session'))
-          expect(flash[:notice]).to eq('Un nouveau lien de confirmation vous a été envoyé par mail.')
-        end
       end
 
-      context 'when the user does not exist' do
+      it 'does not process the confirmation' do
-        before do
+        expect(FranceConnectInformation).not_to receive(:find_by)
-          allow(User).to receive(:find_by).with(confirmation_token: 'non_existent_token').and_return(nil)
+        expect_any_instance_of(FranceConnectInformation).not_to receive(:send_custom_confirmation_instructions)
-          allow(controller).to receive(:set_user_by_confirmation_token).and_call_original
-        end
 
-        it 'redirects with alert for non-existent token' do
+        get :confirm_email, params: { token: 'expired_token' }
-          post :post_resend_confirmation, params: { token: 'non_existent_token' }
-          expect(response).to redirect_to(root_path)
-          expect(flash[:alert]).to eq('Utilisateur non trouvé')
-        end
       end
+    end
+  end
 
-      context 'when the email is already verified' do
+  describe '#set_user_by_confirmation_token' do
-        let(:verified_user) { create(:user, email: 'verified@example.com', email_verified_at: Time.zone.now, confirmation_token: 'verified_token') }
+    let(:current_user) { create(:user) }
+    let(:confirmation_user) { create(:user, confirmation_token: 'valid_token') }
 
-        before do
+    before do
-          allow(controller).to receive(:set_user_by_confirmation_token) do
+      sign_in current_user
-            controller.instance_variable_set(:@user, verified_user)
+      allow(User).to receive(:find_by).with(confirmation_token: 'valid_token').and_return(confirmation_user)
-          end
+    end
-        end
 
-        it 'redirects with alert for already verified email' do
+    it 'signs out current user and redirects to new session path when users do not match' do
-          post :post_resend_confirmation, params: { token: verified_user.confirmation_token }
+      expect(controller).to receive(:sign_out).with(current_user)
-          expect(response).to redirect_to(root_path)
+
-          expect(flash[:alert]).to eq('Adresse email non trouvée ou déjà confirmée.')
+      get :confirm_email, params: { token: 'valid_token' }
-        end
+
+      expect(response).to redirect_to(new_user_session_path)
+      expect(flash[:alert]).to eq(I18n.t('france_connect.particulier.flash.redirect_new_user_session'))
+    end
+
+    context 'when user is not found' do
+      it 'redirects to root path with user not found alert' do
+        allow(User).to receive(:find_by).with(confirmation_token: 'invalid_token').and_return(nil)
+
+        get :confirm_email, params: { token: 'invalid_token' }
+
+        expect(response).to redirect_to(root_path)
+        expect(flash[:alert]).to eq(I18n.t('france_connect.particulier.flash.user_not_found'))
       end
     end
   end

spec/models/france_connect_information_spec.rb

@@ -27,11 +27,6 @@ describe FranceConnectInformation, type: :model do
         expect(user.confirmed_at).to be_present
       end
 
-      it 'sends custom confirmation instructions' do
-        expect(UserMailer).to receive(:custom_confirmation_instructions).and_call_original
-        subject
-      end
-
       it 'associates the user with the FranceConnectInformation' do
         subject
         expect(fci.reload.user.email).to eq(email.downcase)