Refactor user authorization check for a single project to avoid querying all user projects

Currently, even when searching for all authorized issues of *one* project, we run the
`Users#authorized_projects` query (which can be rather slow). This update checks if
we are handling issues of just one project and does the authorization check locally.
It does have the downside of basically repeating the logic of `Users#authorized_projects`
on `Project#authorized_for_user`.

3 files changed, 82 insertions, 0 deletions

diff --git a/app/models/issue.rb b/app/models/issue.rb
index 60abd47409e..60af8c15340 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -52,10 +52,50 @@ class Issue < ActiveRecord::Base
     attributes
   end
 
+  class << self
+    private
+
+    # Returns the project that the current scope belongs to if any, nil otherwise.
+    #
+    # Examples:
+    # - my_project.issues.without_due_date.owner_project => my_project
+    # - Issue.all.owner_project => nil
+    def owner_project
+      # No owner if we're not being called from an association
+      return unless all.respond_to?(:proxy_association)
+
+      owner = all.proxy_association.owner
+
+      # Check if the association is or belongs to a project
+      if owner.is_a?(Project)
+        owner
+      else
+        begin
+          owner.association(:project).target
+        rescue ActiveRecord::AssociationNotFoundError
+          nil
+        end
+      end
+    end
+  end
+
   def self.visible_to_user(user)
     return where('issues.confidential IS NULL OR issues.confidential IS FALSE') if user.blank?
     return all if user.admin?
 
+    # Check if we are scoped to a specific project's issues
+    if owner_project
+      if owner_project.authorized_for_user?(user, Gitlab::Access::REPORTER)
+        # If the project is authorized for the user, they can see all issues in the project
+        return all
+      else
+        # else only non confidential and authored/assigned to them
+        return where('issues.confidential IS NULL OR issues.confidential IS FALSE
+          OR issues.author_id = :user_id OR issues.assignee_id = :user_id',
+          user_id: user.id)
+      end
+    end
+
     where('
       issues.confidential IS NULL
       OR issues.confidential IS FALSE
diff --git a/app/models/project.rb b/app/models/project.rb
index c96de0f6c72..d6191e80e62 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1210,4 +1210,44 @@ class Project < ActiveRecord::Base
       { key: variable.key, value: variable.value, public: false }
     end
   end
+
+  # Checks if `user` is authorized for this project, with at least the
+  # `min_access_level` (if given).
+  #
+  # If you change the logic of this method, please also update `User#authorized_projects`
+  def authorized_for_user?(user, min_access_level = nil)
+    return false unless user
+
+    return true if personal? && namespace_id == user.namespace_id
+
+    authorized_for_user_by_group?(user, min_access_level) ||
+      authorized_for_user_by_members?(user, min_access_level) ||
+      authorized_for_user_by_shared_projects?(user, min_access_level)
+  end
+
+  private
+
+  def authorized_for_user_by_group?(user, min_access_level)
+    member = user.group_members.find_by(source_id: group)
+
+    member && (!min_access_level || member.access_level >= min_access_level)
+  end
+
+  def authorized_for_user_by_members?(user, min_access_level)
+    member = members.find_by(user_id: user)
+
+    member && (!min_access_level || member.access_level >= min_access_level)
+  end
+
+  def authorized_for_user_by_shared_projects?(user, min_access_level)
+    shared_projects = user.group_members.joins(group: :shared_projects).
+      where(project_group_links: { project_id: self })
+
+    if min_access_level
+      members_scope = { access_level: Gitlab::Access.values.select { |access| access >= min_access_level } }
+      shared_projects = shared_projects.where(members: members_scope)
+    end
+
+    shared_projects.any?
+  end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index 975e935fa20..8dc10becd69 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -412,6 +412,8 @@ class User < ActiveRecord::Base
   end
 
   # Returns projects user is authorized to access.
+  #
+  # If you change the logic of this method, please also update `Project#authorized_for_user`
   def authorized_projects(min_access_level = nil)
     Project.where("projects.id IN (#{projects_union(min_access_level).to_sql})")
   end