and an external upload#{I18n.t("emails.secure_media_placeholder")}
") + end + end + + raw + end + def self.get_context_posts(post, topic_user, user) if (user.user_option.email_previous_replies == UserOption.previous_replies_type[:never]) || SiteSetting.private_email? diff --git a/app/models/post.rb b/app/models/post.rb index 06c71b524c9..1763290a0dd 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -300,6 +300,15 @@ class Post < ActiveRecord::Base options[:user_id] = post_user.id if post_user options[:omit_nofollow] = true if omit_nofollow? + if self.with_secure_media? + each_upload_url do |url| + uri = URI.parse(url) + if FileHelper.is_supported_media?(File.basename(uri.path)) + raw = raw.sub(Discourse.store.s3_upload_host, "#{Discourse.base_url}/secure-media-uploads") + end + end + end + cooked = post_analyzer.cook(raw, options) new_cooked = Plugin::Filter.apply(:after_post_cook, self, cooked) @@ -492,6 +501,11 @@ class Post < ActiveRecord::Base ReviewableFlaggedPost.pending.find_by(target: self) end + def with_secure_media? + return false unless SiteSetting.secure_media? + topic&.private_message? || SiteSetting.login_required? + end + def hide!(post_action_type_id, reason = nil) return if hidden? @@ -882,6 +896,13 @@ class Post < ActiveRecord::Base end upload_ids |= Upload.where(id: downloaded_images.values).pluck(:id) + + disallowed_uploads = [] + if SiteSetting.secure_media? && !topic&.private_message? + disallowed_uploads = Upload.where(id: upload_ids, secure: true).pluck(:original_filename) + end + return disallowed_uploads if disallowed_uploads.count > 0 + values = upload_ids.map! { |upload_id| "(#{self.id},#{upload_id})" }.join(",") PostUpload.transaction do @@ -893,6 +914,12 @@ class Post < ActiveRecord::Base end end + def update_uploads_secure_status + if Discourse.store.external? + self.uploads.each { |upload| upload.update_secure_status } + end + end + def downloaded_images JSON.parse(self.custom_fields[Post::DOWNLOADED_IMAGES].presence || "{}") rescue JSON::ParserError @@ -909,6 +936,7 @@ class Post < ActiveRecord::Base ] fragments ||= Nokogiri::HTML::fragment(self.cooked) + links = fragments.css("a/@href", "img/@src").map do |media| src = media.value next if src.blank? diff --git a/app/models/topic_converter.rb b/app/models/topic_converter.rb index 650333749d7..ec54140a297 100644 --- a/app/models/topic_converter.rb +++ b/app/models/topic_converter.rb @@ -30,9 +30,9 @@ class TopicConverter ) update_user_stats + update_post_uploads_secure_status Jobs.enqueue(:topic_action_converter, topic_id: @topic.id) Jobs.enqueue(:delete_inaccessible_notifications, topic_id: @topic.id) - watch_topic(topic) end @topic @@ -49,6 +49,7 @@ class TopicConverter ) add_allowed_users + update_post_uploads_secure_status Jobs.enqueue(:topic_action_converter, topic_id: @topic.id) Jobs.enqueue(:delete_inaccessible_notifications, topic_id: @topic.id) @@ -97,4 +98,11 @@ class TopicConverter end end + def update_post_uploads_secure_status + @topic.posts.each do |post| + next if post.uploads.empty? + post.update_uploads_secure_status + post.rebake! + end + end end diff --git a/app/models/topic_link.rb b/app/models/topic_link.rb index 5222d438dcd..1544e9548bb 100644 --- a/app/models/topic_link.rb +++ b/app/models/topic_link.rb @@ -176,7 +176,7 @@ class TopicLink < ActiveRecord::Base if upload = Upload.get_from_url(url) internal = Discourse.store.internal? # Store the same URL that will be used in the cooked version of the post - url = UrlHelper.cook_url(upload.url) + url = UrlHelper.cook_url(upload.url, secure: upload.secure?) elsif route = Discourse.route_for(parsed) internal = true diff --git a/app/models/upload.rb b/app/models/upload.rb index 5821afea84b..189a08f325c 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -140,11 +140,6 @@ class Upload < ActiveRecord::Base !(url =~ /^(https?:)?\/\//) end - def private? - return false if self.for_theme || self.for_site_setting - SiteSetting.prevent_anons_from_downloading_files && !FileHelper.is_supported_image?(self.original_filename) - end - def fix_dimensions! return if !FileHelper.is_supported_image?("image.#{extension}") @@ -235,6 +230,34 @@ class Upload < ActiveRecord::Base self.posts.where("cooked LIKE '%/_optimized/%'").find_each(&:rebake!) end + def update_secure_status + return false if self.for_theme || self.for_site_setting + mark_secure = should_be_secure? + + self.update_column("secure", mark_secure) + Discourse.store.update_upload_ACL(self) if Discourse.store.external? + end + + def should_be_secure? + mark_secure = false + if FileHelper.is_supported_media?(self.original_filename) + if SiteSetting.secure_media? + mark_secure = true if SiteSetting.login_required? + unless SiteSetting.login_required? + # first post associated with upload determines secure status + # i.e. an already public upload will stay public even if added to a new PM + first_post_with_upload = self.posts.order(sort_order: :asc).first + mark_secure = first_post_with_upload ? first_post_with_upload.with_secure_media? : false + end + else + mark_secure = false + end + else + mark_secure = SiteSetting.prevent_anons_from_downloading_files? + end + mark_secure + end + def self.migrate_to_new_scheme(limit: nil) problems = [] @@ -385,6 +408,7 @@ end # thumbnail_width :integer # thumbnail_height :integer # etag :string +# secure :boolean default(FALSE), not null # # Indexes # diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index ed278c5492d..777895b7dd5 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -140,6 +140,7 @@ en: unsubscribe_not_allowed: "Happens when unsubscribing via email is not allowed for this user." email_not_allowed: "Happens when the email address is not on the whitelist or is on the blacklist." unrecognized_error: "Unrecognized Error" + secure_media_placeholder: "Redacted: this site has secure media enabled, visit the topic to see the attached image/audio/video." errors: &errors format: ! "%{attribute} %{message}" @@ -204,6 +205,7 @@ en: enable_s3_uploads_is_required: "You cannot enable inventory to S3 unless you've enabled the S3 uploads." s3_backup_requires_s3_settings: "You cannot use S3 as backup location unless you've provided the '%{setting_name}'." s3_bucket_reused: "You cannot use the same bucket for 's3_upload_bucket' and 's3_backup_bucket'. Choose a different bucket or use a different path for each bucket." + secure_media_requirements: "S3 uploads must be enabled before enabling secure media." second_factor_cannot_be_enforced_with_disabled_local_login: "You cannot enforce 2FA if local logins are disabled." local_login_cannot_be_disabled_if_second_factor_enforced: "You cannot disable local login if 2FA is enforced. Disable enforced 2FA before disabling local logins." conflicting_google_user_id: 'The Google Account ID for this account has changed; staff intervention is required for security reasons. Please contact staff and point them to#{I18n.t("emails.secure_media_placeholder")}
" + a.remove + end + end + + @fragment.search('img').each do |img| + next unless img['src'] + if img['src'][/secure-media-uploads/] + img.add_next_sibling "#{I18n.t("emails.secure_media_placeholder")}
" + img.remove + end + end + end + def correct_first_body_margin @fragment.css('div.body p').each do |element| element['style'] = "margin-top:0; border: 0;" diff --git a/lib/file_helper.rb b/lib/file_helper.rb index 443cceb4bc0..6d39ef5477b 100644 --- a/lib/file_helper.rb +++ b/lib/file_helper.rb @@ -17,6 +17,10 @@ class FileHelper filename =~ supported_images_regexp end + def self.is_supported_media?(filename) + filename =~ supported_media_regexp + end + class FakeIO attr_accessor :status end @@ -132,8 +136,20 @@ class FileHelper @@supported_images ||= Set.new %w{jpg jpeg png gif svg ico} end + def self.supported_audio + @@supported_audio ||= Set.new %w{mp3 ogg wav m4a} + end + + def self.supported_video + @@supported_video ||= Set.new %w{mov mp4 webm ogv} + end + def self.supported_images_regexp @@supported_images_regexp ||= /\.(#{supported_images.to_a.join("|")})$/i end + def self.supported_media_regexp + media = supported_images | supported_audio | supported_video + @@supported_media_regexp ||= /\.(#{media.to_a.join("|")})$/i + end end diff --git a/lib/file_store/base_store.rb b/lib/file_store/base_store.rb index 5e455ac1b50..0c4c14d2be5 100644 --- a/lib/file_store/base_store.rb +++ b/lib/file_store/base_store.rb @@ -54,6 +54,10 @@ module FileStore not_implemented end + def s3_upload_host + not_implemented + end + def external? not_implemented end @@ -77,7 +81,11 @@ module FileStore if !file max_file_size_kb = [SiteSetting.max_image_size_kb, SiteSetting.max_attachment_size_kb].max.kilobytes - url = Discourse.store.cdn_url(upload.url) + + url = upload.secure? ? + Discourse.store.signed_url_for_path(upload.url) : + Discourse.store.cdn_url(upload.url) + url = SiteSetting.scheme + ":" + url if url =~ /^\/\// file = FileHelper.download( url, diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb index 130e60070fc..bde900f5c18 100644 --- a/lib/file_store/s3_store.rb +++ b/lib/file_store/s3_store.rb @@ -21,13 +21,13 @@ module FileStore def store_upload(file, upload, content_type = nil) path = get_path_for_upload(upload) - url, upload.etag = store_file(file, path, filename: upload.original_filename, content_type: content_type, cache_locally: true, private: upload.private?) + url, upload.etag = store_file(file, path, filename: upload.original_filename, content_type: content_type, cache_locally: true, private_acl: upload.secure?) url end - def store_optimized_image(file, optimized_image, content_type = nil) + def store_optimized_image(file, optimized_image, content_type = nil, secure: false) path = get_path_for_optimized_image(optimized_image) - url, optimized_image.etag = store_file(file, path, content_type: content_type) + url, optimized_image.etag = store_file(file, path, content_type: content_type, private_acl: secure) url end @@ -42,12 +42,12 @@ module FileStore # cache file locally when needed cache_file(file, File.basename(path)) if opts[:cache_locally] options = { - acl: opts[:private] ? "private" : "public-read", + acl: opts[:private_acl] ? "private" : "public-read", cache_control: 'max-age=31556952, public, immutable', content_type: opts[:content_type].presence || MiniMime.lookup_by_filename(filename)&.content_type } # add a "content disposition" header for "attachments" - options[:content_disposition] = "attachment; filename=\"#{filename}\"" unless FileHelper.is_supported_image?(filename) + options[:content_disposition] = "attachment; filename=\"#{filename}\"" unless FileHelper.is_supported_media?(filename) path.prepend(File.join(upload_path, "/")) if Rails.configuration.multisite @@ -88,6 +88,10 @@ module FileStore @absolute_base_url ||= SiteSetting.Upload.absolute_base_url end + def s3_upload_host + SiteSetting.Upload.s3_cdn_url.present? ? SiteSetting.Upload.s3_cdn_url : "https:#{absolute_base_url}" + end + def external? true end @@ -111,22 +115,9 @@ module FileStore end def url_for(upload, force_download: false) - if upload.private? || force_download - opts = { expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS } - - if force_download - opts[:response_content_disposition] = ActionDispatch::Http::ContentDisposition.format( - disposition: "attachment", filename: upload.original_filename - ) - end - - obj = @s3_helper.object(get_upload_key(upload)) - url = obj.presigned_url(:get, opts) - else - url = upload.url - end - - url + upload.secure? || force_download ? + presigned_url(get_upload_key(upload), force_download: force_download, filename: upload.original_filename) : + upload.url end def cdn_url(url) @@ -136,6 +127,11 @@ 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) + key = path.sub(absolute_base_url + "/", "") + presigned_url(key) + end + def cache_avatar(avatar, user_id) source = avatar.url.sub(absolute_base_url + "/", "") destination = avatar_template(avatar, user_id).sub(absolute_base_url + "/", "") @@ -163,14 +159,15 @@ module FileStore end def update_upload_ACL(upload) - private_uploads = SiteSetting.prevent_anons_from_downloading_files key = get_upload_key(upload) + update_ACL(key, upload.secure?) - begin - @s3_helper.object(key).acl.put(acl: private_uploads ? "private" : "public-read") - rescue Aws::S3::Errors::NoSuchKey - Rails.logger.warn("Could not update ACL on upload with key: '#{key}'. Upload is missing.") + upload.optimized_images.find_each do |optimized_image| + optimized_image_key = get_path_for_optimized_image(optimized_image) + update_ACL(optimized_image_key, upload.secure?) end + + true end def download_file(upload, destination_path) @@ -179,6 +176,18 @@ module FileStore private + def presigned_url(url, force_download: false, filename: false) + opts = { expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS } + if force_download && filename + opts[:response_content_disposition] = ActionDispatch::Http::ContentDisposition.format( + disposition: "attachment", filename: filename + ) + end + + obj = @s3_helper.object(url) + obj.presigned_url(:get, opts) + end + def get_upload_key(upload) if Rails.configuration.multisite File.join(upload_path, "/", get_path_for_upload(upload)) @@ -187,6 +196,14 @@ module FileStore end end + def update_ACL(key, secure) + begin + @s3_helper.object(key).acl.put(acl: secure ? "private" : "public-read") + rescue Aws::S3::Errors::NoSuchKey + Rails.logger.warn("Could not update ACL on upload with key: '#{key}'. Upload is missing.") + end + end + def list_missing(model, prefix) connection = ActiveRecord::Base.connection.raw_connection connection.exec('CREATE TEMP TABLE verified_ids(val integer PRIMARY KEY)') diff --git a/lib/oneboxer.rb b/lib/oneboxer.rb index fba6704045b..e726f716852 100644 --- a/lib/oneboxer.rb +++ b/lib/oneboxer.rb @@ -178,7 +178,15 @@ module Oneboxer def self.local_upload_html(url) case File.extname(URI(url).path || "") when VIDEO_REGEX - "" + <<~HTML +#{I18n.t("emails.secure_media_placeholder")}
" + end + end + end + def self.format_for_email(html, post = nil) doc = Nokogiri::HTML.fragment(html) DiscourseEvent.trigger(:reduce_cooked, doc, post) + strip_secure_media(doc) if post&.with_secure_media? strip_image_wrapping(doc) convert_vimeo_iframes(doc) make_all_links_absolute(doc) diff --git a/lib/pretty_text/helpers.rb b/lib/pretty_text/helpers.rb index 417600469bf..eac9384afb1 100644 --- a/lib/pretty_text/helpers.rb +++ b/lib/pretty_text/helpers.rb @@ -64,13 +64,15 @@ module PrettyText reverse_map[value] << key end - Upload.where(sha1: map.values).pluck(:sha1, :url, :extension).each do |row| - sha1, url, extension = row + Upload.where(sha1: map.values).pluck(:sha1, :url, :extension, :original_filename, :secure).each do |row| + sha1, url, extension, original_filename, secure = row if short_urls = reverse_map[sha1] + secure_media = FileHelper.is_supported_media?(original_filename) && SiteSetting.secure_media? && secure + short_urls.each do |short_url| result[short_url] = { - url: Discourse.store.cdn_url(url), + url: secure_media ? secure_media_url(url) : Discourse.store.cdn_url(url), short_path: Upload.short_path(sha1: sha1, extension: extension), base62_sha1: Upload.base62_sha1(sha1) } @@ -82,6 +84,10 @@ module PrettyText result end + def secure_media_url(url) + url.sub(SiteSetting.Upload.absolute_base_url, "/secure-media-uploads") + end + def get_topic_info(topic_id) return unless topic_id.is_a?(Integer) # TODO this only handles public topics, secured one do not get this diff --git a/lib/rake_helpers.rb b/lib/rake_helpers.rb new file mode 100644 index 00000000000..033fdeeb094 --- /dev/null +++ b/lib/rake_helpers.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class RakeHelpers + def self.print_status_with_label(label, current, max) + print "\r\033[K%s%9d / %d (%5.1f%%)" % [label, current, max, ((current.to_f / max.to_f) * 100).round(1)] + end + + def self.print_status(current, max) + print "\r\033[K%9d / %d (%5.1f%%)" % [current, max, ((current.to_f / max.to_f) * 100).round(1)] + end +end diff --git a/lib/site_settings/validations.rb b/lib/site_settings/validations.rb index 3b2708f2b8b..65534e0c90f 100644 --- a/lib/site_settings/validations.rb +++ b/lib/site_settings/validations.rb @@ -121,6 +121,10 @@ module SiteSettings::Validations validate_error :s3_upload_bucket_is_required if new_val == "t" && SiteSetting.s3_upload_bucket.blank? end + def validate_secure_media(new_val) + validate_error :secure_media_requirements if new_val == "t" && !SiteSetting.enable_s3_uploads? + end + def validate_enable_s3_inventory(new_val) validate_error :enable_s3_uploads_is_required if new_val == "t" && !SiteSetting.Upload.enable_s3_uploads end diff --git a/lib/tasks/topics.rake b/lib/tasks/topics.rake index e91c4a6f319..a47a0b0c07a 100644 --- a/lib/tasks/topics.rake +++ b/lib/tasks/topics.rake @@ -1,8 +1,6 @@ # frozen_string_literal: true -def print_status_with_label(label, current, max) - print "\r%s%9d / %d (%5.1f%%)" % [label, current, max, ((current.to_f / max.to_f) * 100).round(1)] -end +require_dependency "rake_helpers" def close_old_topics(category) topics = Topic.where(closed: false, category_id: category.id) @@ -23,7 +21,7 @@ def close_old_topics(category) topics.find_each do |topic| topic.update_status("closed", true, Discourse.system_user) - print_status_with_label(" closing old topics: ", topics_closed += 1, total) + RakeHelpers.print_status_with_label(" closing old topics: ", topics_closed += 1, total) end end @@ -49,7 +47,7 @@ def apply_auto_close(category) topics.find_each do |topic| topic.inherit_auto_close_from_category - print_status_with_label(" applying auto-close to topics: ", topics_closed += 1, total) + RakeHelpers.print_status_with_label(" applying auto-close to topics: ", topics_closed += 1, total) end end @@ -77,7 +75,7 @@ task "topics:watch_all_replied_topics" => :environment do t.topic_users.where(posted: true).find_each do |tp| tp.update!(notification_level: TopicUser.notification_levels[:watching], notifications_reason_id: TopicUser.notification_reasons[:created_post]) end - print_status(count += 1, total) + RakeHelpers.print_status(count += 1, total) end puts "", "Done" @@ -96,12 +94,8 @@ task "topics:update_fancy_titles" => :environment do Topic.find_each do |topic| topic.fancy_title - print_status(count += 1, total) + RakeHelpers.print_status(count += 1, total) end puts "", "Done" end - -def print_status(current, max) - print "\r%9d / %d (%5.1f%%)" % [current, max, ((current.to_f / max.to_f) * 100).round(1)] -end diff --git a/lib/tasks/uploads.rake b/lib/tasks/uploads.rake index f21ddd6e625..0761c4712aa 100644 --- a/lib/tasks/uploads.rake +++ b/lib/tasks/uploads.rake @@ -8,6 +8,8 @@ require "base62" # gather # ################################################################################ +require_dependency "rake_helpers" + task "uploads:gather" => :environment do ENV["RAILS_DB"] ? gather_uploads : gather_uploads_for_all_sites end @@ -426,7 +428,7 @@ def migrate_to_s3 %Q{attachment; filename="#{upload.original_filename}"} end - if upload&.private? + if upload.secure options[:acl] = "private" end end @@ -907,6 +909,108 @@ task "uploads:recover" => :environment do end end +## +# Run this task whenever the secure_media or login_required +# settings are changed for a Discourse instance to update +# the upload secure flag and S3 upload ACLs. +task "uploads:ensure_correct_acl" => :environment do + RailsMultisite::ConnectionManagement.each_connection do |db| + unless Discourse.store.external? + puts "This task only works for external storage." + exit 1 + end + + puts "Ensuring correct ACL for uploads in #{db}...", "" + + Upload.transaction do + mark_secure_in_loop_because_no_login_required = false + + # First of all only get relevant uploads (supported media). + # + # Also only get uploads that are not for a theme or a site setting, so only + # get post related uploads. + uploads_with_supported_media = Upload.includes(:posts, :optimized_images).where( + "LOWER(original_filename) SIMILAR TO '%\.(jpg|jpeg|png|gif|svg|ico|mp3|ogg|wav|m4a|mov|mp4|webm|ogv)'" + ).joins(:post_uploads) + + puts "There are #{uploads_with_supported_media.count} upload(s) with supported media that could be marked secure.", "" + + # Simply mark all these uploads as secure if login_required because no anons will be able to access them + if SiteSetting.login_required? + mark_all_as_secure_login_required(uploads_with_supported_media) + else + + # If NOT login_required, then we have to go for the other slower flow, where in the loop + # we mark the upload as secure if the first post it is used in is with_secure_media? + mark_secure_in_loop_because_no_login_required = true + puts "Marking posts as secure in the next step because login_required is false." + end + + puts "", "Rebaking #{uploads_with_supported_media.count} upload posts and updating ACLs in S3.", "" + + upload_ids_to_mark_as_secure, uploads_skipped_because_of_error = update_acls_and_rebake_upload_posts( + uploads_with_supported_media, mark_secure_in_loop_because_no_login_required + ) + + log_rebake_errors(uploads_skipped_because_of_error) + mark_specific_uploads_as_secure_no_login_required(upload_ids_to_mark_as_secure) + end + end + puts "", "Done" +end + +def mark_all_as_secure_login_required(uploads_with_supported_media) + puts "Marking #{uploads_with_supported_media.count} upload(s) as secure because login_required is true.", "" + uploads_with_supported_media.update_all(secure: true) + puts "Finished marking upload(s) as secure." +end + +def log_rebake_errors(uploads_skipped_because_of_error) + return if uploads_skipped_because_of_error.empty? + puts "Skipped the following uploads due to error:", "" + uploads_skipped_because_of_error.each do |message| + puts message + end +end + +def mark_specific_uploads_as_secure_no_login_required(upload_ids_to_mark_as_secure) + return if upload_ids_to_mark_as_secure.empty? + puts "Marking #{upload_ids_to_mark_as_secure.length} uploads as secure because their first post contains secure media." + Upload.where(id: upload_ids_to_mark_as_secure).update_all(secure: true) + puts "Finished marking uploads as secure." +end + +def update_acls_and_rebake_upload_posts(uploads_with_supported_media, mark_secure_in_loop_because_no_login_required) + upload_ids_to_mark_as_secure = [] + uploads_skipped_because_of_error = [] + + i = 0 + uploads_with_supported_media.find_each(batch_size: 50) do |upload_with_supported_media| + RakeHelpers.print_status_with_label("Updating ACL for upload.......", i, uploads_with_supported_media.count) + + Discourse.store.update_upload_ACL(upload_with_supported_media) + + RakeHelpers.print_status_with_label("Rebaking posts for upload.....", i, uploads_with_supported_media.count) + begin + upload_with_supported_media.posts.each { |post| post.rebake! } + + if mark_secure_in_loop_because_no_login_required + first_post_with_upload = upload_with_supported_media.posts.order(sort_order: :asc).first + mark_secure = first_post_with_upload ? first_post_with_upload.with_secure_media? : false + upload_ids_to_mark_as_secure << upload_with_supported_media.id if mark_secure + end + rescue => e + uploads_skipped_because_of_error << "#{upload_with_supported_media.original_filename} (#{upload_with_supported_media.url}) #{e.message}" + end + + i += 1 + end + RakeHelpers.print_status_with_label("Rebaking complete! ", i, uploads_with_supported_media.count) + puts "" + + [upload_ids_to_mark_as_secure, uploads_skipped_because_of_error] +end + def inline_uploads(post) replaced = false diff --git a/lib/upload_creator.rb b/lib/upload_creator.rb index 0a0c8d43c49..ab1fa9a4a89 100644 --- a/lib/upload_creator.rb +++ b/lib/upload_creator.rb @@ -118,6 +118,13 @@ class UploadCreator @upload.for_site_setting = true if @opts[:for_site_setting] @upload.for_gravatar = true if @opts[:for_gravatar] + if !FileHelper.is_supported_media?(@filename) && + !@upload.for_theme && + !@upload.for_site_setting && + SiteSetting.prevent_anons_from_downloading_files + @upload.secure = true + end + return @upload unless @upload.save # store the file and update its url diff --git a/lib/url_helper.rb b/lib/url_helper.rb index b156013295e..2cb6e7ce3a5 100644 --- a/lib/url_helper.rb +++ b/lib/url_helper.rb @@ -38,6 +38,11 @@ class UrlHelper url.sub(/^http:/i, "") end + def self.secure_proxy_without_cdn(url) + url = url.sub(SiteSetting.Upload.absolute_base_url, "/secure-media-uploads") + self.absolute(url, nil) + end + DOUBLE_ESCAPED_REGEXP ||= /%25([0-9a-f]{2})/i # Prevents double URL encode @@ -48,16 +53,16 @@ class UrlHelper encoded end - def self.cook_url(url) + def self.cook_url(url, secure: false) return url unless is_local(url) uri = URI.parse(url) filename = File.basename(uri.path) - is_attachment = !FileHelper.is_supported_image?(filename) + is_attachment = !FileHelper.is_supported_media?(filename) no_cdn = SiteSetting.login_required || SiteSetting.prevent_anons_from_downloading_files - url = absolute_without_cdn(url) + url = secure ? secure_proxy_without_cdn(url) : absolute_without_cdn(url) unless is_attachment && no_cdn url = Discourse.store.cdn_url(url) diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb index 00e008aba95..eb2111d67ec 100644 --- a/spec/components/cooked_post_processor_spec.rb +++ b/spec/components/cooked_post_processor_spec.rb @@ -812,8 +812,8 @@ describe CookedPostProcessor do it "is always allowed to crawl our own images" do store = stub + Discourse.expects(:store).returns(store).at_least_once store.expects(:has_been_uploaded?).returns(true) - Discourse.expects(:store).returns(store) FastImage.expects(:size).returns([100, 200]) expect(cpp.get_size("http://foo.bar/image2.png")).to eq([100, 200]) end @@ -1104,40 +1104,175 @@ describe CookedPostProcessor do HTML end - it "uses the right CDN when uploads are on S3" do - Rails.configuration.action_controller.stubs(:asset_host).returns("https://local.cdn.com") + context "s3_uploads" do + before do + Rails.configuration.action_controller.stubs(:asset_host).returns("https://local.cdn.com") - SiteSetting.s3_upload_bucket = "some-bucket-on-s3" - SiteSetting.s3_access_key_id = "s3-access-key-id" - SiteSetting.s3_secret_access_key = "s3-secret-access-key" - SiteSetting.s3_cdn_url = "https://s3.cdn.com" - SiteSetting.enable_s3_uploads = true + SiteSetting.s3_upload_bucket = "some-bucket-on-s3" + SiteSetting.s3_access_key_id = "s3-access-key-id" + SiteSetting.s3_secret_access_key = "s3-secret-access-key" + SiteSetting.s3_cdn_url = "https://s3.cdn.com" + SiteSetting.enable_s3_uploads = true + SiteSetting.authorized_extensions = "png|jpg|gif|mov|ogg|" + uploaded_file = file_from_fixtures("smallest.png") + upload_sha1 = Digest::SHA1.hexdigest(File.read(uploaded_file)) - uploaded_file = file_from_fixtures("smallest.png") - upload_sha1 = Digest::SHA1.hexdigest(File.read(uploaded_file)) + upload.update!( + original_filename: "smallest.png", + width: 10, + height: 20, + sha1: upload_sha1, + extension: "png", + ) + end - upload.update!( - original_filename: "smallest.png", - width: 10, - height: 20, - sha1: upload_sha1, - extension: "png", - ) + it "uses the right CDN when uploads are on S3" do + stored_path = Discourse.store.get_path_for_upload(upload) + upload.update_column(:url, "#{SiteSetting.Upload.absolute_base_url}/#{stored_path}") - stored_path = Discourse.store.get_path_for_upload(upload) - upload.update_column(:url, "#{SiteSetting.Upload.absolute_base_url}/#{stored_path}") + the_post = Fabricate(:post, raw: %Q{This post has a local emoji :+1: and an external upload\n\n}) - the_post = Fabricate(:post, raw: %Q{This post has a local emoji :+1: and an external upload\n\n}) + cpp = CookedPostProcessor.new(the_post) + cpp.optimize_urls - cpp = CookedPostProcessor.new(the_post) - cpp.optimize_urls + expect(cpp.html).to match_html <<~HTML +This post has a local emoji and an external upload
This post has a local emoji and an external upload
This post has a local emoji and an external upload
This post has an S3 video onebox:
This post has an S3 video onebox:
+
This post has a video upload.
This post has an audio upload.
+
+
And an image upload.
+
![](\"#{Discourse.base_url}\/secure-media-uploads/original/1X/testimage.png\")
")
+ expect(frag.at('p.secure-media-notice')).to be_present
+ expect(frag.at('img')).not_to be_present
+ expect(frag.at('a')).not_to be_present
+ end
+
+ it "replaces secure images with a placeholder" do
+ frag = html_fragment("![](\"/secure-media-uploads/original/1X/testimage.png\")
")
+ expect(frag.at('p.secure-media-notice')).to be_present
+ expect(frag.at('img')).not_to be_present
+ end
+ end
+
end
diff --git a/spec/components/file_store/base_store_spec.rb b/spec/components/file_store/base_store_spec.rb
index f633b1e455e..13d0c36f8a5 100644
--- a/spec/components/file_store/base_store_spec.rb
+++ b/spec/components/file_store/base_store_spec.rb
@@ -86,5 +86,18 @@ RSpec.describe FileStore::BaseStore do
expect(file.class).to eq(File)
end
+
+ it "should return the file when secure media are enabled" do
+ SiteSetting.login_required = true
+ SiteSetting.secure_media = true
+
+ stub_request(:head, "https://s3-upload-bucket.s3.amazonaws.com/")
+ signed_url = Discourse.store.signed_url_for_path(upload_s3.url)
+ stub_request(:get, signed_url).to_return(status: 200, body: "Hello world")
+
+ file = store.download(upload_s3)
+
+ expect(file.class).to eq(File)
+ end
end
end
diff --git a/spec/components/file_store/s3_store_spec.rb b/spec/components/file_store/s3_store_spec.rb
index fce086b5edc..99f57c71047 100644
--- a/spec/components/file_store/s3_store_spec.rb
+++ b/spec/components/file_store/s3_store_spec.rb
@@ -43,16 +43,16 @@ describe FileStore::S3Store do
let(:s3_object) { stub }
let(:etag) { "etag" }
- before do
- s3_object.stubs(:put).returns(Aws::S3::Types::PutObjectOutput.new(etag: "\"#{etag}\""))
- end
-
describe "#store_upload" do
it "returns an absolute schemaless url" do
store.expects(:get_depth_for).with(upload.id).returns(0)
s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
-
s3_bucket.expects(:object).with("original/1X/#{upload.sha1}.png").returns(s3_object)
+ s3_object.expects(:put).with(
+ acl: "public-read",
+ cache_control: "max-age=31556952, public, immutable",
+ content_type: "image/png",
+ body: uploaded_file).returns(Aws::S3::Types::PutObjectOutput.new(etag: "\"#{etag}\""))
expect(store.store_upload(uploaded_file, upload)).to eq(
"//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/original/1X/#{upload.sha1}.png"
@@ -62,6 +62,7 @@ describe FileStore::S3Store do
describe "when s3_upload_bucket includes folders path" do
before do
+ s3_object.stubs(:put).returns(Aws::S3::Types::PutObjectOutput.new(etag: "\"#{etag}\""))
SiteSetting.s3_upload_bucket = "s3-upload-bucket/discourse-uploads"
end
@@ -78,28 +79,36 @@ describe FileStore::S3Store do
end
end
- describe "when private uploads are enabled" do
- it "returns signed URL for eligible private upload" do
+ describe "when secure uploads are enabled" do
+ it "saves secure attachment using private ACL" do
SiteSetting.prevent_anons_from_downloading_files = true
SiteSetting.authorized_extensions = "pdf|png|jpg|gif"
- upload.update!(original_filename: "small.pdf", extension: "pdf")
+ upload.update!(original_filename: "small.pdf", extension: "pdf", secure: true)
- s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
- s3_bucket.expects(:object).with("original/1X/#{upload.sha1}.pdf").returns(s3_object).at_least_once
- s3_object.expects(:presigned_url).with(:get, expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS)
+ s3_helper.expects(:s3_bucket).returns(s3_bucket)
+ s3_bucket.expects(:object).with("original/1X/#{upload.sha1}.pdf").returns(s3_object)
+ s3_object.expects(:put).with(
+ acl: "private",
+ cache_control: "max-age=31556952, public, immutable",
+ content_type: "application/pdf",
+ content_disposition: "attachment; filename=\"#{upload.original_filename}\"",
+ body: uploaded_file).returns(Aws::S3::Types::PutObjectOutput.new(etag: "\"#{etag}\""))
expect(store.store_upload(uploaded_file, upload)).to eq(
"//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/original/1X/#{upload.sha1}.pdf"
)
-
- expect(store.url_for(upload)).not_to eq(upload.url)
end
- it "returns regular URL for ineligible private upload" do
+ it "saves image upload using public ACL" do
SiteSetting.prevent_anons_from_downloading_files = true
s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
s3_bucket.expects(:object).with("original/1X/#{upload.sha1}.png").returns(s3_object).at_least_once
+ s3_object.expects(:put).with(
+ acl: "public-read",
+ cache_control: "max-age=31556952, public, immutable",
+ content_type: "image/png",
+ body: uploaded_file).returns(Aws::S3::Types::PutObjectOutput.new(etag: "\"#{etag}\""))
expect(store.store_upload(uploaded_file, upload)).to eq(
"//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/original/1X/#{upload.sha1}.png"
@@ -111,6 +120,10 @@ describe FileStore::S3Store do
end
describe "#store_optimized_image" do
+ before do
+ s3_object.stubs(:put).returns(Aws::S3::Types::PutObjectOutput.new(etag: "\"#{etag}\""))
+ end
+
it "returns an absolute schemaless url" do
store.expects(:get_depth_for).with(optimized_image.upload.id).returns(0)
s3_helper.expects(:s3_bucket).returns(s3_bucket)
@@ -355,23 +368,27 @@ describe FileStore::S3Store do
include_context "s3 helpers"
let(:s3_object) { stub }
+ before do
+ SiteSetting.authorized_extensions = "pdf|png"
+ end
+
describe ".update_upload_ACL" do
- it "sets acl to private when private uploads are enabled" do
- SiteSetting.prevent_anons_from_downloading_files = true
+ it "sets acl to public by default" do
+ upload.update!(original_filename: "small.pdf", extension: "pdf")
s3_helper.expects(:s3_bucket).returns(s3_bucket)
- s3_bucket.expects(:object).with("original/1X/#{upload.sha1}.png").returns(s3_object)
+ s3_bucket.expects(:object).with("original/1X/#{upload.sha1}.pdf").returns(s3_object)
s3_object.expects(:acl).returns(s3_object)
- s3_object.expects(:put).with(acl: "private").returns(s3_object)
+ s3_object.expects(:put).with(acl: "public-read").returns(s3_object)
expect(store.update_upload_ACL(upload)).to be_truthy
end
- it "sets acl to public when private uploads are disabled" do
- SiteSetting.prevent_anons_from_downloading_files = false
+ it "sets acl to private when upload is marked secure" do
+ upload.update!(original_filename: "small.pdf", extension: "pdf", secure: true)
s3_helper.expects(:s3_bucket).returns(s3_bucket)
- s3_bucket.expects(:object).with("original/1X/#{upload.sha1}.png").returns(s3_object)
+ s3_bucket.expects(:object).with("original/1X/#{upload.sha1}.pdf").returns(s3_object)
s3_object.expects(:acl).returns(s3_object)
- s3_object.expects(:put).with(acl: "public-read").returns(s3_object)
+ s3_object.expects(:put).with(acl: "private").returns(s3_object)
expect(store.update_upload_ACL(upload)).to be_truthy
end
@@ -421,4 +438,21 @@ describe FileStore::S3Store do
end
end
+ describe ".signed_url_for_path" do
+ include_context "s3 helpers"
+ let(:s3_object) { stub }
+
+ it "returns signed URL for a given path" 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
+ }
+
+ s3_object.expects(:presigned_url).with(:get, opts)
+
+ expect(store.signed_url_for_path("special/optimized/file.png")).not_to eq(upload.url)
+ end
+ end
+
end
diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb
index 9dcb712723f..518a12b2dd1 100644
--- a/spec/components/post_creator_spec.rb
+++ b/spec/components/post_creator_spec.rb
@@ -1388,4 +1388,63 @@ describe PostCreator do
end
end
end
+
+ context "secure media uploads" do
+ fab!(:image_upload) { Fabricate(:upload, secure: true) }
+ fab!(:user2) { Fabricate(:user) }
+ fab!(:public_topic) { Fabricate(:topic) }
+
+ before do
+ SiteSetting.enable_s3_uploads = true
+ SiteSetting.authorized_extensions = "png|jpg|gif|mp4"
+ SiteSetting.s3_upload_bucket = "s3-upload-bucket"
+ SiteSetting.s3_access_key_id = "some key"
+ SiteSetting.s3_secret_access_key = "some secret key"
+ SiteSetting.secure_media = true
+
+ stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/")
+
+ stub_request(
+ :put,
+ "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/original/1X/#{image_upload.sha1}.#{image_upload.extension}?acl"
+ )
+ end
+
+ it "does not allow a secure image to be used in a public topic" do
+ public_post = PostCreator.create(
+ user,
+ topic_id: public_topic.id,
+ raw: "A public post with an image.\n"
+ )
+
+ expect(public_post.errors.count).to be(1)
+ expect(public_post.errors.full_messages).to include(I18n.t('secure_upload_not_allowed_in_public_topic', upload_filenames: image_upload.original_filename))
+
+ # secure upload CAN be used in another PM
+ pm = PostCreator.create(
+ user,
+ title: 'this is another private message',
+ raw: "with an upload: \n",
+ archetype: Archetype.private_message,
+ target_usernames: [user2.username].join(',')
+ )
+
+ expect(pm.errors).to be_blank
+ end
+
+ it "does not allow a secure video to be used in a public topic" do
+ video_upload = Fabricate(:upload_s3, extension: 'mp4', original_filename: "video.mp4", secure: true)
+
+ public_post = PostCreator.create(
+ user,
+ topic_id: public_topic.id,
+ raw: "A public post with a video onebox:\n#{video_upload.url}"
+ )
+
+ expect(public_post.errors.count).to be(1)
+ expect(public_post.errors.full_messages).to include(I18n.t('secure_upload_not_allowed_in_public_topic', upload_filenames: video_upload.original_filename))
+ end
+
+ end
+
end
diff --git a/spec/components/pretty_text_spec.rb b/spec/components/pretty_text_spec.rb
index 8411314775e..c75bb39a727 100644
--- a/spec/components/pretty_text_spec.rb
+++ b/spec/components/pretty_text_spec.rb
@@ -810,6 +810,50 @@ describe PrettyText do
html = "Check out this video – .
" expect(PrettyText.format_for_email(html, post)).to match(Regexp.escape("https://vimeo.com/329875646/%3E%20%3Cscript%3Ealert(1)%3C/script%3E")) end + + describe "#strip_secure_media" do + before do + SiteSetting.s3_upload_bucket = "some-bucket-on-s3" + SiteSetting.s3_access_key_id = "s3-access-key-id" + SiteSetting.s3_secret_access_key = "s3-secret-access-key" + SiteSetting.s3_cdn_url = "https://s3.cdn.com" + SiteSetting.enable_s3_uploads = true + SiteSetting.secure_media = true + SiteSetting.login_required = true + end + + it "replaces secure video content" do + html = <<~HTML + + HTML + + md = PrettyText.format_for_email(html, post) + + expect(md).not_to include('