Merge pull request #17208 from lpichler/restrict_miq_request_by_users…

…_group

Add ownership for MiqRequest in RBAC
(cherry picked from commit 0969759)

https://bugzilla.redhat.com/show_bug.cgi?id=1562777

app/models/miq_request.rb

@@ -105,6 +105,26 @@ def self.class_from_request_data(data)
  model_symbol.to_s.constantize
  end
 
+ def self.user_or_group_owned(user, miq_group)
+ if user && miq_group
+ user_owned(user).or(group_owned(miq_group))
+ elsif user
+ user_owned(user)
+ elsif miq_group
+ group_owned(miq_group)
+ else
+ none
+ end
+ end
+
+ def self.user_owned(user)
+ where(:requester_id => user.id)
+ end
+
+ def self.group_owned(miq_group)
+ where(:requester_id => miq_group.user_ids)
+ end
+
  # Supports old-style requests where specific request was a seperate table connected as a resource
  def resource
  self

lib/rbac/filterer.rb

@@ -41,6 +41,7 @@ class Filterer
  MiddlewareMessaging
  MiddlewareServer
  MiddlewareServerGroup
+ MiqRequest
  NetworkPort
  NetworkRouter
  OrchestrationStack
@@ -124,6 +125,13 @@ class Filterer
  'Vm' => :descendant_ids
  }
 
+ # Classes inherited from these classes or mixins are allowing ownership feature on the target model,
+ # scope user_or_group_owned is required on target model
+ OWNERSHIP_CLASSES = %w(
+ OwnershipMixin
+ MiqRequest
+ ).freeze
+
  include Vmdb::Logging
 
  def self.search(*args)
@@ -351,8 +359,13 @@ def pluck_ids(targets)
  targets.pluck(:id) if targets
  end
 
- def get_self_service_objects(user, miq_group, klass)
- return nil if miq_group.nil? || !miq_group.self_service? || !(klass < OwnershipMixin)
+ def self_service_ownership_scope?(miq_group, klass)
+ is_ownership_class = OWNERSHIP_CLASSES.any? { |allowed_ownership_klass| klass <= allowed_ownership_klass.safe_constantize }
+ miq_group.present? && miq_group.self_service? && is_ownership_class && klass.respond_to?(:user_or_group_owned)
+ end
+
+ def self_service_ownership_scope(user, miq_group, klass)
+ return nil unless self_service_ownership_scope?(miq_group, klass)
 
  # for limited_self_service, use user's resources, not user.current_group's resources
  # for reports (user = nil), still use miq_group
@@ -366,7 +379,7 @@ def calc_filtered_ids(scope, user_filters, user, miq_group, scope_tenant_filter)
  klass = scope.respond_to?(:klass) ? scope.klass : scope
  expression = miq_group.try(:entitlement).try(:filter_expression)
  expression.set_tagged_target(klass) if expression
- u_filtered_ids = pluck_ids(get_self_service_objects(user, miq_group, klass))
+ u_filtered_ids = pluck_ids(self_service_ownership_scope(user, miq_group, klass))
  b_filtered_ids = get_belongsto_filter_object_ids(klass, user_filters['belongsto'])
  m_filtered_ids = pluck_ids(get_managed_filter_object_ids(scope, expression || user_filters['managed']))
  d_filtered_ids = pluck_ids(matches_via_descendants(rbac_class(klass), user_filters['match_via_descendants'],

spec/lib/rbac/filterer_spec.rb

@@ -125,6 +125,62 @@ def combine_filtered_ids(user_filtered_ids, belongsto_filtered_ids, managed_filt
  let(:child_openstack_vm) { FactoryGirl.create(:vm_openstack, :tenant => child_tenant, :miq_group => child_group) }
 
  describe ".search" do
+ context 'for MiqRequests' do
+ # MiqRequest for owner group
+ let!(:miq_request_user_owner) { FactoryGirl.create(:miq_provision_request, :tenant => owner_tenant, :requester => owner_user) }
+ # User for owner group
+ let(:user_a) { FactoryGirl.create(:user, :miq_groups => [owner_group]) }
+
+ # MiqRequests for other group
+ let!(:miq_request_user_a) { FactoryGirl.create(:miq_provision_request, :tenant => owner_tenant, :requester => other_user) }
+ let!(:miq_request_user_b) { FactoryGirl.create(:miq_provision_request, :tenant => owner_tenant, :requester => user_b) }
+
+ # other_group is from owner_tenant
+ let(:other_group) { FactoryGirl.create(:miq_group, :tenant => owner_tenant) }
+ # User for other group
+ let(:user_b) { FactoryGirl.create(:user, :miq_groups => [other_group]) }
+
+ context "self service user (User or group owned)" do
+ before do
+ allow(other_group).to receive(:self_service?).and_return(true)
+ allow(owner_group).to receive(:self_service?).and_return(true)
+ end
+
+ context 'users are in same tenant as requester' do
+ it "displays requests of user's of group owner_group" do
+ results = described_class.search(:class => MiqProvisionRequest, :user => user_a).first
+ expect(results).to match_array([miq_request_user_owner])
+ end
+
+ it "displays requests for users of other_user's group (other_group) so also for user_c" do
+ results = described_class.search(:class => MiqProvisionRequest, :user => user_b).first
+ expect(results).to match_array([miq_request_user_a, miq_request_user_b])
+ end
+ end
+ end
+
+ context "limited self service user (only user owned)" do
+ before do
+ allow(other_group).to receive(:limited_self_service?).and_return(true)
+ allow(other_group).to receive(:self_service?).and_return(true)
+ allow(owner_group).to receive(:limited_self_service?).and_return(true)
+ allow(owner_group).to receive(:self_service?).and_return(true)
+ end
+
+ context 'users are in same tenant as requester' do
+ it "displays requests of user's of group owner_group" do
+ results = described_class.search(:class => MiqProvisionRequest, :user => user_a).first
+ expect(results).to be_empty
+ end
+
+ it "displays requests for users of other_user's group (other_group) so also for user_c" do
+ results = described_class.search(:class => MiqProvisionRequest, :user => user_b).first
+ expect(results).to match_array([miq_request_user_b])
+ end
+ end
+ end
+ end
+
  context 'with tags' do
  let(:role) { FactoryGirl.create(:miq_user_role) }
  let(:tagged_group) { FactoryGirl.create(:miq_group, :tenant => Tenant.root_tenant, :miq_user_role => role) }