Merge pull request #9264 from tchak/feat-validate_url

feat(procedure): validate external links

Gemfile

@@ -8,6 +8,7 @@ gem 'active_link_to' # Automatically set a class on active links
 gem 'active_model_serializers'
 gem 'activestorage-openstack'
 gem 'active_storage_validations'
+gem 'addressable'
 gem 'administrate'
 gem 'administrate-field-enum' # Allow using Field::Enum in administrate
 gem 'after_party'

Gemfile.lock

@@ -800,6 +800,7 @@ DEPENDENCIES
   active_model_serializers
   active_storage_validations
   activestorage-openstack
+  addressable
   administrate
   administrate-field-enum
   after_party

app/components/procedure/notice_component.rb

@@ -0,0 +1,27 @@
+class Procedure::NoticeComponent < ApplicationComponent
+  def initialize(procedure:)
+    @procedure = procedure
+  end
+
+  private
+
+  def render?
+    link? || attachment?
+  end
+
+  def link?
+    @procedure.lien_notice.present?
+  end
+
+  def url
+    @procedure.lien_notice
+  end
+
+  def attachment?
+    @procedure.notice.present?
+  end
+
+  def attachment
+    @procedure.notice
+  end
+end

app/components/procedure/notice_component/notice_component.en.yml

@@ -0,0 +1,3 @@
+---
+en:
+  name: 'Download the notice of the procedure'

app/components/procedure/notice_component/notice_component.fr.yml

@@ -0,0 +1,3 @@
+---
+fr:
+  name: 'Télécharger le guide de la démarche'

app/components/procedure/notice_component/notice_component.html.haml

@@ -0,0 +1,4 @@
+- if link?
+  = link_to url, t(".name"), class: "fr-download__link", title: new_tab_suffix(t(".name")), **external_link_attributes
+- elsif attachment?
+  = render Dsfr::DownloadComponent.new(attachment:, name: t(".name"), ephemeral_link: administrateur_signed_in?)

app/graphql/schema.graphql

@@ -1016,7 +1016,7 @@ type DemarcheDescriptor {
   """
   URL pour commencer la démarche
   """
-  demarcheUrl: String
+  demarcheUrl: URL
 
   """
   Description de la démarche.
@@ -1039,7 +1039,7 @@ type DemarcheDescriptor {
   notice explicative de la démarche
   """
   notice: File
-  noticeUrl: String
+  noticeUrl: URL
 
   """
   Numero de la démarche.

app/graphql/types/demarche_descriptor_type.rb

@@ -25,10 +25,10 @@ Cela évite l’accès récursif aux dossiers."
 
     field :duree_conservation_dossiers, Int, "Durée de conservation des dossiers en mois.", null: false
 
-    field :demarche_url, String, "URL pour commencer la démarche", null: true
+    field :demarche_url, Types::URL, "URL pour commencer la démarche", null: true
     field :site_web_url, String, "URL où les usagers trouvent le lien vers la démarche", null: true
     field :dpo_url, String, "URL ou email pour contacter le Délégué à la Protection des Données (DPO)", null: true
-    field :notice_url, String, null: true
+    field :notice_url, Types::URL, null: true
     field :cadre_juridique_url, String, "URL du cadre juridique qui justifie le droit de collecter les données demandées dans la démarche", null: true
 
     field :opendata, Boolean, null: false

app/graphql/types/url.rb

@@ -3,12 +3,14 @@ module Types
     description "A valid URL, transported as a string"
 
     def self.coerce_input(input_value, context)
-      url = URI.parse(input_value)
-      if url.is_a?(URI::HTTP) || url.is_a?(URI::HTTPS)
+      url = Addressable::URI(input_value)
+      if uri.scheme.in?(['http', 'https'])
         url
       else
         raise GraphQL::CoercionError, "#{input_value.inspect} is not a valid URL"
       end
+    rescue Addressable::URI::InvalidURIError
+      raise GraphQL::CoercionError, "#{input_value.inspect} is not a valid URL"
     end
 
     def self.coerce_result(ruby_value, context)

app/helpers/notice_url_helper.rb

@@ -1,9 +0,0 @@
-module NoticeURLHelper
-  def notice_url(procedure)
-    if procedure.notice.attached?
-      url_for(procedure.notice)
-    elsif procedure.lien_notice.present?
-      procedure.lien_notice
-    end
-  end
-end

app/helpers/procedure_helper.rb

@@ -48,7 +48,7 @@ module ProcedureHelper
   def url_or_email_to_lien_dpo(procedure)
     URI::MailTo.build([procedure.lien_dpo, "subject="]).to_s
   rescue URI::InvalidComponentError
-    uri = URI.parse(procedure.lien_dpo)
+    uri = Addressable::URI.parse(procedure.lien_dpo)
     return "//#{uri}" if uri.scheme.nil?
     uri.to_s
   end

app/jobs/cron/procedure_external_url_check_job.rb

@@ -0,0 +1,7 @@
+class Cron::ProcedureExternalURLCheckJob < Cron::CronJob
+  self.schedule_expression = "every week on monday at 1 am"
+
+  def perform
+    Procedure.with_external_urls.find_each { ::ProcedureExternalURLCheckJob.perform_later(_1) }
+  end
+end

app/jobs/procedure_external_url_check_job.rb

@@ -0,0 +1,33 @@
+class ProcedureExternalURLCheckJob < ApplicationJob
+  def perform(procedure)
+    procedure.validate
+
+    if procedure.lien_notice.present?
+      error = procedure.errors.find { _1.attribute == :lien_notice }
+      if error.present?
+        procedure.update!(lien_notice_error: error.message)
+      else
+        response = Typhoeus.get(procedure.lien_notice, followlocation: true)
+        if response.success?
+          procedure.update!(lien_notice_error: nil)
+        else
+          procedure.update!(lien_notice_error: "#{response.code} #{response.return_message}")
+        end
+      end
+    end
+
+    if procedure.lien_dpo.present? && !procedure.lien_dpo_email?
+      error = procedure.errors.find { _1.attribute == :lien_dpo }
+      if error.present?
+        procedure.update!(lien_dpo_error: error.message)
+      else
+        response = Typhoeus.get(procedure.lien_dpo, followlocation: true)
+        if response.success?
+          procedure.update!(lien_dpo_error: nil)
+        else
+          procedure.update!(lien_dpo_error: "#{response.code} #{response.return_message}")
+        end
+      end
+    end
+  end
+end

app/models/procedure.rb

@@ -32,7 +32,9 @@
 #  juridique_required                        :boolean          default(TRUE)
 #  libelle                                   :string
 #  lien_dpo                                  :string
+#  lien_dpo_error                            :text
 #  lien_notice                               :string
+#  lien_notice_error                         :text
 #  lien_site_web                             :string
 #  max_duree_conservation_dossiers_dans_ds   :integer          default(12), not null
 #  migrated_champ_routage                    :boolean
@@ -232,6 +234,8 @@ class Procedure < ApplicationRecord
   scope :opendata,               -> { where(opendata: true) }
   scope :publiees_ou_closes,     -> { where(aasm_state: [:publiee, :close, :depubliee]) }
 
+  scope :with_external_urls,     -> { where.not(lien_notice: [nil, '']).or(where.not(lien_dpo: [nil, ''])) }
+
   scope :publiques,              -> do
     publiees_ou_closes
       .opendata
@@ -292,7 +296,12 @@ class Procedure < ApplicationRecord
   validates :libelle, presence: true, allow_blank: false, allow_nil: false
   validates :description, presence: true, allow_blank: false, allow_nil: false
   validates :administrateurs, presence: true
+
   validates :lien_site_web, presence: true, if: :publiee?
+  validates :lien_notice, url: { no_local: true, allow_blank: true }
+  validates :lien_dpo, format: { with: Devise.email_regexp, message: "n'est pas valide" }, if: :lien_dpo_email?
+  validates :lien_dpo, url: { no_local: true, allow_blank: true }, unless: :lien_dpo_email?
+
   validates :draft_types_de_champ_public,
     'types_de_champ/no_empty_block': true,
     'types_de_champ/no_empty_drop_down': true,
@@ -318,7 +327,6 @@ class Procedure < ApplicationRecord
                                                     less_than_or_equal_to: 60
                                                   }
 
-  validates :lien_dpo, email_or_link: true, allow_nil: true
   validates_with MonAvisEmbedValidator
 
   validates_associated :draft_revision, on: :publication
@@ -976,6 +984,10 @@ class Procedure < ApplicationRecord
     update!(routing_enabled: self.groupe_instructeurs.active.many?)
   end
 
+  def lien_dpo_email?
+    lien_dpo.present? && lien_dpo.match?(/@/)
+  end
+
   private
 
   def validate_auto_archive_on_in_the_future

app/validators/email_or_link_validator.rb

@@ -1,7 +0,0 @@
-class EmailOrLinkValidator < ActiveModel::EachValidator
-  def validate_each(record, attribute, value)
-    URI.parse(value)
-  rescue URI::InvalidURIError
-    record.errors.add(attribute, :invalid_uri_or_email)
-  end
-end

app/validators/url_validator.rb

@@ -0,0 +1,69 @@
+require 'active_model'
+require 'active_support/i18n'
+require 'public_suffix'
+require 'addressable/uri'
+
+# Most of this code is borowed from https://github.com/perfectline/validates_url
+
+class URLValidator < ActiveModel::EachValidator
+  RESERVED_OPTIONS = [:schemes, :no_local]
+
+  def initialize(options)
+    options.reverse_merge!(schemes: ['http', 'https'])
+    options.reverse_merge!(message: :url)
+    options.reverse_merge!(no_local: false)
+    options.reverse_merge!(public_suffix: false)
+    options.reverse_merge!(accept_array: false)
+
+    super(options)
+  end
+
+  def validate_each(record, attribute, value)
+    message = options.fetch(:message)
+    schemes = [*options.fetch(:schemes)].map(&:to_s)
+
+    if value.respond_to?(:each)
+      # Error out if we're not allowing arrays
+      if !options.include?(:accept_array) || !options.fetch(:accept_array)
+        record.errors.add(attribute, message, **filtered_options(value))
+      end
+
+      # We have to manually handle `:allow_nil` and `:allow_blank` since it's not caught by
+      # ActiveRecord's own validators. We do that by just removing all the nil's if we want to
+      # allow them so it's not passed on later.
+      value = value.compact if options.include?(:allow_nil) && options.fetch(:allow_nil)
+      value = value.compact_blank if options.include?(:allow_blank) && options.fetch(:allow_blank)
+
+      result = value.flat_map { validate_url(record, attribute, _1, message, schemes) }
+      errors = result.compact
+
+      return errors.any? ? errors.first : true
+    end
+
+    validate_url(record, attribute, value, message, schemes)
+  end
+
+  protected
+
+  def filtered_options(value)
+    filtered = options.except(*RESERVED_OPTIONS)
+    filtered[:value] = value
+    filtered
+  end
+
+  def validate_url(record, attribute, value, message, schemes)
+    uri = Addressable::URI.parse(value)
+    host = uri && uri.host
+    scheme = uri && uri.scheme
+
+    valid_scheme = host && scheme && schemes.include?(scheme)
+    valid_no_local = !options.fetch(:no_local) || (host && host.include?('.'))
+    valid_suffix = !options.fetch(:public_suffix) || (host && PublicSuffix.valid?(host, default_rule: nil))
+
+    unless valid_scheme && valid_no_local && valid_suffix
+      record.errors.add(attribute, message, **filtered_options(value))
+    end
+  rescue Addressable::URI::InvalidURIError
+    record.errors.add(attribute, message, **filtered_options(value))
+  end
+end

app/views/shared/_procedure_description.html.haml

@@ -77,6 +77,5 @@
         #accordion-117.fr-collapse
           = t('shared.procedure_description.estimated_fill_duration_detail', estimated_minutes: estimated_fill_duration_minutes(procedure))
 
-- if procedure.notice.attached?
 .fr-my-3w
-    = render Dsfr::DownloadComponent.new(attachment: procedure.notice , url: notice_url(procedure), name: t("views.shared.dossiers.edit.notice"), ephemeral_link: administrateur_signed_in?)
+  = render Procedure::NoticeComponent.new(procedure:)

app/views/shared/dossiers/_edit.html.haml

@@ -16,8 +16,7 @@
           - if dossier.brouillon?
             = t('views.shared.dossiers.edit.autosave')
 
-          - if dossier.procedure.notice.attached?
-            = render Dsfr::DownloadComponent.new(attachment: dossier.procedure.notice , url: notice_url(dossier.procedure), name: t("views.shared.dossiers.edit.notice"), ephemeral_link: administrateur_signed_in?)
+          = render Procedure::NoticeComponent.new(procedure: dossier.procedure)
 
     = render EditableChamp::SectionComponent.new(champs: dossier_for_editing.champs_public)
 

config/locales/en.yml

@@ -335,7 +335,6 @@ en:
           updated_at: "Updated on %{datetime}"
         edit:
           autosave: Your file is automatically saved after each modification. You can close the window at any time and pick up where you left off later.
-          notice: "Download the notice of the procedure"
           pending_correction:
             confirm_label: I certify that I have made all corrections requested by the administration.
         messages:
@@ -569,6 +568,7 @@ en:
       messages:
         not_a_phone: 'Invalid phone number'
         not_a_rna: 'Invalid RNA number'
+        url: 'is not a valid link'
       models:
         attestation_template:
           attributes:

config/locales/fr.yml

@@ -335,7 +335,6 @@ fr:
           updated_at: "Modifié le %{datetime}"
         edit:
           autosave: Votre dossier est enregistré automatiquement après chaque modification. Vous pouvez à tout moment fermer la fenêtre et reprendre plus tard là où vous en étiez.
-          notice: Télécharger le guide de la démarche
           pending_correction:
             confirm_label: Je certifie avoir effectué toutes les corrections demandées par l’administration.
         messages:
@@ -572,6 +571,7 @@ fr:
       messages:
         not_a_phone: 'Numéro de téléphone invalide'
         not_a_rna: 'Numéro RNA invalide'
+        url: 'n’est pas un lien valide'
       models:
         attestation_template:
           attributes:

db/migrate/20230629102031_add_lien_notice_and_dpo_errors_to_procedures.rb

@@ -0,0 +1,6 @@
+class AddLienNoticeAndDpoErrorsToProcedures < ActiveRecord::Migration[7.0]
+  def change
+    add_column :procedures, :lien_notice_error, :text
+    add_column :procedures, :lien_dpo_error, :text
+  end
+end

db/schema.rb

@@ -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_06_23_160831) do
+ActiveRecord::Schema[7.0].define(version: 2023_06_29_102031) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pgcrypto"
   enable_extension "plpgsql"
@@ -743,7 +743,9 @@ ActiveRecord::Schema[7.0].define(version: 2023_06_23_160831) do
     t.string "libelle"
     t.string "lien_demarche"
     t.string "lien_dpo"
+    t.text "lien_dpo_error"
     t.string "lien_notice"
+    t.text "lien_notice_error"
     t.string "lien_site_web"
     t.integer "max_duree_conservation_dossiers_dans_ds", default: 12, null: false
     t.boolean "migrated_champ_routage"

lib/tasks/deployment/20230615182245_validate_procedure_liens.rake

@@ -0,0 +1,66 @@
+namespace :after_party do
+  desc 'Deployment task: validate_procedure_liens'
+  task validate_procedure_liens: :environment do
+    puts "Running deploy task 'validate_procedure_liens'"
+
+    procedures = Procedure.with_discarded.with_external_urls
+    progress = ProgressReport.new(procedures.count)
+
+    errors = []
+
+    each_error = -> (procedure, &block) {
+      procedure.errors.each do |error|
+        case error.attribute
+        when :lien_notice
+          block.call :lien_notice, procedure.lien_notice
+        when :lien_dpo
+          block.call :lien_dpo, procedure.lien_dpo
+        end
+      end
+    }
+
+    procedures.find_each do |procedure|
+      if !procedure.valid?
+        each_error.(procedure) do |attribute, url|
+          h = {}
+          h[attribute] = url.strip.gsub(/[.;]$/, '').gsub(/^hhttps/, 'https').gsub(/^https\/\//, 'https://')
+          procedure.assign_attributes(h)
+        end
+
+        if !procedure.save
+          each_error.(procedure) do |attribute, url|
+            if !url.match?(/@/) && !url.start_with?('http')
+              h = {}
+              h[attribute] = "https://#{url}"
+              procedure.assign_attributes(h)
+            end
+          end
+
+          if !procedure.save
+            each_error.(procedure) do |attribute, url|
+              h = {}
+              h[attribute] = nil
+              procedure.assign_attributes(h)
+              errors << { attribute:, url: url.gsub('https://', '') }
+            end
+            procedure.save
+          end
+        end
+      end
+      progress.inc
+    end
+
+    progress.finish
+
+    errors.each do |error|
+      rake_puts "removed invalid #{error[:attribute]}: #{error[:url]}"
+    end
+
+    Procedure.with_external_urls.find_each { ::ProcedureExternalURLCheckJob.perform_later(_1) }
+
+    # Update task as completed.  If you remove the line below, the task will
+    # run with every deploy (or every time you call after_party:run).
+    AfterParty::TaskRecord
+      .create version: AfterParty::TaskRecorder.new(__FILE__).timestamp
+  end
+end

spec/models/procedure_spec.rb

@@ -1527,6 +1527,59 @@ describe Procedure do
     end
   end
 
+  describe 'lien_notice' do
+    let(:procedure) { build(:procedure, lien_notice:) }
+
+    context 'when empty' do
+      let(:lien_notice) { '' }
+      it { expect(procedure.valid?).to be_truthy }
+    end
+
+    context 'when valid link' do
+      let(:lien_notice) { 'https://www.demarches-simplifiees.fr' }
+      it { expect(procedure.valid?).to be_truthy }
+    end
+
+    context 'when valid link with accents' do
+      let(:lien_notice) { 'https://www.démarches-simplifiées.fr' }
+      it { expect(procedure.valid?).to be_truthy }
+    end
+
+    context 'when not a valid link' do
+      let(:lien_notice) { 'www.démarches-simplifiées.fr' }
+      it { expect(procedure.valid?).to be_falsey }
+    end
+  end
+
+  describe 'lien_dpo' do
+    let(:procedure) { build(:procedure, lien_dpo:) }
+
+    context 'when empty' do
+      let(:lien_dpo) { '' }
+      it { expect(procedure.valid?).to be_truthy }
+    end
+
+    context 'when valid link' do
+      let(:lien_dpo) { 'https://www.demarches-simplifiees.fr' }
+      it { expect(procedure.valid?).to be_truthy }
+    end
+
+    context 'when valid link with accents' do
+      let(:lien_dpo) { 'https://www.démarches-simplifiées.fr' }
+      it { expect(procedure.valid?).to be_truthy }
+    end
+
+    context 'when valid email' do
+      let(:lien_dpo) { 'test@demarches-simplifiees.fr' }
+      it { expect(procedure.valid?).to be_truthy }
+    end
+
+    context 'when not a valid link' do
+      let(:lien_dpo) { 'www.démarches-simplifiées.fr' }
+      it { expect(procedure.valid?).to be_falsey }
+    end
+  end
+
   private
 
   def create_dossier_with_pj_of_size(size, procedure)