From abca91cc4de7bbf1e6f8863f8beba0e8044f7d4f Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Tue, 7 Jan 2020 12:27:24 +1000 Subject: [PATCH] FEATURE: Add rake task to disable secure media (#8669) * Add a rake task to disable secure media. This sets all uploads to `secure: false`, changes the upload ACL to public, and rebakes all the posts using the uploads to make sure they point to the correct URLs. This is in a transaction for each upload with the upload being updated the last step, so if the task fails it can be resumed. * Also allow viewing media via the secure url if secure media is disabled, redirecting to the normal CDN url, because otherwise media links will be broken while we go and rebake all the posts + update ACLs --- app/controllers/uploads_controller.rb | 19 ++++++++++-- lib/tasks/uploads.rake | 37 ++++++++++++++++++++++++ spec/requests/uploads_controller_spec.rb | 17 +++++++++++ 3 files changed, 70 insertions(+), 3 deletions(-) diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 0436c8fcc79..d5db4cbb23f 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -115,12 +115,25 @@ class UploadsController < ApplicationController def show_secure # do not serve uploads requested via XHR to prevent XSS return xhr_not_allowed if request.xhr? + return render_404 if !Discourse.store.external? + + path_with_ext = "#{params[:path]}.#{params[:extension]}" + + sha1 = File.basename(path_with_ext, File.extname(path_with_ext)) + # this takes care of optimized image requests + sha1 = sha1.partition("_").first if sha1.include?("_") + + upload = Upload.find_by(sha1: sha1) + return render_404 if upload.blank? if SiteSetting.secure_media? - redirect_to Discourse.store.signed_url_for_path("#{params[:path]}.#{params[:extension]}") - else - render_404 + return redirect_to Discourse.store.signed_url_for_path(path_with_ext) end + + # we don't want to 404 here if secure media gets disabled + # because all posts with secure uploads will show broken media + # until rebaked, which could take some time + redirect_to Discourse.store.cdn_url(upload.url) end def metadata diff --git a/lib/tasks/uploads.rake b/lib/tasks/uploads.rake index 5cfb1b8785d..1bc6a61eddb 100644 --- a/lib/tasks/uploads.rake +++ b/lib/tasks/uploads.rake @@ -909,6 +909,43 @@ task "uploads:recover" => :environment do end end +task "uploads:disable_secure_media" => :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}...", "" + + SiteSetting.secure_media = false + + secure_uploads = Upload.includes(:posts).where(secure: true) + secure_upload_count = secure_uploads.count + + i = 0 + secure_uploads.find_each(batch_size: 20).each do |upload| + Upload.transaction do + upload.secure = false + + RakeHelpers.print_status_with_label("Updating ACL for upload #{upload.id}.......", i, secure_upload_count) + Discourse.store.update_upload_ACL(upload) + + RakeHelpers.print_status_with_label("Rebaking posts for upload #{upload.id}.......", i, secure_upload_count) + upload.posts.each(&:rebake!) + upload.save + + i += 1 + end + end + + RakeHelpers.print_status_with_label("Rebaking and updating complete! ", i, secure_upload_count) + puts "" + end + + puts "Secure media is now disabled!", "" +end + ## # Run this task whenever the secure_media or login_required # settings are changed for a Discourse instance to update diff --git a/spec/requests/uploads_controller_spec.rb b/spec/requests/uploads_controller_spec.rb index 88799306f52..8e88f75244f 100644 --- a/spec/requests/uploads_controller_spec.rb +++ b/spec/requests/uploads_controller_spec.rb @@ -431,6 +431,23 @@ describe UploadsController do result = JSON.parse(response.body) expect(result[0]["url"]).to match("secure-media-uploads") end + + context "when secure media is disabled" do + before do + SiteSetting.secure_media = false + end + + it "should redirect to the regular show route" do + secure_url = upload.url.sub(SiteSetting.Upload.absolute_base_url, "/secure-media-uploads") + sign_in(user) + stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/") + + get secure_url + + expect(response.status).to eq(302) + expect(response.redirect_url).to eq(Discourse.store.cdn_url(upload.url)) + end + end end end