Skip to content

Commit

Permalink
fixes #10627 - apply host taxonomy scope to facts/report joins
Browse files Browse the repository at this point in the history
Previous changes to perform a join onto hosts with authorisation result
in a query such as Report.joins(:hosts), which ignores any
default_scope on Host::Base.  This commit explicitly passes taxonomy
conditionals into the Authorizer to re-apply it.
  • Loading branch information
Dominic Cleal authored and dLobatog committed Jun 11, 2015
1 parent 9250d8b commit 9926db4
Show file tree
Hide file tree
Showing 9 changed files with 127 additions and 15 deletions.
13 changes: 7 additions & 6 deletions app/models/concerns/authorizable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,14 @@ module Authorizable
#
# Or you may simply use authorized for User.current
#
scope :joins_authorized_as, Proc.new { |user, resource, permission|
# The default scope of `resource` is NOT applied since it's a join, instead
# any extra conditions can be given in `opts[:where]`.
#
scope :joins_authorized_as, Proc.new { |user, resource, permission, opts = {}|
if user.nil?
self.where('1=0')
elsif user.admin?
self.scoped
else
Authorizer.new(user).find_collection(resource, :permission => permission, :joined_on => self)
Authorizer.new(user).find_collection(resource, {:permission => permission, :joined_on => self}.merge(opts) )
end
}

Expand All @@ -65,8 +66,8 @@ def authorized(permission = nil, resource = nil)
authorized_as(User.current, permission, resource)
end

def joins_authorized(resource, permission = nil)
joins_authorized_as(User.current, resource, permission)
def joins_authorized(resource, permission = nil, opts = {})
joins_authorized_as(User.current, resource, permission, opts)
end
end
end
4 changes: 2 additions & 2 deletions app/models/fact_value.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ class FactValue < ActiveRecord::Base
joins(:fact_name).where("fact_names.name = ?",:_timestamp)
}
scope :my_facts, lambda {
unless User.current.admin? and Organization.current.nil? and Location.current.nil?
joins_authorized(Host, :view_hosts)
if !User.current.admin? || Organization.expand(Organization.current).present? || Location.expand(Location.current).present?
joins_authorized(Host, :view_hosts, :where => Host.taxonomy_conditions)
end
}

Expand Down
8 changes: 6 additions & 2 deletions app/models/host/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,17 @@ class Base < ActiveRecord::Base
validate :uniq_interfaces_identifiers

default_scope lambda {
where(taxonomy_conditions)
}

def self.taxonomy_conditions
org = Organization.expand(Organization.current) if SETTINGS[:organizations_enabled]
loc = Location.expand(Location.current) if SETTINGS[:locations_enabled]
conditions = {}
conditions[:organization_id] = Array(org).map { |o| o.subtree_ids }.flatten.uniq if org.present?
conditions[:location_id] = Array(loc).map { |l| l.subtree_ids }.flatten.uniq if loc.present?
where(conditions)
}
conditions
end

scope :no_location, lambda { where(:location_id => nil) }
scope :no_organization, lambda { where(:organization_id => nil) }
Expand Down
2 changes: 1 addition & 1 deletion app/models/report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class Report < ActiveRecord::Base
# returns reports for hosts in the User's filter set
scope :my_reports, lambda {
if !User.current.admin? || Organization.expand(Organization.current).present? || Location.expand(Location.current).present?
joins_authorized(Host, :view_hosts)
joins_authorized(Host, :view_hosts, :where => Host.taxonomy_conditions)
end
}

Expand Down
10 changes: 7 additions & 3 deletions app/services/authorizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ def find_collection(resource_class, options = {})
assoc_name ||= options[:joined_on].reflect_on_all_associations.find { |a| a.klass.base_class == resource_class.base_class }.name

scope = options[:joined_on].joins(assoc_name => scope_components[:includes]).readonly(false)

# allow user to add their own further clauses
scope_components[:where] << options[:where] if options[:where].present?

# apply every where clause to the scope consecutively
scope_components[:where].inject(scope) do |scope_build,where|
where.is_a?(Hash) ? scope_build.where(resource_class.table_name => where) : scope_build.where(where)
Expand All @@ -70,7 +74,7 @@ def build_filtered_scope_components(resource_class, all_filters, options)
if all_filters.empty? || (!@base_collection.nil? && @base_collection.empty?)
Foreman::Logging.logger('permissions').debug 'no filters found for given permission' if all_filters.empty?
Foreman::Logging.logger('permissions').debug 'base collection of objects is empty' if !@base_collection.nil? && @base_collection.empty?
result[:where] << '1=0'
result[:where] << (user.admin? ? '1=1' : '1=0')
return result
end

Expand Down Expand Up @@ -116,8 +120,8 @@ def allowed_taxonomies(resource_class, type)
taxonomy_ids = []
if resource_class.respond_to?("used_#{type}_ids")
taxonomy_ids = resource_class.send("used_#{type}_ids")
if taxonomy_ids.empty? && !User.current.try(:admin?)
taxonomy_ids = User.current.try("#{type}_ids")
if taxonomy_ids.empty? && !user.try(:admin?)
taxonomy_ids = user.try("#{type}_ids")
end
end
taxonomy_ids
Expand Down
2 changes: 1 addition & 1 deletion test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def as_admin(&block)
end

def in_taxonomy(taxonomy)
new_taxonomy = taxonomies(taxonomy)
new_taxonomy = taxonomy.is_a?(Taxonomy) ? taxonomy : taxonomies(taxonomy)
saved_taxonomy = new_taxonomy.class.current
new_taxonomy.class.current = new_taxonomy
result = yield
Expand Down
23 changes: 23 additions & 0 deletions test/unit/authorizer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,15 @@ def setup
refute results.grep(host).first.readonly?
end

test "#find_collection(Host, :permission => :view_hosts, :joined_on: Report) for admin" do
host = FactoryGirl.create(:host)
report = FactoryGirl.create(:report, :host => host)
@user.update_attribute(:admin, true)
auth = Authorizer.new(@user)

assert_includes auth.find_collection(Host::Managed, :permission => :view_hosts, :joined_on => Report), report
end

test "#find_collection(Host, :permission => :view_hosts, :joined_on: Report) for matching unlimited filter" do
permission = Permission.find_by_name('view_hosts')
FactoryGirl.create(:filter, :role => @role, :permissions => [permission], :unlimited => true)
Expand Down Expand Up @@ -290,4 +299,18 @@ def setup
refute_includes collection, report1
assert_includes collection, report2
end

test "#find_collection(Host, :permission => :view_hosts, :joined_on: Report, :where => ..) applies where clause" do
permission = Permission.find_by_name('view_hosts')
FactoryGirl.create(:filter, :role => @role, :permissions => [permission], :unlimited => true)
hosts = FactoryGirl.create_pair(:host)
report1 = FactoryGirl.create(:report, :host => hosts.first)
report2 = FactoryGirl.create(:report, :host => hosts.last)
auth = Authorizer.new(@user)

collection = auth.find_collection(Host::Managed, :permission => :view_hosts, :joined_on => Report,
:where => {'name' => hosts.first.name})
assert_includes collection, report1
refute_includes collection, report2
end
end
40 changes: 40 additions & 0 deletions test/unit/fact_value_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,5 +94,45 @@ def setup
assert_equal target_host.fact_values.map(&:id).sort, FactValue.my_facts.map(&:id).sort
end
end

test "only return facts from host in user's taxonomies" do
user_role = FactoryGirl.create(:user_user_role)
FactoryGirl.create(:filter, :role => user_role.role, :permissions => Permission.where(:name => 'view_hosts'), :search => "hostgroup_id = #{target_host.hostgroup_id}")

orgs = FactoryGirl.create_pair(:organization)
locs = FactoryGirl.create_pair(:location)
target_host.update_attributes(:location => locs.last, :organization => orgs.last)

user_role.owner.update_attributes(:locations => [locs.first], :organizations => [orgs.first])
as_user user_role.owner do
assert_equal [], FactValue.my_facts.map(&:id).sort
end

user_role.owner.update_attributes(:locations => [locs.last], :organizations => [orgs.last])
as_user user_role.owner do
assert_equal target_host.fact_values.map(&:id).sort, FactValue.my_facts.map(&:id).sort
end
end

test "only return facts from host in admin's currently selected taxonomy" do
user = as_admin { FactoryGirl.create(:user, :admin) }
orgs = FactoryGirl.create_pair(:organization)
locs = FactoryGirl.create_pair(:location)
target_host.update_attributes(:location => locs.last, :organization => orgs.last)

as_user user do
in_taxonomy(orgs.first) do
in_taxonomy(locs.first) do
refute_includes FactValue.my_facts, target_host.fact_values.first
end
end

in_taxonomy(orgs.last) do
in_taxonomy(locs.last) do
assert_includes FactValue.my_facts, target_host.fact_values.first
end
end
end
end
end
end
40 changes: 40 additions & 0 deletions test/unit/report_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,5 +106,45 @@ def setup
assert_equal @target_reports.map(&:id).sort, Report.my_reports.map(&:id).sort
end
end

test "only return reports from host in user's taxonomies" do
user_role = FactoryGirl.create(:user_user_role)
FactoryGirl.create(:filter, :role => user_role.role, :permissions => Permission.where(:name => 'view_hosts'), :search => "hostgroup_id = #{@target_host.hostgroup_id}")

orgs = FactoryGirl.create_pair(:organization)
locs = FactoryGirl.create_pair(:location)
@target_host.update_attributes(:location => locs.last, :organization => orgs.last)

user_role.owner.update_attributes(:locations => [locs.first], :organizations => [orgs.first])
as_user user_role.owner do
assert_equal [], Report.my_reports.map(&:id).sort
end

user_role.owner.update_attributes(:locations => [locs.last], :organizations => [orgs.last])
as_user user_role.owner do
assert_equal @target_reports.map(&:id).sort, Report.my_reports.map(&:id).sort
end
end

test "only return reports from host in admin's currently selected taxonomy" do
user = FactoryGirl.create(:user, :admin)
orgs = FactoryGirl.create_pair(:organization)
locs = FactoryGirl.create_pair(:location)
@target_host.update_attributes(:location => locs.last, :organization => orgs.last)

as_user user do
in_taxonomy(orgs.first) do
in_taxonomy(locs.first) do
refute_includes Report.my_reports, @target_reports.first
end
end

in_taxonomy(orgs.last) do
in_taxonomy(locs.last) do
assert_includes Report.my_reports, @target_reports.first
end
end
end
end
end
end

0 comments on commit 9926db4

Please sign in to comment.