Add functionality and security.

This commit is contained in:
Shinya Maeda 2017-06-26 18:25:08 +09:00
parent 3ea04616c3
commit 1247ae0de9
3 changed files with 335 additions and 241 deletions

View File

@ -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)

View File

@ -1,4 +1,15 @@
module Ci
class PipelineSchedulePolicy < PipelinePolicy
alias_method :pipeline_schedule, :subject
def rules
super
access = pipeline_schedule.project.team.max_member_access(user.id)
if access == Gitlab::Access::DEVELOPER && pipeline_schedule.owner != user
cannot! :update_pipeline_schedule
end
end
end
end

View File

@ -1,6 +1,8 @@
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,340 +55,419 @@ describe Projects::PipelineSchedulesController do
end
describe 'POST #create' do
before do
create(:user).tap do |user|
project.add_developer(user)
sign_in(user)
end
end
let(:basic_param) do
{ description: 'aaaaaaaa', cron: '0 4 * * *', cron_timezone: 'UTC', ref: 'master', active: '1' }
end
context 'when variables_attributes is empty' do
let(:schedule) do
basic_param
end
it 'creates a new schedule' do
expect { post :create, namespace_id: project.namespace.to_param, project_id: project, schedule: schedule }
.to change { Ci::PipelineSchedule.count }.by(1)
.and change { Ci::PipelineScheduleVariable.count }.by(0)
expect(response).to have_http_status(:found)
end
end
context 'when variables_attributes has one variable' do
let(:schedule) do
basic_param.merge({
variables_attributes: [ { key: 'AAA', value: 'AAA123' } ]
})
end
it 'creates a new schedule' do
expect { post :create, namespace_id: project.namespace.to_param, project_id: project, schedule: schedule }
.to change { Ci::PipelineSchedule.count }.by(1)
.and change { Ci::PipelineScheduleVariable.count }.by(1)
expect(response).to have_http_status(:found)
expect(Ci::PipelineScheduleVariable.last.key).to eq("AAA")
expect(Ci::PipelineScheduleVariable.last.value).to eq("AAA123")
end
context 'when the same key has already been persisted' do
it 'returns an error that the key of variable is invaild' do
post :create, namespace_id: project.namespace.to_param, project_id: project, schedule: schedule
pipeline_schedule_variable = build(:ci_pipeline_schedule_variable, key: 'AAA', pipeline_schedule: assigns(:schedule))
expect(pipeline_schedule_variable).to be_invalid
describe 'functionality' do
before do
create(:user).tap do |user|
project.add_developer(user)
sign_in(user)
end
end
end
context 'when variables_attributes has one variable and key is empty' do
let(:schedule) do
basic_param.merge({
variables_attributes: [ { key: '', value: 'AAA123' } ]
})
let(:basic_param) do
{ description: 'aaaaaaaa', cron: '0 4 * * *', cron_timezone: 'UTC', ref: 'master', active: '1' }
end
it 'returns an error that the key of variable is invaild' do
expect { post :create, namespace_id: project.namespace.to_param, project_id: project, schedule: schedule }
.to change { Ci::PipelineSchedule.count }.by(0)
.and change { Ci::PipelineScheduleVariable.count }.by(0)
expect(assigns(:schedule).errors['variables.key']).not_to be_empty
end
end
context 'when variables_attributes has two variables and unique' do
let(:schedule) do
basic_param.merge({
variables_attributes: [ { key: 'AAA', value: 'AAA123' }, { key: 'BBB', value: 'BBB123' } ]
})
end
it 'creates a new schedule' do
expect { post :create, namespace_id: project.namespace.to_param, project_id: project, schedule: schedule }
.to change { Ci::PipelineSchedule.count }.by(1)
.and change { Ci::PipelineScheduleVariable.count }.by(2)
expect(response).to have_http_status(:found)
expect(Ci::PipelineScheduleVariable.first.key).to eq("AAA")
expect(Ci::PipelineScheduleVariable.first.value).to eq("AAA123")
expect(Ci::PipelineScheduleVariable.last.key).to eq("BBB")
expect(Ci::PipelineScheduleVariable.last.value).to eq("BBB123")
end
end
context 'when variables_attributes has two variables and duplicted' do
let(:schedule) do
basic_param.merge({
variables_attributes: [ { key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' } ]
})
end
it 'returns an error that the keys of variable are duplicated' do
expect { post :create, namespace_id: project.namespace.to_param, project_id: project, schedule: schedule }
.to change { Ci::PipelineSchedule.count }.by(0)
.and change { Ci::PipelineScheduleVariable.count }.by(0)
expect(assigns(:schedule).errors['variables.key']).not_to be_empty
end
end
end
describe 'PUT #update' do
before do
create(:user).tap do |user|
project.add_developer(user)
sign_in(user)
end
end
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
context 'when variables_attributes is empty' do
let(:schedule) { basic_param }
it 'updates only scheduled pipeline attributes' do
put :update, namespace_id: project.namespace.to_param,
project_id: project, id: pipeline_schedule, schedule: schedule
pipeline_schedule.reload
it 'creates a new schedule' do
expect { post :create, namespace_id: project.namespace.to_param, project_id: project, schedule: schedule }
.to change { Ci::PipelineSchedule.count }.by(1)
.and change { Ci::PipelineScheduleVariable.count }.by(0)
expect(response).to have_http_status(:found)
expect(pipeline_schedule.description).to eq('updated_desc')
expect(pipeline_schedule.cron).to eq('0 1 * * *')
expect(pipeline_schedule.cron_timezone).to eq('UTC')
expect(pipeline_schedule.ref).to eq('patch-x')
expect(pipeline_schedule.active).to eq(true)
expect(pipeline_schedule.variables).to be_empty
end
end
context 'when params include one variable' do
context 'when variables_attributes has one variable' do
let(:schedule) do
basic_param.merge({
variables_attributes: [ { key: 'AAA', value: 'AAA123' } ]
})
end
it 'inserts new variable to the pipeline schedule' do
expect do
put :update, namespace_id: project.namespace.to_param,
project_id: project, id: pipeline_schedule, schedule: schedule
end.to change { Ci::PipelineScheduleVariable.count }.by(1)
pipeline_schedule.reload
it 'creates a new schedule' do
expect { post :create, namespace_id: project.namespace.to_param, project_id: project, schedule: schedule }
.to change { Ci::PipelineSchedule.count }.by(1)
.and change { Ci::PipelineScheduleVariable.count }.by(1)
expect(response).to have_http_status(:found)
expect(pipeline_schedule.variables.last.key).to eq('AAA')
expect(pipeline_schedule.variables.last.value).to eq('AAA123')
expect(Ci::PipelineScheduleVariable.last.key).to eq("AAA")
expect(Ci::PipelineScheduleVariable.last.value).to eq("AAA123")
end
context 'when the same key has already been persisted' do
it 'returns an error that the key of variable is invaild' do
post :create, namespace_id: project.namespace.to_param, project_id: project, schedule: schedule
pipeline_schedule_variable = build(:ci_pipeline_schedule_variable, key: 'AAA', pipeline_schedule: assigns(:schedule))
expect(pipeline_schedule_variable).to be_invalid
end
end
end
context 'when params include two unique variables' do
context 'when variables_attributes has one variable and key is empty' do
let(:schedule) do
basic_param.merge({
variables_attributes: [ { key: '', value: 'AAA123' } ]
})
end
it 'returns an error that the key of variable is invaild' do
expect { post :create, namespace_id: project.namespace.to_param, project_id: project, schedule: schedule }
.to change { Ci::PipelineSchedule.count }.by(0)
.and change { Ci::PipelineScheduleVariable.count }.by(0)
expect(assigns(:schedule).errors['variables.key']).not_to be_empty
end
end
context 'when variables_attributes has two variables and unique' do
let(:schedule) do
basic_param.merge({
variables_attributes: [ { key: 'AAA', value: 'AAA123' }, { key: 'BBB', value: 'BBB123' } ]
})
end
it 'inserts two new variables to the pipeline schedule' do
expect do
put :update, namespace_id: project.namespace.to_param,
project_id: project, id: pipeline_schedule, schedule: schedule
end.to change { Ci::PipelineScheduleVariable.count }.by(2)
pipeline_schedule.reload
it 'creates a new schedule' do
expect { post :create, namespace_id: project.namespace.to_param, project_id: project, schedule: schedule }
.to change { Ci::PipelineSchedule.count }.by(1)
.and change { Ci::PipelineScheduleVariable.count }.by(2)
expect(response).to have_http_status(:found)
expect(pipeline_schedule.variables.first.key).to eq('AAA')
expect(pipeline_schedule.variables.first.value).to eq('AAA123')
expect(pipeline_schedule.variables.last.key).to eq('BBB')
expect(pipeline_schedule.variables.last.value).to eq('BBB123')
expect(Ci::PipelineScheduleVariable.first.key).to eq("AAA")
expect(Ci::PipelineScheduleVariable.first.value).to eq("AAA123")
expect(Ci::PipelineScheduleVariable.last.key).to eq("BBB")
expect(Ci::PipelineScheduleVariable.last.value).to eq("BBB123")
end
end
context 'when params include two duplicated variables' do
context 'when variables_attributes has two variables and duplicted' do
let(:schedule) do
basic_param.merge({
variables_attributes: [ { key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' } ]
})
end
it 'returns an error that variables are duplciated' do
put :update, namespace_id: project.namespace.to_param,
project_id: project, id: pipeline_schedule, schedule: schedule
it 'returns an error that the keys of variable are duplicated' do
expect { post :create, namespace_id: project.namespace.to_param, project_id: project, schedule: schedule }
.to change { Ci::PipelineSchedule.count }.by(0)
.and change { Ci::PipelineScheduleVariable.count }.by(0)
expect(assigns(:schedule).errors['variables.key']).not_to be_empty
end
end
end
context 'when a pipeline schedule has one variable' do
let!(:pipeline_schedule_variable) do
create(:ci_pipeline_schedule_variable, key: 'CCC',
pipeline_schedule: pipeline_schedule)
end
context 'when params do not include variables' do
let(:schedule) { basic_param }
it 'updates only scheduled pipeline attributes' do
put :update, namespace_id: project.namespace.to_param,
project_id: project, id: pipeline_schedule, schedule: schedule
pipeline_schedule.reload
expect(response).to have_http_status(:found)
expect(pipeline_schedule.description).to eq('updated_desc')
expect(pipeline_schedule.cron).to eq('0 1 * * *')
expect(pipeline_schedule.cron_timezone).to eq('UTC')
expect(pipeline_schedule.ref).to eq('patch-x')
expect(pipeline_schedule.active).to eq(true)
expect(pipeline_schedule.variables.count).to eq(1)
expect(pipeline_schedule.variables.last.key).to eq('CCC')
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
context 'when params include one variable' do
context 'when adds a new variable' do
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
project.add_developer(user)
sign_in(user)
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 params do not include variables' do
let(:schedule) { basic_param }
it 'updates only scheduled pipeline attributes' do
put :update, namespace_id: project.namespace.to_param,
project_id: project, id: pipeline_schedule, schedule: schedule
pipeline_schedule.reload
expect(response).to have_http_status(:found)
expect(pipeline_schedule.description).to eq('updated_desc')
expect(pipeline_schedule.cron).to eq('0 1 * * *')
expect(pipeline_schedule.cron_timezone).to eq('UTC')
expect(pipeline_schedule.ref).to eq('patch-x')
expect(pipeline_schedule.active).to eq(true)
expect(pipeline_schedule.variables).to be_empty
end
end
context 'when params include one variable' do
let(:schedule) do
basic_param.merge({
variables_attributes: [ { key: 'AAA', value: 'AAA123' }]
variables_attributes: [ { key: 'AAA', value: 'AAA123' } ]
})
end
it 'adds the new variable' do
it 'inserts new variable to the pipeline schedule' do
expect do
put :update, namespace_id: project.namespace.to_param,
project_id: project, id: pipeline_schedule, schedule: schedule
end.to change { Ci::PipelineScheduleVariable.count }.by(1)
pipeline_schedule.reload
expect(response).to have_http_status(:found)
expect(pipeline_schedule.variables.last.key).to eq('AAA')
expect(pipeline_schedule.variables.last.value).to eq('AAA123')
end
end
context 'when updates a variable' do
context 'when params include two unique variables' do
let(:schedule) do
basic_param.merge({
variables_attributes: [ { id: pipeline_schedule_variable.id, value: 'new_value' } ]
variables_attributes: [ { key: 'AAA', value: 'AAA123' }, { key: 'BBB', value: 'BBB123' } ]
})
end
it 'updates the variable' do
it 'inserts two new variables to the pipeline schedule' do
expect do
put :update, namespace_id: project.namespace.to_param,
project_id: project, id: pipeline_schedule, schedule: schedule
end.not_to change { Ci::PipelineScheduleVariable.count }
end.to change { Ci::PipelineScheduleVariable.count }.by(2)
pipeline_schedule_variable.reload
pipeline_schedule.reload
expect(pipeline_schedule_variable.value).to eq('new_value')
expect(response).to have_http_status(:found)
expect(pipeline_schedule.variables.first.key).to eq('AAA')
expect(pipeline_schedule.variables.first.value).to eq('AAA123')
expect(pipeline_schedule.variables.last.key).to eq('BBB')
expect(pipeline_schedule.variables.last.value).to eq('BBB123')
end
end
context 'when deletes a variable' do
context 'when params include two duplicated variables' do
let(:schedule) do
basic_param.merge({
variables_attributes: [ { id: pipeline_schedule_variable.id, _destroy: true } ]
variables_attributes: [ { key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' } ]
})
end
it 'delete the existsed variable' do
expect do
put :update, namespace_id: project.namespace.to_param,
project_id: project, id: pipeline_schedule, schedule: schedule
end.to change { Ci::PipelineScheduleVariable.count }.by(-1)
it 'returns an error that variables are duplciated' do
put :update, namespace_id: project.namespace.to_param,
project_id: project, id: pipeline_schedule, schedule: schedule
expect(assigns(:schedule).errors['variables.key']).not_to be_empty
end
end
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)
end
context 'when params do not include variables' do
let(:schedule) { basic_param }
it 'updates only scheduled pipeline attributes' do
put :update, namespace_id: project.namespace.to_param,
project_id: project, id: pipeline_schedule, schedule: schedule
pipeline_schedule.reload
expect(response).to have_http_status(:found)
expect(pipeline_schedule.description).to eq('updated_desc')
expect(pipeline_schedule.cron).to eq('0 1 * * *')
expect(pipeline_schedule.cron_timezone).to eq('UTC')
expect(pipeline_schedule.ref).to eq('patch-x')
expect(pipeline_schedule.active).to eq(true)
expect(pipeline_schedule.variables.count).to eq(1)
expect(pipeline_schedule.variables.last.key).to eq('CCC')
end
end
context 'when params include one variable' do
context 'when adds a new variable' do
let(:schedule) do
basic_param.merge({
variables_attributes: [ { key: 'AAA', value: 'AAA123' }]
})
end
it 'adds the new variable' do
expect do
put :update, namespace_id: project.namespace.to_param,
project_id: project, id: pipeline_schedule, schedule: schedule
end.to change { Ci::PipelineScheduleVariable.count }.by(1)
expect(pipeline_schedule.variables.last.key).to eq('AAA')
end
end
context 'when updates a variable' do
let(:schedule) do
basic_param.merge({
variables_attributes: [ { id: pipeline_schedule_variable.id, value: 'new_value' } ]
})
end
it 'updates the variable' do
expect do
put :update, namespace_id: project.namespace.to_param,
project_id: project, id: pipeline_schedule, schedule: schedule
end.not_to change { Ci::PipelineScheduleVariable.count }
pipeline_schedule_variable.reload
expect(pipeline_schedule_variable.value).to eq('new_value')
end
end
context 'when deletes a variable' do
let(:schedule) do
basic_param.merge({
variables_attributes: [ { id: pipeline_schedule_variable.id, _destroy: true } ]
})
end
it 'delete the existsed variable' do
expect do
put :update, namespace_id: project.namespace.to_param,
project_id: project, id: pipeline_schedule, schedule: schedule
end.to change { Ci::PipelineScheduleVariable.count }.by(-1)
end
end
end
end
end
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_developer(developer_1)
end
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
specify { expect(action).to be_allowed_for(developer_1) }
end
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
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
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 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 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 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
specify { expect(action).to be_denied_for(:developer).of(project) }
end
end
end
end
describe 'GET edit' do
context 'TODO: integrate to bottom' do
let(:user) { create(:user) }
let(:user) { create(:user) }
before do
project.add_master(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
sign_in(user)
end
context 'when a developer created a pipeline schedule' do
context 'when the developer edits' do
it 'can edit variables' do
# TODO:
end
end
it 'loads the pipeline schedule' do
get :edit, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
context 'when other developers edit' do
it 'can not edit variables' do
# TODO:
end
end
context 'when a master edits' do
it 'can edit variables' do
# TODO:
end
end
end
context 'when a master created a pipeline schedule' do
context 'when the master edits' do
it 'can edit variables' do
# TODO:
end
end
context 'when other masters edit' do
it 'can edit variables' do
# TODO:
end
end
context 'when developers edit' do
it 'can not edit variables' do
# TODO:
end
end
expect(response).to have_http_status(:ok)
expect(assigns(:schedule)).to eq(pipeline_schedule)
end
end