diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 9c87a938690..36d618cf26d 100644 --- a/app/controllers/uploads_controller.rb +++ b/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 diff --git a/config/site_settings.yml b/config/site_settings.yml index 483ec6d3b4f..658c4cefff6 100644 --- a/config/site_settings.yml +++ b/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 diff --git a/lib/backup_restore/s3_backup_store.rb b/lib/backup_restore/s3_backup_store.rb index 40326f7019d..591eb2ed067 100644 --- a/lib/backup_restore/s3_backup_store.rb +++ b/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, diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb index 54d8dff720f..074bf924468 100644 --- a/lib/file_store/s3_store.rb +++ b/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 } diff --git a/lib/s3_helper.rb b/lib/s3_helper.rb index 12bcd22a3a9..6c6ea76f87a 100644 --- a/lib/s3_helper.rb +++ b/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: diff --git a/spec/lib/backup_restore/s3_backup_store_spec.rb b/spec/lib/backup_restore/s3_backup_store_spec.rb index b7e39a9f4d3..4247001a0a1 100644 --- a/spec/lib/backup_restore/s3_backup_store_spec.rb +++ b/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 diff --git a/spec/lib/file_store/s3_store_spec.rb b/spec/lib/file_store/s3_store_spec.rb index 91ab3b9f109..639927a95d4 100644 --- a/spec/lib/file_store/s3_store_spec.rb +++ b/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) diff --git a/spec/multisite/s3_store_spec.rb b/spec/multisite/s3_store_spec.rb index 28a1941b97c..c0e66060e3c 100644 --- a/spec/multisite/s3_store_spec.rb +++ b/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( diff --git a/spec/requests/uploads_controller_spec.rb b/spec/requests/uploads_controller_spec.rb index d54cd871449..d9235f9a66b 100644 --- a/spec/requests/uploads_controller_spec.rb +++ b/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")