From 56f5b21a90bf5bbf99b2c8e98c53180e6e8e45a1 Mon Sep 17 00:00:00 2001
From: Robin Ward <robin.ward@gmail.com>
Date: Fri, 19 May 2017 15:59:37 -0400
Subject: [PATCH] SECURITY: Validate the `entity` when downloading a CSV

---
 app/assets/javascripts/discourse/lib/export-csv.js.es6 |  2 +-
 app/controllers/export_csv_controller.rb               |  5 ++---
 lib/guardian.rb                                        |  6 ++++--
 spec/controllers/export_csv_controller_spec.rb         | 10 +++++-----
 4 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/app/assets/javascripts/discourse/lib/export-csv.js.es6 b/app/assets/javascripts/discourse/lib/export-csv.js.es6
index dc6e66b3a9a..9bdae8971e7 100644
--- a/app/assets/javascripts/discourse/lib/export-csv.js.es6
+++ b/app/assets/javascripts/discourse/lib/export-csv.js.es6
@@ -2,7 +2,7 @@ import { ajax } from 'discourse/lib/ajax';
 function exportEntityByType(type, entity, args) {
   return ajax("/export_csv/export_entity.json", {
     method: 'POST',
-    data: {entity_type: type, entity, args}
+    data: {entity, args}
   });
 }
 
diff --git a/app/controllers/export_csv_controller.rb b/app/controllers/export_csv_controller.rb
index 5545f872d4f..8f407b92fa3 100644
--- a/app/controllers/export_csv_controller.rb
+++ b/app/controllers/export_csv_controller.rb
@@ -3,7 +3,7 @@ class ExportCsvController < ApplicationController
   skip_before_filter :preload_json, :check_xhr, only: [:show]
 
   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])
     render json: success_json
   end
@@ -29,8 +29,7 @@ class ExportCsvController < ApplicationController
     def export_params
       @_export_params ||= begin
         params.require(:entity)
-        params.require(:entity_type)
-        params.permit(:entity, :entity_type, args: [:name, :start_date, :end_date, :category_id, :group_id, :trust_level])
+        params.permit(:entity, args: [:name, :start_date, :end_date, :category_id, :group_id, :trust_level])
       end
     end
 end
diff --git a/lib/guardian.rb b/lib/guardian.rb
index bd678fd811b..d2a66fe2e2f 100644
--- a/lib/guardian.rb
+++ b/lib/guardian.rb
@@ -284,10 +284,12 @@ class Guardian
     @can_see_emails
   end
 
-  def can_export_entity?(entity_type)
+  def can_export_entity?(entity)
     return false unless @user
     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
   end
 
diff --git a/spec/controllers/export_csv_controller_spec.rb b/spec/controllers/export_csv_controller_spec.rb
index 3505512bc45..261ff03d9fc 100644
--- a/spec/controllers/export_csv_controller_spec.rb
+++ b/spec/controllers/export_csv_controller_spec.rb
@@ -18,19 +18,19 @@ describe ExportCsvController do
     describe ".export_entity" do
       it "enqueues export job" do
         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
       end
 
       it "should not enqueue export job if rate limit is reached" do
         Jobs::ExportCsvFile.any_instance.expects(:execute).never
         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
       end
 
       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
       end
     end
@@ -67,14 +67,14 @@ describe ExportCsvController do
     describe ".export_entity" do
       it "enqueues export job" do
         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
       end
 
       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))
         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
       end
     end