Merge branch 'developers-can-merge' into 'master'

Fix conflicts for the "developers can merge" feature

- This feature was implemented for CE in gitlab-org/gitlab-ce!4892.
- It was reverted in CE-master because of merge conflicts with EE-master
- This MR fixes the conflicts. Once this is merged in, the feature can be reintroduced to CE-master

/cc @rymai @rspeicher 

See merge request !564

CHANGELOG

@@ -51,6 +51,7 @@ v 8.10.0 (unreleased)
   - Add "Enabled Git access protocols" to Application Settings
   - Diffs will create button/diff form on demand no on server side
   - Reduce size of HTML used by diff comment forms
+  - Protected branches have a "Developers can Merge" setting. !4892 (original implementation by Mathias Vestergaard)
   - Fix user creation with stronger minimum password requirements !4054 (nathan-pmt)
   - Only show New Snippet button to users that can create snippets.
   - PipelinesFinder uses git cache data

app/assets/javascripts/protected_branches.js.coffee

 $ ->
   $(".protected-branches-list :checkbox").change (e) ->
     name = $(this).attr("name")
-    if name == "developers_can_push"
+    if name == "developers_can_push" || name == "developers_can_merge"
       id = $(this).val()
-      checked = $(this).is(":checked")
+      can_push = $(this).is(":checked")
       url = $(this).data("url")
       $.ajax
-        type: "PUT"
+        type: "PATCH"
         url: url
         dataType: "json"
         data:
           id: id
           protected_branch:
-            developers_can_push: checked
+            "#{name}": can_push
 
         success: ->
           row = $(e.target)

app/controllers/projects/protected_branches_controller.rb

@@ -50,6 +50,6 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
   end
 
   def protected_branch_params
-    params.require(:protected_branch).permit(:name, :developers_can_push)
+    params.require(:protected_branch).permit(:name, :developers_can_push, :developers_can_merge)
   end
 end

app/helpers/branches_helper.rb

@@ -12,7 +12,7 @@ module BranchesHelper
   def can_push_branch?(project, branch_name)
     return false unless project.repository.branch_exists?(branch_name)
 
-    ::Gitlab::GitAccess.new(current_user, project, 'web').can_push_to_branch?(branch_name)
+    ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(branch_name)
   end
 
   def project_branches

app/models/merge_request.rb

@@ -619,7 +619,13 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def can_be_merged_by?(user)
-    ::Gitlab::GitAccess.new(user, project, 'web').can_push_to_branch?(target_branch)
+    access = ::Gitlab::UserAccess.new(user, project: project)
+    access.can_push_to_branch?(target_branch) || access.can_merge_to_branch?(target_branch)
+  end
+
+  def can_be_merged_via_command_line_by?(user)
+    access = ::Gitlab::UserAccess.new(user, project: project)
+    access.can_push_to_branch?(target_branch)
   end
 
   def mergeable_ci_state?

app/models/project.rb

@@ -948,6 +948,10 @@ class Project < ActiveRecord::Base
     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

app/models/repository.rb

+# coding: utf-8
 require 'securerandom'
 require 'forwardable'
 
@@ -859,9 +860,9 @@ class Repository
     end
   end
 
-  def merge(user, source_sha, target_branch, options = {})
-    our_commit = rugged.branches[target_branch].target
-    their_commit = rugged.lookup(source_sha)
+  def merge(user, merge_request, options = {})
+    our_commit = rugged.branches[merge_request.target_branch].target
+    their_commit = rugged.lookup(merge_request.diff_head_sha)
 
     raise "Invalid merge target" if our_commit.nil?
     raise "Invalid merge source" if their_commit.nil?
@@ -869,14 +870,16 @@ class Repository
     merge_index = rugged.merge_commits(our_commit, their_commit)
     return false if merge_index.conflicts?
 
-    commit_with_hooks(user, target_branch) do |ref|
+    commit_with_hooks(user, merge_request.target_branch) do |tmp_ref|
       actual_options = options.merge(
         parents: [our_commit, their_commit],
         tree: merge_index.write_tree(rugged),
-        update_ref: ref
+        update_ref: tmp_ref
       )
 
-      Rugged::Commit.create(rugged, actual_options)
+      commit_id = Rugged::Commit.create(rugged, actual_options)
+      merge_request.update(in_progress_merge_commit_sha: commit_id)
+      commit_id
     end
   end
 

app/services/commits/change_service.rb

@@ -23,7 +23,7 @@ module Commits
     private
 
     def check_push_permissions
-      allowed = ::Gitlab::GitAccess.new(current_user, project, 'web').can_push_to_branch?(@target_branch)
+      allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@target_branch)
 
       unless allowed
         raise ValidationError.new('You are not allowed to push into this branch')

app/services/files/base_service.rb

@@ -42,7 +42,7 @@ module Files
     end
 
     def validate
-      allowed = ::Gitlab::GitAccess.new(current_user, project, 'web').can_push_to_branch?(@target_branch)
+      allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@target_branch)
 
       unless allowed
         raise_error("You are not allowed to push into this branch")

app/services/git_push_service.rb

@@ -95,7 +95,8 @@ class GitPushService < BaseService
     # Set protection on the default branch if configured
     if current_application_settings.default_branch_protection != PROTECTION_NONE
       developers_can_push = current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_PUSH ? true : false
-      @project.protected_branches.create({ name: @project.default_branch, developers_can_push: developers_can_push })
+      developers_can_merge = current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_MERGE ? true : false
+      @project.protected_branches.create({ name: @project.default_branch, developers_can_push: developers_can_push, developers_can_merge: developers_can_merge })
     end
   end
 

app/services/merge_requests/merge_service.rb

@@ -58,7 +58,7 @@ module MergeRequests
         committer: committer
       }
 
-      commit_id = repository.merge(current_user, merge_request.diff_head_sha, merge_request.target_branch, options)
+      commit_id = repository.merge(current_user, merge_request, options)
       merge_request.update(merge_commit_sha: commit_id)
     rescue GitHooksService::PreReceiveError => e
       merge_request.update(merge_error: e.message)
@@ -67,6 +67,8 @@ module MergeRequests
       merge_request.update(merge_error: "Something went wrong during merge")
       Rails.logger.error(e.message)
       false
+    ensure
+      merge_request.update(in_progress_merge_commit_sha: nil)
     end
 
     def after_merge

app/services/merge_requests/refresh_service.rb

@@ -49,7 +49,7 @@ module MergeRequests
     end
 
     def force_push?
-      Gitlab::ForcePushCheck.force_push?(@project, @oldrev, @newrev)
+      Gitlab::Checks::ForcePush.force_push?(@project, @oldrev, @newrev)
     end
 
     # Refresh merge request diff if we push to source or target branch of merge request

app/views/projects/merge_requests/widget/open/_conflicts.html.haml

@@ -4,7 +4,7 @@
 
 %p
   Please resolve these conflicts or
-  - if @merge_request.can_be_merged_by?(current_user)
+  - if @merge_request.can_be_merged_via_command_line_by?(current_user)
     #{link_to "merge this request manually", "#modal_merge_info", class: "how_to_merge_link vlink", "data-toggle" => "modal"}.
   - else
     ask someone with write access to this repository to merge this request manually.

app/views/projects/protected_branches/_branches_list.html.haml

@@ -8,6 +8,7 @@
   .table-responsive
     %table.table.protected-branches-list
       %colgroup
+        %col{ width: "20%" }
         %col{ width: "30%" }
         %col{ width: "25%" }
         %col{ width: "25%" }
@@ -18,6 +19,7 @@
           %th Protected Branch
           %th Commit
           %th Developers Can Push
+          %th Developers Can Merge
           - if can_admin_project
             %th
       %tbody

app/views/projects/protected_branches/_protected_branch.html.haml

@@ -16,6 +16,8 @@
         (branch was removed from repository)
   %td
     = check_box_tag("developers_can_push", protected_branch.id, protected_branch.developers_can_push, data: { url: url })
+  %td
+    = check_box_tag("developers_can_merge", protected_branch.id, protected_branch.developers_can_merge, data: { url: url })
   - if can_admin_project
     %td
       = link_to 'Unprotect', [@project.namespace.becomes(Namespace), @project, protected_branch], data: { confirm: 'Branch will be writable for developers. Are you sure?' }, method: :delete, class: "btn btn-warning btn-sm pull-right"

app/views/projects/protected_branches/index.html.haml

@@ -36,6 +36,14 @@
             = f.label :developers_can_push, "Developers can push", class: "label-light append-bottom-0"
             %p.light.append-bottom-0
               Allow developers to push to this branch
+
+        .form-group
+          = f.check_box :developers_can_merge, class: "pull-left"
+          .prepend-left-20
+            = f.label :developers_can_merge, "Developers can merge", class: "label-light append-bottom-0"
+            %p.light.append-bottom-0
+              Allow developers to accept merge requests to this branch
         = f.submit "Protect", class: "btn-create btn protect-branch-btn", disabled: true
+
     %hr
     = render "branches_list"

db/migrate/20160519203051_add_developers_can_merge_to_protected_branches.rb

+class AddDevelopersCanMergeToProtectedBranches < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  def change
+    add_column_with_default :protected_branches, :developers_can_merge, :boolean, default: false, allow_null: false
+  end
+end

db/migrate/20160629025435_add_column_in_progress_merge_commit_sha_to_merge_requests.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddColumnInProgressMergeCommitShaToMergeRequests < ActiveRecord::Migration
+  def change
+    add_column :merge_requests, :in_progress_merge_commit_sha, :string
+  end
+end

db/schema.rb

@@ -701,6 +701,7 @@ ActiveRecord::Schema.define(version: 20160712171823) do
     t.datetime "deleted_at"
     t.integer  "approvals_before_merge"
     t.string   "rebase_commit_sha"
+    t.string   "in_progress_merge_commit_sha"
   end
 
   add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
@@ -979,6 +980,7 @@ ActiveRecord::Schema.define(version: 20160712171823) do
     t.datetime "created_at"
     t.datetime "updated_at"
     t.boolean  "developers_can_push",  default: false, null: false
+    t.boolean  "developers_can_merge", default: false, null: false
   end
 
   add_index "protected_branches", ["project_id"], name: "index_protected_branches_on_project_id", using: :btree

lib/api/helpers.rb

@@ -17,7 +17,7 @@ module API
     def current_user
       @current_user ||= (find_user_by_private_token || doorkeeper_guard)
 
-      unless @current_user && Gitlab::UserAccess.allowed?(@current_user)
+      unless @current_user && Gitlab::UserAccess.new(@current_user).allowed?
         return nil
       end
 

lib/gitlab/access.rb

@@ -17,6 +17,7 @@ module Gitlab
     PROTECTION_NONE          = 0
     PROTECTION_DEV_CAN_PUSH  = 1
     PROTECTION_FULL          = 2
+    PROTECTION_DEV_CAN_MERGE = 3
 
     class << self
       def values
@@ -54,6 +55,7 @@ module Gitlab
       def protection_options
         {
           "Not protected: Both developers and masters can push new commits, force push, or delete the branch." => PROTECTION_NONE,
+          "Protected against pushes: Developers cannot push new commits, but are allowed to accept merge requests to the branch." => PROTECTION_DEV_CAN_MERGE,
           "Partially protected: Developers can push new commits, but cannot force push or delete the branch. Masters can do all of those." => PROTECTION_DEV_CAN_PUSH,
           "Fully protected: Developers cannot push new commits, force push, or delete the branch. Only masters can do any of those." => PROTECTION_FULL,
         }

lib/gitlab/checks/change_access.rb

+module Gitlab
+  module Checks
+    class ChangeAccess
+      include PathLocksHelper
+      attr_reader :user_access, :project
+
+      def initialize(change, user_access:, project:)
+        @oldrev, @newrev, @ref = change.split(' ')
+        @branch_name = branch_name(@ref)
+        @user_access = user_access
+        @project = project
+      end
+
+      def exec
+        error = protected_branch_checks || tag_checks || push_checks || push_rules_checks
+
+        if error
+          GitAccessStatus.new(false, error)
+        else
+          GitAccessStatus.new(true)
+        end
+      end
+
+      protected
+
+      def protected_branch_checks
+        return unless project.protected_branch?(@branch_name)
+
+        if forced_push? && user_access.cannot_do_action?(:force_push_code_to_protected_branches)
+          return "You are not allowed to force push code to a protected branch on this project."
+        elsif Gitlab::Git.blank_ref?(@newrev) && user_access.cannot_do_action?(:remove_protected_branches)
+          return "You are not allowed to delete protected branches from this project."
+        end
+
+        if matching_merge_request?
+          if user_access.can_merge_to_branch?(@branch_name) || user_access.can_push_to_branch?(@branch_name)
+            return
+          else
+            "You are not allowed to merge code into protected branches on this project."
+          end
+        else
+          if user_access.can_push_to_branch?(@branch_name)
+            return
+          else
+            "You are not allowed to push code to protected branches on this project."
+          end
+        end
+      end
+
+      def tag_checks
+        tag_ref = tag_name(@ref)
+
+        if tag_ref && protected_tag?(tag_ref) && user_access.cannot_do_action?(:admin_project)
+          "You are not allowed to change existing tags on this project."
+        end
+      end
+
+      def push_checks
+        if user_access.cannot_do_action?(:push_code)
+          "You are not allowed to push code to this project."
+        end
+      end
+
+      def push_rules_checks
+        # Returns nil if all push rule checks passed successfully
+        # or the error if any hook fails
+        error = push_rule_check
+
+        if !error && license_allows_file_locks?
+          error = path_locks_check
+        end
+
+        error
+      end
+
+      private
+
+      def protected_tag?(tag_name)
+        project.repository.tag_exists?(tag_name)
+      end
+
+      def forced_push?
+        Gitlab::Checks::ForcePush.force_push?(@project, @oldrev, @newrev)
+      end
+
+      def matching_merge_request?
+        Checks::MatchingMergeRequest.new(@newrev, @branch_name, @project).match?
+      end
+
+      def branch_name(ref)
+        ref = @ref.to_s
+        if Gitlab::Git.branch_ref?(ref)
+          Gitlab::Git.ref_name(ref)
+        else
+          nil
+        end
+      end
+
+      def tag_name(ref)
+        ref = @ref.to_s
+        if Gitlab::Git.tag_ref?(ref)
+          Gitlab::Git.ref_name(ref)
+        else
+          nil
+        end
+      end
+
+      def push_rule_check
+        return unless project.push_rule && @newrev && @oldrev
+
+        push_rule = project.push_rule
+
+        # Prevent tag removal
+        if tag_name(@ref)
+          if push_rule.deny_delete_tag && protected_tag?(tag_name(@ref)) && Gitlab::Git.blank_ref?(@newrev)
+            "You can not delete tag"
+          end
+        else
+          # if newrev is blank, the branch was deleted
+          return if Gitlab::Git.blank_ref?(@newrev) || !push_rule.commit_validation?
+
+          # if oldrev is blank, the branch was just created
+          oldrev = Gitlab::Git.blank_ref?(@oldrev) ? project.default_branch : @oldrev
+
+          commits(oldrev).each do |commit|
+            next if commit_from_annex_sync?(commit.safe_message) || old_commit?(commit)
+
+            if error = check_commit(commit, push_rule)
+              return error
+            end
+          end
+        end
+
+        nil
+      end
+
+      def path_locks_check
+        return unless project.path_locks.any? && @newrev && @oldrev
+
+        # locks protect default branch only
+        return if project.default_branch != branch_name(@ref)
+
+        commits(@oldrev).each do |commit|
+          next if commit_from_annex_sync?(commit.safe_message)
+
+          commit.diffs.each do |diff|
+            path = diff.new_path || diff.old_path
+
+            lock_info = project.find_path_lock(path)
+
+            if lock_info && lock_info.user != user_access.user
+              return "The path '#{lock_info.path}' is locked by #{lock_info.user.name}"
+            end
+          end
+        end
+
+        nil
+      end
+
+      # If commit does not pass push rule validation the whole push should be rejected.
+      # This method should return nil if no error found or status object if there are some errors.
+      # In case of errors - all other checks will be canceled and push will be rejected.
+      def check_commit(commit, push_rule)
+        unless push_rule.commit_message_allowed?(commit.safe_message)
+          return "Commit message does not follow the pattern '#{push_rule.commit_message_regex}'"
+        end
+
+        unless push_rule.author_email_allowed?(commit.committer_email)
+          return "Committer's email '#{commit.committer_email}' does not follow the pattern '#{push_rule.author_email_regex}'"
+        end
+
+        unless push_rule.author_email_allowed?(commit.author_email)
+          return "Author's email '#{commit.author_email}' does not follow the pattern '#{push_rule.author_email_regex}'"
+        end
+
+        # Check whether author is a GitLab member
+        if push_rule.member_check
+          unless User.existing_member?(commit.author_email.downcase)
+            return "Author '#{commit.author_email}' is not a member of team"
+          end
+
+          if commit.author_email.casecmp(commit.committer_email) == -1
+            unless User.existing_member?(commit.committer_email.downcase)
+              return "Committer '#{commit.committer_email}' is not a member of team"
+            end
+          end
+        end
+
+        if error = check_commit_diff(commit, push_rule)
+          return error
+        end
+
+        nil
+      end
+
+      def check_commit_diff(commit, push_rule)
+        if push_rule.file_name_regex.present?
+          commit.diffs.each do |diff|
+            if (diff.renamed_file || diff.new_file) && diff.new_path =~ Regexp.new(push_rule.file_name_regex)
+              return "File name #{diff.new_path.inspect} is prohibited by the pattern '#{push_rule.file_name_regex}'"
+            end
+          end
+        end
+
+        if push_rule.max_file_size > 0
+          commit.diffs.each do |diff|
+            next if diff.deleted_file
+
+            blob = project.repository.blob_at(commit.id, diff.new_path)
+            if blob && blob.size && blob.size > push_rule.max_file_size.megabytes
+              return "File #{diff.new_path.inspect} is larger than the allowed size of #{push_rule.max_file_size} MB"
+            end
+          end
+        end
+
+        nil
+      end
+
+      def commits(oldrev)
+        if oldrev
+          project.repository.commits_between(oldrev, @newrev)
+        else
+          project.repository.commits(@newrev)
+        end
+      end
+
+      def commit_from_annex_sync?(commit_message)
+        return false unless Gitlab.config.gitlab_shell.git_annex_enabled
+
+        # Commit message starting with <git-annex in > so avoid push rules on this
+        commit_message.start_with?('git-annex in')
+      end
+
+      def old_commit?(commit)
+        # We skip refs/tmp ref because we use it for Web UI commiting
+        commit.refs(project.repository).reject { |ref| ref.name.start_with?('refs/tmp') }.any?
+      end
+    end
+  end
+end

lib/gitlab/checks/force_push.rb

+module Gitlab
+  module Checks
+    class ForcePush
+      def self.force_push?(project, oldrev, newrev)
+        return false if project.empty_repo?
+
+        # Created or deleted branch
+        if Gitlab::Git.blank_ref?(oldrev) || Gitlab::Git.blank_ref?(newrev)
+          false
+        else
+          missed_refs, _ = Gitlab::Popen.popen(%W(#{Gitlab.config.git.bin_path} --git-dir=#{project.repository.path_to_repo} rev-list #{oldrev} ^#{newrev}))
+          missed_refs.split("\n").size > 0
+        end
+      end
+    end
+  end
+end

lib/gitlab/checks/matching_merge_request.rb

+module Gitlab
+  module Checks
+    class MatchingMergeRequest
+      def initialize(newrev, branch_name, project)
+        @newrev = newrev
+        @branch_name = branch_name
+        @project = project
+      end
+
+      def match?
+        @project.merge_requests
+          .with_state(:locked)
+          .where(in_progress_merge_commit_sha: @newrev, target_branch: @branch_name)
+          .exists?
+      end
+    end
+  end
+end

lib/gitlab/force_push_check.rb

-module Gitlab
-  class ForcePushCheck
-    def self.force_push?(project, oldrev, newrev)
-      return false if project.empty_repo?
-
-      # Created or deleted branch
-      if Gitlab::Git.blank_ref?(oldrev) || Gitlab::Git.blank_ref?(newrev)
-        false
-      else
-        missed_refs, _ = Gitlab::Popen.popen(%W(#{Gitlab.config.git.bin_path} --git-dir=#{project.repository.path_to_repo} rev-list #{oldrev} ^#{newrev}))
-        missed_refs.split("\n").size > 0
-      end
-    end
-  end
-end

lib/gitlab/git_access.rb

+# Check a user's access to perform a git action. All public methods in this
+# class return an instance of `GitlabAccessStatus`
 module Gitlab
   class GitAccess
     include PathLocksHelper
@@ -6,58 +8,13 @@ module Gitlab
     PUSH_COMMANDS = %w{ git-receive-pack }
     GIT_ANNEX_COMMANDS = %w{ git-annex-shell }
 
-    attr_reader :actor, :project, :protocol
+    attr_reader :actor, :project, :protocol, :user_access
 
     def initialize(actor, project, protocol)
       @actor    = actor
       @project  = project
       @protocol = protocol
-    end
-
-    def user
-      return @user if defined?(@user)
-
-      @user =
-        case actor
-        when User
-          actor
-        when DeployKey
-          nil
-        when GeoNodeKey
-          nil
-        when Key
-          actor.user
-        end
-    end
-
-    def deploy_key
-      actor if actor.is_a?(DeployKey)
-    end
-
-    def geo_node_key
-      actor if actor.is_a?(GeoNodeKey)
-    end
-
-    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)
-      else
-        user.can?(:push_code, project)
-      end
-    end
-
-    def can_read_project?
-      if user
-        user.can?(:read_project, project)
-      elsif deploy_key
-        deploy_key.projects.include?(project)
-      elsif geo_node_key
-        true
-      else
-        false
-      end
+      @user_access = UserAccess.new(user, project: project)
     end
 
     def check(cmd, changes = nil)
@@ -67,11 +24,11 @@ module Gitlab
         return build_status_object(false, "No user or key was provided.")
       end
 
-      if user && !user_allowed?
+      if user && !user_access.allowed?
         return build_status_object(false, "Your account has been blocked.")
       end
 
-      unless project && can_read_project?
+      unless project && (user_access.can_read_project? || deploy_key_can_read_project? || geo_node_key)
         return build_status_object(false, 'The project you were looking for could not be found.')
       end
 
@@ -118,7 +75,7 @@ module Gitlab
     end
 
     def user_download_access_check
-      unless user.can?(:download_code, project)
+      unless user_access.can_do_action?(:download_code)
         return build_status_object(false, "You are not allowed to download code from this project.")
       end
 
@@ -153,50 +110,8 @@ module Gitlab
       build_status_object(true)
     end
 
-    def can_user_do_action?(action)
-      @permission_cache ||= {}
-      @permission_cache[action] ||= user.can?(action, project)
-    end
-
     def change_access_check(change)
-      oldrev, newrev, ref = change.split(' ')
-
-      action =
-        if project.protected_branch?(branch_name(ref))
-          protected_branch_action(oldrev, newrev, branch_name(ref))
-        elsif (tag_ref = tag_name(ref)) && protected_tag?(tag_ref)
-          # Prevent any changes to existing git tag unless user has permissions
-          :admin_project
-        else
-          :push_code
-        end
-
-      unless can_user_do_action?(action)
-        status =
-          case action
-          when :force_push_code_to_protected_branches
-            build_status_object(false, "You are not allowed to force push code to a protected branch on this project.")
-          when :remove_protected_branches
-            build_status_object(false, "You are not allowed to deleted protected branches from this project.")
-          when :push_code_to_protected_branches
-            build_status_object(false, "You are not allowed to push code to protected branches on this project.")
-          when :admin_project
-            build_status_object(false, "You are not allowed to change existing tags on this project.")
-          else # :push_code
-            build_status_object(false, "You are not allowed to push code to this project.")
-          end
-        return status
-      end
-
-      # Return build_status_object(true) if all push rule checks passed successfully
-      # or build_status_object(false) if any hook fails
-      result = push_rule_check(user, project, ref, oldrev, newrev)
-
-      if result.status && license_allows_file_locks?
-        result = path_locks_check(user, project, ref, oldrev, newrev)
-      end
-
-      result
+      Checks::ChangeAccess.new(change, user_access: user_access, project: project).exec
     end
 
     def forced_push?(oldrev, newrev)
@@ -207,64 +122,8 @@ module Gitlab
       Gitlab::ProtocolAccess.allowed?(protocol)
     end
 
-    def path_locks_check(user, project, ref, oldrev, newrev)
-      unless project.path_locks.any? && newrev && oldrev
-        return build_status_object(true)
-      end
-
-      # locks protect default branch only
-      if project.default_branch != branch_name(ref)
-        return build_status_object(true)
-      end
-
-      commits(newrev, oldrev, project).each do |commit|
-        next if commit_from_annex_sync?(commit.safe_message)
-
-        commit.diffs.each do |diff|
-          path = diff.new_path || diff.old_path
-
-          lock_info = project.find_path_lock(path)
-
-          if lock_info && lock_info.user != user
-            return build_status_object(false, "The path '#{lock_info.path}' is locked by #{lock_info.user.name}")
-          end
-        end
-      end
-
-      build_status_object(true)
-    end
-
-    def push_rule_check(user, project, ref, oldrev, newrev)
-      unless project.push_rule && newrev && oldrev
-        return build_status_object(true)
-      end
-
-      push_rule = project.push_rule
-
-      # Prevent tag removal
-      if Gitlab::Git.tag_ref?(ref)
-        if push_rule.deny_delete_tag && protected_tag?(tag_name(ref)) && Gitlab::Git.blank_ref?(newrev)
-          return build_status_object(false, "You can not delete tag")
-        end
-      else
-        # if newrev is blank, the branch was deleted
-        if Gitlab::Git.blank_ref?(newrev) || !push_rule.commit_validation?
-          return build_status_object(true)
-        end
-
-        # if oldrev is blank, the branch was just created
-        oldrev = project.default_branch if Gitlab::Git.blank_ref?(oldrev)
-
-        commits(newrev, oldrev, project).each do |commit|
-          next if commit_from_annex_sync?(commit.safe_message) || old_commit?(commit)
-
-          if status_object = check_commit(commit, push_rule)
-            return status_object
-          end
-        end
-      end
-
-      build_status_object(true)
+    def matching_merge_request?(newrev, branch_name)
+      Checks::MatchingMergeRequest.new(newrev, branch_name, project).match?
     end
 
     private
@@ -343,6 +202,8 @@ module Gitlab
       elsif Gitlab::Git.blank_ref?(newrev)
         # and we dont allow remove of protected branch
         :remove_protected_branches
+      elsif matching_merge_request?(newrev, branch_name) && project.developers_can_merge_to_protected_branch?(branch_name)
+        :push_code
       elsif project.developers_can_push_to_protected_branch?(branch_name)
         :push_code
       else
@@ -354,25 +215,39 @@ module Gitlab
       project.repository.tag_exists?(tag_name)
     end
 
-    def user_allowed?
-      Gitlab::UserAccess.allowed?(user)
+    private
+
+    def deploy_key
+      actor if actor.is_a?(DeployKey)
+    end
+
+    def geo_node_key
+      actor if actor.is_a?(GeoNodeKey)
     end
 
-    def branch_name(ref)
-      ref = ref.to_s
-      if Gitlab::Git.branch_ref?(ref)
-        Gitlab::Git.ref_name(ref)
+    def deploy_key_can_read_project?
+      if deploy_key
+        deploy_key.projects.include?(project)
       else
-        nil
+        false
       end
     end
 
-    def tag_name(ref)
-      ref = ref.to_s
-      if Gitlab::Git.tag_ref?(ref)
-        Gitlab::Git.ref_name(ref)
-      else
+    protected
+
+    def user
+      return @user if defined?(@user)
+
+      @user =
+        case actor
+        when User
+          actor
+        when DeployKey
+          nil
+        when GeoNodeKey
           nil
+        when Key
+          actor.user
         end
     end
 

lib/gitlab/git_access_wiki.rb

@@ -3,7 +3,7 @@ module Gitlab
     def change_access_check(change)
       if Gitlab::Geo.enabled? && Gitlab::Geo.secondary?
         build_status_object(false, "You can't push code to a secondary GitLab Geo node.")
-      elsif user.can?(:create_wiki, project)
+      elsif user_access.can_do_action?(:create_wiki)
         build_status_object(true)
       else
         build_status_object(false, "You are not allowed to write to this project's wiki.")

lib/gitlab/user_access.rb

 module Gitlab
-  module UserAccess
-    def self.allowed?(user)
-      return false if user.blocked?
+  class UserAccess
+    attr_reader :user, :project
+
+    def initialize(user, project: nil)
+      @user = user
+      @project = project
+    end
+
+    def can_do_action?(action)
+      @permission_cache ||= {}
+      @permission_cache[action] ||= user.can?(action, project)
+    end
+
+    def cannot_do_action?(action)
+      !can_do_action?(action)
+    end
+
+    def allowed?
+      return false if user.blank? || user.blocked?
 
       if user.requires_ldap_check? && user.try_obtain_ldap_lease
         return false unless Gitlab::LDAP::Access.allowed?(user)
@@ -9,5 +25,31 @@ module Gitlab
 
       true
     end
+
+    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)
+      else
+        user.can?(:push_code, project)
+      end
+    end
+
+    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)
+      else
+        user.can?(:push_code, project)
+      end
+    end
+
+    def can_read_project?
+      return false unless user
+
+      user.can?(:read_project, project)
+    end
   end
 end

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
 

spec/lib/gitlab/user_access_spec.rb

+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

spec/models/repository_spec.rb

@@ -4,16 +4,17 @@ describe Repository, models: true do
   include RepoHelpers
   TestBlob = Struct.new(:name)
 
-  let(:repository) { create(:project).repository }
+  let(:project) { create(:project) }
+  let(:repository) { project.repository }
   let(:user) { create(:user) }
   let(:commit_options) do
     author = repository.user_to_committer(user)
     { message: 'Test message', committer: author, author: author }
   end
   let(:merge_commit) do
-    source_sha = repository.find_branch('feature').target
-    merge_commit_sha = repository.merge(user, source_sha, 'master', commit_options)
-    repository.commit(merge_commit_sha)
+    merge_request = create(:merge_request, source_branch: 'feature', target_branch: 'master', source_project: project)
+    merge_commit_id = repository.merge(user, merge_request, commit_options)
+    repository.commit(merge_commit_id)
   end
 
   describe '#branch_names_contains' do

spec/requests/api/api_helpers_spec.rb

@@ -49,7 +49,7 @@ describe API::Helpers, api: true do
 
       it "should return nil for a user without access" do
         env[API::Helpers::PRIVATE_TOKEN_HEADER] = user.private_token
-        allow(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
+        allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
         expect(current_user).to be_nil
       end
 
@@ -73,7 +73,7 @@ describe API::Helpers, api: true do
 
       it "should return nil for a user without access" do
         env[API::Helpers::PRIVATE_TOKEN_HEADER] = personal_access_token.token
-        allow(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
+        allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
         expect(current_user).to be_nil
       end
 

spec/services/git_push_service_spec.rb

@@ -240,7 +240,7 @@ describe GitPushService, services: true do
       it "when pushing a branch for the first time" do
         expect(project).to receive(:execute_hooks)
         expect(project.default_branch).to eq("master")
-        expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: false })
+        expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: false, developers_can_merge: false })
         execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' )
       end
 
@@ -258,7 +258,17 @@ describe GitPushService, services: true do
 
         expect(project).to receive(:execute_hooks)
         expect(project.default_branch).to eq("master")
-        expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: true })
+        expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: true, developers_can_merge: false })
+
+        execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master')
+      end
+
+      it "when pushing a branch for the first time with default branch protection set to 'developers can merge'" do
+        stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE)
+
+        expect(project).to receive(:execute_hooks)
+        expect(project.default_branch).to eq("master")
+        expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: false, developers_can_merge: true })
         execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' )
       end
 

spec/services/merge_requests/refresh_service_spec.rb

@@ -100,8 +100,7 @@ describe MergeRequests::RefreshService, services: true do
         # Merge master -> feature branch
         author = { email: 'test@gitlab.com', time: Time.now, name: "Me" }
         commit_options = { message: 'Test message', committer: author, author: author }
-        master_commit = @project.repository.commit('master')
-        @project.repository.merge(@user, master_commit.id, 'feature', commit_options)
+        @project.repository.merge(@user, @merge_request, commit_options)
         commit = @project.repository.commit('feature')
         service.new(@project, @user).execute(@oldrev, commit.id, 'refs/heads/feature')
         reload_mrs