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,9 +46,11 @@ class Admin::BackupsController < Admin::AdminController
 
   def email
     if backup = Backup[params.fetch(:id)]
-      token = EmailBackupToken.set(current_user.id)
+      Jobs.enqueue(:download_backup_email,
-      download_url = "#{url_for(controller: 'backups', action: 'show')}?token=#{token}"
+        user_id: current_user.id,
-      Jobs.enqueue(:download_backup_email, to_address: current_user.email, backup_file_path: download_url)
+        backup_file_path: url_for(controller: 'backups', action: 'show')
+      )
+
       render body: nil
     else
       render body: nil, status: 404

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]
+      user_id = args[:user_id]
-      backup_file_path = args[:backup_file_path]
+      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

@@ -127,27 +127,6 @@ describe Admin::BackupsController do
 
     end
 
-    describe ".email" do
-
-      let(:b) { Backup.new(backup_filename) }
-
-      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))
-
-        put :email, params: { id: backup_filename }, format: :json
-
-        expect(response).to be_success
-      end
-
-      it "returns 404 when the backup does not exist" do
-        put :email, params: { id: backup_filename }, format: :json
-
-        expect(response).to be_not_found
-      end
-
-    end
-
     describe ".destroy" do
 
       let(:b) { Backup.new(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

@@ -36,4 +36,29 @@ RSpec.describe Admin::BackupsController do
         .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