Refactor `Gitlab::GitAccess`

1. Don't use case statements for dispatch anymore. This leads to a lot
   of duplication, and makes the logic harder to follow.

2. Remove duplicated logic.

    - For example, the `can_push_to_branch?` exists, but we also have a
      different way of checking the same condition within `change_access_check`.

    - This kind of duplication is removed, and the `can_push_to_branch?`
      method is used in both places.

3. Move checks returning true/false to `UserAccess`.

    - All public methods in `GitAccess` now return an instance of
      `GitAccessStatus`. Previously, some methods would return
      true/false as well, which was confusing.

    - It makes sense for these kinds of checks to be at the level of a
      user, so the `UserAccess` class was repurposed for this. The prior
      `UserAccess.allowed?` classmethod is converted into an instance
      method.

    - All external uses of these checks have been migrated to use the
      `UserAccess` class

4. Move the "change_access_check" into a separate class.

    - Create the `GitAccess::ChangeAccessCheck` class to run these
      checks, which are quite substantial.

    - `ChangeAccessCheck` returns an instance of `GitAccessStatus` as
      well.

5. Break out the boolean logic in `ChangeAccessCheck` into `if/else`
   chains - this seems more readable.

6. I can understand that this might look like overkill for !4892, but I
   think this is a good opportunity to clean it up.

    - http://martinfowler.com/bliki/OpportunisticRefactoring.html

1 files changed, 45 insertions, 3 deletions

diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb
index d1b42c1f9b9..c0f85e9b3a8 100644
--- a/lib/gitlab/user_access.rb
+++ b/lib/gitlab/user_access.rb
@@ -1,7 +1,23 @@
 module Gitlab
-  module UserAccess
-    def self.allowed?(user)
-      return false if user.blocked?
+  class UserAccess
+    attr_reader :user, :project
+
+    def initialize(user, project: nil)
+      @user = user
+      @project = project
+    end
+
+    def can_do_action?(action)
+      @permission_cache ||= {}
+      @permission_cache[action] ||= user.can?(action, project)
+    end
+
+    def cannot_do_action?(action)
+      !can_do_action?(action)
+    end
+
+    def allowed?
+      return false if user.blank? || user.blocked?
 
       if user.requires_ldap_check? && user.try_obtain_ldap_lease
         return false unless Gitlab::LDAP::Access.allowed?(user)
@@ -9,5 +25,31 @@ module Gitlab
 
       true
     end
+
+    def can_push_to_branch?(ref)
+      return false unless user
+
+      if project.protected_branch?(ref) && !project.developers_can_push_to_protected_branch?(ref)
+        user.can?(:push_code_to_protected_branches, project)
+      else
+        user.can?(:push_code, project)
+      end
+    end
+
+    def can_merge_to_branch?(ref)
+      return false unless user
+
+      if project.protected_branch?(ref) && !project.developers_can_merge_to_protected_branch?(ref)
+        user.can?(:push_code_to_protected_branches, project)
+      else
+        user.can?(:push_code, project)
+      end
+    end
+
+    def can_read_project?
+      return false unless user
+
+      user.can?(:read_project, project)
+    end
   end
 end