FEATURE: Make S3 presigned GET URL expiry configurable (#16912)

Previously we hardcoded the DOWNLOAD_URL_EXPIRES_AFTER_SECONDS const
inside S3Helper to be 5 minutes (300 seconds). For various reasons,
some hosted sites may need this to be longer for other integrations.

The maximum expiry time for presigned URLs is 1 week (which is
604800 seconds), so that has been added as a validation on the
setting as well. The setting is hidden because 99% of the time
it should not be changed.

app/controllers/uploads_controller.rb

@@ -160,7 +160,7 @@ class UploadsController < ApplicationController
     end
 
     # defaults to public: false, so only cached by the client browser
-    cache_seconds = S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS - SECURE_REDIRECT_GRACE_SECONDS
+    cache_seconds = SiteSetting.s3_presigned_get_url_expires_after_seconds - SECURE_REDIRECT_GRACE_SECONDS
     expires_in cache_seconds.seconds
 
     # url_for figures out the full URL, handling multisite DBs,
@@ -171,7 +171,7 @@ class UploadsController < ApplicationController
 
     redirect_to Discourse.store.signed_url_for_path(
       path_with_ext,
-      expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS,
+      expires_in: SiteSetting.s3_presigned_get_url_expires_after_seconds,
       force_download: force_download?
     ), allow_other_host: true
   end

config/site_settings.yml

@@ -1376,6 +1376,11 @@ files:
   s3_configure_inventory_policy:
     default: true
     hidden: true
+  s3_presigned_get_url_expires_after_seconds:
+    default: 300
+    hidden: true
+    min: 60
+    max: 604800
   allow_profile_backgrounds:
     client: true
     default: true

lib/backup_restore/s3_backup_store.rb

@@ -132,7 +132,7 @@ module BackupRestore
     end
 
     def create_file_from_object(obj, include_download_source = false)
-      expires = S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS
+      expires = SiteSetting.s3_presigned_get_url_expires_after_seconds
       BackupFile.new(
         filename: File.basename(obj.key),
         size: obj.size,

lib/file_store/s3_store.rb

@@ -224,7 +224,7 @@ 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, force_download: false)
+    def signed_url_for_path(path, expires_in: SiteSetting.s3_presigned_get_url_expires_after_seconds, force_download: false)
       key = path.sub(absolute_base_url + "/", "")
       presigned_get_url(key, expires_in: expires_in, force_download: force_download)
     end
@@ -343,7 +343,7 @@ module FileStore
       url,
       force_download: false,
       filename: false,
-      expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS
+      expires_in: SiteSetting.s3_presigned_get_url_expires_after_seconds
     )
       opts = { expires_in: expires_in }
 

lib/s3_helper.rb

@@ -15,7 +15,8 @@ class S3Helper
   # * 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 ||= 5.minutes.to_i
+  #
+  # SiteSetting.s3_presigned_get_url_expires_after_seconds
 
   ##
   # Controls the following:

spec/lib/backup_restore/s3_backup_store_spec.rb

@@ -124,7 +124,7 @@ describe BackupRestore::S3BackupStore do
     bucket = Regexp.escape(SiteSetting.s3_backup_bucket)
     prefix = file_prefix(db_name, multisite)
     filename = Regexp.escape(filename)
-    expires = S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS
+    expires = SiteSetting.s3_presigned_get_url_expires_after_seconds
 
     /\Ahttps:\/\/#{bucket}.*#{prefix}\/#{filename}\?.*X-Amz-Expires=#{expires}.*X-Amz-Signature=.*\z/
   end

spec/lib/file_store/s3_store_spec.rb

@@ -448,7 +448,7 @@ describe FileStore::S3Store do
       s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
       s3_bucket.expects(:object).with(regexp_matches(%r{original/\d+X.*/#{upload.sha1}\.png})).returns(s3_object)
       opts = {
-        expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS,
+        expires_in: SiteSetting.s3_presigned_get_url_expires_after_seconds,
         response_content_disposition: %Q|attachment; filename="#{upload.original_filename}"; filename*=UTF-8''#{upload.original_filename}|
       }
 
@@ -463,7 +463,7 @@ describe FileStore::S3Store do
       s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
       s3_bucket.expects(:object).with("special/optimized/file.png").returns(s3_object)
       opts = {
-        expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS
+        expires_in: SiteSetting.s3_presigned_get_url_expires_after_seconds
       }
 
       s3_object.expects(:presigned_url).with(:get, opts)

spec/multisite/s3_store_spec.rb

@@ -186,7 +186,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
 
           s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
           s3_bucket.expects(:object).with("#{upload_path}/#{path}").returns(s3_object).at_least_once
-          s3_object.expects(:presigned_url).with(:get, expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS)
+          s3_object.expects(:presigned_url).with(:get, expires_in: SiteSetting.s3_presigned_get_url_expires_after_seconds)
 
           upload.url = store.store_upload(uploaded_file, upload)
           expect(upload.url).to eq(

spec/requests/uploads_controller_spec.rb

@@ -425,7 +425,7 @@ describe UploadsController do
           sign_in(user)
           get upload.short_path
 
-          expected_max_age = S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS - UploadsController::SECURE_REDIRECT_GRACE_SECONDS
+          expected_max_age = SiteSetting.s3_presigned_get_url_expires_after_seconds - UploadsController::SECURE_REDIRECT_GRACE_SECONDS
           expect(expected_max_age).to be > 0 # Sanity check that the constants haven't been set to broken values
 
           expect(response.headers["Cache-Control"]).to eq("max-age=#{expected_max_age}, private")