Fixes #36726 - Non-admin users with the view_smart_proxies permission…

… can now access capsules from the API

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.

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

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

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)