Enforce "No One Can Push" during git operations.
1. The crux of this change is in `UserAccess`, which looks through all the access levels, asking each if the user has access to push/merge for the current project. 2. Update the `protected_branches` factory to create access levels as necessary. 3. Fix and augment `user_access` and `git_access` specs.
This commit is contained in:
parent
ab6096c172
commit
828f6eb6e5
|
@ -1,5 +1,14 @@
|
|||
class ProtectedBranch::MergeAccessLevel < ActiveRecord::Base
|
||||
belongs_to :protected_branch
|
||||
delegate :project, to: :protected_branch
|
||||
|
||||
enum access_level: [:masters, :developers]
|
||||
|
||||
def check_access(user)
|
||||
if masters?
|
||||
user.can?(:push_code, project) if project.team.master?(user)
|
||||
elsif developers?
|
||||
user.can?(:push_code, project) if (project.team.master?(user) || project.team.developer?(user))
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,5 +1,16 @@
|
|||
class ProtectedBranch::PushAccessLevel < ActiveRecord::Base
|
||||
belongs_to :protected_branch
|
||||
delegate :project, to: :protected_branch
|
||||
|
||||
enum access_level: [:masters, :developers, :no_one]
|
||||
|
||||
def check_access(user)
|
||||
if masters?
|
||||
user.can?(:push_code, project) if project.team.master?(user)
|
||||
elsif developers?
|
||||
user.can?(:push_code, project) if (project.team.master?(user) || project.team.developer?(user))
|
||||
elsif no_one?
|
||||
false
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -29,8 +29,9 @@ module Gitlab
|
|||
def can_push_to_branch?(ref)
|
||||
return false unless user
|
||||
|
||||
if project.protected_branch?(ref) && !project.developers_can_push_to_protected_branch?(ref)
|
||||
user.can?(:push_code_to_protected_branches, project)
|
||||
if project.protected_branch?(ref)
|
||||
access_levels = project.protected_branches.matching(ref).map(&:push_access_level)
|
||||
access_levels.any? { |access_level| access_level.check_access(user) }
|
||||
else
|
||||
user.can?(:push_code, project)
|
||||
end
|
||||
|
@ -39,8 +40,9 @@ module Gitlab
|
|||
def can_merge_to_branch?(ref)
|
||||
return false unless user
|
||||
|
||||
if project.protected_branch?(ref) && !project.developers_can_merge_to_protected_branch?(ref)
|
||||
user.can?(:push_code_to_protected_branches, project)
|
||||
if project.protected_branch?(ref)
|
||||
access_levels = project.protected_branches.matching(ref).map(&:merge_access_level)
|
||||
access_levels.any? { |access_level| access_level.check_access(user) }
|
||||
else
|
||||
user.can?(:push_code, project)
|
||||
end
|
||||
|
|
|
@ -2,5 +2,22 @@ FactoryGirl.define do
|
|||
factory :protected_branch do
|
||||
name
|
||||
project
|
||||
|
||||
after(:create) do |protected_branch|
|
||||
protected_branch.create_push_access_level!(access_level: :masters)
|
||||
protected_branch.create_merge_access_level!(access_level: :masters)
|
||||
end
|
||||
|
||||
trait :developers_can_push do
|
||||
after(:create) { |protected_branch| protected_branch.push_access_level.developers! }
|
||||
end
|
||||
|
||||
trait :developers_can_merge do
|
||||
after(:create) { |protected_branch| protected_branch.merge_access_level.developers! }
|
||||
end
|
||||
|
||||
trait :no_one_can_push do
|
||||
after(:create) { |protected_branch| protected_branch.push_access_level.no_one! }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -217,29 +217,32 @@ describe Gitlab::GitAccess, lib: true do
|
|||
run_permission_checks(permissions_matrix)
|
||||
end
|
||||
|
||||
context "when 'developers can push' is turned on for the #{protected_branch_type} protected branch" do
|
||||
before { create(:protected_branch, name: protected_branch_name, developers_can_push: true, project: project) }
|
||||
context "when developers are allowed to push into the #{protected_branch_type} protected branch" do
|
||||
before { create(:protected_branch, :developers_can_push, name: protected_branch_name, project: project) }
|
||||
|
||||
run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true }))
|
||||
end
|
||||
|
||||
context "when 'developers can merge' is turned on for the #{protected_branch_type} protected branch" do
|
||||
before { create(:protected_branch, name: protected_branch_name, developers_can_merge: true, project: project) }
|
||||
context "developers are allowed to merge into the #{protected_branch_type} protected branch" do
|
||||
before { create(:protected_branch, :developers_can_merge, name: protected_branch_name, project: project) }
|
||||
|
||||
context "when a merge request exists for the given source/target branch" do
|
||||
context "when the merge request is in progress" do
|
||||
before do
|
||||
create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature', state: 'locked', in_progress_merge_commit_sha: merge_into_protected_branch)
|
||||
create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature',
|
||||
state: 'locked', in_progress_merge_commit_sha: merge_into_protected_branch)
|
||||
end
|
||||
|
||||
run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: true }))
|
||||
context "when the merge request is not in progress" do
|
||||
before do
|
||||
create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature', in_progress_merge_commit_sha: nil)
|
||||
end
|
||||
|
||||
run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: false }))
|
||||
end
|
||||
end
|
||||
|
||||
context "when the merge request is not in progress" do
|
||||
before do
|
||||
create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature', in_progress_merge_commit_sha: nil)
|
||||
end
|
||||
|
||||
context "when a merge request does not exist for the given source/target branch" do
|
||||
run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: false }))
|
||||
end
|
||||
end
|
||||
|
@ -249,44 +252,51 @@ describe Gitlab::GitAccess, lib: true do
|
|||
end
|
||||
end
|
||||
|
||||
context "when 'developers can merge' and 'developers can push' are turned on for the #{protected_branch_type} protected branch" do
|
||||
before { create(:protected_branch, name: protected_branch_name, developers_can_merge: true, developers_can_push: true, project: project) }
|
||||
context "when developers are allowed to push and merge into the #{protected_branch_type} protected branch" do
|
||||
before { create(:protected_branch, :developers_can_merge, :developers_can_push, name: protected_branch_name, project: project) }
|
||||
|
||||
run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true }))
|
||||
end
|
||||
end
|
||||
|
||||
describe 'deploy key permissions' do
|
||||
let(:key) { create(:deploy_key) }
|
||||
let(:actor) { key }
|
||||
context "when no one is allowed to push to the #{protected_branch_name} protected branch" do
|
||||
before { create(:protected_branch, :no_one_can_push, name: protected_branch_name, project: project) }
|
||||
|
||||
context 'push code' do
|
||||
subject { access.check('git-receive-pack') }
|
||||
run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: false, push_all: false, merge_into_protected_branch: false },
|
||||
master: { push_protected_branch: false, push_all: false, merge_into_protected_branch: false }))
|
||||
end
|
||||
end
|
||||
|
||||
context 'when project is authorized' do
|
||||
before { key.projects << project }
|
||||
describe 'deploy key permissions' do
|
||||
let(:key) { create(:deploy_key) }
|
||||
let(:actor) { key }
|
||||
|
||||
context 'push code' do
|
||||
subject { access.check('git-receive-pack') }
|
||||
|
||||
context 'when project is authorized' do
|
||||
before { key.projects << project }
|
||||
|
||||
it { expect(subject).not_to be_allowed }
|
||||
end
|
||||
|
||||
context 'when unauthorized' do
|
||||
context 'to public project' do
|
||||
let(:project) { create(:project, :public) }
|
||||
|
||||
it { expect(subject).not_to be_allowed }
|
||||
end
|
||||
|
||||
context 'when unauthorized' do
|
||||
context 'to public project' do
|
||||
let(:project) { create(:project, :public) }
|
||||
context 'to internal project' do
|
||||
let(:project) { create(:project, :internal) }
|
||||
|
||||
it { expect(subject).not_to be_allowed }
|
||||
end
|
||||
it { expect(subject).not_to be_allowed }
|
||||
end
|
||||
|
||||
context 'to internal project' do
|
||||
let(:project) { create(:project, :internal) }
|
||||
context 'to private project' do
|
||||
let(:project) { create(:project, :internal) }
|
||||
|
||||
it { expect(subject).not_to be_allowed }
|
||||
end
|
||||
|
||||
context 'to private project' do
|
||||
let(:project) { create(:project, :internal) }
|
||||
|
||||
it { expect(subject).not_to be_allowed }
|
||||
end
|
||||
it { expect(subject).not_to be_allowed }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -44,7 +44,7 @@ describe Gitlab::UserAccess, lib: true do
|
|||
|
||||
describe 'push to protected branch if allowed for developers' do
|
||||
before do
|
||||
@branch = create :protected_branch, project: project, developers_can_push: true
|
||||
@branch = create :protected_branch, :developers_can_push, project: project
|
||||
end
|
||||
|
||||
it 'returns true if user is a master' do
|
||||
|
@ -65,7 +65,7 @@ describe Gitlab::UserAccess, lib: true do
|
|||
|
||||
describe 'merge to protected branch if allowed for developers' do
|
||||
before do
|
||||
@branch = create :protected_branch, project: project, developers_can_merge: true
|
||||
@branch = create :protected_branch, :developers_can_merge, project: project
|
||||
end
|
||||
|
||||
it 'returns true if user is a master' do
|
||||
|
|
Loading…
Reference in New Issue