From ce3a156a469c3fd8e5dba398626f071e795d3869 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Fri, 15 Mar 2024 14:20:31 +0100 Subject: [PATCH] refactor(file retrieval): use ciphered query params instead of cookie to move email --- app/controllers/recoveries_controller.rb | 17 +++++++++++------ app/views/recoveries/selection.html.haml | 1 + spec/controllers/recoveries_controller_spec.rb | 5 +++-- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/app/controllers/recoveries_controller.rb b/app/controllers/recoveries_controller.rb index 87e919ff2..5c74d4d59 100644 --- a/app/controllers/recoveries_controller.rb +++ b/app/controllers/recoveries_controller.rb @@ -18,15 +18,17 @@ class RecoveriesController < ApplicationController end def post_identification - # cookies are used to avoid leaking - # email in url - cookies[:recover_previous_email] = previous_email + # cipher previous_user email + # to avoid leaks in the url + ciphered_email = cipher(previous_email) - redirect_to selection_recovery_path + redirect_to selection_recovery_path(ciphered_email:) end def selection - previous_user = User.find_by(email: cookies[:recover_previous_email]) + @previous_email = uncipher(params[:ciphered_email]) + + previous_user = User.find_by(email: @previous_email) @recoverables = RecoveryService .recoverable_procedures(previous_user:, siret:) @@ -35,7 +37,7 @@ class RecoveriesController < ApplicationController end def post_selection - previous_user = User.find_by(email: cookies[:recover_previous_email]) + previous_user = User.find_by(email: previous_email) RecoveryService.recover_procedure!(previous_user:, next_user: current_user, @@ -58,6 +60,9 @@ class RecoveriesController < ApplicationController def previous_email = params[:previous_email] def procedure_ids = params[:procedure_ids].map(&:to_i) + def cipher(email) = message_verifier.generate(email, purpose: :agent_files_recovery, expires_in: 1.hour) + def uncipher(email) = message_verifier.verified(email, purpose: :agent_files_recovery) rescue nil + def structure_name # we know that the structure exists because # of the ensure_collectivite_territoriale guard diff --git a/app/views/recoveries/selection.html.haml b/app/views/recoveries/selection.html.haml index d5ada90c5..0ee886ac8 100644 --- a/app/views/recoveries/selection.html.haml +++ b/app/views/recoveries/selection.html.haml @@ -16,4 +16,5 @@ = check_box_tag 'procedure_ids[]', procedure_id, false, class: 'fr-checkbox', id: procedure_id = label_tag procedure_id, libelle, class: 'fr-label' + = hidden_field_tag 'previous_email', @previous_email %button.fr-btn{ disabled: true, data: { 'enable-submit-if-checked-target': 'submit' } } Continuer diff --git a/spec/controllers/recoveries_controller_spec.rb b/spec/controllers/recoveries_controller_spec.rb index 7d5827f50..a4ea7d743 100644 --- a/spec/controllers/recoveries_controller_spec.rb +++ b/spec/controllers/recoveries_controller_spec.rb @@ -100,8 +100,9 @@ describe RecoveriesController, type: :controller do subject { post :post_identification, params: { previous_email: 'email@a.com' } } it do - is_expected.to redirect_to(selection_recovery_path) - expect(cookies[:recover_previous_email]).to eq('email@a.com') + response = subject + expect(response).to have_http_status(:redirect) + expect(response.location).to start_with(selection_recovery_url) end end