diff --git a/app/assets/javascripts/discourse/app/components/uppy-backup-uploader.js b/app/assets/javascripts/discourse/app/components/uppy-backup-uploader.js index 5891106f904..55be0926b74 100644 --- a/app/assets/javascripts/discourse/app/components/uppy-backup-uploader.js +++ b/app/assets/javascripts/discourse/app/components/uppy-backup-uploader.js @@ -7,6 +7,7 @@ export default Component.extend(UppyUploadMixin, { tagName: "span", type: "backup", useMultipartUploadsIfAvailable: true, + uploadRootPath: "/admin/backups", @discourseComputed("uploading", "uploadProgress") uploadButtonText(uploading, progress) { diff --git a/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js b/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js index 5146583d401..b31587a7025 100644 --- a/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js +++ b/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js @@ -33,6 +33,7 @@ import bootbox from "bootbox"; // functionality and event binding. // export default Mixin.create(ExtendableUploader, UppyS3Multipart, { + uploadRootPath: "/uploads", uploadTargetBound: false, @observes("composerModel.uploadCancelled") diff --git a/app/assets/javascripts/discourse/app/mixins/uppy-s3-multipart.js b/app/assets/javascripts/discourse/app/mixins/uppy-s3-multipart.js index 62532342976..2c4d72bedd5 100644 --- a/app/assets/javascripts/discourse/app/mixins/uppy-s3-multipart.js +++ b/app/assets/javascripts/discourse/app/mixins/uppy-s3-multipart.js @@ -1,4 +1,5 @@ import Mixin from "@ember/object/mixin"; +import getUrl from "discourse-common/lib/get-url"; import { bind } from "discourse-common/utils/decorators"; import { Promise } from "rsvp"; import { ajax } from "discourse/lib/ajax"; @@ -50,7 +51,7 @@ export default Mixin.create({ data.metadata = { "sha1-checksum": file.meta.sha1_checksum }; } - return ajax("/uploads/create-multipart.json", { + return ajax(getUrl(`${this.uploadRootPath}/create-multipart.json`), { type: "POST", data, // uppy is inconsistent, an error here fires the upload-error event @@ -70,13 +71,16 @@ export default Mixin.create({ if (file.preparePartsRetryAttempts === undefined) { file.preparePartsRetryAttempts = 0; } - return ajax("/uploads/batch-presign-multipart-parts.json", { - type: "POST", - data: { - part_numbers: partData.partNumbers, - unique_identifier: file.meta.unique_identifier, - }, - }) + return ajax( + getUrl(`${this.uploadRootPath}/batch-presign-multipart-parts.json`), + { + type: "POST", + data: { + part_numbers: partData.partNumbers, + unique_identifier: file.meta.unique_identifier, + }, + } + ) .then((data) => { if (file.preparePartsRetryAttempts) { delete file.preparePartsRetryAttempts; @@ -122,7 +126,7 @@ export default Mixin.create({ const parts = data.parts.map((part) => { return { part_number: part.PartNumber, etag: part.ETag }; }); - return ajax("/uploads/complete-multipart.json", { + return ajax(getUrl(`${this.uploadRootPath}/complete-multipart.json`), { type: "POST", contentType: "application/json", data: JSON.stringify({ @@ -155,7 +159,7 @@ export default Mixin.create({ return; } - return ajax("/uploads/abort-multipart.json", { + return ajax(getUrl(`${this.uploadRootPath}/abort-multipart.json`), { type: "POST", data: { external_upload_identifier: uploadId, diff --git a/app/assets/javascripts/discourse/app/mixins/uppy-upload.js b/app/assets/javascripts/discourse/app/mixins/uppy-upload.js index cabcfc2ae42..f94ca0b40e9 100644 --- a/app/assets/javascripts/discourse/app/mixins/uppy-upload.js +++ b/app/assets/javascripts/discourse/app/mixins/uppy-upload.js @@ -27,6 +27,7 @@ export default Mixin.create(UppyS3Multipart, { autoStartUploads: true, _inProgressUploads: 0, id: null, + uploadRootPath: "/uploads", uploadDone() { warn("You should implement `uploadDone`", { @@ -223,7 +224,7 @@ export default Mixin.create(UppyS3Multipart, { data.metadata = { "sha1-checksum": file.meta.sha1_checksum }; } - return ajax(getUrl("/uploads/generate-presigned-put"), { + return ajax(getUrl(`${this.uploadRootPath}/generate-presigned-put`), { type: "POST", data, }) @@ -277,7 +278,7 @@ export default Mixin.create(UppyS3Multipart, { }, _completeExternalUpload(file) { - return ajax(getUrl("/uploads/complete-external-upload"), { + return ajax(getUrl(`${this.uploadRootPath}/complete-external-upload`), { type: "POST", data: deepMerge( { unique_identifier: file.meta.uniqueUploadIdentifier }, diff --git a/app/controllers/admin/backups_controller.rb b/app/controllers/admin/backups_controller.rb index 0ea55fe1505..188db1fdcc9 100644 --- a/app/controllers/admin/backups_controller.rb +++ b/app/controllers/admin/backups_controller.rb @@ -4,6 +4,8 @@ require "backup_restore" require "backup_restore/backup_store" class Admin::BackupsController < Admin::AdminController + include ExternalUploadHelpers + before_action :ensure_backups_enabled skip_before_action :check_xhr, only: [:index, :show, :logs, :check_backup_chunk, :upload_backup_chunk] @@ -234,4 +236,24 @@ class Admin::BackupsController < Admin::AdminController def render_error(message_key) render json: failed_json.merge(message: I18n.t(message_key)) end + + def validate_before_create_multipart(file_name:, file_size:, upload_type:) + raise ExternalUploadHelpers::ExternalUploadValidationError.new(I18n.t("backup.backup_file_should_be_tar_gz")) unless valid_extension?(file_name) + raise ExternalUploadHelpers::ExternalUploadValidationError.new(I18n.t("backup.invalid_filename")) unless valid_filename?(file_name) + end + + def self.serialize_upload(_upload) + {} # noop, the backup does not create an upload record + end + + def create_direct_multipart_upload + begin + yield + rescue BackupRestore::BackupStore::StorageError => err + message = debug_upload_error(err, I18n.t("upload.create_multipart_failure", additional_detail: err.message)) + raise ExternalUploadHelpers::ExternalUploadValidationError.new(message) + rescue BackupRestore::BackupStore::BackupFileExists + raise ExternalUploadHelpers::ExternalUploadValidationError.new(I18n.t("backup.file_exists")) + end + end end diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index c2ea7a175f1..f84a0ebd0a9 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -3,36 +3,17 @@ require "mini_mime" class UploadsController < ApplicationController + include ExternalUploadHelpers + requires_login except: [:show, :show_short, :show_secure] skip_before_action :preload_json, :check_xhr, :redirect_to_login_if_required, only: [:show, :show_short, :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, - :generate_presigned_put, - :complete_external_upload, - :create_multipart, - :batch_presign_multipart_parts, - :abort_multipart, - :complete_multipart - ] - before_action :direct_s3_uploads_check, only: [ - :generate_presigned_put, - :complete_external_upload, - :create_multipart, - :batch_presign_multipart_parts, - :abort_multipart, - :complete_multipart - ] - before_action :can_upload_external?, only: [:create_multipart, :generate_presigned_put] + before_action :external_store_check, only: [:show_secure] SECURE_REDIRECT_GRACE_SECONDS = 5 - PRESIGNED_PUT_RATE_LIMIT_PER_MINUTE = 10 - CREATE_MULTIPART_RATE_LIMIT_PER_MINUTE = 10 - COMPLETE_MULTIPART_RATE_LIMIT_PER_MINUTE = 10 - BATCH_PRESIGN_RATE_LIMIT_PER_MINUTE = 10 def create # capture current user for block later on @@ -208,285 +189,25 @@ class UploadsController < ApplicationController } end - def generate_presigned_put - RateLimiter.new( - current_user, "generate-presigned-put-upload-stub", PRESIGNED_PUT_RATE_LIMIT_PER_MINUTE, 1.minute - ).performed! - - file_name = params.require(:file_name) - file_size = params.require(:file_size).to_i - type = params.require(:type) - - if file_size_too_big?(file_name, file_size) - return render_json_error( - I18n.t("upload.attachments.too_large_humanized", max_size: ActiveSupport::NumberHelper.number_to_human_size(SiteSetting.max_attachment_size_kb.kilobytes)), - status: 422 - ) - end - - external_upload_data = ExternalUploadManager.create_direct_upload( - current_user: current_user, - file_name: file_name, - file_size: file_size, - upload_type: type, - metadata: parse_allowed_metadata(params[:metadata]) - ) - - render json: external_upload_data - end - - def complete_external_upload - unique_identifier = params.require(:unique_identifier) - external_upload_stub = ExternalUploadStub.find_by( - unique_identifier: unique_identifier, created_by: current_user - ) - return render_404 if external_upload_stub.blank? - - complete_external_upload_via_manager(external_upload_stub) - end - - def complete_external_upload_via_manager(external_upload_stub) - opts = { - for_private_message: params[:for_private_message]&.to_s == "true", - for_site_setting: params[:for_site_setting]&.to_s == "true", - pasted: params[:pasted]&.to_s == "true", - } - - external_upload_manager = ExternalUploadManager.new(external_upload_stub, opts) - hijack do - begin - upload = external_upload_manager.transform! - - if upload.errors.empty? - response_serialized = external_upload_stub.upload_type != "backup" ? UploadsController.serialize_upload(upload) : {} - external_upload_stub.destroy! - render json: response_serialized, status: 200 - else - render_json_error(upload.errors.to_hash.values.flatten, status: 422) - end - rescue ExternalUploadManager::SizeMismatchError => err - render_json_error( - debug_upload_error(err, "upload.size_mismatch_failure", additional_detail: err.message), - status: 422 - ) - rescue ExternalUploadManager::ChecksumMismatchError => err - render_json_error( - debug_upload_error(err, "upload.checksum_mismatch_failure", additional_detail: err.message), - status: 422 - ) - rescue ExternalUploadManager::CannotPromoteError => err - render_json_error( - debug_upload_error(err, "upload.cannot_promote_failure", additional_detail: err.message), - status: 422 - ) - rescue ExternalUploadManager::DownloadFailedError, Aws::S3::Errors::NotFound => err - render_json_error( - debug_upload_error(err, "upload.download_failure", additional_detail: err.message), - status: 422 - ) - rescue => err - Discourse.warn_exception( - err, message: "Complete external upload failed unexpectedly for user #{current_user.id}" - ) - - render_json_error(I18n.t("upload.failed"), status: 422) - end - end - end - - def create_multipart - RateLimiter.new( - current_user, "create-multipart-upload", CREATE_MULTIPART_RATE_LIMIT_PER_MINUTE, 1.minute - ).performed! - - file_name = params.require(:file_name) - file_size = params.require(:file_size).to_i - upload_type = params.require(:upload_type) - - if upload_type == "backup" - ensure_staff - return render_json_error(I18n.t("backup.backup_file_should_be_tar_gz")) unless valid_backup_extension?(file_name) - return render_json_error(I18n.t("backup.invalid_filename")) unless valid_backup_filename?(file_name) - else - if file_size_too_big?(file_name, file_size) - return render_json_error( - I18n.t("upload.attachments.too_large_humanized", max_size: ActiveSupport::NumberHelper.number_to_human_size(SiteSetting.max_attachment_size_kb.kilobytes)), - status: 422 - ) - end - end - - begin - external_upload_data = ExternalUploadManager.create_direct_multipart_upload( - current_user: current_user, - file_name: file_name, - file_size: file_size, - upload_type: upload_type, - metadata: parse_allowed_metadata(params[:metadata]) - ) - rescue Aws::S3::Errors::ServiceError => err - return render_json_error( - debug_upload_error(err, "upload.create_multipart_failure", additional_detail: err.message), - status: 422 - ) - rescue BackupRestore::BackupStore::BackupFileExists - return render_json_error(I18n.t("backup.file_exists"), status: 422) - rescue BackupRestore::BackupStore::StorageError => err - return render_json_error( - debug_upload_error(err, "upload.create_multipart_failure", additional_detail: err.message), - status: 422 - ) - end - - render json: external_upload_data - end - - def batch_presign_multipart_parts - part_numbers = params.require(:part_numbers) - unique_identifier = params.require(:unique_identifier) - - RateLimiter.new( - current_user, "batch-presign", BATCH_PRESIGN_RATE_LIMIT_PER_MINUTE, 1.minute - ).performed! - - part_numbers = part_numbers.map do |part_number| - validate_part_number(part_number) - end - - external_upload_stub = ExternalUploadStub.find_by( - unique_identifier: unique_identifier, created_by: current_user - ) - return render_404 if external_upload_stub.blank? - - if !multipart_upload_exists?(external_upload_stub) - return render_404 - end - - store = multipart_store(external_upload_stub.upload_type) - - presigned_urls = {} - part_numbers.each do |part_number| - presigned_urls[part_number] = store.presign_multipart_part( - upload_id: external_upload_stub.external_upload_identifier, - key: external_upload_stub.key, - part_number: part_number - ) - end - - render json: { presigned_urls: presigned_urls } - end - - def validate_part_number(part_number) - part_number = part_number.to_i - if !part_number.between?(1, 10000) - raise Discourse::InvalidParameters.new( - "Each part number should be between 1 and 10000" - ) - end - part_number - end - - def multipart_upload_exists?(external_upload_stub) - store = multipart_store(external_upload_stub.upload_type) - begin - store.list_multipart_parts( - upload_id: external_upload_stub.external_upload_identifier, - key: external_upload_stub.key, - max_parts: 1 - ) - rescue Aws::S3::Errors::NoSuchUpload => err - debug_upload_error(err, "upload.external_upload_not_found", { additional_detail: "path: #{external_upload_stub.key}" }) - return false - end - true - end - - def abort_multipart - external_upload_identifier = params.require(:external_upload_identifier) - external_upload_stub = ExternalUploadStub.find_by( - external_upload_identifier: external_upload_identifier - ) - - # The stub could have already been deleted by an earlier error via - # ExternalUploadManager, so we consider this a great success if the - # stub is already gone. - return render json: success_json if external_upload_stub.blank? - - return render_404 if external_upload_stub.created_by_id != current_user.id - store = multipart_store(external_upload_stub.upload_type) - - begin - store.abort_multipart( - upload_id: external_upload_stub.external_upload_identifier, - key: external_upload_stub.key - ) - rescue Aws::S3::Errors::ServiceError => err - return render_json_error( - debug_upload_error(err, "upload.abort_multipart_failure", additional_detail: "external upload stub id: #{external_upload_stub.id}"), - status: 422 - ) - end - - external_upload_stub.destroy! - - render json: success_json - end - - def complete_multipart - unique_identifier = params.require(:unique_identifier) - parts = params.require(:parts) - - RateLimiter.new( - current_user, "complete-multipart-upload", COMPLETE_MULTIPART_RATE_LIMIT_PER_MINUTE, 1.minute - ).performed! - - external_upload_stub = ExternalUploadStub.find_by( - unique_identifier: unique_identifier, created_by: current_user - ) - return render_404 if external_upload_stub.blank? - - if !multipart_upload_exists?(external_upload_stub) - return render_404 - end - - store = multipart_store(external_upload_stub.upload_type) - parts = parts.map do |part| - part_number = part[:part_number] - etag = part[:etag] - part_number = validate_part_number(part_number) - - if etag.blank? - raise Discourse::InvalidParameters.new("All parts must have an etag and a valid part number") - end - - # this is done so it's an array of hashes rather than an array of - # ActionController::Parameters - { part_number: part_number, etag: etag } - end.sort_by do |part| - part[:part_number] - end - - begin - complete_response = store.complete_multipart( - upload_id: external_upload_stub.external_upload_identifier, - key: external_upload_stub.key, - parts: parts - ) - rescue Aws::S3::Errors::ServiceError => err - return render_json_error( - debug_upload_error(err, "upload.complete_multipart_failure", additional_detail: "external upload stub id: #{external_upload_stub.id}"), - status: 422 - ) - end - - complete_external_upload_via_manager(external_upload_stub) - end - protected - def multipart_store(upload_type) - ensure_staff if upload_type == "backup" - ExternalUploadManager.store_for_upload_type(upload_type) + def validate_before_create_multipart(file_name:, file_size:, upload_type:) + validate_file_size(file_name: file_name, file_size: file_size) + end + + def validate_before_create_direct_upload(file_name:, file_size:, upload_type:) + validate_file_size(file_name: file_name, file_size: file_size) + end + + def validate_file_size(file_name:, file_size:) + if file_size_too_big?(file_name, file_size) + raise ExternalUploadValidationError.new( + I18n.t( + "upload.attachments.too_large_humanized", + max_size: ActiveSupport::NumberHelper.number_to_human_size(SiteSetting.max_attachment_size_kb.kilobytes) + ) + ) + end end def force_download? @@ -497,10 +218,6 @@ class UploadsController < ApplicationController raise Discourse::InvalidParameters.new("XHR not allowed") end - def render_404 - raise Discourse::NotFound - end - def self.serialize_upload(data) # as_json.as_json is not a typo... as_json in AM serializer returns keys as symbols, we need them # as strings here @@ -556,18 +273,6 @@ class UploadsController < ApplicationController private - def external_store_check - return render_404 if !Discourse.store.external? - end - - def direct_s3_uploads_check - return render_404 if !SiteSetting.enable_direct_s3_uploads - end - - def can_upload_external? - raise Discourse::InvalidAccess if !guardian.can_upload_external? - end - # We can pre-emptively check size for attachments, but not for images # as they may be further reduced in size by UploadCreator (at this point # they may have already been reduced in size by preprocessors) @@ -593,29 +298,12 @@ class UploadsController < ApplicationController send_file(file_path, opts) end - def debug_upload_error(err, translation_key, translation_params = {}) - return if !SiteSetting.enable_upload_debug_mode - message = I18n.t(translation_key, translation_params) - Discourse.warn_exception(err, message: message) - Rails.env.development? ? message : I18n.t("upload.failed") - end - - # don't want people posting arbitrary S3 metadata so we just take the - # one we need. all of these will be converted to x-amz-meta- metadata - # fields in S3 so it's best to use dashes in the names for consistency - # - # this metadata is baked into the presigned url and is not altered when - # sending the PUT from the clientside to the presigned url - def parse_allowed_metadata(metadata) - return if metadata.blank? - metadata.permit("sha1-checksum").to_h - end - - def valid_backup_extension?(filename) - /\.(tar\.gz|t?gz)$/i =~ filename - end - - def valid_backup_filename?(filename) - !!(/^[a-zA-Z0-9\._\-]+$/ =~ filename) + def create_direct_multipart_upload + begin + yield + rescue Aws::S3::Errors::ServiceError => err + message = debug_upload_error(err, I18n.t("upload.create_multipart_failure", additional_detail: err.message)) + raise ExternalUploadHelpers::ExternalUploadValidationError.new(message) + end end end diff --git a/config/routes.rb b/config/routes.rb index 1c52c52961a..943a773fff5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -304,6 +304,12 @@ Discourse::Application.routes.draw do post "restore" => "backups#restore", constraints: { id: RouteFormat.backup } end collection do + # multipart uploads + post "create-multipart" => "backups#create_multipart", format: :json + post "complete-multipart" => "backups#complete_multipart", format: :json + post "abort-multipart" => "backups#abort_multipart", format: :json + post "batch-presign-multipart-parts" => "backups#batch_presign_multipart_parts", format: :json + get "logs" => "backups#logs" get "status" => "backups#status" delete "cancel" => "backups#cancel" diff --git a/lib/external_upload_helpers.rb b/lib/external_upload_helpers.rb new file mode 100644 index 00000000000..ff187e5de38 --- /dev/null +++ b/lib/external_upload_helpers.rb @@ -0,0 +1,347 @@ +# frozen_string_literal: true + +# Extends controllers with the methods required to do direct +# external uploads. +module ExternalUploadHelpers + extend ActiveSupport::Concern + + class ExternalUploadValidationError < StandardError; end + + PRESIGNED_PUT_RATE_LIMIT_PER_MINUTE = 10 + CREATE_MULTIPART_RATE_LIMIT_PER_MINUTE = 10 + COMPLETE_MULTIPART_RATE_LIMIT_PER_MINUTE = 10 + BATCH_PRESIGN_RATE_LIMIT_PER_MINUTE = 10 + + included do + before_action :external_store_check, only: [ + :generate_presigned_put, + :complete_external_upload, + :create_multipart, + :batch_presign_multipart_parts, + :abort_multipart, + :complete_multipart + ] + before_action :direct_s3_uploads_check, only: [ + :generate_presigned_put, + :complete_external_upload, + :create_multipart, + :batch_presign_multipart_parts, + :abort_multipart, + :complete_multipart + ] + before_action :can_upload_external?, only: [:create_multipart, :generate_presigned_put] + end + + def generate_presigned_put + RateLimiter.new( + current_user, "generate-presigned-put-upload-stub", ExternalUploadHelpers::PRESIGNED_PUT_RATE_LIMIT_PER_MINUTE, 1.minute + ).performed! + + file_name = params.require(:file_name) + file_size = params.require(:file_size).to_i + type = params.require(:type) + + begin + validate_before_create_direct_upload( + file_name: file_name, + file_size: file_size, + upload_type: type + ) + rescue ExternalUploadValidationError => err + return render_json_error(err.message, status: 422) + end + + external_upload_data = ExternalUploadManager.create_direct_upload( + current_user: current_user, + file_name: file_name, + file_size: file_size, + upload_type: type, + metadata: parse_allowed_metadata(params[:metadata]) + ) + + render json: external_upload_data + end + + def complete_external_upload + unique_identifier = params.require(:unique_identifier) + external_upload_stub = ExternalUploadStub.find_by( + unique_identifier: unique_identifier, created_by: current_user + ) + return render_404 if external_upload_stub.blank? + + complete_external_upload_via_manager(external_upload_stub) + end + + def create_multipart + RateLimiter.new( + current_user, "create-multipart-upload", ExternalUploadHelpers::CREATE_MULTIPART_RATE_LIMIT_PER_MINUTE, 1.minute + ).performed! + + file_name = params.require(:file_name) + file_size = params.require(:file_size).to_i + upload_type = params.require(:upload_type) + + begin + validate_before_create_multipart( + file_name: file_name, + file_size: file_size, + upload_type: upload_type + ) + rescue ExternalUploadValidationError => err + return render_json_error(err.message, status: 422) + end + + begin + external_upload_data = create_direct_multipart_upload do + ExternalUploadManager.create_direct_multipart_upload( + current_user: current_user, + file_name: file_name, + file_size: file_size, + upload_type: upload_type, + metadata: parse_allowed_metadata(params[:metadata]) + ) + end + rescue ExternalUploadHelpers::ExternalUploadValidationError => err + return render_json_error(err.message, status: 422) + end + + render json: external_upload_data + end + + def batch_presign_multipart_parts + part_numbers = params.require(:part_numbers) + unique_identifier = params.require(:unique_identifier) + + RateLimiter.new( + current_user, "batch-presign", ExternalUploadHelpers::BATCH_PRESIGN_RATE_LIMIT_PER_MINUTE, 1.minute + ).performed! + + part_numbers = part_numbers.map do |part_number| + validate_part_number(part_number) + end + + external_upload_stub = ExternalUploadStub.find_by( + unique_identifier: unique_identifier, created_by: current_user + ) + return render_404 if external_upload_stub.blank? + + if !multipart_upload_exists?(external_upload_stub) + return render_404 + end + + store = multipart_store(external_upload_stub.upload_type) + + presigned_urls = {} + part_numbers.each do |part_number| + presigned_urls[part_number] = store.presign_multipart_part( + upload_id: external_upload_stub.external_upload_identifier, + key: external_upload_stub.key, + part_number: part_number + ) + end + + render json: { presigned_urls: presigned_urls } + end + + def multipart_upload_exists?(external_upload_stub) + store = multipart_store(external_upload_stub.upload_type) + begin + store.list_multipart_parts( + upload_id: external_upload_stub.external_upload_identifier, + key: external_upload_stub.key, + max_parts: 1 + ) + rescue Aws::S3::Errors::NoSuchUpload => err + debug_upload_error(err, I18n.t("upload.external_upload_not_found", additional_detail: "path: #{external_upload_stub.key}")) + return false + end + true + end + + def abort_multipart + external_upload_identifier = params.require(:external_upload_identifier) + external_upload_stub = ExternalUploadStub.find_by( + external_upload_identifier: external_upload_identifier + ) + + # The stub could have already been deleted by an earlier error via + # ExternalUploadManager, so we consider this a great success if the + # stub is already gone. + return render json: success_json if external_upload_stub.blank? + + return render_404 if external_upload_stub.created_by_id != current_user.id + store = multipart_store(external_upload_stub.upload_type) + + begin + store.abort_multipart( + upload_id: external_upload_stub.external_upload_identifier, + key: external_upload_stub.key + ) + rescue Aws::S3::Errors::ServiceError => err + return render_json_error( + debug_upload_error(err, I18n.t("upload.abort_multipart_failure", additional_detail: "external upload stub id: #{external_upload_stub.id}")), + status: 422 + ) + end + + external_upload_stub.destroy! + + render json: success_json + end + + def complete_multipart + unique_identifier = params.require(:unique_identifier) + parts = params.require(:parts) + + RateLimiter.new( + current_user, "complete-multipart-upload", ExternalUploadHelpers::COMPLETE_MULTIPART_RATE_LIMIT_PER_MINUTE, 1.minute + ).performed! + + external_upload_stub = ExternalUploadStub.find_by( + unique_identifier: unique_identifier, created_by: current_user + ) + return render_404 if external_upload_stub.blank? + + if !multipart_upload_exists?(external_upload_stub) + return render_404 + end + + store = multipart_store(external_upload_stub.upload_type) + parts = parts.map do |part| + part_number = part[:part_number] + etag = part[:etag] + part_number = validate_part_number(part_number) + + if etag.blank? + raise Discourse::InvalidParameters.new("All parts must have an etag and a valid part number") + end + + # this is done so it's an array of hashes rather than an array of + # ActionController::Parameters + { part_number: part_number, etag: etag } + end.sort_by do |part| + part[:part_number] + end + + begin + complete_response = store.complete_multipart( + upload_id: external_upload_stub.external_upload_identifier, + key: external_upload_stub.key, + parts: parts + ) + rescue Aws::S3::Errors::ServiceError => err + return render_json_error( + debug_upload_error(err, I18n.t("upload.complete_multipart_failure", additional_detail: "external upload stub id: #{external_upload_stub.id}")), + status: 422 + ) + end + + complete_external_upload_via_manager(external_upload_stub) + end + + private + + def complete_external_upload_via_manager(external_upload_stub) + opts = { + for_private_message: params[:for_private_message]&.to_s == "true", + for_site_setting: params[:for_site_setting]&.to_s == "true", + pasted: params[:pasted]&.to_s == "true", + } + + external_upload_manager = ExternalUploadManager.new(external_upload_stub, opts) + hijack do + begin + upload = external_upload_manager.transform! + + if upload.errors.empty? + response_serialized = self.class.serialize_upload(upload) + external_upload_stub.destroy! + render json: response_serialized, status: 200 + else + render_json_error(upload.errors.to_hash.values.flatten, status: 422) + end + rescue ExternalUploadManager::SizeMismatchError => err + render_json_error( + debug_upload_error(err, I18n.t("upload.size_mismatch_failure", additional_detail: err.message)), + status: 422 + ) + rescue ExternalUploadManager::ChecksumMismatchError => err + render_json_error( + debug_upload_error(err, I18n.t("upload.checksum_mismatch_failure", additional_detail: err.message)), + status: 422 + ) + rescue ExternalUploadManager::CannotPromoteError => err + render_json_error( + debug_upload_error(err, I18n.t("upload.cannot_promote_failure", additional_detail: err.message)), + status: 422 + ) + rescue ExternalUploadManager::DownloadFailedError, Aws::S3::Errors::NotFound => err + render_json_error( + debug_upload_error(err, I18n.t("upload.download_failure", additional_detail: err.message)), + status: 422 + ) + rescue => err + Discourse.warn_exception( + err, message: "Complete external upload failed unexpectedly for user #{current_user.id}" + ) + + render_json_error(I18n.t("upload.failed"), status: 422) + end + end + end + + def validate_before_create_direct_upload(file_name:, file_size:, upload_type:) + # noop, should be overridden + end + + def validate_before_create_multipart(file_name:, file_size:, upload_type:) + # noop, should be overridden + end + + def validate_part_number(part_number) + part_number = part_number.to_i + if !part_number.between?(1, 10000) + raise Discourse::InvalidParameters.new( + "Each part number should be between 1 and 10000" + ) + end + part_number + end + + def debug_upload_error(err, friendly_message) + return if !SiteSetting.enable_upload_debug_mode + Discourse.warn_exception(err, message: friendly_message) + Rails.env.development? ? friendly_message : I18n.t("upload.failed") + end + + def multipart_store(upload_type) + ExternalUploadManager.store_for_upload_type(upload_type) + end + + def external_store_check + return render_404 if !Discourse.store.external? + end + + def direct_s3_uploads_check + return render_404 if !SiteSetting.enable_direct_s3_uploads + end + + def can_upload_external? + raise Discourse::InvalidAccess if !guardian.can_upload_external? + end + + # don't want people posting arbitrary S3 metadata so we just take the + # one we need. all of these will be converted to x-amz-meta- metadata + # fields in S3 so it's best to use dashes in the names for consistency + # + # this metadata is baked into the presigned url and is not altered when + # sending the PUT from the clientside to the presigned url + def parse_allowed_metadata(metadata) + return if metadata.blank? + metadata.permit("sha1-checksum").to_h + end + + def render_404 + raise Discourse::NotFound + end +end diff --git a/spec/requests/admin/backups_controller_spec.rb b/spec/requests/admin/backups_controller_spec.rb index fa72233a591..6e3c06ec492 100644 --- a/spec/requests/admin/backups_controller_spec.rb +++ b/spec/requests/admin/backups_controller_spec.rb @@ -416,4 +416,120 @@ RSpec.describe Admin::BackupsController do expect(response).to be_not_found end end + + describe "S3 multipart uploads" do + let(:upload_type) { "backup" } + let(:test_bucket_prefix) { "test_#{ENV['TEST_ENV_NUMBER'].presence || '0'}" } + let(:backup_file_exists_response) { { status: 404 } } + let(:mock_multipart_upload_id) { "ibZBv_75gd9r8lH_gqXatLdxMVpAlj6CFTR.OwyF3953YdwbcQnMA2BLGn8Lx12fQNICtMw5KyteFeHw.Sjng--" } + + before do + setup_s3 + SiteSetting.enable_direct_s3_uploads = true + SiteSetting.s3_backup_bucket = "s3-backup-bucket" + SiteSetting.backup_location = BackupLocationSiteSetting::S3 + stub_request(:head, "https://s3-backup-bucket.s3.us-west-1.amazonaws.com/").to_return(status: 200, body: "", headers: {}) + stub_request(:head, "https://s3-backup-bucket.s3.us-west-1.amazonaws.com/default/test.tar.gz").to_return( + backup_file_exists_response + ) + end + + context "when the user is not admin" do + before do + admin.update(admin: false) + end + + it "errors with invalid access error" do + post "/admin/backups/create-multipart.json", params: { + file_name: "test.tar.gz", + upload_type: upload_type, + file_size: 4098 + } + expect(response.status).to eq(404) + end + end + + context "when the user is admin" do + def stub_create_multipart_backup_request + BackupRestore::S3BackupStore.any_instance.stubs(:temporary_upload_path).returns( + "temp/default/#{test_bucket_prefix}/28fccf8259bbe75b873a2bd2564b778c/2u98j832nx93272x947823.gz" + ) + create_multipart_result = <<~BODY + \n + + s3-backup-bucket + temp/default/#{test_bucket_prefix}/28fccf8259bbe75b873a2bd2564b778c/2u98j832nx93272x947823.gz + #{mock_multipart_upload_id} + + BODY + stub_request(:post, "https://s3-backup-bucket.s3.us-west-1.amazonaws.com/temp/default/#{test_bucket_prefix}/28fccf8259bbe75b873a2bd2564b778c/2u98j832nx93272x947823.gz?uploads"). + to_return(status: 200, body: create_multipart_result) + end + + it "creates the multipart upload" do + stub_create_multipart_backup_request + post "/admin/backups/create-multipart.json", params: { + file_name: "test.tar.gz", + upload_type: upload_type, + file_size: 4098 + } + expect(response.status).to eq(200) + result = response.parsed_body + + external_upload_stub = ExternalUploadStub.where( + unique_identifier: result["unique_identifier"], + original_filename: "test.tar.gz", + created_by: admin, + upload_type: upload_type, + key: result["key"], + multipart: true + ) + expect(external_upload_stub.exists?).to eq(true) + end + + context "when backup of same filename already exists" do + let(:backup_file_exists_response) { { status: 200, body: "" } } + + it "throws an error" do + post "/admin/backups/create-multipart.json", params: { + file_name: "test.tar.gz", + upload_type: upload_type, + file_size: 4098 + } + expect(response.status).to eq(422) + expect(response.parsed_body["errors"]).to include( + I18n.t("backup.file_exists") + ) + end + end + + context "when filename is invalid" do + it "throws an error" do + post "/admin/backups/create-multipart.json", params: { + file_name: "blah $$##.tar.gz", + upload_type: upload_type, + file_size: 4098 + } + expect(response.status).to eq(422) + expect(response.parsed_body["errors"]).to include( + I18n.t("backup.invalid_filename") + ) + end + end + + context "when extension is invalid" do + it "throws an error" do + post "/admin/backups/create-multipart.json", params: { + file_name: "test.png", + upload_type: upload_type, + file_size: 4098 + } + expect(response.status).to eq(422) + expect(response.parsed_body["errors"]).to include( + I18n.t("backup.backup_file_should_be_tar_gz") + ) + end + end + end + end end diff --git a/spec/requests/uploads_controller_spec.rb b/spec/requests/uploads_controller_spec.rb index 85d7df04da2..4043c018c81 100644 --- a/spec/requests/uploads_controller_spec.rb +++ b/spec/requests/uploads_controller_spec.rb @@ -764,7 +764,7 @@ describe UploadsController do RateLimiter.enable RateLimiter.clear_all! - stub_const(UploadsController, "PRESIGNED_PUT_RATE_LIMIT_PER_MINUTE", 1) do + stub_const(ExternalUploadHelpers, "PRESIGNED_PUT_RATE_LIMIT_PER_MINUTE", 1) do post "/uploads/generate-presigned-put.json", params: { file_name: "test.png", type: "card_background", file_size: 1024 } post "/uploads/generate-presigned-put.json", params: { file_name: "test.png", type: "card_background", file_size: 1024 } end @@ -793,8 +793,6 @@ describe UploadsController do sign_in(user) SiteSetting.enable_direct_s3_uploads = true setup_s3 - SiteSetting.s3_backup_bucket = "s3-backup-bucket" - SiteSetting.backup_location = BackupLocationSiteSetting::S3 end it "errors if the correct params are not provided" do @@ -902,7 +900,7 @@ describe UploadsController do RateLimiter.clear_all! stub_create_multipart_request - stub_const(UploadsController, "CREATE_MULTIPART_RATE_LIMIT_PER_MINUTE", 1) do + stub_const(ExternalUploadHelpers, "CREATE_MULTIPART_RATE_LIMIT_PER_MINUTE", 1) do post "/uploads/create-multipart.json", params: { file_name: "test.png", upload_type: "composer", @@ -918,88 +916,6 @@ describe UploadsController do expect(response.status).to eq(429) end end - - context "when the upload_type is backup" do - let(:upload_type) { "backup" } - let(:backup_file_exists_response) { { status: 404 } } - - before do - stub_request(:head, "https://s3-backup-bucket.s3.us-west-1.amazonaws.com/").to_return(status: 200, body: "", headers: {}) - stub_request(:head, "https://s3-backup-bucket.s3.us-west-1.amazonaws.com/default/test.tar.gz").to_return( - backup_file_exists_response - ) - end - - context "when the user is not admin" do - it "errors with invalid access error" do - post "/uploads/create-multipart.json", params: { - file_name: "test.tar.gz", - upload_type: upload_type, - file_size: 4098 - } - expect(response.status).to eq(403) - end - end - - context "when the user is admin" do - before do - user.update(admin: true) - end - - def stub_create_multipart_backup_request - BackupRestore::S3BackupStore.any_instance.stubs(:temporary_upload_path).returns( - "temp/default/#{test_bucket_prefix}/28fccf8259bbe75b873a2bd2564b778c/2u98j832nx93272x947823.gz" - ) - create_multipart_result = <<~BODY - \n - - s3-backup-bucket - temp/default/#{test_bucket_prefix}/28fccf8259bbe75b873a2bd2564b778c/2u98j832nx93272x947823.gz - #{mock_multipart_upload_id} - - BODY - stub_request(:post, "https://s3-backup-bucket.s3.us-west-1.amazonaws.com/temp/default/#{test_bucket_prefix}/28fccf8259bbe75b873a2bd2564b778c/2u98j832nx93272x947823.gz?uploads"). - to_return(status: 200, body: create_multipart_result) - end - - it "creates the multipart upload" do - stub_create_multipart_backup_request - post "/uploads/create-multipart.json", params: { - file_name: "test.tar.gz", - upload_type: upload_type, - file_size: 4098 - } - expect(response.status).to eq(200) - result = response.parsed_body - - external_upload_stub = ExternalUploadStub.where( - unique_identifier: result["unique_identifier"], - original_filename: "test.tar.gz", - created_by: user, - upload_type: upload_type, - key: result["key"], - multipart: true - ) - expect(external_upload_stub.exists?).to eq(true) - end - - context "when backup of same filename already exists" do - let(:backup_file_exists_response) { { status: 200, body: "" } } - - it "throws an error" do - post "/uploads/create-multipart.json", params: { - file_name: "test.tar.gz", - upload_type: upload_type, - file_size: 4098 - } - expect(response.status).to eq(422) - expect(response.parsed_body["errors"]).to include( - I18n.t("backup.file_exists") - ) - end - end - end - end end context "when the store is not external" do @@ -1127,7 +1043,7 @@ describe UploadsController do RateLimiter.enable RateLimiter.clear_all! - stub_const(UploadsController, "BATCH_PRESIGN_RATE_LIMIT_PER_MINUTE", 1) do + stub_const(ExternalUploadHelpers, "BATCH_PRESIGN_RATE_LIMIT_PER_MINUTE", 1) do stub_list_multipart_request post "/uploads/batch-presign-multipart-parts.json", params: { unique_identifier: external_upload_stub.unique_identifier, @@ -1316,7 +1232,7 @@ describe UploadsController do RateLimiter.enable RateLimiter.clear_all! - stub_const(UploadsController, "COMPLETE_MULTIPART_RATE_LIMIT_PER_MINUTE", 1) do + stub_const(ExternalUploadHelpers, "COMPLETE_MULTIPART_RATE_LIMIT_PER_MINUTE", 1) do post "/uploads/complete-multipart.json", params: { unique_identifier: "blah", parts: [{ part_number: 1, etag: "test1" }, { part_number: 2, etag: "test2" }]