Merge pull request #16178 from gtanzillo/rbac-sti-base-class

Ensure that `base_class` of first target is used for RBAC scope
(cherry picked from commit b02bfce)

https://bugzilla.redhat.com/show_bug.cgi?id=1511135
https://bugzilla.redhat.com/show_bug.cgi?id=1511130

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)

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