diff --git a/Gemfile b/Gemfile index c1c879295..520c04354 100644 --- a/Gemfile +++ b/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' diff --git a/Gemfile.lock b/Gemfile.lock index 8f80f0d35..3d36d8933 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -800,6 +800,7 @@ DEPENDENCIES active_model_serializers active_storage_validations activestorage-openstack + addressable administrate administrate-field-enum after_party diff --git a/app/components/procedure/notice_component.rb b/app/components/procedure/notice_component.rb new file mode 100644 index 000000000..ffa31c4d1 --- /dev/null +++ b/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 diff --git a/app/components/procedure/notice_component/notice_component.en.yml b/app/components/procedure/notice_component/notice_component.en.yml new file mode 100644 index 000000000..ffb8d8cb0 --- /dev/null +++ b/app/components/procedure/notice_component/notice_component.en.yml @@ -0,0 +1,3 @@ +--- +en: + name: 'Download the notice of the procedure' diff --git a/app/components/procedure/notice_component/notice_component.fr.yml b/app/components/procedure/notice_component/notice_component.fr.yml new file mode 100644 index 000000000..a75ecb6a9 --- /dev/null +++ b/app/components/procedure/notice_component/notice_component.fr.yml @@ -0,0 +1,3 @@ +--- +fr: + name: 'Télécharger le guide de la démarche' diff --git a/app/components/procedure/notice_component/notice_component.html.haml b/app/components/procedure/notice_component/notice_component.html.haml new file mode 100644 index 000000000..9733133c1 --- /dev/null +++ b/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?) diff --git a/app/graphql/schema.graphql b/app/graphql/schema.graphql index 7ecc93333..10548210b 100644 --- a/app/graphql/schema.graphql +++ b/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. diff --git a/app/graphql/types/demarche_descriptor_type.rb b/app/graphql/types/demarche_descriptor_type.rb index 7562dfcc6..c9eff3c59 100644 --- a/app/graphql/types/demarche_descriptor_type.rb +++ b/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 diff --git a/app/graphql/types/url.rb b/app/graphql/types/url.rb index 287b5789e..fad71e4eb 100644 --- a/app/graphql/types/url.rb +++ b/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) diff --git a/app/helpers/notice_url_helper.rb b/app/helpers/notice_url_helper.rb deleted file mode 100644 index c1e7df802..000000000 --- a/app/helpers/notice_url_helper.rb +++ /dev/null @@ -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 diff --git a/app/helpers/procedure_helper.rb b/app/helpers/procedure_helper.rb index ce16c40f8..993b8bb08 100644 --- a/app/helpers/procedure_helper.rb +++ b/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 diff --git a/app/jobs/cron/procedure_external_url_check_job.rb b/app/jobs/cron/procedure_external_url_check_job.rb new file mode 100644 index 000000000..ab18c8c37 --- /dev/null +++ b/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 diff --git a/app/jobs/procedure_external_url_check_job.rb b/app/jobs/procedure_external_url_check_job.rb new file mode 100644 index 000000000..fb4229eb3 --- /dev/null +++ b/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 diff --git a/app/models/procedure.rb b/app/models/procedure.rb index f892b5c31..375c944b3 100644 --- a/app/models/procedure.rb +++ b/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 diff --git a/app/validators/email_or_link_validator.rb b/app/validators/email_or_link_validator.rb deleted file mode 100644 index 7b0358256..000000000 --- a/app/validators/email_or_link_validator.rb +++ /dev/null @@ -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 diff --git a/app/validators/url_validator.rb b/app/validators/url_validator.rb new file mode 100644 index 000000000..c24e3590b --- /dev/null +++ b/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 diff --git a/app/views/shared/_procedure_description.html.haml b/app/views/shared/_procedure_description.html.haml index cc8fb366d..8bd0a7ed0 100644 --- a/app/views/shared/_procedure_description.html.haml +++ b/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?) +.fr-my-3w + = render Procedure::NoticeComponent.new(procedure:) diff --git a/app/views/shared/dossiers/_edit.html.haml b/app/views/shared/dossiers/_edit.html.haml index 3607a123d..0df412dfe 100644 --- a/app/views/shared/dossiers/_edit.html.haml +++ b/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) diff --git a/config/locales/en.yml b/config/locales/en.yml index ec9dd32bc..d525d9f93 100644 --- a/config/locales/en.yml +++ b/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: diff --git a/config/locales/fr.yml b/config/locales/fr.yml index a62c1af9e..a82963b38 100644 --- a/config/locales/fr.yml +++ b/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: diff --git a/db/migrate/20230629102031_add_lien_notice_and_dpo_errors_to_procedures.rb b/db/migrate/20230629102031_add_lien_notice_and_dpo_errors_to_procedures.rb new file mode 100644 index 000000000..9791fc2db --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index a387e005c..07578f826 100644 --- a/db/schema.rb +++ b/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" diff --git a/lib/tasks/deployment/20230615182245_validate_procedure_liens.rake b/lib/tasks/deployment/20230615182245_validate_procedure_liens.rake new file mode 100644 index 000000000..8160cfc80 --- /dev/null +++ b/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 diff --git a/spec/models/procedure_spec.rb b/spec/models/procedure_spec.rb index 50f791e5c..a15d0fef0 100644 --- a/spec/models/procedure_spec.rb +++ b/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)