diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index 6f9cbda83..6ffdd2455 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/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: } + render :confirmation_sent, locals: { email:, destination_path: destination_path(user) } 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é' - else - redirect_to france_connect_resend_confirmation_path(token: @user.confirmation_token), alert: "Lien de confirmation expiré. Un nouveau lien de confirmation a été envoyé." + redirect_to destination_path(@user), notice: I18n.t('france_connect.particulier.flash.email_confirmed') + return end - end - def resend_confirmation - if @user.email_verified_at - redirect_to root_path, alert: 'Email déjà confirmé.' - end - end + fci = FranceConnectInformation.find_by(user: @user) - def post_resend_confirmation - if !@user.email_verified_at - fci = FranceConnectInformation.find_by(user: @user) + if fci 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 diff --git a/app/models/france_connect_information.rb b/app/models/france_connect_information.rb index 63cb341dd..8761dd5d1 100644 --- a/app/models/france_connect_information.rb +++ b/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. diff --git a/app/views/france_connect/particulier/resend_confirmation.html.haml b/app/views/france_connect/particulier/resend_confirmation.html.haml deleted file mode 100644 index bc03ca5c7..000000000 --- a/app/views/france_connect/particulier/resend_confirmation.html.haml +++ /dev/null @@ -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' diff --git a/config/locales/en.yml b/config/locales/en.yml index c7341c757..c42e6453e 100644 --- a/config/locales/en.yml +++ b/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." diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 8fb2c88b2..2ce66e013 100644 --- a/config/locales/fr.yml +++ b/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." diff --git a/config/routes.rb b/config/routes.rb index 70495e4a7..0a82f4c72 100644 --- a/config/routes.rb +++ b/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 diff --git a/spec/controllers/france_connect/particulier_controller_spec.rb b/spec/controllers/france_connect/particulier_controller_spec.rb index 5eb8c0165..cf50c71f4 100644 --- a/spec/controllers/france_connect/particulier_controller_spec.rb +++ b/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 - subject - expect(response).to render_template(:confirmation_sent) + context 'when association is successful' do + it 'renders the confirmation_sent template' do + 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 - expect(fci).to receive(:associate_user!).with(email) - 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 }) + context 'when association fails due to taken email' do + before do + allow(fci).to receive(:associate_user!).and_raise(ActiveRecord::RecordInvalid.new(User.new)) + allow_any_instance_of(User).to receive_message_chain(:errors, :where).and_return(['Some error']) + 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(fci).to receive(:valid_for_merge?).and_return(true) + allow(controller).to receive(:securely_retrieve_fci) do + 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) - get :confirm_email, params: { token: expired_user_confirmation.confirmation_token } + allow(User).to receive(:find_by).with(confirmation_token: 'expired_token').and_return(expired_user_confirmation) + 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 - expect(response).to redirect_to(france_connect_resend_confirmation_path(token: expired_user_confirmation.confirmation_token)) - expect(flash[:alert]).to eq('Lien de confirmation expiré. Un nouveau lien de confirmation a été envoyé.') - end - end + it 'redirects to root path with an alert when FranceConnectInformation is not found' do + get :confirm_email, params: { token: 'expired_token' } - context 'GET #resend_confirmation' do - let(:user) { create(:user, email: 'test@example.com', email_verified_at: nil, confirmation_token: 'valid_token') } - - 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 + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to eq(I18n.t('france_connect.particulier.flash.confirmation_mail_resent_error')) end - context 'when the email is already confirmed' do - let(:confirmed_user) { create(:user, email: 'confirmed@example.com', email_verified_at: Time.zone.now, confirmation_token: nil) } + context 'when FranceConnectInformation exists' do + let(:france_connect_information) { instance_double(FranceConnectInformation) } before do - allow(controller).to receive(:set_user_by_confirmation_token) do - controller.instance_variable_set(:@user, confirmed_user) - end + allow(FranceConnectInformation).to receive(:find_by).with(user: expired_user_confirmation).and_return(france_connect_information) + allow(france_connect_information).to receive(:send_custom_confirmation_instructions) end - it 'redirects to root path with an alert about already confirmed email' do - get :resend_confirmation, params: { email: confirmed_user.email } + it 'resends the confirmation email and redirects to root path with a notice' do + 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é.') - 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.") + expect(flash[:notice]).to eq(I18n.t('france_connect.particulier.flash.confirmation_mail_resent')) end end end - context 'POST #post_resend_confirmation' do - let(:user) { create(:user, email: 'test@example.com', email_verified_at: nil, confirmation_token: 'valid_token') } - let(:fci) { create(:france_connect_information, user: user) } + context 'when a different user is signed in' do + let(:expired_user_confirmation) do + 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(controller).to receive(:set_user_by_confirmation_token).and_call_original + allow(User).to receive(:find_by).with(confirmation_token: 'expired_token').and_return(expired_user_confirmation) + 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 - before do - allow(controller).to receive(:set_user_by_confirmation_token) do - controller.instance_variable_set(:@user, user) - end - end + it 'signs out the current user and redirects to sign in path' do + expect(controller).to receive(:sign_out).with(current_user) - it 'sends custom confirmation instructions and redirects with notice' do - expect(fci).to receive(:send_custom_confirmation_instructions).with(user) - post :post_resend_confirmation, params: { token: user.confirmation_token } - expect(response).to redirect_to(root_path) - expect(flash[:notice]).to eq('Un nouveau lien de confirmation vous a été envoyé par mail.') - end + get :confirm_email, params: { token: 'expired_token' } + + 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 the user does not exist' do - before do - allow(User).to receive(:find_by).with(confirmation_token: 'non_existent_token').and_return(nil) - allow(controller).to receive(:set_user_by_confirmation_token).and_call_original - end + it 'does not process the confirmation' do + expect(FranceConnectInformation).not_to receive(:find_by) + expect_any_instance_of(FranceConnectInformation).not_to receive(:send_custom_confirmation_instructions) - it 'redirects with alert for non-existent token' do - 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 + get :confirm_email, params: { token: 'expired_token' } end + end + end - context 'when the email is already verified' do - let(:verified_user) { create(:user, email: 'verified@example.com', email_verified_at: Time.zone.now, confirmation_token: 'verified_token') } + describe '#set_user_by_confirmation_token' do + let(:current_user) { create(:user) } + let(:confirmation_user) { create(:user, confirmation_token: 'valid_token') } - before do - allow(controller).to receive(:set_user_by_confirmation_token) do - controller.instance_variable_set(:@user, verified_user) - end - end + before do + sign_in current_user + allow(User).to receive(:find_by).with(confirmation_token: 'valid_token').and_return(confirmation_user) + end - it 'redirects with alert for already verified email' do - post :post_resend_confirmation, params: { token: verified_user.confirmation_token } - expect(response).to redirect_to(root_path) - expect(flash[:alert]).to eq('Adresse email non trouvée ou déjà confirmée.') - end + it 'signs out current user and redirects to new session path when users do not match' do + expect(controller).to receive(:sign_out).with(current_user) + + get :confirm_email, params: { token: 'valid_token' } + + 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 diff --git a/spec/models/france_connect_information_spec.rb b/spec/models/france_connect_information_spec.rb index 53573c126..aaefe9da5 100644 --- a/spec/models/france_connect_information_spec.rb +++ b/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)