Merge branch 'refactor-attachment-mask-referer' into develop

app/controllers/attachment_masks_controller.rb

@@ -4,14 +4,14 @@
 #
 class AttachmentMasksController < ApplicationController
   before_action :set_no_crawl_headers
-  before_action :find_attachment
-  before_action :ensure_referer, :ensure_attachment
+  before_action :decode_referer, :ensure_referer
+  before_action :find_attachment, :ensure_attachment
 
   def wait
     if @attachment.masked?
       redirect_to done_attachment_mask_path(
         id: @attachment.to_signed_global_id,
-        referer: params[:referer]
+        referer: verifier.generate(@referer)
       )
 
     else
@@ -23,11 +23,9 @@ def done
     unless @attachment.masked?
       redirect_to wait_for_attachment_mask_path(
         id: @attachment.to_signed_global_id,
-        referer: params[:referer]
+        referer: verifier.generate(@referer)
       )
     end
-
-    @show_attachment_path = params[:referer]
   end
 
   private
@@ -36,16 +34,24 @@ def set_no_crawl_headers
     headers['X-Robots-Tag'] = 'noindex'
   end
 
+  def decode_referer
+    @referer = verifier.verified(params[:referer])
+  end
+
   def find_attachment
     @attachment = GlobalID::Locator.locate_signed(params[:id])
   rescue ActiveRecord::RecordNotFound
   end
 
   def ensure_referer
-    raise RouteNotFound unless params[:referer].present?
+    raise RouteNotFound unless @referer
   end
 
   def ensure_attachment
-    redirect_to(params[:referer]) unless @attachment
+    redirect_to(@referer) unless @attachment
+  end
+
+  def verifier
+    Rails.application.message_verifier('AttachmentsController')
   end
 end

app/controllers/attachments_controller.rb

@@ -36,7 +36,7 @@ def show
   rescue Timeout::Error
     redirect_to wait_for_attachment_mask_path(
       @attachment.to_signed_global_id,
-      referer: request.fullpath
+      referer: verifier.generate(request.fullpath)
     )
   end
 
@@ -224,4 +224,8 @@ def prominence
   def with_prominence
     @info_request
   end
+
+  def verifier
+    Rails.application.message_verifier('AttachmentsController')
+  end
 end

app/views/attachment_masks/done.html.erb

@@ -7,5 +7,5 @@
 </p>
 
 <p>
-  <%= link_to _('Download attachment'), @show_attachment_path, class: 'button' %>
+  <%= link_to _('Download attachment'), @referer, class: 'button' %>
 </p>

spec/controllers/attachment_masks_controller_spec.rb

@@ -2,11 +2,17 @@
 
 RSpec.describe AttachmentMasksController, type: :controller do
   let(:attachment) { FactoryBot.build(:body_text, id: 1) }
-  let(:referer) { '/referer' }
+  let(:referer) { 'DEF' }
 
   before do
     allow(GlobalID::Locator).to receive(:locate_signed).with('ABC').
       and_return(attachment)
+
+    verifier = double('ActiveSupport::MessageVerifier')
+    allow(controller).to receive(:verifier).and_return(verifier)
+    allow(verifier).to receive(:generate).with('/referer').and_return('DEF')
+    allow(verifier).to receive(:verified).and_return(nil)
+    allow(verifier).to receive(:verified).with('DEF').and_return('/referer')
   end
 
   describe 'GET wait' do
@@ -19,7 +25,7 @@ def wait
         allow(attachment).to receive(:to_signed_global_id).and_return('ABC')
         wait
         expect(response).to redirect_to(
-          done_attachment_mask_path('ABC', referer: '/referer')
+          done_attachment_mask_path('ABC', referer: 'DEF')
         )
       end
     end
@@ -69,6 +75,14 @@ def wait
         expect { wait }.to raise_error(ApplicationController::RouteNotFound)
       end
     end
+
+    context 'with modified referer' do
+      let(:referer) { 'http://example.com/attack' }
+
+      it 'raises route not found error' do
+        expect { wait }.to raise_error(ApplicationController::RouteNotFound)
+      end
+    end
   end
 
   describe 'GET done' do
@@ -83,7 +97,7 @@ def done
         allow(attachment).to receive(:to_signed_global_id).and_return('ABC')
         done
         expect(response).to redirect_to(
-          wait_for_attachment_mask_path('ABC', referer: '/referer')
+          wait_for_attachment_mask_path('ABC', referer: 'DEF')
         )
       end
     end
@@ -125,5 +139,13 @@ def done
         expect { done }.to raise_error(ApplicationController::RouteNotFound)
       end
     end
+
+    context 'with modified referer' do
+      let(:referer) { 'http://example.com/attack' }
+
+      it 'raises route not found error' do
+        expect { done }.to raise_error(ApplicationController::RouteNotFound)
+      end
+    end
   end
 end

spec/controllers/attachments_controller_spec.rb

@@ -138,9 +138,21 @@ def show(params = {})
         it 'redirects to wait for attachment mask route' do
           allow_any_instance_of(FoiAttachment).to receive(:to_signed_global_id).
             and_return('ABC')
+
+          verifier = double('ActiveSupport::MessageVerifier')
+          allow(controller).to receive(:verifier).and_return(verifier)
+          allow(verifier).to receive(:generate).with(
+            get_attachment_path(
+              incoming_message_id: attachment.incoming_message_id,
+              id: info_request.id,
+              part: attachment.url_part_number,
+              file_name: attachment.filename
+            )
+          ).and_return('DEF')
+
           show
           expect(response).to redirect_to(
-            wait_for_attachment_mask_path('ABC', referer: request.fullpath)
+            wait_for_attachment_mask_path('ABC', referer: 'DEF')
           )
         end
       end