Revision d213e460
Added by Daniel Lobato Garcia over 8 years ago
| app/controllers/api/v1/reports_controller.rb | ||
|---|---|---|
|
module Api
|
||
|
module V1
|
||
|
class ReportsController < V1::BaseController
|
||
|
before_filter :find_resource, :only => %w{show update destroy}
|
||
|
before_filter :find_resource, :only => %w{show destroy}
|
||
|
before_filter :setup_search_options, :only => [:index, :last]
|
||
|
|
||
|
api :GET, "/reports/", "List all reports."
|
||
| ... | ... | |
|
param :id, :identifier, :required => true
|
||
|
|
||
|
def last
|
||
|
conditions = { :host_id => Host.find(params[:host_id]).try(:id) } unless params[:host_id].blank?
|
||
|
max_id = Report.authorized(:view_reports).my_reports.where(conditions).maximum(:id)
|
||
|
@report = Report.authorized(:view_reports).includes(:logs => [:message, :source]).find(max_id)
|
||
|
conditions = { :host_id => Host.authorized(:view_hosts).find(params[:host_id]).try(:id) } if params[:host_id].present?
|
||
|
max_id = resource_scope.where(conditions).maximum(:id)
|
||
|
@report = resource_scope.includes(:logs => [:message, :source]).find(max_id)
|
||
|
render :show
|
||
|
end
|
||
|
|
||
|
private
|
||
|
|
||
|
def resource_scope(options = {})
|
||
|
super(options).my_reports
|
||
|
end
|
||
|
|
||
|
def action_permission
|
||
|
case params[:action]
|
||
|
when 'last'
|
||
|
'view'
|
||
|
else
|
||
|
super
|
||
|
end
|
||
|
end
|
||
|
end
|
||
|
end
|
||
|
end
|
||
| app/controllers/api/v2/reports_controller.rb | ||
|---|---|---|
|
include Api::Version2
|
||
|
include Foreman::Controller::SmartProxyAuth
|
||
|
|
||
|
before_filter :find_resource, :only => %w{show update destroy}
|
||
|
before_filter :find_resource, :only => %w{show destroy}
|
||
|
before_filter :setup_search_options, :only => [:index, :last]
|
||
|
|
||
|
add_smart_proxy_filters :create, :features => ReportImporter.report_features
|
||
| ... | ... | |
|
param :id, :identifier, :required => true
|
||
|
|
||
|
def last
|
||
|
conditions = { :host_id => Host.find(params[:host_id]).id } unless params[:host_id].blank?
|
||
|
max_id = Report.authorized(:view_reports).my_reports.where(conditions).maximum(:id)
|
||
|
@report = Report.authorized(:view_reports).includes(:logs => [:message, :source]).find(max_id)
|
||
|
conditions = { :host_id => Host.authorized(:view_hosts).find(params[:host_id]).try(:id) } if params[:host_id].present?
|
||
|
max_id = resource_scope.where(conditions).maximum(:id)
|
||
|
@report = resource_scope.includes(:logs => [:message, :source]).find(max_id)
|
||
|
render :show
|
||
|
end
|
||
|
|
||
|
private
|
||
|
|
||
|
def resource_scope(options = {})
|
||
|
super(options).my_reports
|
||
|
end
|
||
|
|
||
|
def action_permission
|
||
|
case params[:action]
|
||
|
when 'last'
|
||
|
'view'
|
||
|
else
|
||
|
super
|
||
|
end
|
||
|
end
|
||
|
end
|
||
|
end
|
||
|
end
|
||
| app/controllers/reports_controller.rb | ||
|---|---|---|
|
before_filter :setup_search_options, :only => :index
|
||
|
|
||
|
def index
|
||
|
report_authorized = resource_base.my_reports
|
||
|
@reports = report_authorized.search_for(params[:search], :order => params[:order]).paginate(:page => params[:page], :per_page => params[:per_page]).includes(:host)
|
||
|
@reports = resource_base.search_for(params[:search], :order => params[:order]).paginate(:page => params[:page], :per_page => params[:per_page]).includes(:host)
|
||
|
end
|
||
|
|
||
|
def show
|
||
|
# are we searching for the last report?
|
||
|
if params[:id] == "last"
|
||
|
conditions = { :host_id => Host.find(params[:host_id]).try(:id) } unless params[:host_id].blank?
|
||
|
conditions = { :host_id => Host.authorized(:view_hosts).find(params[:host_id]).try(:id) } if params[:host_id].present?
|
||
|
params[:id] = resource_base.where(conditions).maximum(:id)
|
||
|
end
|
||
|
|
||
|
return not_found if params[:id].blank?
|
||
|
|
||
|
@report = resource_base.includes(:logs => [:message, :source]).find(params[:id])
|
||
|
@offset = @report.reported_at - @report.created_at
|
||
|
end
|
||
| ... | ... | |
|
def destroy
|
||
|
@report = resource_base.find(params[:id])
|
||
|
if @report.destroy
|
||
|
process_success :success_msg => _("Successfully deleted report."), :success_redirect => reports_path
|
||
|
process_success(:success_msg => _("Successfully deleted report."), :success_redirect => reports_path)
|
||
|
else
|
||
|
process_error
|
||
|
end
|
||
|
end
|
||
|
|
||
|
private
|
||
|
|
||
|
def resource_base
|
||
|
super.my_reports
|
||
|
end
|
||
|
end
|
||
| test/factories/user_related.rb | ||
|---|---|---|
|
trait :architecture do
|
||
|
resource_type 'Architecture'
|
||
|
end
|
||
|
|
||
|
trait :report do
|
||
|
resource_type 'Report'
|
||
|
end
|
||
|
end
|
||
|
|
||
|
factory :role do
|
||
| test/functional/api/v1/reports_controller_test.rb | ||
|---|---|---|
|
require 'test_helper'
|
||
|
require 'functional/shared/report_host_permissions_test'
|
||
|
|
||
|
class Api::V1::ReportsControllerTest < ActionController::TestCase
|
||
|
include ::ReportHostPermissionsTest
|
||
|
|
||
|
test "should get index" do
|
||
|
get :index, { }
|
||
|
assert_response :success
|
||
| ... | ... | |
|
get :last, {:host_id => host.to_param }
|
||
|
assert_response :not_found
|
||
|
end
|
||
|
|
||
|
test 'cannot view the last report without hosts view permission' do
|
||
|
setup_user('view', 'reports')
|
||
|
report = FactoryGirl.create(:report)
|
||
|
get :last, { :host_id => report.host.id }, set_session_user.merge(:user => User.current)
|
||
|
assert_response :not_found
|
||
|
end
|
||
|
end
|
||
| test/functional/api/v2/reports_controller_test.rb | ||
|---|---|---|
|
require 'test_helper'
|
||
|
require 'functional/shared/report_host_permissions_test'
|
||
|
|
||
|
class Api::V2::ReportsControllerTest < ActionController::TestCase
|
||
|
include ::ReportHostPermissionsTest
|
||
|
|
||
|
describe "Non Admin User" do
|
||
|
def setup
|
||
|
User.current = users(:one) #use an unpriviledged user, not apiadmin
|
||
|
User.current = users(:one) #use an unprivileged user, not apiadmin
|
||
|
end
|
||
|
|
||
|
def create_a_puppet_transaction_report
|
||
| ... | ... | |
|
get :last, {:host_id => host.to_param }
|
||
|
assert_response :not_found
|
||
|
end
|
||
|
|
||
|
test 'cannot view the last report without hosts view permission' do
|
||
|
setup_user('view', 'reports')
|
||
|
report = FactoryGirl.create(:report)
|
||
|
get :last, { :host_id => report.host.id }, set_session_user.merge(:user => User.current)
|
||
|
assert_response :not_found
|
||
|
end
|
||
|
end
|
||
| test/functional/reports_controller_test.rb | ||
|---|---|---|
|
require 'test_helper'
|
||
|
require 'functional/shared/report_host_permissions_test'
|
||
|
|
||
|
class ReportsControllerTest < ActionController::TestCase
|
||
|
setup do
|
||
|
User.current = users :admin
|
||
|
end
|
||
|
include ::ReportHostPermissionsTest
|
||
|
|
||
|
def test_index
|
||
|
get :index, {}, set_session_user
|
||
| ... | ... | |
|
assert_template 'show'
|
||
|
end
|
||
|
|
||
|
test '404 on show when id is blank' do
|
||
|
get :show, {:id => ' '}, set_session_user
|
||
|
assert_response :missing
|
||
|
assert_template 'common/404'
|
||
|
end
|
||
|
|
||
|
def test_show_last
|
||
|
FactoryGirl.create(:report)
|
||
|
get :show, {:id => "last"}, set_session_user
|
||
|
assert_template 'show'
|
||
|
end
|
||
|
|
||
|
test '404 on last when no reports available' do
|
||
|
get :show, { :id => 'last', :host_id => FactoryGirl.create(:host) }, set_session_user
|
||
|
assert_response :missing
|
||
|
assert_template 'common/404'
|
||
|
end
|
||
|
|
||
|
def test_show_last_report_for_host
|
||
|
report = FactoryGirl.create(:report)
|
||
|
get :show, {:id => "last", :host_id => report.host.to_param}, set_session_user
|
||
| ... | ... | |
|
assert_redirected_to reports_path
|
||
|
end
|
||
|
|
||
|
def create_a_report
|
||
|
@report = Report.import JSON.parse(File.read(File.expand_path(File.dirname(__FILE__) + "/../fixtures/report-empty.json")))
|
||
|
test 'cannot view the last report without hosts view permission' do
|
||
|
setup_user('view', 'reports')
|
||
|
report = FactoryGirl.create(:report)
|
||
|
get :show, { :id => 'last', :host_id => report.host.id }, set_session_user.merge(:user => User.current)
|
||
|
assert_response :not_found
|
||
|
end
|
||
|
|
||
|
def user_setup
|
||
|
@request.session[:user] = users(:one).id
|
||
|
users(:one).roles = [Role.find_by_name('Anonymous'), Role.find_by_name('Viewer')]
|
||
|
end
|
||
|
private
|
||
|
|
||
|
test 'user with viewer rights should succeed in viewing reports' do
|
||
|
user_setup
|
||
|
get :index, {}, set_session_user
|
||
|
assert_response :success
|
||
|
def create_a_report
|
||
|
@report = Report.import JSON.parse(File.read(File.expand_path(File.dirname(__FILE__) + "/../fixtures/report-empty.json")))
|
||
|
end
|
||
|
end
|
||
| test/functional/shared/report_host_permissions_test.rb | ||
|---|---|---|
|
module ReportHostPermissionsTest
|
||
|
extend ActiveSupport::Concern
|
||
|
included do
|
||
|
context 'when user does not have permission to view hosts' do
|
||
|
setup { setup_user('view', 'reports') }
|
||
|
|
||
|
test 'cannot view any reports' do
|
||
|
report = FactoryGirl.create(:report)
|
||
|
get :show, { :id => report.id }, set_session_user.merge(:user => User.current)
|
||
|
assert_response :not_found
|
||
|
end
|
||
|
|
||
|
test 'cannot delete host reports' do
|
||
|
setup_user 'destroy', 'reports'
|
||
|
report = FactoryGirl.create(:report)
|
||
|
delete :destroy, { :id => report.id }, set_session_user.merge(:user => User.current)
|
||
|
assert_response :not_found
|
||
|
end
|
||
|
end
|
||
|
end
|
||
|
end
|
||
Also available in: Unified diff
Fixes #11579 - Reports show/destroy restricted by host authorization (CVE-2015-5233)
ReportsController 'show' and 'destroy' now perform a check to see if
the User is authorized to see the Host associated with the Report. In
case it's not, it returns 404, as to not give hints whether a Report
ID or Host ID are valid.
I followed the same approach on the API controllers. 'last' was not
vulnerable due to using my_reports which performs the necessary
check on 'view_hosts' permission.
(cherry picked from commit 293036dfa71ae70624663647f1ef70798bf53d3e)