Merge pull request #4718 from oblakeerickson/email_discourse_backups
FEATURE: further restrict downloading of backups
This commit is contained in:
commit
0b81a93020
|
@ -1,4 +1,3 @@
|
||||||
import DiscourseURL from 'discourse/lib/url';
|
|
||||||
import { ajax } from 'discourse/lib/ajax';
|
import { ajax } from 'discourse/lib/ajax';
|
||||||
|
|
||||||
export default Ember.Controller.extend({
|
export default Ember.Controller.extend({
|
||||||
|
@ -39,7 +38,11 @@ export default Ember.Controller.extend({
|
||||||
},
|
},
|
||||||
|
|
||||||
download(backup) {
|
download(backup) {
|
||||||
DiscourseURL.redirectTo(backup.get('link'));
|
let link = backup.get('filename');
|
||||||
|
ajax("/admin/backups/" + link, { type: "PUT" })
|
||||||
|
.then(() => {
|
||||||
|
bootbox.alert(I18n.t("admin.backups.operations.download.alert"));
|
||||||
|
});
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
|
||||||
|
@ -48,7 +51,7 @@ export default Ember.Controller.extend({
|
||||||
ajax("/admin/backups/readonly", {
|
ajax("/admin/backups/readonly", {
|
||||||
type: "PUT",
|
type: "PUT",
|
||||||
data: { enable: enable }
|
data: { enable: enable }
|
||||||
}).then(function() {
|
}).then(() => {
|
||||||
site.set("isReadOnly", enable);
|
site.set("isReadOnly", enable);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
|
@ -1,4 +1,5 @@
|
||||||
require "backup_restore/backup_restore"
|
require "backup_restore/backup_restore"
|
||||||
|
require "email_backup_token"
|
||||||
|
|
||||||
class Admin::BackupsController < Admin::AdminController
|
class Admin::BackupsController < Admin::AdminController
|
||||||
|
|
||||||
|
@ -44,14 +45,33 @@ class Admin::BackupsController < Admin::AdminController
|
||||||
render json: success_json
|
render json: success_json
|
||||||
end
|
end
|
||||||
|
|
||||||
# download
|
def email
|
||||||
def show
|
|
||||||
if backup = Backup[params.fetch(:id)]
|
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
|
||||||
|
else
|
||||||
|
render nothing: true, status: 404
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def show
|
||||||
|
|
||||||
|
if !EmailBackupToken.compare(current_user.id, params.fetch(:token))
|
||||||
|
@error = I18n.t('download_backup_mailer.no_token')
|
||||||
|
end
|
||||||
|
if !@error && backup = Backup[params.fetch(:id)]
|
||||||
|
EmailBackupToken.del(current_user.id)
|
||||||
StaffActionLogger.new(current_user).log_backup_download(backup)
|
StaffActionLogger.new(current_user).log_backup_download(backup)
|
||||||
headers['Content-Length'] = File.size(backup.path).to_s
|
headers['Content-Length'] = File.size(backup.path).to_s
|
||||||
send_file backup.path
|
send_file backup.path
|
||||||
else
|
else
|
||||||
render nothing: true, status: 404
|
if @error
|
||||||
|
render layout: 'no_ember', status: 422
|
||||||
|
else
|
||||||
|
render nothing: true, status: 404
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -0,0 +1,22 @@
|
||||||
|
require_dependency 'email/sender'
|
||||||
|
|
||||||
|
module Jobs
|
||||||
|
|
||||||
|
class DownloadBackupEmail < Jobs::Base
|
||||||
|
|
||||||
|
sidekiq_options queue: 'critical'
|
||||||
|
|
||||||
|
def execute(args)
|
||||||
|
to_address = args[:to_address]
|
||||||
|
backup_file_path = args[:backup_file_path]
|
||||||
|
|
||||||
|
raise Discourse::InvalidParameters.new(:to_address) if to_address.blank?
|
||||||
|
raise Discourse::InvalidParameters.new(:backup_file_path) if backup_file_path.blank?
|
||||||
|
|
||||||
|
message = DownloadBackupMailer.send_email(to_address, backup_file_path)
|
||||||
|
Email::Sender.new(message, :download_backup_message).send
|
||||||
|
end
|
||||||
|
|
||||||
|
end
|
||||||
|
|
||||||
|
end
|
|
@ -0,0 +1,9 @@
|
||||||
|
require_dependency 'email/message_builder'
|
||||||
|
|
||||||
|
class DownloadBackupMailer < ActionMailer::Base
|
||||||
|
include Email::BuildEmailHelper
|
||||||
|
|
||||||
|
def send_email(to_address, backup_file_path)
|
||||||
|
build_email(to_address, template: 'download_backup_mailer', backup_file_path: backup_file_path)
|
||||||
|
end
|
||||||
|
end
|
|
@ -0,0 +1,7 @@
|
||||||
|
<div id="simple-container">
|
||||||
|
<%if @error%>
|
||||||
|
<div class='alert alert-error'>
|
||||||
|
<%= @error %>
|
||||||
|
</div>
|
||||||
|
<%end%>
|
||||||
|
</div>
|
|
@ -2689,7 +2689,8 @@ en:
|
||||||
without_uploads: "Yes (do not include files)"
|
without_uploads: "Yes (do not include files)"
|
||||||
download:
|
download:
|
||||||
label: "Download"
|
label: "Download"
|
||||||
title: "Download the backup"
|
title: "Send email with download link"
|
||||||
|
alert: "A link to download this backup has been emailed to you."
|
||||||
destroy:
|
destroy:
|
||||||
title: "Remove the backup"
|
title: "Remove the backup"
|
||||||
confirm: "Are you sure you want to destroy this backup?"
|
confirm: "Are you sure you want to destroy this backup?"
|
||||||
|
|
|
@ -1762,6 +1762,14 @@ en:
|
||||||
|
|
||||||
(If the link above has expired, choose "I forgot my password" when logging in with your email address.)
|
(If the link above has expired, choose "I forgot my password" when logging in with your email address.)
|
||||||
|
|
||||||
|
download_backup_mailer:
|
||||||
|
title: "Download Backup Mailer"
|
||||||
|
subject_template: "[%{site_name}] Backup Download"
|
||||||
|
text_body_template: |
|
||||||
|
Click this link to download your backup file: %{backup_file_path}
|
||||||
|
no_token: |
|
||||||
|
Sorry, this backup download link has already been used or has expired.
|
||||||
|
|
||||||
test_mailer:
|
test_mailer:
|
||||||
title: "Test Mailer"
|
title: "Test Mailer"
|
||||||
subject_template: "[%{site_name}] Email Deliverability Test"
|
subject_template: "[%{site_name}] Email Deliverability Test"
|
||||||
|
|
|
@ -232,6 +232,7 @@ Discourse::Application.routes.draw do
|
||||||
resources :backups, only: [:index, :create], constraints: AdminConstraint.new do
|
resources :backups, only: [:index, :create], constraints: AdminConstraint.new do
|
||||||
member do
|
member do
|
||||||
get "" => "backups#show", constraints: { id: BACKUP_ROUTE_FORMAT }
|
get "" => "backups#show", constraints: { id: BACKUP_ROUTE_FORMAT }
|
||||||
|
put "" => "backups#email", constraints: { id: BACKUP_ROUTE_FORMAT }
|
||||||
delete "" => "backups#destroy", constraints: { id: BACKUP_ROUTE_FORMAT }
|
delete "" => "backups#destroy", constraints: { id: BACKUP_ROUTE_FORMAT }
|
||||||
post "restore" => "backups#restore", constraints: { id: BACKUP_ROUTE_FORMAT }
|
post "restore" => "backups#restore", constraints: { id: BACKUP_ROUTE_FORMAT }
|
||||||
end
|
end
|
||||||
|
|
|
@ -0,0 +1,28 @@
|
||||||
|
class EmailBackupToken
|
||||||
|
|
||||||
|
def self.key(user_id)
|
||||||
|
"email-backup-token:#{user_id}"
|
||||||
|
end
|
||||||
|
|
||||||
|
def self.generate
|
||||||
|
SecureRandom.hex
|
||||||
|
end
|
||||||
|
|
||||||
|
def self.set(user_id)
|
||||||
|
token = self.generate
|
||||||
|
$redis.setex self.key(user_id), 1.day.to_i, token
|
||||||
|
token
|
||||||
|
end
|
||||||
|
|
||||||
|
def self.get(user_id)
|
||||||
|
$redis.get self.key(user_id)
|
||||||
|
end
|
||||||
|
|
||||||
|
def self.del(user_id)
|
||||||
|
$redis.del self.key(user_id)
|
||||||
|
end
|
||||||
|
|
||||||
|
def self.compare(user_id, token)
|
||||||
|
token == self.get(user_id)
|
||||||
|
end
|
||||||
|
end
|
|
@ -90,25 +90,69 @@ describe Admin::BackupsController do
|
||||||
describe ".show" do
|
describe ".show" do
|
||||||
|
|
||||||
it "uses send_file to transmit the backup" do
|
it "uses send_file to transmit the backup" do
|
||||||
path = File.join(Backup.base_directory, backup_filename)
|
begin
|
||||||
File.open(path, "w") { |f| f.write("hello") }
|
token = EmailBackupToken.set(@admin.id)
|
||||||
|
path = File.join(Backup.base_directory, backup_filename)
|
||||||
|
File.open(path, "w") { |f| f.write("hello") }
|
||||||
|
|
||||||
Backup.create_from_filename(backup_filename)
|
Backup.create_from_filename(backup_filename)
|
||||||
|
|
||||||
StaffActionLogger.any_instance.expects(:log_backup_download).once
|
StaffActionLogger.any_instance.expects(:log_backup_download).once
|
||||||
|
|
||||||
get :show, id: backup_filename
|
get :show, id: backup_filename, token: token
|
||||||
|
|
||||||
File.delete(path) rescue nil
|
|
||||||
|
|
||||||
expect(response.headers['Content-Length']).to eq("5")
|
expect(response.headers['Content-Length']).to eq("5")
|
||||||
expect(response.headers['Content-Disposition']).to match(/attachment; filename/)
|
expect(response.headers['Content-Disposition']).to match(/attachment; filename/)
|
||||||
|
ensure
|
||||||
|
File.delete(path)
|
||||||
|
EmailBackupToken.del(@admin.id)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
it "returns 422 when token is bad" do
|
||||||
|
begin
|
||||||
|
path = File.join(Backup.base_directory, backup_filename)
|
||||||
|
File.open(path, "w") { |f| f.write("hello") }
|
||||||
|
|
||||||
|
Backup.create_from_filename(backup_filename)
|
||||||
|
|
||||||
|
get :show, id: backup_filename, token: "bad_value"
|
||||||
|
|
||||||
|
expect(response.status).to eq(422)
|
||||||
|
ensure
|
||||||
|
File.delete(path)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
it "returns 404 when the backup does not exist" do
|
it "returns 404 when the backup does not exist" do
|
||||||
|
token = EmailBackupToken.set(@admin.id)
|
||||||
Backup.expects(:[]).returns(nil)
|
Backup.expects(:[]).returns(nil)
|
||||||
|
|
||||||
get :show, id: backup_filename
|
get :show, id: backup_filename, token: token
|
||||||
|
|
||||||
|
EmailBackupToken.del(@admin.id)
|
||||||
|
|
||||||
|
expect(response).to be_not_found
|
||||||
|
end
|
||||||
|
|
||||||
|
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))
|
||||||
|
|
||||||
|
xhr :put, :email, id: backup_filename
|
||||||
|
|
||||||
|
expect(response).to be_success
|
||||||
|
end
|
||||||
|
|
||||||
|
it "returns 404 when the backup does not exist" do
|
||||||
|
xhr :put, :email, id: backup_filename
|
||||||
|
|
||||||
expect(response).to be_not_found
|
expect(response).to be_not_found
|
||||||
end
|
end
|
||||||
|
@ -227,23 +271,25 @@ describe Admin::BackupsController do
|
||||||
|
|
||||||
describe "when filename is valid" do
|
describe "when filename is valid" do
|
||||||
it "should upload the file successfully" do
|
it "should upload the file successfully" do
|
||||||
described_class.any_instance.expects(:has_enough_space_on_disk?).returns(true)
|
begin
|
||||||
|
described_class.any_instance.expects(:has_enough_space_on_disk?).returns(true)
|
||||||
|
|
||||||
filename = 'test_Site-0123456789.tar.gz'
|
filename = 'test_Site-0123456789.tar.gz'
|
||||||
|
|
||||||
xhr :post, :upload_backup_chunk,
|
xhr :post, :upload_backup_chunk,
|
||||||
resumableFilename: filename,
|
resumableFilename: filename,
|
||||||
resumableTotalSize: 1,
|
resumableTotalSize: 1,
|
||||||
resumableIdentifier: 'test',
|
resumableIdentifier: 'test',
|
||||||
resumableChunkNumber: '1',
|
resumableChunkNumber: '1',
|
||||||
resumableChunkSize: '1',
|
resumableChunkSize: '1',
|
||||||
resumableCurrentChunkSize: '1',
|
resumableCurrentChunkSize: '1',
|
||||||
file: fixture_file_upload(Tempfile.new)
|
file: fixture_file_upload(Tempfile.new)
|
||||||
|
|
||||||
expect(response.status).to eq(200)
|
expect(response.status).to eq(200)
|
||||||
expect(response.body).to eq("")
|
expect(response.body).to eq("")
|
||||||
|
ensure
|
||||||
File.delete(File.join(Backup.base_directory, filename)) rescue nil
|
File.delete(File.join(Backup.base_directory, filename))
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue