From 4e9bff822991fe9bc3a0f74f57ca94f5d74fe21c Mon Sep 17 00:00:00 2001 From: Alberto Bellotti Date: Wed, 8 Feb 2017 16:58:10 -0500 Subject: [PATCH 1/2] Allows specification for optional multiple identifiers - Supports current single identifier, where user must be allowed that specific role for authorization: :post: - :name: delete :identifier: vm_delete - Supports multiple identifiers, where user must be allowed at least one of the roles specified for authorization: :post: - :name: delete :identifier: - vm_delete - instance_terminate - Adding role based authorization rspecs --- .../api/base_controller/renderer.rb | 2 +- spec/requests/api/authentication_spec.rb | 58 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/base_controller/renderer.rb b/app/controllers/api/base_controller/renderer.rb index 7c84234242a..215b665bd6e 100644 --- a/app/controllers/api/base_controller/renderer.rb +++ b/app/controllers/api/base_controller/renderer.rb @@ -387,7 +387,7 @@ def fetch_typed_subcollection_actions(method, is_subcollection) def api_user_role_allows?(action_identifier) return true unless action_identifier - User.current_user.role_allows?(:identifier => action_identifier) + Array(action_identifier).any? { |identifier| User.current_user.role_allows?(:identifier => identifier) } end def render_actions(resource) diff --git a/spec/requests/api/authentication_spec.rb b/spec/requests/api/authentication_spec.rb index 8ed9f64ad55..9d237576d3b 100644 --- a/spec/requests/api/authentication_spec.rb +++ b/spec/requests/api/authentication_spec.rb @@ -292,4 +292,62 @@ def systoken(server_guid, userid, timestamp) expect_result_to_have_keys(ENTRYPOINT_KEYS) end end + + context "Role Based Authorization" do + before do + FactoryGirl.create(:vm_vmware, :name => "vm1") + end + + def vms_get_test(new_identifier) + config = Api::ApiConfig.collections.vms.collection_actions.get.first + saved_identifier = config.identifier + config.identifier = new_identifier + yield + config.identifier = saved_identifier + end + + context "actions with single role identifier" do + it "are rejected when user is not authorized with the single role identifier" do + vms_get_test("vm_view_role1") do + api_basic_authorize + + run_get vms_url + + expect(response).to have_http_status(:forbidden) + end + end + + it "are accepted when user is authorized with the single role identifier" do + vms_get_test("vm_view_role1") do + api_basic_authorize "vm_view_role1" + + run_get vms_url + + expect_query_result(:vms, 1, 1) + end + end + end + + context "actions with multiple role identifiers" do + it "are rejected when user is not authorized with any of the role identifiers" do + vms_get_test(%w(vm_view_role1 vm_view_role2)) do + api_basic_authorize + + run_get vms_url + + expect(response).to have_http_status(:forbidden) + end + end + + it "are accepted when user is authorized with at least one of the role identifiers" do + vms_get_test(%w(vm_view_role1 vm_view_role2)) do + api_basic_authorize "vm_view_role2" + + run_get vms_url + + expect_query_result(:vms, 1, 1) + end + end + end + end end From 598ced7cb5623593b9e55c40924d8fafb4ebddc9 Mon Sep 17 00:00:00 2001 From: Alberto Bellotti Date: Fri, 10 Feb 2017 13:46:21 -0500 Subject: [PATCH 2/2] Updated PR to use a preferred API config stub method - Introduced stub_api_action_role - Updated tests to use the new stub_api_action_role - Fixed issue in api spec helper update_user_role not able to handle identifier arrays. --- spec/requests/api/authentication_spec.rb | 44 +++++++++--------------- spec/support/api_helper.rb | 10 +++++- 2 files changed, 25 insertions(+), 29 deletions(-) diff --git a/spec/requests/api/authentication_spec.rb b/spec/requests/api/authentication_spec.rb index 9d237576d3b..2220051a898 100644 --- a/spec/requests/api/authentication_spec.rb +++ b/spec/requests/api/authentication_spec.rb @@ -298,55 +298,43 @@ def systoken(server_guid, userid, timestamp) FactoryGirl.create(:vm_vmware, :name => "vm1") end - def vms_get_test(new_identifier) - config = Api::ApiConfig.collections.vms.collection_actions.get.first - saved_identifier = config.identifier - config.identifier = new_identifier - yield - config.identifier = saved_identifier - end - context "actions with single role identifier" do it "are rejected when user is not authorized with the single role identifier" do - vms_get_test("vm_view_role1") do - api_basic_authorize + stub_api_action_role(:vms, :collection_actions, :get, :read, "vm_view_role1") + api_basic_authorize - run_get vms_url + run_get vms_url - expect(response).to have_http_status(:forbidden) - end + expect(response).to have_http_status(:forbidden) end it "are accepted when user is authorized with the single role identifier" do - vms_get_test("vm_view_role1") do - api_basic_authorize "vm_view_role1" + stub_api_action_role(:vms, :collection_actions, :get, :read, "vm_view_role1") + api_basic_authorize "vm_view_role1" - run_get vms_url + run_get vms_url - expect_query_result(:vms, 1, 1) - end + expect_query_result(:vms, 1, 1) end end context "actions with multiple role identifiers" do it "are rejected when user is not authorized with any of the role identifiers" do - vms_get_test(%w(vm_view_role1 vm_view_role2)) do - api_basic_authorize + stub_api_action_role(:vms, :collection_actions, :get, :read, %w(vm_view_role1 vm_view_role2)) + api_basic_authorize - run_get vms_url + run_get vms_url - expect(response).to have_http_status(:forbidden) - end + expect(response).to have_http_status(:forbidden) end it "are accepted when user is authorized with at least one of the role identifiers" do - vms_get_test(%w(vm_view_role1 vm_view_role2)) do - api_basic_authorize "vm_view_role2" + stub_api_action_role(:vms, :collection_actions, :get, :read, %w(vm_view_role1 vm_view_role2)) + api_basic_authorize "vm_view_role2" - run_get vms_url + run_get vms_url - expect_query_result(:vms, 1, 1) - end + expect_query_result(:vms, 1, 1) end end end diff --git a/spec/support/api_helper.rb b/spec/support/api_helper.rb index 51aa2fb95aa..42c4e52aa59 100644 --- a/spec/support/api_helper.rb +++ b/spec/support/api_helper.rb @@ -93,7 +93,7 @@ def basic_authorize(user, password) def update_user_role(role, *identifiers) return if identifiers.blank? - product_features = identifiers.collect do |identifier| + product_features = identifiers.flatten.collect do |identifier| MiqProductFeature.find_or_create_by(:identifier => identifier) end role.update_attributes!(:miq_product_features => product_features) @@ -103,6 +103,14 @@ def miq_server_guid @miq_server_guid ||= MiqUUID.new_guid end + def stub_api_action_role(collection, action_type, method, action, identifier) + new_action_role = Config::Options.new.merge!("name" => action.to_s, "identifier" => identifier) + updated_method = Api::ApiConfig.collections[collection][action_type][method].collect do |method_action| + method_action.name == action.to_s ? new_action_role : method_action + end + allow(Api::ApiConfig.collections[collection][action_type]).to receive(method) { updated_method } + end + def action_identifier(type, action, selection = :resource_actions, method = :post) Api::ApiConfig.collections[type][selection][method] .detect { |spec| spec[:name] == action.to_s }[:identifier]