Trigger virus scanning after completing direct upload to S3 storage

https://community.openproject.org/wp/67664

When storing attachments on S3, the file is not fully uploaded until the
direct upload finishes (hook called from client browser when file is
finished uploading).

The `FinishDirectUploadService#schedule_jobs` now calls
`attachment.enqueue_jobs` to ensure the same jobs are run on attachment
creation (useful when s3 storage is not used) and on attachment upload
completion. This means virus scanning job is now triggered correctly.
This commit is contained in:
Christophe Bliard
2025-09-24 12:21:58 +02:00
parent 7a68b8eb9d
commit 643a58f801
2 changed files with 58 additions and 16 deletions
@@ -91,7 +91,7 @@ module Attachments
end
def schedule_jobs
attachment.extract_fulltext
attachment.enqueue_jobs
end
def journalize_container
@@ -43,8 +43,8 @@ RSpec.describe Attachments::FinishDirectUploadJob, "integration", type: :job do
let(:job) { described_class.new }
shared_examples_for "turning pending attachment into a standard attachment" do
it do
shared_examples_for "completing direct upload of attachment" do |expect_extract_fulltext_job: true|
it "turns the pending attachment into a standard attachment" do
job.perform(pending_attachment.id)
attachment = Attachment.find(pending_attachment.id)
@@ -58,10 +58,8 @@ RSpec.describe Attachments::FinishDirectUploadJob, "integration", type: :job do
expect(attachment.digest)
.to eql("9473fdd0d880a43c21b7778d34872157")
end
end
shared_examples_for "adding a journal to the attachment in the name of the attachment's author" do
it do
it "adds a journal to the attachment in the name of the attachment's author" do
job.perform(pending_attachment.id)
journals = Attachment.find(pending_attachment.id).journals
@@ -72,14 +70,60 @@ RSpec.describe Attachments::FinishDirectUploadJob, "integration", type: :job do
expect(journals.last.user)
.to eql(pending_attachment.author)
end
if expect_extract_fulltext_job
it "enqueues the ExtractFulltextJob job" do
clear_enqueued_jobs
job.perform(pending_attachment.id)
expect(Attachments::ExtractFulltextJob)
.to have_been_enqueued
.with(pending_attachment.id)
end
else
it "does not enqueue the ExtractFulltextJob job" do
clear_enqueued_jobs
job.perform(pending_attachment.id)
expect(Attachments::ExtractFulltextJob)
.not_to have_been_enqueued
end
end
context "when antivirus scan is disabled (default)",
with_settings: { antivirus_scan_mode: :disabled } do
it "does not enqueue the VirusScanJob job" do
clear_enqueued_jobs
job.perform(pending_attachment.id)
expect(Attachments::VirusScanJob)
.not_to have_been_enqueued
end
end
context "when antivirus scan is enabled",
with_ee: %i[virus_scanning],
with_settings: { antivirus_scan_mode: :clamav_socket } do
it "enqueues the VirusScanJob job" do
clear_enqueued_jobs
job.perform(pending_attachment.id)
expect(Attachments::VirusScanJob)
.to have_been_enqueued
.with(pending_attachment)
end
end
end
context "for a journalized container" do
let!(:container) { create(:work_package) }
let!(:container_timestamp) { container.updated_at }
it_behaves_like "turning pending attachment into a standard attachment"
it_behaves_like "adding a journal to the attachment in the name of the attachment's author"
it_behaves_like "completing direct upload of attachment"
it "adds a journal to the container in the name of the attachment's author" do
job.perform(pending_attachment.id)
@@ -125,15 +169,13 @@ RSpec.describe Attachments::FinishDirectUploadJob, "integration", type: :job do
context "for a non journalized container" do
let!(:container) { create(:wiki_page) }
it_behaves_like "turning pending attachment into a standard attachment"
it_behaves_like "adding a journal to the attachment in the name of the attachment's author"
it_behaves_like "completing direct upload of attachment", expect_extract_fulltext_job: false
end
context "for a nil container" do
let!(:container) { nil }
it_behaves_like "turning pending attachment into a standard attachment"
it_behaves_like "adding a journal to the attachment in the name of the attachment's author"
it_behaves_like "completing direct upload of attachment"
end
context "for an attachment that has been removed in the meantime" do
@@ -153,7 +195,7 @@ RSpec.describe Attachments::FinishDirectUploadJob, "integration", type: :job do
let!(:container) { create(:work_package) }
let!(:container_timestamp) { container.updated_at }
it "Does not save the attachment" do
it "does not save the attachment" do
allow(pending_attachment).to receive(:save!)
allow(OpenProject.logger).to receive(:error)
@@ -169,13 +211,13 @@ RSpec.describe Attachments::FinishDirectUploadJob, "integration", type: :job do
end
context "when the job is getting a allowlist override" do
it "Does save the attachment" do
it "does save the attachment" do
job.perform(pending_attachment.id, allowlist: false)
container.reload
expect(container.attachments.count).to eq 1
expect { pending_attachment.reload }.not_to raise_error(ActiveRecord::RecordNotFound)
expect { pending_attachment.reload }.not_to raise_error
expect(pending_attachment.downloads).to eq 0
end
@@ -187,7 +229,7 @@ RSpec.describe Attachments::FinishDirectUploadJob, "integration", type: :job do
shared_let(:user) { create(:user) }
let!(:container) { create(:work_package) }
it "Does not save the attachment" do
it "does not save the attachment" do
allow(pending_attachment).to receive(:save!)
job.perform(pending_attachment.id)