Skip to content

Commit

Permalink
Merge pull request #17061 from lpichler/add_tenant_to_rbac_and_fix_us…
Browse files Browse the repository at this point in the history
…er_and_miq_group

Fix RBAC for User and enable tagging for Tenants
(cherry picked from commit d31d9d7)

https://bugzilla.redhat.com/show_bug.cgi?id=1553776
https://bugzilla.redhat.com/show_bug.cgi?id=1553779
  • Loading branch information
gtanzillo authored and simaishi committed Mar 9, 2018
1 parent 8dae522 commit 37471e6
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 21 deletions.
4 changes: 2 additions & 2 deletions app/models/miq_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 3 additions & 2 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion lib/rbac/filterer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
42 changes: 28 additions & 14 deletions spec/lib/rbac/filterer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}",
Expand All @@ -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
Expand All @@ -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

Expand Down
6 changes: 4 additions & 2 deletions spec/lib/services/resource_sharer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 37471e6

Please sign in to comment.