diff --git a/app/controllers/admin/backups_controller.rb b/app/controllers/admin/backups_controller.rb index 99bc0ca2d35..7a86b2ef457 100644 --- a/app/controllers/admin/backups_controller.rb +++ b/app/controllers/admin/backups_controller.rb @@ -34,6 +34,14 @@ class Admin::BackupsController < Admin::AdminController end def create + RateLimiter.new( + current_user, + "max-backups-per-minute", + 1, + 1.minute, + apply_limit_to_staff: true, + ).performed! + opts = { publish_to_message_bus: true, with_uploads: params.fetch(:with_uploads) == "true", diff --git a/lib/backup_restore/backuper.rb b/lib/backup_restore/backuper.rb index ef8d019886d..a0b6c923afa 100644 --- a/lib/backup_restore/backuper.rb +++ b/lib/backup_restore/backuper.rb @@ -5,7 +5,7 @@ require "file_store/s3_store" module BackupRestore class Backuper - attr_reader :success + attr_reader :success, :store def initialize(user_id, opts = {}) @user_id = user_id @@ -46,7 +46,6 @@ module BackupRestore rescue Exception => ex log "EXCEPTION: " + ex.message log ex.backtrace.join("\n") - @success = false else @success = true @backup_filename @@ -55,7 +54,7 @@ module BackupRestore clean_up notify_user log "Finished!" - publish_completion(@success) + publish_completion end protected @@ -337,12 +336,12 @@ module BackupRestore end def upload_archive - return unless @store.remote? + return unless store.remote? log "Uploading archive..." content_type = MiniMime.lookup_by_filename(@backup_filename).content_type archive_path = File.join(@archive_directory, @backup_filename) - @store.upload_file(@backup_filename, archive_path, content_type) + store.upload_file(@backup_filename, archive_path, content_type) end def after_create_hook @@ -354,16 +353,16 @@ module BackupRestore return if Rails.env.development? log "Deleting old backups..." - @store.delete_old + store.delete_old rescue => ex log "Something went wrong while deleting old backups.", ex end def notify_user - return if @success && @user.id == Discourse::SYSTEM_USER_ID + return if success && @user.id == Discourse::SYSTEM_USER_ID log "Notifying '#{@user.username}' of the end of the backup..." - status = @success ? :backup_succeeded : :backup_failed + status = success ? :backup_succeeded : :backup_failed logs = Discourse::Utils.logs_markdown(@logs, user: @user) post = SystemMessage.create_from_system_user(@user, status, logs: logs) @@ -378,11 +377,11 @@ module BackupRestore delete_uploaded_archive remove_tar_leftovers mark_backup_as_not_running - refresh_disk_space + refresh_disk_space if success end def delete_uploaded_archive - return unless @store.remote? + return unless store.remote? archive_path = File.join(@archive_directory, @backup_filename) @@ -396,7 +395,7 @@ module BackupRestore def refresh_disk_space log "Refreshing disk stats..." - @store.reset_cache + store.reset_cache rescue => ex log "Something went wrong while refreshing disk stats.", ex end @@ -450,7 +449,7 @@ module BackupRestore @logs << "[#{timestamp}] #{message}" end - def publish_completion(success) + def publish_completion if success log("[SUCCESS]") DiscourseEvent.trigger(:backup_complete, logs: @logs, ticket: @ticket) diff --git a/spec/lib/backup_restore/backuper_spec.rb b/spec/lib/backup_restore/backuper_spec.rb index 1578a44f446..258eedfec42 100644 --- a/spec/lib/backup_restore/backuper_spec.rb +++ b/spec/lib/backup_restore/backuper_spec.rb @@ -1,18 +1,20 @@ # frozen_string_literal: true RSpec.describe BackupRestore::Backuper do - it "returns a non-empty parameterized title when site title contains unicode" do - SiteSetting.title = "Ɣ" - backuper = BackupRestore::Backuper.new(Discourse.system_user.id) + describe "#get_parameterized_title" do + it "returns a non-empty parameterized title when site title contains unicode" do + SiteSetting.title = "Ɣ" + backuper = BackupRestore::Backuper.new(Discourse.system_user.id) - expect(backuper.send(:get_parameterized_title)).to eq("discourse") - end + expect(backuper.send(:get_parameterized_title)).to eq("discourse") + end - it "returns a valid parameterized site title" do - SiteSetting.title = "Coding Horror" - backuper = BackupRestore::Backuper.new(Discourse.system_user.id) + it "returns a valid parameterized site title" do + SiteSetting.title = "Coding Horror" + backuper = BackupRestore::Backuper.new(Discourse.system_user.id) - expect(backuper.send(:get_parameterized_title)).to eq("coding-horror") + expect(backuper.send(:get_parameterized_title)).to eq("coding-horror") + end end describe "#notify_user" do @@ -69,4 +71,32 @@ RSpec.describe BackupRestore::Backuper do ) end end + + describe "#run" do + subject(:run) { backup.run } + + let(:backup) { described_class.new(user.id) } + let(:user) { Discourse.system_user } + let(:store) { backup.store } + + before { backup.stubs(:success).returns(success) } + + context "when the result isn't successful" do + let(:success) { false } + + it "doesn't refresh disk stats" do + store.expects(:reset_cache).never + run + end + end + + context "when the result is successful" do + let(:success) { true } + + it "refreshes disk stats" do + store.expects(:reset_cache) + run + end + end + end end diff --git a/spec/requests/admin/backups_controller_spec.rb b/spec/requests/admin/backups_controller_spec.rb index b52278832c0..6bb5519c5f0 100644 --- a/spec/requests/admin/backups_controller_spec.rb +++ b/spec/requests/admin/backups_controller_spec.rb @@ -137,7 +137,10 @@ RSpec.describe Admin::BackupsController do describe "#create" do context "when logged in as an admin" do - before { sign_in(admin) } + before do + sign_in(admin) + BackupRestore.stubs(:backup!) + end it "starts a backup" do BackupRestore.expects(:backup!).with( @@ -149,6 +152,22 @@ RSpec.describe Admin::BackupsController do expect(response.status).to eq(200) end + + context "with rate limiting enabled" do + before do + RateLimiter.clear_all! + RateLimiter.enable + end + + after { RateLimiter.disable } + + it "is rate limited" do + post "/admin/backups.json", params: { with_uploads: false, client_id: "foo" } + post "/admin/backups.json", params: { with_uploads: false, client_id: "foo" } + + expect(response).to have_http_status :too_many_requests + end + end end shared_examples "backups creation not allowed" do