Commit 40088098 authored by GitLab Release Tools Bot's avatar GitLab Release Tools Bot

Merge branch 'security-force-ci-schedule-ownership-14-10' into '14-10-stable-ee'

Prevent maintainers from editing PipelineSchedule

See merge request gitlab-org/security/gitlab!2421
parents 822d06fa 2ce38054
...@@ -7,7 +7,8 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController ...@@ -7,7 +7,8 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
before_action :authorize_play_pipeline_schedule!, only: [:play] before_action :authorize_play_pipeline_schedule!, only: [:play]
before_action :authorize_read_pipeline_schedule! before_action :authorize_read_pipeline_schedule!
before_action :authorize_create_pipeline_schedule!, only: [:new, :create] before_action :authorize_create_pipeline_schedule!, only: [:new, :create]
before_action :authorize_update_pipeline_schedule!, except: [:index, :new, :create, :play] before_action :authorize_update_pipeline_schedule!, only: [:edit, :update]
before_action :authorize_take_ownership_pipeline_schedule!, only: [:take_ownership]
before_action :authorize_admin_pipeline_schedule!, only: [:destroy] before_action :authorize_admin_pipeline_schedule!, only: [:destroy]
feature_category :continuous_integration feature_category :continuous_integration
...@@ -108,6 +109,10 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController ...@@ -108,6 +109,10 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
return access_denied! unless can?(current_user, :update_pipeline_schedule, schedule) return access_denied! unless can?(current_user, :update_pipeline_schedule, schedule)
end end
def authorize_take_ownership_pipeline_schedule!
return access_denied! unless can?(current_user, :take_ownership_pipeline_schedule, schedule)
end
def authorize_admin_pipeline_schedule! def authorize_admin_pipeline_schedule!
return access_denied! unless can?(current_user, :admin_pipeline_schedule, schedule) return access_denied! unless can?(current_user, :admin_pipeline_schedule, schedule)
end end
......
...@@ -15,11 +15,14 @@ module Ci ...@@ -15,11 +15,14 @@ module Ci
rule { can?(:create_pipeline) }.enable :play_pipeline_schedule rule { can?(:create_pipeline) }.enable :play_pipeline_schedule
rule { can?(:admin_pipeline) | (can?(:update_build) & owner_of_schedule) }.policy do rule { can?(:admin_pipeline) | (can?(:update_build) & owner_of_schedule) }.policy do
enable :update_pipeline_schedule
enable :admin_pipeline_schedule enable :admin_pipeline_schedule
enable :read_pipeline_schedule_variables enable :read_pipeline_schedule_variables
end end
rule { admin | (owner_of_schedule & can?(:update_build)) }.policy do
enable :update_pipeline_schedule
end
rule { can?(:admin_pipeline_schedule) & ~owner_of_schedule }.policy do rule { can?(:admin_pipeline_schedule) & ~owner_of_schedule }.policy do
enable :take_ownership_pipeline_schedule enable :take_ownership_pipeline_schedule
end end
......
...@@ -39,6 +39,20 @@ To add a pipeline schedule: ...@@ -39,6 +39,20 @@ To add a pipeline schedule:
These variables are available only when the scheduled pipeline runs, These variables are available only when the scheduled pipeline runs,
and not in any other pipeline run. and not in any other pipeline run.
## Edit a pipeline schedule
> Introduced in GitLab 14.8, only a pipeline schedule owner can edit the schedule.
The owner of a pipeline schedule can edit it:
1. On the top bar, select **Menu > Projects** and find your project.
1. In the left sidebar, select **CI/CD > Schedules**.
1. Next to the schedule, select **Edit** (**{pencil}**) and fill in the form.
The user must have the Developer role or above for the project. If the user is
not the owner of the schedule, they must first [take ownership](#take-ownership)
of the schedule.
## Run manually ## Run manually
To trigger a pipeline schedule manually, so that it runs immediately instead of To trigger a pipeline schedule manually, so that it runs immediately instead of
......
...@@ -93,7 +93,7 @@ module API ...@@ -93,7 +93,7 @@ module API
requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id'
end end
post ':id/pipeline_schedules/:pipeline_schedule_id/take_ownership' do post ':id/pipeline_schedules/:pipeline_schedule_id/take_ownership' do
authorize! :update_pipeline_schedule, pipeline_schedule authorize! :take_ownership_pipeline_schedule, pipeline_schedule
if pipeline_schedule.own!(current_user) if pipeline_schedule.own!(current_user)
present pipeline_schedule, with: Entities::Ci::PipelineScheduleDetails present pipeline_schedule, with: Entities::Ci::PipelineScheduleDetails
......
...@@ -13,10 +13,43 @@ RSpec.describe Projects::PipelineSchedulesController do ...@@ -13,10 +13,43 @@ RSpec.describe Projects::PipelineSchedulesController do
project.add_developer(user) project.add_developer(user)
end end
shared_examples 'access update schedule' do
describe 'security' do
it 'is allowed for admin when admin mode enabled', :enable_admin_mode do
expect { go }.to be_allowed_for(:admin)
end
it 'is denied for admin when admin mode disabled' do
expect { go }.to be_denied_for(:admin)
end
it { expect { go }.to be_denied_for(:owner).of(project) }
it { expect { go }.to be_denied_for(:maintainer).of(project) }
it { expect { go }.to be_denied_for(:developer).of(project) }
it { expect { go }.to be_denied_for(:reporter).of(project) }
it { expect { go }.to be_denied_for(:guest).of(project) }
it { expect { go }.to be_denied_for(:user) }
it { expect { go }.to be_denied_for(:external) }
it { expect { go }.to be_denied_for(:visitor) }
context 'when user is schedule owner' do
it { expect { go }.to be_allowed_for(:owner).of(project).own(pipeline_schedule) }
it { expect { go }.to be_allowed_for(:maintainer).of(project).own(pipeline_schedule) }
it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) }
it { expect { go }.to be_denied_for(:reporter).of(project).own(pipeline_schedule) }
it { expect { go }.to be_denied_for(:guest).of(project).own(pipeline_schedule) }
it { expect { go }.to be_denied_for(:user).own(pipeline_schedule) }
it { expect { go }.to be_denied_for(:external).own(pipeline_schedule) }
it { expect { go }.to be_denied_for(:visitor).own(pipeline_schedule) }
end
end
end
describe 'GET #index' do describe 'GET #index' do
render_views render_views
let(:scope) { nil } let(:scope) { nil }
let!(:inactive_pipeline_schedule) do let!(:inactive_pipeline_schedule) do
create(:ci_pipeline_schedule, :inactive, project: project) create(:ci_pipeline_schedule, :inactive, project: project)
end end
...@@ -130,12 +163,15 @@ RSpec.describe Projects::PipelineSchedulesController do ...@@ -130,12 +163,15 @@ RSpec.describe Projects::PipelineSchedulesController do
it 'is allowed for admin when admin mode enabled', :enable_admin_mode do it 'is allowed for admin when admin mode enabled', :enable_admin_mode do
expect { go }.to be_allowed_for(:admin) expect { go }.to be_allowed_for(:admin)
end end
it 'is denied for admin when admin mode disabled' do it 'is denied for admin when admin mode disabled' do
expect { go }.to be_denied_for(:admin) expect { go }.to be_denied_for(:admin)
end end
it { expect { go }.to be_allowed_for(:owner).of(project) } it { expect { go }.to be_allowed_for(:owner).of(project) }
it { expect { go }.to be_allowed_for(:maintainer).of(project) } it { expect { go }.to be_allowed_for(:maintainer).of(project) }
it { expect { go }.to be_allowed_for(:developer).of(project) } it { expect { go }.to be_allowed_for(:developer).of(project) }
it { expect { go }.to be_denied_for(:reporter).of(project) } it { expect { go }.to be_denied_for(:reporter).of(project) }
it { expect { go }.to be_denied_for(:guest).of(project) } it { expect { go }.to be_denied_for(:guest).of(project) }
it { expect { go }.to be_denied_for(:user) } it { expect { go }.to be_denied_for(:user) }
...@@ -284,20 +320,7 @@ RSpec.describe Projects::PipelineSchedulesController do ...@@ -284,20 +320,7 @@ RSpec.describe Projects::PipelineSchedulesController do
describe 'security' do describe 'security' do
let(:schedule) { { description: 'updated_desc' } } let(:schedule) { { description: 'updated_desc' } }
it 'is allowed for admin when admin mode enabled', :enable_admin_mode do it_behaves_like 'access update schedule'
expect { go }.to be_allowed_for(:admin)
end
it 'is denied for admin when admin mode disabled' do
expect { go }.to be_denied_for(:admin)
end
it { expect { go }.to be_allowed_for(:owner).of(project) }
it { expect { go }.to be_allowed_for(:maintainer).of(project) }
it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) }
it { expect { go }.to be_denied_for(:reporter).of(project) }
it { expect { go }.to be_denied_for(:guest).of(project) }
it { expect { go }.to be_denied_for(:user) }
it { expect { go }.to be_denied_for(:external) }
it { expect { go }.to be_denied_for(:visitor) }
context 'when a developer created a pipeline schedule' do context 'when a developer created a pipeline schedule' do
let(:developer_1) { create(:user) } let(:developer_1) { create(:user) }
...@@ -308,8 +331,10 @@ RSpec.describe Projects::PipelineSchedulesController do ...@@ -308,8 +331,10 @@ RSpec.describe Projects::PipelineSchedulesController do
end end
it { expect { go }.to be_allowed_for(developer_1) } it { expect { go }.to be_allowed_for(developer_1) }
it { expect { go }.to be_denied_for(:owner).of(project) }
it { expect { go }.to be_denied_for(:maintainer).of(project) }
it { expect { go }.to be_denied_for(:developer).of(project) } it { expect { go }.to be_denied_for(:developer).of(project) }
it { expect { go }.to be_allowed_for(:maintainer).of(project) }
end end
context 'when a maintainer created a pipeline schedule' do context 'when a maintainer created a pipeline schedule' do
...@@ -321,17 +346,21 @@ RSpec.describe Projects::PipelineSchedulesController do ...@@ -321,17 +346,21 @@ RSpec.describe Projects::PipelineSchedulesController do
end end
it { expect { go }.to be_allowed_for(maintainer_1) } it { expect { go }.to be_allowed_for(maintainer_1) }
it { expect { go }.to be_allowed_for(:maintainer).of(project) }
it { expect { go }.to be_denied_for(:owner).of(project) }
it { expect { go }.to be_denied_for(:maintainer).of(project) }
it { expect { go }.to be_denied_for(:developer).of(project) } it { expect { go }.to be_denied_for(:developer).of(project) }
end end
end end
def go def go
put :update, params: { namespace_id: project.namespace.to_param, put :update, params: {
project_id: project, namespace_id: project.namespace.to_param,
id: pipeline_schedule, project_id: project,
schedule: schedule }, id: pipeline_schedule,
as: :html schedule: schedule
},
as: :html
end end
end end
...@@ -341,6 +370,7 @@ RSpec.describe Projects::PipelineSchedulesController do ...@@ -341,6 +370,7 @@ RSpec.describe Projects::PipelineSchedulesController do
before do before do
project.add_maintainer(user) project.add_maintainer(user)
pipeline_schedule.update!(owner: user)
sign_in(user) sign_in(user)
end end
...@@ -352,22 +382,7 @@ RSpec.describe Projects::PipelineSchedulesController do ...@@ -352,22 +382,7 @@ RSpec.describe Projects::PipelineSchedulesController do
end end
end end
describe 'security' do it_behaves_like 'access update schedule'
it 'is allowed for admin when admin mode enabled', :enable_admin_mode do
expect { go }.to be_allowed_for(:admin)
end
it 'is denied for admin when admin mode disabled' do
expect { go }.to be_denied_for(:admin)
end
it { expect { go }.to be_allowed_for(:owner).of(project) }
it { expect { go }.to be_allowed_for(:maintainer).of(project) }
it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) }
it { expect { go }.to be_denied_for(:reporter).of(project) }
it { expect { go }.to be_denied_for(:guest).of(project) }
it { expect { go }.to be_denied_for(:user) }
it { expect { go }.to be_denied_for(:external) }
it { expect { go }.to be_denied_for(:visitor) }
end
def go def go
get :edit, params: { namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id } get :edit, params: { namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id }
...@@ -379,17 +394,30 @@ RSpec.describe Projects::PipelineSchedulesController do ...@@ -379,17 +394,30 @@ RSpec.describe Projects::PipelineSchedulesController do
it 'is allowed for admin when admin mode enabled', :enable_admin_mode do it 'is allowed for admin when admin mode enabled', :enable_admin_mode do
expect { go }.to be_allowed_for(:admin) expect { go }.to be_allowed_for(:admin)
end end
it 'is denied for admin when admin mode disabled' do it 'is denied for admin when admin mode disabled' do
expect { go }.to be_denied_for(:admin) expect { go }.to be_denied_for(:admin)
end end
it { expect { go }.to be_allowed_for(:owner).of(project) } it { expect { go }.to be_allowed_for(:owner).of(project) }
it { expect { go }.to be_allowed_for(:maintainer).of(project) } it { expect { go }.to be_allowed_for(:maintainer).of(project) }
it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) } it { expect { go }.to be_denied_for(:developer).of(project) }
it { expect { go }.to be_denied_for(:reporter).of(project) } it { expect { go }.to be_denied_for(:reporter).of(project) }
it { expect { go }.to be_denied_for(:guest).of(project) } it { expect { go }.to be_denied_for(:guest).of(project) }
it { expect { go }.to be_denied_for(:user) } it { expect { go }.to be_denied_for(:user) }
it { expect { go }.to be_denied_for(:external) } it { expect { go }.to be_denied_for(:external) }
it { expect { go }.to be_denied_for(:visitor) } it { expect { go }.to be_denied_for(:visitor) }
context 'when user is schedule owner' do
it { expect { go }.to be_denied_for(:owner).of(project).own(pipeline_schedule) }
it { expect { go }.to be_denied_for(:maintainer).of(project).own(pipeline_schedule) }
it { expect { go }.to be_denied_for(:developer).of(project).own(pipeline_schedule) }
it { expect { go }.to be_denied_for(:reporter).of(project).own(pipeline_schedule) }
it { expect { go }.to be_denied_for(:guest).of(project).own(pipeline_schedule) }
it { expect { go }.to be_denied_for(:user).own(pipeline_schedule) }
it { expect { go }.to be_denied_for(:external).own(pipeline_schedule) }
it { expect { go }.to be_denied_for(:visitor).own(pipeline_schedule) }
end
end end
def go def go
......
...@@ -9,7 +9,77 @@ RSpec.describe 'Pipeline Schedules', :js do ...@@ -9,7 +9,77 @@ RSpec.describe 'Pipeline Schedules', :js do
let(:scope) { nil } let(:scope) { nil }
let!(:user) { create(:user) } let!(:user) { create(:user) }
context 'logged in as maintainer' do context 'logged in as the pipeline scheduler owner' do
before do
stub_feature_flags(bootstrap_confirmation_modals: false)
project.add_developer(user)
pipeline_schedule.update!(owner: user)
gitlab_sign_in(user)
end
describe 'GET /projects/pipeline_schedules' do
before do
visit_pipelines_schedules
end
it 'edits the pipeline' do
page.within('.pipeline-schedule-table-row') do
click_link 'Edit'
end
expect(page).to have_content('Edit Pipeline Schedule')
end
end
describe 'PATCH /projects/pipelines_schedules/:id/edit' do
before do
edit_pipeline_schedule
end
it 'displays existing properties' do
description = find_field('schedule_description').value
expect(description).to eq('pipeline schedule')
expect(page).to have_button('master')
expect(page).to have_button('UTC')
end
it 'edits the scheduled pipeline' do
fill_in 'schedule_description', with: 'my brand new description'
save_pipeline_schedule
expect(page).to have_content('my brand new description')
end
context 'when ref is nil' do
before do
pipeline_schedule.update_attribute(:ref, nil)
edit_pipeline_schedule
end
it 'shows the pipeline schedule with default ref' do
page.within('[data-testid="schedule-target-ref"]') do
expect(first('.gl-new-dropdown-button-text').text).to eq('master')
end
end
end
context 'when ref is empty' do
before do
pipeline_schedule.update_attribute(:ref, '')
edit_pipeline_schedule
end
it 'shows the pipeline schedule with default ref' do
page.within('[data-testid="schedule-target-ref"]') do
expect(first('.gl-new-dropdown-button-text').text).to eq('master')
end
end
end
end
end
context 'logged in as a project maintainer' do
before do before do
stub_feature_flags(bootstrap_confirmation_modals: false) stub_feature_flags(bootstrap_confirmation_modals: false)
project.add_maintainer(user) project.add_maintainer(user)
...@@ -46,14 +116,6 @@ RSpec.describe 'Pipeline Schedules', :js do ...@@ -46,14 +116,6 @@ RSpec.describe 'Pipeline Schedules', :js do
end end
end end
it 'edits the pipeline' do
page.within('.pipeline-schedule-table-row') do
click_link 'Edit'
end
expect(page).to have_content('Edit Pipeline Schedule')
end
it 'deletes the pipeline' do it 'deletes the pipeline' do
accept_confirm { click_link 'Delete' } accept_confirm { click_link 'Delete' }
...@@ -108,53 +170,6 @@ RSpec.describe 'Pipeline Schedules', :js do ...@@ -108,53 +170,6 @@ RSpec.describe 'Pipeline Schedules', :js do
end end
end end
describe 'PATCH /projects/pipelines_schedules/:id/edit' do
before do
edit_pipeline_schedule
end
it 'displays existing properties' do
description = find_field('schedule_description').value
expect(description).to eq('pipeline schedule')
expect(page).to have_button('master')
expect(page).to have_button('UTC')
end
it 'edits the scheduled pipeline' do
fill_in 'schedule_description', with: 'my brand new description'
save_pipeline_schedule
expect(page).to have_content('my brand new description')
end
context 'when ref is nil' do
before do
pipeline_schedule.update_attribute(:ref, nil)
edit_pipeline_schedule
end
it 'shows the pipeline schedule with default ref' do
page.within('[data-testid="schedule-target-ref"]') do
expect(first('.gl-new-dropdown-button-text').text).to eq('master')
end
end
end
context 'when ref is empty' do
before do
pipeline_schedule.update_attribute(:ref, '')
edit_pipeline_schedule
end
it 'shows the pipeline schedule with default ref' do
page.within('[data-testid="schedule-target-ref"]') do
expect(first('.gl-new-dropdown-button-text').text).to eq('master')
end
end
end
end
context 'when user creates a new pipeline schedule with variables' do context 'when user creates a new pipeline schedule with variables' do
before do before do
visit_pipelines_schedules visit_pipelines_schedules
......
...@@ -84,11 +84,14 @@ RSpec.describe Ci::PipelineSchedulePolicy, :models do ...@@ -84,11 +84,14 @@ RSpec.describe Ci::PipelineSchedulePolicy, :models do
project.add_maintainer(user) project.add_maintainer(user)
end end
it 'includes abilities to do all operations on pipeline schedule' do it 'allows for playing and destroying a pipeline schedule' do
expect(policy).to be_allowed :play_pipeline_schedule expect(policy).to be_allowed :play_pipeline_schedule
expect(policy).to be_allowed :update_pipeline_schedule
expect(policy).to be_allowed :admin_pipeline_schedule expect(policy).to be_allowed :admin_pipeline_schedule
end end
it 'does not allow for updating of an existing schedule' do
expect(policy).not_to be_allowed :update_pipeline_schedule
end
end end
describe 'rules for non-owner of schedule' do describe 'rules for non-owner of schedule' do
......
...@@ -291,10 +291,36 @@ RSpec.describe API::Ci::PipelineSchedules do ...@@ -291,10 +291,36 @@ RSpec.describe API::Ci::PipelineSchedules do
end end
context 'authenticated user with invalid permissions' do context 'authenticated user with invalid permissions' do
it 'does not update pipeline_schedule' do context 'as a project maintainer' do
put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user) before do
project.add_maintainer(user)
end
expect(response).to have_gitlab_http_status(:not_found) it 'does not update pipeline_schedule' do
put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user)
expect(response).to have_gitlab_http_status(:forbidden)
end
end
context 'as a project owner' do
before do
project.add_owner(user)
end
it 'does not update pipeline_schedule' do
put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user)
expect(response).to have_gitlab_http_status(:forbidden)
end
end
context 'with no special role' do
it 'does not update pipeline_schedule' do
put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user)
expect(response).to have_gitlab_http_status(:not_found)
end
end end
end end
...@@ -312,16 +338,21 @@ RSpec.describe API::Ci::PipelineSchedules do ...@@ -312,16 +338,21 @@ RSpec.describe API::Ci::PipelineSchedules do
create(:ci_pipeline_schedule, project: project, owner: developer) create(:ci_pipeline_schedule, project: project, owner: developer)
end end
context 'authenticated user with valid permissions' do let(:project_maintainer) do
create(:user).tap { |u| project.add_maintainer(u) }
end
context 'as an authenticated user with valid permissions' do
it 'updates owner' do it 'updates owner' do
post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", developer) expect { post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", project_maintainer) }
.to change { pipeline_schedule.reload.owner }.from(developer).to(project_maintainer)
expect(response).to have_gitlab_http_status(:created) expect(response).to have_gitlab_http_status(:created)
expect(response).to match_response_schema('pipeline_schedule') expect(response).to match_response_schema('pipeline_schedule')
end end
end end
context 'authenticated user with invalid permissions' do context 'as an authenticated user with invalid permissions' do
it 'does not update owner' do it 'does not update owner' do
post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", user) post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", user)
...@@ -329,13 +360,23 @@ RSpec.describe API::Ci::PipelineSchedules do ...@@ -329,13 +360,23 @@ RSpec.describe API::Ci::PipelineSchedules do
end end
end end
context 'unauthenticated user' do context 'as an unauthenticated user' do
it 'does not update owner' do it 'does not update owner' do
post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership") post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership")
expect(response).to have_gitlab_http_status(:unauthorized) expect(response).to have_gitlab_http_status(:unauthorized)
end end
end end
context 'as the existing owner of the schedule' do
it 'rejects the request and leaves the schedule unchanged' do
expect do
post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", developer)
end.not_to change { pipeline_schedule.reload.owner }
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end end
describe 'DELETE /projects/:id/pipeline_schedules/:pipeline_schedule_id' do describe 'DELETE /projects/:id/pipeline_schedules/:pipeline_schedule_id' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment