Merge branch 'feature/1376-allow-write-access-deploy-keys' into 'master'

Allow to add deploy keys with write-access

Closes #1376

See merge request !7383

6 files changed, 107 insertions, 81 deletions

diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 9f15c08f472..d2fadf6a3d0 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -317,7 +317,7 @@ module API
     end
 
     class SSHKey < Grape::Entity
-      expose :id, :title, :key, :created_at
+      expose :id, :title, :key, :created_at, :can_push
     end
 
     class SSHKeyWithUser < SSHKey
diff --git a/lib/gitlab/auth/result.rb b/lib/gitlab/auth/result.rb
index 6be7f690676..39b86c61a18 100644
--- a/lib/gitlab/auth/result.rb
+++ b/lib/gitlab/auth/result.rb
@@ -9,8 +9,7 @@ module Gitlab
 
       def lfs_deploy_token?(for_project)
         type == :lfs_deploy_token &&
-          actor &&
-          actor.projects.include?(for_project)
+          actor.try(:has_access_to?, for_project)
       end
 
       def success?
diff --git a/lib/gitlab/checks/change_access.rb b/lib/gitlab/checks/change_access.rb
index 3d203017d9f..9c391fa92a3 100644
--- a/lib/gitlab/checks/change_access.rb
+++ b/lib/gitlab/checks/change_access.rb
@@ -1,14 +1,16 @@
 module Gitlab
   module Checks
     class ChangeAccess
-      attr_reader :user_access, :project
+      attr_reader :user_access, :project, :skip_authorization
 
-      def initialize(change, user_access:, project:, env: {})
+      def initialize(
+        change, user_access:, project:, env: {}, skip_authorization: false)
         @oldrev, @newrev, @ref = change.values_at(:oldrev, :newrev, :ref)
         @branch_name = Gitlab::Git.branch_name(@ref)
         @user_access = user_access
         @project = project
         @env = env
+        @skip_authorization = skip_authorization
       end
 
       def exec
@@ -24,6 +26,7 @@ module Gitlab
       protected
 
       def protected_branch_checks
+        return if skip_authorization
         return unless @branch_name
         return unless project.protected_branch?(@branch_name)
 
@@ -49,6 +52,8 @@ module Gitlab
       end
 
       def tag_checks
+        return if skip_authorization
+
         tag_ref = Gitlab::Git.tag_name(@ref)
 
         if tag_ref && protected_tag?(tag_ref) && user_access.cannot_do_action?(:admin_project)
@@ -57,6 +62,8 @@ module Gitlab
       end
 
       def push_checks
+        return if skip_authorization
+
         if user_access.cannot_do_action?(:push_code)
           "You are not allowed to push code to this project."
         end
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index c6b6efda360..7e1484613f2 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.'
     }
 
@@ -31,12 +32,13 @@ module Gitlab
       check_active_user!
       check_project_accessibility!
       check_command_existence!(cmd)
+      check_repository_existence!
 
       case cmd
       when *DOWNLOAD_COMMANDS
-        download_access_check
+        check_download_access!
       when *PUSH_COMMANDS
-        push_access_check(changes)
+        check_push_access!(changes)
       end
 
       build_status_object(true)
@@ -44,32 +46,10 @@ module Gitlab
       build_status_object(false, ex.message)
     end
 
-    def download_access_check
-      if user
-        user_download_access_check
-      elsif deploy_key.nil? && !guest_can_downlod_code?
-        raise UnauthorizedError, ERROR_MESSAGES[:download]
-      end
-    end
-
-    def push_access_check(changes)
-      if user
-        user_push_access_check(changes)
-      else
-        raise UnauthorizedError, ERROR_MESSAGES[deploy_key ? :deploy_key : :upload]
-      end
-    end
-
-    def guest_can_downlod_code?
+    def guest_can_download_code?
       Guest.can?(:download_code, project)
     end
 
-    def user_download_access_check
-      unless user_can_download_code? || build_can_download_code?
-        raise UnauthorizedError, ERROR_MESSAGES[:download]
-      end
-    end
-
     def user_can_download_code?
       authentication_abilities.include?(:download_code) && user_access.can_do_action?(:download_code)
     end
@@ -78,35 +58,6 @@ module Gitlab
       authentication_abilities.include?(:build_download_code) && user_access.can_do_action?(:build_download_code)
     end
 
-    def user_push_access_check(changes)
-      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
-    end
-
     def protocol_allowed?
       Gitlab::ProtocolAccess.allowed?(protocol)
     end
@@ -120,6 +71,8 @@ module Gitlab
     end
 
     def check_active_user!
+      return if deploy_key?
+
       if user && !user_access.allowed?
         raise UnauthorizedError, "Your account has been blocked."
       end
@@ -137,33 +90,92 @@ module Gitlab
       end
     end
 
-    def matching_merge_request?(newrev, branch_name)
-      Checks::MatchingMergeRequest.new(newrev, branch_name, project).match?
+    def check_repository_existence!
+      unless project.repository.exists?
+        raise UnauthorizedError, ERROR_MESSAGES[:no_repo]
+      end
     end
 
-    def deploy_key
-      actor if actor.is_a?(DeployKey)
+    def check_download_access!
+      return if deploy_key?
+
+      passed = user_can_download_code? ||
+        build_can_download_code? ||
+        guest_can_download_code?
+
+      unless passed
+        raise UnauthorizedError, ERROR_MESSAGES[:download]
+      end
     end
 
-    def deploy_key_can_read_project?
+    def check_push_access!(changes)
       if deploy_key
-        return true if project.public?
-        deploy_key.projects.include?(project)
+        check_deploy_key_push_access!
+      elsif user
+        check_user_push_access!
       else
-        false
+        raise UnauthorizedError, ERROR_MESSAGES[:upload]
       end
+
+      return if changes.blank? # Allow access.
+
+      check_change_access!(changes)
     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)
+    def check_user_push_access!
+      unless authentication_abilities.include?(:push_code)
+        raise UnauthorizedError, ERROR_MESSAGES[:upload]
       end
     end
 
+    def check_deploy_key_push_access!
+      unless deploy_key.can_push_to?(project)
+        raise UnauthorizedError, ERROR_MESSAGES[:deploy_key_upload]
+      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 deploy_key?
+    end
+
+    def deploy_key?
+      actor.is_a?(DeployKey)
+    end
+
+    def can_read_project?
+      if deploy_key
+        deploy_key.has_access_to?(project)
+      elsif user
+        user.can?(:read_project, project)
+      end || Guest.can?(:read_project, project)
+    end
+
     protected
 
     def user
diff --git a/lib/gitlab/git_access_wiki.rb b/lib/gitlab/git_access_wiki.rb
index 2c06c4ff1ef..67eaa5e088d 100644
--- a/lib/gitlab/git_access_wiki.rb
+++ b/lib/gitlab/git_access_wiki.rb
@@ -1,6 +1,6 @@
 module Gitlab
   class GitAccessWiki < GitAccess
-    def guest_can_downlod_code?
+    def guest_can_download_code?
       Guest.can?(:download_wiki_code, project)
     end
 
@@ -8,7 +8,7 @@ module Gitlab
       authentication_abilities.include?(:download_code) && user_access.can_do_action?(:download_wiki_code)
     end
 
-    def change_access_check(change)
+    def check_single_change_access(change)
       if user_access.can_do_action?(:create_wiki)
         build_status_object(true)
       else
diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb
index 9858d2e7d83..6c7e673fb9f 100644
--- a/lib/gitlab/user_access.rb
+++ b/lib/gitlab/user_access.rb
@@ -8,6 +8,8 @@ module Gitlab
     end
 
     def can_do_action?(action)
+      return false if no_user_or_blocked?
+
       @permission_cache ||= {}
       @permission_cache[action] ||= user.can?(action, project)
     end
@@ -17,7 +19,7 @@ module Gitlab
     end
 
     def allowed?
-      return false if user.blank? || user.blocked?
+      return false if no_user_or_blocked?
 
       if user.requires_ldap_check? && user.try_obtain_ldap_lease
         return false unless Gitlab::LDAP::Access.allowed?(user)
@@ -27,7 +29,7 @@ module Gitlab
     end
 
     def can_push_to_branch?(ref)
-      return false unless user
+      return false if no_user_or_blocked?
 
       if project.protected_branch?(ref)
         return true if project.empty_repo? && project.user_can_push_to_empty_repo?(user)
@@ -40,7 +42,7 @@ module Gitlab
     end
 
     def can_merge_to_branch?(ref)
-      return false unless user
+      return false if no_user_or_blocked?
 
       if project.protected_branch?(ref)
         access_levels = project.protected_branches.matching(ref).map(&:merge_access_levels).flatten
@@ -51,9 +53,15 @@ module Gitlab
     end
 
     def can_read_project?
-      return false unless user
+      return false if no_user_or_blocked?
 
       user.can?(:read_project, project)
     end
+
+    private
+
+    def no_user_or_blocked?
+      user.nil? || user.blocked?
+    end
   end
 end