FIX: Ensure lightbox image download has correct content disposition in S3 (#7845)

app/controllers/uploads_controller.rb

@@ -104,7 +104,7 @@ class UploadsController < ApplicationController
       if Discourse.store.internal?
         send_file_local_upload(upload)
       else
-        redirect_to Discourse.store.url_for(upload)
+        redirect_to Discourse.store.url_for(upload, force_download: params[:dl] == "1")
       end
     else
       render_404

lib/cooked_post_processor.rb

@@ -382,7 +382,7 @@ class CookedPostProcessor
     a = create_link_node("lightbox", img["src"])
     img.add_next_sibling(a)
 
-    if upload && Discourse.store.internal?
+    if upload
       a["data-download-href"] = Discourse.store.download_url(upload)
     end
 

lib/file_store/s3_store.rb

@@ -99,15 +99,23 @@ module FileStore
       File.join("uploads", "tombstone", RailsMultisite::ConnectionManagement.current_db, "/")
     end
 
+    def download_url(upload)
+      return unless upload
+      "#{upload.short_path}?dl=1"
+    end
+
     def path_for(upload)
       url = upload&.url
       FileStore::LocalStore.new.path_for(upload) if url && url[/^\/[^\/]/]
     end
 
-    def url_for(upload)
+    def url_for(upload, force_download: false)
-      if upload.private?
+      if upload.private? || force_download
+        opts = { expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS }
+        opts[:response_content_disposition] = "attachment; filename=\"#{upload.original_filename}\"" if force_download
+
         obj = @s3_helper.object(get_upload_key(upload))
-        url = obj.presigned_url(:get, expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS)
+        url = obj.presigned_url(:get, opts)
       else
         url = upload.url
       end

spec/components/file_store/s3_store_spec.rb

@@ -395,4 +395,31 @@ describe FileStore::S3Store do
     end
   end
 
+  describe ".download_url" do
+    include_context "s3 helpers"
+    let(:s3_object) { stub }
+
+    it "returns correct short URL with dl=1 param" do
+      expect(store.download_url(upload)).to eq("#{upload.short_path}?dl=1")
+    end
+  end
+
+  describe ".url_for" do
+    include_context "s3 helpers"
+    let(:s3_object) { stub }
+
+    it "returns signed URL with content disposition when requesting to download image" do
+      s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
+      s3_bucket.expects(:object).with("original/1X/#{upload.sha1}.png").returns(s3_object)
+      opts = {
+        expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS,
+        response_content_disposition: "attachment; filename=\"#{upload.original_filename}\""
+      }
+
+      s3_object.expects(:presigned_url).with(:get, opts)
+
+      expect(store.url_for(upload, force_download: true)).not_to eq(upload.url)
+    end
+  end
+
 end