diff --git a/lib/rbac/filterer.rb b/lib/rbac/filterer.rb index 7d304f58c00..0f36dcb42df 100644 --- a/lib/rbac/filterer.rb +++ b/lib/rbac/filterer.rb @@ -212,6 +212,7 @@ def search(options = {}) target_ids = targets.collect(&:id) klass = targets.first.class klass = base_class if !klass.respond_to?(:find) && (base_class = rbac_base_class(klass)) + klass = safe_base_class(klass) if is_sti?(klass) # always scope to base class if model is STI end scope = apply_scope(klass, scope) scope = apply_select(klass, scope, options[:extra_cols]) if options[:extra_cols] @@ -273,6 +274,10 @@ def search(options = {}) return targets, attrs end + def is_sti?(klass) + klass.respond_to?(:finder_needs_type_condition?) ? klass.finder_needs_type_condition? : false + end + def include_references(scope, klass, include_for_find, exp_includes) ref_includes = Hash(include_for_find).merge(Hash(exp_includes)) unless polymorphic_include?(klass, ref_includes) diff --git a/spec/lib/rbac/filterer_spec.rb b/spec/lib/rbac/filterer_spec.rb index 704ba39fbf4..ed0d77696b9 100644 --- a/spec/lib/rbac/filterer_spec.rb +++ b/spec/lib/rbac/filterer_spec.rb @@ -71,15 +71,44 @@ end context 'when class does not participate in RBAC' do + before do + @vm = FactoryGirl.create(:vm_vmware, :name => "VM1", :host => @host1, :ext_management_system => @ems) + ["2010-04-14T20:52:30Z", "2010-04-14T21:51:10Z"].each do |t| + @vm.metric_rollups << FactoryGirl.create(:metric_rollup_vm_hr, :timestamp => t) + end + end let(:miq_ae_domain) { FactoryGirl.create(:miq_ae_domain) } - it 'returns same class as input' do + it 'returns the same class as input for MiqAeDomain' do User.with_user(admin_user) do results = described_class.search(:targets => [miq_ae_domain]).first expect(results.first).to be_an_instance_of(MiqAeDomain) expect(results).to match_array [miq_ae_domain] end end + + it 'returns the same class as input for parent class that is not STI' do + User.with_user(admin_user) do + targets = @vm.metric_rollups + + results = described_class.search(:targets => targets, :user => admin_user) + objects = results.first + expect(objects.length).to eq(2) + expect(objects).to match_array(targets) + end + end + + it 'returns the same class as input for subclass that is not STI' do + User.with_user(admin_user) do + vm_perf = VmPerformance.find(@vm.metric_rollups.last.id) + targets = [vm_perf] + + results = described_class.search(:targets => targets, :user => admin_user) + objects = results.first + expect(objects.length).to eq(1) + expect(objects).to match_array(targets) + end + end end describe "with find_options_for_tenant filtering" do @@ -659,6 +688,18 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects) expect(objects).to eq(targets) end + it "returns the correct class for different classes of targets" do + @ems3 = FactoryGirl.create(:ems_vmware, :name => 'ems3') + @ems4 = FactoryGirl.create(:ems_microsoft, :name => 'ems4') + + targets = [@ems2, @ems4, @ems3, @ems] + + results = described_class.search(:targets => targets, :user => user) + objects = results.first + expect(objects.length).to eq(4) + expect(objects).to match_array(targets) + end + it "finds both EMSes without belongsto filters" do results = described_class.search(:class => "ExtManagementSystem", :user => user) objects = results.first