Fixes #4479 - PDF attachments should not be opened without downloading them first.

app/assets/javascripts/app/controllers/ticket_zoom/article_view.coffee

@@ -142,7 +142,7 @@ class ArticleViewItem extends App.ControllerObserver
       for attachment in article.attachments
 
         dispositionParams = ''
-        if attachment?.preferences['Content-Type'] isnt 'application/pdf' && attachment?.preferences['Content-Type'] isnt 'text/html'
+        if attachment?.preferences['Content-Type'] isnt 'text/html'
           dispositionParams = '?disposition=attachment'
 
         attachment.url = "#{App.Config.get('api_path')}/ticket_attachment/#{article.ticket_id}/#{article.id}/#{attachment.id}#{dispositionParams}"

config/application.rb

@@ -63,6 +63,9 @@ module Zammad
 
     config.active_record.use_yaml_unsafe_load = true
 
+    # Remove PDF from the allowed inline content types so they have to be downloaded first (#4479).
+    config.active_storage.content_types_allowed_inline.delete('application/pdf')
+
     # Use custom logger to log Thread id next to Process pid
     config.log_formatter = ::Logger::Formatter.new
 

spec/controllers/application_controller/has_download/download_file_spec.rb

@@ -5,9 +5,9 @@ require 'rails_helper'
 RSpec.describe ApplicationController::HasDownload::DownloadFile do
   subject(:download_file) { described_class.new(stored_file.id, disposition: 'inline') }
 
-  let(:file_content_type) { 'application/pdf' }
+  let(:file_content_type) { 'image/jpeg' }
   let(:file_data)         { 'A example file.' }
-  let(:file_name)         { 'example.pdf' }
+  let(:file_name)         { 'example.jpg' }
 
   let(:stored_file) do
     create(:store,
@@ -22,7 +22,13 @@ RSpec.describe ApplicationController::HasDownload::DownloadFile do
   end
 
   describe '#disposition' do
-    context "with given object dispostion 'inline'" do
+    shared_examples 'forcing disposition to attachment' do
+      it 'forces disposition to attachment' do
+        expect(download_file.disposition).to eq('attachment')
+      end
+    end
+
+    context "with given object disposition 'inline'" do
       context 'with allowed inline content type (from ActiveStorage.content_types_allowed_inline)' do
         it 'disposition is inline' do
           expect(download_file.disposition).to eq('inline')
@@ -32,25 +38,27 @@ RSpec.describe ApplicationController::HasDownload::DownloadFile do
       context 'with binary content type (ActiveStorage.content_types_to_serve_as_binary)' do
         let(:file_content_type) { 'image/svg+xml' }
 
-        it 'disposition forced to attachment' do
-          expect(download_file.disposition).to eq('attachment')
-        end
+        it_behaves_like 'forcing disposition to attachment'
+      end
+
+      context 'with PDF content type (#4479)' do
+        let(:file_content_type) { 'application/pdf' }
+
+        it_behaves_like 'forcing disposition to attachment'
       end
     end
 
     context "with given object dispostion 'attachment'" do
       subject(:download_file) { described_class.new(stored_file.id, disposition: 'attachment') }
 
-      it 'disposition is attachment' do
-        expect(download_file.disposition).to eq('attachment')
-      end
+      it_behaves_like 'forcing disposition to attachment'
     end
   end
 
   describe '#content_type' do
     context 'with none binary content type' do
       it 'check content type' do
-        expect(download_file.content_type).to eq('application/pdf')
+        expect(download_file.content_type).to eq('image/jpeg')
       end
     end
 
@@ -72,8 +80,8 @@ RSpec.describe ApplicationController::HasDownload::DownloadFile do
 
     context 'with image content type' do
       let(:file_content_type) { 'image/jpg' }
-      let(:file_data) { Rails.root.join('test/data/upload/upload2.jpg').binread }
-      let(:file_name) { 'image.jpg' }
+      let(:file_data)         { Rails.root.join('test/data/upload/upload2.jpg').binread }
+      let(:file_name)         { 'image.jpg' }
 
       it 'check that inline content will be returned' do
         expect(download_file.content('inline')).to not_eq(file_data)

spec/requests/ticket/article_attachments_spec.rb

@@ -99,7 +99,7 @@ RSpec.describe 'Ticket Article Attachments', authenticated_as: -> { agent }, typ
         end
 
         context 'with allowed inline file content type' do
-          let(:store_file_content_type) { 'application/pdf' }
+          let(:store_file_content_type) { 'image/jpeg' }
 
           it 'disposition is inline' do
             get "/api/v1/ticket_attachment/#{ticket1.id}/#{article1.id}/#{store_file.id}?disposition=inline", params: {}
@@ -107,11 +107,20 @@ RSpec.describe 'Ticket Article Attachments', authenticated_as: -> { agent }, typ
           end
         end
 
+        context 'with PDF content type (#4479)' do
+          let(:store_file_content_type) { 'application/pdf' }
+
+          it 'disposition is attachment' do
+            get "/api/v1/ticket_attachment/#{ticket1.id}/#{article1.id}/#{store_file.id}?disposition=inline", params: {}
+            expect(response.headers['Content-Disposition']).to include('attachment')
+          end
+        end
+
         context 'with calendar preview' do
           let(:store_file_content) do
             Rails.root.join('spec/fixtures/files/calendar/basic.ics').read
           end
-          let(:store_file_name) { 'basic.ics' }
+          let(:store_file_name)         { 'basic.ics' }
           let(:store_file_content_type) { 'text/calendar' }
 
           let(:expected_event) do