Skip to content

Commit

Permalink
Remove Request taggable and prevent tag filtering
Browse files Browse the repository at this point in the history
MiqRequest was changed to allow ownership for self service and limited
self-service users in ManageIQ ManageIQ#17208, BZ: 1545395

This caused a problem if you had tag filters assign to a user's group
undefined method `find_tags_by_grouping'.  This was fixed in
ManageIQ ManageIQ#17466, BZ: 1576129, and shipped with:

Fine: https://bugzilla.redhat.com/show_bug.cgi?id=1583711
Gaprindindashvili: https://bugzilla.redhat.com/show_bug.cgi?id=1583710

Unfortunately, this second fix to add taggable caused a new bug: users in
groups having tag filters could not see their own requests.

This commit changes MiqRequest to no longer be taggable, since it's not
even taggable in the UI and instead, we add MiqRequest to a list of
models that are RBAC'able but not taggable so we don't try to filter
MiqRequest based on a user's group tag filters.

Credit goes to github user LorkScorguar who reported this issue and
provided lots of diagnostics to help us fix this properly.

For gaprindashvili and fine:
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1596738

For hammer:
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1576129
  • Loading branch information
jrafanie committed Jun 29, 2018
1 parent 9734478 commit 7aa3716
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 3 deletions.
2 changes: 0 additions & 2 deletions app/models/miq_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ class MiqRequest < ApplicationRecord
has_many :miq_approvals, :dependent => :destroy
has_many :miq_request_tasks, :dependent => :destroy

acts_as_miq_taggable

alias_attribute :state, :request_state

serialize :options, Hash
Expand Down
2 changes: 1 addition & 1 deletion lib/rbac/filterer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class Filterer
VmOrTemplate
)

TAGGABLE_FILTER_CLASSES = CLASSES_THAT_PARTICIPATE_IN_RBAC - %w(EmsFolder) + %w(MiqGroup User Tenant)
TAGGABLE_FILTER_CLASSES = CLASSES_THAT_PARTICIPATE_IN_RBAC - %w(EmsFolder MiqRequest) + %w(MiqGroup User Tenant)

NETWORK_MODELS_FOR_BELONGSTO_FILTER = %w(
CloudNetwork
Expand Down
41 changes: 41 additions & 0 deletions spec/lib/rbac/filterer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ def combine_filtered_ids(user_filtered_ids, belongsto_filtered_ids, managed_filt

let(:child_tenant) { FactoryGirl.create(:tenant, :divisible => false, :parent => owner_tenant) }
let(:child_group) { FactoryGirl.create(:miq_group, :tenant => child_tenant) }
let(:child_user) { FactoryGirl.create(:user, :miq_groups => [child_group]) }
let(:child_openstack_vm) { FactoryGirl.create(:vm_openstack, :tenant => child_tenant, :miq_group => child_group) }

describe ".search" do
Expand Down Expand Up @@ -619,6 +620,46 @@ def combine_filtered_ids(user_filtered_ids, belongsto_filtered_ids, managed_filt
results = described_class.search(:class => "MiqRequestTask", :miq_group => owner_group).first
expect(results).to match_array []
end
end

context "with accessible_tenant_ids filtering (strategy = :descendants_id) through" do
it "can see their own request in the same tenant" do
request = FactoryGirl.create(:miq_host_provision_request, :tenant => owner_tenant, :requester => owner_user)
results = described_class.search(:class => "MiqRequest", :user => owner_user).first
expect(results).to match_array [request]
end

it "a child tenant user in a group with tag filters can see their own request" do
child_group.entitlement = Entitlement.new
child_group.entitlement.set_managed_filters([["/managed/environment/prod"], ["/managed/service_level/silver"]])
child_group.entitlement.set_belongsto_filters([])
child_group.save!

request = FactoryGirl.create(:miq_host_provision_request, :tenant => child_tenant, :requester => child_user)
results = described_class.search(:class => "MiqRequest", :user => child_user).first
expect(results).to match_array [request]
end

it "can see other's request in the same tenant" do
group = FactoryGirl.create(:miq_group, :tenant => owner_tenant)
user = FactoryGirl.create(:user, :miq_groups => [group])

request = FactoryGirl.create(:miq_host_provision_request, :tenant => owner_tenant, :requester => owner_user)
results = described_class.search(:class => "MiqRequest", :user => user).first
expect(results).to match_array [request]
end

it "can't see parent tenant's request" do
FactoryGirl.create(:miq_host_provision_request, :tenant => owner_tenant, :requester => owner_user)
results = described_class.search(:class => "MiqRequest", :miq_group => child_group).first
expect(results).to match_array []
end

it "can see descendant tenant's request" do
request = FactoryGirl.create(:miq_host_provision_request, :tenant => child_tenant, :requester => child_user)
results = described_class.search(:class => "MiqRequest", :miq_group => owner_group).first
expect(results).to match_array [request]
end
end

context "tenant access strategy VMs and Templates" do
Expand Down

0 comments on commit 7aa3716

Please sign in to comment.