SECURITY: Validate the entity when downloading a CSV

This commit is contained in:
Robin Ward 2017-05-19 15:59:37 -04:00
parent 7f9c2f75b4
commit 56f5b21a90
4 changed files with 12 additions and 11 deletions

View File

@ -2,7 +2,7 @@ import { ajax } from 'discourse/lib/ajax';
function exportEntityByType(type, entity, args) { function exportEntityByType(type, entity, args) {
return ajax("/export_csv/export_entity.json", { return ajax("/export_csv/export_entity.json", {
method: 'POST', method: 'POST',
data: {entity_type: type, entity, args} data: {entity, args}
}); });
} }

View File

@ -3,7 +3,7 @@ class ExportCsvController < ApplicationController
skip_before_filter :preload_json, :check_xhr, only: [:show] skip_before_filter :preload_json, :check_xhr, only: [:show]
def export_entity def export_entity
guardian.ensure_can_export_entity!(export_params[:entity_type]) guardian.ensure_can_export_entity!(export_params[:entity])
Jobs.enqueue(:export_csv_file, entity: export_params[:entity], user_id: current_user.id, args: export_params[:args]) Jobs.enqueue(:export_csv_file, entity: export_params[:entity], user_id: current_user.id, args: export_params[:args])
render json: success_json render json: success_json
end end
@ -29,8 +29,7 @@ class ExportCsvController < ApplicationController
def export_params def export_params
@_export_params ||= begin @_export_params ||= begin
params.require(:entity) params.require(:entity)
params.require(:entity_type) params.permit(:entity, args: [:name, :start_date, :end_date, :category_id, :group_id, :trust_level])
params.permit(:entity, :entity_type, args: [:name, :start_date, :end_date, :category_id, :group_id, :trust_level])
end end
end end
end end

View File

@ -284,10 +284,12 @@ class Guardian
@can_see_emails @can_see_emails
end end
def can_export_entity?(entity_type) def can_export_entity?(entity)
return false unless @user return false unless @user
return true if is_staff? return true if is_staff?
return false if entity_type == "admin"
# Regular users can only export their archives
return false unless entity == "user_archive"
UserExport.where(user_id: @user.id, created_at: (Time.zone.now.beginning_of_day..Time.zone.now.end_of_day)).count == 0 UserExport.where(user_id: @user.id, created_at: (Time.zone.now.beginning_of_day..Time.zone.now.end_of_day)).count == 0
end end

View File

@ -18,19 +18,19 @@ describe ExportCsvController do
describe ".export_entity" do describe ".export_entity" do
it "enqueues export job" do it "enqueues export job" do
Jobs.expects(:enqueue).with(:export_csv_file, has_entries(entity: "user_archive", user_id: @user.id)) Jobs.expects(:enqueue).with(:export_csv_file, has_entries(entity: "user_archive", user_id: @user.id))
xhr :post, :export_entity, entity: "user_archive", entity_type: "user" xhr :post, :export_entity, entity: "user_archive"
expect(response).to be_success expect(response).to be_success
end end
it "should not enqueue export job if rate limit is reached" do it "should not enqueue export job if rate limit is reached" do
Jobs::ExportCsvFile.any_instance.expects(:execute).never Jobs::ExportCsvFile.any_instance.expects(:execute).never
UserExport.create(file_name: "user-archive-codinghorror-150116-003249", user_id: @user.id) UserExport.create(file_name: "user-archive-codinghorror-150116-003249", user_id: @user.id)
xhr :post, :export_entity, entity: "user_archive", entity_type: "user" xhr :post, :export_entity, entity: "user_archive"
expect(response).not_to be_success expect(response).not_to be_success
end end
it "returns 404 when normal user tries to export admin entity" do it "returns 404 when normal user tries to export admin entity" do
xhr :post, :export_entity, entity: "staff_action", entity_type: "admin" xhr :post, :export_entity, entity: "staff_action"
expect(response).not_to be_success expect(response).not_to be_success
end end
end end
@ -67,14 +67,14 @@ describe ExportCsvController do
describe ".export_entity" do describe ".export_entity" do
it "enqueues export job" do it "enqueues export job" do
Jobs.expects(:enqueue).with(:export_csv_file, has_entries(entity: "staff_action", user_id: @admin.id)) Jobs.expects(:enqueue).with(:export_csv_file, has_entries(entity: "staff_action", user_id: @admin.id))
xhr :post, :export_entity, entity: "staff_action", entity_type: "admin" xhr :post, :export_entity, entity: "staff_action"
expect(response).to be_success expect(response).to be_success
end end
it "should not rate limit export for staff" do it "should not rate limit export for staff" do
Jobs.expects(:enqueue).with(:export_csv_file, has_entries(entity: "staff_action", user_id: @admin.id)) Jobs.expects(:enqueue).with(:export_csv_file, has_entries(entity: "staff_action", user_id: @admin.id))
UserExport.create(file_name: "screened-email-150116-010145", user_id: @admin.id) UserExport.create(file_name: "screened-email-150116-010145", user_id: @admin.id)
xhr :post, :export_entity, entity: "staff_action", entity_type: "admin" xhr :post, :export_entity, entity: "staff_action"
expect(response).to be_success expect(response).to be_success
end end
end end