diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index 449153961..bcfa9b577 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -11,7 +11,10 @@ class FranceConnect::ParticulierController < ApplicationController def callback fci = FranceConnectService.find_or_retrieve_france_connect_information(params[:code]) - fci.associate_user! + + if fci.user.nil? + fci.associate_user! + end user = fci.user diff --git a/app/models/france_connect_information.rb b/app/models/france_connect_information.rb index d76ee60d8..1bd3220c1 100644 --- a/app/models/france_connect_information.rb +++ b/app/models/france_connect_information.rb @@ -21,21 +21,17 @@ class FranceConnectInformation < ApplicationRecord validates :france_connect_particulier_id, presence: true, allow_blank: false, allow_nil: false def associate_user! - user = User.find_by(email: email_france_connect.downcase) - - if user.nil? - begin - user = User.create!( - email: email_france_connect.downcase, - password: Devise.friendly_token[0, 20], - confirmed_at: Time.zone.now - ) - rescue ActiveRecord::RecordNotUnique - # ignore this exception because we check before is user is nil. - # exception can be raised in race conditions, when FranceConnect calls callback 2 times. - # At the 2nd call, user is nil but exception is raised at the creation of the user - # because the first call has already created a user - end + begin + user = User.create!( + email: email_france_connect.downcase, + password: Devise.friendly_token[0, 20], + confirmed_at: Time.zone.now + ) + rescue ActiveRecord::RecordNotUnique + # ignore this exception because we check before is user is nil. + # exception can be raised in race conditions, when FranceConnect calls callback 2 times. + # At the 2nd call, user is nil but exception is raised at the creation of the user + # because the first call has already created a user end update_attribute('user_id', user.id) diff --git a/spec/controllers/france_connect/particulier_controller_spec.rb b/spec/controllers/france_connect/particulier_controller_spec.rb index 5cce7c467..569d444e3 100644 --- a/spec/controllers/france_connect/particulier_controller_spec.rb +++ b/spec/controllers/france_connect/particulier_controller_spec.rb @@ -1,6 +1,6 @@ describe FranceConnect::ParticulierController, type: :controller do let(:birthdate) { '20150821' } - let(:email) { 'test@test.com' } + let(:email) { 'email_from_fc@test.com' } let(:user_info) do { @@ -49,31 +49,37 @@ describe FranceConnect::ParticulierController, type: :controller do .and_return(FranceConnectInformation.new(user_info)) end - context 'when france_connect_particulier_id exist in database' do - let!(:france_connect_information) { create(:france_connect_information, :with_user, user_info) } - let(:user) { france_connect_information.user } + context 'when france_connect_particulier_id exists in database' do + let!(:fci) { FranceConnectInformation.create!(user_info.merge(user_id: fc_user.id)) } - it { expect { subject }.not_to change { FranceConnectInformation.count } } + context 'and is linked to an user' do + let(:fc_user) { create(:user, email: 'associated_user@a.com') } - it do - subject - expect(user.reload.loged_in_with_france_connect).to eq(User.loged_in_with_france_connects.fetch(:particulier)) + it { expect { subject }.not_to change { FranceConnectInformation.count } } + + it 'signs in with the fci associated user' do + subject + expect(controller.current_user).to eq(fc_user) + expect(fc_user.reload.loged_in_with_france_connect).to eq(User.loged_in_with_france_connects.fetch(:particulier)) + end + + context 'and the user has a stored location' do + let(:stored_location) { '/plip/plop' } + before { controller.store_location_for(:user, stored_location) } + + it { is_expected.to redirect_to(stored_location) } + end end - context 'and the user has a stored location' do - let(:stored_location) { '/plip/plop' } - before { controller.store_location_for(:user, stored_location) } + context 'and is linked an instructeur' do + let(:fc_user) { create(:instructeur, email: 'another_email@a.com').user } - it { is_expected.to redirect_to(stored_location) } - end - - context 'and the user is also instructeur' do - let!(:instructeur) { create(:instructeur, email: email) } before { subject } - it { expect(response).to redirect_to(new_user_session_path) } - - it { expect(flash[:alert]).to be_present } + it do + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to be_present + end end end diff --git a/spec/models/france_connect_information_spec.rb b/spec/models/france_connect_information_spec.rb index 6e8a0822b..150b92fc3 100644 --- a/spec/models/france_connect_information_spec.rb +++ b/spec/models/france_connect_information_spec.rb @@ -18,15 +18,5 @@ describe FranceConnectInformation, type: :model do expect(fci.user.email).to eq(fci.email_france_connect) end end - - context 'when a user with same email (but who is not an instructeur) exist' do - let(:user) { create(:user) } - let(:fci) { build(:france_connect_information, email_france_connect: user.email) } - let(:subject) { fci.associate_user! } - - before { subject } - - it { expect(fci.user).to eq(user) } - end end end