Skip to content

Commit

Permalink
Merge pull request ManageIQ#17208 from lpichler/restrict_miq_request_…
Browse files Browse the repository at this point in the history
…by_users_group

Add ownership for MiqRequest in RBAC
  • Loading branch information
gtanzillo authored and d-m-u committed Jun 6, 2018
1 parent f64514d commit fa4670a
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 3 deletions.
20 changes: 20 additions & 0 deletions app/models/miq_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,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
Expand Down
19 changes: 16 additions & 3 deletions lib/rbac/filterer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class Filterer
Host
LoadBalancer
MiqCimInstance
MiqRequest
NetworkPort
NetworkRouter
OrchestrationTemplate
Expand Down Expand Up @@ -118,6 +119,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)
Expand Down Expand Up @@ -347,8 +355,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
Expand All @@ -360,7 +373,7 @@ def get_self_service_objects(user, miq_group, klass)

def 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))
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, user_filters['managed']))
d_filtered_ids = pluck_ids(matches_via_descendants(rbac_class(klass), user_filters['match_via_descendants'],
Expand Down
56 changes: 56 additions & 0 deletions spec/lib/rbac/filterer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,62 @@
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 virtual custom attributes' do
let(:virtual_custom_attribute_1) { "virtual_custom_attribute_attribute_1" }
let(:virtual_custom_attribute_2) { "virtual_custom_attribute_attribute_2" }
Expand Down

0 comments on commit fa4670a

Please sign in to comment.