1 files changed, 61 insertions, 53 deletions

diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index c6b6efda360..545506f3dfd 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -7,7 +7,8 @@ module Gitlab
     ERROR_MESSAGES = {
       upload: 'You are not allowed to upload code for this project.',
       download: 'You are not allowed to download code from this project.',
-      deploy_key: 'Deploy keys are not allowed to push code.',
+      deploy_key_upload:
+        'This deploy key does not have write access to this project.',
       no_repo: 'A repository for this project does not exist yet.'
     }
 
@@ -28,13 +29,14 @@ module Gitlab
 
     def check(cmd, changes)
       check_protocol!
-      check_active_user!
+      check_active_user! unless deploy_key?
       check_project_accessibility!
       check_command_existence!(cmd)
+      check_repository_existence!
 
       case cmd
       when *DOWNLOAD_COMMANDS
-        download_access_check
+        download_access_check unless deploy_key?
       when *PUSH_COMMANDS
         push_access_check(changes)
       end
@@ -45,29 +47,31 @@ module Gitlab
     end
 
     def download_access_check
-      if user
-        user_download_access_check
-      elsif deploy_key.nil? && !guest_can_downlod_code?
+      passed = user_can_download_code? ||
+               build_can_download_code? ||
+               guest_can_download_code?
+
+      unless passed
         raise UnauthorizedError, ERROR_MESSAGES[:download]
       end
     end
 
     def push_access_check(changes)
-      if user
-        user_push_access_check(changes)
+      if deploy_key
+        deploy_key_push_access_check
+      elsif user
+        user_push_access_check
       else
-        raise UnauthorizedError, ERROR_MESSAGES[deploy_key ? :deploy_key : :upload]
+        raise UnauthorizedError, ERROR_MESSAGES[:upload]
       end
-    end
 
-    def guest_can_downlod_code?
-      Guest.can?(:download_code, project)
+      return if changes.blank? # Allow access.
+
+      check_change_access!(changes)
     end
 
-    def user_download_access_check
-      unless user_can_download_code? || build_can_download_code?
-        raise UnauthorizedError, ERROR_MESSAGES[:download]
-      end
+    def guest_can_download_code?
+      Guest.can?(:download_code, project)
     end
 
     def user_can_download_code?
@@ -78,33 +82,16 @@ module Gitlab
       authentication_abilities.include?(:build_download_code) && user_access.can_do_action?(:build_download_code)
     end
 
-    def user_push_access_check(changes)
+    def user_push_access_check
       unless authentication_abilities.include?(:push_code)
         raise UnauthorizedError, ERROR_MESSAGES[:upload]
       end
-
-      if changes.blank?
-        return # Allow access.
-      end
-
-      unless project.repository.exists?
-        raise UnauthorizedError, ERROR_MESSAGES[:no_repo]
-      end
-
-      changes_list = Gitlab::ChangesList.new(changes)
-
-      # Iterate over all changes to find if user allowed all of them to be applied
-      changes_list.each do |change|
-        status = change_access_check(change)
-        unless status.allowed?
-          # If user does not have access to make at least one change - cancel all push
-          raise UnauthorizedError, status.message
-        end
-      end
     end
 
-    def change_access_check(change)
-      Checks::ChangeAccess.new(change, user_access: user_access, project: project, env: @env).exec
+    def deploy_key_push_access_check
+      unless deploy_key.can_push_to?(project)
+        raise UnauthorizedError, ERROR_MESSAGES[:deploy_key_upload]
+      end
     end
 
     def protocol_allowed?
@@ -137,31 +124,52 @@ module Gitlab
       end
     end
 
+    def check_repository_existence!
+      unless project.repository.exists?
+        raise UnauthorizedError, ERROR_MESSAGES[:no_repo]
+      end
+    end
+
+    def check_change_access!(changes)
+      changes_list = Gitlab::ChangesList.new(changes)
+
+      # Iterate over all changes to find if user allowed all of them to be applied
+      changes_list.each do |change|
+        status = check_single_change_access(change)
+        unless status.allowed?
+          # If user does not have access to make at least one change - cancel all push
+          raise UnauthorizedError, status.message
+        end
+      end
+    end
+
+    def check_single_change_access(change)
+      Checks::ChangeAccess.new(
+        change,
+        user_access: user_access,
+        project: project,
+        env: @env,
+        skip_authorization: deploy_key?).exec
+    end
+
     def matching_merge_request?(newrev, branch_name)
       Checks::MatchingMergeRequest.new(newrev, branch_name, project).match?
     end
 
     def deploy_key
-      actor if actor.is_a?(DeployKey)
+      actor if deploy_key?
     end
 
-    def deploy_key_can_read_project?
-      if deploy_key
-        return true if project.public?
-        deploy_key.projects.include?(project)
-      else
-        false
-      end
+    def deploy_key?
+      actor.is_a?(DeployKey)
     end
 
     def can_read_project?
-      if user
-        user_access.can_read_project?
-      elsif deploy_key
-        deploy_key_can_read_project?
-      else
-        Guest.can?(:read_project, project)
-      end
+      if deploy_key
+        deploy_key.has_access_to?(project)
+      elsif user
+        user.can?(:read_project, project)
+      end || Guest.can?(:read_project, project)
     end
 
     protected