Don't run single change checks when changes are unknown

When the `changes` passed to `GitAccess` are the literal string `_any`,
which indicates that this is a pre-authorization check, we now check
whether the user can push to any branch in the project in question,
instead of running the per-change check with `oldrev` `_any`, `newrev`
`nil`, and `ref` `nil`.

4 files changed, 33 insertions, 17 deletions

diff --git a/lib/gitlab/checks/push_check.rb b/lib/gitlab/checks/push_check.rb
index f3a52f09868..91f8d0bdbc8 100644
--- a/lib/gitlab/checks/push_check.rb
+++ b/lib/gitlab/checks/push_check.rb
@@ -6,7 +6,7 @@ module Gitlab
       def validate!
         logger.log_timed("Checking if you are allowed to push...") do
           unless can_push?
-            raise GitAccess::UnauthorizedError, 'You are not allowed to push code to this project.'
+            raise GitAccess::UnauthorizedError, GitAccess::ERROR_MESSAGES[:push_code]
           end
         end
       end
@@ -15,7 +15,7 @@ module Gitlab
 
       def can_push?
         user_access.can_do_action?(:push_code) ||
-          user_access.can_push_to_branch?(branch_name)
+          project.branch_allows_collaboration?(user_access.user, branch_name)
       end
     end
   end
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 04a9a2ff7dc..679296e031c 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -28,7 +28,8 @@ module Gitlab
       upload_pack_disabled_over_http: 'Pulling over HTTP is not allowed.',
       receive_pack_disabled_over_http: 'Pushing over HTTP is not allowed.',
       read_only: 'The repository is temporarily read-only. Please try again later.',
-      cannot_push_to_read_only: "You can't push code to a read-only GitLab instance."
+      cannot_push_to_read_only: "You can't push code to a read-only GitLab instance.",
+      push_code: 'You are not allowed to push code to this project.'
     }.freeze
 
     INTERNAL_TIMEOUT = 50.seconds.freeze
@@ -269,18 +270,27 @@ module Gitlab
       # Deploy keys with write access can push anything
       return if deploy_key?
 
-      # If there are worktrees with a HEAD pointing to a non-existent object,
-      # calls to `git rev-list --all` will fail in git 2.15+. This should also
-      # clear stale lock files.
-      project.repository.clean_stale_repository_files
+      if changes == ANY
+        can_push = user_access.can_do_action?(:push_code) ||
+          project.any_branch_allows_collaboration?(user_access.user)
 
-      # Iterate over all changes to find if user allowed all of them to be applied
-      changes_list.each.with_index do |change, index|
-        first_change = index == 0
-
-        # If user does not have access to make at least one change, cancel all
-        # push by allowing the exception to bubble up
-        check_single_change_access(change, skip_lfs_integrity_check: !first_change)
+        unless can_push
+          raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:push_code]
+        end
+      else
+        # If there are worktrees with a HEAD pointing to a non-existent object,
+        # calls to `git rev-list --all` will fail in git 2.15+. This should also
+        # clear stale lock files.
+        project.repository.clean_stale_repository_files
+
+        # Iterate over all changes to find if user allowed all of them to be applied
+        changes_list.each.with_index do |change, index|
+          first_change = index == 0
+
+          # If user does not have access to make at least one change, cancel all
+          # push by allowing the exception to bubble up
+          check_single_change_access(change, skip_lfs_integrity_check: !first_change)
+        end
       end
     end
 
@@ -354,7 +364,7 @@ module Gitlab
     protected
 
     def changes_list
-      @changes_list ||= Gitlab::ChangesList.new(changes)
+      @changes_list ||= Gitlab::ChangesList.new(changes == ANY ? [] : changes)
     end
 
     def user
diff --git a/lib/gitlab/git_access_wiki.rb b/lib/gitlab/git_access_wiki.rb
index 3f24001e4ee..0af91957fa8 100644
--- a/lib/gitlab/git_access_wiki.rb
+++ b/lib/gitlab/git_access_wiki.rb
@@ -15,7 +15,7 @@ module Gitlab
       authentication_abilities.include?(:download_code) && user_access.can_do_action?(:download_wiki_code)
     end
 
-    def check_single_change_access(change, _options = {})
+    def check_change_access!
       unless user_access.can_do_action?(:create_wiki)
         raise UnauthorizedError, ERROR_MESSAGES[:write_to_wiki]
       end
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index 68ecaf58071..3e34dd592f2 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -736,7 +736,8 @@ describe Gitlab::GitAccess do
     end
 
     let(:changes) do
-      { push_new_branch: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/heads/wow",
+      { any: Gitlab::GitAccess::ANY,
+        push_new_branch: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/heads/wow",
         push_master: '6f6d7e7ed 570e7b2ab refs/heads/master',
         push_protected_branch: '6f6d7e7ed 570e7b2ab refs/heads/feature',
         push_remove_protected_branch: "570e7b2ab #{Gitlab::Git::BLANK_SHA} "\
@@ -798,6 +799,7 @@ describe Gitlab::GitAccess do
 
     permissions_matrix = {
       admin: {
+        any: true,
         push_new_branch: true,
         push_master: true,
         push_protected_branch: true,
@@ -809,6 +811,7 @@ describe Gitlab::GitAccess do
       },
 
       maintainer: {
+        any: true,
         push_new_branch: true,
         push_master: true,
         push_protected_branch: true,
@@ -820,6 +823,7 @@ describe Gitlab::GitAccess do
       },
 
       developer: {
+        any: true,
         push_new_branch: true,
         push_master: true,
         push_protected_branch: false,
@@ -831,6 +835,7 @@ describe Gitlab::GitAccess do
       },
 
       reporter: {
+        any: false,
         push_new_branch: false,
         push_master: false,
         push_protected_branch: false,
@@ -842,6 +847,7 @@ describe Gitlab::GitAccess do
       },
 
       guest: {
+        any: false,
         push_new_branch: false,
         push_master: false,
         push_protected_branch: false,