Commit e3fbcd00 authored by James Edwards-Jones's avatar James Edwards-Jones

Protected Tags enforced over git

parent b5fce1d5
...@@ -893,6 +893,7 @@ class Project < ActiveRecord::Base ...@@ -893,6 +893,7 @@ class Project < ActiveRecord::Base
end end
# Check if current branch name is marked as protected in the system # Check if current branch name is marked as protected in the system
#TODO: Move elsewhere
def protected_branch?(branch_name) def protected_branch?(branch_name)
return true if empty_repo? && default_branch_protected? return true if empty_repo? && default_branch_protected?
...@@ -900,6 +901,13 @@ class Project < ActiveRecord::Base ...@@ -900,6 +901,13 @@ class Project < ActiveRecord::Base
ProtectedBranch.matching(branch_name, protected_branches: @protected_branches).present? ProtectedBranch.matching(branch_name, protected_branches: @protected_branches).present?
end end
#TODO: Move elsewhere
def protected_tag?(tag_name)
#TODO: Check if memoization necessary, find way to have it work elsewhere
@protected_tags ||= self.protected_tags.to_a
ProtectedTag.matching(tag_name, protected_tags: @protected_tags).present?
end
def user_can_push_to_empty_repo?(user) def user_can_push_to_empty_repo?(user)
!default_branch_protected? || team.max_member_access(user.id) > Gitlab::Access::DEVELOPER !default_branch_protected? || team.max_member_access(user.id) > Gitlab::Access::DEVELOPER
end end
......
...@@ -10,6 +10,7 @@ module Gitlab ...@@ -10,6 +10,7 @@ module Gitlab
) )
@oldrev, @newrev, @ref = change.values_at(:oldrev, :newrev, :ref) @oldrev, @newrev, @ref = change.values_at(:oldrev, :newrev, :ref)
@branch_name = Gitlab::Git.branch_name(@ref) @branch_name = Gitlab::Git.branch_name(@ref)
@tag_name = Gitlab::Git.tag_name(@ref)
@user_access = user_access @user_access = user_access
@project = project @project = project
@env = env @env = env
...@@ -36,7 +37,7 @@ module Gitlab ...@@ -36,7 +37,7 @@ module Gitlab
if forced_push? if forced_push?
return "You are not allowed to force push code to a protected branch on this project." return "You are not allowed to force push code to a protected branch on this project."
elsif Gitlab::Git.blank_ref?(@newrev) elsif blank_ref?
return "You are not allowed to delete protected branches from this project." return "You are not allowed to delete protected branches from this project."
end end
...@@ -58,11 +59,33 @@ module Gitlab ...@@ -58,11 +59,33 @@ module Gitlab
def tag_checks def tag_checks
return if skip_authorization return if skip_authorization
tag_ref = Gitlab::Git.tag_name(@ref) return unless @tag_name
if tag_ref && protected_tag?(tag_ref) && user_access.cannot_do_action?(:admin_project) if tag_exists? && user_access.cannot_do_action?(:admin_project)
"You are not allowed to change existing tags on this project." "You are not allowed to change existing tags on this project."
end end
protected_tag_checks
end
def protected_tag_checks
return unless tag_protected?
if forced_push?
return "You are not allowed to force push protected tags." #TODO: Wording, 'not allowed to update proteted tags'?
end
if Gitlab::Git.blank_ref?(@newrev)
return "You are not allowed to delete protected tags." #TODO: Wording, do these need to mention 'you' if the rule applies to everyone
end
if !user_access.can_push_tag?(@tag_name)
return "You are not allowed to create protected tags on this project." #TODO: Wording, it is a specific tag which you don't have access too, not all protected tags which might have different levels
end
end
def tag_protected?
project.protected_tag?(@tag_name)
end end
def push_checks def push_checks
...@@ -75,14 +98,18 @@ module Gitlab ...@@ -75,14 +98,18 @@ module Gitlab
private private
def protected_tag?(tag_name) def tag_exists?
project.repository.tag_exists?(tag_name) project.repository.tag_exists?(@tag_name)
end end
def forced_push? def forced_push?
Gitlab::Checks::ForcePush.force_push?(@project, @oldrev, @newrev, env: @env) Gitlab::Checks::ForcePush.force_push?(@project, @oldrev, @newrev, env: @env)
end end
def blank_ref?
Gitlab::Git.blank_ref?(@newrev)
end
def matching_merge_request? def matching_merge_request?
Checks::MatchingMergeRequest.new(@newrev, @branch_name, @project).match? Checks::MatchingMergeRequest.new(@newrev, @branch_name, @project).match?
end end
......
...@@ -28,6 +28,22 @@ module Gitlab ...@@ -28,6 +28,22 @@ module Gitlab
true true
end end
#TODO: Test this
#TODO move most to ProtectedTag::AccessChecker. Or maybe UserAccess::Protections::Tag
#TODO: then consider removing method, if it turns out can_access_git? and can?(:push_code are checked in change_access
def can_push_tag?(ref)
return false unless can_access_git?
if project.protected_tag?(ref)
access_levels = project.protected_tags.matching(ref).map(&:push_access_levels).flatten
has_access = access_levels.any? { |access_level| access_level.check_access(user) }
has_access
else
user.can?(:push_code, project)
end
end
def can_push_to_branch?(ref) def can_push_to_branch?(ref)
return false unless can_access_git? return false unless can_access_git?
......
...@@ -23,7 +23,7 @@ describe Gitlab::Checks::ChangeAccess, lib: true do ...@@ -23,7 +23,7 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
).exec ).exec
end end
before { allow(user_access).to receive(:can_do_action?).with(:push_code).and_return(true) } before { project.add_developer(user) }
context 'without failed checks' do context 'without failed checks' do
it "doesn't return any error" do it "doesn't return any error" do
...@@ -50,11 +50,51 @@ describe Gitlab::Checks::ChangeAccess, lib: true do ...@@ -50,11 +50,51 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
end end
it 'returns an error if the user is not allowed to update tags' do it 'returns an error if the user is not allowed to update tags' do
allow(user_access).to receive(:can_do_action?).with(:push_code).and_return(true)
expect(user_access).to receive(:can_do_action?).with(:admin_project).and_return(false) expect(user_access).to receive(:can_do_action?).with(:admin_project).and_return(false)
expect(subject.status).to be(false) expect(subject.status).to be(false)
expect(subject.message).to eq('You are not allowed to change existing tags on this project.') expect(subject.message).to eq('You are not allowed to change existing tags on this project.')
end end
context 'with protected tag' do
let!(:protected_tag) { create(:protected_tag, project: project, name: 'v*') }
context 'deletion' do
let(:changes) do
{
oldrev: 'be93687618e4b132087f430a4d8fc3a609c9b77c',
newrev: '0000000000000000000000000000000000000000',
ref: 'refs/tags/v1.0.0'
}
end
it 'is prevented' do
expect(subject.status).to be(false)
expect(subject.message).to include('delete protected tags')
end
end
it 'prevents force push' do
expect(Gitlab::Checks::ForcePush).to receive(:force_push?).and_return(true)
expect(subject.status).to be(false)
expect(subject.message).to include('force push protected tags')
end
it 'prevents creation below access level' do
expect(subject.status).to be(false)
expect(subject.message).to include('allowed to')
end
context 'when user has access' do
let!(:protected_tag) { create(:protected_tag, :developers_can_push, project: project, name: 'v*') }
it 'allows tag creation' do
expect(subject.status).to be(true)
end
end
end
end end
context 'protected branches check' do context 'protected branches check' 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