diff options
author | Rémy Coutable <remy@rymai.me> | 2016-07-18 10:16:56 +0200 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-07-18 10:16:56 +0200 |
commit | 2cf7f09b1e9ccb000433374a42bd7b33b73f8116 (patch) | |
tree | 743e4ed2a6f2739541891749a6ef31bfd09e6805 /spec/lib | |
parent | 1266e2664e7b080b440f5fc004424418284ee87b (diff) | |
download | gitlab-ce-2cf7f09b1e9ccb000433374a42bd7b33b73f8116.tar.gz |
Revert "Revert "Merge branch '18193-developers-can-merge' into 'master'
""
This reverts commit 530f5158e297f3cde27f3566cfe13bad74ba3b50.
See !4892.
Signed-off-by: Rémy Coutable <remy@rymai.me>
Diffstat (limited to 'spec/lib')
-rw-r--r-- | spec/lib/gitlab/diff/position_tracer_spec.rb | 3 | ||||
-rw-r--r-- | spec/lib/gitlab/git_access_spec.rb | 247 | ||||
-rw-r--r-- | spec/lib/gitlab/user_access_spec.rb | 88 |
3 files changed, 204 insertions, 134 deletions
diff --git a/spec/lib/gitlab/diff/position_tracer_spec.rb b/spec/lib/gitlab/diff/position_tracer_spec.rb index 08312e60f4a..c268f84c759 100644 --- a/spec/lib/gitlab/diff/position_tracer_spec.rb +++ b/spec/lib/gitlab/diff/position_tracer_spec.rb @@ -1639,7 +1639,8 @@ describe Gitlab::Diff::PositionTracer, lib: true do committer: committer } - repository.merge(current_user, second_create_file_commit.sha, branch_name, options) + merge_request = create(:merge_request, source_branch: second_create_file_commit.sha, target_branch: branch_name, source_project: project) + repository.merge(current_user, merge_request, options) project.commit(branch_name) end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index c79ba11f782..db33c7a22bb 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -6,67 +6,6 @@ describe Gitlab::GitAccess, lib: true do let(:user) { create(:user) } let(:actor) { user } - describe 'can_push_to_branch?' do - describe 'push to none protected branch' do - it "returns true if user is a master" do - project.team << [user, :master] - expect(access.can_push_to_branch?("random_branch")).to be_truthy - end - - it "returns true if user is a developer" do - project.team << [user, :developer] - expect(access.can_push_to_branch?("random_branch")).to be_truthy - end - - it "returns false if user is a reporter" do - project.team << [user, :reporter] - expect(access.can_push_to_branch?("random_branch")).to be_falsey - end - end - - describe 'push to protected branch' do - before do - @branch = create :protected_branch, project: project - end - - it "returns true if user is a master" do - project.team << [user, :master] - expect(access.can_push_to_branch?(@branch.name)).to be_truthy - end - - it "returns false if user is a developer" do - project.team << [user, :developer] - expect(access.can_push_to_branch?(@branch.name)).to be_falsey - end - - it "returns false if user is a reporter" do - project.team << [user, :reporter] - expect(access.can_push_to_branch?(@branch.name)).to be_falsey - end - end - - describe 'push to protected branch if allowed for developers' do - before do - @branch = create :protected_branch, project: project, developers_can_push: true - end - - it "returns true if user is a master" do - project.team << [user, :master] - expect(access.can_push_to_branch?(@branch.name)).to be_truthy - end - - it "returns true if user is a developer" do - project.team << [user, :developer] - expect(access.can_push_to_branch?(@branch.name)).to be_truthy - end - - it "returns false if user is a reporter" do - project.team << [user, :reporter] - expect(access.can_push_to_branch?(@branch.name)).to be_falsey - end - end - end - describe '#check with single protocols allowed' do def disable_protocol(protocol) settings = ::ApplicationSetting.create_from_defaults @@ -160,96 +99,46 @@ describe Gitlab::GitAccess, lib: true do end describe 'push_access_check' do - def protect_feature_branch - create(:protected_branch, name: 'feature', project: project) - end + before { merge_into_protected_branch } + let(:unprotected_branch) { FFaker::Internet.user_name } - def changes - { - push_new_branch: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/heads/wow", + let(:changes) do + { push_new_branch: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/heads/wow", push_master: '6f6d7e7ed 570e7b2ab refs/heads/master', push_protected_branch: '6f6d7e7ed 570e7b2ab refs/heads/feature', push_remove_protected_branch: "570e7b2ab #{Gitlab::Git::BLANK_SHA} "\ 'refs/heads/feature', push_tag: '6f6d7e7ed 570e7b2ab refs/tags/v1.0.0', push_new_tag: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/tags/v7.8.9", - push_all: ['6f6d7e7ed 570e7b2ab refs/heads/master', '6f6d7e7ed 570e7b2ab refs/heads/feature'] - } + push_all: ['6f6d7e7ed 570e7b2ab refs/heads/master', '6f6d7e7ed 570e7b2ab refs/heads/feature'], + merge_into_protected_branch: "0b4bc9a #{merge_into_protected_branch} refs/heads/feature" } end - def self.permissions_matrix - { - master: { - push_new_branch: true, - push_master: true, - push_protected_branch: true, - push_remove_protected_branch: false, - push_tag: true, - push_new_tag: true, - push_all: true, - }, - - developer: { - push_new_branch: true, - push_master: true, - push_protected_branch: false, - push_remove_protected_branch: false, - push_tag: false, - push_new_tag: true, - push_all: false, - }, - - reporter: { - push_new_branch: false, - push_master: false, - push_protected_branch: false, - push_remove_protected_branch: false, - push_tag: false, - push_new_tag: false, - push_all: false, - }, - - guest: { - push_new_branch: false, - push_master: false, - push_protected_branch: false, - push_remove_protected_branch: false, - push_tag: false, - push_new_tag: false, - push_all: false, - } - } - end - - def self.updated_permissions_matrix - updated_permissions_matrix = permissions_matrix.dup - updated_permissions_matrix[:developer][:push_protected_branch] = true - updated_permissions_matrix[:developer][:push_all] = true - updated_permissions_matrix + def stub_git_hooks + # Running the `pre-receive` hook is expensive, and not necessary for this test. + allow_any_instance_of(GitHooksService).to receive(:execute).and_yield end - permissions_matrix.keys.each do |role| - describe "#{role} access" do - before { protect_feature_branch } - before { project.team << [user, role] } - - permissions_matrix[role].each do |action, allowed| - context action do - subject { access.push_access_check(changes[action]) } + def merge_into_protected_branch + @protected_branch_merge_commit ||= begin + stub_git_hooks + project.repository.add_branch(user, unprotected_branch, '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) + rugged = project.repository.rugged + author = { email: "email@example.com", time: Time.now, name: "Example Git User" } - it { expect(subject.allowed?).to allowed ? be_truthy : be_falsey } - end - end + 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)) end end - context "with enabled developers push to protected branches " do - updated_permissions_matrix.keys.each do |role| + def self.run_permission_checks(permissions_matrix) + permissions_matrix.keys.each do |role| describe "#{role} access" do - before { create(:protected_branch, name: 'feature', developers_can_push: true, project: project) } before { project.team << [user, role] } - updated_permissions_matrix[role].each do |action, allowed| + permissions_matrix[role].each do |action, allowed| context action do subject { access.push_access_check(changes[action]) } @@ -259,5 +148,97 @@ describe Gitlab::GitAccess, lib: true do end end end + + permissions_matrix = { + master: { + push_new_branch: true, + push_master: true, + push_protected_branch: true, + push_remove_protected_branch: false, + push_tag: true, + push_new_tag: true, + push_all: true, + merge_into_protected_branch: true + }, + + developer: { + push_new_branch: true, + push_master: true, + push_protected_branch: false, + push_remove_protected_branch: false, + push_tag: false, + push_new_tag: true, + push_all: false, + merge_into_protected_branch: false + }, + + reporter: { + push_new_branch: false, + push_master: false, + push_protected_branch: false, + push_remove_protected_branch: false, + push_tag: false, + push_new_tag: false, + push_all: false, + merge_into_protected_branch: false + }, + + guest: { + push_new_branch: false, + push_master: false, + push_protected_branch: false, + push_remove_protected_branch: false, + push_tag: false, + push_new_tag: false, + push_all: false, + merge_into_protected_branch: false + } + } + + [['feature', 'exact'], ['feat*', 'wildcard']].each do |protected_branch_name, protected_branch_type| + context do + before { create(:protected_branch, name: protected_branch_name, project: project) } + + 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) } + + 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 "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) + end + + run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: true })) + 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 + + run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: false })) + 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 })) + 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) } + + run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true })) + end + end end end diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb new file mode 100644 index 00000000000..aa9ec243498 --- /dev/null +++ b/spec/lib/gitlab/user_access_spec.rb @@ -0,0 +1,88 @@ +require 'spec_helper' + +describe Gitlab::UserAccess, lib: true do + let(:access) { Gitlab::UserAccess.new(user, project: project) } + let(:project) { create(:project) } + let(:user) { create(:user) } + + describe 'can_push_to_branch?' do + describe 'push to none protected branch' do + it 'returns true if user is a master' do + project.team << [user, :master] + expect(access.can_push_to_branch?('random_branch')).to be_truthy + end + + it 'returns true if user is a developer' do + project.team << [user, :developer] + expect(access.can_push_to_branch?('random_branch')).to be_truthy + end + + it 'returns false if user is a reporter' do + project.team << [user, :reporter] + expect(access.can_push_to_branch?('random_branch')).to be_falsey + end + end + + describe 'push to protected branch' do + let(:branch) { create :protected_branch, project: project } + + it 'returns true if user is a master' do + project.team << [user, :master] + expect(access.can_push_to_branch?(branch.name)).to be_truthy + end + + it 'returns false if user is a developer' do + project.team << [user, :developer] + expect(access.can_push_to_branch?(branch.name)).to be_falsey + end + + it 'returns false if user is a reporter' do + project.team << [user, :reporter] + expect(access.can_push_to_branch?(branch.name)).to be_falsey + end + end + + describe 'push to protected branch if allowed for developers' do + before do + @branch = create :protected_branch, project: project, developers_can_push: true + end + + it 'returns true if user is a master' do + project.team << [user, :master] + expect(access.can_push_to_branch?(@branch.name)).to be_truthy + end + + it 'returns true if user is a developer' do + project.team << [user, :developer] + expect(access.can_push_to_branch?(@branch.name)).to be_truthy + end + + it 'returns false if user is a reporter' do + project.team << [user, :reporter] + expect(access.can_push_to_branch?(@branch.name)).to be_falsey + end + end + + describe 'merge to protected branch if allowed for developers' do + before do + @branch = create :protected_branch, project: project, developers_can_merge: true + end + + it 'returns true if user is a master' do + project.team << [user, :master] + expect(access.can_merge_to_branch?(@branch.name)).to be_truthy + end + + it 'returns true if user is a developer' do + project.team << [user, :developer] + expect(access.can_merge_to_branch?(@branch.name)).to be_truthy + end + + it 'returns false if user is a reporter' do + project.team << [user, :reporter] + expect(access.can_merge_to_branch?(@branch.name)).to be_falsey + end + end + + end +end |