Reused obsolete RBAC method for fetching single record

app/controllers/application_controller/performance.rb

@@ -525,8 +525,8 @@ def date_time_running_msg(typ, timestamp)
 
   # Send error message if record is found and authorized, else return the record
   def perf_menu_record_valid(model, id)
-    record = find_records_with_rbac(model.constantize, id)
-    if record.empty?
+    record = find_record_with_rbac(model.constantize, id)
+    if record.present?
       add_flash(_("Can't access selected record"))
     end
     unless @flash_array.blank?

app/controllers/ems_common.rb

@@ -349,31 +349,31 @@ def button
     elsif params[:pressed] == "host_aggregate_edit"
       javascript_redirect :controller => "host_aggregate",
                           :action     => "edit",
-                          :id         => find_records_with_rbac(HostAggregate, checked_or_params).first
+                          :id         => find_record_with_rbac(HostAggregate, checked_or_params)
     elsif params[:pressed] == "cloud_tenant_edit"
       javascript_redirect :controller => "cloud_tenant",
                           :action     => "edit",
-                          :id         => find_records_with_rbac(CloudTenant, checked_or_params).first
+                          :id         => find_record_with_rbac(CloudTenant, checked_or_params)
     elsif params[:pressed] == "cloud_volume_new"
       javascript_redirect :controller         => "cloud_volume",
                           :action             => "new",
                           :storage_manager_id => params[:id]
     elsif params[:pressed] == "cloud_volume_snapshot_create"
       javascript_redirect :controller => "cloud_volume",
                           :action     => "snapshot_new",
-                          :id         => find_records_with_rbac(CloudVolume, checked_or_params).first
+                          :id         => find_record_with_rbac(CloudVolume, checked_or_params)
     elsif params[:pressed] == "cloud_volume_attach"
       javascript_redirect :controller => "cloud_volume",
                           :action     => "attach",
-                          :id         => find_records_with_rbac(CloudVolume, checked_or_params).first
+                          :id         => find_record_with_rbac(CloudVolume, checked_or_params)
     elsif params[:pressed] == "cloud_volume_detach"
       javascript_redirect :controller => "cloud_volume",
                           :action     => "detach",
-                          :id         => find_records_with_rbac(CloudVolume, checked_or_params).first
+                          :id         => find_record_with_rbac(CloudVolume, checked_or_params)
     elsif params[:pressed] == "cloud_volume_edit"
       javascript_redirect :controller => "cloud_volume",
                           :action     => "edit",
-                          :id         => find_records_with_rbac(CloudVolume, checked_or_params).first
+                          :id         => find_record_with_rbac(CloudVolume, checked_or_params)
     elsif params[:pressed] == "cloud_volume_delete"
       # Clear CloudVolumeController's lastaction, since we are calling the delete_volumes from
       # an external controller. This will ensure that the final redirect is properly handled.
@@ -384,15 +384,15 @@ def button
     elsif params[:pressed] == "network_router_edit"
       javascript_redirect :controller => "network_router",
                           :action     => "edit",
-                          :id         => find_records_with_rbac(NetworkRouter, checked_or_params).first
+                          :id         => find_record_with_rbac(NetworkRouter, checked_or_params)
     elsif params[:pressed] == "network_router_add_interface"
       javascript_redirect :controller => "network_router",
                           :action     => "add_interface_select",
-                          :id         => find_records_with_rbac(NetworkRouter, checked_or_params).first
+                          :id         => find_record_with_rbac(NetworkRouter, checked_or_params)
     elsif params[:pressed] == "network_router_remove_interface"
       javascript_redirect :controller => "network_router",
                           :action     => "remove_interface_select",
-                          :id         => find_records_with_rbac(NetworkRouter, checked_or_params).first
+                          :id         => find_record_with_rbac(NetworkRouter, checked_or_params)
     elsif params[:pressed].ends_with?("_edit") || ["#{pfx}_miq_request_new", "#{pfx}_clone",
                                                    "#{pfx}_migrate", "#{pfx}_publish"].include?(params[:pressed])
       render_or_redirect_partial(pfx)

app/controllers/mixins/actions/vm_actions/live_migrate.rb

@@ -22,7 +22,7 @@ def live_migrate
           @in_a_form = true
           @live_migrate = true
 
-          @live_migrate_items = find_records_with_rbac(VmOrTemplate, session[:live_migrate_items]).sort_by(&:name)
+          @live_migrate_items = find_records_with_rbac(VmOrTemplate.order(:name), session[:live_migrate_items])
           build_targets_hash(@live_migrate_items)
           @view = get_db_view(VmOrTemplate)
           @view.table = MiqFilter.records2table(@live_migrate_items, @view.cols + ['id'])
@@ -32,7 +32,7 @@ def live_migrate
 
         def live_migrate_form_fields
           assert_privileges("instance_live_migrate")
-          @record = find_records_with_rbac(VmOrTemplate, [params[:id]]).first
+          @record = find_record_with_rbac(VmOrTemplate, params[:id])
           hosts = []
           unless @record.ext_management_system.nil?
             # wrap in a rescue block in the event the connection to the provider fails
@@ -61,7 +61,7 @@ def live_migrate_vm
           when "cancel"
             add_flash(_("Live migration of Instances was cancelled by the user"))
           when "submit"
-            @live_migrate_items = find_records_with_rbac(VmOrTemplate, session[:live_migrate_items]).sort_by(&:name)
+            @live_migrate_items = find_records_with_rbac(VmOrTemplate.order(:name), session[:live_migrate_items])
             @live_migrate_items.each do |vm|
               if vm.supports_live_migrate?
                 options = {

app/controllers/mixins/actions/vm_actions/retire.rb

@@ -76,7 +76,7 @@ def retire
                   Vm
                 end
           # Check RBAC for all items in session[:retire_items]
-          @retireitems = find_records_with_rbac(kls, session[:retire_items]).sort_by(&:name)
+          @retireitems = find_records_with_rbac(kls.order(:name), session[:retire_items])
           if params[:button]
             flash = retire_handle_form_buttons(kls)
             add_flash(flash)

app/controllers/mixins/actions/vm_actions/right_size.rb

@@ -6,7 +6,7 @@ def vm_right_size
           assert_privileges(params[:pressed])
           # check to see if coming from show_list or drilled into vms from another CI
           rec_cls = "vm"
-          record = find_records_with_rbac(VmOrTemplate, checked_or_params).first
+          record = find_record_with_rbac(VmOrTemplate, checked_or_params)
           if record.nil?
             add_flash(_("One %{model} must be selected to Right-Size Recommendations") %
               {:model => ui_lookup(:table => request.parameters[:controller])}, :error)

app/controllers/mixins/checked_id_mixin.rb

@@ -57,27 +57,6 @@ def find_checked_records_with_rbac(klass, ids = nil)
       filtered
     end
 
-    # !============================================!
-    # PLEASE PREFER find_records_with_rbac OVER THIS
-    # !============================================!
-    #
-    # Test RBAC in case there is only one record
-    # Params:
-    #   klass - class of accessed object
-    #   id    - accessed object id
-    # Returns:
-    #   database record of checked item. If user does not have rights for it,
-    #   raises an exception
-    def find_record_with_rbac(klass, id, options = {})
-      raise _("Invalid input") unless is_integer?(id)
-      tested_object = klass.find_by(:id => id)
-      if tested_object.nil?
-        raise(_("Can't access selected records"))
-      end
-      Rbac.filtered_object(tested_object, :user => current_user, :named_scope => options[:named_scope]) ||
-        raise(_("Can't access selected records"))
-    end
-
     # !============================================!
     # PLEASE PREFER find_records_with_rbac OVER THIS
     # !============================================!
@@ -117,8 +96,25 @@ def checked_or_params_id
     end
 
     # Find a record by model and id and test it with RBAC
+    #
+    # Wrapper for find_records_with_rbac method for case when only a single
+    # record is required
+    #
+    # Params:
+    #   klass   - class of accessed object
+    #   id      - accessed object id
+    #   options - additional options
+    #
+    # Returns:
+    #   Instance of selected item
+    #
+    def find_record_with_rbac(klass, id, options = {})
+      find_records_with_rbac(klass, Array.wrap(id), options).first
+    end
+
+    # Find records by model and id and test it with RBAC
     # Params:
-    #   klass   - class of accessed objects
+    #   klass   - class or scope of accessed objects
     #   ids     - accessed object ids
     #   options - :named_scope :
     #
@@ -127,9 +123,7 @@ def checked_or_params_id
     #   either sets flash or raises exception
     #
     def find_records_with_rbac(klass, ids, options = {})
-      filtered = Rbac.filtered(klass.where(:id => ids),
-                               :user        => current_user,
-                               :named_scope => options[:named_scope])
+      filtered = Rbac.filtered(klass.where(:id => ids), :named_scope => options[:named_scope])
       raise(_("Can't access selected records")) unless ids.length == filtered.length
       filtered
     end

app/controllers/vm_common.rb

@@ -539,7 +539,7 @@ def policy_options
 
   # Set right_size selected db records
   def right_size(record = nil)
-    @record ||= record ? record : find_records_with_rbac(params[:id]).first
+    @record ||= record ? record : find_record_with_rbac(Vm, params[:id])
     @lastaction = "right_size"
     @rightsize = true
     @in_a_form = true

spec/controllers/application_controller_spec.rb

@@ -13,7 +13,7 @@
     end
 
     it "Verify Invalid input flash error message when invalid id is passed in" do
-      expect { controller.send(:find_record_with_rbac, ExtManagementSystem, "invalid") }.to raise_error(RuntimeError, "Invalid input")
+      expect { controller.send(:find_record_with_rbac, ExtManagementSystem, "invalid") }.to raise_error(RuntimeError, "Can't access selected records")
     end
 
     it "Verify flash error message when passed in id no longer exists in database" do

spec/controllers/container_image_controller_spec.rb

@@ -16,6 +16,7 @@
     ApplicationController.handle_exceptions = true
 
     expect(controller).to receive(:check_compliance).and_call_original
+    expect(controller).to receive(:add_flash).with("Error: Record no longer exists in the database", :error, true)
     expect(controller).to receive(:add_flash).with("Container Image no longer exists", :error)
     expect(controller).to receive(:add_flash).with("No Container Images were selected for Compliance Check", :error)
     post :button, :params => { :pressed => 'container_image_check_compliance', :format => :js }