From 2c618ab418b0ce6c7133d87362bf6c8ef8eee89b Mon Sep 17 00:00:00 2001 From: Alberto Bellotti Date: Tue, 21 Mar 2017 16:49:36 -0400 Subject: [PATCH 1/2] Added Api::Utils.resource_search_by_href_slug helper method Added related rspec --- lib/api/utils.rb | 10 ++++++ spec/lib/api/utils_spec.rb | 63 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 spec/lib/api/utils_spec.rb diff --git a/lib/api/utils.rb b/lib/api/utils.rb index dba4d93fefd..858df445976 100644 --- a/lib/api/utils.rb +++ b/lib/api/utils.rb @@ -5,5 +5,15 @@ def self.build_href_slug(klass, id) collection = Api::CollectionConfig.new.name_for_subclass(klass) "#{collection}/#{id}" if collection end + + def self.resource_search_by_href_slug(href_slug) + return unless href_slug + collection, id = href_slug.split('/') + collection_config = Api::CollectionConfig.new if collection + raise _("Invalid href_slug #{href_slug} specified") unless collection && id && collection_config.collection?(collection) + + klass = collection_config.klass(collection) + Rbac.filtered_object(klass.find(id), :user => User.current_user, :class => klass) + end end end diff --git a/spec/lib/api/utils_spec.rb b/spec/lib/api/utils_spec.rb new file mode 100644 index 00000000000..4868204e8ce --- /dev/null +++ b/spec/lib/api/utils_spec.rb @@ -0,0 +1,63 @@ +RSpec.describe Api::Utils do + describe ".resource_search_by_href_slug" do + before { allow(User).to receive_messages(:server_timezone => "UTC") } + + it "returns nil when nil is specified" do + actual = described_class.resource_search_by_href_slug(nil) + + expect(actual).to be_nil + end + + it "raises an exception with missing id" do + expect { described_class.resource_search_by_href_slug("vms/") } + .to raise_error("Invalid href_slug vms/ specified") + end + + it "raises an exception with missing collection" do + expect { described_class.resource_search_by_href_slug("/10") } + .to raise_error("Invalid href_slug /10 specified") + end + + it "raises an exception with missing delimited" do + expect { described_class.resource_search_by_href_slug("vms") } + .to raise_error("Invalid href_slug vms specified") + end + + it "raises an exception with a bogus href_slug" do + expect { described_class.resource_search_by_href_slug("bogus/123") } + .to raise_error("Invalid href_slug bogus/123 specified") + end + + it "raises an exception with a non primary collection href_slug" do + expect { described_class.resource_search_by_href_slug("auth/123") } + .to raise_error("Invalid href_slug auth/123 specified") + end + + it "raises an ActiveRecord::RecordNotFound with a non-existent href_slug" do + expect { described_class.resource_search_by_href_slug("vms/99999") } + .to raise_error(ActiveRecord::RecordNotFound) + end + + it "returns the resource with a valid href_slug" do + vm = FactoryGirl.create(:vm_vmware) + + actual = described_class.resource_search_by_href_slug("vms/#{vm.id}") + + expect(actual).to eq(vm) + end + + it "does not return the resource when Rbac fails" do + owner_tenant = FactoryGirl.create(:tenant) + + unauth_tenant = FactoryGirl.create(:tenant) + unauth_group = FactoryGirl.create(:miq_group, :tenant => unauth_tenant) + + vm = FactoryGirl.create(:vm_vmware, :tenant => owner_tenant) + User.current_user = FactoryGirl.create(:user, :miq_groups => [unauth_group]) + + actual = described_class.resource_search_by_href_slug("vms/#{vm.id}") + + expect(actual).to eq(nil) + end + end +end From d727f00b3b80e40600aefbe5dcf9e64363d09b4c Mon Sep 17 00:00:00 2001 From: Alberto Bellotti Date: Wed, 22 Mar 2017 10:27:01 -0400 Subject: [PATCH 2/2] Enhanced resource_search_by_href_slug to support optional user object User.current_user or specified user must be specified Updated tests for the new signature --- lib/api/utils.rb | 7 ++++-- spec/lib/api/utils_spec.rb | 47 +++++++++++++++++++++++++++++++++++--- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/lib/api/utils.rb b/lib/api/utils.rb index 858df445976..6623e056bb1 100644 --- a/lib/api/utils.rb +++ b/lib/api/utils.rb @@ -6,14 +6,17 @@ def self.build_href_slug(klass, id) "#{collection}/#{id}" if collection end - def self.resource_search_by_href_slug(href_slug) + def self.resource_search_by_href_slug(href_slug, user = User.current_user) return unless href_slug + collection, id = href_slug.split('/') collection_config = Api::CollectionConfig.new if collection + raise _("Invalid href_slug #{href_slug} specified") unless collection && id && collection_config.collection?(collection) + raise _("User must be defined") unless user klass = collection_config.klass(collection) - Rbac.filtered_object(klass.find(id), :user => User.current_user, :class => klass) + Rbac.filtered_object(klass.find(id), :user => user, :class => klass) end end end diff --git a/spec/lib/api/utils_spec.rb b/spec/lib/api/utils_spec.rb index 4868204e8ce..95723f0e888 100644 --- a/spec/lib/api/utils_spec.rb +++ b/spec/lib/api/utils_spec.rb @@ -34,19 +34,46 @@ end it "raises an ActiveRecord::RecordNotFound with a non-existent href_slug" do - expect { described_class.resource_search_by_href_slug("vms/99999") } + owner_tenant = FactoryGirl.create(:tenant) + owner_group = FactoryGirl.create(:miq_group, :tenant => owner_tenant) + owner = FactoryGirl.create(:user, :miq_groups => [owner_group]) + + expect { described_class.resource_search_by_href_slug("vms/99999", owner) } .to raise_error(ActiveRecord::RecordNotFound) end - it "returns the resource with a valid href_slug" do + it "raises an exception with an undefined user" do vm = FactoryGirl.create(:vm_vmware) + expect { described_class.resource_search_by_href_slug("vms/#{vm.id}") } + .to raise_error("User must be defined") + end + + it "returns the resource when Rbac succeeds for current_user" do + owner_tenant = FactoryGirl.create(:tenant) + owner_group = FactoryGirl.create(:miq_group, :tenant => owner_tenant) + + vm = FactoryGirl.create(:vm_vmware, :tenant => owner_tenant) + User.current_user = FactoryGirl.create(:user, :miq_groups => [owner_group]) + actual = described_class.resource_search_by_href_slug("vms/#{vm.id}") expect(actual).to eq(vm) end - it "does not return the resource when Rbac fails" do + it "returns the resource when Rbac succeeds for specified user" do + owner_tenant = FactoryGirl.create(:tenant) + owner_group = FactoryGirl.create(:miq_group, :tenant => owner_tenant) + owner = FactoryGirl.create(:user, :miq_groups => [owner_group]) + + vm = FactoryGirl.create(:vm_vmware, :tenant => owner_tenant) + + actual = described_class.resource_search_by_href_slug("vms/#{vm.id}", owner) + + expect(actual).to eq(vm) + end + + it "does not return the resource when Rbac fails for current_user" do owner_tenant = FactoryGirl.create(:tenant) unauth_tenant = FactoryGirl.create(:tenant) @@ -59,5 +86,19 @@ expect(actual).to eq(nil) end + + it "does not return the resource when Rbac fails for specified user" do + owner_tenant = FactoryGirl.create(:tenant) + + unauth_tenant = FactoryGirl.create(:tenant) + unauth_group = FactoryGirl.create(:miq_group, :tenant => unauth_tenant) + unauth_user = FactoryGirl.create(:user, :miq_groups => [unauth_group]) + + vm = FactoryGirl.create(:vm_vmware, :tenant => owner_tenant) + + actual = described_class.resource_search_by_href_slug("vms/#{vm.id}", unauth_user) + + expect(actual).to eq(nil) + end end end