developer cannot push to protected branch when project is empty or he has not…

CHANGELOG

@@ -13,6 +13,7 @@ v 8.11.0 (unreleased)
   - Environments have an url to link to
   - Limit git rev-list output count to one in forced push check
   - Clean up unused routes (Josef Strzibny)
+  - Fix issue on empty project to allow developers to only push to protected branches if given permission
   - Add green outline to New Branch button. !5447 (winniehell)
   - Update to gitlab_git 10.4.1 and take advantage of preserved Ref objects
   - Retrieve rendered HTML from cache in one request

app/models/project.rb

@@ -870,10 +870,22 @@ class Project < ActiveRecord::Base
 
   # Check if current branch name is marked as protected in the system
   def protected_branch?(branch_name)
+    return true if empty_repo? && default_branch_protected?
+
     @protected_branches ||= self.protected_branches.to_a
     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)
+  end
+
   def forked?
     !(forked_project_link.nil? || forked_project_link.forked_from_project.nil?)
   end
@@ -1265,6 +1277,10 @@ class Project < ActiveRecord::Base
 
   private
 
+  def default_branch_protected?
+    current_application_settings.default_branch_protection == Gitlab::Access::PROTECTION_FULL
+  end
+
   def authorized_for_user_by_group?(user, min_access_level)
     member = user.group_members.find_by(source_id: group)
 

spec/lib/gitlab/user_access_spec.rb

@@ -23,6 +23,45 @@ describe Gitlab::UserAccess, lib: true do
       end
     end
 
+    describe 'push to empty project' do
+      let(:empty_project) { create(:project_empty_repo) }
+      let(:project_access) { Gitlab::UserAccess.new(user, project: empty_project) }
+
+      it 'returns true if user is master' do
+        empty_project.team << [user, :master]
+
+        expect(project_access.can_push_to_branch?('master')).to be_truthy
+      end
+
+      it 'returns false if user is developer and project is fully protected' do
+        empty_project.team << [user, :developer]
+        stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL)
+
+        expect(project_access.can_push_to_branch?('master')).to be_falsey
+      end
+
+      it 'returns false if user is developer and it is not allowed to push new commits but can merge into branch' do
+        empty_project.team << [user, :developer]
+        stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE)
+
+        expect(project_access.can_push_to_branch?('master')).to be_falsey
+      end
+
+      it 'returns true if user is developer and project is unprotected' do
+        empty_project.team << [user, :developer]
+        stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE)
+
+        expect(project_access.can_push_to_branch?('master')).to be_truthy
+      end
+
+      it 'returns true if user is developer and project grants developers permission' do
+        empty_project.team << [user, :developer]
+        stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH)
+
+        expect(project_access.can_push_to_branch?('master')).to be_truthy
+      end
+    end
+
     describe 'push to protected branch' do
       let(:branch) { create :protected_branch, project: project }
 

spec/models/project_spec.rb

@@ -69,6 +69,7 @@ describe Project, models: true do
     it { is_expected.to include_module(Gitlab::ConfigHelper) }
     it { is_expected.to include_module(Gitlab::ShellAdapter) }
     it { is_expected.to include_module(Gitlab::VisibilityLevel) }
+    it { is_expected.to include_module(Gitlab::CurrentSettings) }
     it { is_expected.to include_module(Referable) }
     it { is_expected.to include_module(Sortable) }
   end
@@ -1070,28 +1071,124 @@ describe Project, models: true do
   end
 
   describe '#protected_branch?' do
+    context 'existing project' do
+      let(:project) { create(:project) }
+
+      it 'returns true when the branch matches a protected branch via direct match' do
+        project.protected_branches.create!(name: 'foo')
+
+        expect(project.protected_branch?('foo')).to eq(true)
+      end
+
+      it 'returns true when the branch matches a protected branch via wildcard match' do
+        project.protected_branches.create!(name: 'production/*')
+
+        expect(project.protected_branch?('production/some-branch')).to eq(true)
+      end
+
+      it 'returns false when the branch does not match a protected branch via direct match' do
+        expect(project.protected_branch?('foo')).to eq(false)
+      end
+
+      it 'returns false when the branch does not match a protected branch via wildcard match' do
+        project.protected_branches.create!(name: 'production/*')
+
+        expect(project.protected_branch?('staging/some-branch')).to eq(false)
+      end
+    end
+
+    context "new project" do
+      let(:project) { create(:empty_project) }
+
+      it 'returns false when default_protected_branch is unprotected' do
+        stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE)
+
+        expect(project.protected_branch?('master')).to be false
+      end
+
+      it 'returns false when default_protected_branch lets developers push' do
+        stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH)
+
+        expect(project.protected_branch?('master')).to be false
+      end
+
+      it 'returns true when default_branch_protection does not let developers push but let developer merge branches' do
+        stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE)
+
+        expect(project.protected_branch?('master')).to be true
+      end
+
+      it 'returns true when default_branch_protection is in full protection' do
+        stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL)
+
+        expect(project.protected_branch?('master')).to be true
+      end
+    end
+  end
+
+  describe "#developers_can_push_to_protected_branch?" do
     let(:project) { create(:empty_project) }
 
-    it 'returns true when the branch matches a protected branch via direct match' do
-      project.protected_branches.create!(name: 'foo')
+    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)
+
+        expect(project.developers_can_push_to_protected_branch?('production')).to be true
+      end
 
-      expect(project.protected_branch?('foo')).to eq(true)
+      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
     end
 
-    it 'returns true when the branch matches a protected branch via wildcard match' do
-      project.protected_branches.create!(name: 'production/*')
+    context "when project is new" do
+      it "returns true if project is unprotected" do
+        stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE)
+
+        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.protected_branch?('production/some-branch')).to eq(true)
+        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
     end
 
-    it 'returns false when the branch does not match a protected branch via direct match' do
-      expect(project.protected_branch?('foo')).to eq(false)
+    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)
+
+        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
     end
 
-    it 'returns false when the branch does not match a protected branch via wildcard match' do
-      project.protected_branches.create!(name: 'production/*')
+    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)
 
-      expect(project.protected_branch?('staging/some-branch')).to eq(false)
+        expect(project.developers_can_push_to_protected_branch?('staging/some-branch')).to be false
+      end
     end
   end