Remove update|admin_pipeline_schedule from Project, and grant it in PipelineSchedule

5 files changed, 16 insertions, 22 deletions

diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb
index ca0dbdf7b05..ec7c645df5a 100644
--- a/app/controllers/projects/pipeline_schedules_controller.rb
+++ b/app/controllers/projects/pipeline_schedules_controller.rb
@@ -73,4 +73,8 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
   def authorize_update_pipeline_schedule!
     return access_denied! unless can?(current_user, :update_pipeline_schedule, schedule)
   end
+
+  def authorize_admin_pipeline_schedule!
+    return access_denied! unless can?(current_user, :admin_pipeline_schedule, schedule)
+  end
 end
diff --git a/app/policies/ci/pipeline_schedule_policy.rb b/app/policies/ci/pipeline_schedule_policy.rb
index db561dafbd3..6b7598e1821 100644
--- a/app/policies/ci/pipeline_schedule_policy.rb
+++ b/app/policies/ci/pipeline_schedule_policy.rb
@@ -2,24 +2,13 @@ module Ci
   class PipelineSchedulePolicy < PipelinePolicy
     alias_method :pipeline_schedule, :subject
 
-    condition(:protected_action) do
-      owned_by_developer? && owned_by_another?
+    condition(:owner_of_schedule) do
+      can?(:developer_access) && pipeline_schedule.owned_by?(@user)
     end
 
-    rule { protected_action }.prevent :update_pipeline_schedule
-
-    private
-
-    def owned_by_developer?
-      return false unless @user
-
-      pipeline_schedule.project.team.developer?(@user)
-    end
-
-    def owned_by_another?
-      return false unless @user
-
-      !pipeline_schedule.owned_by?(@user)
+    rule { can?(:master_access) | owner_of_schedule }.policy do
+      enable :update_pipeline_schedule
+      enable :admin_pipeline_schedule
     end
   end
 end
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index 7cbca63fab4..323131c0f7e 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -162,7 +162,6 @@ class ProjectPolicy < BasePolicy
     enable :create_pipeline
     enable :update_pipeline
     enable :create_pipeline_schedule
-    enable :update_pipeline_schedule
     enable :create_merge_request
     enable :create_wiki
     enable :push_code
@@ -188,7 +187,6 @@ class ProjectPolicy < BasePolicy
     enable :admin_build
     enable :admin_container_image
     enable :admin_pipeline
-    enable :admin_pipeline_schedule
     enable :admin_environment
     enable :admin_deployment
     enable :admin_pages
diff --git a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
index 08ccd57c81a..97c0407a01d 100644
--- a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
+++ b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
@@ -26,7 +26,7 @@
           = pipeline_schedule.owner&.name
     %td
       .pull-right.btn-group
-        - if can?(current_user, :update_pipeline_schedule, @project) && !pipeline_schedule.owned_by?(current_user)
+        - if can?(current_user, :update_pipeline_schedule, pipeline_schedule)
           = link_to take_ownership_pipeline_schedule_path(pipeline_schedule), method: :post, title: s_('PipelineSchedules|Take ownership'), class: 'btn' do
             = s_('PipelineSchedules|Take ownership')
         - if can?(current_user, :update_pipeline_schedule, pipeline_schedule)
diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb
index 93d89209934..dbeaf9e17ef 100644
--- a/lib/api/pipeline_schedules.rb
+++ b/lib/api/pipeline_schedules.rb
@@ -74,9 +74,10 @@ module API
         optional :active, type: Boolean, desc: 'The activation of pipeline schedule'
       end
       put ':id/pipeline_schedules/:pipeline_schedule_id' do
-        authorize! :update_pipeline_schedule, user_project
+        authorize! :read_pipeline_schedule, user_project
 
         not_found!('PipelineSchedule') unless pipeline_schedule
+        authorize! :update_pipeline_schedule, pipeline_schedule
 
         if pipeline_schedule.update(declared_params(include_missing: false))
           present pipeline_schedule, with: Entities::PipelineScheduleDetails
@@ -92,9 +93,10 @@ module API
         requires :pipeline_schedule_id, type: Integer,  desc: 'The pipeline schedule id'
       end
       post ':id/pipeline_schedules/:pipeline_schedule_id/take_ownership' do
-        authorize! :update_pipeline_schedule, user_project
+        authorize! :read_pipeline_schedule, user_project
 
         not_found!('PipelineSchedule') unless pipeline_schedule
+        authorize! :update_pipeline_schedule, pipeline_schedule
 
         if pipeline_schedule.own!(current_user)
           present pipeline_schedule, with: Entities::PipelineScheduleDetails
@@ -110,9 +112,10 @@ module API
         requires :pipeline_schedule_id, type: Integer,  desc: 'The pipeline schedule id'
       end
       delete ':id/pipeline_schedules/:pipeline_schedule_id' do
-        authorize! :admin_pipeline_schedule, user_project
+        authorize! :read_pipeline_schedule, user_project
 
         not_found!('PipelineSchedule') unless pipeline_schedule
+        authorize! :admin_pipeline_schedule, pipeline_schedule
 
         status :accepted
         present pipeline_schedule.destroy, with: Entities::PipelineScheduleDetails