SECURITY: Don't pass email backup token to sidekiq as a parameter.

* This exposes the token in the Sidekiq dashboard which can be
  viewed by an admin and defeats the purpose of using a token
  in the download backup email ink.

app/controllers/admin/backups_controller.rb

@@ -1,5 +1,4 @@
 require "backup_restore/backup_restore"
-require "email_backup_token"
 
 class Admin::BackupsController < Admin::AdminController
 
@@ -47,10 +46,12 @@ class Admin::BackupsController < Admin::AdminController
 
   def email
     if backup = Backup[params.fetch(:id)]
-      token = EmailBackupToken.set(current_user.id)
-      download_url = "#{url_for(controller: 'backups', action: 'show')}?token=#{token}"
-      Jobs.enqueue(:download_backup_email, to_address: current_user.email, backup_file_path: download_url)
-      render nothing: true
+      Jobs.enqueue(:download_backup_email,
+        user_id: current_user.id,
+        backup_file_path: url_for(controller: 'backups', action: 'show')
+      )
+
+      render body: nil
     else
       render nothing: true, status: 404
     end

app/jobs/regular/download_backup_email.rb

@@ -1,4 +1,5 @@
 require_dependency 'email/sender'
+require_dependency "email_backup_token"
 
 module Jobs
 
@@ -7,13 +8,17 @@ module Jobs
     sidekiq_options queue: 'critical'
 
     def execute(args)
-      to_address = args[:to_address]
-      backup_file_path = args[:backup_file_path]
+      user_id = args[:user_id]
+      user = User.find_by(id: user_id)
+      raise Discourse::InvalidParameters.new(:user_id) unless user
 
-      raise Discourse::InvalidParameters.new(:to_address) if to_address.blank?
+      backup_file_path = args[:backup_file_path]
       raise Discourse::InvalidParameters.new(:backup_file_path) if backup_file_path.blank?
 
-      message = DownloadBackupMailer.send_email(to_address, backup_file_path)
+      backup_file_path = URI(backup_file_path)
+      backup_file_path.query = URI.encode_www_form(token: EmailBackupToken.set(user.id))
+
+      message = DownloadBackupMailer.send_email(user.email, backup_file_path.to_s)
       Email::Sender.new(message, :download_backup_message).send
     end
 

spec/controllers/admin/backups_controller_spec.rb

@@ -132,7 +132,11 @@ describe Admin::BackupsController do
 
       it "enqueues email job" do
         Backup.expects(:[]).with(backup_filename).returns(b)
-        Jobs.expects(:enqueue).with(:download_backup_email, has_entries(to_address: @admin.email))
+
+        Jobs.expects(:enqueue).with(:download_backup_email,
+          user_id: @admin.id,
+          backup_file_path: "http://test.host/admin/backups/2014-02-10-065935.tar.gz"
+        )
 
         xhr :put, :email, id: backup_filename
 

spec/jobs/download_backup_email_spec.rb

@@ -0,0 +1,22 @@
+require 'rails_helper'
+
+RSpec.describe Jobs::DownloadBackupEmail do
+  let(:user) { Fabricate(:admin) }
+
+  it "should work" do
+    described_class.new.execute(
+      user_id: user.id,
+      backup_file_path: "http://some.example.test/"
+    )
+
+    email = ActionMailer::Base.deliveries.last
+
+    expect(email.subject).to eq(I18n.t('download_backup_mailer.subject_template',
+      email_prefix: SiteSetting.title
+    ))
+
+    expect(email.body.raw_source).to eq(I18n.t('download_backup_mailer.text_body_template',
+      backup_file_path: "http://some.example.test/?token=#{EmailBackupToken.get(user.id)}"
+    ))
+  end
+end

spec/requests/admin/backups_controller_spec.rb

@@ -0,0 +1,64 @@
+require 'rails_helper'
+
+RSpec.describe Admin::BackupsController do
+  let(:admin) { Fabricate(:admin) }
+
+  before do
+    sign_in(admin)
+  end
+
+  describe '#rollback' do
+    it 'should rollback the restore' do
+      BackupRestore.expects(:rollback!)
+
+      post "/admin/backups/rollback.json"
+
+      expect(response).to be_success
+    end
+
+    it 'should not allow rollback via a GET request' do
+      expect { get "/admin/backups/rollback.json" }
+        .to raise_error(ActionController::RoutingError)
+    end
+  end
+
+  describe '#cancel' do
+    it "should cancel an backup" do
+      BackupRestore.expects(:cancel!)
+
+      delete "/admin/backups/cancel.json"
+
+      expect(response).to be_success
+    end
+
+    it 'should not allow cancel via a GET request' do
+      expect { get "/admin/backups/cancel.json" }
+        .to raise_error(ActionController::RoutingError)
+    end
+  end
+
+  describe "#email" do
+    let(:backup_filename) { "test.tar.gz" }
+    let(:backup) { Backup.new(backup_filename) }
+
+    it "enqueues email job" do
+      Backup.expects(:[]).with(backup_filename).returns(backup)
+
+      Jobs.expects(:enqueue).with(:download_backup_email,
+        user_id: admin.id,
+        backup_file_path: 'http://www.example.com/admin/backups/test.tar.gz'
+      )
+
+      put "/admin/backups/#{backup_filename}.json"
+
+      expect(response).to be_success
+    end
+
+    it "returns 404 when the backup does not exist" do
+      put "/admin/backups/#{backup_filename}.json"
+
+      expect(response).to be_not_found
+    end
+
+  end
+end