From 2b73aaa15ad9f651f51f8c71de461da6664a4fbb Mon Sep 17 00:00:00 2001
From: Ali Ibrahim <aliibrahim@gmail.com>
Date: Sun, 14 Aug 2016 13:49:08 -0400
Subject: Allow to add deploy keys with write-access

---
 lib/gitlab/git_access.rb | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

(limited to 'lib')

diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 1882eb8d050..f208df3cdce 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -50,10 +50,13 @@ module Gitlab
     end
 
     def push_access_check(changes)
+      unless project.repository.exists?
+        return build_status_object(false, "A repository for this project does not exist yet.")
+      end
       if user
         user_push_access_check(changes)
       elsif deploy_key
-        build_status_object(false, "Deploy keys are not allowed to push code.")
+        deploy_key_push_access_check(changes)
       else
         raise 'Wrong actor'
       end
@@ -72,10 +75,6 @@ module Gitlab
         return build_status_object(true)
       end
 
-      unless project.repository.exists?
-        return build_status_object(false, "A repository for this project does not exist yet.")
-      end
-
       changes_list = Gitlab::ChangesList.new(changes)
 
       # Iterate over all changes to find if user allowed all of them to be applied
@@ -90,6 +89,14 @@ module Gitlab
       build_status_object(true)
     end
 
+    def deploy_key_push_access_check(changes)
+      if actor.can_push?
+        build_status_object(true)
+      else
+        build_status_object(false, "The deploy key does not have write access to the project.")
+      end
+    end
+
     def change_access_check(change)
       Checks::ChangeAccess.new(change, user_access: user_access, project: project).exec
     end
-- 
cgit v1.2.1


From fcc2c43ebb32c0e9a3ae636e66460b42cbae4d53 Mon Sep 17 00:00:00 2001
From: Ali Ibrahim <aliibrahim@gmail.com>
Date: Thu, 18 Aug 2016 03:27:45 -0400
Subject: Added can_push attribute to deploy keys and update docs for API

---
 lib/api/entities.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'lib')

diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 055716ab1e3..5df65a2327d 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -234,7 +234,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
-- 
cgit v1.2.1


From 2811a213a6199978ca1f3eea002a7a77f8b87554 Mon Sep 17 00:00:00 2001
From: Ali Ibrahim <aliibrahim@gmail.com>
Date: Fri, 19 Aug 2016 14:40:45 -0400
Subject: added spacing

---
 lib/gitlab/git_access.rb | 1 +
 1 file changed, 1 insertion(+)

(limited to 'lib')

diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index f208df3cdce..f7432df5557 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -53,6 +53,7 @@ module Gitlab
       unless project.repository.exists?
         return build_status_object(false, "A repository for this project does not exist yet.")
       end
+
       if user
         user_push_access_check(changes)
       elsif deploy_key
-- 
cgit v1.2.1


From 05cc87052a7755da3d352409ef3ab024921593c4 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 11 Nov 2016 20:40:28 +0800
Subject: Improve write access check for deploy key

---
 lib/gitlab/git_access.rb | 81 +++++++++++++++++++++---------------------------
 1 file changed, 36 insertions(+), 45 deletions(-)

(limited to 'lib')

diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 64b5c4b98dc..819e0657bdd 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -7,7 +7,7 @@ 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: 'This deploy key does not have write access to this project.',
       no_repo: 'A repository for this project does not exist yet.'
     }
 
@@ -46,22 +46,18 @@ module Gitlab
     def download_access_check
       if user
         user_download_access_check
-      elsif deploy_key.nil? && !Guest.can?(:download_code, project)
+      elsif !Guest.can?(:download_code, project)
         raise UnauthorizedError, ERROR_MESSAGES[:download]
       end
     end
 
     def push_access_check(changes)
-      unless project.repository.exists?
-        return build_status_object(false, "A repository for this project does not exist yet.")
-      end
-
-      if user
-        user_push_access_check(changes)
-      elsif deploy_key
+      if deploy_key
         deploy_key_push_access_check(changes)
+      elsif user
+        user_push_access_check(changes)
       else
-        raise UnauthorizedError, ERROR_MESSAGES[deploy_key ? :deploy_key : :upload]
+        raise UnauthorizedError, ERROR_MESSAGES[:upload]
       end
     end
 
@@ -88,34 +84,19 @@ module Gitlab
         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
+      check_repository_existence!
+      check_change_access!(changes)
     end
 
     def deploy_key_push_access_check(changes)
-      if actor.can_push?
-        build_status_object(true)
+      if deploy_key.can_push?
+        check_repository_existence!
+        check_change_access!(changes)
       else
-        build_status_object(false, "The deploy key does not have write access to the project.")
+        raise UnauthorizedError, ERROR_MESSAGES[:deploy_key]
       end
     end
 
-    def change_access_check(change)
-      Checks::ChangeAccess.new(change, user_access: user_access, project: project).exec
-    end
-
     def protocol_allowed?
       Gitlab::ProtocolAccess.allowed?(protocol)
     end
@@ -146,6 +127,27 @@ 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 = Checks::ChangeAccess.new(change,
+                                          user_access: user_access,
+                                          project: project).exec
+        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 matching_merge_request?(newrev, branch_name)
       Checks::MatchingMergeRequest.new(newrev, branch_name, project).match?
     end
@@ -154,20 +156,11 @@ module Gitlab
       actor if actor.is_a?(DeployKey)
     end
 
-    def deploy_key_can_read_project?
-      if deploy_key
-        return true if project.public?
-        deploy_key.projects.include?(project)
-      else
-        false
-      end
-    end
-
     def can_read_project?
-      if user
+      if deploy_key
+        project.public? || deploy_key.projects.include?(project)
+      elsif user
         user_access.can_read_project?
-      elsif deploy_key
-        deploy_key_can_read_project?
       else
         Guest.can?(:read_project, project)
       end
@@ -182,8 +175,6 @@ module Gitlab
         case actor
         when User
           actor
-        when DeployKey
-          nil
         when Key
           actor.user
         end
-- 
cgit v1.2.1


From 38fbcb99dba61cfae1b788e0ec947911c4d14dd8 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 11 Nov 2016 21:24:21 +0800
Subject: So deploy key might not have a corresponding user

---
 lib/gitlab/git_access.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'lib')

diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 819e0657bdd..78f562821ea 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -46,7 +46,7 @@ module Gitlab
     def download_access_check
       if user
         user_download_access_check
-      elsif !Guest.can?(:download_code, project)
+      elsif deploy_key.nil? && !Guest.can?(:download_code, project)
         raise UnauthorizedError, ERROR_MESSAGES[:download]
       end
     end
@@ -91,7 +91,7 @@ module Gitlab
     def deploy_key_push_access_check(changes)
       if deploy_key.can_push?
         check_repository_existence!
-        check_change_access!(changes)
+        check_change_access!(changes) if user
       else
         raise UnauthorizedError, ERROR_MESSAGES[:deploy_key]
       end
-- 
cgit v1.2.1


From 71ae01fefe62caf396640affb8ca618fe68db5a0 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 11 Nov 2016 21:44:33 +0800
Subject: Add more tests and fix write to project check

---
 lib/gitlab/git_access.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'lib')

diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 78f562821ea..96979398c83 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -89,7 +89,7 @@ module Gitlab
     end
 
     def deploy_key_push_access_check(changes)
-      if deploy_key.can_push?
+      if deploy_key.can_push_to?(project)
         check_repository_existence!
         check_change_access!(changes) if user
       else
-- 
cgit v1.2.1


From 5da9bfa453268474b3bff13c63e55b29bbcdf016 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 11 Nov 2016 22:26:05 +0800
Subject: Fix test for GitAccessWiki, it's overriding change_access_check

---
 lib/gitlab/git_access.rb      | 9 ++++++---
 lib/gitlab/git_access_wiki.rb | 2 +-
 2 files changed, 7 insertions(+), 4 deletions(-)

(limited to 'lib')

diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 96979398c83..b462cffeaf6 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -138,9 +138,7 @@ module Gitlab
 
       # Iterate over all changes to find if user allowed all of them to be applied
       changes_list.each do |change|
-        status = Checks::ChangeAccess.new(change,
-                                          user_access: user_access,
-                                          project: project).exec
+        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
@@ -148,6 +146,11 @@ module Gitlab
       end
     end
 
+    def check_single_change_access(change)
+      Checks::ChangeAccess.new(
+        change, user_access: user_access, project: project).exec
+    end
+
     def matching_merge_request?(newrev, branch_name)
       Checks::MatchingMergeRequest.new(newrev, branch_name, project).match?
     end
diff --git a/lib/gitlab/git_access_wiki.rb b/lib/gitlab/git_access_wiki.rb
index f71d3575909..f7976a64ef5 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 change_access_check(change)
+    def check_single_change_access(change)
       if user_access.can_do_action?(:create_wiki)
         build_status_object(true)
       else
-- 
cgit v1.2.1


From 721478123068c6718ec73c72a7b7d32c00c816df Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 16 Nov 2016 19:48:54 +0800
Subject: Also need to check against push rules:

Feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7383#note_18440615
---
 lib/gitlab/git_access.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'lib')

diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index b462cffeaf6..19bdfc878b1 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -91,7 +91,7 @@ module Gitlab
     def deploy_key_push_access_check(changes)
       if deploy_key.can_push_to?(project)
         check_repository_existence!
-        check_change_access!(changes) if user
+        check_change_access!(changes)
       else
         raise UnauthorizedError, ERROR_MESSAGES[:deploy_key]
       end
-- 
cgit v1.2.1


From a9765fb47fbbd1e1070434fc06cc76b25a42caa6 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 16 Nov 2016 20:31:08 +0800
Subject: Introduce has_access_to? so that we could reuse it

Feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7383#note_18439108
---
 lib/gitlab/auth/result.rb | 3 +--
 lib/gitlab/git_access.rb  | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

(limited to 'lib')

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/git_access.rb b/lib/gitlab/git_access.rb
index 19bdfc878b1..a7ad944e79e 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -161,7 +161,7 @@ module Gitlab
 
     def can_read_project?
       if deploy_key
-        project.public? || deploy_key.projects.include?(project)
+        project.public? || deploy_key.has_access_to?(project)
       elsif user
         user_access.can_read_project?
       else
-- 
cgit v1.2.1


From 48090a9188e13e3ddaffb5957a7b5a264024f060 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 16 Nov 2016 22:07:04 +0800
Subject: Introduce no_user_or_blocked? and fix tests due to

checking user permission.
---
 lib/gitlab/user_access.rb | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

(limited to 'lib')

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
-- 
cgit v1.2.1


From 8c1a01e05fd3c6e1621242aaf31a0ce2789ad546 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 18 Nov 2016 03:48:23 +0800
Subject: We never check user privilege if it's a deploy key

---
 lib/gitlab/checks/change_access.rb | 11 +++++++++--
 lib/gitlab/git_access.rb           | 29 +++++++++++++++++++----------
 2 files changed, 28 insertions(+), 12 deletions(-)

(limited to 'lib')

diff --git a/lib/gitlab/checks/change_access.rb b/lib/gitlab/checks/change_access.rb
index cb1065223d4..6b6a86ffde9 100644
--- a/lib/gitlab/checks/change_access.rb
+++ b/lib/gitlab/checks/change_access.rb
@@ -1,13 +1,15 @@
 module Gitlab
   module Checks
     class ChangeAccess
-      attr_reader :user_access, :project
+      attr_reader :user_access, :project, :skip_authorization
 
-      def initialize(change, user_access:, project:)
+      def initialize(
+        change, user_access:, project:, 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
+        @skip_authorization = skip_authorization
       end
 
       def exec
@@ -23,6 +25,7 @@ module Gitlab
       protected
 
       def protected_branch_checks
+        return if skip_authorization
         return unless @branch_name
         return unless project.protected_branch?(@branch_name)
 
@@ -48,6 +51,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)
@@ -56,6 +61,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 a7ad944e79e..6be0ab08a1f 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -27,7 +27,7 @@ module Gitlab
 
     def check(cmd, changes)
       check_protocol!
-      check_active_user!
+      check_active_user! unless deploy_key?
       check_project_accessibility!
       check_command_existence!(cmd)
 
@@ -44,9 +44,13 @@ module Gitlab
     end
 
     def download_access_check
-      if user
+      if deploy_key
+        true
+      elsif user
         user_download_access_check
-      elsif deploy_key.nil? && !Guest.can?(:download_code, project)
+      elsif Guest.can?(:download_code, project)
+        true
+      else
         raise UnauthorizedError, ERROR_MESSAGES[:download]
       end
     end
@@ -148,7 +152,10 @@ module Gitlab
 
     def check_single_change_access(change)
       Checks::ChangeAccess.new(
-        change, user_access: user_access, project: project).exec
+        change,
+        user_access: user_access,
+        project: project,
+        skip_authorization: deploy_key?).exec
     end
 
     def matching_merge_request?(newrev, branch_name)
@@ -156,17 +163,19 @@ module Gitlab
     end
 
     def deploy_key
-      actor if actor.is_a?(DeployKey)
+      actor if deploy_key?
+    end
+
+    def deploy_key?
+      actor.is_a?(DeployKey)
     end
 
     def can_read_project?
       if deploy_key
-        project.public? || deploy_key.has_access_to?(project)
+        deploy_key.has_access_to?(project)
       elsif user
-        user_access.can_read_project?
-      else
-        Guest.can?(:read_project, project)
-      end
+        user.can?(:read_project, project)
+      end || Guest.can?(:read_project, project)
     end
 
     protected
-- 
cgit v1.2.1


From 0c532dbb243bf9bb5bf77248ce87a2a0e4478421 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 18 Nov 2016 04:08:24 +0800
Subject: Check if the key could really download, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7383#note_18518792
---
 lib/gitlab/git_access.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'lib')

diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 6be0ab08a1f..d5690f870e9 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -45,7 +45,7 @@ module Gitlab
 
     def download_access_check
       if deploy_key
-        true
+        deploy_key.has_access_to?(project)
       elsif user
         user_download_access_check
       elsif Guest.can?(:download_code, project)
-- 
cgit v1.2.1


From e72e2f9ba0a160960f68035fbbdbe3f0f86b0dba Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 18 Nov 2016 04:11:04 +0800
Subject: Still grant :download_code if guest could do that

Feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7383#note_18518792
---
 lib/gitlab/git_access.rb | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

(limited to 'lib')

diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index d5690f870e9..3f674532488 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -48,11 +48,9 @@ module Gitlab
         deploy_key.has_access_to?(project)
       elsif user
         user_download_access_check
-      elsif Guest.can?(:download_code, project)
-        true
-      else
-        raise UnauthorizedError, ERROR_MESSAGES[:download]
-      end
+      end ||
+        Guest.can?(:download_code, project) ||
+        raise(UnauthorizedError, ERROR_MESSAGES[:download])
     end
 
     def push_access_check(changes)
-- 
cgit v1.2.1


From 8dbea582497bbc45735cf145a3da4c88c9e0e78d Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 18 Nov 2016 17:28:05 +0800
Subject: Check download privilege more specifically and

add another error message for the new error.
---
 lib/gitlab/git_access.rb | 58 ++++++++++++++++++++++++------------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

(limited to 'lib')

diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 3f674532488..b87ca316240 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -7,7 +7,10 @@ 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: 'This deploy key does not have write access to this project.',
+      deploy_key_upload:
+        'This deploy key does not have write access to this project.',
+      deploy_key:
+        'This deploy key does not have access to this project.',
       no_repo: 'A repository for this project does not exist yet.'
     }
 
@@ -44,29 +47,36 @@ module Gitlab
     end
 
     def download_access_check
-      if deploy_key
-        deploy_key.has_access_to?(project)
-      elsif user
-        user_download_access_check
-      end ||
-        Guest.can?(:download_code, project) ||
-        raise(UnauthorizedError, ERROR_MESSAGES[:download])
+      passed = if deploy_key
+                 deploy_key.has_access_to?(project)
+               elsif user
+                 user_can_download_code? || build_can_download_code?
+               end || Guest.can?(:download_code, project)
+
+      unless passed
+        message = if deploy_key
+                    ERROR_MESSAGES[:deploy_key]
+                  else
+                    ERROR_MESSAGES[:download]
+                  end
+
+        raise UnauthorizedError, message
+      end
     end
 
     def push_access_check(changes)
       if deploy_key
-        deploy_key_push_access_check(changes)
+        deploy_key_push_access_check
       elsif user
-        user_push_access_check(changes)
+        user_push_access_check
       else
         raise UnauthorizedError, ERROR_MESSAGES[:upload]
       end
-    end
 
-    def user_download_access_check
-      unless user_can_download_code? || build_can_download_code?
-        raise UnauthorizedError, ERROR_MESSAGES[:download]
-      end
+      return if changes.blank? # Allow access.
+
+      check_repository_existence!
+      check_change_access!(changes)
     end
 
     def user_can_download_code?
@@ -77,25 +87,15 @@ 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
-
-      check_repository_existence!
-      check_change_access!(changes)
     end
 
-    def deploy_key_push_access_check(changes)
-      if deploy_key.can_push_to?(project)
-        check_repository_existence!
-        check_change_access!(changes)
-      else
-        raise UnauthorizedError, ERROR_MESSAGES[:deploy_key]
+    def deploy_key_push_access_check
+      unless deploy_key.can_push_to?(project)
+        raise UnauthorizedError, ERROR_MESSAGES[:deploy_key_upload]
       end
     end
 
-- 
cgit v1.2.1


From 1b150576d9de9096796f76e1635d27ddaa2b1dd5 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 6 Dec 2016 21:09:23 +0800
Subject: Prefer guest_can_downlod_code?

---
 lib/gitlab/git_access.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'lib')

diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 9563fa7cafb..d42879e38cd 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -51,7 +51,7 @@ module Gitlab
                  deploy_key.has_access_to?(project)
                elsif user
                  user_can_download_code? || build_can_download_code?
-               end || Guest.can?(:download_code, project)
+               end || guest_can_downlod_code?
 
       unless passed
         message = if deploy_key
-- 
cgit v1.2.1


From 46d7c1d2b3cf87023cb6bcf40dd53aa79606d203 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 6 Dec 2016 21:10:27 +0800
Subject: Prefer guest_can_download_code? and fix typo

---
 lib/gitlab/git_access.rb      | 4 ++--
 lib/gitlab/git_access_wiki.rb | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

(limited to 'lib')

diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index d42879e38cd..412f42c6320 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -51,7 +51,7 @@ module Gitlab
                  deploy_key.has_access_to?(project)
                elsif user
                  user_can_download_code? || build_can_download_code?
-               end || guest_can_downlod_code?
+               end || guest_can_download_code?
 
       unless passed
         message = if deploy_key
@@ -79,7 +79,7 @@ module Gitlab
       check_change_access!(changes)
     end
 
-    def guest_can_downlod_code?
+    def guest_can_download_code?
       Guest.can?(:download_code, project)
     end
 
diff --git a/lib/gitlab/git_access_wiki.rb b/lib/gitlab/git_access_wiki.rb
index 74171f4f90e..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
 
-- 
cgit v1.2.1


From 57e3e942de1adef2c8621905370f07d7da7870c4 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Sat, 10 Dec 2016 01:45:13 +0800
Subject: Don't pass the actor for deploy key, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7383#note_19579483
---
 lib/gitlab/git_access.rb | 2 ++
 1 file changed, 2 insertions(+)

(limited to 'lib')

diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 412f42c6320..d483038e8e9 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -189,6 +189,8 @@ module Gitlab
         case actor
         when User
           actor
+        when DeployKey
+          nil
         when Key
           actor.user
         end
-- 
cgit v1.2.1


From 8ac50d78eb921989d100a91aad9eb6e59cbf43dc Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Sat, 10 Dec 2016 02:04:36 +0800
Subject: Check project existence for push too, and

we don't have to check for deploy key for downloading
because deploy key could certainly download when it
could already read the project.

Feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7383#note_19578626
---
 lib/gitlab/git_access.rb | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

(limited to 'lib')

diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index d483038e8e9..13efc1ed73d 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -9,8 +9,6 @@ module Gitlab
       download: 'You are not allowed to download code from this project.',
       deploy_key_upload:
         'This deploy key does not have write access to this project.',
-      deploy_key:
-        'This deploy key does not have access to this project.',
       no_repo: 'A repository for this project does not exist yet.'
     }
 
@@ -33,10 +31,11 @@ module Gitlab
       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
@@ -47,20 +46,12 @@ module Gitlab
     end
 
     def download_access_check
-      passed = if deploy_key
-                 deploy_key.has_access_to?(project)
-               elsif user
-                 user_can_download_code? || build_can_download_code?
-               end || guest_can_download_code?
+      passed = user_can_download_code? ||
+               build_can_download_code? ||
+               guest_can_download_code?
 
       unless passed
-        message = if deploy_key
-                    ERROR_MESSAGES[:deploy_key]
-                  else
-                    ERROR_MESSAGES[:download]
-                  end
-
-        raise UnauthorizedError, message
+        raise UnauthorizedError, ERROR_MESSAGES[:download]
       end
     end
 
@@ -75,7 +66,6 @@ module Gitlab
 
       return if changes.blank? # Allow access.
 
-      check_repository_existence!
       check_change_access!(changes)
     end
 
-- 
cgit v1.2.1


From 884f57c9102416805427d773eb21e09fd30c2452 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 20 Dec 2016 21:19:07 +0800
Subject: Use consistent names and move checks to the method,

and move those checks to be private. Feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7383#note_20285012

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7383#note_20285279
---
 lib/gitlab/git_access.rb | 82 +++++++++++++++++++++++++-----------------------
 1 file changed, 43 insertions(+), 39 deletions(-)

(limited to 'lib')

diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 545506f3dfd..f0b241fb5e6 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -29,16 +29,16 @@ module Gitlab
 
     def check(cmd, changes)
       check_protocol!
-      check_active_user! unless deploy_key?
+      check_active_user!
       check_project_accessibility!
       check_command_existence!(cmd)
       check_repository_existence!
 
       case cmd
       when *DOWNLOAD_COMMANDS
-        download_access_check unless deploy_key?
+        check_download_access!
       when *PUSH_COMMANDS
-        push_access_check(changes)
+        check_push_access!(changes)
       end
 
       build_status_object(true)
@@ -46,30 +46,6 @@ module Gitlab
       build_status_object(false, ex.message)
     end
 
-    def download_access_check
-      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 deploy_key
-        deploy_key_push_access_check
-      elsif user
-        user_push_access_check
-      else
-        raise UnauthorizedError, ERROR_MESSAGES[:upload]
-      end
-
-      return if changes.blank? # Allow access.
-
-      check_change_access!(changes)
-    end
-
     def guest_can_download_code?
       Guest.can?(:download_code, project)
     end
@@ -82,18 +58,6 @@ module Gitlab
       authentication_abilities.include?(:build_download_code) && user_access.can_do_action?(:build_download_code)
     end
 
-    def user_push_access_check
-      unless authentication_abilities.include?(:push_code)
-        raise UnauthorizedError, ERROR_MESSAGES[:upload]
-      end
-    end
-
-    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?
       Gitlab::ProtocolAccess.allowed?(protocol)
     end
@@ -107,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
@@ -130,6 +96,44 @@ module Gitlab
       end
     end
 
+    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 check_push_access!(changes)
+      if deploy_key
+        check_deploy_key_push_access!
+      elsif user
+        check_user_push_access!
+      else
+        raise UnauthorizedError, ERROR_MESSAGES[:upload]
+      end
+
+      return if changes.blank? # Allow access.
+
+      check_change_access!(changes)
+    end
+
+    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)
 
-- 
cgit v1.2.1


From c1d11bf57c3091aa4695e302e21c39b9ec723f54 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 20 Dec 2016 23:30:01 +0800
Subject: Rubocop prefers to indent this way

---
 lib/gitlab/git_access.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'lib')

diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index f0b241fb5e6..7e1484613f2 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -100,8 +100,8 @@ module Gitlab
       return if deploy_key?
 
       passed = user_can_download_code? ||
-               build_can_download_code? ||
-               guest_can_download_code?
+        build_can_download_code? ||
+        guest_can_download_code?
 
       unless passed
         raise UnauthorizedError, ERROR_MESSAGES[:download]
-- 
cgit v1.2.1