Allow protected branch creation via web and API

This commit includes changes to add `UserAccess#can_create_branch?`
which will check whether the user is allowed to create a branch even
if it matches a protected branch.

This is used in `Gitlab::Checks::BranchCheck` when the branch name
matches a protected branch.

A `push_to_create_protected_branch` ability in `ProjectPolicy` has been
added to allow Developers and above to create protected branches.
This commit is contained in:
Patrick Bajao 2019-03-06 12:20:27 +00:00 committed by Kamil Trzciński
parent e94c13d39d
commit e371520f46
9 changed files with 205 additions and 10 deletions

View file

@ -18,13 +18,23 @@ class ProtectedBranch < ActiveRecord::Base
def self.protected?(project, ref_name)
return true if project.empty_repo? && default_branch_protected?
refs = project.protected_branches.select(:name)
self.matching(ref_name, protected_refs: protected_refs(project)).present?
end
self.matching(ref_name, protected_refs: refs).present?
def self.any_protected?(project, ref_names)
protected_refs(project).any? do |protected_ref|
ref_names.any? do |ref_name|
protected_ref.matches?(ref_name)
end
end
end
def self.default_branch_protected?
Gitlab::CurrentSettings.default_branch_protection == Gitlab::Access::PROTECTION_FULL ||
Gitlab::CurrentSettings.default_branch_protection == Gitlab::Access::PROTECTION_DEV_CAN_MERGE
end
def self.protected_refs(project)
project.protected_branches.select(:name)
end
end

View file

@ -12,7 +12,7 @@
%p
By default, protected branches are designed to:
%ul
%li prevent their creation, if not already created, from everybody except Maintainers
%li prevent their creation, if not already created, from everybody except users who are allowed to merge
%li prevent pushes from everybody except Maintainers
%li prevent <strong>anyone</strong> from force pushing to the branch
%li prevent <strong>anyone</strong> from deleting the branch

View file

@ -0,0 +1,5 @@
---
title: Allow creation of branches that match a wildcard protection, except directly through git
merge_request: 24969
author:
type: added

View file

@ -10,7 +10,7 @@ created protected branches.
By default, a protected branch does four simple things:
- it prevents its creation, if not already created, from everybody except users
with Maintainer permission
who are allowed to merge
- it prevents pushes from everybody except users with Maintainer permission
- it prevents **anyone** from force pushing to the branch
- it prevents **anyone** from deleting the branch
@ -94,6 +94,25 @@ all matching branches:
![Protected branch matches](img/protected_branches_matches.png)
## Creating a protected branch
> [Introduced][https://gitlab.com/gitlab-org/gitlab-ce/issues/53361] in GitLab 11.9.
When a protected branch or wildcard protected branches are set to
[**No one** is **Allowed to push**](#using-the-allowed-to-merge-and-allowed-to-push-settings),
Developers (and users with higher [permission levels](../permissions.md)) are allowed
to create a new protected branch, but only via the UI or through the API (to avoid
creating protected branches accidentally from the command line or from a Git
client application).
To create a new branch through the user interface:
1. Visit **Repository > Branches**.
1. Click on **New branch**.
1. Fill in the branch name and select an existing branch, tag, or commit that
the new branch will be based off. Only existing protected branches and commits
that are already in protected branches will be accepted.
## Deleting a protected branch
> [Introduced][ce-21393] in GitLab 9.3.
@ -125,6 +144,10 @@ for details about the pipelines security model.
## Changelog
**11.9**
- [Allow protected branches to be created](https://gitlab.com/gitlab-org/gitlab-ce/issues/53361) by Developers (and users with higher permission levels) through the API and the user interface.
**9.2**
- Allow deletion of protected branches via the web interface [gitlab-org/gitlab-ce#21393][ce-21393]

View file

@ -9,13 +9,17 @@ module Gitlab
non_master_delete_protected_branch: 'You are not allowed to delete protected branches from this project. Only a project maintainer or owner can delete a protected branch.',
non_web_delete_protected_branch: 'You can only delete protected branches using the web interface.',
merge_protected_branch: 'You are not allowed to merge code into protected branches on this project.',
push_protected_branch: 'You are not allowed to push code to protected branches on this project.'
push_protected_branch: 'You are not allowed to push code to protected branches on this project.',
create_protected_branch: 'You are not allowed to create protected branches on this project.',
invalid_commit_create_protected_branch: 'You can only use an existing protected branch ref as the basis of a new protected branch.',
non_web_create_protected_branch: 'You can only create protected branches using the web interface and API.'
}.freeze
LOG_MESSAGES = {
delete_default_branch_check: "Checking if default branch is being deleted...",
protected_branch_checks: "Checking if you are force pushing to a protected branch...",
protected_branch_push_checks: "Checking if you are allowed to push to the protected branch...",
protected_branch_creation_checks: "Checking if you are allowed to create a protected branch...",
protected_branch_deletion_checks: "Checking if you are allowed to delete the protected branch..."
}.freeze
@ -42,13 +46,31 @@ module Gitlab
end
end
if deletion?
if creation? && protected_branch_creation_enabled?
protected_branch_creation_checks
elsif deletion?
protected_branch_deletion_checks
else
protected_branch_push_checks
end
end
def protected_branch_creation_checks
logger.log_timed(LOG_MESSAGES[:protected_branch_creation_checks]) do
unless user_access.can_merge_to_branch?(branch_name)
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:create_protected_branch]
end
unless safe_commit_for_new_protected_branch?
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:invalid_commit_create_protected_branch]
end
unless updated_from_web?
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:non_web_create_protected_branch]
end
end
end
def protected_branch_deletion_checks
logger.log_timed(LOG_MESSAGES[:protected_branch_deletion_checks]) do
unless user_access.can_delete_branch?(branch_name)
@ -98,6 +120,10 @@ module Gitlab
Gitlab::Routing.url_helpers.project_project_members_url(project)
end
def protected_branch_creation_enabled?
Feature.enabled?(:protected_branch_creation, project, default_enabled: true)
end
def matching_merge_request?
Checks::MatchingMergeRequest.new(newrev, branch_name, project).match?
end
@ -105,6 +131,10 @@ module Gitlab
def forced_push?
Gitlab::Checks::ForcePush.force_push?(project, oldrev, newrev)
end
def safe_commit_for_new_protected_branch?
ProtectedBranch.any_protected?(project, project.repository.branch_names_contains_sha(newrev))
end
end
end
end

View file

@ -118,8 +118,8 @@ module Gitlab
protected_refs: project.protected_tags)
end
request_cache def protected?(kind, project, ref)
kind.protected?(project, ref)
request_cache def protected?(kind, project, refs)
kind.protected?(project, refs)
end
end
end

View file

@ -55,6 +55,106 @@ describe Gitlab::Checks::BranchCheck do
end
end
context 'branch creation' do
let(:oldrev) { '0000000000000000000000000000000000000000' }
let(:ref) { 'refs/heads/feature' }
context 'protected branch creation feature is disabled' do
before do
stub_feature_flags(protected_branch_creation: false)
end
context 'user is not allowed to push to protected branch' do
before do
allow(user_access)
.to receive(:can_push_to_branch?)
.and_return(false)
end
it 'raises an error' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to protected branches on this project.')
end
end
context 'user is allowed to push to protected branch' do
before do
allow(user_access)
.to receive(:can_push_to_branch?)
.and_return(true)
end
it 'does not raise an error' do
expect { subject.validate! }.not_to raise_error
end
end
end
context 'protected branch creation feature is enabled' do
context 'user is not allowed to create protected branches' do
before do
allow(user_access)
.to receive(:can_merge_to_branch?)
.with('feature')
.and_return(false)
end
it 'raises an error' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to create protected branches on this project.')
end
end
context 'user is allowed to create protected branches' do
before do
allow(user_access)
.to receive(:can_merge_to_branch?)
.with('feature')
.and_return(true)
allow(project.repository)
.to receive(:branch_names_contains_sha)
.with(newrev)
.and_return(['branch'])
end
context "newrev isn't in any protected branches" do
before do
allow(ProtectedBranch)
.to receive(:any_protected?)
.with(project, ['branch'])
.and_return(false)
end
it 'raises an error' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only use an existing protected branch ref as the basis of a new protected branch.')
end
end
context 'newrev is included in a protected branch' do
before do
allow(ProtectedBranch)
.to receive(:any_protected?)
.with(project, ['branch'])
.and_return(true)
end
context 'via web interface' do
let(:protocol) { 'web' }
it 'allows branch creation' do
expect { subject.validate! }.not_to raise_error
end
end
context 'via SSH' do
it 'raises an error' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only create protected branches using the web interface and API.')
end
end
end
end
end
end
context 'branch deletion' do
let(:newrev) { '0000000000000000000000000000000000000000' }
let(:ref) { 'refs/heads/feature' }

View file

@ -190,4 +190,32 @@ describe ProtectedBranch do
end
end
end
describe '#any_protected?' do
context 'existing project' do
let(:project) { create(:project, :repository) }
it 'returns true when any of the branch names match a protected branch via direct match' do
create(:protected_branch, project: project, name: 'foo')
expect(described_class.any_protected?(project, ['foo', 'production/some-branch'])).to eq(true)
end
it 'returns true when any of the branch matches a protected branch via wildcard match' do
create(:protected_branch, project: project, name: 'production/*')
expect(described_class.any_protected?(project, ['foo', 'production/some-branch'])).to eq(true)
end
it 'returns false when none of branches does not match a protected branch via direct match' do
expect(described_class.any_protected?(project, ['foo'])).to eq(false)
end
it 'returns false when none of the branches does not match a protected branch via wildcard match' do
create(:protected_branch, project: project, name: 'production/*')
expect(described_class.any_protected?(project, ['staging/some-branch'])).to eq(false)
end
end
end
end

View file

@ -45,8 +45,7 @@ describe ProjectPolicy do
let(:base_maintainer_permissions) do
%i[
push_to_delete_protected_branch update_project_snippet update_environment
update_deployment admin_project_snippet
admin_project_member admin_note admin_wiki admin_project
update_deployment admin_project_snippet admin_project_member admin_note admin_wiki admin_project
admin_commit_status admin_build admin_container_image
admin_pipeline admin_environment admin_deployment destroy_release add_cluster
daily_statistics