diff --git a/app/models/project.rb b/app/models/project.rb index 507813bccf8..16a418d5a3f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -876,14 +876,8 @@ class Project < ActiveRecord::Base ProtectedBranch.matching(branch_name, protected_branches: @protected_branches).present? end - def developers_can_push_to_protected_branch?(branch_name) - return true if empty_repo? && !default_branch_protected? - - protected_branches.matching(branch_name).any?(&:developers_can_push) - end - - def developers_can_merge_to_protected_branch?(branch_name) - protected_branches.matching(branch_name).any?(&:developers_can_merge) + def user_can_push_to_empty_repo?(user) + !default_branch_protected? || team.max_member_access(user.id) > Gitlab::Access::DEVELOPER end def forked? @@ -1278,7 +1272,8 @@ class Project < ActiveRecord::Base private def default_branch_protected? - current_application_settings.default_branch_protection == Gitlab::Access::PROTECTION_FULL + current_application_settings.default_branch_protection == Gitlab::Access::PROTECTION_FULL || + current_application_settings.default_branch_protection == Gitlab::Access::PROTECTION_DEV_CAN_MERGE end def authorized_for_user_by_group?(user, min_access_level) diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb index 3a69027368f..c55a7fc4d3d 100644 --- a/lib/gitlab/user_access.rb +++ b/lib/gitlab/user_access.rb @@ -30,6 +30,8 @@ module Gitlab return false unless user if project.protected_branch?(ref) + return true if project.empty_repo? && project.user_can_push_to_empty_repo?(user) + access_levels = project.protected_branches.matching(ref).map(&:push_access_level) access_levels.any? { |access_level| access_level.check_access(user) } else diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb index 26918f7b82d..d3c3b800b94 100644 --- a/spec/lib/gitlab/user_access_spec.rb +++ b/spec/lib/gitlab/user_access_spec.rb @@ -9,16 +9,19 @@ describe Gitlab::UserAccess, lib: true 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 @@ -67,16 +70,19 @@ describe Gitlab::UserAccess, lib: true do 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 @@ -88,16 +94,19 @@ describe Gitlab::UserAccess, lib: true do 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 @@ -109,19 +118,21 @@ describe Gitlab::UserAccess, lib: true do 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 diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1e6f735699b..567f87b9970 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1126,69 +1126,42 @@ describe Project, models: true do end end - describe "#developers_can_push_to_protected_branch?" do + describe '#user_can_push_to_empty_repo?' do let(:project) { create(:empty_project) } + let(:user) { create(:user) } - context "when the branch matches a protected branch via direct match" do - it "returns true if 'Developers can Push' is turned on" do - create(:protected_branch, name: "production", project: project, developers_can_push: true) + it 'returns false when default_branch_protection is in full protection and user is developer' do + project.team << [user, :developer] + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL) - expect(project.developers_can_push_to_protected_branch?('production')).to be true - end - - it "returns false if 'Developers can Push' is turned off" do - create(:protected_branch, name: "production", project: project, developers_can_push: false) - - expect(project.developers_can_push_to_protected_branch?('production')).to be false - end + expect(project.user_can_push_to_empty_repo?(user)).to be_falsey end - context "when project is new" do - it "returns true if project is unprotected" do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) + it 'returns false when default_branch_protection only lets devs merge and user is dev' do + project.team << [user, :developer] + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) - expect(project.developers_can_push_to_protected_branch?('master')).to be true - end - - it "returns true if project allows developers to push to protected branch" do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) - - expect(project.developers_can_push_to_protected_branch?('master')).to be true - end - - it "returns false if project does not let developer push to protected branch but let them merge branches" do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) - - expect(project.developers_can_push_to_protected_branch?('master')).to be false - end - - it "returns false if project is on full protection mode" do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL) - - expect(project.developers_can_push_to_protected_branch?('master')).to be false - end + expect(project.user_can_push_to_empty_repo?(user)).to be_falsey end - context "when the branch matches a protected branch via wilcard match" do - it "returns true if 'Developers can Push' is turned on" do - create(:protected_branch, name: "production/*", project: project, developers_can_push: true) + it 'returns true when default_branch_protection lets devs push and user is developer' do + project.team << [user, :developer] + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) - expect(project.developers_can_push_to_protected_branch?('production/some-branch')).to be true - end - - it "returns false if 'Developers can Push' is turned off" do - create(:protected_branch, name: "production/*", project: project, developers_can_push: false) - - expect(project.developers_can_push_to_protected_branch?('production/some-branch')).to be false - end + expect(project.user_can_push_to_empty_repo?(user)).to be_truthy end - context "when the branch does not match a protected branch" do - it "returns false" do - create(:protected_branch, name: "production/*", project: project, developers_can_push: true) + it 'returns true when default_branch_protection is unprotected and user is developer' do + project.team << [user, :developer] + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) - expect(project.developers_can_push_to_protected_branch?('staging/some-branch')).to be false - end + expect(project.user_can_push_to_empty_repo?(user)).to be_truthy + end + + it 'returns true when user is master' do + project.team << [user, :master] + + expect(project.user_can_push_to_empty_repo?(user)).to be_truthy end end