From 4eb4293e666939cb4ad02d399f8d8560fa22d704 Mon Sep 17 00:00:00 2001
From: Martin Brennan <mjrbrennan@gmail.com>
Date: Mon, 16 Mar 2020 11:54:14 +1000
Subject: [PATCH] FIX: Ensure show_short URLs handle secure uploads using
 multisite (#9212)

Meta report: https://meta.discourse.org/t/short-url-secure-uploads-s3/144224
* if the show_short route is hit for an upload that is
  secure, we redirect to the secure presigned URL. however
  this was not taking into account multisite so the db name
  was left off the path which broke the presigned URL
* we now use the correct url_for method if we know the
  upload (like in the show_short case) which takes into
  account multisite
---
 app/controllers/uploads_controller.rb    | 14 +++++++++++---
 spec/requests/uploads_controller_spec.rb | 12 +++++++++++-
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb
index 04840a36a16..ad72f998c9e 100644
--- a/app/controllers/uploads_controller.rb
+++ b/app/controllers/uploads_controller.rb
@@ -102,7 +102,9 @@ class UploadsController < ApplicationController
     sha1 = Upload.sha1_from_base62_encoded(params[:base62])
 
     if upload = Upload.find_by(sha1: sha1)
-      return handle_secure_upload_request(upload, Discourse.store.get_path_for_upload(upload)) if upload.secure? && SiteSetting.secure_media?
+      if upload.secure? && SiteSetting.secure_media?
+        return handle_secure_upload_request(upload)
+      end
 
       if Discourse.store.internal?
         send_file_local_upload(upload)
@@ -128,7 +130,6 @@ class UploadsController < ApplicationController
     upload = Upload.find_by(sha1: sha1)
     return render_404 if upload.blank?
 
-    signed_secure_url = Discourse.store.signed_url_for_path(path_with_ext)
     return handle_secure_upload_request(upload, path_with_ext) if SiteSetting.secure_media?
 
     # we don't want to 404 here if secure media gets disabled
@@ -138,14 +139,21 @@ class UploadsController < ApplicationController
     # if the upload is still secure, that means the ACL is probably still
     # private, so we don't want to go to the CDN url just yet otherwise we
     # will get a 403. if the upload is not secure we assume the ACL is public
+    signed_secure_url = Discourse.store.signed_url_for_path(path_with_ext)
     redirect_to upload.secure? ? signed_secure_url : Discourse.store.cdn_url(upload.url)
   end
 
-  def handle_secure_upload_request(upload, path_with_ext)
+  def handle_secure_upload_request(upload, path_with_ext = nil)
     if upload.access_control_post_id.present?
       raise Discourse::InvalidAccess if !guardian.can_see?(upload.access_control_post)
     end
 
+    # 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)
+    end
+
     redirect_to Discourse.store.signed_url_for_path(path_with_ext)
   end
 
diff --git a/spec/requests/uploads_controller_spec.rb b/spec/requests/uploads_controller_spec.rb
index e27a4d4671e..d4c4e875478 100644
--- a/spec/requests/uploads_controller_spec.rb
+++ b/spec/requests/uploads_controller_spec.rb
@@ -397,9 +397,19 @@ describe UploadsController do
           upload.update(access_control_post: post)
 
           get upload.short_path
-
           expect(response.code).to eq("403")
         end
+
+        context "when running on a multisite connection" do
+          before do
+            Rails.configuration.multisite = true
+          end
+          it "redirects to the signed_url_for_path with the multisite DB name in the url" do
+            freeze_time
+            get upload.short_path
+            expect(response.body).to include(RailsMultisite::ConnectionManagement.current_db)
+          end
+        end
       end
     end
   end