Merge pull request #5959 from betagouv/main

2021-03-04-03

app/graphql/schema.graphql

@@ -103,7 +103,7 @@ enum AddressType {
 }
 
 type Association {
-  dateCreation: ISO8601Date!
+  dateCreation: ISO8601Date
   dateDeclaration: ISO8601Date!
   datePublication: ISO8601Date!
   objet: String!

app/graphql/types/personne_morale_type.rb

@@ -75,7 +75,7 @@ module Types
       field :rna, String, null: false
       field :titre, String, null: false
       field :objet, String, null: false
-      field :date_creation, GraphQL::Types::ISO8601Date, null: false
+      field :date_creation, GraphQL::Types::ISO8601Date, null: true
       field :date_declaration, GraphQL::Types::ISO8601Date, null: false
       field :date_publication, GraphQL::Types::ISO8601Date, null: false
     end

app/models/concerns/blob_titre_identite_watermark_concern.rb

@@ -1,8 +1,19 @@
+# Request a watermark on blobs attached to a `Champs::TitreIdentiteChamp`
+# after the virus scan has run.
+#
+# We're using a class extension here, but we could as well have a periodic
+# job that watermarks relevant attachments.
+#
+# The `after_commit` hook is triggered, among other cases, when
+# the analyzer or virus scan updates the blob metadata. When both the analyzer
+# and the virus scan have run, it is now safe to start the watermarking,
+# without  risking to replace the picture while it is being scanned in a
+# concurrent job.
 module BlobTitreIdentiteWatermarkConcern
   extend ActiveSupport::Concern
 
   included do
-    after_update_commit :enqueue_watermark_job
+    after_commit :enqueue_watermark_job
   end
 
   def watermark_pending?
@@ -12,7 +23,7 @@ module BlobTitreIdentiteWatermarkConcern
   private
 
   def watermark_required?
-    attachments.find { |attachment| attachment.record.class.name == 'Champs::TitreIdentiteChamp' }
+    attachments.any? { |attachment| attachment.record.class.name == 'Champs::TitreIdentiteChamp' }
   end
 
   def watermark_done?

app/models/concerns/blob_virus_scanner_concern.rb

@@ -1,13 +1,21 @@
-# TODO: once we're using Rails 6, use the hooks on attachments creation
+# Run a virus scan on all blobs after they are analyzed.
-# (rather than on blob creation).
+#
-# This will help to avoid cloberring metadata accidentally (as metadata
+# We're using a class extension to ensure that all blobs get scanned,
-# are more stable on attachment creation than on blob creation).
+# regardless on how they were created. This could be an ActiveStorage::Analyzer,
+# but as of Rails 6.1 only the first matching analyzer is ever run on
+# a blob (and we may want to analyze the dimension of a picture as well
+# as scanning it).
+#
+# The `after_commit` hook is triggered, among other cases, when
+# the analyzer updates the blob metadata. When the analyzer has run,
+# it is now safe to start our own scanning, without risking to have
+# two concurrent jobs overwriting the metadata of the blob.
 module BlobVirusScannerConcern
   extend ActiveSupport::Concern
 
   included do
     before_create :set_pending
-    after_update_commit :enqueue_virus_scan
+    after_commit :enqueue_virus_scan
   end
 
   def virus_scanner

app/models/procedure.rb

@@ -202,7 +202,7 @@ class Procedure < ApplicationRecord
     "image/jpg",
     "image/png",
     "text/plain"
-  ], size: { less_than: 20.megabytes }
+  ], size: { less_than: 20.megabytes }, if: -> { new_record? || created_at > Date.new(2020, 2, 27) }
 
   validates :deliberation, content_type: [
     "application/msword",
@@ -213,9 +213,12 @@ class Procedure < ApplicationRecord
     "image/jpg",
     "image/png",
     "text/plain"
-  ], size: { less_than: 20.megabytes }
+  ], size: { less_than: 20.megabytes }, if: -> { new_record? || created_at > Date.new(2020, 4, 29) }
+
+  validates :logo, content_type: ['image/png', 'image/jpg', 'image/jpeg'],
+    size: { less_than: 5.megabytes },
+    if: -> { new_record? || created_at > Date.new(2020, 11, 13) }
 
-  validates :logo, content_type: ['image/png', 'image/jpg', 'image/jpeg'], size: { less_than: 5.megabytes }
   validates :api_entreprise_token, jwt_token: true, allow_blank: true
 
   before_save :update_juridique_required

spec/models/champ_spec.rb

@@ -1,4 +1,6 @@
 describe Champ do
+  include ActiveJob::TestHelper
+
   require 'models/champ_shared_example.rb'
 
   it_should_behave_like "champ_spec"
@@ -435,19 +437,56 @@ describe Champ do
     end
   end
 
-  describe '#enqueue_virus_check' do
+  describe '#enqueue_virus_scan' do
-    let(:champ) { build(:champ_piece_justificative, type_de_champ: type_de_champ) }
-
     context 'when type_champ is type_de_champ_piece_justificative' do
       let(:type_de_champ) { create(:type_de_champ_piece_justificative) }
+      let(:champ) { build(:champ_piece_justificative, type_de_champ: type_de_champ) }
 
       context 'and there is a blob' do
         before do
-          champ.piece_justificative_file.attach(io: StringIO.new("toto"), filename: "toto.txt", content_type: "text/plain")
+          allow(ClamavService).to receive(:safe_file?).and_return(true)
-          champ.save!
         end
 
-        it { expect(champ.piece_justificative_file.virus_scanner.started?).to be_truthy }
+        subject do
+          champ.piece_justificative_file.attach(io: StringIO.new("toto"), filename: "toto.txt", content_type: "text/plain")
+          champ.save!
+          champ
+        end
+
+        it 'marks the file as pending virus scan' do
+          expect(subject.piece_justificative_file.virus_scanner.started?).to be_truthy
+        end
+
+        it 'marks the file as safe once the scan completes' do
+          perform_enqueued_jobs { subject }
+          expect(champ.reload.piece_justificative_file.virus_scanner.safe?).to be_truthy
+        end
+      end
+    end
+  end
+
+  describe '#enqueue_watermark_job' do
+    context 'when type_champ is type_de_champ_titre_identite' do
+      let(:type_de_champ) { create(:type_de_champ_titre_identite) }
+      let(:champ) { build(:champ_titre_identite, type_de_champ: type_de_champ) }
+
+      before do
+        allow(ClamavService).to receive(:safe_file?).and_return(true)
+      end
+
+      subject do
+        champ.piece_justificative_file.attach(fixture_file_upload('spec/fixtures/files/logo_test_procedure.png', 'image/png'))
+        champ.save!
+        champ
+      end
+
+      it 'enqueues a watermark job on file attachment' do
+        expect(subject.piece_justificative_file.watermark_pending?).to be_truthy
+      end
+
+      it 'watermarks the file' do
+        perform_enqueued_jobs { subject }
+        expect(champ.reload.piece_justificative_file.blob.metadata[:watermark]).to be_truthy
       end
     end
   end
@@ -535,7 +574,6 @@ describe Champ do
     end
 
     context "fetch_external_data_later" do
-      include ActiveJob::TestHelper
       let(:data) { 'some other data' }
 
       it "fill data from external source" do