Merge pull request #15472 from isimluk/CVE-2016-7047-final

CVE-2016-7047: API leaks any MiqReportResult

(transferred from ManageIQ/manageiq@a13b67b)

app/controllers/api/reports_controller.rb

@@ -7,8 +7,16 @@ class ReportsController < BaseController
 
     before_action :set_additional_attributes, :only => [:index, :show]
 
-    def run_resource(_type, id, _data)
-      report = MiqReport.find(id)
+    def reports_search_conditions
+      MiqReport.for_user(User.current_user).where_clause.ast unless User.current_user.admin?
+    end
+
+    def find_reports(id)
+      MiqReport.for_user(User.current_user).find(id)
+    end
+
+    def run_resource(type, id, _data)
+      report = resource_search(id, type, MiqReport)
       report_result = MiqReportResult.find(report.queue_generate_table)
       run_report_result(true,
                         "running report #{report.id}",

app/controllers/api/results_controller.rb

@@ -2,6 +2,14 @@ module Api
   class ResultsController < BaseController
     before_action :set_additional_attributes, :only => [:index, :show]
 
+    def results_search_conditions
+      MiqReportResult.for_user(User.current_user).where_clause.ast
+    end
+
+    def find_results(id)
+      MiqReportResult.for_user(User.current_user).find(id)
+    end
+
     private
 
     def set_additional_attributes

app/controllers/api/subcollections/results.rb

@@ -1,8 +1,12 @@
 module Api
   module Subcollections
     module Results
+      def find_results(id)
+        MiqReportResult.for_user(User.current_user).find(id)
+      end
+
       def results_query_resource(object)
-        object.miq_report_results
+        object.miq_report_results.for_user(User.current_user)
       end
     end
   end

spec/requests/api/collections_spec.rb

@@ -185,7 +185,7 @@ def test_collection_bulk_query(collection, collection_url, klass, id = nil)
     end
 
     it "query Report Results" do
-      FactoryGirl.create(:miq_report_result)
+      FactoryGirl.create(:miq_report_result, :miq_group => @user.current_group)
       test_collection_query(:results, results_url, MiqReportResult)
     end
 
@@ -438,7 +438,7 @@ def test_collection_bulk_query(collection, collection_url, klass, id = nil)
     end
 
     it "bulk query Report Results" do
-      FactoryGirl.create(:miq_report_result)
+      FactoryGirl.create(:miq_report_result, :miq_group => @user.current_group)
       test_collection_bulk_query(:results, results_url, MiqReportResult)
     end
 

spec/requests/api/reports_spec.rb

@@ -33,71 +33,97 @@
     expect(response).to have_http_status(:ok)
   end
 
-  it "can fetch a report's results" do
-    report = FactoryGirl.create(:miq_report_with_results)
-    report_result = report.miq_report_results.first
+  context 'authorized to see its own report results' do
+    let(:group) { FactoryGirl.create(:miq_group) }
+    let(:user) do
+      @user.current_group ||= group
+      @user
+    end
+    let(:report) { FactoryGirl.create(:miq_report_with_results, :miq_group => user.current_group) }
 
-    api_basic_authorize
-    run_get "#{reports_url(report.id)}/results"
+    it "can fetch a report's results" do
+      report_result = report.miq_report_results.first
 
-    expect_result_resources_to_include_hrefs(
-      "resources",
-      [
-        "#{reports_url(report.id)}/results/#{report_result.to_param}"
-      ]
-    )
-    expect(response.parsed_body["resources"]).not_to be_any { |resource| resource.key?("result_set") }
-    expect(response).to have_http_status(:ok)
-  end
+      api_basic_authorize
+      run_get "#{reports_url(report.id)}/results"
 
-  it "can fetch a report's result" do
-    report = FactoryGirl.create(:miq_report_with_results)
-    report_result = report.miq_report_results.first
-    table = Ruport::Data::Table.new(
-      :column_names => %w(foo),
-      :data         => [%w(bar), %w(baz)]
-    )
-    allow(report).to receive(:table).and_return(table)
-    allow_any_instance_of(MiqReportResult).to receive(:report_results).and_return(report)
+      expect_result_resources_to_include_hrefs(
+        "resources",
+        [
+          "#{reports_url(report.id)}/results/#{report_result.to_param}"
+        ]
+      )
+      expect(response.parsed_body["resources"]).not_to be_any { |resource| resource.key?("result_set") }
+      expect(response).to have_http_status(:ok)
+    end
 
-    api_basic_authorize
-    run_get "#{reports_url(report.id)}/results/#{report_result.to_param}"
+    it "can fetch a report's result" do
+      report_result = report.miq_report_results.first
+      table = Ruport::Data::Table.new(
+        :column_names => %w(foo),
+        :data         => [%w(bar), %w(baz)]
+      )
+      allow(report).to receive(:table).and_return(table)
+      allow_any_instance_of(MiqReportResult).to receive(:report_results).and_return(report)
 
-    expect_result_to_match_hash(response.parsed_body, "result_set" => [{"foo" => "bar"}, {"foo" => "baz"}])
-    expect(response).to have_http_status(:ok)
-  end
+      api_basic_authorize
+      run_get "#{reports_url(report.id)}/results/#{report_result.to_param}"
 
-  it "can fetch all the results" do
-    report = FactoryGirl.create(:miq_report_with_results)
-    result = report.miq_report_results.first
+      expect_result_to_match_hash(response.parsed_body, "result_set" => [{"foo" => "bar"}, {"foo" => "baz"}])
+      expect(response).to have_http_status(:ok)
+    end
 
-    api_basic_authorize collection_action_identifier(:results, :read, :get)
-    run_get results_url
+    it "can fetch all the results" do
+      result = report.miq_report_results.first
 
-    expect_result_resources_to_include_hrefs(
-      "resources",
-      [
-        results_url(result.id).to_s
-      ]
-    )
-    expect(response).to have_http_status(:ok)
-  end
+      api_basic_authorize collection_action_identifier(:results, :read, :get)
+      run_get results_url
 
-  it "can fetch a specific result as a primary collection" do
-    report = FactoryGirl.create(:miq_report_with_results)
-    report_result = report.miq_report_results.first
-    table = Ruport::Data::Table.new(
-      :column_names => %w(foo),
-      :data         => [%w(bar), %w(baz)]
-    )
-    allow(report).to receive(:table).and_return(table)
-    allow_any_instance_of(MiqReportResult).to receive(:report_results).and_return(report)
+      expect_result_resources_to_include_hrefs(
+        "resources",
+        [
+          results_url(result.id).to_s
+        ]
+      )
+      expect(response).to have_http_status(:ok)
+    end
 
-    api_basic_authorize action_identifier(:results, :read, :resource_actions, :get)
-    run_get results_url(report_result.id)
+    it "can fetch a specific result as a primary collection" do
+      report_result = report.miq_report_results.first
+      table = Ruport::Data::Table.new(
+        :column_names => %w(foo),
+        :data         => [%w(bar), %w(baz)]
+      )
+      allow(report).to receive(:table).and_return(table)
+      allow_any_instance_of(MiqReportResult).to receive(:report_results).and_return(report)
 
-    expect_result_to_match_hash(response.parsed_body, "result_set" => [{"foo" => "bar"}, {"foo" => "baz"}])
-    expect(response).to have_http_status(:ok)
+      api_basic_authorize action_identifier(:results, :read, :resource_actions, :get)
+      run_get results_url(report_result.id)
+
+      expect_result_to_match_hash(response.parsed_body, "result_set" => [{"foo" => "bar"}, {"foo" => "baz"}])
+      expect(response).to have_http_status(:ok)
+    end
+
+    it "returns an empty result set if none has been run" do
+      report_result = report.miq_report_results.first
+
+      api_basic_authorize
+      run_get "#{reports_url(report.id)}/results/#{report_result.id}"
+
+      expect_result_to_match_hash(response.parsed_body, "result_set" => [])
+      expect(response).to have_http_status(:ok)
+    end
+
+    it "returns an empty result set if none has been run" do
+      report = FactoryGirl.create(:miq_report_with_results, :miq_group => user.current_group)
+      report_result = report.miq_report_results.first
+
+      api_basic_authorize
+      run_get "#{reports_url(report.id)}/results/#{report_result.id}"
+
+      expect_result_to_match_hash(response.parsed_body, "result_set" => [])
+      expect(response).to have_http_status(:ok)
+    end
   end
 
   it "can fetch all the schedule" do
@@ -166,17 +192,6 @@
     expect(response).to have_http_status(:forbidden)
   end
 
-  it "returns an empty result set if none has been run" do
-    report = FactoryGirl.create(:miq_report_with_results)
-    report_result = report.miq_report_results.first
-
-    api_basic_authorize
-    run_get "#{reports_url(report.id)}/results/#{report_result.id}"
-
-    expect_result_to_match_hash(response.parsed_body, "result_set" => [])
-    expect(response).to have_http_status(:ok)
-  end
-
   context "with an appropriate role" do
     it "can run a report" do
       report = FactoryGirl.create(:miq_report)