FIX: Respect force download when downloading secure media via lightbox (#10769)

The download link on the lightbox for images was not downloading the image if the upload was marked secure, because the code in the upload controller route was not respecting the dl=1 param for force download.

This PR fixes this so the download link works for secure images as well as regular ligthboxed images.

app/controllers/uploads_controller.rb

@@ -111,7 +111,7 @@ class UploadsController < ApplicationController
       if Discourse.store.internal?
         send_file_local_upload(upload)
       else
-        redirect_to Discourse.store.url_for(upload, force_download: params[:dl] == "1")
+        redirect_to Discourse.store.url_for(upload, force_download: force_download?)
       end
     else
       render_404
@@ -160,11 +160,13 @@ class UploadsController < ApplicationController
     # url_for figures out the full URL, handling multisite DBs,
     # and will return a presigned URL for the upload
     if path_with_ext.blank?
-      return redirect_to Discourse.store.url_for(upload)
+      return redirect_to Discourse.store.url_for(upload, force_download: force_download?)
     end
 
     redirect_to Discourse.store.signed_url_for_path(
-      path_with_ext, expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS
+      path_with_ext,
+      expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS,
+      force_download: force_download?
     )
   end
 
@@ -183,6 +185,10 @@ class UploadsController < ApplicationController
 
   protected
 
+  def force_download?
+    params[:dl] == "1"
+  end
+
   def xhr_not_allowed
     raise Discourse::InvalidParameters.new("XHR not allowed")
   end

lib/file_store/s3_store.rb

@@ -169,9 +169,9 @@ module FileStore
       url.sub(File.join("#{schema}#{absolute_base_url}", folder), File.join(SiteSetting.Upload.s3_cdn_url, "/"))
     end
 
-    def signed_url_for_path(path, expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS)
+    def signed_url_for_path(path, expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS, force_download: false)
       key = path.sub(absolute_base_url + "/", "")
-      presigned_url(key, expires_in: expires_in)
+      presigned_url(key, expires_in: expires_in, force_download: force_download)
     end
 
     def cache_avatar(avatar, user_id)

spec/requests/uploads_controller_spec.rb

@@ -402,6 +402,14 @@ describe UploadsController do
           get upload.short_path
 
           expect(response).to redirect_to(Discourse.store.signed_url_for_path(Discourse.store.get_path_for_upload(upload)))
+          expect(response.header['Location']).not_to include('response-content-disposition=attachment')
+        end
+
+        it "respects the force download (dl) param" do
+          sign_in(user)
+          freeze_time
+          get upload.short_path, params: { dl: '1' }
+          expect(response.header['Location']).to include('response-content-disposition=attachment')
         end
 
         it "has the correct caching header" do