From 75f055205349a1897ba5ecc7417fc769e9ca7bc5 Mon Sep 17 00:00:00 2001 From: lpichler Date: Thu, 18 May 2017 13:54:04 +0200 Subject: [PATCH 1/2] Add RBAC check for virtual attributes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Virtual attributes are passed to API thru ‘attributes’ parameter and it can be: - direct association e.g. attributes=vms (plural associations - collection) e.g. attributes=parent_manage (belongs_to association) /api/providers/2?attributes=parent_manages,vms - indirect association e.g. attributes=parent_manager.cloud_tenants (plural associations - collection) e.g. attributes=parent_manager.host (belongs_to association) localhost:3000/api/providers/2?attributes=parent_manager.host, parent_manager.cloud_tenants This RBAC situation is covered by method virtual_attribute_search(resource, attribute) where attribute can be has_many association or belongs_to (There are different RBAC check) it can also pure class as MiqRequestWorkflow (last if-else leg) --- app/controllers/api/base_controller/renderer.rb | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/base_controller/renderer.rb b/app/controllers/api/base_controller/renderer.rb index b40f2ae4700..f6d823806c0 100644 --- a/app/controllers/api/base_controller/renderer.rb +++ b/app/controllers/api/base_controller/renderer.rb @@ -165,6 +165,19 @@ def collection_search(is_subcollection, type, klass) Rbac.filtered(res, options) end + def virtual_attribute_search(resource, attribute) + if resource.class < ApplicationRecord + # is relation in 'attribute' variable plural in the model class (from 'resource.class') ? + if [:has_many, :has_and_belongs_to_many].include?(resource.class.reflection_with_virtual(attribute).try(:macro)) + Rbac.filtered(resource.public_send(attribute)) + else + Rbac.filtered_object(resource).try(:public_send, attribute) + end + else + resource.public_send(attribute) + end + end + # # Let's expand subcollections for objects if asked for # @@ -205,7 +218,7 @@ def expand_virtual_attributes(json, type, resource) def fetch_direct_virtual_attribute(type, resource, attr) return unless attr_accessible?(resource, attr) virtattr_accessor = virtual_attribute_accessor(type, attr) - value = virtattr_accessor.nil? ? resource.public_send(attr) : send(virtattr_accessor, resource) + value = virtattr_accessor ? send(virtattr_accessor, resource) : virtual_attribute_search(resource, attr) result = {attr => normalize_attr(attr, value)} # set nil vtype above to "#{type}/#{resource.id}/#{attr}" to support id normalization [value, result] @@ -214,7 +227,7 @@ def fetch_direct_virtual_attribute(type, resource, attr) def fetch_indirect_virtual_attribute(_type, resource, base, attr, object_hash) query_related_objects(base, resource, object_hash) return unless attr_accessible?(object_hash[base], attr) - value = object_hash[base].public_send(attr) + value = virtual_attribute_search(object_hash[base], attr) result = {attr => normalize_attr(attr, value)} # set nil vtype above to "#{type}/#{resource.id}/#{base.tr('.', '/')}/#{attr}" to support id normalization base.split(".").reverse_each { |level| result = {level => result} } From 0d479b19c4ac11360b03202ae737653f329c85e4 Mon Sep 17 00:00:00 2001 From: lpichler Date: Fri, 19 May 2017 14:23:29 +0200 Subject: [PATCH 2/2] Add specs for API RBAC tests for direct and indirect associations --- spec/requests/api/providers_spec.rb | 47 +++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/spec/requests/api/providers_spec.rb b/spec/requests/api/providers_spec.rb index 78c5dc8670d..14818072824 100644 --- a/spec/requests/api/providers_spec.rb +++ b/spec/requests/api/providers_spec.rb @@ -137,6 +137,53 @@ def have_endpoint_attributes(expected_hash) have_attributes(h) end + context 'Provider\'s virtual attributes(= direct or indirect associations) with RBAC' do + let(:ems_openstack) { FactoryGirl.create(:ems_openstack, :tenant_mapping_enabled => true) } + let(:ems_cinder) { FactoryGirl.create(:ems_cinder, :parent_manager => ems_openstack) } + let(:ems_cinder_url) { providers_url(ems_cinder.id) } + + let(:tenant) { FactoryGirl.create(:tenant, :source_type => 'CloudTenant') } + let!(:cloud_tenant_1) { FactoryGirl.create(:cloud_tenant, :source_tenant => tenant, :ext_management_system => ems_openstack) } + let!(:cloud_tenant_2) { FactoryGirl.create(:cloud_tenant, :source_tenant => Tenant.root_tenant, :ext_management_system => ems_openstack) } + + let(:role) { FactoryGirl.create(:miq_user_role) } + let!(:group) { FactoryGirl.create(:miq_group, :tenant => tenant, :miq_user_role => role) } + let!(:vm) { FactoryGirl.create(:vm_openstack, :ext_management_system => ems_cinder, :miq_group => group) } + let!(:vm_1) { FactoryGirl.create(:vm_openstack, :ext_management_system => ems_cinder) } + + context 'with restricted user' do + let(:user) do + FactoryGirl.create(:user, :miq_groups => [group], :password => api_config(:password), :userid => api_config(:user), :name => api_config(:user_name)) + end + + def define_user + @role = role + user + end + + it 'lists only CloudTenant for the restricted user(indirect association)' do + api_basic_authorize action_identifier(:providers, :read, :resource_actions, :get) + run_get(ems_cinder_url, :attributes => 'parent_manager.cloud_tenants') + cloud_tenant_ids = response.parsed_body['parent_manager']['cloud_tenants'].map { |x| x['id'] } + expect([cloud_tenant_1.id]).to match_array(cloud_tenant_ids) + end + + it 'lists only CloudTenant for the restricted user(direct association)' do + api_basic_authorize action_identifier(:providers, :read, :resource_actions, :get) + run_get(ems_cinder_url, :attributes => 'vms') + vm_ids = response.parsed_body['vms'].map { |x| x['id'] } + expect([vm.id]).to match_array(vm_ids) + end + end + + it 'lists all CloudTenants' do + api_basic_authorize action_identifier(:providers, :read, :resource_actions, :get) + run_get(ems_cinder_url, :attributes => 'parent_manager.cloud_tenants') + cloud_tenant_ids = response.parsed_body['parent_manager']['cloud_tenants'].map { |x| x['id'] } + expect([cloud_tenant_1.id, cloud_tenant_2.id]).to match_array(cloud_tenant_ids) + end + end + context "Provider custom_attributes" do let(:provider) { FactoryGirl.create(:ext_management_system, sample_rhevm) } let(:provider_url) { providers_url(provider.id) }