Commit db2ae26b authored by Timothy Andrew's avatar Timothy Andrew

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.
parent 9168f531
class ProtectedBranch::MergeAccessLevel < ActiveRecord::Base class ProtectedBranch::MergeAccessLevel < ActiveRecord::Base
belongs_to :protected_branch belongs_to :protected_branch
delegate :project, to: :protected_branch
enum access_level: [:masters, :developers] 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 end
class ProtectedBranch::PushAccessLevel < ActiveRecord::Base class ProtectedBranch::PushAccessLevel < ActiveRecord::Base
belongs_to :protected_branch belongs_to :protected_branch
delegate :project, to: :protected_branch
enum access_level: [:masters, :developers, :no_one] 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 end
...@@ -29,8 +29,9 @@ module Gitlab ...@@ -29,8 +29,9 @@ module Gitlab
def can_push_to_branch?(ref) def can_push_to_branch?(ref)
return false unless user return false unless user
if project.protected_branch?(ref) && !project.developers_can_push_to_protected_branch?(ref) if project.protected_branch?(ref)
user.can?(:push_code_to_protected_branches, project) access_levels = project.protected_branches.matching(ref).map(&:push_access_level)
access_levels.any? { |access_level| access_level.check_access(user) }
else else
user.can?(:push_code, project) user.can?(:push_code, project)
end end
...@@ -39,8 +40,9 @@ module Gitlab ...@@ -39,8 +40,9 @@ module Gitlab
def can_merge_to_branch?(ref) def can_merge_to_branch?(ref)
return false unless user return false unless user
if project.protected_branch?(ref) && !project.developers_can_merge_to_protected_branch?(ref) if project.protected_branch?(ref)
user.can?(:push_code_to_protected_branches, project) access_levels = project.protected_branches.matching(ref).map(&:merge_access_level)
access_levels.any? { |access_level| access_level.check_access(user) }
else else
user.can?(:push_code, project) user.can?(:push_code, project)
end end
......
...@@ -2,5 +2,22 @@ FactoryGirl.define do ...@@ -2,5 +2,22 @@ FactoryGirl.define do
factory :protected_branch do factory :protected_branch do
name name
project 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
end end
...@@ -152,16 +152,16 @@ describe Gitlab::GitAccess, lib: true do ...@@ -152,16 +152,16 @@ describe Gitlab::GitAccess, lib: true do
def merge_into_protected_branch def merge_into_protected_branch
@protected_branch_merge_commit ||= begin @protected_branch_merge_commit ||= begin
stub_git_hooks stub_git_hooks
project.repository.add_branch(user, unprotected_branch, 'feature') project.repository.add_branch(user, unprotected_branch, 'feature')
target_branch = project.repository.lookup('feature') target_branch = project.repository.lookup('feature')
source_branch = project.repository.commit_file(user, FFaker::InternetSE.login_user_name, FFaker::HipsterIpsum.paragraph, FFaker::HipsterIpsum.sentence, unprotected_branch, false) source_branch = project.repository.commit_file(user, FFaker::InternetSE.login_user_name, FFaker::HipsterIpsum.paragraph, FFaker::HipsterIpsum.sentence, unprotected_branch, false)
rugged = project.repository.rugged rugged = project.repository.rugged
author = { email: "email@example.com", time: Time.now, name: "Example Git User" } author = { email: "email@example.com", time: Time.now, name: "Example Git User" }
merge_index = rugged.merge_commits(target_branch, source_branch) merge_index = rugged.merge_commits(target_branch, source_branch)
Rugged::Commit.create(rugged, author: author, committer: author, message: "commit message", parents: [target_branch, source_branch], tree: merge_index.write_tree(rugged)) Rugged::Commit.create(rugged, author: author, committer: author, message: "commit message", parents: [target_branch, source_branch], tree: merge_index.write_tree(rugged))
end end
end end
def self.run_permission_checks(permissions_matrix) def self.run_permission_checks(permissions_matrix)
...@@ -233,29 +233,32 @@ describe Gitlab::GitAccess, lib: true do ...@@ -233,29 +233,32 @@ describe Gitlab::GitAccess, lib: true do
run_permission_checks(permissions_matrix) run_permission_checks(permissions_matrix)
end end
context "when 'developers can push' is turned on for the #{protected_branch_type} protected branch" do context "when developers are allowed to push into the #{protected_branch_type} protected branch" do
before { create(:protected_branch, name: protected_branch_name, developers_can_push: true, project: project) } 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 })) run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true }))
end end
context "when 'developers can merge' is turned on for the #{protected_branch_type} protected branch" do context "developers are allowed to merge into the #{protected_branch_type} protected branch" do
before { create(:protected_branch, name: protected_branch_name, developers_can_merge: true, project: project) } 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 a merge request exists for the given source/target branch" do
context "when the merge request is in progress" do context "when the merge request is in progress" do
before 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 end
run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: true })) context "when the merge request is not in progress" do
end before do
create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature', in_progress_merge_commit_sha: nil)
end
context "when the merge request is not in progress" do run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: false }))
before do
create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature', in_progress_merge_commit_sha: nil)
end end
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 })) run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: false }))
end end
end end
...@@ -265,11 +268,18 @@ describe Gitlab::GitAccess, lib: true do ...@@ -265,11 +268,18 @@ describe Gitlab::GitAccess, lib: true do
end end
end end
context "when 'developers can merge' and 'developers can push' are turned on for the #{protected_branch_type} protected branch" do context "when developers are allowed to push and merge into 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) } 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 })) run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true }))
end end
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) }
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 end
context "when license blocks changes" do context "when license blocks changes" do
...@@ -493,38 +503,38 @@ describe Gitlab::GitAccess, lib: true do ...@@ -493,38 +503,38 @@ describe Gitlab::GitAccess, lib: true do
end end
end end
end end
end
describe 'deploy key permissions' do describe 'deploy key permissions' do
let(:key) { create(:deploy_key) } let(:key) { create(:deploy_key) }
let(:actor) { key } let(:actor) { key }
context 'push code' do context 'push code' do
subject { access.check('git-receive-pack') } subject { access.check('git-receive-pack') }
context 'when project is authorized' do context 'when project is authorized' do
before { key.projects << project } before { key.projects << project }
it { expect(subject).not_to be_allowed } it { expect(subject).not_to be_allowed }
end end
context 'when unauthorized' do context 'when unauthorized' do
context 'to public project' do context 'to public project' do
let(:project) { create(:project, :public) } let(:project) { create(:project, :public) }
it { expect(subject).not_to be_allowed } it { expect(subject).not_to be_allowed }
end end
context 'to internal project' do context 'to internal project' do
let(:project) { create(:project, :internal) } let(:project) { create(:project, :internal) }
it { expect(subject).not_to be_allowed } it { expect(subject).not_to be_allowed }
end end
context 'to private project' do context 'to private project' do
let(:project) { create(:project, :internal) } let(:project) { create(:project, :internal) }
it { expect(subject).not_to be_allowed } it { expect(subject).not_to be_allowed }
end
end end
end end
end end
......
...@@ -44,7 +44,7 @@ describe Gitlab::UserAccess, lib: true do ...@@ -44,7 +44,7 @@ describe Gitlab::UserAccess, lib: true do
describe 'push to protected branch if allowed for developers' do describe 'push to protected branch if allowed for developers' do
before do before do
@branch = create :protected_branch, project: project, developers_can_push: true @branch = create :protected_branch, :developers_can_push, project: project
end end
it 'returns true if user is a master' do it 'returns true if user is a master' do
...@@ -65,7 +65,7 @@ describe Gitlab::UserAccess, lib: true do ...@@ -65,7 +65,7 @@ describe Gitlab::UserAccess, lib: true do
describe 'merge to protected branch if allowed for developers' do describe 'merge to protected branch if allowed for developers' do
before do before do
@branch = create :protected_branch, project: project, developers_can_merge: true @branch = create :protected_branch, :developers_can_merge, project: project
end end
it 'returns true if user is a master' do it 'returns true if user is a master' 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