Merge pull request #5875 from betagouv/5867-fc-only-4-user

5867-authorize-france-connect-only-for-user

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

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

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

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)
 

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 à <a href='%{reset_link}'>réininitialiser votre mot de passe</a>."
       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:

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

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

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)