Merge pull request #3884 from betagouv/dev

2019-05-16-01

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

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

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

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

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

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'

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 }

config/initializers/rack_mini_profiler.rb

@@ -0,0 +1 @@
+Rack::MiniProfiler.config.authorization_mode = :whitelist

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
 

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

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 }