Revision be0b9bee
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
|
||
... | ... | |
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)