diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb
index d075440b147b1daa39c848384c9da38ec01464c3..597431be65a96b1066185db5433c7370f0971c97 100644
--- a/app/models/protected_branch.rb
+++ b/app/models/protected_branch.rb
@@ -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
diff --git a/app/views/projects/protected_branches/shared/_index.html.haml b/app/views/projects/protected_branches/shared/_index.html.haml
index 539b184e5c23fb95a9cd307194a2757d77e870d8..4997770321edd304e154cfec138f7cdf163d27f2 100644
--- a/app/views/projects/protected_branches/shared/_index.html.haml
+++ b/app/views/projects/protected_branches/shared/_index.html.haml
@@ -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 anyone from force pushing to the branch
%li prevent anyone from deleting the branch
diff --git a/changelogs/unreleased/53361-fresh-protected-branches.yml b/changelogs/unreleased/53361-fresh-protected-branches.yml
new file mode 100644
index 0000000000000000000000000000000000000000..55080e719b7e1ccda7bf6c89be8621fcc72fe661
--- /dev/null
+++ b/changelogs/unreleased/53361-fresh-protected-branches.yml
@@ -0,0 +1,5 @@
+---
+title: Allow creation of branches that match a wildcard protection, except directly through git
+merge_request: 24969
+author:
+type: added
diff --git a/doc/user/project/protected_branches.md b/doc/user/project/protected_branches.md
index db706e5020ee911242065dade0808b03801e1ce2..3eb8123144f807ad6605fe7380e2c0876aec7673 100644
--- a/doc/user/project/protected_branches.md
+++ b/doc/user/project/protected_branches.md
@@ -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:
+## 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]
diff --git a/lib/gitlab/checks/branch_check.rb b/lib/gitlab/checks/branch_check.rb
index d06b2df36f24c23e08c70eddddc02929a9c50430..bd305ace0a040e10f24680f6b3ab2932acad2948 100644
--- a/lib/gitlab/checks/branch_check.rb
+++ b/lib/gitlab/checks/branch_check.rb
@@ -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
diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb
index 980a8014409b7752a762c85214c604a354a9cced..9ef23cf849f8501b35e1e054263629d9b182b95c 100644
--- a/lib/gitlab/user_access.rb
+++ b/lib/gitlab/user_access.rb
@@ -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
diff --git a/spec/lib/gitlab/checks/branch_check_spec.rb b/spec/lib/gitlab/checks/branch_check_spec.rb
index 77366e91dca56713a1e3aaedd6dd7e541b28fed1..f99fc639dbde8d448385273aec51a44cf57df6a6 100644
--- a/spec/lib/gitlab/checks/branch_check_spec.rb
+++ b/spec/lib/gitlab/checks/branch_check_spec.rb
@@ -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' }
diff --git a/spec/models/protected_branch_spec.rb b/spec/models/protected_branch_spec.rb
index 4c677200ae214ac41b119d18abbdb630750633f9..dafe76463661324c3d53f3a4070f31198b87c72e 100644
--- a/spec/models/protected_branch_spec.rb
+++ b/spec/models/protected_branch_spec.rb
@@ -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
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index 47491f708e906301f0ab48ad477784d4e920c3b5..772d1fbee2bd55042ddd5cca2e2b7158e62003a0 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -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