From 6b7a5f5c6cfd2e24120c2ec5353f5f0e13354f27 Mon Sep 17 00:00:00 2001 From: lpichler Date: Thu, 23 Feb 2017 13:57:08 +0100 Subject: [PATCH 1/7] Pass user or group to the rss generator to be able use it for RBAC --- app/models/miq_widget/rss_content.rb | 8 ++++---- app/models/rss_feed.rb | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/models/miq_widget/rss_content.rb b/app/models/miq_widget/rss_content.rb index 84d5b58417c..74aeb126957 100644 --- a/app/models/miq_widget/rss_content.rb +++ b/app/models/miq_widget/rss_content.rb @@ -9,22 +9,22 @@ def external? resource.nil? end - def generate(_user_or_group) + def generate(user_or_group) opts = {:tz => timezone} data = if external? opts[:limit_to_count] = widget_options[:row_count] || 5 external_feed else - internal_feed + internal_feed(user_or_group) end RssFeed.to_html(data, opts) end - def internal_feed + def internal_feed(user_or_group) resource.options[:limit_to_count] = widget_options[:row_count] || 5 - SimpleRSS.parse(resource.generate) + SimpleRSS.parse(resource.generate(nil, nil, nil, user_or_group)) end def external_feed diff --git a/app/models/rss_feed.rb b/app/models/rss_feed.rb index 58a5ff834e7..4ee3192b080 100644 --- a/app/models/rss_feed.rb +++ b/app/models/rss_feed.rb @@ -17,7 +17,7 @@ def url(host = nil) "#{host_url}#{link}" end - def generate(host = nil, local = false, proto = nil) + def generate(host = nil, local = false, proto = nil, user_or_group = nil) proto ||= VMDB::Config.new("vmdb").config[:webservices][:consume_protocol] host_url = host.nil? ? "#{proto}://localhost:3000" : "#{proto}://" + host From 5e979f61e5395e543b451ad6551da4e499c4fb8a Mon Sep 17 00:00:00 2001 From: lpichler Date: Thu, 23 Feb 2017 13:58:37 +0100 Subject: [PATCH 2/7] Add RBAC call for items in RSS Feed and process user or group as parameter for RBAC --- app/models/rss_feed.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/models/rss_feed.rb b/app/models/rss_feed.rb index 4ee3192b080..9bc6744fec4 100644 --- a/app/models/rss_feed.rb +++ b/app/models/rss_feed.rb @@ -35,7 +35,14 @@ def generate(host = nil, local = false, proto = nil, user_or_group = nil) } } - feed = Rss.rss_feed_for(find_items, options) + rbac_options = {} + if user_or_group + user_or_group_key = user_or_group.kind_of?(User) ? :user : :miq_group + rbac_options[user_or_group_key] = user_or_group + end + + filtered_items = Rbac.filtered(find_items, rbac_options) + feed = Rss.rss_feed_for(filtered_items, options) local ? feed : {:text => feed, :content_type => Mime[:rss]} end From ff4229b35c98a23d22697b3ded722b6b41dcdbea Mon Sep 17 00:00:00 2001 From: lpichler Date: Thu, 23 Feb 2017 14:06:30 +0100 Subject: [PATCH 3/7] Add spec for RSS feed generation --- spec/models/rss_feed/rss_feed_spec.rb | 45 +++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/spec/models/rss_feed/rss_feed_spec.rb b/spec/models/rss_feed/rss_feed_spec.rb index adfd510ed27..ab0a501f883 100644 --- a/spec/models/rss_feed/rss_feed_spec.rb +++ b/spec/models/rss_feed/rss_feed_spec.rb @@ -3,6 +3,51 @@ before(:each) { Kernel.silence_warnings { RssFeed.const_set(:YML_DIR, Y_DIR) } } + context "with vms" do + before do + Tenant.seed + allow(User).to receive_messages(:server_timezone => "UTC") + RssFeed.sync_from_yml_file("newest_vms") + end + + let(:owner_tenant) { FactoryGirl.create(:tenant) } + let(:owner_group) { FactoryGirl.create(:miq_group, :tenant => owner_tenant) } + let(:owner_user) { FactoryGirl.create(:user, :miq_groups => [owner_group]) } + let!(:owned_vm) { FactoryGirl.create(:vm_vmware, :tenant => owner_tenant) } + let!(:tenant_root_vm) { FactoryGirl.create(:vm_vmware, :tenant => Tenant.root_tenant) } + let(:rss_feed) { RssFeed.find_by(:name => "newest_vms") } + + it "#generate 1 vms with owner_tenant tenant in newest_vms rss" do + [owner_group, owner_user].each do |user_or_group| + User.with_user(owner_user) do + feed_container = rss_feed.generate(nil, nil, nil, user_or_group) + + expect(feed_container[:text]).to eq <<-EOXML + + + + Recently Discovered VMs + https://localhost:3000/alert/rss?feed=newest_vms + Virtual machines added + en-us + 40 + + #{owned_vm.name} - location unknown + #{owned_vm.name} is a #{owned_vm.vendor_display} VM located at "#{owned_vm.location}" + #{owned_vm.created_on.rfc2822} + https://localhost:3000/vm/show/#{owned_vm.id} + https://localhost:3000/vm/show/#{owned_vm.id} + + + + EOXML + + expect(feed_container[:content_type]).to eq('application/rss+xml') + end + end + end + end + context "with 2 hosts" do before(:each) do @host1 = FactoryGirl.create(:host, :created_on => Time.utc(2013, 1, 1, 0, 0, 0)) From 2245103d0104e482761aa0c39e446e4e306f5ca6 Mon Sep 17 00:00:00 2001 From: lpichler Date: Thu, 23 Feb 2017 14:24:59 +0100 Subject: [PATCH 4/7] Fix order clause to determine order column from concrete table otherwise it leads to error ambiguous column when RBAC was added. --- product/alerts/rss/newest_vms.yml | 2 +- spec/models/rss_feed/data/newest_vms.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/product/alerts/rss/newest_vms.yml b/product/alerts/rss/newest_vms.yml index dbda1b0fad6..9ffd35f48df 100644 --- a/product/alerts/rss/newest_vms.yml +++ b/product/alerts/rss/newest_vms.yml @@ -27,7 +27,7 @@ item_class: Vm search_method: limit_to_time: limit_to_count: -orderby: "created_on DESC" +orderby: "vms.created_on DESC" # Included tables and columns for query performance include: diff --git a/spec/models/rss_feed/data/newest_vms.yml b/spec/models/rss_feed/data/newest_vms.yml index dbda1b0fad6..9ffd35f48df 100644 --- a/spec/models/rss_feed/data/newest_vms.yml +++ b/spec/models/rss_feed/data/newest_vms.yml @@ -27,7 +27,7 @@ item_class: Vm search_method: limit_to_time: limit_to_count: -orderby: "created_on DESC" +orderby: "vms.created_on DESC" # Included tables and columns for query performance include: From c373db4c3506d471078e1a63119159a971175473 Mon Sep 17 00:00:00 2001 From: lpichler Date: Tue, 14 Mar 2017 08:57:08 +0100 Subject: [PATCH 5/7] Add scoping to tenant for resources in performance reports - visible ids of resource class are determined for tenant - those ids are used for filtering as well --- app/models/rbac.rb | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/app/models/rbac.rb b/app/models/rbac.rb index 23d37bb0cb0..7a54d7d9728 100644 --- a/app/models/rbac.rb +++ b/app/models/rbac.rb @@ -134,7 +134,7 @@ def self.get_self_service_objects(user, miq_group, klass) klass.user_or_group_owned(user, miq_group).except(:order) end - def self.calc_filtered_ids(scope, user_filters, user, miq_group) + def self.calc_filtered_ids(scope, user_filters, user, miq_group, scope_tenant_filter) klass = scope.respond_to?(:klass) ? scope.klass : scope u_filtered_ids = pluck_ids(get_self_service_objects(user, miq_group, klass)) b_filtered_ids = get_belongsto_filter_object_ids(klass, user_filters['belongsto']) @@ -142,13 +142,14 @@ def self.calc_filtered_ids(scope, user_filters, user, miq_group) d_filtered_ids = pluck_ids(matches_via_descendants(rbac_class(klass), user_filters['match_via_descendants'], :user => user, :miq_group => miq_group)) - filtered_ids = combine_filtered_ids(u_filtered_ids, b_filtered_ids, m_filtered_ids, d_filtered_ids) + filtered_ids = combine_filtered_ids(u_filtered_ids, b_filtered_ids, m_filtered_ids, d_filtered_ids, + scope_tenant_filter.try(:ids)) [filtered_ids, u_filtered_ids] end # # Algorithm: filter = u_filtered_ids UNION (b_filtered_ids INTERSECTION m_filtered_ids) - # filter = filter UNION d_filtered_ids if filter is not nil + # filter = (filter UNION d_filtered_ids if filter is not nil) UNION tenant_filter_ids # # a nil as input for any field means it does not apply # a nil as output means there is not filter @@ -159,7 +160,7 @@ def self.calc_filtered_ids(scope, user_filters, user, miq_group) # @param d_filtered_ids [nil|Array] ids from descendants # @return nil if filters do not aply # @return [Array] target ids for filter - def self.combine_filtered_ids(u_filtered_ids, b_filtered_ids, m_filtered_ids, d_filtered_ids) + def self.combine_filtered_ids(u_filtered_ids, b_filtered_ids, m_filtered_ids, d_filtered_ids, tenant_filter_ids) filtered_ids = if b_filtered_ids.nil? m_filtered_ids @@ -179,12 +180,27 @@ def self.combine_filtered_ids(u_filtered_ids, b_filtered_ids, m_filtered_ids, d_ filtered_ids.uniq! end - filtered_ids + if filtered_ids.kind_of?(Array) + filtered_ids | tenant_filter_ids.to_a + elsif filtered_ids.nil? && tenant_filter_ids.kind_of?(Array) && tenant_filter_ids.present? + tenant_filter_ids + end + end + + def self.scope_to_tenant(scope, user, miq_group) + klass = scope.respond_to?(:klass) ? scope.klass : scope + user_or_group = user || miq_group + tenant_id_clause = klass.tenant_id_clause(user_or_group) + + tenant_id_clause ? scope.where(tenant_id_clause) : scope end def self.find_targets_with_indirect_rbac(scope, rbac_filters, find_options, user, miq_group) parent_class = rbac_class(scope) - filtered_ids, _ = calc_filtered_ids(parent_class, rbac_filters, user, miq_group) + if parent_class.try(:scope_by_tenant?) + scope_tenant_filter = scope_to_tenant(parent_class, user, miq_group) + end + filtered_ids, _ = calc_filtered_ids(parent_class, rbac_filters, user, miq_group, scope_tenant_filter) find_targets_filtered_by_parent_ids(parent_class, scope, find_options, filtered_ids) end @@ -252,7 +268,7 @@ def self.get_managed_filter_object_ids(scope, filter) end def self.find_targets_with_direct_rbac(scope, rbac_filters, find_options, user, miq_group) - filtered_ids, u_filtered_ids = calc_filtered_ids(scope, rbac_filters, user, miq_group) + filtered_ids, u_filtered_ids = calc_filtered_ids(scope, rbac_filters, user, miq_group, nil) find_targets_filtered_by_ids(scope, find_options, u_filtered_ids, filtered_ids) end From 6921903d5abbba365f78ba4c8e0f736d01a2877b Mon Sep 17 00:00:00 2001 From: lpichler Date: Thu, 2 Mar 2017 15:06:59 +0100 Subject: [PATCH 6/7] Add factory for VmPerfomance --- spec/factories/vm_performance.rb | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 spec/factories/vm_performance.rb diff --git a/spec/factories/vm_performance.rb b/spec/factories/vm_performance.rb new file mode 100644 index 00000000000..3453e2c6430 --- /dev/null +++ b/spec/factories/vm_performance.rb @@ -0,0 +1,5 @@ +FactoryGirl.define do + factory :vm_performance do + timestamp { Time.now.utc } + end +end From e01041f5994b1c0c9e300f1a84de099cff898197 Mon Sep 17 00:00:00 2001 From: lpichler Date: Tue, 14 Mar 2017 10:51:26 +0100 Subject: [PATCH 7/7] Specs for tenant scoping for VmPerformance --- spec/models/rbac_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/spec/models/rbac_spec.rb b/spec/models/rbac_spec.rb index 7541fae3c4b..ea2f56832bf 100644 --- a/spec/models/rbac_spec.rb +++ b/spec/models/rbac_spec.rb @@ -156,6 +156,22 @@ let(:child_tenant) { FactoryGirl.create(:tenant, :divisible => false, :parent => owner_tenant) } let(:child_group) { FactoryGirl.create(:miq_group, :tenant => child_tenant) } + context 'with Vm as resource of VmPerformance model' do + let!(:root_tenant_vm) { FactoryGirl.create(:vm_vmware, :tenant => Tenant.root_tenant) } + let!(:vm_performance_root_tenant) { FactoryGirl.create(:vm_performance, :resource => root_tenant_vm) } + let!(:vm_performance_other_tenant) { FactoryGirl.create(:vm_performance, :resource => other_vm) } + + it 'list only other_user\'s VmPerformances' do + results = described_class.search(:class => VmPerformance, :user => other_user).first + expect(results).to match_array [vm_performance_other_tenant.id] + end + + it 'list all VmPerformances' do + results = described_class.search(:class => VmPerformance, :user => admin_user).first + expect(results).to match_array [vm_performance_other_tenant.id, vm_performance_root_tenant.id] + end + end + context "searching MiqTemplate" do it "can't see descendant tenant's templates" do owned_template.update_attributes!(:tenant_id => child_tenant.id, :miq_group_id => child_group.id)