diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index e3ef69241..595a50e78 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -10,22 +10,17 @@ class FranceConnect::ParticulierController < ApplicationController end def callback - fetched_fci = FranceConnectService.retrieve_user_informations_particulier(params[:code]) + fci = FranceConnectService.find_or_retrieve_france_connect_information(params[:code]) + fci.associate_user! - fci = FranceConnectInformation - .find_by(france_connect_particulier_id: fetched_fci[:france_connect_particulier_id]) || - fetched_fci.tap(&:save) - - if fci.user.nil? - user = User.find_or_create_by!(email: fci.email_france_connect.downcase) do |new_user| - new_user.password = Devise.friendly_token[0, 20] - new_user.confirmed_at = Time.zone.now - end - - fci.update_attribute('user_id', user.id) + if fci.user && !fci.user.can_france_connect? + fci.destroy + redirect_to new_user_session_path, alert: t('errors.messages.france_connect.forbidden_html', reset_link: new_user_password_path) + return end connect_france_connect_particulier(fci.user) + rescue Rack::OAuth2::Client::Error => e Rails.logger.error e.message redirect_france_connect_error_connection diff --git a/app/controllers/super_admins_controller.rb b/app/controllers/super_admins_controller.rb index aa1c608c1..a297c70a4 100644 --- a/app/controllers/super_admins_controller.rb +++ b/app/controllers/super_admins_controller.rb @@ -25,7 +25,7 @@ class SuperAdminsController < ApplicationController if Rails.env.development? issuer += " (local)" - elsif staging? + elsif helpers.staging? issuer += " (dev)" end diff --git a/app/models/france_connect_information.rb b/app/models/france_connect_information.rb index 848a781ed..1bdf52131 100644 --- a/app/models/france_connect_information.rb +++ b/app/models/france_connect_information.rb @@ -18,4 +18,25 @@ class FranceConnectInformation < ApplicationRecord belongs_to :user, optional: true 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 + end + + update_attribute('user_id', user.id) + end end diff --git a/app/models/user.rb b/app/models/user.rb index d2a449528..7de9ff077 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -112,6 +112,7 @@ class User < ApplicationRecord if user.valid? if user.instructeur_id.nil? user.create_instructeur! + user.update(france_connect_information: nil) end user.instructeur.administrateurs << administrateurs @@ -125,6 +126,7 @@ class User < ApplicationRecord if user.valid? && user.administrateur_id.nil? user.create_administrateur! + user.update(france_connect_information: nil) end user @@ -152,6 +154,18 @@ class User < ApplicationRecord last_sign_in_at.present? end + def administrateur? + administrateur_id.present? + end + + def instructeur? + instructeur_id.present? + end + + def can_france_connect? + !administrateur? && !instructeur? + end + def can_be_deleted? administrateur.nil? && instructeur.nil? && dossiers.with_discarded.state_instruction_commencee.empty? end diff --git a/app/services/france_connect_service.rb b/app/services/france_connect_service.rb index 70e8785b8..31b2491c4 100644 --- a/app/services/france_connect_service.rb +++ b/app/services/france_connect_service.rb @@ -14,6 +14,13 @@ class FranceConnectService ) end + def self.find_or_retrieve_france_connect_information(code) + fetched_fci = FranceConnectService.retrieve_user_informations_particulier(code) + FranceConnectInformation.find_by(france_connect_particulier_id: fetched_fci[:france_connect_particulier_id]) || fetched_fci + end + + private + def self.retrieve_user_informations_particulier(code) client = FranceConnectParticulierClient.new(code) diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 5409206ad..397819ef1 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -114,6 +114,7 @@ fr: # etablissement_fail: 'Désolé, nous n’avons pas réussi à enregistrer l’établissement correspondant à ce numéro SIRET' france_connect: connexion: "Erreur lors de la connexion à France Connect." + forbidden_html: "Seul-e-s les usagers peuvent se connecter via France Connect. En tant qu'instructeur ou administrateur, nous vous invitons à réininitialiser votre mot de passe." procedure_archived: "Cette démarche en ligne a été close, il n’est plus possible de déposer de dossier." # procedure_not_draft: "Cette démarche n’est maintenant plus en brouillon." cadastres_empty: diff --git a/spec/controllers/france_connect/particulier_controller_spec.rb b/spec/controllers/france_connect/particulier_controller_spec.rb index d718a65ff..939c6b559 100644 --- a/spec/controllers/france_connect/particulier_controller_spec.rb +++ b/spec/controllers/france_connect/particulier_controller_spec.rb @@ -55,7 +55,7 @@ describe FranceConnect::ParticulierController, type: :controller do it { expect { subject }.not_to change { FranceConnectInformation.count } } context 'when france_connect_particulier_id have an associate user' do - let!(:user) { create(:user, email: 'plop@plop.com', france_connect_information: france_connect_information) } + let!(:user) { create(:user, email: email, france_connect_information: france_connect_information) } it do subject @@ -84,6 +84,17 @@ describe FranceConnect::ParticulierController, type: :controller do expect(user.reload.loged_in_with_france_connect).to eq(User.loged_in_with_france_connects.fetch(:particulier)) expect(subject).to redirect_to(root_path) end + + context 'and the user is also instructeur' do + let(:instructeur) { create(:instructeur) } + let(:email) { instructeur.email } + let(:user) { instructeur.user } + before { subject } + + it { expect(response).to redirect_to(new_user_session_path) } + + it { expect(flash[:alert]).to be_present } + end end context 'when a differently cased email address is already used' do diff --git a/spec/models/france_connect_information_spec.rb b/spec/models/france_connect_information_spec.rb index 1abc85a30..3115b9bee 100644 --- a/spec/models/france_connect_information_spec.rb +++ b/spec/models/france_connect_information_spec.rb @@ -6,4 +6,27 @@ describe FranceConnectInformation, type: :model do it { is_expected.to allow_value('mon super projet').for(:france_connect_particulier_id) } end end + + describe 'associate_user!' do + context 'when there is no user with same email' do + let(:fci) { create(:france_connect_information) } + let(:subject) { fci.associate_user! } + + it { expect { subject }.to change(User, :count).by(1) } + it do + subject + 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 diff --git a/spec/services/france_connect_service_spec.rb b/spec/services/france_connect_service_spec.rb index 80a611a66..c985fccbe 100644 --- a/spec/services/france_connect_service_spec.rb +++ b/spec/services/france_connect_service_spec.rb @@ -15,7 +15,7 @@ describe FranceConnectService do let(:user_info_hash) { { sub: france_connect_particulier_id, given_name: given_name, family_name: family_name, birthdate: birthdate, gender: gender, birthplace: birthplace, email: email, phone: phone } } let(:user_info) { instance_double('OpenIDConnect::ResponseObject::UserInfo', raw_attributes: user_info_hash) } - subject { described_class.retrieve_user_informations_particulier code } + subject { described_class.find_or_retrieve_france_connect_information code } before do allow_any_instance_of(FranceConnectParticulierClient).to receive(:access_token!).and_return(access_token)