Add FoiAttachmentMaskJob

Move all logic of censoring an attachment in this new job. Job is currently performed straight away but this will change coming commits.
mysociety · Jul 31, 2023 · 9974cf0 · 9974cf0
1 parent bcfaeeb
commit 9974cf0
Show file tree

Hide file tree

Showing 4 changed files with 111 additions and 60 deletions.
diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb
@@ -19,20 +19,7 @@ class AttachmentsController < ApplicationController
   around_action :cache_attachments
 
   def show
-    # Prevent spam to magic request address. Note that the binary
-    # substitution method used depends on the content type
-    body = @incoming_message.apply_masks(
-      @attachment.default_body,
-      @attachment.content_type
-    )
-
-    if content_type == 'text/html'
-      body =
-        Loofah.scrub_document(body, :prune).
-        to_html(encoding: 'UTF-8').
-        try(:html_safe)
-    end
-
+    body = FoiAttachmentMaskJob.perform_now(@attachment)
     render body: body, content_type: content_type
   end
 

diff --git a/app/jobs/foi_attachment_mask_job.rb b/app/jobs/foi_attachment_mask_job.rb
@@ -0,0 +1,44 @@
+##
+# Job to apply masks and censor rules to FoiAttachment objects. Masked file will
+# be stored as FoiAttachment#file ActiveStorage association.
+#
+# Example:
+#   FoiAttachmentMaskJob.perform(FoiAttachment.first)
+#
+class FoiAttachmentMaskJob < ApplicationJob
+  queue_as :default
+  unique :until_and_while_executing, on_conflict: :log
+
+  attr_reader :attachment
+
+  delegate :incoming_message, to: :attachment
+  delegate :info_request, to: :incoming_message
+
+  def perform(attachment)
+    @attachment = attachment
+
+    body = AlaveteliTextMasker.apply_masks(
+      attachment.default_body,
+      attachment.content_type,
+      masks
+    )
+
+    if attachment.content_type == 'text/html'
+      body =
+        Loofah.scrub_document(body, :prune).
+        to_html(encoding: 'UTF-8').
+        try(:html_safe)
+    end
+
+    body
+  end
+
+  private
+
+  def masks
+    {
+      censor_rules: info_request.applicable_censor_rules,
+      masks: info_request.masks
+    }
+  end
+end
diff --git a/spec/controllers/attachments_controller_spec.rb b/spec/controllers/attachments_controller_spec.rb
@@ -112,53 +112,12 @@ def show(params = {})
       expect(response.status).to eq(303)
     end
 
-    context 'when attachment is a HTML file' do
-      let(:attachment) do
-        FactoryBot.create(
-          :html_attachment,
-          incoming_message: message,
-          prominence: attachment_prominence
-        )
-      end
-
-      it 'should sanitise the output' do
-        show
-
-        # Nokogiri adds the meta tag; see
-        # https://github.com/sparklemotion/nokogiri/issues/1008
-        expected = <<-EOF.squish
-        <!DOCTYPE html>
-        <html>
-          <head>
-            <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
-          </head>
-          <body>dull
-          </body>
-        </html>
-        EOF
-
-        expect(response.body.squish).to eq(expected)
-      end
-    end
-
-    it 'censors attachments downloaded directly' do
-      info_request.censor_rules.create!(
-        text: 'hereisthetext', replacement: 'Mouse',
-        last_edit_editor: 'unknown', last_edit_comment: 'none'
-      )
-      show
-      expect(response.media_type).to eq('text/plain')
-      expect(response.body).to have_content 'Mouse'
-    end
-
-    it 'should censor with rules on the user (rather than the request)' do
-      info_request.user.censor_rules.create!(
-        text: 'hereisthetext', replacement: 'Mouse',
-        last_edit_editor: 'unknown', last_edit_comment: 'none'
-      )
+    it 'returns body from FoiAttachmentMaskJob' do
+      expect(FoiAttachmentMaskJob).to receive(:perform_now).
+        with(attachment).
+        and_return('Monkey')
       show
-      expect(response.media_type).to eq('text/plain')
-      expect(response.body).to have_content 'Mouse'
+      expect(response.body).to match('Monkey')
     end
 
     context 'when request is embargoed' do

diff --git a/spec/jobs/foi_attachment_mask_job_spec.rb b/spec/jobs/foi_attachment_mask_job_spec.rb
@@ -0,0 +1,61 @@
+require 'spec_helper'
+
+RSpec.describe FoiAttachmentMaskJob, type: :job do
+  let(:info_request) { FactoryBot.create(:info_request_with_html_attachment) }
+  let(:incoming_message) { info_request.incoming_messages.first }
+  let(:attachment) { incoming_message.foi_attachments.last }
+  let(:body) { described_class.new.perform(attachment) }
+
+  it 'sanitises HTML attachments' do
+    # Nokogiri adds the meta tag; see
+    # https://github.com/sparklemotion/nokogiri/issues/1008
+    expected = <<-EOF.squish
+    <!DOCTYPE html>
+    <html>
+      <head>
+        <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
+      </head>
+      <body>dull
+      </body>
+    </html>
+    EOF
+
+    expect(body.squish).to eq(expected)
+  end
+
+  it 'censors attachments downloaded directly' do
+    info_request.censor_rules.create!(
+      text: 'dull', replacement: 'Boy',
+      last_edit_editor: 'unknown', last_edit_comment: 'none'
+    )
+    expect(body).to_not include 'dull'
+    expect(body).to include 'Boy'
+  end
+
+  it 'censors with rules on the user (rather than the request)' do
+    info_request.user.censor_rules.create!(
+      text: 'dull', replacement: 'Mole',
+      last_edit_editor: 'unknown', last_edit_comment: 'none'
+    )
+    expect(body).to_not include 'dull'
+    expect(body).to include 'Mole'
+  end
+
+  it 'censors with rules on the public body (rather than the request)' do
+    info_request.public_body.censor_rules.create!(
+      text: 'dull', replacement: 'Fox',
+      last_edit_editor: 'unknown', last_edit_comment: 'none'
+    )
+    expect(body).to_not include 'dull'
+    expect(body).to include 'Fox'
+  end
+
+  it 'censors with rules globally (rather than the request)' do
+    CensorRule.create!(
+      text: 'dull', replacement: 'Horse',
+      last_edit_editor: 'unknown', last_edit_comment: 'none'
+    )
+    expect(body).to_not include 'dull'
+    expect(body).to include 'Horse'
+  end
+end