From 2309127577a534c5e024bd0a1d67dfb992176eb5 Mon Sep 17 00:00:00 2001 From: Quinn James <35753203+qcjames53@users.noreply.github.com> Date: Mon, 18 Sep 2023 08:55:18 -0400 Subject: [PATCH] Fixes #36726 - Non-admin users with the view_smart_proxies permission can now access capsules from the API (#10722) Added test cases for the capsules controller to enforce permissions handling. Comment cleanup. Fixed broken test cases and queried more edge cases of the capsule controller. Added location support for custom users created in unit tests. --- .../katello/api/v2/capsules_controller.rb | 12 +++- .../api/v2/capsules_controller_test.rb | 56 +++++++++++++++++++ test/support/controller_support.rb | 15 +++-- 3 files changed, 78 insertions(+), 5 deletions(-) create mode 100644 test/controllers/api/v2/capsules_controller_test.rb diff --git a/app/controllers/katello/api/v2/capsules_controller.rb b/app/controllers/katello/api/v2/capsules_controller.rb index 5bc3accb837..d741b9477ae 100644 --- a/app/controllers/katello/api/v2/capsules_controller.rb +++ b/app/controllers/katello/api/v2/capsules_controller.rb @@ -4,6 +4,9 @@ class Api::V2::CapsulesController < ::Api::V2::SmartProxiesController api_base_url "/katello/api" end + skip_before_action :find_resource, only: [:show] + before_action :find_smart_proxy, :only => [:show] + api :GET, '/capsules', 'List all smart proxies that have content' param_group :search, Api::V2::ApiController def index @@ -15,7 +18,6 @@ def index api :GET, '/capsules/:id', 'Show the smart proxy details' param :id, Integer, :desc => 'Id of the smart proxy', :required => true def show - super end def resource_name @@ -31,5 +33,13 @@ def resource_class def authorized User.current.allowed_to?(params.slice(:action, :id).merge(controller: 'api/v2/smart_proxies')) end + + # Without this method, Foreman requires non-admin users to be admin or have a + # non-existent "view_capsule" permission. By replacing the find_resource + # before action, we check the user for "view_smart_proxies" permission instead. + def find_smart_proxy + resource_scope = SmartProxy.authorized("view_smart_proxies", SmartProxy) + instance_variable_set("@smart_proxy", resource_finder(resource_scope, params[:id])) + end end end diff --git a/test/controllers/api/v2/capsules_controller_test.rb b/test/controllers/api/v2/capsules_controller_test.rb new file mode 100644 index 00000000000..872bc08f9be --- /dev/null +++ b/test/controllers/api/v2/capsules_controller_test.rb @@ -0,0 +1,56 @@ +# encoding: utf-8 + +require "katello_test_helper" + +module Katello + class Api::V2::CapsulesControllerTest < ActionController::TestCase + include Support::CapsuleSupport + include Support::ForemanTasks::Task + + def setup + setup_controller_defaults_api + @repository = katello_repositories(:fedora_17_unpublished) + @library_dev_view = ContentView.find(katello_content_views(:library_dev_view).id) + @location = Location.all + @organization = [get_organization] + + proxy_with_pulp.organizations = @organization + proxy_with_pulp.locations = @location + end + + def view_smart_proxies_perms + [[:view_smart_proxies]] + end + + def incorrect_perms + [[:view_capsule_content, :manage_capsule_content]] + end + + def environment + @environment ||= katello_environments(:library) + end + + def test_admin_index + get :index + assert_response :success + end + + def test_admin_show + get :show, params: { :id => proxy_with_pulp.id} + assert_response :success + end + + def test_user_index + assert_protected_action(:index, view_smart_proxies_perms, incorrect_perms) do + get :index + end + end + + def test_user_show + assert_protected_action(:show, view_smart_proxies_perms, incorrect_perms, + @organization, @location) do + get :show, params: { :id => proxy_with_pulp.id} + end + end + end +end diff --git a/test/support/controller_support.rb b/test/support/controller_support.rb index 50d9d9e9f63..7cb205921d4 100644 --- a/test/support/controller_support.rb +++ b/test/support/controller_support.rb @@ -3,13 +3,15 @@ module ControllerSupport include Katello::AuthorizationSupportMethods - def check_permission(permission:, action:, request:, organizations:, authorized: true, expect_404: false) + def check_permission(permission:, action:, request:, organizations:, locations:, + authorized: true, expect_404: false) permissions = permission.is_a?(Array) ? permission : [permission] permissions.each do |perm| user = User.unscoped.find(users(:restricted).id) as_admin do user.organizations = organizations unless organizations.blank? + user.locations = locations unless locations.blank? setup_user_with_permissions(perm, user) end @@ -34,12 +36,14 @@ def check_permission(permission:, action:, request:, organizations:, authorized: end end - def assert_protected_action(action_name, allowed_perms, denied_perms = [], organizations = [], expect_404: false, &block) + def assert_protected_action(action_name, allowed_perms, denied_perms = [], + organizations = [], locations = [], expect_404: false, &block) assert_authorized( :permission => allowed_perms, :action => action_name, :request => block, :organizations => organizations, + :locations => locations, :expect_404 => expect_404 ) @@ -49,13 +53,16 @@ def assert_protected_action(action_name, allowed_perms, denied_perms = [], organ :action => action_name, :request => block, :organizations => organizations, + :locations => locations, :expect_404 => expect_404 ) end end - def assert_protected_object(action_name, allowed_perms, denied_perms = [], organizations = [], &block) - assert_protected_action(action_name, allowed_perms, denied_perms, organizations, expect_404: true, &block) + def assert_protected_object(action_name, allowed_perms, denied_perms = [], + organizations = [], locations = [], &block) + assert_protected_action(action_name, allowed_perms, denied_perms, organizations, + locations, expect_404: true, &block) end def assert_authorized(params)