diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 64fcd795b58..b1be8f1994a 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 6dd62140c7f..bdf56622cf1 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 865e9b4e6f1..85cd8043d3a 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)