Merge pull request #9166 from tchak/fix-prefill-test

ETQ Administrateur, je voudrais pouvoir pré-remplir un dossier sur une démarche en teste

Gemfile

@@ -75,7 +75,6 @@ gem 'redcarpet'
 gem 'rexml' # add missing gem due to ruby3 (https://github.com/Shopify/bootsnap/issues/325)
 gem 'rqrcode'
 gem 'saml_idp'
-gem 'sanitize-url'
 gem 'sassc-rails' # Use SCSS for stylesheets
 gem 'sentry-delayed_job'
 gem 'sentry-rails'

Gemfile.lock

@@ -635,7 +635,6 @@ GEM
     sanitize (6.0.1)
       crass (~> 1.0.2)
       nokogiri (>= 1.12.0)
-    sanitize-url (0.1.4)
     sass (3.7.4)
       sass-listen (~> 4.0.0)
     sass-listen (4.0.0)
@@ -895,7 +894,6 @@ DEPENDENCIES
   rubocop-rails
   rubocop-rspec
   saml_idp
-  sanitize-url
   sassc-rails
   scss_lint
   selenium-devtools

app/controllers/administrateurs/procedures_controller.rb

@@ -116,8 +116,6 @@ module Administrateurs
       @procedure.validate(:publication)
 
       @current_administrateur = current_administrateur
-      @procedure_lien = commencer_url(path: @procedure.path)
-      @procedure_lien_test = commencer_test_url(path: @procedure.path)
     end
 
     def edit
@@ -267,8 +265,6 @@ module Administrateurs
           draft_revision: :types_de_champ
         ).find(params[:procedure_id])
 
-      @procedure_lien = commencer_url(path: @procedure.path)
-      @procedure_lien_test = commencer_test_url(path: @procedure.path)
       @procedure.path = @procedure.suggested_path(current_administrateur)
       @current_administrateur = current_administrateur
       @closed_procedures = current_administrateur.procedures.with_discarded.closes.map { |p| ["#{p.libelle} (#{p.id})", p.id] }.to_h

app/controllers/concerns/procedure_context_concern.rb

@@ -26,13 +26,7 @@ module ProcedureContextConcern
     uri = URI(get_stored_location_for(:user))
     path_components = uri.path.split('/')
 
-    if uri.path.start_with?('/commencer/test/')
+    Procedure.publiees_ou_brouillons.find_by(path: path_components[2])
-      Procedure.brouillon.find_by(path: path_components[3])
-    elsif uri.path.start_with?('/commencer/')
-      Procedure.publiee.find_by(path: path_components[2])
-    else
-      nil
-    end
   end
 
   def find_prefill_token_in_context

app/controllers/users/commencer_controller.rb

@@ -4,8 +4,10 @@ module Users
 
     def commencer
       @procedure = retrieve_procedure
-      return procedure_not_found if @procedure.blank? || @procedure.brouillon?
+
-      @revision = @procedure.published_revision
+      return procedure_not_found if @procedure.blank?
+
+      @revision = params[:test] ? @procedure.draft_revision : @procedure.active_revision
 
       if params[:prefill_token].present? || commencer_page_is_reloaded?
         retrieve_prefilled_dossier(params[:prefill_token] || session[:prefill_token])
@@ -22,11 +24,7 @@ module Users
     end
 
     def commencer_test
-      @procedure = retrieve_procedure
+      redirect_to commencer_path(params[:path], **extra_query_params)
-      return procedure_not_found if @procedure.blank? || (@procedure.publiee? && !@procedure.draft_changed?)
-      @revision = @procedure.draft_revision
-
-      render 'commencer/show'
     end
 
     def dossier_vide_pdf
@@ -73,8 +71,12 @@ module Users
 
     private
 
+    def extra_query_params
+      params.slice(:prefill_token, :test).to_unsafe_h.compact
+    end
+
     def commencer_page_is_reloaded?
-      session[:prefill_token].present? && session[:prefill_params] == params.to_unsafe_h
+      session[:prefill_token].present? && session[:prefill_params_digest] == PrefillParams.digest(params)
     end
 
     def prefill_params_present?
@@ -101,7 +103,7 @@ module Users
         @prefilled_dossier.prefill!(PrefillParams.new(@prefilled_dossier, params.to_unsafe_h).to_a)
       end
       session[:prefill_token] = @prefilled_dossier.prefill_token
-      session[:prefill_params] = params.to_unsafe_h
+      session[:prefill_params_digest] = PrefillParams.digest(params)
     end
 
     def retrieve_prefilled_dossier(prefill_token)
@@ -123,7 +125,7 @@ module Users
       procedure = Procedure.find_by(path: params[:path])
 
       if procedure&.replaced_by_procedure
-        redirect_to commencer_path(path: procedure.replaced_by_procedure.path)
+        redirect_to commencer_path(procedure.replaced_by_procedure.path, **extra_query_params)
         return
       elsif procedure&.close?
         flash.alert = procedure.service.presence ?
@@ -137,7 +139,7 @@ module Users
     end
 
     def store_user_location!(procedure)
-      store_location_for(:user, helpers.procedure_lien(procedure, prefill_token: params[:prefill_token]))
+      store_location_for(:user, commencer_path(procedure.path, **extra_query_params))
     end
 
     def generate_empty_pdf(revision)

app/graphql/types/demarche_descriptor_type.rb

@@ -75,12 +75,8 @@ Cela évite l’accès récursif aux dossiers."
     delegate :description, :opendata, :tags, to: :procedure
 
     def demarche_url
-      if procedure.brouillon?
-        Rails.application.routes.url_helpers.commencer_test_url(path: procedure.path)
-      else
       Rails.application.routes.url_helpers.commencer_url(path: procedure.path)
     end
-    end
 
     def dpo_url
       procedure.lien_dpo

app/helpers/application_helper.rb

@@ -1,6 +1,4 @@
 module ApplicationHelper
-  include SanitizeUrl
-
   def html_lang
     I18n.locale.to_s
   end
@@ -17,12 +15,6 @@ module ApplicationHelper
     end
   end
 
-  def sanitize_url(url)
-    if !url.nil?
-      super(url, schemes: ['http', 'https'], replace_evil_with: root_url)
-    end
-  end
-
   def flash_class(level, sticky: false, fixed: false)
     class_names = []
 

app/helpers/procedure_helper.rb

@@ -1,12 +1,4 @@
 module ProcedureHelper
-  def procedure_lien(procedure, prefill_token: nil)
-    if procedure.brouillon?
-      commencer_test_url(path: procedure.path, prefill_token: prefill_token)
-    else
-      commencer_url(path: procedure.path, prefill_token: prefill_token)
-    end
-  end
-
   def procedure_libelle(procedure)
     parts = procedure.brouillon? ? [procedure_badge(procedure)] : []
     parts << procedure.libelle

app/models/prefill_params.rb

@@ -10,6 +10,10 @@ class PrefillParams
     build_prefill_values.filter(&:prefillable?).map(&:champ_attributes).flatten
   end
 
+  def self.digest(params)
+    Digest::SHA256.hexdigest(params.reject { |(key, _)| key.split('_').first != "champ" }.to_json)
+  end
+
   private
 
   def build_prefill_values

app/serializers/procedure_serializer.rb

@@ -28,12 +28,8 @@ class ProcedureSerializer < ActiveModel::Serializer
   end
 
   def link
-    if object.brouillon?
-      commencer_test_url(path: object.path)
-    else
     commencer_url(path: object.path)
   end
-  end
 
   def state
     object.aasm_state

app/views/administrateurs/_breadcrumbs.html.haml

@@ -28,6 +28,6 @@
         %p
           - if @procedure.locked?
             = t('published', scope: [:layouts, :breadcrumb])
-            = link_to procedure_lien(@procedure), procedure_lien(@procedure)
+            = link_to commencer_url(@procedure.path), commencer_url(@procedure.path)
           - else
             = t('draft', scope: [:layouts, :breadcrumb])

app/views/administrateurs/procedures/_procedures_list.html.haml

@@ -6,7 +6,7 @@
       .flex.column.ml-1
         .card-title
           = link_to procedure.libelle, admin_procedure_path(procedure), style: 'color: black;'
-        = link_to(procedure_lien(procedure), procedure_lien(procedure), class: 'fr-link fr-mb-1w')
+        = link_to commencer_url(procedure.path), commencer_url(procedure.path), class: 'fr-link fr-mb-1w'
 
       .admin-procedures-list-timestamps
         %p.notice N° #{procedure.id}
@@ -48,7 +48,7 @@
 
           - if !procedure.close? && !procedure.discarded?
             - menu.with_item do
-              = link_to(sanitize_url(procedure.brouillon? ? commencer_test_url(path: procedure.path) : commencer_url(path: procedure.path)), target: :blank, rel: :noopener, role: 'menuitem') do
+              = link_to commencer_url(path: procedure.path), target: :blank, rel: :noopener, role: 'menuitem' do
                 %span.icon.in-progress
                 .dropdown-description
                   %h4= t('administrateurs.dropdown_actions.to_test')

app/views/administrateurs/procedures/publication.html.haml

@@ -26,7 +26,7 @@
       - elsif @procedure.publiee?
         %p Cette démarche est <strong>publiée</strong>, certains éléments ne peuvent plus être modifiés.
         Pour y accéder vous pouvez utiliser le lien :
-        = link_to @procedure_lien, sanitize_url(@procedure_lien), target: :blank, rel: :noopener, class: "mb-4"
+        = link_to commencer_url(@procedure.path), commencer_url(@procedure.path), target: :blank, rel: :noopener, class: "mb-4"
 
         %p.mb-4 Attention, diffusez toujours le <strong>lien complet</strong> affiché ci-dessus, et non pas un lien générique vers #{APPLICATION_NAME}. Ne dites pas non plus aux usagers de se rendre sur le site générique #{APPLICATION_NAME}, donnez-leur toujours le lien complet.
       - elsif @procedure.brouillon?
@@ -34,7 +34,7 @@
           %p
             Cette démarche est actuellement <strong>en test</strong>,
             pour y accéder vous pouvez utiliser le lien :
-            = link_to @procedure_lien_test, sanitize_url(@procedure_lien_test), target: :blank, rel: :noopener
+            = link_to commencer_url(@procedure.path), commencer_url(@procedure.path), target: :blank, rel: :noopener
           %p.mb-4
             Toute personne ayant la connaissance de ce lien pourra ainsi remplir des dossiers de test sur votre démarche.
 

app/views/administrateurs/procedures/show.html.haml

@@ -12,7 +12,7 @@
         = link_to 'PDF', commencer_dossier_vide_for_revision_path(@procedure.active_revision), target: "_blank", rel: "noopener", class: 'fr-btn fr-btn--tertiary fr-btn--icon-left fr-icon-printer-line', id: "pdf-procedure"
 
       - if @procedure.brouillon? || @procedure.draft_changed?
-        = link_to 'Tester la démarche', sanitize_url(@procedure_lien_test), target: :blank, rel: :noopener, class: 'fr-btn fr-btn--tertiary fr-btn--icon-left fr-icon-edit-line'
+        = link_to 'Tester la démarche', commencer_url(@procedure.path, test: true), target: :blank, rel: :noopener, class: 'fr-btn fr-btn--tertiary fr-btn--icon-left fr-icon-edit-line'
 
       - if @procedure.publiee? || @procedure.brouillon?
         = link_to 'Envoyer une copie', admin_procedure_transfert_path(@procedure), class: 'fr-btn fr-btn--tertiary fr-btn--icon-left fr-icon-arrow-right-up-line'

app/views/fields/procedure_link_field/_show.html.haml

@@ -1,4 +1,4 @@
 - if field.data.present?
-  = link_to Addressable::URI.parse(procedure_lien(field.resource)).path, procedure_lien(field.resource), target: '_blank', rel: 'noopener'
+  = link_to commencer_path(field.resource.path), commencer_path(field.resource.path), target: '_blank', rel: 'noopener'
 - else
   Plus en ligne

app/views/instructeurs/procedures/_header.html.haml

@@ -2,7 +2,7 @@
   .flex.clipboard-container
     %h1
       = "#{procedure_libelle procedure} - n°#{procedure.id}"
-    = render Dsfr::CopyButtonComponent.new(title: t('instructeurs.procedures.index.copy_link_button'), text: procedure_lien(procedure))
+    = render Dsfr::CopyButtonComponent.new(title: t('instructeurs.procedures.index.copy_link_button'), text: commencer_url(procedure.path))
   = link_to t('instructeurs.dossiers.header.banner.notification_management'), email_notifications_instructeur_procedure_path(procedure), class: 'header-link'
   |
   = link_to t('instructeurs.dossiers.header.banner.statistics'), stats_instructeur_procedure_path(procedure), class: 'header-link'

app/views/instructeurs/procedures/_list.html.haml

@@ -8,7 +8,7 @@
         %p.fr-mb-2w
           = procedure_badge(p)
           = link_to("#{p.libelle} - n°#{p.id}", instructeur_procedure_path(p), class: "fr-link fr-ml-1w")
-          = render Dsfr::CopyButtonComponent.new(title: t('instructeurs.procedures.index.copy_link_button'), text: procedure_lien(p))
+          = render Dsfr::CopyButtonComponent.new(title: t('instructeurs.procedures.index.copy_link_button'), text: commencer_url(p.path))
 
       %ul.procedure-stats.flex
         %li

app/views/users/dossiers/_dossier_actions.html.haml

@@ -34,7 +34,7 @@
 
     - if has_new_dossier_action
       - menu.with_item do
-        = link_to(procedure_lien(dossier.procedure), role: 'menuitem') do
+        = link_to(commencer_url(dossier.procedure.path), role: 'menuitem') do
           %span.icon.new-folder
           .dropdown-description
             = t('views.users.dossiers.dossier_action.start_other_dossier')

app/views/users/dossiers/_merci.html.haml

@@ -20,7 +20,7 @@
 
     .flex.column.align-center
       = link_to t('views.users.dossiers.merci.acces_dossier'), dossier ? dossier_path(dossier) : "#dossier" , class: 'fr-btn fr-btn--xl fr-mt-5w'
-      = link_to t('views.users.dossiers.merci.submit_dossier'), procedure_lien(procedure), class: 'fr-btn fr-btn--secondary fr-mt-3w'
+      = link_to t('views.users.dossiers.merci.submit_dossier'), commencer_url(procedure.path), class: 'fr-btn fr-btn--secondary fr-mt-3w'
 
       .monavis
         != procedure.monavis_embed

spec/controllers/concerns/procedure_context_concern_spec.rb

@@ -45,24 +45,11 @@ RSpec.describe ProcedureContextConcern, type: :controller do
         end
       end
 
-      context 'when the procedure path exists, but not with the same publication status' do
-        let(:published_procedure) { create :procedure, :published }
-
-        before do
-          controller.store_location_for(:user, commencer_test_path(path: published_procedure.path))
-        end
-
-        it 'redirects with an error' do
-          expect(subject.status).to eq 302
-          expect(subject).to redirect_to root_path
-        end
-      end
-
       context 'when the stored procedure is in test' do
         let(:test_procedure) { create :procedure, :with_path }
 
         before do
-          controller.store_location_for(:user, commencer_test_path(path: test_procedure.path))
+          controller.store_location_for(:user, commencer_path(path: test_procedure.path))
         end
 
         it 'succeeds, and assigns the procedure on the controller' do
@@ -74,7 +61,7 @@ RSpec.describe ProcedureContextConcern, type: :controller do
           let(:dossier) { create :dossier, :prefilled, procedure: test_procedure }
 
           before do
-            controller.store_location_for(:user, commencer_test_path(path: test_procedure.path, prefill_token: dossier.prefill_token))
+            controller.store_location_for(:user, commencer_path(path: test_procedure.path, prefill_token: dossier.prefill_token))
           end
 
           it 'succeeds, and assigns the prefill token on the controller' do

spec/controllers/users/commencer_controller_spec.rb

@@ -20,8 +20,11 @@ describe Users::CommencerController, type: :controller do
     context 'when the path is for a draft procedure' do
       let(:path) { draft_procedure.path }
 
-      it 'redirects with an error message' do
+      it 'renders the view' do
-        expect(subject).to redirect_to(root_path)
+        expect(subject.status).to eq(200)
+        expect(subject).to render_template('show')
+        expect(assigns(:procedure)).to eq draft_procedure
+        expect(assigns(:revision)).to eq draft_procedure.draft_revision
       end
     end
 
@@ -155,7 +158,7 @@ describe Users::CommencerController, type: :controller do
             subject
             expect(Dossier.count).to eq(1)
             expect(session[:prefill_token]).to eq(Dossier.last.prefill_token)
-            expect(session[:prefill_params]).to eq({ "action" => "commencer", "champ_#{type_de_champ_text.to_typed_id}" => "blabla", "controller" => "users/commencer", "path" => path.to_s })
+            expect(session[:prefill_params_digest]).to eq(PrefillParams.digest({ "champ_#{type_de_champ_text.to_typed_id}" => "blabla" }))
             expect(Dossier.last.champs.where(type_de_champ: type_de_champ_text).first.value).to eq("blabla")
           end
         end
@@ -181,6 +184,7 @@ describe Users::CommencerController, type: :controller do
           end
         end
       end
+
       context "when prefilled params are passed" do
         subject { get :commencer, params: { path: path, prefill_token: "token", "champ_#{type_de_champ_text.to_typed_id}" => "blabla" } }
 
@@ -198,12 +202,13 @@ describe Users::CommencerController, type: :controller do
           it { expect { subject }.to raise_error(ActiveRecord::RecordNotFound) }
         end
       end
+
       context "when session params exists" do
         subject { get :commencer, params: { path: path, "champ_#{type_de_champ_text.to_typed_id}" => "blabla" } }
 
         before do
           session[:prefill_token] = "token"
-          session[:prefill_params] = { "action" => "commencer", "champ_#{type_de_champ_text.to_typed_id}" => "blabla", "controller" => "users/commencer", "path" => path.to_s }
+          session[:prefill_params_digest] = PrefillParams.digest({ "champ_#{type_de_champ_text.to_typed_id}" => "blabla" })
         end
 
         context "when the associated dossier exists" do
@@ -215,6 +220,7 @@ describe Users::CommencerController, type: :controller do
             expect(assigns(:prefilled_dossier)).to eq(dossier)
           end
         end
+
         context "when the associated dossier does not exists" do
           it { expect { subject }.to raise_error(ActiveRecord::RecordNotFound) }
         end
@@ -222,37 +228,6 @@ describe Users::CommencerController, type: :controller do
     end
   end
 
-  describe '#commencer_test' do
-    subject { get :commencer_test, params: { path: path } }
-
-    context 'when the path is for a draft procedure' do
-      let(:path) { draft_procedure.path }
-
-      it 'renders the view' do
-        expect(subject.status).to eq(200)
-        expect(subject).to render_template('show')
-        expect(assigns(:procedure)).to eq draft_procedure
-        expect(assigns(:revision)).to eq draft_procedure.draft_revision
-      end
-    end
-
-    context 'when the path is for a published procedure' do
-      let(:path) { published_procedure.path }
-
-      it 'redirects with an error message' do
-        expect(subject).to redirect_to(root_path)
-      end
-    end
-
-    context 'when the path does not exist' do
-      let(:path) { 'hello' }
-
-      it 'redirects with an error message' do
-        expect(subject).to redirect_to(root_path)
-      end
-    end
-  end
-
   shared_examples 'a prefill token storage' do
     it 'stores the prefill token' do
       subject
@@ -283,7 +258,7 @@ describe Users::CommencerController, type: :controller do
 
       it 'set the path to return after sign-in to the draft procedure start page' do
         subject
-        expect(controller.stored_location_for(:user)).to eq(commencer_test_path(path: draft_procedure.path))
+        expect(controller.stored_location_for(:user)).to eq(commencer_path(path: draft_procedure.path))
       end
 
       it { expect(subject).to redirect_to(new_user_session_path) }
@@ -327,7 +302,7 @@ describe Users::CommencerController, type: :controller do
 
       it 'set the path to return after sign-up to the draft procedure start page' do
         subject
-        expect(controller.stored_location_for(:user)).to eq(commencer_test_path(path: draft_procedure.path))
+        expect(controller.stored_location_for(:user)).to eq(commencer_path(path: draft_procedure.path))
       end
 
       it { expect(subject).to redirect_to(new_user_registration_path) }
@@ -371,7 +346,7 @@ describe Users::CommencerController, type: :controller do
 
       it 'set the path to return after sign-up to the draft procedure start page' do
         subject
-        expect(controller.stored_location_for(:user)).to eq(commencer_test_path(path: draft_procedure.path))
+        expect(controller.stored_location_for(:user)).to eq(commencer_path(path: draft_procedure.path))
       end
 
       it { expect(subject).to redirect_to(france_connect_particulier_path) }
@@ -401,6 +376,7 @@ describe Users::CommencerController, type: :controller do
         expect(response).to have_http_status(:success)
       end
     end
+
     context 'not yet published procedure' do
       let(:procedure) { create(:procedure, :with_service, :with_path) }
 
@@ -408,6 +384,7 @@ describe Users::CommencerController, type: :controller do
         expect(response).to have_http_status(302)
       end
     end
+
     context 'closed procedure' do
       it 'works' do
         procedure.service = create(:service)
@@ -428,6 +405,7 @@ describe Users::CommencerController, type: :controller do
         expect(response).to have_http_status(:success)
       end
     end
+
     context 'not published procedure without service' do
       let(:procedure) { create(:procedure, :with_path, service: nil, organisation: nil) }
 

spec/helpers/application_helper_spec.rb

@@ -1,23 +1,4 @@
 describe ApplicationHelper do
-  describe "#sanitize_url" do
-    subject { sanitize_url(url) }
-
-    describe 'does nothing on clean url' do
-      let(:url) { "https://tps.fr/toto" }
-      it { is_expected.to eq(url) }
-    end
-
-    describe 'clean a dangerous url' do
-      let(:url) { "javascript:alert('coucou jtai hacké')" }
-      it { is_expected.to eq(root_url) }
-    end
-
-    describe 'can deal with a nil url' do
-      let(:url) { nil }
-      it { is_expected.to be_nil }
-    end
-  end
-
   describe "#flash_class" do
     it { expect(flash_class('notice')).to eq 'alert-success' }
     it { expect(flash_class('alert', sticky: true, fixed: true)).to eq 'alert-danger sticky alert-fixed' }

spec/system/users/dossier_prefill_get_spec.rb

@@ -96,7 +96,7 @@ describe 'Prefilling a dossier (with a GET request):', js: true do
       create(:champ_text, dossier: dossier, type_de_champ: type_de_champ_text, value: text_value)
 
       page.set_rack_session(prefill_token: "token")
-      page.set_rack_session(prefill_params: { "action" => "commencer", "champ_#{type_de_champ_text.to_typed_id}" => text_value, "controller" => "users/commencer", "path" => procedure.path })
+      page.set_rack_session(prefill_params_digest: PrefillParams.digest({ "champ_#{type_de_champ_text.to_typed_id}" => text_value }))
 
       visit "/users/sign_in"
       sign_in_with user.email, password
@@ -117,7 +117,7 @@ describe 'Prefilling a dossier (with a GET request):', js: true do
       expect(page).to have_field(type_de_champ_text.libelle, with: text_value)
 
       expect(page.get_rack_session[:prefill_token]).to be_nil
-      expect(page.get_rack_session[:prefill_params]).to be_nil
+      expect(page.get_rack_session[:prefill_params_digest]).to be_nil
     end
   end