Sign in with a user linked by france connect sub (openid)

instead of looking linked user by email because :

- follows FC recommendation to fetch ds account by openid
- the email is not a valid key as many user can share the same FCI email.

The following scenario is now working

A user A (email: 1@mail.com) uses FC to connect to DS
=> It is connected as 1@mail.com

Another user B (email: generic@mail.com) uses FC to connect
=> It is connected as generic@mail.com

The first user A change its FC email to generic@mail.com and connect to DS
=> It is still connected as 1@mail.com
This commit is contained in:
simon lehericey 2021-10-11 11:39:14 +02:00
parent 06dee2e023
commit 6826bf03b0
4 changed files with 40 additions and 45 deletions

View file

@ -11,7 +11,10 @@ class FranceConnect::ParticulierController < ApplicationController
def callback def callback
fci = FranceConnectService.find_or_retrieve_france_connect_information(params[:code]) fci = FranceConnectService.find_or_retrieve_france_connect_information(params[:code])
fci.associate_user!
if fci.user.nil?
fci.associate_user!
end
user = fci.user user = fci.user

View file

@ -21,21 +21,17 @@ class FranceConnectInformation < ApplicationRecord
validates :france_connect_particulier_id, presence: true, allow_blank: false, allow_nil: false validates :france_connect_particulier_id, presence: true, allow_blank: false, allow_nil: false
def associate_user! def associate_user!
user = User.find_by(email: email_france_connect.downcase) begin
user = User.create!(
if user.nil? email: email_france_connect.downcase,
begin password: Devise.friendly_token[0, 20],
user = User.create!( confirmed_at: Time.zone.now
email: email_france_connect.downcase, )
password: Devise.friendly_token[0, 20], rescue ActiveRecord::RecordNotUnique
confirmed_at: Time.zone.now # ignore this exception because we check before is user is nil.
) # exception can be raised in race conditions, when FranceConnect calls callback 2 times.
rescue ActiveRecord::RecordNotUnique # At the 2nd call, user is nil but exception is raised at the creation of the user
# ignore this exception because we check before is user is nil. # because the first call has already created a user
# 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
end end
update_attribute('user_id', user.id) update_attribute('user_id', user.id)

View file

@ -1,6 +1,6 @@
describe FranceConnect::ParticulierController, type: :controller do describe FranceConnect::ParticulierController, type: :controller do
let(:birthdate) { '20150821' } let(:birthdate) { '20150821' }
let(:email) { 'test@test.com' } let(:email) { 'email_from_fc@test.com' }
let(:user_info) do let(:user_info) do
{ {
@ -49,31 +49,37 @@ describe FranceConnect::ParticulierController, type: :controller do
.and_return(FranceConnectInformation.new(user_info)) .and_return(FranceConnectInformation.new(user_info))
end end
context 'when france_connect_particulier_id exist in database' do context 'when france_connect_particulier_id exists in database' do
let!(:france_connect_information) { create(:france_connect_information, :with_user, user_info) } let!(:fci) { FranceConnectInformation.create!(user_info.merge(user_id: fc_user.id)) }
let(:user) { france_connect_information.user }
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 it { expect { subject }.not_to change { FranceConnectInformation.count } }
subject
expect(user.reload.loged_in_with_france_connect).to eq(User.loged_in_with_france_connects.fetch(:particulier)) 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 end
context 'and the user has a stored location' do context 'and is linked an instructeur' do
let(:stored_location) { '/plip/plop' } let(:fc_user) { create(:instructeur, email: 'another_email@a.com').user }
before { controller.store_location_for(:user, stored_location) }
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 } before { subject }
it { expect(response).to redirect_to(new_user_session_path) } it do
expect(response).to redirect_to(new_user_session_path)
it { expect(flash[:alert]).to be_present } expect(flash[:alert]).to be_present
end
end end
end end

View file

@ -18,15 +18,5 @@ describe FranceConnectInformation, type: :model do
expect(fci.user.email).to eq(fci.email_france_connect) expect(fci.user.email).to eq(fci.email_france_connect)
end end
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
end end