Refactor to let GitAccess errors bubble up

No external behavior change.

This allows `GitHttpController` to set the HTTP status based on the type of error. Alternatively, we could have added an attribute to GitAccessStatus, but this pattern seemed appropriate.

1 files changed, 15 insertions, 17 deletions

diff --git a/lib/gitlab/checks/change_access.rb b/lib/gitlab/checks/change_access.rb
index 4b6f8abd61d..e9782623be5 100644
--- a/lib/gitlab/checks/change_access.rb
+++ b/lib/gitlab/checks/change_access.rb
@@ -33,20 +33,18 @@ module Gitlab
       def exec
         return GitAccessStatus.new(true) if skip_authorization
 
-        error = push_checks || branch_checks || tag_checks
+        push_checks
+        branch_checks
+        tag_checks
 
-        if error
-          GitAccessStatus.new(false, error)
-        else
-          GitAccessStatus.new(true)
-        end
+        GitAccessStatus.new(true)
       end
 
       protected
 
       def push_checks
         if user_access.cannot_do_action?(:push_code)
-          ERROR_MESSAGES[:push_code]
+          raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:push_code]
         end
       end
 
@@ -54,7 +52,7 @@ module Gitlab
         return unless @branch_name
 
         if deletion? && @branch_name == project.default_branch
-          return ERROR_MESSAGES[:delete_default_branch]
+          raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:delete_default_branch]
         end
 
         protected_branch_checks
@@ -64,7 +62,7 @@ module Gitlab
         return unless ProtectedBranch.protected?(project, @branch_name)
 
         if forced_push?
-          return ERROR_MESSAGES[:force_push_protected_branch]
+          raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:force_push_protected_branch]
         end
 
         if deletion?
@@ -76,22 +74,22 @@ module Gitlab
 
       def protected_branch_deletion_checks
         unless user_access.can_delete_branch?(@branch_name)
-          return ERROR_MESSAGES[:non_master_delete_protected_branch]
+          raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:non_master_delete_protected_branch]
         end
 
         unless protocol == 'web'
-          ERROR_MESSAGES[:non_web_delete_protected_branch]
+          raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:non_web_delete_protected_branch]
         end
       end
 
       def protected_branch_push_checks
         if matching_merge_request?
           unless user_access.can_merge_to_branch?(@branch_name) || user_access.can_push_to_branch?(@branch_name)
-            ERROR_MESSAGES[:merge_protected_branch]
+            raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:merge_protected_branch]
           end
         else
           unless user_access.can_push_to_branch?(@branch_name)
-            ERROR_MESSAGES[:push_protected_branch]
+            raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:push_protected_branch]
           end
         end
       end
@@ -100,7 +98,7 @@ module Gitlab
         return unless @tag_name
 
         if tag_exists? && user_access.cannot_do_action?(:admin_project)
-          return ERROR_MESSAGES[:change_existing_tags]
+          raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:change_existing_tags]
         end
 
         protected_tag_checks
@@ -109,11 +107,11 @@ module Gitlab
       def protected_tag_checks
         return unless ProtectedTag.protected?(project, @tag_name)
 
-        return ERROR_MESSAGES[:update_protected_tag] if update?
-        return ERROR_MESSAGES[:delete_protected_tag] if deletion?
+        raise(GitAccess::UnauthorizedError, ERROR_MESSAGES[:update_protected_tag]) if update?
+        raise(GitAccess::UnauthorizedError, ERROR_MESSAGES[:delete_protected_tag]) if deletion?
 
         unless user_access.can_create_tag?(@tag_name)
-          return ERROR_MESSAGES[:create_protected_tag]
+          raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:create_protected_tag]
         end
       end