Commit 4408da47 authored by Lin Jen-Shin's avatar Lin Jen-Shin

Move the check to Pipeline.allowed_to_create?

So that we could use it for the schedule before
trying to use CreatePipelineService
parent 9ecb85a4
...@@ -162,6 +162,20 @@ module Ci ...@@ -162,6 +162,20 @@ module Ci
where.not(duration: nil).sum(:duration) where.not(duration: nil).sum(:duration)
end end
def self.allowed_to_create?(user, project, ref)
repo = project.repository
access = Gitlab::UserAccess.new(user, project: project)
Ability.allowed?(user, :create_pipeline, project) &&
if repo.ref_exists?("#{Gitlab::Git::BRANCH_REF_PREFIX}#{ref}")
access.can_merge_to_branch?(ref)
elsif repo.ref_exists?("#{Gitlab::Git::TAG_REF_PREFIX}#{ref}")
access.can_create_tag?(ref)
else
false
end
end
def stage(name) def stage(name)
stage = Ci::Stage.new(self, name: name) stage = Ci::Stage.new(self, name: name)
stage unless stage.statuses_count.zero? stage unless stage.statuses_count.zero?
......
...@@ -37,7 +37,7 @@ module Ci ...@@ -37,7 +37,7 @@ module Ci
end end
def runnable_by_owner? def runnable_by_owner?
Ability.allowed?(owner, :create_pipeline, project) Ci::Pipeline.allowed_to_create?(owner, project, ref)
end end
def set_next_run_at def set_next_run_at
......
...@@ -27,10 +27,8 @@ module Ci ...@@ -27,10 +27,8 @@ module Ci
return error('Reference not found') return error('Reference not found')
end end
if tag? unless Ci::Pipeline.allowed_to_create?(current_user, project, ref)
return error("#{ref} is protected") unless access.can_create_tag?(ref) return error("Insufficient permissions for protected #{ref}")
else
return error("#{ref} is protected") unless access.can_merge_to_branch?(ref)
end end
unless commit unless commit
...@@ -53,6 +51,12 @@ module Ci ...@@ -53,6 +51,12 @@ module Ci
return error('No builds for this pipeline.') return error('No builds for this pipeline.')
end end
process!
end
private
def process!
Ci::Pipeline.transaction do Ci::Pipeline.transaction do
update_merge_requests_head_pipeline if pipeline.save update_merge_requests_head_pipeline if pipeline.save
...@@ -66,8 +70,6 @@ module Ci ...@@ -66,8 +70,6 @@ module Ci
pipeline.tap(&:process!) pipeline.tap(&:process!)
end end
private
def update_merge_requests_head_pipeline def update_merge_requests_head_pipeline
return unless pipeline.latest? return unless pipeline.latest?
...@@ -100,10 +102,6 @@ module Ci ...@@ -100,10 +102,6 @@ module Ci
@commit ||= project.commit(origin_sha || origin_ref) @commit ||= project.commit(origin_sha || origin_ref)
end end
def access
@access ||= Gitlab::UserAccess.new(current_user, project: project)
end
def sha def sha
commit.try(:id) commit.try(:id)
end end
...@@ -121,10 +119,16 @@ module Ci ...@@ -121,10 +119,16 @@ module Ci
end end
def branch? def branch?
return @is_branch if defined?(@is_branch)
@is_branch =
project.repository.ref_exists?(Gitlab::Git::BRANCH_REF_PREFIX + ref) project.repository.ref_exists?(Gitlab::Git::BRANCH_REF_PREFIX + ref)
end end
def tag? def tag?
return @is_tag if defined?(@is_tag)
@is_tag =
project.repository.ref_exists?(Gitlab::Git::TAG_REF_PREFIX + ref) project.repository.ref_exists?(Gitlab::Git::TAG_REF_PREFIX + ref)
end end
......
...@@ -28,6 +28,103 @@ describe Ci::Pipeline, models: true do ...@@ -28,6 +28,103 @@ describe Ci::Pipeline, models: true do
it { is_expected.to respond_to :git_author_email } it { is_expected.to respond_to :git_author_email }
it { is_expected.to respond_to :short_sha } it { is_expected.to respond_to :short_sha }
describe '.allowed_to_create?' do
let(:user) { create(:user) }
let(:project) { create(:project, :repository) }
let(:ref) { 'master' }
subject { described_class.allowed_to_create?(user, project, ref) }
context 'when user is a developer' do
before do
project.add_developer(user)
end
it { is_expected.to be_truthy }
context 'when the branch is protected' do
let!(:protected_branch) do
create(:protected_branch, project: project, name: ref)
end
it { is_expected.to be_falsey }
context 'when developers are allowed to merge' do
let!(:protected_branch) do
create(:protected_branch,
:developers_can_merge,
project: project,
name: ref)
end
it { is_expected.to be_truthy }
end
end
context 'when the tag is protected' do
let(:ref) { 'v1.0.0' }
let!(:protected_tag) do
create(:protected_tag, project: project, name: ref)
end
it { is_expected.to be_falsey }
context 'when developers are allowed to create the tag' do
let!(:protected_tag) do
create(:protected_tag,
:developers_can_create,
project: project,
name: ref)
end
it { is_expected.to be_truthy }
end
end
end
context 'when user is a master' do
before do
project.add_master(user)
end
it { is_expected.to be_truthy }
context 'when the branch is protected' do
let!(:protected_branch) do
create(:protected_branch, project: project, name: ref)
end
it { is_expected.to be_truthy }
end
context 'when the tag is protected' do
let(:ref) { 'v1.0.0' }
let!(:protected_tag) do
create(:protected_tag, project: project, name: ref)
end
it { is_expected.to be_truthy }
context 'when no one can create the tag' do
let!(:protected_tag) do
create(:protected_tag,
:no_one_can_create,
project: project,
name: ref)
end
it { is_expected.to be_falsey }
end
end
end
context 'when owner cannot create pipeline' do
it { is_expected.to be_falsey }
end
end
describe '#source' do describe '#source' do
context 'when creating new pipeline' do context 'when creating new pipeline' do
let(:pipeline) do let(:pipeline) 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