From 119d2691ca63f5730e7f7cfa37b8b7c029e7fa2a Mon Sep 17 00:00:00 2001 From: Gregg Tanzillo Date: Wed, 10 Oct 2018 14:08:21 -0400 Subject: [PATCH] Merge pull request #17851 from alexander-demichev/named-scopes-templates Add tenant filtering for templates in provisioning and summary pages (cherry picked from commit 142a184d4619fd9d3ecebb6cff71ccd13667832e) https://bugzilla.redhat.com/show_bug.cgi?id=1623561 --- .../providers/cloud_manager/template.rb | 8 +- spec/lib/rbac/filterer_spec.rb | 78 +++++++++++++++---- .../providers/cloud_manager/template_spec.rb | 9 ++- 3 files changed, 75 insertions(+), 20 deletions(-) diff --git a/app/models/manageiq/providers/cloud_manager/template.rb b/app/models/manageiq/providers/cloud_manager/template.rb index cad90fa6699..446b720e7a8 100644 --- a/app/models/manageiq/providers/cloud_manager/template.rb +++ b/app/models/manageiq/providers/cloud_manager/template.rb @@ -22,9 +22,13 @@ def self.eligible_for_provisioning def self.tenant_id_clause(user_or_group) template_tenant_ids = MiqTemplate.accessible_tenant_ids(user_or_group, Rbac.accessible_tenant_ids_strategy(self)) - return if template_tenant_ids.empty? + tenant = user_or_group.current_tenant - ["(vms.template = true AND (vms.tenant_id IN (?) OR vms.publicly_available = true))", template_tenant_ids] + 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 diff --git a/spec/lib/rbac/filterer_spec.rb b/spec/lib/rbac/filterer_spec.rb index b157725a194..4fd190dabc6 100644 --- a/spec/lib/rbac/filterer_spec.rb +++ b/spec/lib/rbac/filterer_spec.rb @@ -697,29 +697,73 @@ def combine_filtered_ids(user_filtered_ids, belongsto_filtered_ids, managed_filt let(:admin_user) { FactoryGirl.create(:user, :role => "super_administrator") } let!(:cloud_template_root) { FactoryGirl.create(:template_cloud, :publicly_available => false) } - it 'returns all cloud templates when user is admin' do - results = described_class.filtered(TemplateCloud, :user => admin_user) - expect(results).to match_array(TemplateCloud.all) + let(:tenant_2) { FactoryGirl.create(:tenant, :parent => default_tenant, :source_type => 'CloudTenant') } # T2 + let(:group_2) { FactoryGirl.create(:miq_group, :tenant => tenant_2) } # T1 + let(:user_2) { FactoryGirl.create(:user, :miq_groups => [group_2]) } + + context "when tenant is not mapped to cloud tenant" do + it 'returns all cloud templates when user is admin' do + User.current_user = admin_user + results = described_class.filtered(TemplateCloud, :user => admin_user) + expect(results).to match_array(TemplateCloud.all) + end + + 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) } + + it "returns all public cloud templates" do + User.current_user = user_2 + results = described_class.filtered(TemplateCloud, :user => user_2) + expect(results).to match_array([cloud_template, cloud_template_root]) + end + + context "should ignore other tenant's private cloud templates" do + let!(:cloud_template) { 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]) + end + end + end end - context "when user is restricted user" do - let(:tenant_2) { FactoryGirl.create(:tenant, :parent => default_tenant, :source_type => 'CloudTenant') } # T2 - let(:group_2) { FactoryGirl.create(:miq_group, :tenant => tenant_2) } # T1 - let(:user_2) { FactoryGirl.create(:user, :miq_groups => [group_2]) } - let(:tenant_3) { FactoryGirl.create(:tenant, :parent => tenant_2) } # T3 - let!(:cloud_template) { FactoryGirl.create(:template_cloud, :tenant => tenant_3, :publicly_available => true) } + context "when tenant is mapped to cloud tenant" do + let(:tenant_2) { FactoryGirl.create(:tenant, :parent => default_tenant, :source_type => 'CloudTenant', :source_id => 1) } - it "returns all public cloud templates" do + it "finds tenant's private cloud templates" do + cloud_template2 = FactoryGirl.create(:template_cloud, :tenant => tenant_2, :publicly_available => false) + User.current_user = user_2 results = described_class.filtered(TemplateCloud, :user => user_2) - expect(results).to match_array([cloud_template, cloud_template_root]) + expect(results).to match_array([cloud_template2]) end - context "should ignore" do - let!(:cloud_template) { FactoryGirl.create(:template_cloud, :tenant => tenant_3, :publicly_available => false) } - it "private cloud templates" do - results = described_class.filtered(TemplateCloud, :user => user_2) - expect(results).to match_array([cloud_template_root]) - end + it "finds tenant's private and public cloud templates" do + cloud_template2 = FactoryGirl.create(:template_cloud, :tenant => tenant_2, :publicly_available => false) + cloud_template3 = FactoryGirl.create(:template_cloud, :tenant => tenant_2, :publicly_available => true) + User.current_user = user_2 + results = described_class.filtered(TemplateCloud, :user => user_2) + expect(results).to match_array([cloud_template2, cloud_template3]) + end + + it "ignores other tenant's private templates" do + cloud_template2 = FactoryGirl.create(:template_cloud, :tenant => tenant_2, :publicly_available => false) + cloud_template3 = FactoryGirl.create(:template_cloud, :tenant => tenant_2, :publicly_available => true) + FactoryGirl.create(:template_cloud, :tenant => default_tenant, :publicly_available => false) + User.current_user = user_2 + results = described_class.filtered(TemplateCloud, :user => user_2) + expect(results).to match_array([cloud_template2, cloud_template3]) + end + + it "finds other tenant's public templates" do + cloud_template2 = FactoryGirl.create(:template_cloud, :tenant => tenant_2, :publicly_available => false) + cloud_template3 = FactoryGirl.create(:template_cloud, :tenant => tenant_2, :publicly_available => true) + cloud_template4 = FactoryGirl.create(:template_cloud, :tenant => default_tenant, :publicly_available => true) + FactoryGirl.create(:template_cloud, :tenant => default_tenant, :publicly_available => false) + User.current_user = user_2 + results = described_class.filtered(TemplateCloud, :user => user_2) + expect(results).to match_array([cloud_template2, cloud_template3, cloud_template4]) end end end diff --git a/spec/models/manageiq/providers/cloud_manager/template_spec.rb b/spec/models/manageiq/providers/cloud_manager/template_spec.rb index 426ef09c2ad..414c254e55c 100644 --- a/spec/models/manageiq/providers/cloud_manager/template_spec.rb +++ b/spec/models/manageiq/providers/cloud_manager/template_spec.rb @@ -25,8 +25,15 @@ 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 for regular tenants" do + 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