From 8c487c65a920e73fe7cb27ab0f351d9470a9ad88 Mon Sep 17 00:00:00 2001 From: Simon Lehericey Date: Thu, 11 Jan 2018 11:27:56 +0100 Subject: [PATCH 01/16] FC: remove unused code --- app/controllers/users/sessions_controller.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 8c77f7aac..496ac0ea8 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -49,9 +49,6 @@ class Users::SessionsController < Sessions::SessionsController sign_out :user case connected_with_france_connect - when 'entreprise' - redirect_to FRANCE_CONNECT.entreprise_logout_endpoint - return when 'particulier' redirect_to FRANCE_CONNECT.particulier_logout_endpoint return From 4c2da46dd812e65e1111e10e62228106056d8206 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 11 Jan 2018 11:07:25 +0100 Subject: [PATCH 02/16] FC ParticulierClient: simplier initialize --- app/models/france_connect_particulier_client.rb | 7 +++++-- app/services/france_connect_service.rb | 2 +- spec/models/france_connect_particulier_client_spec.rb | 4 ++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/app/models/france_connect_particulier_client.rb b/app/models/france_connect_particulier_client.rb index bc0e99e9c..5c157aa8c 100644 --- a/app/models/france_connect_particulier_client.rb +++ b/app/models/france_connect_particulier_client.rb @@ -1,5 +1,5 @@ class FranceConnectParticulierClient < OpenIDConnect::Client - def initialize params={} + def initialize(code = nil) super( identifier: FRANCE_CONNECT.particulier_identifier, secret: FRANCE_CONNECT.particulier_secret, @@ -11,6 +11,9 @@ class FranceConnectParticulierClient < OpenIDConnect::Client userinfo_endpoint: FRANCE_CONNECT.particulier_userinfo_endpoint, logout_endpoint: FRANCE_CONNECT.particulier_logout_endpoint ) - self.authorization_code = params[:code] if params.has_key? :code + + if code.present? + self.authorization_code = code + end end end diff --git a/app/services/france_connect_service.rb b/app/services/france_connect_service.rb index 883169e70..a5ebc6cab 100644 --- a/app/services/france_connect_service.rb +++ b/app/services/france_connect_service.rb @@ -1,6 +1,6 @@ class FranceConnectService def self.retrieve_user_informations_particulier code - client = FranceConnectParticulierClient.new code: code + client = FranceConnectParticulierClient.new(code) access_token = client.access_token!(client_auth_method: :secret) user_info = access_token.userinfo! diff --git a/spec/models/france_connect_particulier_client_spec.rb b/spec/models/france_connect_particulier_client_spec.rb index 7f3168d13..a93a6eb34 100644 --- a/spec/models/france_connect_particulier_client_spec.rb +++ b/spec/models/france_connect_particulier_client_spec.rb @@ -7,10 +7,10 @@ describe FranceConnectParticulierClient do end context 'when given code in params' do let(:code) { 'plop' } - subject { described_class.new(code: code) } + subject { described_class.new(code) } it 'set authorisation code' do expect_any_instance_of(described_class).to receive(:authorization_code=).with(code) - described_class.new(code: code) + described_class.new(code) end end end From 06d9c4356ee7cd835fd017a9fdea73bcb7ac3a83 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 11 Jan 2018 11:50:04 +0100 Subject: [PATCH 03/16] FC ParticulierClientSpec: simplier spec --- .../france_connect_particulier_client_spec.rb | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/spec/models/france_connect_particulier_client_spec.rb b/spec/models/france_connect_particulier_client_spec.rb index a93a6eb34..b73825d7d 100644 --- a/spec/models/france_connect_particulier_client_spec.rb +++ b/spec/models/france_connect_particulier_client_spec.rb @@ -2,16 +2,14 @@ require 'spec_helper' describe FranceConnectParticulierClient do describe '.initialize' do - it 'create an openid client' do - expect(described_class).to be < OpenIDConnect::Client - end + subject { FranceConnectParticulierClient.new(code) } + context 'when given code in params' do let(:code) { 'plop' } - subject { described_class.new(code) } - it 'set authorisation code' do - expect_any_instance_of(described_class).to receive(:authorization_code=).with(code) - described_class.new(code) - end + + before { allow_any_instance_of(FranceConnectParticulierClient).to receive(:authorization_code=) } + + it { is_expected.to have_received(:authorization_code=).with(code) } end end end From 0d46f927958ffab27be1b097bba033ddc3d9fbf1 Mon Sep 17 00:00:00 2001 From: Simon Lehericey Date: Mon, 15 Jan 2018 12:07:54 +0100 Subject: [PATCH 04/16] FC: move FC constants under particulier namespace --- app/controllers/users/sessions_controller.rb | 2 +- .../france_connect_particulier_client.rb | 16 +++++++------- app/services/france_connect_salt_service.rb | 2 +- config/initializers/france_connect.rb | 21 ++++++++++++------- .../users/sessions_controller_spec.rb | 4 ++-- 5 files changed, 24 insertions(+), 21 deletions(-) diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 496ac0ea8..302c19596 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -50,7 +50,7 @@ class Users::SessionsController < Sessions::SessionsController case connected_with_france_connect when 'particulier' - redirect_to FRANCE_CONNECT.particulier_logout_endpoint + redirect_to FRANCE_CONNECT.particulier.logout_endpoint return end end diff --git a/app/models/france_connect_particulier_client.rb b/app/models/france_connect_particulier_client.rb index 5c157aa8c..934dddb6a 100644 --- a/app/models/france_connect_particulier_client.rb +++ b/app/models/france_connect_particulier_client.rb @@ -1,15 +1,13 @@ class FranceConnectParticulierClient < OpenIDConnect::Client def initialize(code = nil) super( - identifier: FRANCE_CONNECT.particulier_identifier, - secret: FRANCE_CONNECT.particulier_secret, - - redirect_uri: FRANCE_CONNECT.particulier_redirect_uri, - - authorization_endpoint: FRANCE_CONNECT.particulier_authorization_endpoint, - token_endpoint: FRANCE_CONNECT.particulier_token_endpoint, - userinfo_endpoint: FRANCE_CONNECT.particulier_userinfo_endpoint, - logout_endpoint: FRANCE_CONNECT.particulier_logout_endpoint + identifier: FRANCE_CONNECT.particulier.identifier, + secret: FRANCE_CONNECT.particulier.secret, + redirect_uri: FRANCE_CONNECT.particulier.redirect_uri, + authorization_endpoint: FRANCE_CONNECT.particulier.authorization_endpoint, + token_endpoint: FRANCE_CONNECT.particulier.token_endpoint, + userinfo_endpoint: FRANCE_CONNECT.particulier.userinfo_endpoint, + logout_endpoint: FRANCE_CONNECT.particulier.logout_endpoint ) if code.present? diff --git a/app/services/france_connect_salt_service.rb b/app/services/france_connect_salt_service.rb index a28476952..2e35b5515 100644 --- a/app/services/france_connect_salt_service.rb +++ b/app/services/france_connect_salt_service.rb @@ -11,6 +11,6 @@ class FranceConnectSaltService end def salt - Digest::MD5.hexdigest(model.france_connect_particulier_id + model.given_name + model.family_name + FRANCE_CONNECT.particulier_secret + DateTime.now.to_date.to_s) + Digest::MD5.hexdigest(model.france_connect_particulier_id + model.given_name + model.family_name + FRANCE_CONNECT.particulier.secret + DateTime.now.to_date.to_s) end end diff --git a/config/initializers/france_connect.rb b/config/initializers/france_connect.rb index c9e9d11cf..f4bc1d681 100644 --- a/config/initializers/france_connect.rb +++ b/config/initializers/france_connect.rb @@ -1,14 +1,19 @@ FRANCE_CONNECT = if !Rails.env.test? file_path = "#{Rails.root}/config/france_connect.yml" - Hashie::Mash.load(file_path) + config_hash = YAML.safe_load(File.read(file_path)) + .reduce({}) { |acc, (key, value)| acc[key.gsub('particulier_', '')] = value, acc } + + Hashie::Mash.new(particulier: config_hash) else Hashie::Mash.new({ - particulier_identifier: 'plop', - particulier_secret: 'plip', - particulier_redirect_uri: 'https://bidon.com/endpoint', - particulier_authorization_endpoint: 'https://bidon.com/endpoint', - particulier_token_endpoint: 'https://bidon.com/endpoint', - particulier_userinfo_endpoint: 'https://bidon.com/endpoint', - particulier_logout_endpoint: 'https://bidon.com/endpoint', + particulier: { + identifier: 'plop', + secret: 'plip', + redirect_uri: 'https://bidon.com/endpoint', + authorization_endpoint: 'https://bidon.com/endpoint', + token_endpoint: 'https://bidon.com/endpoint', + userinfo_endpoint: 'https://bidon.com/endpoint', + logout_endpoint: 'https://bidon.com/endpoint', + } }) end diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index 6cbaac682..8c812369a 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -118,7 +118,7 @@ describe Users::SessionsController, type: :controller do let(:loged_in_with_france_connect) { 'particulier' } it 'redirect to france connect logout page' do - expect(response).to redirect_to(FRANCE_CONNECT.particulier_logout_endpoint) + expect(response).to redirect_to(FRANCE_CONNECT.particulier.logout_endpoint) end end @@ -161,7 +161,7 @@ describe Users::SessionsController, type: :controller do user.update_attributes(loged_in_with_france_connect: 'particulier') sign_in user delete :destroy - expect(@response.headers["Location"]).to eq(FRANCE_CONNECT.particulier_logout_endpoint) + expect(@response.headers["Location"]).to eq(FRANCE_CONNECT.particulier.logout_endpoint) end context "when associated administrateur" do From ec69be0f7bdf23e20089ae1eebf1f81b4c1b28ec Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 11 Jan 2018 11:36:20 +0100 Subject: [PATCH 05/16] FC ParticulierClient: simplier initializer --- app/models/france_connect_particulier_client.rb | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/app/models/france_connect_particulier_client.rb b/app/models/france_connect_particulier_client.rb index 934dddb6a..c3b9575a2 100644 --- a/app/models/france_connect_particulier_client.rb +++ b/app/models/france_connect_particulier_client.rb @@ -1,14 +1,6 @@ class FranceConnectParticulierClient < OpenIDConnect::Client def initialize(code = nil) - super( - identifier: FRANCE_CONNECT.particulier.identifier, - secret: FRANCE_CONNECT.particulier.secret, - redirect_uri: FRANCE_CONNECT.particulier.redirect_uri, - authorization_endpoint: FRANCE_CONNECT.particulier.authorization_endpoint, - token_endpoint: FRANCE_CONNECT.particulier.token_endpoint, - userinfo_endpoint: FRANCE_CONNECT.particulier.userinfo_endpoint, - logout_endpoint: FRANCE_CONNECT.particulier.logout_endpoint - ) + super(FRANCE_CONNECT.particulier) if code.present? self.authorization_code = code From f8519c5345cbcc158b71122d0d02e3cf260c8bfb Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 11 Jan 2018 14:04:24 +0100 Subject: [PATCH 06/16] FC initializers: remove Hashie --- app/controllers/users/sessions_controller.rb | 2 +- .../france_connect_particulier_client.rb | 2 +- app/services/france_connect_salt_service.rb | 2 +- config/initializers/france_connect.rb | 7 ++++--- .../users/sessions_controller_spec.rb | 4 ++-- .../france_connect_particulier_spec.rb | 19 +++++++++++-------- 6 files changed, 20 insertions(+), 16 deletions(-) diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 302c19596..ea455dbfc 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -50,7 +50,7 @@ class Users::SessionsController < Sessions::SessionsController case connected_with_france_connect when 'particulier' - redirect_to FRANCE_CONNECT.particulier.logout_endpoint + redirect_to FRANCE_CONNECT[:particulier][:logout_endpoint] return end end diff --git a/app/models/france_connect_particulier_client.rb b/app/models/france_connect_particulier_client.rb index c3b9575a2..072b5ab90 100644 --- a/app/models/france_connect_particulier_client.rb +++ b/app/models/france_connect_particulier_client.rb @@ -1,6 +1,6 @@ class FranceConnectParticulierClient < OpenIDConnect::Client def initialize(code = nil) - super(FRANCE_CONNECT.particulier) + super(FRANCE_CONNECT[:particulier]) if code.present? self.authorization_code = code diff --git a/app/services/france_connect_salt_service.rb b/app/services/france_connect_salt_service.rb index 2e35b5515..97d5d83d0 100644 --- a/app/services/france_connect_salt_service.rb +++ b/app/services/france_connect_salt_service.rb @@ -11,6 +11,6 @@ class FranceConnectSaltService end def salt - Digest::MD5.hexdigest(model.france_connect_particulier_id + model.given_name + model.family_name + FRANCE_CONNECT.particulier.secret + DateTime.now.to_date.to_s) + Digest::MD5.hexdigest(model.france_connect_particulier_id + model.given_name + model.family_name + FRANCE_CONNECT[:particulier][:secret] + DateTime.now.to_date.to_s) end end diff --git a/config/initializers/france_connect.rb b/config/initializers/france_connect.rb index f4bc1d681..171f9b5b3 100644 --- a/config/initializers/france_connect.rb +++ b/config/initializers/france_connect.rb @@ -2,10 +2,11 @@ FRANCE_CONNECT = if !Rails.env.test? file_path = "#{Rails.root}/config/france_connect.yml" config_hash = YAML.safe_load(File.read(file_path)) .reduce({}) { |acc, (key, value)| acc[key.gsub('particulier_', '')] = value, acc } + .symbolize_keys - Hashie::Mash.new(particulier: config_hash) + { particulier: config_hash } else - Hashie::Mash.new({ + { particulier: { identifier: 'plop', secret: 'plip', @@ -15,5 +16,5 @@ else userinfo_endpoint: 'https://bidon.com/endpoint', logout_endpoint: 'https://bidon.com/endpoint', } - }) + } end diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index 8c812369a..2263df65e 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -118,7 +118,7 @@ describe Users::SessionsController, type: :controller do let(:loged_in_with_france_connect) { 'particulier' } it 'redirect to france connect logout page' do - expect(response).to redirect_to(FRANCE_CONNECT.particulier.logout_endpoint) + expect(response).to redirect_to(FRANCE_CONNECT[:particulier][:logout_endpoint]) end end @@ -161,7 +161,7 @@ describe Users::SessionsController, type: :controller do user.update_attributes(loged_in_with_france_connect: 'particulier') sign_in user delete :destroy - expect(@response.headers["Location"]).to eq(FRANCE_CONNECT.particulier.logout_endpoint) + expect(@response.headers["Location"]).to eq(FRANCE_CONNECT[:particulier][:logout_endpoint]) end context "when associated administrateur" do diff --git a/spec/features/france_connect/france_connect_particulier_spec.rb b/spec/features/france_connect/france_connect_particulier_spec.rb index 720523714..bbf163217 100644 --- a/spec/features/france_connect/france_connect_particulier_spec.rb +++ b/spec/features/france_connect/france_connect_particulier_spec.rb @@ -10,14 +10,17 @@ feature 'France Connect Particulier Connexion' do let(:email) { 'plop@plop.com' } let(:france_connect_particulier_id) { 'blabla' } - let(:user_info) { Hashie::Mash.new(france_connect_particulier_id: france_connect_particulier_id, - given_name: given_name, - family_name: family_name, - birthdate: birthdate, - birthplace: birthplace, - gender: gender, - email: email) - } + let(:user_info) do + { + france_connect_particulier_id: france_connect_particulier_id, + given_name: given_name, + family_name: family_name, + birthdate: birthdate, + birthplace: birthplace, + gender: gender, + email_france_connect: email + } + end context 'when user is on login page' do before do From b35a88ffd42f3465eb4da9fbbfb0e16cd702e7ae Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 11 Jan 2018 14:37:53 +0100 Subject: [PATCH 07/16] FC initializers: simplify --- config/initializers/france_connect.rb | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/config/initializers/france_connect.rb b/config/initializers/france_connect.rb index 171f9b5b3..a3702335b 100644 --- a/config/initializers/france_connect.rb +++ b/config/initializers/france_connect.rb @@ -1,11 +1,4 @@ -FRANCE_CONNECT = if !Rails.env.test? - file_path = "#{Rails.root}/config/france_connect.yml" - config_hash = YAML.safe_load(File.read(file_path)) - .reduce({}) { |acc, (key, value)| acc[key.gsub('particulier_', '')] = value, acc } - .symbolize_keys - - { particulier: config_hash } -else +FRANCE_CONNECT = if Rails.env.test? { particulier: { identifier: 'plop', @@ -17,4 +10,13 @@ else logout_endpoint: 'https://bidon.com/endpoint', } } +else + fc_config_file_path = "#{Rails.root}/config/france_connect.yml" + + # FIXME: with a yaml with a { particulier: {} } structure + config_hash = YAML.safe_load(File.read(fc_config_file_path)) + .reduce({}) { |acc, (key, value)| acc[key.gsub('particulier_', '')] = value; acc } + .symbolize_keys + + { particulier: config_hash } end From fc4ce4460cde2b727b3fa448d42a211d2d83887f Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 11 Jan 2018 15:23:07 +0100 Subject: [PATCH 08/16] FC Particulier Controller: do not parse an URI into URI --- app/controllers/france_connect/particulier_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index 3d0e0a0f7..1f0ee1a05 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -10,7 +10,7 @@ class FranceConnect::ParticulierController < ApplicationController state: session[:state], nonce: session[:nonce] ) - redirect_to URI.parse(authorization_uri).to_s + redirect_to authorization_uri end def callback From 9dc242d9075c3cab7f51c41a8d9fafecad454883 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 11 Jan 2018 15:29:58 +0100 Subject: [PATCH 09/16] FC Particulier Controller: simplier login --- .../france_connect/particulier_controller.rb | 12 +----------- app/services/france_connect_service.rb | 9 +++++++++ 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index 1f0ee1a05..271bfe89a 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -1,16 +1,6 @@ class FranceConnect::ParticulierController < ApplicationController def login - client = FranceConnectParticulierClient.new - - session[:state] = SecureRandom.hex(16) - session[:nonce] = SecureRandom.hex(16) - - authorization_uri = client.authorization_uri( - scope: [:profile, :email], - state: session[:state], - nonce: session[:nonce] - ) - redirect_to authorization_uri + redirect_to FranceConnectService.authorization_uri end def callback diff --git a/app/services/france_connect_service.rb b/app/services/france_connect_service.rb index a5ebc6cab..d844fd82a 100644 --- a/app/services/france_connect_service.rb +++ b/app/services/france_connect_service.rb @@ -1,4 +1,13 @@ class FranceConnectService + def self.authorization_uri + client = FranceConnectParticulierClient.new + + client.authorization_uri( + scope: [:profile, :email], + state: SecureRandom.hex(16), + nonce: SecureRandom.hex(16)) + end + def self.retrieve_user_informations_particulier code client = FranceConnectParticulierClient.new(code) From 4294c8eec7520f2e4e5fbc4728a8fcd9f8780aa8 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 11 Jan 2018 15:54:39 +0100 Subject: [PATCH 10/16] Brakeman: make it happy --- config/brakeman.ignore | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 config/brakeman.ignore diff --git a/config/brakeman.ignore b/config/brakeman.ignore new file mode 100644 index 000000000..a8ca1ae00 --- /dev/null +++ b/config/brakeman.ignore @@ -0,0 +1,26 @@ +{ + "ignored_warnings": [ + { + "warning_type": "Redirect", + "warning_code": 18, + "fingerprint": "a0a4cede6d50308b90bd747efd0a2ebd58947fbd5d100349ccd640c60413b1a9", + "check_name": "Redirect", + "message": "Possible unprotected redirect", + "file": "app/controllers/france_connect/particulier_controller.rb", + "line": 3, + "link": "http://brakemanscanner.org/docs/warning_types/redirect/", + "code": "redirect_to(FranceConnectParticulierClient.new.authorization_uri)", + "render_path": null, + "location": { + "type": "method", + "class": "FranceConnect::ParticulierController", + "method": "login" + }, + "user_input": "FranceConnectParticulierClient.new.authorization_uri", + "confidence": "High", + "note": "We trust FC OpenId implem" + } + ], + "updated": "2018-01-11 15:53:22 +0100", + "brakeman_version": "3.7.0" +} From e68fc0811f1a7875cecb07108511c0b0c7d31126 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 11 Jan 2018 16:00:14 +0100 Subject: [PATCH 11/16] FC ParticulierController: callback syntax cleaning --- .../france_connect/particulier_controller.rb | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index 271bfe89a..451149820 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -4,14 +4,17 @@ class FranceConnect::ParticulierController < ApplicationController end def callback - return redirect_to new_user_session_path if !params.has_key?(:code) + if params[:code].nil? + return redirect_to new_user_session_path + end user_infos = FranceConnectService.retrieve_user_informations_particulier(params[:code]) if user_infos.present? - france_connect_information = FranceConnectInformation.find_by_france_connect_particulier user_infos + france_connect_information = FranceConnectInformation.find_by_france_connect_particulier(user_infos) - france_connect_information = FranceConnectInformation.create( + if france_connect_information.nil? + france_connect_information = FranceConnectInformation.create( {gender: user_infos[:gender], given_name: user_infos[:given_name], family_name: user_infos[:family_name], @@ -19,14 +22,17 @@ class FranceConnect::ParticulierController < ApplicationController birthdate: user_infos[:birthdate], birthplace: user_infos[:birthplace], france_connect_particulier_id: user_infos[:france_connect_particulier_id]} - ) if france_connect_information.nil? + ) + end user = france_connect_information.user salt = FranceConnectSaltService.new(france_connect_information).salt - return redirect_to france_connect_particulier_new_path(fci_id: france_connect_information.id, salt: salt) if user.nil? + if user.nil? + return redirect_to france_connect_particulier_new_path(fci_id: france_connect_information.id, salt: salt) + end - connect_france_connect_particulier user + connect_france_connect_particulier(user) end rescue Rack::OAuth2::Client::Error => e Rails.logger.error e.message From 8e26a50f162775b58939ba3a058029ef66679b78 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 11 Jan 2018 17:07:35 +0100 Subject: [PATCH 12/16] FC service: clean syntax --- app/services/france_connect_service.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/services/france_connect_service.rb b/app/services/france_connect_service.rb index d844fd82a..3ccca2f50 100644 --- a/app/services/france_connect_service.rb +++ b/app/services/france_connect_service.rb @@ -8,12 +8,13 @@ class FranceConnectService nonce: SecureRandom.hex(16)) end - def self.retrieve_user_informations_particulier code + def self.retrieve_user_informations_particulier(code) client = FranceConnectParticulierClient.new(code) access_token = client.access_token!(client_auth_method: :secret) + user_info = access_token.userinfo! - hash = Hashie::Mash.new user_info.raw_attributes + hash = Hashie::Mash.new(user_info.raw_attributes) hash.france_connect_particulier_id = hash.sub hash From 7024e14d1cbf575561b9aacd289c7c1eac24c689 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 11 Jan 2018 18:26:55 +0100 Subject: [PATCH 13/16] FC information: remove funny find_by --- .../france_connect/particulier_controller.rb | 3 ++- app/models/france_connect_information.rb | 4 ---- spec/models/france_connect_information_spec.rb | 16 ---------------- 3 files changed, 2 insertions(+), 21 deletions(-) diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index 451149820..71b9f8e0e 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -11,7 +11,8 @@ class FranceConnect::ParticulierController < ApplicationController user_infos = FranceConnectService.retrieve_user_informations_particulier(params[:code]) if user_infos.present? - france_connect_information = FranceConnectInformation.find_by_france_connect_particulier(user_infos) + france_connect_information = FranceConnectInformation + .find_by(france_connect_particulier_id: user_infos[:france_connect_particulier_id]) if france_connect_information.nil? france_connect_information = FranceConnectInformation.create( diff --git a/app/models/france_connect_information.rb b/app/models/france_connect_information.rb index 6bf7f7a85..76e804399 100644 --- a/app/models/france_connect_information.rb +++ b/app/models/france_connect_information.rb @@ -2,8 +2,4 @@ class FranceConnectInformation < ActiveRecord::Base belongs_to :user validates :france_connect_particulier_id, presence: true, allow_blank: false, allow_nil: false - - def self.find_by_france_connect_particulier user_info - FranceConnectInformation.find_by(france_connect_particulier_id: user_info[:france_connect_particulier_id]) - end end diff --git a/spec/models/france_connect_information_spec.rb b/spec/models/france_connect_information_spec.rb index 330fc679c..96413d621 100644 --- a/spec/models/france_connect_information_spec.rb +++ b/spec/models/france_connect_information_spec.rb @@ -8,20 +8,4 @@ describe FranceConnectInformation, type: :model do it { is_expected.to allow_value('mon super projet').for(:france_connect_particulier_id) } end end - - describe '.find_by_france_connect_particulier' do - let(:user_info) { {france_connect_particulier_id: '123456'} } - - subject { described_class.find_by_france_connect_particulier user_info } - - context 'when france_connect_particulier_id is prensent in database' do - let!(:france_connect_information) { create(:france_connect_information, france_connect_particulier_id: '123456') } - - it { is_expected.to eq france_connect_information } - end - - context 'when france_connect_particulier_id is prensent in database' do - it { is_expected.to eq nil } - end - end end From 02395e732cb3e7fb8232fcd3d276528560f389c8 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 11 Jan 2018 18:16:38 +0100 Subject: [PATCH 14/16] FC service: return domain info --- .../france_connect/particulier_controller.rb | 17 +++++----------- app/services/france_connect_service.rb | 17 ++++++++++------ .../particulier_controller_spec.rb | 5 +++-- .../france_connect_particulier_spec.rb | 2 +- spec/services/france_connect_service_spec.rb | 20 +++++++++---------- 5 files changed, 30 insertions(+), 31 deletions(-) diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index 71b9f8e0e..d93d2ef0a 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -8,22 +8,15 @@ class FranceConnect::ParticulierController < ApplicationController return redirect_to new_user_session_path end - user_infos = FranceConnectService.retrieve_user_informations_particulier(params[:code]) + fetched_fc_information = FranceConnectService.retrieve_user_informations_particulier(params[:code]) - if user_infos.present? + if fetched_fc_information.present? france_connect_information = FranceConnectInformation - .find_by(france_connect_particulier_id: user_infos[:france_connect_particulier_id]) + .find_by(france_connect_particulier_id: fetched_fc_information[:france_connect_particulier_id]) if france_connect_information.nil? - france_connect_information = FranceConnectInformation.create( - {gender: user_infos[:gender], - given_name: user_infos[:given_name], - family_name: user_infos[:family_name], - email_france_connect: user_infos[:email], - birthdate: user_infos[:birthdate], - birthplace: user_infos[:birthplace], - france_connect_particulier_id: user_infos[:france_connect_particulier_id]} - ) + fetched_fc_information.save + france_connect_information = fetched_fc_information end user = france_connect_information.user diff --git a/app/services/france_connect_service.rb b/app/services/france_connect_service.rb index 3ccca2f50..bbfe73794 100644 --- a/app/services/france_connect_service.rb +++ b/app/services/france_connect_service.rb @@ -11,12 +11,17 @@ class FranceConnectService def self.retrieve_user_informations_particulier(code) client = FranceConnectParticulierClient.new(code) - access_token = client.access_token!(client_auth_method: :secret) + user_info = client.access_token!(client_auth_method: :secret) + .userinfo! + .raw_attributes - user_info = access_token.userinfo! - hash = Hashie::Mash.new(user_info.raw_attributes) - - hash.france_connect_particulier_id = hash.sub - hash + FranceConnectInformation.new( + gender: user_info[:gender], + given_name: user_info[:given_name], + family_name: user_info[:family_name], + email_france_connect: user_info[:email], + birthdate: user_info[:birthdate], + birthplace: user_info[:birthplace], + france_connect_particulier_id: user_info[:sub]) end end diff --git a/spec/controllers/france_connect/particulier_controller_spec.rb b/spec/controllers/france_connect/particulier_controller_spec.rb index 279f7b9ef..9bc9e61fd 100644 --- a/spec/controllers/france_connect/particulier_controller_spec.rb +++ b/spec/controllers/france_connect/particulier_controller_spec.rb @@ -11,7 +11,7 @@ describe FranceConnect::ParticulierController, type: :controller do let(:email) { 'test@test.com' } let(:password) { '' } - let(:user_info) { Hashie::Mash.new(france_connect_particulier_id: france_connect_particulier_id, given_name: given_name, family_name: family_name, birthdate: birthdate, birthplace: birthplace, gender: gender, email: email, password: password) } + let(:user_info) { { france_connect_particulier_id: france_connect_particulier_id, given_name: given_name, family_name: family_name, birthdate: birthdate, birthplace: birthplace, gender: gender, email_france_connect: email } } describe '.auth' do it 'redirect to france connect serveur' do @@ -31,7 +31,8 @@ describe FranceConnect::ParticulierController, type: :controller do context 'when params code is present' do context 'when code is correct' do before do - allow(FranceConnectService).to receive(:retrieve_user_informations_particulier).and_return(user_info) + allow(FranceConnectService).to receive(:retrieve_user_informations_particulier) + .and_return(FranceConnectInformation.new(user_info)) end context 'when france_connect_particulier_id exist in database' do diff --git a/spec/features/france_connect/france_connect_particulier_spec.rb b/spec/features/france_connect/france_connect_particulier_spec.rb index bbf163217..7f1b263af 100644 --- a/spec/features/france_connect/france_connect_particulier_spec.rb +++ b/spec/features/france_connect/france_connect_particulier_spec.rb @@ -47,7 +47,7 @@ feature 'France Connect Particulier Connexion' do before do allow_any_instance_of(FranceConnectParticulierClient).to receive(:authorization_uri).and_return(france_connect_particulier_callback_path(code: code)) - allow(FranceConnectService).to receive(:retrieve_user_informations_particulier).and_return(user_info) + allow(FranceConnectService).to receive(:retrieve_user_informations_particulier).and_return(FranceConnectInformation.new(user_info)) end context 'when is the first connexion' do diff --git a/spec/services/france_connect_service_spec.rb b/spec/services/france_connect_service_spec.rb index 7374ecb32..342b8eb2e 100644 --- a/spec/services/france_connect_service_spec.rb +++ b/spec/services/france_connect_service_spec.rb @@ -7,7 +7,7 @@ describe FranceConnectService do let(:given_name) { 'plop1' } let(:family_name) { 'plop2' } - let(:birthdate) { 'plop3' } + let(:birthdate) { '2012-12-31' } let(:gender) { 'plop4' } let(:birthplace) { 'plop5' } let(:email) { 'plop@emaiL.com' } @@ -29,15 +29,15 @@ describe FranceConnectService do subject end - it 'returns user informations in a object' do - expect(subject.given_name).to eq(given_name) - expect(subject.family_name).to eq(family_name) - expect(subject.birthdate).to eq(birthdate) - expect(subject.gender).to eq(gender) - expect(subject.email).to eq(email) - expect(subject.phone).to eq(phone) - expect(subject.birthplace).to eq(birthplace) - expect(subject.france_connect_particulier_id).to eq(france_connect_particulier_id) + it 'returns user informations' do + expect(subject).to have_attributes({ + given_name: given_name, + family_name: family_name, + birthdate: DateTime.parse(birthdate), + birthplace: birthplace, + gender: gender, + email_france_connect: email, + france_connect_particulier_id: france_connect_particulier_id }) end end end From 34bd3a11e986fa2ecde4b536d7fa1f3f55d17cb3 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 15 Jan 2018 13:19:01 +0100 Subject: [PATCH 15/16] FC ParticulierController: remove superfluous if --- .../france_connect/particulier_controller.rb | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index d93d2ef0a..12c2b828f 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -10,24 +10,22 @@ class FranceConnect::ParticulierController < ApplicationController fetched_fc_information = FranceConnectService.retrieve_user_informations_particulier(params[:code]) - if fetched_fc_information.present? - france_connect_information = FranceConnectInformation - .find_by(france_connect_particulier_id: fetched_fc_information[:france_connect_particulier_id]) + france_connect_information = FranceConnectInformation + .find_by(france_connect_particulier_id: fetched_fc_information[:france_connect_particulier_id]) - if france_connect_information.nil? - fetched_fc_information.save - france_connect_information = fetched_fc_information - end - - user = france_connect_information.user - salt = FranceConnectSaltService.new(france_connect_information).salt - - if user.nil? - return redirect_to france_connect_particulier_new_path(fci_id: france_connect_information.id, salt: salt) - end - - connect_france_connect_particulier(user) + if france_connect_information.nil? + fetched_fc_information.save + france_connect_information = fetched_fc_information end + + user = france_connect_information.user + salt = FranceConnectSaltService.new(france_connect_information).salt + + if user.nil? + return redirect_to france_connect_particulier_new_path(fci_id: france_connect_information.id, salt: salt) + end + + connect_france_connect_particulier(user) rescue Rack::OAuth2::Client::Error => e Rails.logger.error e.message redirect_france_connect_error_connection From d086b82d44fde461fcf1d24a1876049ba18058d6 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 15 Jan 2018 13:20:47 +0100 Subject: [PATCH 16/16] FC ParticulierController: remove one return --- app/controllers/france_connect/particulier_controller.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index 12c2b828f..283660636 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -22,10 +22,10 @@ class FranceConnect::ParticulierController < ApplicationController salt = FranceConnectSaltService.new(france_connect_information).salt if user.nil? - return redirect_to france_connect_particulier_new_path(fci_id: france_connect_information.id, salt: salt) + redirect_to france_connect_particulier_new_path(fci_id: france_connect_information.id, salt: salt) + else + connect_france_connect_particulier(user) end - - connect_france_connect_particulier(user) rescue Rack::OAuth2::Client::Error => e Rails.logger.error e.message redirect_france_connect_error_connection