From 8ebd5edd1e02e6fafe7732515edefec5a5dfc3f7 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 29 Sep 2022 09:24:33 +1000 Subject: [PATCH] DEV: Rename secure_media to secure_uploads (#18376) This commit renames all secure_media related settings to secure_uploads_* along with the associated functionality. This is being done because "media" does not really cover it, we aren't just doing this for images and videos etc. but for all uploads in the site. Additionally, in future we want to secure more types of uploads, and enable a kind of "mixed mode" where some uploads are secure and some are not, so keeping media in the name is just confusing. This also keeps compatibility with the `secure-media-uploads` path, and changes new secure URLs to be `secure-uploads`. Deprecated settings: * secure_media -> secure_uploads * secure_media_allow_embed_images_in_emails -> secure_uploads_allow_embed_images_in_emails * secure_media_max_email_embed_image_size_kb -> secure_uploads_max_email_embed_image_size_kb --- .../discourse/app/lib/lazy-load-images.js | 4 +- .../javascripts/discourse/app/lib/url.js | 1 + .../discourse/tests/helpers/site-settings.js | 2 +- .../tests/unit/lib/pretty-text-test.js | 42 ++++++------ .../tests/unit/lib/upload-short-url-test.js | 14 ++-- .../discourse/tests/unit/lib/url-test.js | 8 +-- .../addon/engines/discourse-markdown-it.js | 2 +- .../pretty-text/addon/upload-short-url.js | 9 +-- .../discourse-markdown/upload-protocol.js | 7 +- app/assets/javascripts/service-worker.js.erb | 2 +- app/controllers/published_pages_controller.rb | 2 +- app/controllers/uploads_controller.rb | 21 ++++-- app/helpers/email_helper.rb | 4 ++ app/jobs/regular/pull_hotlinked_images.rb | 6 +- app/jobs/regular/sync_acls_for_uploads.rb | 2 +- app/models/post.rb | 10 +-- app/models/upload.rb | 14 ++-- app/serializers/topic_view_serializer.rb | 2 +- app/serializers/upload_serializer.rb | 2 +- config/initializers/006-mini_profiler.rb | 1 + config/locales/server.en.yml | 13 ++-- config/nginx.sample.conf | 2 +- config/routes.rb | 7 +- config/site_settings.yml | 12 ++++ ...ttings_based_on_secure_media_equivalent.rb | 36 +++++++++++ lib/cooked_post_processor.rb | 14 ++-- lib/cooked_processor_mixin.rb | 6 +- lib/email/sender.rb | 6 +- lib/email/styles.rb | 18 +++--- lib/guardian.rb | 2 +- lib/post_creator.rb | 2 +- lib/pretty_text.rb | 22 +++---- lib/pretty_text/helpers.rb | 4 +- lib/site_settings/deprecated_settings.rb | 3 + lib/site_settings/validations.rb | 6 +- lib/tasks/uploads.rake | 20 +++--- lib/topic_upload_security_manager.rb | 8 +-- lib/upload_creator.rb | 12 ++-- lib/upload_security.rb | 18 +++--- lib/url_helper.rb | 8 +-- spec/jobs/pull_hotlinked_images_spec.rb | 26 ++++---- .../update_post_uploads_secure_status_spec.rb | 2 +- spec/lib/cooked_post_processor_spec.rb | 32 +++++----- spec/lib/email/sender_spec.rb | 12 ++-- spec/lib/email/styles_spec.rb | 52 +++++++-------- spec/lib/file_store/base_store_spec.rb | 4 +- spec/lib/guardian_spec.rb | 4 +- spec/lib/post_creator_spec.rb | 4 +- spec/lib/post_revisor_spec.rb | 4 +- spec/lib/pretty_text_spec.rb | 36 +++++------ spec/lib/site_settings/validations_spec.rb | 14 ++-- .../lib/topic_upload_security_manager_spec.rb | 22 +++---- spec/lib/upload_creator_spec.rb | 14 ++-- spec/lib/upload_recovery_spec.rb | 2 +- spec/lib/upload_security_spec.rb | 14 ++-- spec/lib/url_helper_spec.rb | 8 +-- spec/models/optimized_image_spec.rb | 2 +- spec/models/post_spec.rb | 34 +++++----- spec/models/upload_spec.rb | 64 +++++++++---------- spec/multisite/s3_store_spec.rb | 6 +- .../published_pages_controller_spec.rb | 4 +- .../uploads_controller_multisite_spec.rb | 2 +- spec/requests/uploads_controller_spec.rb | 34 +++++----- .../serializers/topic_view_serializer_spec.rb | 4 +- spec/serializers/upload_serializer_spec.rb | 6 +- spec/support/uploads_helpers.rb | 9 ++- spec/tasks/uploads_spec.rb | 14 ++-- 67 files changed, 442 insertions(+), 361 deletions(-) create mode 100644 db/migrate/20220927065328_set_secure_uploads_settings_based_on_secure_media_equivalent.rb diff --git a/app/assets/javascripts/discourse/app/lib/lazy-load-images.js b/app/assets/javascripts/discourse/app/lib/lazy-load-images.js index d905ec313f3..8f68218a3bd 100644 --- a/app/assets/javascripts/discourse/app/lib/lazy-load-images.js +++ b/app/assets/javascripts/discourse/app/lib/lazy-load-images.js @@ -21,8 +21,8 @@ export function nativeLazyLoading(api) { // Support for smallUpload should be maintained until Post::BAKED_VERSION is bumped higher than 2 const { smallUpload, dominantColor } = img.dataset; - if (siteSettings.secure_media && smallUpload) { - // Secure media requests go through the app. In topics with many images, + if (siteSettings.secure_uploads && smallUpload) { + // Secure uploads requests go through the app. In topics with many images, // this makes it very easy to hit rate limiters. Skipping the low-res // placeholders reduces the chance of this problem occuring. return; diff --git a/app/assets/javascripts/discourse/app/lib/url.js b/app/assets/javascripts/discourse/app/lib/url.js index b41e3e5c432..622911c4173 100644 --- a/app/assets/javascripts/discourse/app/lib/url.js +++ b/app/assets/javascripts/discourse/app/lib/url.js @@ -19,6 +19,7 @@ const SERVER_SIDE_ONLY = [ /^\/assets\//, /^\/uploads\//, /^\/secure-media-uploads\//, + /^\/secure-uploads\//, /^\/stylesheets\//, /^\/site_customizations\//, /^\/raw\//, diff --git a/app/assets/javascripts/discourse/tests/helpers/site-settings.js b/app/assets/javascripts/discourse/tests/helpers/site-settings.js index 4f02fb7839a..7909b030a26 100644 --- a/app/assets/javascripts/discourse/tests/helpers/site-settings.js +++ b/app/assets/javascripts/discourse/tests/helpers/site-settings.js @@ -97,7 +97,7 @@ const ORIGINAL_SETTINGS = { enable_personal_messages: true, personal_message_enabled_groups: "11", // TL1 group unicode_usernames: false, - secure_media: false, + secure_uploads: false, external_emoji_url: "", remove_muted_tags_from_latest: "always", enable_group_directory: true, diff --git a/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js b/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js index 3164a6a2abf..853573405e9 100644 --- a/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js +++ b/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js @@ -997,12 +997,12 @@ eviltrout

); }); - test("attachment - mapped url - secure media disabled", function (assert) { + test("attachment - mapped url - secure uploads disabled", function (assert) { function lookupUploadUrls() { let cache = {}; cache["upload://o8iobpLcW3WSFvVH7YQmyGlKmGM.pdf"] = { short_url: "upload://o8iobpLcW3WSFvVH7YQmyGlKmGM.pdf", - url: "/secure-media-uploads/original/3X/c/b/o8iobpLcW3WSFvVH7YQmyGlKmGM.pdf", + url: "/secure-uploads/original/3X/c/b/o8iobpLcW3WSFvVH7YQmyGlKmGM.pdf", short_path: "/uploads/short-url/blah", }; return cache; @@ -1010,20 +1010,20 @@ eviltrout

assert.cookedOptions( "[test.pdf|attachment](upload://o8iobpLcW3WSFvVH7YQmyGlKmGM.pdf)", { - siteSettings: { secure_media: false }, + siteSettings: { secure_uploads: false }, lookupUploadUrls, }, `

test.pdf

`, - "It returns the correct attachment link HTML when the URL is mapped without secure media" + "It returns the correct attachment link HTML when the URL is mapped without secure uploads" ); }); - test("attachment - mapped url - secure media enabled", function (assert) { + test("attachment - mapped url - secure uploads enabled", function (assert) { function lookupUploadUrls() { let cache = {}; cache["upload://o8iobpLcW3WSFvVH7YQmyGlKmGM.pdf"] = { short_url: "upload://o8iobpLcW3WSFvVH7YQmyGlKmGM.pdf", - url: "/secure-media-uploads/original/3X/c/b/o8iobpLcW3WSFvVH7YQmyGlKmGM.pdf", + url: "/secure-uploads/original/3X/c/b/o8iobpLcW3WSFvVH7YQmyGlKmGM.pdf", short_path: "/uploads/short-url/blah", }; return cache; @@ -1031,11 +1031,11 @@ eviltrout

assert.cookedOptions( "[test.pdf|attachment](upload://o8iobpLcW3WSFvVH7YQmyGlKmGM.pdf)", { - siteSettings: { secure_media: true }, + siteSettings: { secure_uploads: true }, lookupUploadUrls, }, - `

test.pdf

`, - "It returns the correct attachment link HTML when the URL is mapped with secure media" + `

test.pdf

`, + "It returns the correct attachment link HTML when the URL is mapped with secure uploads" ); }); @@ -1052,12 +1052,12 @@ eviltrout

); }); - test("video - mapped url - secure media enabled", function (assert) { + test("video - mapped url - secure uploads enabled", function (assert) { function lookupUploadUrls() { let cache = {}; cache["upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp4"] = { short_url: "upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp4", - url: "/secure-media-uploads/original/3X/c/b/test.mp4", + url: "/secure-uploads/original/3X/c/b/test.mp4", short_path: "/uploads/short-url/blah", }; return cache; @@ -1065,16 +1065,16 @@ eviltrout

assert.cookedOptions( "![baby shark|video](upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp4)", { - siteSettings: { secure_media: true }, + siteSettings: { secure_uploads: true }, lookupUploadUrls, }, `

`, - "It returns the correct video HTML when the URL is mapped with secure media, removing data-orig-src" + "It returns the correct video HTML when the URL is mapped with secure uploads, removing data-orig-src" ); }); @@ -1089,12 +1089,12 @@ eviltrout

); }); - test("audio - mapped url - secure media enabled", function (assert) { + test("audio - mapped url - secure uploads enabled", function (assert) { function lookupUploadUrls() { let cache = {}; cache["upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp3"] = { short_url: "upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp3", - url: "/secure-media-uploads/original/3X/c/b/test.mp3", + url: "/secure-uploads/original/3X/c/b/test.mp3", short_path: "/uploads/short-url/blah", }; return cache; @@ -1102,14 +1102,14 @@ eviltrout

assert.cookedOptions( "![baby shark|audio](upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp3)", { - siteSettings: { secure_media: true }, + siteSettings: { secure_uploads: true }, lookupUploadUrls, }, `

`, - "It returns the correct audio HTML when the URL is mapped with secure media, removing data-orig-src" + "It returns the correct audio HTML when the URL is mapped with secure uploads, removing data-orig-src" ); }); diff --git a/app/assets/javascripts/discourse/tests/unit/lib/upload-short-url-test.js b/app/assets/javascripts/discourse/tests/unit/lib/upload-short-url-test.js index 79520604a8e..ffa6a7b7bee 100644 --- a/app/assets/javascripts/discourse/tests/unit/lib/upload-short-url-test.js +++ b/app/assets/javascripts/discourse/tests/unit/lib/upload-short-url-test.js @@ -96,7 +96,7 @@ module("Unit | Utility | pretty-text/upload-short-url", function (hooks) { lookup = lookupCachedUploadUrl("upload://a.jpeg"); assert.deepEqual(lookup, {}); - await resolveAllShortUrls(ajax, { secure_media: false }, fixture()); + await resolveAllShortUrls(ajax, { secure_uploads: false }, fixture()); await settled(); lookup = lookupCachedUploadUrl("upload://a.jpeg"); @@ -143,7 +143,7 @@ module("Unit | Utility | pretty-text/upload-short-url", function (hooks) { test("resolveAllShortUrls - href + src replaced correctly", async function (assert) { stubUrls(); - await resolveAllShortUrls(ajax, { secure_media: false }, fixture()); + await resolveAllShortUrls(ajax, { secure_uploads: false }, fixture()); await settled(); let image1 = fixture().querySelector("img"); @@ -167,7 +167,7 @@ module("Unit | Utility | pretty-text/upload-short-url", function (hooks) { test("resolveAllShortUrls - url with full origin replaced correctly", async function (assert) { stubUrls(); - await resolveAllShortUrls(ajax, { secure_media: false }, fixture()); + await resolveAllShortUrls(ajax, { secure_uploads: false }, fixture()); await settled(); let video = fixture().querySelectorAll("video")[1]; @@ -177,25 +177,25 @@ module("Unit | Utility | pretty-text/upload-short-url", function (hooks) { ); }); - test("resolveAllShortUrls - when secure media is enabled use the attachment full URL", async function (assert) { + test("resolveAllShortUrls - when secure uploads is enabled use the attachment full URL", async function (assert) { stubUrls( null, [ { short_url: "upload://c.pdf", - url: "/secure-media-uploads/default/original/3X/c/b/3.pdf", + url: "/secure-uploads/default/original/3X/c/b/3.pdf", short_path: "/uploads/short-url/c.pdf", }, ], null ); - await resolveAllShortUrls(ajax, { secure_media: true }, fixture()); + await resolveAllShortUrls(ajax, { secure_uploads: true }, fixture()); await settled(); let link = fixture().querySelector("a"); assert.strictEqual( link.getAttribute("href"), - "/secure-media-uploads/default/original/3X/c/b/3.pdf" + "/secure-uploads/default/original/3X/c/b/3.pdf" ); }); diff --git a/app/assets/javascripts/discourse/tests/unit/lib/url-test.js b/app/assets/javascripts/discourse/tests/unit/lib/url-test.js index 632e5435977..bdb99b3ce48 100644 --- a/app/assets/javascripts/discourse/tests/unit/lib/url-test.js +++ b/app/assets/javascripts/discourse/tests/unit/lib/url-test.js @@ -143,14 +143,12 @@ module("Unit | Utility | url", function () { ); }); - test("routeTo redirects secure media URLS because they are server side only", async function (assert) { + test("routeTo redirects secure uploads URLS because they are server side only", async function (assert) { sinon.stub(DiscourseURL, "redirectTo"); sinon.stub(DiscourseURL, "handleURL"); - DiscourseURL.routeTo("/secure-media-uploads/original/1X/test.pdf"); + DiscourseURL.routeTo("/secure-uploads/original/1X/test.pdf"); assert.ok( - DiscourseURL.redirectTo.calledWith( - "/secure-media-uploads/original/1X/test.pdf" - ) + DiscourseURL.redirectTo.calledWith("/secure-uploads/original/1X/test.pdf") ); }); diff --git a/app/assets/javascripts/pretty-text/addon/engines/discourse-markdown-it.js b/app/assets/javascripts/pretty-text/addon/engines/discourse-markdown-it.js index 5b1fc15712f..8f6d02d5fd7 100644 --- a/app/assets/javascripts/pretty-text/addon/engines/discourse-markdown-it.js +++ b/app/assets/javascripts/pretty-text/addon/engines/discourse-markdown-it.js @@ -525,7 +525,7 @@ export function setup(opts, siteSettings, state) { getOptions.f = () => opts.discourse; opts.discourse.limitedSiteSettings = { - secureMedia: siteSettings.secure_media, + secureUploads: siteSettings.secure_uploads, enableDiffhtmlPreview: siteSettings.enable_diffhtml_preview, traditionalMarkdownLinebreaks: siteSettings.traditional_markdown_linebreaks, enableMarkdownLinkify: siteSettings.enable_markdown_linkify, diff --git a/app/assets/javascripts/pretty-text/addon/upload-short-url.js b/app/assets/javascripts/pretty-text/addon/upload-short-url.js index 4eab94ca823..4ff0dd89d99 100644 --- a/app/assets/javascripts/pretty-text/addon/upload-short-url.js +++ b/app/assets/javascripts/pretty-text/addon/upload-short-url.js @@ -112,11 +112,12 @@ function getAttributeBasedUrl(dataAttribute, cachedUpload, siteSettings) { return cachedUpload.url; } - // attachments should use the full /secure-media-uploads/ URL - // in this case for permission checks + // attachments should use the full /secure-media-uploads/ or + // /secure-uploads/ URL in this case for permission checks if ( - siteSettings.secure_media && - cachedUpload.url.includes("secure-media-uploads") + siteSettings.secure_uploads && + (cachedUpload.url.includes("secure-media-uploads") || + cachedUpload.url.includes("secure-uploads")) ) { return cachedUpload.url; } diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown/upload-protocol.js b/app/assets/javascripts/pretty-text/engines/discourse-markdown/upload-protocol.js index 5a7a10e24d1..cc6b925d47f 100644 --- a/app/assets/javascripts/pretty-text/engines/discourse-markdown/upload-protocol.js +++ b/app/assets/javascripts/pretty-text/engines/discourse-markdown/upload-protocol.js @@ -136,11 +136,12 @@ function rule(state) { } } else if (token.tag === "a") { if (mapped) { - // when secure media is enabled we want the full /secure-media-uploads/ + // when secure uploads is enabled we want the full /secure-media-uploads or /secure-uploads // url to take advantage of access control security if ( - state.md.options.discourse.limitedSiteSettings.secureMedia && - mapped.url.includes("secure-media-uploads") + state.md.options.discourse.limitedSiteSettings.secureUploads && + (mapped.url.includes("secure-media-uploads") || + mapped.url.includes("secure-uploads")) ) { token.attrs[srcIndex][1] = mapped.url; } else { diff --git a/app/assets/javascripts/service-worker.js.erb b/app/assets/javascripts/service-worker.js.erb index 5f16ce4653e..8580e6e6fce 100644 --- a/app/assets/javascripts/service-worker.js.erb +++ b/app/assets/javascripts/service-worker.js.erb @@ -28,7 +28,7 @@ workbox.routing.registerRoute( plugins: [ new workbox.cacheableResponse.Plugin({ statuses: [200] // opaque responses will return status code '0' - }), // for s3 secure media signed urls + }), // for s3 secure uploads signed urls new workbox.expiration.Plugin({ maxAgeSeconds: 7* 24 * 60 * 60, // 7 days maxEntries: 250, diff --git a/app/controllers/published_pages_controller.rb b/app/controllers/published_pages_controller.rb index eabe216f119..abf27ccda50 100644 --- a/app/controllers/published_pages_controller.rb +++ b/app/controllers/published_pages_controller.rb @@ -94,7 +94,7 @@ private end def ensure_publish_enabled - if !SiteSetting.enable_page_publishing? || SiteSetting.secure_media + if !SiteSetting.enable_page_publishing? || SiteSetting.secure_uploads raise Discourse::NotFound end end diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 36d618cf26d..f1041f57bc6 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -5,13 +5,13 @@ require "mini_mime" class UploadsController < ApplicationController include ExternalUploadHelpers - requires_login except: [:show, :show_short, :show_secure] + requires_login except: [:show, :show_short, :_show_secure_deprecated, :show_secure] - skip_before_action :preload_json, :check_xhr, :redirect_to_login_if_required, only: [:show, :show_short, :show_secure] + skip_before_action :preload_json, :check_xhr, :redirect_to_login_if_required, only: [:show, :show_short, :_show_secure_deprecated, :show_secure] protect_from_forgery except: :show - before_action :is_asset_path, :apply_cdn_headers, only: [:show, :show_short, :show_secure] - before_action :external_store_check, only: [:show_secure] + before_action :is_asset_path, :apply_cdn_headers, only: [:show, :show_short, :_show_secure_deprecated, :show_secure] + before_action :external_store_check, only: [:_show_secure_deprecated, :show_secure] SECURE_REDIRECT_GRACE_SECONDS = 5 @@ -111,7 +111,7 @@ class UploadsController < ApplicationController sha1 = Upload.sha1_from_base62_encoded(params[:base62]) if upload = Upload.find_by(sha1: sha1) - if upload.secure? && SiteSetting.secure_media? + if upload.secure? && SiteSetting.secure_uploads? return handle_secure_upload_request(upload) end @@ -125,6 +125,13 @@ class UploadsController < ApplicationController end end + # Kept to avoid rebaking old posts with /show-secure-uploads/ in their + # contents, this will ensure the uploads in these posts continue to + # work in future. + def _show_secure_deprecated + show_secure + end + def show_secure # do not serve uploads requested via XHR to prevent XSS return xhr_not_allowed if request.xhr? @@ -139,9 +146,9 @@ class UploadsController < ApplicationController return render_404 if upload.blank? return render_404 if SiteSetting.prevent_anons_from_downloading_files && current_user.nil? - return handle_secure_upload_request(upload, path_with_ext) if SiteSetting.secure_media? + return handle_secure_upload_request(upload, path_with_ext) if SiteSetting.secure_uploads? - # we don't want to 404 here if secure media gets disabled + # we don't want to 404 here if secure uploads gets disabled # because all posts with secure uploads will show broken media # until rebaked, which could take some time # diff --git a/app/helpers/email_helper.rb b/app/helpers/email_helper.rb index 3455d026118..952fb40cfeb 100644 --- a/app/helpers/email_helper.rb +++ b/app/helpers/email_helper.rb @@ -67,6 +67,10 @@ module EmailHelper border-color: #454545 !important; } + [data-stripped-secure-upload] { + border-color: #454545 !important; + } + [dm='text-color'] { color: #dddddd; } diff --git a/app/jobs/regular/pull_hotlinked_images.rb b/app/jobs/regular/pull_hotlinked_images.rb index 57aaad6f9d8..7c1accdb89a 100644 --- a/app/jobs/regular/pull_hotlinked_images.rb +++ b/app/jobs/regular/pull_hotlinked_images.rb @@ -108,9 +108,9 @@ module Jobs class UploadCreateError < StandardError; end def attempt_download(src, user_id) - # secure-media-uploads endpoint prevents anonymous downloads, so we + # secure-uploads endpoint prevents anonymous downloads, so we # need the presigned S3 URL here - src = Upload.signed_url_from_secure_media_url(src) if Upload.secure_media_url?(src) + src = Upload.signed_url_from_secure_uploads_url(src) if Upload.secure_uploads_url?(src) hotlinked = download(src) raise ImageBrokenError if !hotlinked @@ -147,7 +147,7 @@ module Jobs ].compact.map { |s| normalize_src(s) } if Discourse.store.has_been_uploaded?(src) || normalize_src(src).start_with?(*local_bases) || src =~ /\A\/[^\/]/i - return false if !(src =~ /\/uploads\// || Upload.secure_media_url?(src)) + return false if !(src =~ /\/uploads\// || Upload.secure_uploads_url?(src)) # Someone could hotlink a file from a different site on the same CDN, # so check whether we have it in this database diff --git a/app/jobs/regular/sync_acls_for_uploads.rb b/app/jobs/regular/sync_acls_for_uploads.rb index 73837ff7e7c..237795cb5b6 100644 --- a/app/jobs/regular/sync_acls_for_uploads.rb +++ b/app/jobs/regular/sync_acls_for_uploads.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Jobs - # Sometimes we need to update a _lot_ of ACLs on S3 (such as when secure media + # Sometimes we need to update a _lot_ of ACLs on S3 (such as when secure uploads # is enabled), and since it takes ~1s per upload to update the ACL, this is # best spread out over many jobs instead of having to do the whole thing serially. class SyncAclsForUploads < ::Jobs::Base diff --git a/app/models/post.rb b/app/models/post.rb index cd2c6c7f459..5f493c45850 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -309,7 +309,7 @@ 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? + if self.with_secure_uploads? each_upload_url do |url| uri = URI.parse(url) if FileHelper.is_supported_media?(File.basename(uri.path)) @@ -517,8 +517,8 @@ class Post < ActiveRecord::Base ReviewableFlaggedPost.pending.find_by(target: self) end - def with_secure_media? - return false if !SiteSetting.secure_media? + def with_secure_uploads? + return false if !SiteSetting.secure_uploads? SiteSetting.login_required? || \ (topic.present? && (topic.private_message? || topic.category&.read_restricted)) end @@ -971,7 +971,7 @@ class Post < ActiveRecord::Base UploadReference.where(target: self).delete_all UploadReference.insert_all(upload_references) if upload_references.size > 0 - if SiteSetting.secure_media? + if SiteSetting.secure_uploads? Upload.where( id: upload_ids, access_control_post_id: nil ).where( @@ -1033,7 +1033,7 @@ class Post < ActiveRecord::Base next if Rails.configuration.multisite && src.exclude?(current_db) src = "#{SiteSetting.force_https ? "https" : "http"}:#{src}" if src.start_with?("//") - next unless Discourse.store.has_been_uploaded?(src) || Upload.secure_media_url?(src) || (include_local_upload && src =~ /\A\/[^\/]/i) + next unless Discourse.store.has_been_uploaded?(src) || Upload.secure_uploads_url?(src) || (include_local_upload && src =~ /\A\/[^\/]/i) path = begin URI(UrlHelper.unencode(GlobalSetting.cdn_url ? src.sub(GlobalSetting.cdn_url, "") : src))&.path diff --git a/app/models/upload.rb b/app/models/upload.rb index f218a9c741f..7bd583f2a6c 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -186,7 +186,7 @@ class Upload < ActiveRecord::Base "upload://#{short_url_basename}" end - def uploaded_before_secure_media_enabled? + def uploaded_before_secure_uploads_enabled? original_sha1.blank? end @@ -204,14 +204,14 @@ class Upload < ActiveRecord::Base end def self.consider_for_reuse(upload, post) - return upload if !SiteSetting.secure_media? || upload.blank? || post.blank? - return nil if !upload.matching_access_control_post?(post) || upload.uploaded_before_secure_media_enabled? + return upload if !SiteSetting.secure_uploads? || upload.blank? || post.blank? + return nil if !upload.matching_access_control_post?(post) || upload.uploaded_before_secure_uploads_enabled? upload end - def self.secure_media_url?(url) + def self.secure_uploads_url?(url) # we do not want to exclude topic links that for whatever reason - # have secure-media-uploads in the URL e.g. /t/secure-media-uploads-are-cool/223452 + # have secure-uploads in the URL e.g. /t/secure-uploads-are-cool/223452 route = UrlHelper.rails_route_from_url(url) return false if route.blank? route[:action] == "show_secure" && route[:controller] == "uploads" && FileHelper.is_supported_media?(url) @@ -219,14 +219,14 @@ class Upload < ActiveRecord::Base false end - def self.signed_url_from_secure_media_url(url) + def self.signed_url_from_secure_uploads_url(url) route = UrlHelper.rails_route_from_url(url) url = Rails.application.routes.url_for(route.merge(only_path: true)) secure_upload_s3_path = url[url.index(route[:path])..-1] Discourse.store.signed_url_for_path(secure_upload_s3_path) end - def self.secure_media_url_from_upload_url(url) + def self.secure_uploads_url_from_upload_url(url) return url if !url.include?(SiteSetting.Upload.absolute_base_url) uri = URI.parse(url) Rails.application.routes.url_for( diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb index 1207a9a5152..57bd35aaf48 100644 --- a/app/serializers/topic_view_serializer.rb +++ b/app/serializers/topic_view_serializer.rb @@ -290,7 +290,7 @@ class TopicViewSerializer < ApplicationSerializer SiteSetting.enable_page_publishing? && scope.is_staff? && object.published_page.present? && - !SiteSetting.secure_media + !SiteSetting.secure_uploads end def thumbnails diff --git a/app/serializers/upload_serializer.rb b/app/serializers/upload_serializer.rb index a31a6d02100..7626c0e1e23 100644 --- a/app/serializers/upload_serializer.rb +++ b/app/serializers/upload_serializer.rb @@ -17,6 +17,6 @@ class UploadSerializer < ApplicationSerializer :dominant_color def url - object.for_site_setting ? object.url : UrlHelper.cook_url(object.url, secure: SiteSetting.secure_media? && object.secure) + object.for_site_setting ? object.url : UrlHelper.cook_url(object.url, secure: SiteSetting.secure_uploads? && object.secure) end end diff --git a/config/initializers/006-mini_profiler.rb b/config/initializers/006-mini_profiler.rb index d23a75b1ac1..96428943525 100644 --- a/config/initializers/006-mini_profiler.rb +++ b/config/initializers/006-mini_profiler.rb @@ -45,6 +45,7 @@ if defined?(Rack::MiniProfiler) && defined?(Rack::MiniProfiler::Config) /^\/site_customizations/, /^\/uploads/, /^\/secure-media-uploads/, + /^\/secure-uploads/, /^\/javascripts\//, /^\/images\//, /^\/stylesheets\//, diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 5558bc40d26..fdfa668e779 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -139,7 +139,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 allowlist or is on the blocklist." unrecognized_error: "Unrecognized Error" - secure_media_placeholder: "Redacted: This site has secure media enabled, visit the topic or click View Media to see the attached media." + secure_uploads_placeholder: "Redacted: This site has secure uploads enabled, visit the topic or click View Media to see the attached uploads." view_redacted_media: "View Media" errors: &errors @@ -210,7 +210,7 @@ en: page_publishing_requirements: "Page publishing cannot be enabled if secure media is enabled." 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." + secure_uploads_requirements: "S3 uploads must be enabled before enabling secure uploads." share_quote_facebook_requirements: "You must set a Facebook app id to enable quote sharing for Facebook." second_factor_cannot_enforce_with_socials: "You cannot enforce 2FA with social logins enabled. You must first disable login via: %{auth_provider_names}" second_factor_cannot_be_enforced_with_disabled_local_login: "You cannot enforce 2FA if local logins are disabled." @@ -2219,9 +2219,12 @@ en: bootstrap_mode_min_users: "Minimum number of users required to disable bootstrap mode (set to 0 to disable)" prevent_anons_from_downloading_files: "Prevent anonymous users from downloading attachments." - secure_media: 'Limits access to ALL uploads (images, video, audio, text, pdfs, zips, and others). If “login required” is enabled, only logged-in users can access uploads. Otherwise, access will be limited only for media uploads in private messages and private categories. WARNING: This setting is complex and requires deep administrative understanding. See the secure media topic on Meta for details.' - secure_media_allow_embed_images_in_emails: "Allows embedding secure images that would normally be redacted in emails, if their size is smaller than the 'secure media max email embed image size kb' setting." - secure_media_max_email_embed_image_size_kb: "The size cutoff for secure images that will be embedded in emails if the 'secure media allow embed in emails' setting is enabled. Without that setting enabled, this setting has no effect." + secure_media: 'DEPRECATED: Use the secure_uploads setting instead, will be removed in Discourse 3.0.' + secure_uploads: 'Limits access to ALL uploads (images, video, audio, text, pdfs, zips, and others). If "login required” is enabled, only logged-in users can access uploads. Otherwise, access will be limited only for media uploads in private messages and private categories. WARNING: This setting is complex and requires deep administrative understanding. See the secure uploads topic on Meta for details.' + secure_media_allow_embed_images_in_emails: "DEPRECATED: Use secure_uploads_allow_embed_images_in_emails, will remove in Discourse 3.0." + secure_uploads_allow_embed_images_in_emails: "Allows embedding secure images that would normally be redacted in emails, if their size is smaller than the 'secure uploads max email embed image size kb' setting." + secure_media_max_email_embed_image_size_kb: "DEPRECATED: Use secure_uploads_max_email_embed_image_size_kb, will be removed in Discourse 3.0." + secure_uploads_max_email_embed_image_size_kb: "The size cutoff for secure images that will be embedded in emails if the 'secure uploads allow embed in emails' setting is enabled. Without that setting enabled, this setting has no effect." slug_generation_method: "Choose a slug generation method. 'encoded' will generate percent encoding string. 'none' will disable slug at all." enable_emoji: "Enable emoji" diff --git a/config/nginx.sample.conf b/config/nginx.sample.conf index a791d82dc2d..970f845ba0f 100644 --- a/config/nginx.sample.conf +++ b/config/nginx.sample.conf @@ -110,7 +110,7 @@ server { break; } - location ~ ^/secure-media-uploads/ { + location ~ ^/(secure-media-uploads/|secure-uploads)/ { proxy_set_header Host $http_host; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Request-Start "t=${msec}"; diff --git a/config/routes.rb b/config/routes.rb index 6b9e4958f28..7bd9ae757d3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -583,7 +583,12 @@ Discourse::Application.routes.draw do end # used to download attachments (old route) get "uploads/:site/:id/:sha" => "uploads#show", constraints: { site: /\w+/, id: /\d+/, sha: /\h{16}/, format: /.*/ } - get "secure-media-uploads/*path(.:extension)" => "uploads#show_secure", constraints: { extension: /[a-z0-9\._]+/i } + + # NOTE: secure-media-uploads is the old form, all new URLs generated for + # secure uploads will be secure-uploads, this is left in for backwards + # compat without needing to rebake all posts for each site. + get "secure-media-uploads/*path(.:extension)" => "uploads#_show_secure_deprecated", constraints: { extension: /[a-z0-9\._]+/i } + get "secure-uploads/*path(.:extension)" => "uploads#show_secure", constraints: { extension: /[a-z0-9\._]+/i } get "posts" => "posts#latest", id: "latest_posts", constraints: { format: /(json|rss)/ } get "private-posts" => "posts#latest", id: "private_posts", constraints: { format: /(json|rss)/ } diff --git a/config/site_settings.yml b/config/site_settings.yml index f2106b26382..7dc15b97466 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1365,12 +1365,24 @@ files: secure_media: default: false client: true + hidden: true secure_media_allow_embed_images_in_emails: default: true + hidden: true secure_media_max_email_embed_image_size_kb: default: 1024 min: 1 max: 10240 + hidden: true + secure_uploads: + default: false + client: true + secure_uploads_allow_embed_images_in_emails: + default: true + secure_uploads_max_email_embed_image_size_kb: + default: 1024 + min: 1 + max: 10240 enable_s3_uploads: default: false client: true diff --git a/db/migrate/20220927065328_set_secure_uploads_settings_based_on_secure_media_equivalent.rb b/db/migrate/20220927065328_set_secure_uploads_settings_based_on_secure_media_equivalent.rb new file mode 100644 index 00000000000..bf7095c0a1e --- /dev/null +++ b/db/migrate/20220927065328_set_secure_uploads_settings_based_on_secure_media_equivalent.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +class SetSecureUploadsSettingsBasedOnSecureMediaEquivalent < ActiveRecord::Migration[7.0] + def up + secure_media_enabled = DB.query_single("SELECT value FROM site_settings WHERE name = 'secure_media'") + + if secure_media_enabled.present? && secure_media_enabled[0] == "t" + execute <<~SQL + INSERT INTO site_settings(name, data_type, value, created_at, updated_at) + VALUES ('secure_uploads', 5, 't', now(), now()) + SQL + end + + secure_media_allow_embed_images_in_emails = DB.query_single("SELECT value FROM site_settings WHERE name = 'secure_media_allow_embed_images_in_emails'") + + if secure_media_allow_embed_images_in_emails.present? && secure_media_allow_embed_images_in_emails[0] == "t" + execute <<~SQL + INSERT INTO site_settings(name, data_type, value, created_at, updated_at) + VALUES ('secure_uploads_allow_embed_images_in_emails', 5, 't', now(), now()) + SQL + end + + secure_media_max_email_embed_image_size_kb = DB.query_single("SELECT value FROM site_settings WHERE name = 'secure_media_max_email_embed_image_size_kb'") + + if secure_media_max_email_embed_image_size_kb.present? + execute <<~SQL + INSERT INTO site_settings(name, data_type, value, created_at, updated_at) + VALUES ('secure_uploads_max_email_embed_image_size_kb', 3, '#{secure_uploads_max_email_embed_image_size_kb[0]}', now(), now()) + SQL + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb index f2a3717ab19..f3b6e712d76 100644 --- a/lib/cooked_post_processor.rb +++ b/lib/cooked_post_processor.rb @@ -22,7 +22,7 @@ class CookedPostProcessor @cooking_options = post.cooking_options || opts[:cooking_options] || {} @cooking_options[:topic_id] = post.topic_id @cooking_options = @cooking_options.symbolize_keys - @with_secure_media = @post.with_secure_media? + @with_secure_uploads = @post.with_secure_uploads? @category_id = @post&.topic&.category_id cooked = post.cook(post.raw, @cooking_options) @@ -225,14 +225,14 @@ class CookedPostProcessor resized_h = (h * ratio).to_i if !cropped && upload.width && resized_w > upload.width - cooked_url = UrlHelper.cook_url(upload.url, secure: @post.with_secure_media?) + cooked_url = UrlHelper.cook_url(upload.url, secure: @post.with_secure_uploads?) srcset << ", #{cooked_url} #{ratio.to_s.sub(/\.0$/, "")}x" elsif t = upload.thumbnail(resized_w, resized_h) - cooked_url = UrlHelper.cook_url(t.url, secure: @post.with_secure_media?) + cooked_url = UrlHelper.cook_url(t.url, secure: @post.with_secure_uploads?) srcset << ", #{cooked_url} #{ratio.to_s.sub(/\.0$/, "")}x" end - img["srcset"] = "#{UrlHelper.cook_url(img["src"], secure: @post.with_secure_media?)}#{srcset}" if srcset.present? + img["srcset"] = "#{UrlHelper.cook_url(img["src"], secure: @post.with_secure_uploads?)}#{srcset}" if srcset.present? end else img["src"] = upload.url @@ -250,7 +250,7 @@ class CookedPostProcessor lightbox.add_child(img) # then, the link to our larger image - src = UrlHelper.cook_url(img["src"], secure: @post.with_secure_media?) + src = UrlHelper.cook_url(img["src"], secure: @post.with_secure_uploads?) a = create_link_node("lightbox", src) img.add_next_sibling(a) @@ -323,7 +323,7 @@ class CookedPostProcessor @doc.css("img[#{selector}]").each do |img| custom_emoji = img["class"]&.include?("emoji-custom") && Emoji.custom?(img["title"]) img[selector] = UrlHelper.cook_url( - img[selector].to_s, secure: @post.with_secure_media? && !custom_emoji + img[selector].to_s, secure: @post.with_secure_uploads? && !custom_emoji ) end end @@ -384,7 +384,7 @@ class CookedPostProcessor still_an_image = false elsif info&.downloaded? && upload = info&.upload - img["src"] = UrlHelper.cook_url(upload.url, secure: @with_secure_media) + img["src"] = UrlHelper.cook_url(upload.url, secure: @with_secure_uploads) img.delete(PrettyText::BLOCKED_HOTLINKED_SRC_ATTR) end diff --git a/lib/cooked_processor_mixin.rb b/lib/cooked_processor_mixin.rb index 975f765b8fa..e22777ba73b 100644 --- a/lib/cooked_processor_mixin.rb +++ b/lib/cooked_processor_mixin.rb @@ -175,10 +175,10 @@ module CookedProcessorMixin # FastImage fails when there's no scheme absolute_url = SiteSetting.scheme + ":" + absolute_url if absolute_url.start_with?("//") - # we can't direct FastImage to our secure-media-uploads url because it bounces + # we can't direct FastImage to our secure-uploads url because it bounces # anonymous requests with a 404 error - if url && Upload.secure_media_url?(url) - absolute_url = Upload.signed_url_from_secure_media_url(absolute_url) + if url && Upload.secure_uploads_url?(url) + absolute_url = Upload.signed_url_from_secure_uploads_url(absolute_url) end return unless is_valid_image_url?(absolute_url) diff --git a/lib/email/sender.rb b/lib/email/sender.rb index 0bcf4a2cf7e..fd6148cdc65 100644 --- a/lib/email/sender.rb +++ b/lib/email/sender.rb @@ -242,7 +242,7 @@ module Email # Embeds any of the secure images that have been attached inline, # removing the redaction notice. - if SiteSetting.secure_media_allow_embed_images_in_emails + if SiteSetting.secure_uploads_allow_embed_images_in_emails style.inline_secure_images(@message.attachments, @message_attachments_index) end @@ -357,8 +357,8 @@ module Email end def should_attach_image?(upload, optimized_1X = nil) - return if !SiteSetting.secure_media_allow_embed_images_in_emails || !upload.secure? - return if (optimized_1X&.filesize || upload.filesize) > SiteSetting.secure_media_max_email_embed_image_size_kb.kilobytes + return if !SiteSetting.secure_uploads_allow_embed_images_in_emails || !upload.secure? + return if (optimized_1X&.filesize || upload.filesize) > SiteSetting.secure_uploads_max_email_embed_image_size_kb.kilobytes true end diff --git a/lib/email/styles.rb b/lib/email/styles.rb index 7502341178c..ed98e5e2a8a 100644 --- a/lib/email/styles.rb +++ b/lib/email/styles.rb @@ -257,10 +257,10 @@ module Email end def inline_secure_images(attachments, attachments_index) - stripped_media = @fragment.css('[data-stripped-secure-media]') + stripped_media = @fragment.css('[data-stripped-secure-media], [data-stripped-secure-upload]') upload_shas = {} stripped_media.each do |div| - url = div['data-stripped-secure-media'] + url = div['data-stripped-secure-media'] || div['data-stripped-secure-upload'] filename = File.basename(url) filename_bare = filename.gsub(File.extname(filename), "") sha1 = filename_bare.partition('_').first @@ -269,7 +269,9 @@ module Email uploads = Upload.select(:original_filename, :sha1).where(sha1: upload_shas.values) stripped_media.each do |div| - upload = uploads.find { |upl| upl.sha1 == upload_shas[div['data-stripped-secure-media']] } + upload = uploads.find do |upl| + upl.sha1 == (upload_shas[div['data-stripped-secure-media']] || upload_shas[div['data-stripped-secure-upload']]) + end next if !upload if attachments[attachments_index[upload.sha1]] @@ -294,7 +296,7 @@ module Email def to_html # needs to be before class + id strip because we need to style redacted # media and also not double-redact already redacted from lower levels - replace_secure_media_urls if SiteSetting.secure_media? + replace_secure_uploads_urls if SiteSetting.secure_uploads? strip_classes_and_ids replace_relative_urls @@ -369,13 +371,13 @@ module Email end end - def replace_secure_media_urls + def replace_secure_uploads_urls # strip again, this can be done at a lower level like in the user # notification template but that may not catch everything - PrettyText.strip_secure_media(@fragment) + PrettyText.strip_secure_uploads(@fragment) - style('div.secure-media-notice', 'border: 5px solid #e9e9e9; padding: 5px; display: inline-block;') - style('div.secure-media-notice a', "color: #{SiteSetting.email_link_color}") + style('div.secure-upload-notice', 'border: 5px solid #e9e9e9; padding: 5px; display: inline-block;') + style('div.secure-upload-notice a', "color: #{SiteSetting.email_link_color}") end def correct_first_body_margin diff --git a/lib/guardian.rb b/lib/guardian.rb index d4b557ec4ac..16d50003d05 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -543,7 +543,7 @@ class Guardian def can_publish_page?(topic) return false if !SiteSetting.enable_page_publishing? - return false if SiteSetting.secure_media? + return false if SiteSetting.secure_uploads? return false if topic.blank? return false if topic.private_message? return false unless can_see_topic?(topic) diff --git a/lib/post_creator.rb b/lib/post_creator.rb index b368edb0976..f58af75a00b 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -398,7 +398,7 @@ class PostCreator end def update_uploads_secure_status - @post.update_uploads_secure_status(source: "post creator") if SiteSetting.secure_media? + @post.update_uploads_secure_status(source: "post creator") if SiteSetting.secure_uploads? end def delete_owned_bookmarks diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index 3aa05989bbd..a0a558de96a 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -488,14 +488,14 @@ module PrettyText end end - def self.strip_secure_media(doc) + def self.strip_secure_uploads(doc) # images inside a lightbox or other link doc.css('a[href]').each do |a| - next if !Upload.secure_media_url?(a['href']) + next if !Upload.secure_uploads_url?(a['href']) non_image_media = %w(video audio).include?(a&.parent&.name) target = non_image_media ? a.parent : a - next if target.to_s.include?('stripped-secure-view-media') + next if target.to_s.include?('stripped-secure-view-media') || target.to_s.include?('stripped-secure-view-upload') next if a.css('img[src]').empty? && !non_image_media @@ -509,12 +509,12 @@ module PrettyText else url = img['src'] end - a.add_next_sibling secure_media_placeholder(doc, url, width: img['width'], height: img['height']) + a.add_next_sibling secure_uploads_placeholder(doc, url, width: img['width'], height: img['height']) a.remove else width = non_image_media ? nil : a.at_css('img').attr('width') height = non_image_media ? nil : a.at_css('img').attr('height') - target.add_next_sibling secure_media_placeholder(doc, a['href'], width: width, height: height) + target.add_next_sibling secure_uploads_placeholder(doc, a['href'], width: width, height: height) target.remove end end @@ -551,20 +551,20 @@ module PrettyText height = 16 end - if Upload.secure_media_url?(url) - img.add_next_sibling secure_media_placeholder(doc, url, onebox_type: onebox_type, width: width, height: height) + if Upload.secure_uploads_url?(url) + img.add_next_sibling secure_uploads_placeholder(doc, url, onebox_type: onebox_type, width: width, height: height) img.remove end end end - def self.secure_media_placeholder(doc, url, onebox_type: false, width: nil, height: nil) + def self.secure_uploads_placeholder(doc, url, onebox_type: false, width: nil, height: nil) data_width = width ? "data-width=#{width}" : '' data_height = height ? "data-height=#{height}" : '' data_onebox_type = onebox_type ? "data-onebox-type='#{onebox_type}'" : '' <<~HTML -
- #{I18n.t('emails.secure_media_placeholder')} #{I18n.t("emails.view_redacted_media")}. +
+ #{I18n.t('emails.secure_uploads_placeholder')} #{I18n.t("emails.view_redacted_media")}.
HTML end @@ -572,7 +572,7 @@ module PrettyText def self.format_for_email(html, post = nil) doc = Nokogiri::HTML5.fragment(html) DiscourseEvent.trigger(:reduce_cooked, doc, post) - strip_secure_media(doc) if post&.with_secure_media? + strip_secure_uploads(doc) if post&.with_secure_uploads? 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 b675900b4c9..8a4c13def9b 100644 --- a/lib/pretty_text/helpers.rb +++ b/lib/pretty_text/helpers.rb @@ -70,11 +70,11 @@ module PrettyText sha1, url, extension, original_filename, secure = row if short_urls = reverse_map[sha1] - secure_media = SiteSetting.secure_media? && secure + secure_uploads = SiteSetting.secure_uploads? && secure short_urls.each do |short_url| result[short_url] = { - url: secure_media ? Upload.secure_media_url_from_upload_url(url) : Discourse.store.cdn_url(url), + url: secure_uploads ? Upload.secure_uploads_url_from_upload_url(url) : Discourse.store.cdn_url(url), short_path: Upload.short_path(sha1: sha1, extension: extension), base62_sha1: Upload.base62_sha1(sha1) } diff --git a/lib/site_settings/deprecated_settings.rb b/lib/site_settings/deprecated_settings.rb index 8690c79ae90..898ca2ceaa6 100644 --- a/lib/site_settings/deprecated_settings.rb +++ b/lib/site_settings/deprecated_settings.rb @@ -9,6 +9,9 @@ module SiteSettings::DeprecatedSettings ['default_categories_regular', 'default_categories_normal', true, '3.0'], ['min_trust_to_send_messages', 'personal_message_enabled_groups', false, '3.0'], ['enable_personal_messages', 'personal_message_enabled_groups', false, '3.0'], + ['secure_media', 'secure_uploads', true, '3.0'], + ['secure_media_allow_embed_images_in_emails', 'secure_uploads_allow_embed_images_in_emails', true, '3.0'], + ['secure_media_max_email_embed_image_size_kb', 'secure_uploads_max_email_embed_image_size_kb', true, '3.0'], ] def setup_deprecated_methods diff --git a/lib/site_settings/validations.rb b/lib/site_settings/validations.rb index e154bb5fee5..43a34d48255 100644 --- a/lib/site_settings/validations.rb +++ b/lib/site_settings/validations.rb @@ -162,12 +162,12 @@ module SiteSettings::Validations validate_error :s3_upload_bucket_is_required if SiteSetting.s3_upload_bucket.blank? end - def validate_secure_media(new_val) - validate_error :secure_media_requirements if new_val == "t" && !SiteSetting.Upload.enable_s3_uploads + def validate_secure_uploads(new_val) + validate_error :secure_uploads_requirements if new_val == "t" && !SiteSetting.Upload.enable_s3_uploads end def validate_enable_page_publishing(new_val) - validate_error :page_publishing_requirements if new_val == "t" && SiteSetting.secure_media? + validate_error :page_publishing_requirements if new_val == "t" && SiteSetting.secure_uploads? end def validate_share_quote_buttons(new_val) diff --git a/lib/tasks/uploads.rake b/lib/tasks/uploads.rake index 29df0e4e97e..4d685580b40 100644 --- a/lib/tasks/uploads.rake +++ b/lib/tasks/uploads.rake @@ -518,16 +518,16 @@ task "uploads:sync_s3_acls" => :environment do end end -task "uploads:disable_secure_media" => :environment do +task "uploads:disable_secure_uploads" => :environment do RailsMultisite::ConnectionManagement.each_connection do |db| unless Discourse.store.external? puts "This task only works for external storage." exit 1 end - puts "Disabling secure media and resetting uploads to not secure in #{db}...", "" + puts "Disabling secure upload and resetting uploads to not secure in #{db}...", "" - SiteSetting.secure_media = false + SiteSetting.secure_uploads = false secure_uploads = Upload.joins(:upload_references).where(upload_references: { target_type: 'Post' }).where(secure: true) secure_upload_count = secure_uploads.count @@ -537,7 +537,7 @@ task "uploads:disable_secure_media" => :environment do secure_uploads.update_all( secure: false, security_last_changed_at: Time.zone.now, - security_last_changed_reason: "marked as not secure by disable_secure_media task" + security_last_changed_reason: "marked as not secure by disable_secure_uploads task" ) post_ids_to_rebake = DB.query_single( @@ -550,11 +550,11 @@ task "uploads:disable_secure_media" => :environment do puts "", "Rebaking and uploading complete!", "" end - puts "", "Secure media is now disabled!", "" + puts "", "Secure uploads are now disabled!", "" end ## -# Run this task whenever the secure_media or login_required +# Run this task whenever the secure_uploads or login_required # settings are changed for a Discourse instance to update # the upload secure flag and S3 upload ACLs. Any uploads that # have their secure status changed will have all associated posts @@ -569,12 +569,12 @@ task "uploads:secure_upload_analyse_and_update" => :environment do puts "Analyzing security for uploads in #{db}...", "" all_upload_ids_changed, post_ids_to_rebake = nil Upload.transaction do - # If secure media is enabled we need to first set the access control post of + # If secure upload is enabled we need to first set the access control post of # all post uploads (even uploads that are linked to multiple posts). If the - # upload is not set to secure media then this has no other effect on the upload, - # but we _must_ know what the access control post is because the with_secure_media? + # upload is not set to secure upload then this has no other effect on the upload, + # but we _must_ know what the access control post is because the with_secure_uploads? # method is on the post, and this knows about the category security & PM status - if SiteSetting.secure_media? + if SiteSetting.secure_uploads? update_uploads_access_control_post end diff --git a/lib/topic_upload_security_manager.rb b/lib/topic_upload_security_manager.rb index edbb28f7dd0..3425297a2d0 100644 --- a/lib/topic_upload_security_manager.rb +++ b/lib/topic_upload_security_manager.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true ## -# There are certain conditions with secure media when the security of +# There are certain conditions with secure uploads when the security of # uploads will need to change depending on the context they reside in. # # For example on these conditions: @@ -38,16 +38,16 @@ class TopicUploadSecurityManager end end - return if !SiteSetting.secure_media + return if !SiteSetting.secure_uploads - # we only want to do this if secure media is enabled. if + # we only want to do this if secure uploads is enabled. if # the setting is turned on after a site has been running # already, we want to make sure that any post moves after # this are handled and upload secure statuses and ACLs # are updated appropriately, as well as setting the access control # post for secure uploads missing it. # - # examples (all after secure media is enabled): + # examples (all after secure uploads is enabled): # # -> a public topic is moved to a private category after # -> a PM is converted to a public topic diff --git a/lib/upload_creator.rb b/lib/upload_creator.rb index 8b5655c4b3b..52ace3202a5 100644 --- a/lib/upload_creator.rb +++ b/lib/upload_creator.rb @@ -94,15 +94,15 @@ class UploadCreator if !external_upload_too_big sha1 = Upload.generate_digest(@file) end - if SiteSetting.secure_media || external_upload_too_big + if SiteSetting.secure_uploads || external_upload_too_big unique_hash = generate_fake_sha1_hash end - # we do not check for duplicate uploads if secure media is + # we do not check for duplicate uploads if secure uploads is # enabled because we use a unique access hash to differentiate # between uploads instead of the sha1, and to get around various # access/permission issues for uploads - if !SiteSetting.secure_media && !external_upload_too_big + if !SiteSetting.secure_uploads && !external_upload_too_big # do we already have that upload? @upload = Upload.find_by(sha1: sha1) @@ -140,8 +140,8 @@ class UploadCreator @upload.user_id = user_id @upload.original_filename = fixed_original_filename || @filename @upload.filesize = filesize - @upload.sha1 = (SiteSetting.secure_media? || external_upload_too_big) ? unique_hash : sha1 - @upload.original_sha1 = SiteSetting.secure_media? ? sha1 : nil + @upload.sha1 = (SiteSetting.secure_uploads? || external_upload_too_big) ? unique_hash : sha1 + @upload.original_sha1 = SiteSetting.secure_uploads? ? sha1 : nil @upload.url = "" @upload.origin = @opts[:origin][0...1000] if @opts[:origin] @upload.extension = image_type || File.extname(@filename)[1..10] @@ -173,7 +173,7 @@ class UploadCreator add_metadata! - if SiteSetting.secure_media + if SiteSetting.secure_uploads secure, reason = UploadSecurity.new(@upload, @opts.merge(creating: true)).should_be_secure_with_reason attrs = @upload.secure_params(secure, reason, "upload creator") @upload.assign_attributes(attrs) diff --git a/lib/upload_security.rb b/lib/upload_security.rb index 6eead29250e..dcaa7f10980 100644 --- a/lib/upload_security.rb +++ b/lib/upload_security.rb @@ -66,8 +66,8 @@ class UploadSecurity [false, "no checks satisfied"] end - def secure_media_disabled_check - !SiteSetting.secure_media? + def secure_uploads_disabled_check + !SiteSetting.secure_uploads? end def insecure_creation_for_modifiers_check @@ -96,14 +96,14 @@ class UploadSecurity # whether the upload should remain secure or not after posting depends on its context, # which is based on the post it is linked to via access_control_post_id. - # if that post is with_secure_media? then the upload should also be secure. + # if that post is with_secure_uploads? then the upload should also be secure. # this may change to false if the upload was set to secure on upload e.g. in # a post composer then it turned out that the post itself was not in a secure context # - # a post is with secure media if it is a private message or in a read restricted + # a post is with secure uploads if it is a private message or in a read restricted # category - def access_control_post_has_secure_media_check - access_control_post&.with_secure_media? + def access_control_post_has_secure_uploads_check + access_control_post&.with_secure_uploads? end def uploading_in_composer_check @@ -127,7 +127,7 @@ class UploadSecurity def insecure_context_checks { - secure_media_disabled: "secure media is disabled", + secure_uploads_disabled: "secure uploads is disabled", insecure_creation_for_modifiers: "one or more creation for_modifiers was satisfied", public_type: "upload is public type", custom_emoji: "upload is used for custom emoji", @@ -138,7 +138,7 @@ class UploadSecurity def secure_context_checks { login_required: "login is required", - access_control_post_has_secure_media: "access control post dictates security", + access_control_post_has_secure_uploads: "access control post dictates security", secure_creation_for_modifiers: "one or more creation for_modifiers was satisfied", uploading_in_composer: "uploading via the composer", already_secure: "upload is already secure" @@ -149,7 +149,7 @@ class UploadSecurity # of whether an upload should be secure or not, and thus should be returned # immediately if there is an access control post def priority_check?(check) - check == :access_control_post_has_secure_media && access_control_post + check == :access_control_post_has_secure_uploads && access_control_post end def perform_check(check) diff --git a/lib/url_helper.rb b/lib/url_helper.rb index 8065d267969..6b43cf609ab 100644 --- a/lib/url_helper.rb +++ b/lib/url_helper.rb @@ -57,7 +57,7 @@ class UrlHelper end def self.secure_proxy_without_cdn(url) - self.absolute(Upload.secure_media_url_from_upload_url(url), nil) + self.absolute(Upload.secure_uploads_url_from_upload_url(url), nil) end def self.escape_uri(uri) @@ -105,14 +105,14 @@ class UrlHelper end def self.cook_url(url, secure: false, local: nil) - is_secure = SiteSetting.secure_media && secure + is_secure = SiteSetting.secure_uploads && secure local = is_local(url) if local.nil? return url if !local url = is_secure ? secure_proxy_without_cdn(url) : absolute_without_cdn(url) - # we always want secure media to come from - # Discourse.base_url_no_prefix/secure-media-uploads + # we always want secure uploads to come from + # Discourse.base_url_no_prefix/secure-uploads # to avoid asset_host mixups return schemaless(url) if is_secure diff --git a/spec/jobs/pull_hotlinked_images_spec.rb b/spec/jobs/pull_hotlinked_images_spec.rb index c30f5a714ba..8e1709ef86e 100644 --- a/spec/jobs/pull_hotlinked_images_spec.rb +++ b/spec/jobs/pull_hotlinked_images_spec.rb @@ -222,14 +222,14 @@ RSpec.describe Jobs::PullHotlinkedImages do expect(post.raw).to eq(raw) end - context "when secure media enabled for an upload that has already been downloaded and exists" do + context "when secure uploads enabled for an upload that has already been downloaded and exists" do it "doesnt redownload the secure upload" do setup_s3 - SiteSetting.secure_media = true + SiteSetting.secure_uploads = true upload = Fabricate(:secure_upload_s3, secure: true) stub_s3(upload) - url = Upload.secure_media_url_from_upload_url(upload.url) + url = Upload.secure_uploads_url_from_upload_url(upload.url) url = Discourse.base_url + url post = Fabricate(:post, raw: "") upload.update(access_control_post: post) @@ -240,12 +240,12 @@ RSpec.describe Jobs::PullHotlinkedImages do context "when the upload original_sha1 is missing" do it "redownloads the upload" do setup_s3 - SiteSetting.secure_media = true + SiteSetting.secure_uploads = true upload = Fabricate(:upload_s3, secure: true) stub_s3(upload) - Upload.stubs(:signed_url_from_secure_media_url).returns(upload.url) - url = Upload.secure_media_url_from_upload_url(upload.url) + Upload.stubs(:signed_url_from_secure_uploads_url).returns(upload.url) + url = Upload.secure_uploads_url_from_upload_url(upload.url) url = Discourse.base_url + url post = Fabricate(:post, raw: "") upload.update(access_control_post: post) @@ -261,12 +261,12 @@ RSpec.describe Jobs::PullHotlinkedImages do context "when the upload access_control_post is different to the current post" do it "redownloads the upload" do setup_s3 - SiteSetting.secure_media = true + SiteSetting.secure_uploads = true upload = Fabricate(:secure_upload_s3, secure: true) stub_s3(upload) - Upload.stubs(:signed_url_from_secure_media_url).returns(upload.url) - url = Upload.secure_media_url_from_upload_url(upload.url) + Upload.stubs(:signed_url_from_secure_uploads_url).returns(upload.url) + url = Upload.secure_uploads_url_from_upload_url(upload.url) url = Discourse.base_url + url post = Fabricate(:post, raw: "") upload.update(access_control_post: Fabricate(:post)) @@ -466,14 +466,14 @@ RSpec.describe Jobs::PullHotlinkedImages do expect(subject.should_download_image?(Fabricate(:upload).url)).to eq(false) end - context "when secure media enabled" do - it 'should return false for secure-media-upload url' do + context "when secure uploads enabled" do + it 'should return false for secure-upload url' do setup_s3 - SiteSetting.secure_media = true + SiteSetting.secure_uploads = true upload = Fabricate(:upload_s3, secure: true) stub_s3(upload) - url = Upload.secure_media_url_from_upload_url(upload.url) + url = Upload.secure_uploads_url_from_upload_url(upload.url) expect(subject.should_download_image?(url)).to eq(false) end end diff --git a/spec/jobs/regular/update_post_uploads_secure_status_spec.rb b/spec/jobs/regular/update_post_uploads_secure_status_spec.rb index f8e2b7be530..8d728f770d9 100644 --- a/spec/jobs/regular/update_post_uploads_secure_status_spec.rb +++ b/spec/jobs/regular/update_post_uploads_secure_status_spec.rb @@ -14,7 +14,7 @@ RSpec.describe Jobs::UpdatePostUploadsSecureStatus do before do setup_s3 stub_s3_store - SiteSetting.secure_media = true + SiteSetting.secure_uploads = true end context "when login_required" do diff --git a/spec/lib/cooked_post_processor_spec.rb b/spec/lib/cooked_post_processor_spec.rb index e808b0116cc..9dcf7759338 100644 --- a/spec/lib/cooked_post_processor_spec.rb +++ b/spec/lib/cooked_post_processor_spec.rb @@ -485,7 +485,7 @@ RSpec.describe CookedPostProcessor do stub_upload(upload) SiteSetting.login_required = true - SiteSetting.secure_media = true + SiteSetting.secure_uploads = true end let(:optimized_size) { "600x500" } @@ -496,7 +496,7 @@ RSpec.describe CookedPostProcessor do let(:cooked_html) do <<~HTML -