From ca175245594fc54ffddcd0c1f4a2974fe10f2f8c Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 19 Mar 2024 14:57:19 +0100 Subject: [PATCH] Allow instructeur to have multiple agent_connect_information AC will return two different sub depending of the domain demarches.gouv.fr or ds. Both agent_connect_information are stored and the corresponding instructeur is found by its email. We do not store anymore the agent_connect_id on the instructeur as the are many. --- .../agent_connect/agent_controller.rb | 10 ++---- app/models/instructeur.rb | 4 ++- .../agent_connect/agent_controller_spec.rb | 33 +++++++++++++++++-- 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/app/controllers/agent_connect/agent_controller.rb b/app/controllers/agent_connect/agent_controller.rb index a98a1cff3..fa520652f 100644 --- a/app/controllers/agent_connect/agent_controller.rb +++ b/app/controllers/agent_connect/agent_controller.rb @@ -22,20 +22,16 @@ class AgentConnect::AgentController < ApplicationController user_info, id_token = AgentConnectService.user_info(params[:code], cookies.encrypted[NONCE_COOKIE_NAME]) cookies.delete NONCE_COOKIE_NAME - instructeur = Instructeur.find_by(agent_connect_id: user_info['sub']) - - if instructeur.nil? - instructeur = Instructeur.find_by(users: { email: santized_email(user_info) }) - end + instructeur = Instructeur.find_by(users: { email: santized_email(user_info) }) if instructeur.nil? user = User.create_or_promote_to_instructeur(santized_email(user_info), Devise.friendly_token[0, 20]) instructeur = user.instructeur end - instructeur.update(agent_connect_id: user_info['sub'], agent_connect_id_token: id_token) + instructeur.update(agent_connect_id_token: id_token) - aci = AgentConnectInformation.find_or_initialize_by(instructeur:) + aci = AgentConnectInformation.find_or_initialize_by(instructeur:, sub: user_info['sub']) aci.update(user_info.slice('given_name', 'usual_name', 'email', 'sub', 'siret', 'organizational_unit', 'belonging_population', 'phone')) sign_in(:user, instructeur.user) diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb index 67f091f71..aef5b1ee0 100644 --- a/app/models/instructeur.rb +++ b/app/models/instructeur.rb @@ -1,8 +1,10 @@ class Instructeur < ApplicationRecord + self.ignored_columns += [:agent_connect_id] + include UserFindByConcern has_and_belongs_to_many :administrateurs - has_one :agent_connect_information, dependent: :destroy + has_many :agent_connect_information, dependent: :destroy has_many :assign_to, dependent: :destroy has_many :groupe_instructeurs, -> { order(:label) }, through: :assign_to diff --git a/spec/controllers/agent_connect/agent_controller_spec.rb b/spec/controllers/agent_connect/agent_controller_spec.rb index 979166cf5..c29ed7338 100644 --- a/spec/controllers/agent_connect/agent_controller_spec.rb +++ b/spec/controllers/agent_connect/agent_controller_spec.rb @@ -50,7 +50,6 @@ describe AgentConnect::AgentController, type: :controller do expect(last_user.email).to eq(email) expect(last_user.confirmed_at).to be_present - expect(last_user.instructeur.agent_connect_id).to eq('sub') expect(last_user.instructeur.agent_connect_id_token).to eq('id_token') expect(response).to redirect_to(instructeur_procedures_path) expect(state_cookie).to be_nil @@ -70,7 +69,6 @@ describe AgentConnect::AgentController, type: :controller do instructeur.reload - expect(instructeur.agent_connect_id).to eq('sub') expect(instructeur.agent_connect_id_token).to eq('id_token') expect(response).to redirect_to(instructeur_procedures_path) end @@ -88,13 +86,42 @@ describe AgentConnect::AgentController, type: :controller do instructeur = user.reload.instructeur - expect(instructeur.agent_connect_id).to eq('sub') expect(instructeur.agent_connect_id_token).to eq('id_token') expect(response).to redirect_to(instructeur_procedures_path) end end end + context 'when the instructeur connects two times with the same domain' do + before do + expect(AgentConnectService).to receive(:user_info).with(code, nonce).and_return([user_info, id_token]).twice + expect(controller).to receive(:sign_in).twice + end + + it 'creates another agent_connect_information' do + get :callback, params: { code: code, state: state } + get :callback, params: { code: code, state: state } + + expect(Instructeur.last.agent_connect_information.count).to eq(1) + end + end + + context 'when the instructeur connects two times with different domains' do + before do + expect(controller).to receive(:sign_in).twice + end + + it 'creates another agent_connect_information' do + expect(AgentConnectService).to receive(:user_info).with(code, nonce).and_return([user_info, id_token]) + get :callback, params: { code: code, state: state } + + expect(AgentConnectService).to receive(:user_info).with(code, nonce).and_return([user_info.merge('sub' => 'sub2'), id_token]) + get :callback, params: { code: code, state: state } + + expect(Instructeur.last.agent_connect_information.pluck(:sub)).to match_array(['sub', 'sub2']) + end + end + context 'but user_info raises and error' do before do expect(AgentConnectService).to receive(:user_info).and_raise(Rack::OAuth2::Client::Error.new(500, error: 'Unknown'))