From 37471e6278ae5e05568fb6902b64f965ab316fac Mon Sep 17 00:00:00 2001 From: Gregg Tanzillo Date: Wed, 7 Mar 2018 09:29:48 -0500 Subject: [PATCH] Merge pull request #17061 from lpichler/add_tenant_to_rbac_and_fix_user_and_miq_group Fix RBAC for User and enable tagging for Tenants (cherry picked from commit d31d9d76e40d1824f11d6d97902d24251e3c0207) https://bugzilla.redhat.com/show_bug.cgi?id=1553776 https://bugzilla.redhat.com/show_bug.cgi?id=1553779 --- app/models/miq_group.rb | 4 +-- app/models/user.rb | 5 +-- lib/rbac/filterer.rb | 6 +++- spec/lib/rbac/filterer_spec.rb | 42 +++++++++++++++-------- spec/lib/services/resource_sharer_spec.rb | 6 ++-- 5 files changed, 42 insertions(+), 21 deletions(-) diff --git a/app/models/miq_group.rb b/app/models/miq_group.rb index cec640ce867..eab91e1f5e8 100644 --- a/app/models/miq_group.rb +++ b/app/models/miq_group.rb @@ -249,8 +249,8 @@ def self.non_tenant_groups_in_my_region in_my_region.non_tenant_groups end - def self.with_current_user_groups - current_user = User.current_user + def self.with_current_user_groups(user = nil) + current_user = user || User.current_user current_user.admin_user? ? all : where(:id => current_user.miq_group_ids) end diff --git a/app/models/user.rb b/app/models/user.rb index a283299d314..4d2929f6728 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -283,8 +283,9 @@ def self.current_user Thread.current[:user] ||= find_by_userid(current_userid) end - def self.with_current_user_groups - current_user.admin_user? ? all : includes(:miq_groups).where(:miq_groups => {:id => current_user.miq_group_ids}) + def self.with_current_user_groups(user = nil) + user ||= current_user + user.admin_user? ? all : includes(:miq_groups).where(:miq_groups => {:id => user.miq_group_ids}) end def self.missing_user_features(db_user) diff --git a/lib/rbac/filterer.rb b/lib/rbac/filterer.rb index c7fa4537b80..d5ad0e56386 100644 --- a/lib/rbac/filterer.rb +++ b/lib/rbac/filterer.rb @@ -53,7 +53,7 @@ class Filterer VmOrTemplate ) - TAGGABLE_FILTER_CLASSES = CLASSES_THAT_PARTICIPATE_IN_RBAC - %w(EmsFolder) + %w(MiqGroup User) + TAGGABLE_FILTER_CLASSES = CLASSES_THAT_PARTICIPATE_IN_RBAC - %w(EmsFolder) + %w(MiqGroup User Tenant) NETWORK_MODELS_FOR_BELONGSTO_FILTER = %w( CloudNetwork @@ -464,6 +464,7 @@ def scope_for_user_role_group(klass, scope, miq_group, user, managed_filters) if MiqUserRole != klass filtered_ids = pluck_ids(get_managed_filter_object_ids(scope, managed_filters)) + scope = scope.with_current_user_groups(user) end scope_by_ids(scope, filtered_ids) @@ -499,6 +500,9 @@ def scope_targets(klass, scope, rbac_filters, user, miq_group) scope_by_parent_ids(associated_class, scope, filtered_ids) elsif [MiqUserRole, MiqGroup, User].include?(klass) scope_for_user_role_group(klass, scope, miq_group, user, rbac_filters['managed']) + elsif klass == Tenant + filtered_ids = pluck_ids(get_managed_filter_object_ids(scope, rbac_filters['managed'])) + scope_by_ids(scope, filtered_ids) else scope end diff --git a/spec/lib/rbac/filterer_spec.rb b/spec/lib/rbac/filterer_spec.rb index d568bad42a4..b34bde5d2c5 100644 --- a/spec/lib/rbac/filterer_spec.rb +++ b/spec/lib/rbac/filterer_spec.rb @@ -169,6 +169,17 @@ def combine_filtered_ids(user_filtered_ids, belongsto_filtered_ids, managed_filt expect(results).to match_array [host_aggregate] end end + + context "searching for tenants" do + before do + owner_tenant.tag_with('/managed/environment/prod', :ns => '*') + end + + it 'list tagged tenants' do + results = described_class.search(:class => Tenant, :user => user).first + expect(results).to match_array [owner_tenant] + end + end end context 'with virtual custom attributes' do @@ -646,13 +657,14 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects) end end - it "returns all users" do + it "returns users from current user's groups" do + other_user.miq_groups << group get_rbac_results_for_and_expect_objects(User, [user, other_user]) end - it "returns all groups" do + it "returns user's groups" do _expected_groups = [group, other_group] # this will create more groups than 2 - get_rbac_results_for_and_expect_objects(MiqGroup, MiqGroup.all) + get_rbac_results_for_and_expect_objects(MiqGroup, user.miq_groups) end context "with self-service user" do @@ -1604,8 +1616,10 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects) end context "with group's VMs" do + let(:group_user) { FactoryGirl.create(:user, :miq_groups => [group2, group]) } + let(:group2) { FactoryGirl.create(:miq_group, :role => 'support') } + before(:each) do - group2 = FactoryGirl.create(:miq_group, :role => 'support') 4.times do |i| FactoryGirl.create(:vm_vmware, :name => "Test VM #{i}", @@ -1622,12 +1636,12 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects) value: connected field: MiqGroup.vms-connection_state ' - results, attrs = described_class.search(:class => "MiqGroup", - :filter => filter, - :miq_group => group) - expect(results.length).to eq(2) - expect(attrs[:auth_count]).to eq(2) + User.with_user(group_user) do + results, attrs = described_class.search(:class => "MiqGroup", :filter => filter, :miq_group => group) + expect(results.length).to eq(2) + expect(attrs[:auth_count]).to eq(2) + end end it "when filtering on a virtual column (FB15509)" do @@ -1638,11 +1652,11 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects) value: false field: MiqGroup.vms-disconnected ' - results, attrs = described_class.search(:class => "MiqGroup", - :filter => filter, - :miq_group => group) - expect(results.length).to eq(2) - expect(attrs[:auth_count]).to eq(2) + User.with_user(group_user) do + results, attrs = described_class.search(:class => "MiqGroup", :filter => filter, :miq_group => group) + expect(results.length).to eq(2) + expect(attrs[:auth_count]).to eq(2) + end end end diff --git a/spec/lib/services/resource_sharer_spec.rb b/spec/lib/services/resource_sharer_spec.rb index ae02c48b602..ac25a9cce95 100644 --- a/spec/lib/services/resource_sharer_spec.rb +++ b/spec/lib/services/resource_sharer_spec.rb @@ -78,8 +78,10 @@ let(:resource_to_be_shared) { FactoryGirl.build(:miq_group) } it "is invalid" do - expect(subject).not_to be_valid - expect(subject.errors.full_messages).to include(a_string_including("Resource is not sharable")) + User.with_user(user) do + expect(subject).not_to be_valid + expect(subject.errors.full_messages).to include(a_string_including("Resource is not sharable")) + end end end