Fix rbac call for templates and vms

app/models/manageiq/providers/cloud_manager/template.rb

@@ -122,17 +122,6 @@ def self.display_name(number = 1)
     n_('Image', 'Images', number)
   end
 
-  def self.tenant_id_clause(user_or_group)
-    template_tenant_ids = MiqTemplate.accessible_tenant_ids(user_or_group, Rbac.accessible_tenant_ids_strategy(self))
-    tenant = user_or_group.current_tenant
-
-    if tenant.source_id
-      ["(vms.template = true AND (vms.tenant_id = (?) AND vms.publicly_available = false OR vms.publicly_available = true))", tenant.id]
-    else
-      ["(vms.template = true AND (vms.tenant_id IN (?) OR vms.publicly_available = true))", template_tenant_ids] unless template_tenant_ids.empty?
-    end
-  end
-
   private
 
   def raise_created_event

app/models/vm_or_template.rb

@@ -1752,8 +1752,19 @@ def self.tenant_id_clause(user_or_group)
     vm_tenant_ids       = Vm.accessible_tenant_ids(user_or_group, Rbac.accessible_tenant_ids_strategy(Vm))
     return if template_tenant_ids.empty? && vm_tenant_ids.empty?
 
-    ["(vms.template = true AND vms.tenant_id IN (?)) OR (vms.template = false AND vms.tenant_id IN (?))",
-     template_tenant_ids, vm_tenant_ids]
+    tenant = user_or_group.current_tenant
+    tenant_vms       = "vms.template = false AND vms.tenant_id IN (?)"
+    public_templates = "vms.template = true AND vms.publicly_available = true"
+
+    if tenant.source_id
+      private_tenant_templates = "vms.template = true AND vms.tenant_id = (?) AND vms.publicly_available = false"
+      ["#{private_tenant_templates} OR #{public_templates} OR #{tenant_vms}", tenant.id, vm_tenant_ids]
+    elsif template_tenant_ids.empty?
+      [tenant_vms, vm_tenant_ids]
+    else
+      tenant_templates = "vms.template = true AND vms.tenant_id IN (?)"
+      ["#{tenant_templates} OR #{public_templates} OR #{tenant_vms}", template_tenant_ids, vm_tenant_ids]
+    end
   end
 
   def self.with_ownership

spec/factories/vm_or_template.rb

@@ -31,6 +31,10 @@
     vendor "openstack"
   end
 
+  factory :volume_template_openstack, :class => "ManageIQ::Providers::Openstack::CloudManager::VolumeTemplate", :parent => :template_cloud do
+    vendor "openstack"
+  end
+
   factory :miq_template do
     name "ubuntu-16.04-stable"
     location "Minneapolis, MN"

spec/lib/rbac/filterer_spec.rb

@@ -815,19 +815,25 @@ def combine_filtered_ids(user_filtered_ids, belongsto_filtered_ids, managed_filt
             context "when user is restricted user" do
               let(:tenant_3) { FactoryGirl.create(:tenant, :parent => tenant_2) } # T3
               let!(:cloud_template) { FactoryGirl.create(:template_cloud, :tenant => tenant_3, :publicly_available => true) }
+              let!(:volume_template_openstack_1) { FactoryGirl.create(:template_cloud, :tenant => tenant_3, :publicly_available => true) }
 
-              it "returns all public cloud templates" do
+              it "returns all public cloud templates and its descendants" do
                 User.current_user = user_2
+
+                results = described_class.filtered(VmOrTemplate, :user => user_2)
+                expect(results).to match_array([cloud_template, cloud_template_root, volume_template_openstack_1])
+
                 results = described_class.filtered(TemplateCloud, :user => user_2)
-                expect(results).to match_array([cloud_template, cloud_template_root])
+                expect(results).to match_array([cloud_template, cloud_template_root, volume_template_openstack_1])
               end
 
               context "should ignore other tenant's private cloud templates" do
                 let!(:cloud_template) { FactoryGirl.create(:template_cloud, :tenant => tenant_3, :publicly_available => false) }
+                let!(:volume_template_openstack_2) { FactoryGirl.create(:template_cloud, :tenant => tenant_3, :publicly_available => false) }
                 it "returns public templates" do
                   User.current_user = user_2
                   results = described_class.filtered(TemplateCloud, :user => user_2)
-                  expect(results).to match_array([cloud_template_root])
+                  expect(results).to match_array([cloud_template_root, volume_template_openstack_1])
                 end
               end
             end

spec/models/manageiq/providers/cloud_manager/template_spec.rb

@@ -8,32 +8,4 @@
       subject.post_create_actions
     end
   end
-
-  let(:root_tenant) do
-    Tenant.seed
-  end
-
-  let(:default_tenant) do
-    root_tenant
-    Tenant.default_tenant
-  end
-
-  describe "miq_group" do
-    let(:user)         { FactoryGirl.create(:user, :userid => 'user', :miq_groups => [tenant_group]) }
-    let(:tenant)       { FactoryGirl.build(:tenant, :parent => default_tenant) }
-    let(:tenant_users) { FactoryGirl.create(:miq_user_role, :name => "tenant-users") }
-    let(:tenant_group) { FactoryGirl.create(:miq_group, :miq_user_role => tenant_users, :tenant => tenant) }
-    let(:cloud_template_1) { FactoryGirl.create(:class => "TemplateCloud") }
-
-    it "finds correct tenant id clause when tenant doesn't have source_id" do
-      User.current_user = user
-      expect(TemplateCloud.tenant_id_clause(user)).to eql ["(vms.template = true AND (vms.tenant_id IN (?) OR vms.publicly_available = true))", [default_tenant.id, tenant.id]]
-    end
-
-    it "finds correct tenant id clause when tenant has source_id" do
-      User.current_user = user
-      tenant.source_id = 1
-      expect(TemplateCloud.tenant_id_clause(user)).to eql ["(vms.template = true AND (vms.tenant_id = (?) AND vms.publicly_available = false OR vms.publicly_available = true))", tenant.id]
-    end
-  end
 end

spec/models/manageiq/providers/cloud_manager/vm_or_template_spec.rb

@@ -25,4 +25,32 @@
       expect(described_class.archived).to match_array([vm, t])
     end
   end
+
+  let(:root_tenant) do
+    Tenant.seed
+  end
+
+  let(:default_tenant) do
+    root_tenant
+    Tenant.default_tenant
+  end
+
+  describe "miq_group" do
+    let(:user)         { FactoryGirl.create(:user, :userid => 'user', :miq_groups => [tenant_group]) }
+    let(:tenant)       { FactoryGirl.build(:tenant, :parent => default_tenant) }
+    let(:tenant_users) { FactoryGirl.create(:miq_user_role, :name => "tenant-users") }
+    let(:tenant_group) { FactoryGirl.create(:miq_group, :miq_user_role => tenant_users, :tenant => tenant) }
+    let(:cloud_template_1) { FactoryGirl.create(:class => "TemplateCloud") }
+
+    it "finds correct tenant id clause when tenant has source_id" do
+      User.current_user = user
+      tenant.source_id = 1
+      expect(VmOrTemplate.tenant_id_clause(user)).to eql ["(vms.template = true AND (vms.tenant_id = (?) AND vms.publicly_available = false OR vms.publicly_available = true)) OR (vms.template = false AND vms.tenant_id IN (?))", tenant.id, [tenant.id]]
+    end
+
+    it "finds correct tenant id clause when tenant doesn't have source_id" do
+      User.current_user = user
+      expect(VmOrTemplate.tenant_id_clause(user)).to eql ["(vms.template = true AND (vms.tenant_id IN (?) OR vms.publicly_available = true)) OR (vms.template = false AND vms.tenant_id IN (?))", [default_tenant.id, tenant.id], [tenant.id]]
+    end
+  end
 end

spec/models/mixins/cloud_tenancy_mixin_spec.rb

@@ -15,7 +15,7 @@
     let(:tenant_group) { FactoryGirl.create(:miq_group, :miq_user_role => tenant_users, :tenant => tenant) }
 
     it "finds correct tenant id clause for regular tenants" do
-      expect(VmOrTemplate.tenant_id_clause(user)).to eql ["(vms.template = true AND vms.tenant_id IN (?)) OR (vms.template = false AND vms.tenant_id IN (?))", [default_tenant.id, tenant.id], [tenant.id]]
+      expect(VmOrTemplate.tenant_id_clause(user)).to eql ["(vms.template = true AND (vms.tenant_id IN (?) OR vms.publicly_available = true)) OR (vms.template = false AND vms.tenant_id IN (?))", [default_tenant.id, tenant.id], [tenant.id]]
 
       query = VmOrTemplate.where(VmOrTemplate.tenant_id_clause(user))
       expect { query.load }.not_to raise_error