Merge pull request #8556 from demarches-simplifiees/remove_a_balise_from_text

secu: remove a balise from sane user input

app/components/editable_champ/champ_label_component/champ_label_component.html.haml

@@ -7,4 +7,4 @@
     = render EditableChamp::ChampLabelContentComponent.new champ: @champ, seen_at: @seen_at
 
 - if @champ.description.present?
-  .notice{ id: @champ.describedby_id }= string_to_html(@champ.description)
+  .notice{ id: @champ.describedby_id }= string_to_html(@champ.description, allow_a: true)

app/components/editable_champ/editable_champ_component/editable_champ_component.html.haml

@@ -2,7 +2,7 @@
   - if @champ.block?
     %h3.header-subsection= @champ.libelle
     - if @champ.description.present?
-      %p.notice= string_to_html(@champ.description, false)
+      %p.notice= string_to_html(@champ.description, false, allow_a: true)
 
   - elsif has_label?(@champ)
     = render EditableChamp::ChampLabelComponent.new form: @form, champ: @champ, seen_at: @seen_at

app/components/editable_champ/explication_component/explication_component.html.haml

@@ -1,7 +1,7 @@
 = render Dsfr::CalloutComponent.new(title: @champ.libelle, extra_class_names: ['fr-mb-2w', 'fr-callout--blue-cumulus']) do |c|
   - c.with_body do
 
-    = string_to_html(@champ.description)
+    = string_to_html(@champ.description, allow_a: true)
 
     - if @champ.collapsible_explanation_enabled? && @champ.collapsible_explanation_text.present?
       %div

app/components/editable_champ/linked_drop_down_list_component/linked_drop_down_list_component.html.haml

@@ -8,7 +8,7 @@
     = @form.label :secondary_value, for: "#{@champ.input_id}-secondary" do
       - sanitize((@champ.drop_down_secondary_libelle.presence || "Valeur secondaire dépendant de la première") + (@champ.type_de_champ.mandatory? ? tag.span(' *', class: 'mandatory') : ''))
     - if @champ.drop_down_secondary_description.present?
-      .notice{ id: "#{@champ.describedby_id}-secondary" }= string_to_html(@champ.drop_down_secondary_description)
+      .notice{ id: "#{@champ.describedby_id}-secondary" }= string_to_html(@champ.drop_down_secondary_description, allow_a: true)
     = @form.select :secondary_value,
       @champ.secondary_options[@champ.primary_value],
       {},

app/controllers/administrateurs/mail_templates_controller.rb

@@ -40,7 +40,7 @@ module Administrateurs
       @dossier = dossier
       @logo_url = procedure.logo_url
       @service = procedure.service
-      @rendered_template = sanitize(mail_template.body_for_dossier(dossier))
+      @rendered_template = sanitize(mail_template.body_for_dossier(dossier), scrubber: Sanitizers::MailScrubber.new)
       @actions = mail_template.actions_for_dossier(dossier)
 
       render(template: 'notification_mailer/send_notification', layout: 'mailers/notifications_layout')

app/helpers/string_to_html_helper.rb

@@ -1,8 +1,15 @@
 module StringToHtmlHelper
-  def string_to_html(str, wrapper_tag = 'p')
+  def string_to_html(str, wrapper_tag = 'p', allow_a: false)
     return nil if str.blank?
     html_formatted = simple_format(str, {}, { wrapper_tag: wrapper_tag })
     with_links = Anchored::Linker.auto_link(html_formatted, target: '_blank', rel: 'noopener')
-    sanitize(with_links, attributes: ['target', 'rel', 'href'])
+
+    tags = if allow_a
+      Rails.configuration.action_view.sanitized_allowed_tags + ['a']
+    else
+      Rails.configuration.action_view.sanitized_allowed_tags
+    end
+
+    sanitize(with_links, tags:, attributes: ['target', 'rel', 'href'])
   end
 end

app/lib/sanitizers/mail_scrubber.rb

@@ -0,0 +1,12 @@
+module Sanitizers
+  class MailScrubber < Rails::Html::PermitScrubber
+    def initialize
+      super
+      self.tags = Rails.application.config.action_view.sanitized_allowed_tags + ['a']
+    end
+
+    def skip_node?(node)
+      node.text?
+    end
+  end
+end

app/mailers/notification_mailer.rb

@@ -20,7 +20,7 @@ class NotificationMailer < ApplicationMailer
   def send_notification
     @service = @dossier.procedure.service
     @logo_url = attach_logo(@dossier.procedure)
-    @rendered_template = sanitize(@body)
+    @rendered_template = sanitize(@body, scrubber: Sanitizers::MailScrubber.new)
     attachments[@attachment[:filename]] = @attachment[:content] if @attachment.present?
 
     I18n.with_locale(@dossier.user_locale) do

app/views/shared/_procedure_description.html.haml

@@ -23,5 +23,5 @@
 
 .procedure-description
   .procedure-description-body.read-more-enabled.read-more-collapsed
-    = h string_to_html(procedure.description)
+    = h string_to_html(procedure.description, allow_a: true)
   = button_tag "Afficher la description complète", class: 'button read-more-button'

config/application.rb

@@ -41,7 +41,7 @@ module TPS
     config.assets.precompile += ['.woff']
 
     default_allowed_tags = ActionView::Base.sanitized_allowed_tags
-    config.action_view.sanitized_allowed_tags = default_allowed_tags + ['u'] - ['img']
+    config.action_view.sanitized_allowed_tags = default_allowed_tags + ['u'] - ['img', 'a']
 
     # ActionDispatch's IP spoofing detection is quite limited, and often rejects
     # legitimate requests from misconfigured proxies (such as mobile telcos).

spec/helpers/string_to_html_helper_spec.rb

@@ -1,6 +1,7 @@
 RSpec.describe StringToHtmlHelper, type: :helper do
   describe "#string_to_html" do
-    subject { string_to_html(description) }
+    let(:allow_a) { false }
+    subject { string_to_html(description, allow_a:) }
 
     context "with some simple texte" do
       let(:description) { "1er ligne \n 2ieme ligne" }
@@ -11,9 +12,17 @@ RSpec.describe StringToHtmlHelper, type: :helper do
     context "with a link" do
       context "using an authorized scheme" do
         let(:description) { "Cliquez sur https://d-s.fr pour continuer." }
+
+        context 'with a tag authorized' do
+          let(:allow_a) { true }
           it { is_expected.to eq("<p>Cliquez sur <a href=\"https://d-s.fr\" target=\"_blank\" rel=\"noopener\">https://d-s.fr</a> pour continuer.</p>") }
         end
 
+        context 'without a tag' do
+          it { is_expected.to eq("<p>Cliquez sur https://d-s.fr pour continuer.</p>") }
+        end
+      end
+
       context "using a non-authorized scheme" do
         let(:description) { "Cliquez sur file://etc/password pour continuer." }
         it { is_expected.to eq("<p>Cliquez sur file://etc/password pour continuer.</p>") }