Skip to content

Commit

Permalink
Merge pull request demarches-simplifiees#9904 from demarches-simplifi…
Browse files Browse the repository at this point in the history
…ees/use_email_merge_token

Use email merge token

# Conflicts:
#	db/schema.rb
  • Loading branch information
LeSim authored and maatinito committed Jan 16, 2024
1 parent 3efc11f commit 9bc2e39
Show file tree
Hide file tree
Showing 9 changed files with 91 additions and 20 deletions.
39 changes: 34 additions & 5 deletions app/controllers/france_connect/particulier_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
class FranceConnect::ParticulierController < ApplicationController
before_action :redirect_to_login_if_fc_aborted, only: [:callback]
before_action :securely_retrieve_fci, only: [:merge, :merge_with_existing_account, :merge_with_new_account, :mail_merge_with_existing_account, :resend_and_renew_merge_confirmation]
before_action :securely_retrieve_fci, only: [:merge, :merge_with_existing_account, :merge_with_new_account, :resend_and_renew_merge_confirmation]
before_action :securely_retrieve_fci_from_email_merge_token, only: [:mail_merge_with_existing_account]

def login
if FranceConnectService.enabled?
Expand All @@ -14,7 +15,7 @@ def callback
fci = FranceConnectService.find_or_retrieve_france_connect_information(params[:code])

if fci.user.nil?
preexisting_unlinked_user = User.find_by(email: fci.email_france_connect.downcase)
preexisting_unlinked_user = User.find_by(email: sanitize(fci.email_france_connect))

if preexisting_unlinked_user.nil?
fci.associate_user!(fci.email_france_connect)
Expand Down Expand Up @@ -57,6 +58,7 @@ def merge_with_existing_account
else
@fci.update(user: user)
@fci.delete_merge_token!
@fci.delete_email_merge_token!

flash.notice = t('france_connect.particulier.flash.connection_done', application_name: APPLICATION_NAME)
connect_france_connect_particulier(user)
Expand All @@ -67,7 +69,7 @@ def merge_with_existing_account
end

def mail_merge_with_existing_account
user = User.find_by(email: @fci.email_france_connect.downcase)
user = User.find_by(email: sanitize(@fci.email_france_connect.downcase))
if user.can_france_connect?
@fci.update(user: user)
@fci.delete_merge_token!
Expand Down Expand Up @@ -96,14 +98,33 @@ def merge_with_new_account
end

def resend_and_renew_merge_confirmation
@fci.create_email_merge_token!
UserMailer.france_connect_merge_confirmation(
@fci.email_france_connect,
@fci.email_merge_token,
@fci.email_merge_token_created_at
)
.deliver_later

merge_token = @fci.create_merge_token!
UserMailer.france_connect_merge_confirmation(@fci.email_france_connect, merge_token, @fci.merge_token_created_at).deliver_later
redirect_to france_connect_particulier_merge_path(merge_token),
notice: t('france_connect.particulier.flash.confirmation_mail_sent')
end

private

def securely_retrieve_fci_from_email_merge_token
@fci = FranceConnectInformation.find_by(email_merge_token: email_merge_token_params)

if @fci.nil? || !@fci.valid_for_email_merge?
flash.alert = t('france_connect.particulier.flash.merger_token_expired', application_name: APPLICATION_NAME)

redirect_to root_path
else
@fci.delete_email_merge_token!
end
end

def securely_retrieve_fci
@fci = FranceConnectInformation.find_by(merge_token: merge_token_params)

Expand Down Expand Up @@ -141,11 +162,19 @@ def merge_token_params
params[:merge_token]
end

def email_merge_token_params
params[:email_merge_token]
end

def password_params
params[:password]
end

def sanitized_email_params
params[:email]&.gsub(/[[:space:]]/, ' ')&.strip&.downcase
sanitize(params[:email])
end

def sanitize(string)
string&.gsub(/[[:space:]]/, ' ')&.strip&.downcase
end
end
6 changes: 3 additions & 3 deletions app/mailers/user_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ def ask_for_merge(user, requested_email)
mail(to: requested_email, subject: @subject)
end

def france_connect_merge_confirmation(email, merge_token, merge_token_created_at)
@merge_token = merge_token
@merge_token_created_at = merge_token_created_at
def france_connect_merge_confirmation(email, email_merge_token, email_merge_token_created_at)
@email_merge_token = email_merge_token
@email_merge_token_created_at = email_merge_token_created_at
@subject = "Veuillez confirmer la fusion de compte"

mail(to: email, subject: @subject)
Expand Down
17 changes: 16 additions & 1 deletion app/models/france_connect_information.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,34 @@ def associate_user!(email)

def create_merge_token!
merge_token = SecureRandom.uuid
update(merge_token: merge_token, merge_token_created_at: Time.zone.now)
update(merge_token:, merge_token_created_at: Time.zone.now)

merge_token
end

def create_email_merge_token!
email_merge_token = SecureRandom.uuid
update(email_merge_token:, email_merge_token_created_at: Time.zone.now)

email_merge_token
end

def valid_for_merge?
(MERGE_VALIDITY.ago < merge_token_created_at) && user_id.nil?
end

def valid_for_email_merge?
(MERGE_VALIDITY.ago < email_merge_token_created_at) && user_id.nil?
end

def delete_merge_token!
update(merge_token: nil, merge_token_created_at: nil)
end

def delete_email_merge_token!
update(email_merge_token: nil, email_merge_token_created_at: nil)
end

def full_name
[given_name, family_name].compact.join(" ")
end
Expand Down
7 changes: 4 additions & 3 deletions app/views/user_mailer/france_connect_merge_confirmation.haml
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
- content_for(:title, @subject)
- merge_link = france_connect_particulier_mail_merge_with_existing_account_url(email_merge_token: @email_merge_token)

%p
Bonjour,

%p
Pour confirmer la fusion de votre compte, veuillez cliquer sur le lien suivant :
= round_button 'Je confirme', france_connect_particulier_mail_merge_with_existing_account_url(merge_token: @merge_token), :primary
= round_button 'Je confirme', merge_link, :primary

%p
Vous pouvez aussi visiter ce lien : #{link_to france_connect_particulier_mail_merge_with_existing_account_url(merge_token: @merge_token), france_connect_particulier_mail_merge_with_existing_account_url(merge_token: @merge_token)}
Vous pouvez aussi visiter ce lien : #{link_to merge_link, merge_link}

%p Ce lien est valide #{distance_of_time_in_words(FranceConnectInformation::MERGE_VALIDITY)}, jusqu'à #{@merge_token_created_at.strftime("%d-%m-%Y à %H:%M (%Z)")}
%p Ce lien est valide #{distance_of_time_in_words(FranceConnectInformation::MERGE_VALIDITY)}, jusqu'à #{@email_merge_token_created_at.strftime("%d-%m-%Y à %H:%M (%Z)")}

%p
Si vous n’êtes pas à l’origine de cette demande, vous pouvez ignorer ce message. Et si vous avez besoin d’assistance, n’hésitez pas à nous contacter à
Expand Down
2 changes: 1 addition & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@
get 'particulier' => 'particulier#login'
get 'particulier/callback' => 'particulier#callback'
get 'particulier/merge/:merge_token' => 'particulier#merge', as: :particulier_merge
get 'particulier/mail_merge_with_existing_account/:merge_token' => 'particulier#mail_merge_with_existing_account', as: :particulier_mail_merge_with_existing_account
get 'particulier/mail_merge_with_existing_account/:email_merge_token' => 'particulier#mail_merge_with_existing_account', as: :particulier_mail_merge_with_existing_account
post 'particulier/resend_and_renew_merge_confirmation' => 'particulier#resend_and_renew_merge_confirmation', as: :particulier_resend_and_renew_merge_confirmation
post 'particulier/merge_with_existing_account' => 'particulier#merge_with_existing_account'
post 'particulier/merge_with_new_account' => 'particulier#merge_with_new_account'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
class AddEmailMergeTokenColumnToFranceConnectInformation < ActiveRecord::Migration[7.0]
disable_ddl_transaction!

def change
add_column :france_connect_informations, :email_merge_token, :string
add_column :france_connect_informations, :email_merge_token_created_at, :datetime

add_index :france_connect_informations, :email_merge_token, algorithm: :concurrently
end
end
5 changes: 4 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.0].define(version: 2023_09_07_014353) do
ActiveRecord::Schema[7.0].define(version: 2024_01_10_113623) do
# These are extensions that must be enabled in order to support this database
enable_extension "pgcrypto"
enable_extension "plpgsql"
Expand Down Expand Up @@ -602,6 +602,8 @@
t.datetime "created_at", null: false
t.jsonb "data"
t.string "email_france_connect"
t.string "email_merge_token"
t.datetime "email_merge_token_created_at"
t.string "family_name"
t.string "france_connect_particulier_id"
t.string "gender"
Expand All @@ -610,6 +612,7 @@
t.datetime "merge_token_created_at"
t.datetime "updated_at", null: false
t.integer "user_id"
t.index ["email_merge_token"], name: "index_france_connect_informations_on_email_merge_token"
t.index ["merge_token"], name: "index_france_connect_informations_on_merge_token"
t.index ["user_id"], name: "index_france_connect_informations_on_user_id"
end
Expand Down
23 changes: 18 additions & 5 deletions spec/controllers/france_connect/particulier_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 +258,10 @@

describe '#mail_merge_with_existing_account' do
let(:fci) { FranceConnectInformation.create!(user_info) }
let!(:merge_token) { fci.create_merge_token! }
let!(:email_merge_token) { fci.create_email_merge_token! }

context 'when the merge_token is ok and the user is found' do
subject { post :mail_merge_with_existing_account, params: { merge_token: fci.merge_token } }
subject { post :mail_merge_with_existing_account, params: { email_merge_token: } }

let!(:user) { create(:user, email: email, password: SECURE_PASSWORD) }

Expand All @@ -271,6 +271,7 @@

expect(fci.user).to eq(user)
expect(fci.merge_token).to be_nil
expect(fci.email_merge_token).to be_nil
expect(controller.current_user).to eq(user)
expect(flash[:notice]).to eq("Les comptes FranceConnect et #{APPLICATION_NAME} sont à présent fusionnés")
end
Expand All @@ -288,8 +289,8 @@
end
end

context 'when the merge_token is not ok' do
subject { post :mail_merge_with_existing_account, params: { merge_token: 'ko' } }
context 'when the email_merge_token is not ok' do
subject { post :mail_merge_with_existing_account, params: { email_merge_token: 'ko' } }

let!(:user) { create(:user, email: email) }

Expand All @@ -298,7 +299,7 @@
fci.reload

expect(fci.user).to be_nil
expect(fci.merge_token).not_to be_nil
expect(fci.email_merge_token).not_to be_nil
expect(controller.current_user).to be_nil
expect(response).to redirect_to(root_path)
end
Expand Down Expand Up @@ -330,6 +331,8 @@
context 'when an account with the same email exists' do
let!(:user) { create(:user, email: email) }

before { allow(controller).to receive(:sign_in).and_call_original }

render_views

it 'asks for the corresponding password' do
Expand All @@ -342,6 +345,15 @@

expect(response.body).to include('entrez votre mot de passe')
end

it 'cannot use the merge token in the email confirmation route' do
subject
fci.reload

get :mail_merge_with_existing_account, params: { email_merge_token: fci.merge_token }
expect(controller).not_to have_received(:sign_in)
expect(flash[:alert]).to be_present
end
end
end

Expand All @@ -350,6 +362,7 @@
let(:merge_token) { fci.create_merge_token! }
it 'renew token' do
expect { post :resend_and_renew_merge_confirmation, params: { merge_token: merge_token } }.to change { fci.reload.merge_token }
expect(fci.email_merge_token).to be_present
expect(response).to redirect_to(france_connect_particulier_merge_path(fci.reload.merge_token))
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/mailers/user_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
subject { described_class.france_connect_merge_confirmation(email, code, 15.minutes.from_now) }

it { expect(subject.to).to eq([email]) }
it { expect(subject.body).to include(france_connect_particulier_mail_merge_with_existing_account_url(merge_token: code)) }
it { expect(subject.body).to include(france_connect_particulier_mail_merge_with_existing_account_url(email_merge_token: code)) }

context 'without SafeMailer configured' do
it { expect(subject[BalancerDeliveryMethod::FORCE_DELIVERY_METHOD_HEADER]&.value).to eq(nil) }
Expand Down

0 comments on commit 9bc2e39

Please sign in to comment.