Ensure a users own tasks are the only ones returned when the users ro…

…le has View/My Tasks

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1639387

app/controllers/api/base_controller/renderer.rb

@@ -132,12 +132,16 @@ def resource_search(id, type, klass)
           if respond_to?("find_#{type}")
             public_send("find_#{type}", id)
           else
-            key_id == "id" ? klass.find(id) : klass.find_by(key_id => id)
+            find_resource(klass, key_id, id)
           end
         raise NotFoundError, "Couldn't find #{klass} with '#{key_id}'=#{id}" unless target
         filter_resource(target, type, klass)
       end
 
+      def find_resource(klass, key_id, id)
+        key_id == "id" ? klass.find(id) : klass.find_by(key_id => id)
+      end
+
       def filter_resource(target, type, klass)
         res = Rbac.filtered_object(target, :user => User.current_user, :class => klass)
         raise ForbiddenError, "Access to the resource #{type}/#{target.id} is forbidden" unless res
@@ -151,13 +155,17 @@ def collection_search(is_subcollection, type, klass)
           elsif by_tag_param
             klass.find_tagged_with(:all => by_tag_param, :ns => TAG_NAMESPACE, :separator => ',')
           else
-            klass.all
+            find_collection(klass)
           end
 
         res = res.where(public_send("#{type}_search_conditions")) if respond_to?("#{type}_search_conditions")
         collection_filterer(res, type, klass, is_subcollection)
       end
 
+      def find_collection(klass)
+        klass.all
+      end
+
       def collection_filterer(res, type, klass, is_subcollection = false)
         miq_expression = filter_param(klass)
 

app/controllers/api/tasks_controller.rb

@@ -1,4 +1,15 @@
 module Api
   class TasksController < BaseController
+    def find_collection(klass)
+      return klass.where(:userid => [current_user.userid]) if current_user.only_my_user_tasks?
+
+      klass.all
+    end
+
+    def find_resource(klass, key_id, id)
+      return klass.find_by(key_id => id, :userid => [current_user.userid]) if current_user.only_my_user_tasks?
+
+      key_id == "id" ? klass.find(id) : klass.find_by(key_id => id)
+    end
   end
 end

spec/requests/tasks_spec.rb

@@ -1,24 +1,44 @@
 describe 'TasksController' do
-  let(:task) { FactoryBot.create(:miq_task, :state => MiqTask::STATE_FINISHED) }
-  let(:task2) { FactoryBot.create(:miq_task, :state => MiqTask::STATE_FINISHED) }
+  let(:task)    { FactoryBot.create(:miq_task, :state => MiqTask::STATE_FINISHED, :userid => "testuser") }
+  let(:task2)   { FactoryBot.create(:miq_task, :state => MiqTask::STATE_FINISHED) }
+  let(:my_task) { FactoryBot.create(:miq_task, :state => MiqTask::STATE_FINISHED, :userid => "api_user_id") }
 
   def expect_deleted(*args)
     args.each do |arg|
       expect(MiqTask.find_by(:id => arg.id)).to be_nil
     end
   end
 
+  def expect_not_deleted(*args)
+    expect(MiqTask.where(:id => args.collect(&:id)).length).to eq(args.length)
+  end
+
+  it 'will not delete other users tasks on DELETE when role is miq_task_my_ui' do
+    api_basic_authorize 'miq_task_my_ui', resource_action_identifier(:tasks, :delete, :delete)
+    delete(api_task_url(nil, task))
+    expect_not_deleted(task)
+  end
+
   it 'deletes on DELETE' do
-    api_basic_authorize resource_action_identifier(:tasks, :delete, :delete)
+    api_basic_authorize 'miq_task_all_ui', resource_action_identifier(:tasks, :delete, :delete)
 
     delete(api_task_url(nil, task))
 
     expect(response).to have_http_status(:no_content) # 204
     expect_deleted(task)
   end
 
+  it 'will not delete other users tasks on POST when role is miq_task_my_ui' do
+    api_basic_authorize 'miq_task_my_ui', resource_action_identifier(:tasks, :delete)
+    data = {
+      :action => 'delete'
+    }
+    post(api_task_url(nil, task), :params => data)
+    expect_not_deleted(task)
+  end
+
   it 'deletes on POST' do
-    api_basic_authorize resource_action_identifier(:tasks, :delete)
+    api_basic_authorize 'miq_task_all_ui', resource_action_identifier(:tasks, :delete)
 
     data = {
       :action => 'delete'
@@ -36,7 +56,7 @@ def expect_deleted(*args)
   end
 
   it 'bulk deletes' do
-    api_basic_authorize collection_action_identifier(:tasks, :delete)
+    api_basic_authorize 'miq_task_all_ui', collection_action_identifier(:tasks, :delete)
 
     data = {
       :action    => 'delete',
@@ -99,14 +119,20 @@ def expect_deleted(*args)
       expect(response.parsed_body).to include(expected)
     end
 
+    it 'does not returns a task for other users when role is miq_task_my_ui' do
+      api_basic_authorize('miq_task_my_ui')
+      get(api_task_url(nil, task))
+      expect(response.parsed_body["error"]["message"]).to include("Couldn't find MiqTask")
+    end
+
     it 'returns a task miq_task_my_ui role' do
       api_basic_authorize('miq_task_my_ui')
 
-      get(api_task_url(nil, task))
+      get(api_task_url(nil, my_task))
 
       expected = {
-        'href' => api_task_url(nil, task),
-        'name' => task.name
+        'href' => api_task_url(nil, my_task),
+        'name' => my_task.name
       }
       expect(response).to have_http_status(:ok)
       expect(response.parsed_body).to include(expected)