diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index aa629b365..a1f95b292 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -30,7 +30,7 @@ class FranceConnect::ParticulierController < ApplicationController private def redirect_to_login_if_fc_aborted - if params[:code].empty? + if params[:code].blank? redirect_to new_user_session_path end end diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb index 4fcd54b44..cc69b4555 100644 --- a/app/controllers/users/registrations_controller.rb +++ b/app/controllers/users/registrations_controller.rb @@ -26,12 +26,10 @@ class Users::RegistrationsController < Devise::RegistrationsController if existing_user.present? if existing_user.confirmed? UserMailer.new_account_warning(existing_user).deliver_later - flash.notice = t('devise.registrations.signed_up_but_unconfirmed') - return redirect_to root_path else existing_user.resend_confirmation_instructions - return redirect_to after_inactive_sign_up_path_for(existing_user) end + return redirect_to after_inactive_sign_up_path_for(existing_user) end super diff --git a/app/mailers/notification_mailer.rb b/app/mailers/notification_mailer.rb index e9e88b4c1..2822d405a 100644 --- a/app/mailers/notification_mailer.rb +++ b/app/mailers/notification_mailer.rb @@ -45,6 +45,7 @@ class NotificationMailer < ApplicationMailer @logo_url = attachments[logo_filename].url rescue StandardError => e # A problem occured when reading logo, maybe the logo is missing and we should clean the procedure to remove logo reference ? + Raven.extra_context(procedure_id: dossier.procedure.id) Raven.capture_exception(e) end end diff --git a/spec/controllers/france_connect/particulier_controller_spec.rb b/spec/controllers/france_connect/particulier_controller_spec.rb index 498087145..7e4a00f15 100644 --- a/spec/controllers/france_connect/particulier_controller_spec.rb +++ b/spec/controllers/france_connect/particulier_controller_spec.rb @@ -25,6 +25,12 @@ describe FranceConnect::ParticulierController, type: :controller do subject { get :callback, params: { code: code } } + context 'when params are missing' do + subject { get :callback } + + it { is_expected.to redirect_to(new_user_session_path) } + end + context 'when param code is missing' do let(:code) { nil } diff --git a/spec/controllers/users/registrations_controller_spec.rb b/spec/controllers/users/registrations_controller_spec.rb index c5cbd2155..206b15211 100644 --- a/spec/controllers/users/registrations_controller_spec.rb +++ b/spec/controllers/users/registrations_controller_spec.rb @@ -74,9 +74,13 @@ describe Users::RegistrationsController, type: :controller do before { subject } - it { expect(response).to redirect_to(root_path) } - it { expect(flash.notice).to eq(I18n.t('devise.registrations.signed_up_but_unconfirmed')) } - it { expect(UserMailer).to have_received(:new_account_warning) } + it 'sends an email to the user, stating that the account already exists' do + expect(UserMailer).to have_received(:new_account_warning) + end + + it 'avoids leaking information about the account existence, by redirecting to the same page than normal signup' do + expect(response).to redirect_to(new_user_confirmation_path(user: { email: user[:email] })) + end end context 'and the user is not confirmed' do @@ -87,8 +91,13 @@ describe Users::RegistrationsController, type: :controller do subject end - it { expect(response).to redirect_to(new_user_confirmation_path(user: { email: user[:email] })) } - it { expect(UserMailer).not_to have_received(:new_account_warning) } + it 'does not send a warning email' do + expect(UserMailer).not_to have_received(:new_account_warning) + end + + it 'avoids leaking information about the account existence, by redirecting to the same page than normal signup' do + expect(response).to redirect_to(new_user_confirmation_path(user: { email: user[:email] })) + end end end end