From 348b15f595a793cef5b790001b4d61328c7e305f Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 15 May 2019 15:57:57 +0200 Subject: [PATCH 1/2] Put devtools behind feature flags --- app/controllers/application_controller.rb | 2 +- app/views/layouts/application.html.haml | 4 ++-- config/features.rb | 7 +++++++ config/initializers/rack_mini_profiler.rb | 1 + 4 files changed, 11 insertions(+), 3 deletions(-) create mode 100644 config/initializers/rack_mini_profiler.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 8ab881d8b..5dbfd7a61 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -24,7 +24,7 @@ class ApplicationController < ActionController::Base end def authorize_request_for_profiler - if administration_signed_in? + if Flipflop.mini_profiler_enabled? Rack::MiniProfiler.authorize_request end end diff --git a/app/views/layouts/application.html.haml b/app/views/layouts/application.html.haml index 5a0486f44..2eb205c1a 100644 --- a/app/views/layouts/application.html.haml +++ b/app/views/layouts/application.html.haml @@ -21,7 +21,7 @@ = Gon::Base.render_data(camel_case: true, init: true, nonce: request.content_security_policy_nonce) - - if Rails.env.development? + - if Flipflop.xray_enabled? = stylesheet_link_tag :xray %body{ id: content_for(:page_id), class: browser.platform.ios? ? 'ios' : nil } @@ -39,7 +39,7 @@ - if content_for?(:footer) = content_for(:footer) - - if Rails.env.development? + - if Flipflop.xray_enabled? = javascript_include_tag :xray = yield :charts_js diff --git a/config/features.rb b/config/features.rb index 4c2516549..38dd51141 100644 --- a/config/features.rb +++ b/config/features.rb @@ -18,6 +18,13 @@ Flipflop.configure do feature :operation_log_serialize_subject + group :development do + feature :mini_profiler_enabled, + default: Rails.env.development? + feature :xray_enabled, + default: Rails.env.development? + end + group :production do feature :remote_storage, default: ENV['FOG_ENABLED'] == 'enabled' diff --git a/config/initializers/rack_mini_profiler.rb b/config/initializers/rack_mini_profiler.rb new file mode 100644 index 000000000..5065ab5cd --- /dev/null +++ b/config/initializers/rack_mini_profiler.rb @@ -0,0 +1 @@ +Rack::MiniProfiler.config.authorization_mode = :whitelist From 42235e81b113249d5aafe69554cc84cbdcd9178c Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 16 May 2019 18:59:34 +0200 Subject: [PATCH 2/2] Use active storage load hook to extend blob --- app/lib/active_storage/virus_scanner.rb | 27 +++++++------------ app/models/concerns/blob_virus_scanner.rb | 24 +++++++++++++++++ .../shared/piece_jointe/_pj_link.html.haml | 4 +-- config/initializers/active_storage.rb | 12 ++------- .../gestionnaires/avis_controller_spec.rb | 27 ++++++++++++------- spec/models/champ_spec.rb | 2 +- spec/serializers/champ_serializer_spec.rb | 2 +- 7 files changed, 58 insertions(+), 40 deletions(-) create mode 100644 app/models/concerns/blob_virus_scanner.rb diff --git a/app/lib/active_storage/virus_scanner.rb b/app/lib/active_storage/virus_scanner.rb index bfe41a773..23cbd1c91 100644 --- a/app/lib/active_storage/virus_scanner.rb +++ b/app/lib/active_storage/virus_scanner.rb @@ -23,28 +23,21 @@ class ActiveStorage::VirusScanner blob.metadata[:virus_scan_result] == SAFE end - def analyzed? + def done? + started? && blob.metadata[:virus_scan_result] != PENDING + end + + def started? blob.metadata[:virus_scan_result].present? end - def analyze_later - if !analyzed? - blob.update!(metadata: blob.metadata.merge(virus_scan_result: PENDING)) - VirusScannerJob.perform_later(blob) - end - end - def metadata - begin - download_blob_to_tempfile do |file| - if ClamavService.safe_file?(file.path) - { virus_scan_result: SAFE, scanned_at: Time.zone.now } - else - { virus_scan_result: INFECTED, scanned_at: Time.zone.now } - end + download_blob_to_tempfile do |file| + if ClamavService.safe_file?(file.path) + { virus_scan_result: SAFE, scanned_at: Time.zone.now } + else + { virus_scan_result: INFECTED, scanned_at: Time.zone.now } end - rescue StandardError => e - Raven.capture_exception(e) end end end diff --git a/app/models/concerns/blob_virus_scanner.rb b/app/models/concerns/blob_virus_scanner.rb new file mode 100644 index 000000000..4cd5f8476 --- /dev/null +++ b/app/models/concerns/blob_virus_scanner.rb @@ -0,0 +1,24 @@ +module BlobVirusScanner + extend ActiveSupport::Concern + + included do + before_create :set_pending + after_update_commit :enqueue_virus_scan + end + + def virus_scanner + ActiveStorage::VirusScanner.new(self) + end + + private + + def set_pending + self.metadata[:virus_scan_result] ||= ActiveStorage::VirusScanner::PENDING + end + + def enqueue_virus_scan + if analyzed? && !virus_scanner.done? + VirusScannerJob.perform_later(self) + end + end +end diff --git a/app/views/shared/piece_jointe/_pj_link.html.haml b/app/views/shared/piece_jointe/_pj_link.html.haml index 0c75f967c..1821ebf70 100644 --- a/app/views/shared/piece_jointe/_pj_link.html.haml +++ b/app/views/shared/piece_jointe/_pj_link.html.haml @@ -1,10 +1,10 @@ - if pj.attached? .pj-link - - if pj.virus_scanner.safe? || !pj.virus_scanner.analyzed? + - if pj.virus_scanner.safe? || !pj.virus_scanner.started? = link_to url_for(pj), target: '_blank', rel: 'noopener', title: "Télécharger la pièce jointe" do %span.icon.attachment = pj.filename.to_s - - if !pj.virus_scanner.analyzed? + - if !pj.virus_scanner.started? (ce fichier n’a pas été analysé par notre antivirus, téléchargez-le avec précaution) - else diff --git a/config/initializers/active_storage.rb b/config/initializers/active_storage.rb index 88dd9b3d4..121d0c3f1 100644 --- a/config/initializers/active_storage.rb +++ b/config/initializers/active_storage.rb @@ -10,16 +10,6 @@ ActiveStorage::Service.url_expires_in = 1.hour # The way analyzable blob interface work is by running the first accepted analyzer. # This is not what we want for the virus scan. Using analyzer interface is still beneficial # as it takes care of downloading the blob. -ActiveStorage::Attachment.class_eval do - after_create_commit :virus_scan - - private - - def virus_scan - ActiveStorage::VirusScanner.new(blob).analyze_later - end -end - ActiveStorage::Attached::One.class_eval do def virus_scanner if attached? @@ -27,3 +17,5 @@ ActiveStorage::Attached::One.class_eval do end end end + +ActiveSupport.on_load(:active_storage_blob) { include BlobVirusScanner } diff --git a/spec/controllers/gestionnaires/avis_controller_spec.rb b/spec/controllers/gestionnaires/avis_controller_spec.rb index 662937feb..074396ad1 100644 --- a/spec/controllers/gestionnaires/avis_controller_spec.rb +++ b/spec/controllers/gestionnaires/avis_controller_spec.rb @@ -59,23 +59,32 @@ describe Gestionnaires::AvisController, type: :controller do avis_without_answer.reload end - it { expect(response).to redirect_to(instruction_gestionnaire_avis_path(avis_without_answer)) } - it { expect(avis_without_answer.answer).to eq('answer') } - it { expect(avis_without_answer.piece_justificative_file).to_not be_attached } - it { expect(flash.notice).to eq('Votre réponse est enregistrée.') } + it 'should be ok' do + expect(response).to redirect_to(instruction_gestionnaire_avis_path(avis_without_answer)) + expect(avis_without_answer.answer).to eq('answer') + expect(avis_without_answer.piece_justificative_file).to_not be_attached + expect(flash.notice).to eq('Votre réponse est enregistrée.') + end end + describe 'with attachment' do + include ActiveJob::TestHelper let(:file) { Rack::Test::UploadedFile.new("./spec/fixtures/files/piece_justificative_0.pdf", 'application/pdf') } before do - post :update, params: { id: avis_without_answer.id, avis: { answer: 'answer', piece_justificative_file: file } } + expect(ClamavService).to receive(:safe_file?).and_return(true) + perform_enqueued_jobs do + post :update, params: { id: avis_without_answer.id, avis: { answer: 'answer', piece_justificative_file: file } } + end avis_without_answer.reload end - it { expect(response).to redirect_to(instruction_gestionnaire_avis_path(avis_without_answer)) } - it { expect(avis_without_answer.answer).to eq('answer') } - it { expect(avis_without_answer.piece_justificative_file).to be_attached } - it { expect(flash.notice).to eq('Votre réponse est enregistrée.') } + it 'should be ok' do + expect(response).to redirect_to(instruction_gestionnaire_avis_path(avis_without_answer)) + expect(avis_without_answer.answer).to eq('answer') + expect(avis_without_answer.piece_justificative_file).to be_attached + expect(flash.notice).to eq('Votre réponse est enregistrée.') + end end end diff --git a/spec/models/champ_spec.rb b/spec/models/champ_spec.rb index 7c8c895ec..73845b53a 100644 --- a/spec/models/champ_spec.rb +++ b/spec/models/champ_spec.rb @@ -388,7 +388,7 @@ describe Champ do champ.save end - it { expect(champ.piece_justificative_file.virus_scanner.analyzed?).to be_truthy } + it { expect(champ.piece_justificative_file.virus_scanner.started?).to be_truthy } end context 'and there is no blob' do diff --git a/spec/serializers/champ_serializer_spec.rb b/spec/serializers/champ_serializer_spec.rb index d9b24f62e..f2b4d2775 100644 --- a/spec/serializers/champ_serializer_spec.rb +++ b/spec/serializers/champ_serializer_spec.rb @@ -10,7 +10,7 @@ describe ChampSerializer do before do champ.piece_justificative_file.attach({ filename: __FILE__, io: File.open(__FILE__) }) - champ.piece_justificative_file.virus_scanner.analyze_later + champ.piece_justificative_file.blob.send(:enqueue_virus_scan) end after { champ.piece_justificative_file.purge }