FIX: Increase time of DOWNLOAD_URL_EXPIRES_AFTER_SECONDS to 5 minutes (#10160)

FIX: Increase time of DOWNLOAD_URL_EXPIRES_AFTER_SECONDS to 5 minutes (#10160)

  • Change S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS to 5 minutes, which controls presigned URL expiry and secure-media route cache time.
  • This is done because of the composer preview refreshing while typing causes a lot of requests sent to our server because of the short URL expiry. If this ends up being not enough we can always increase the time or explore other avenues (e.g. GitHub has a 7 day validity for secure URLs)
diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb
index 64fcd79..b1be8f1 100644
--- a/app/controllers/uploads_controller.rb
+++ b/app/controllers/uploads_controller.rb
@@ -153,8 +153,9 @@ class UploadsController < ApplicationController
       return render_404 if current_user.nil?
     end
 
+    # defaults to public: false, so only cached by the client browser
     cache_seconds = S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS - SECURE_REDIRECT_GRACE_SECONDS
-    expires_in cache_seconds.seconds # defaults to public: false, so only cached by the client browser
+    expires_in cache_seconds.seconds
 
     # url_for figures out the full URL, handling multisite DBs,
     # and will return a presigned URL for the upload
@@ -162,7 +163,9 @@ class UploadsController < ApplicationController
       return redirect_to Discourse.store.url_for(upload)
     end
 
-    redirect_to Discourse.store.signed_url_for_path(path_with_ext)
+    redirect_to Discourse.store.signed_url_for_path(
+      path_with_ext, expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS
+    )
   end
 
   def metadata
diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb
index 6dd6214..bdf5662 100644
--- a/lib/file_store/s3_store.rb
+++ b/lib/file_store/s3_store.rb
@@ -168,9 +168,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)
+    def signed_url_for_path(path, expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS)
       key = path.sub(absolute_base_url + "/", "")
-      presigned_url(key)
+      presigned_url(key, expires_in: expires_in)
     end
 
     def cache_avatar(avatar, user_id)
@@ -243,8 +243,14 @@ module FileStore
 
     private
 
-    def presigned_url(url, force_download: false, filename: false)
-      opts = { expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS }
+    def presigned_url(
+      url,
+      force_download: false,
+      filename: false,
+      expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS
+    )
+      opts = { expires_in: expires_in }
+
       if force_download && filename
         opts[:response_content_disposition] = ActionDispatch::Http::ContentDisposition.format(
           disposition: "attachment", filename: filename
diff --git a/lib/s3_helper.rb b/lib/s3_helper.rb
index 865e9b4..85cd804 100644
--- a/lib/s3_helper.rb
+++ b/lib/s3_helper.rb
@@ -8,7 +8,13 @@ class S3Helper
 
   attr_reader :s3_bucket_name, :s3_bucket_folder_path
 
-  DOWNLOAD_URL_EXPIRES_AFTER_SECONDS ||= 15
+  ##
+  # Controls the following:
+  #
+  # * cache time for secure-media URLs
+  # * expiry time for S3 presigned URLs, which include backup downloads and
+  #   any upload that has a private ACL (e.g. secure uploads)
+  DOWNLOAD_URL_EXPIRES_AFTER_SECONDS ||= 300
 
   def initialize(s3_bucket_name, tombstone_prefix = '', options = {})
     @s3_client = options.delete(:client)

GitHub sha: 8ef782bd

This commit appears in #10160 which was approved by davidtaylorhq. It was merged by martin.