Add functionality and security.

app/controllers/projects/pipeline_schedules_controller.rb

@@ -33,6 +33,8 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
   end
 
   def update
+    return access_denied! unless can?(current_user, :update_pipeline_schedule, schedule)
+
     if Ci::CreatePipelineScheduleService
         .new(@project, current_user, schedule_params).update(schedule)
       redirect_to namespace_project_pipeline_schedules_path(@project.namespace.becomes(Namespace), @project)

app/policies/ci/pipeline_schedule_policy.rb

@@ -2,24 +2,14 @@ module Ci
   class PipelineSchedulePolicy < PipelinePolicy
     alias_method :pipeline_schedule, :subject
 
-    condition(:protected_action) do
-      owned_by_developer? && owned_by_another?
-    end
-
-    rule { protected_action }.prevent :update_pipeline_schedule
-
-    private
+    def rules
+      super
 
-    def owned_by_developer?
-      return false unless @user
+      access = pipeline_schedule.project.team.max_member_access(user.id)
 
-      pipeline_schedule.project.team.developer?(@user)
+      if access == Gitlab::Access::DEVELOPER && pipeline_schedule.owner != user
+        cannot! :update_pipeline_schedule
       end
-
-    def owned_by_another?
-      return false unless @user
-
-      !pipeline_schedule.owned_by?(@user)
     end
   end
 end

spec/controllers/projects/pipeline_schedules_controller_spec.rb

 require 'spec_helper'
 
 describe Projects::PipelineSchedulesController do
+  include AccessMatchersForController
+
   set(:project) { create(:empty_project, :public) }
   let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project) }
 
@@ -53,6 +55,7 @@ describe Projects::PipelineSchedulesController do
   end
 
   describe 'POST #create' do
+    describe 'functionality' do
       before do
         create(:user).tap do |user|
           project.add_developer(user)
@@ -65,9 +68,7 @@ describe Projects::PipelineSchedulesController do
       end
 
       context 'when variables_attributes is empty' do
-      let(:schedule) do
-        basic_param
-      end
+        let(:schedule) { basic_param }
 
         it 'creates a new schedule' do
           expect { post :create, namespace_id: project.namespace.to_param, project_id: project, schedule: schedule }
@@ -158,19 +159,44 @@ describe Projects::PipelineSchedulesController do
       end
     end
 
+    describe 'security' do
+      let(:action) do
+        proc do |user|
+          post :create, namespace_id: project.namespace.to_param, 
+            project_id: project,
+            schedule: { description: 'aaaaaaaa', cron: '0 4 * * *',
+              cron_timezone: 'UTC', ref: 'master', active: '1' }
+        end
+      end
+
+      specify { expect(action).to be_allowed_for(:admin) }
+      specify { expect(action).to be_allowed_for(:owner).of(project) }
+      specify { expect(action).to be_allowed_for(:master).of(project) }
+      specify { expect(action).to be_allowed_for(:developer).of(project) }
+      specify { expect(action).to be_denied_for(:reporter).of(project) }
+      specify { expect(action).to be_denied_for(:guest).of(project) }
+      specify { expect(action).to be_denied_for(:user) }
+      specify { expect(action).to be_denied_for(:external) }
+      specify { expect(action).to be_denied_for(:visitor) }
+    end
+  end
+
   describe 'PUT #update' do
+    describe 'functionality' do
+      let(:user) { create(:user) }
+      let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: user) }
+
       before do
-      create(:user).tap do |user|
         project.add_developer(user)
+
         sign_in(user)
       end
-    end
 
+      context 'when a pipeline schedule has no variables' do
         let(:basic_param) do
           { description: 'updated_desc', cron: '0 1 * * *', cron_timezone: 'UTC', ref: 'patch-x', active: '1' }
         end
 
-    context 'when a pipeline schedule has no variables' do
         context 'when params do not include variables' do
           let(:schedule) { basic_param }
 
@@ -251,6 +277,10 @@ describe Projects::PipelineSchedulesController do
       end
 
       context 'when a pipeline schedule has one variable' do
+        let(:basic_param) do
+          { description: 'updated_desc', cron: '0 1 * * *', cron_timezone: 'UTC', ref: 'patch-x', active: '1' }
+        end
+
         let!(:pipeline_schedule_variable) do
           create(:ci_pipeline_schedule_variable, key: 'CCC',
             pipeline_schedule: pipeline_schedule)
@@ -331,65 +361,116 @@ describe Projects::PipelineSchedulesController do
       end
     end
 
-  describe 'GET edit' do
-    context 'TODO: integrate to bottom' do
-      let(:user) { create(:user) }
+    describe 'security' do
+      context 'when a developer created a pipeline schedule' do
+        let(:developer_1) { create(:user) }
+        let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: developer_1) }
 
         before do
-        project.add_master(user)
+          project.add_developer(developer_1)
+        end
 
-        sign_in(user)
+        context 'when the developer updates' do
+          let(:action) do
+            proc do |user|
+              put :update, namespace_id: project.namespace.to_param,
+                project_id: project, id: pipeline_schedule,
+                schedule: { description: 'updated_desc' }
+            end
           end
 
-      it 'loads the pipeline schedule' do
-        get :edit, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
+          specify { expect(action).to be_allowed_for(developer_1) }
+        end
 
-        expect(response).to have_http_status(:ok)
-        expect(assigns(:schedule)).to eq(pipeline_schedule)
+        context 'when another developer updates' do
+          let(:action) do
+            proc do |user|
+              put :update, namespace_id: project.namespace.to_param,
+                project_id: project, id: pipeline_schedule,
+                schedule: { description: 'updated_desc' }
             end
           end
 
-    context 'when a developer created a pipeline schedule' do
-      context 'when the developer edits' do
-        it 'can edit variables' do
-          # TODO: 
+          specify { expect(action).to be_denied_for(:developer).of(project) }
+        end
+
+        context 'when a master updates' do
+          let(:action) do
+            proc do |user|
+              put :update, namespace_id: project.namespace.to_param,
+                project_id: project, id: pipeline_schedule,
+                schedule: { description: 'updated_desc' }
             end
           end
 
-      context 'when other developers edit' do
-        it 'can not edit variables' do
-          # TODO: 
+          specify { expect(action).to be_allowed_for(:master).of(project) }
+        end
       end
+
+      context 'when a master created a pipeline schedule' do
+        let(:master_1) { create(:user) }
+        let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: master_1) }
+
+        before do
+          project.add_master(master_1)
         end
 
-      context 'when a master edits' do
-        it 'can edit variables' do
-          # TODO: 
+        context 'when the master updates' do
+          let(:action) do
+            proc do |user|
+              put :update, namespace_id: project.namespace.to_param,
+                project_id: project, id: pipeline_schedule,
+                schedule: { description: 'updated_desc' }
             end
           end
+
+          specify { expect(action).to be_allowed_for(master_1) }
         end
 
-    context 'when a master created a pipeline schedule' do
-      context 'when the master edits' do
-        it 'can edit variables' do
-          # TODO: 
+        context 'when other masters updates' do
+          let(:action) do
+            proc do |user|
+              put :update, namespace_id: project.namespace.to_param,
+                project_id: project, id: pipeline_schedule,
+                schedule: { description: 'updated_desc' }
+            end
           end
+
+          specify { expect(action).to be_allowed_for(:master).of(project) }
         end
 
-      context 'when other masters edit' do
-        it 'can edit variables' do
-          # TODO: 
+        context 'when a developer updates' do
+          let(:action) do
+            proc do |user|
+              put :update, namespace_id: project.namespace.to_param,
+                project_id: project, id: pipeline_schedule,
+                schedule: { description: 'updated_desc' }
             end
           end
 
-      context 'when developers edit' do
-        it 'can not edit variables' do
-          # TODO: 
+          specify { expect(action).to be_denied_for(:developer).of(project) }
         end
       end
     end
   end
 
+  describe 'GET edit' do
+    let(:user) { create(:user) }
+
+    before do
+      project.add_master(user)
+
+      sign_in(user)
+    end
+
+    it 'loads the pipeline schedule' do
+      get :edit, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
+
+      expect(response).to have_http_status(:ok)
+      expect(assigns(:schedule)).to eq(pipeline_schedule)
+    end
+  end
+
   describe 'DELETE #destroy' do
     set(:user) { create(:user) }