Merge branch 'bvl-external-auth-port' into 'master'

Port `read_cross_project` ability from EE

See merge request gitlab-org/gitlab-ce!17208

5 files changed, 47 insertions, 3 deletions

diff --git a/app/models/ability.rb b/app/models/ability.rb
index 0b6bcbde5d9..6dae49f38dc 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -22,12 +22,30 @@ class Ability
     #
     # issues - The issues to reduce down to those readable by the user.
     # user - The User for which to check the issues
-    def issues_readable_by_user(issues, user = nil)
+    # filters - A hash of abilities and filters to apply if the user lacks this
+    #           ability
+    def issues_readable_by_user(issues, user = nil, filters: {})
+      issues = apply_filters_if_needed(issues, user, filters)
+
       DeclarativePolicy.user_scope do
         issues.select { |issue| issue.visible_to_user?(user) }
       end
     end
 
+    # Returns an Array of MergeRequests that can be read by the given user.
+    #
+    # merge_requests - MRs out of which to collect mr's readable by the user.
+    # user - The User for which to check the merge_requests
+    # filters - A hash of abilities and filters to apply if the user lacks this
+    #           ability
+    def merge_requests_readable_by_user(merge_requests, user = nil, filters: {})
+      merge_requests = apply_filters_if_needed(merge_requests, user, filters)
+
+      DeclarativePolicy.user_scope do
+        merge_requests.select { |mr| allowed?(user, :read_merge_request, mr) }
+      end
+    end
+
     def can_edit_note?(user, note)
       allowed?(user, :edit_note, note)
     end
@@ -53,5 +71,15 @@ class Ability
       cache = RequestStore.active? ? RequestStore : {}
       DeclarativePolicy.policy_for(user, subject, cache: cache)
     end
+
+    private
+
+    def apply_filters_if_needed(elements, user, filters)
+      filters.each do |ability, filter|
+        elements = filter.call(elements) unless allowed?(user, ability)
+      end
+
+      elements
+    end
   end
 end
diff --git a/app/models/concerns/protected_ref_access.rb b/app/models/concerns/protected_ref_access.rb
index 80c9f7d4eb4..bfda5b1678b 100644
--- a/app/models/concerns/protected_ref_access.rb
+++ b/app/models/concerns/protected_ref_access.rb
@@ -35,6 +35,7 @@ module ProtectedRefAccess
   def check_access(user)
     return true if user.admin?
 
-    project.team.max_member_access(user.id) >= access_level
+    user.can?(:push_code, project) &&
+      project.team.max_member_access(user.id) >= access_level
   end
 end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 93628b456f2..c81f7e52bb1 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -159,7 +159,18 @@ class Issue < ActiveRecord::Base
       object.all_references(current_user, extractor: ext)
     end
 
-    ext.merge_requests.sort_by(&:iid)
+    merge_requests = ext.merge_requests.sort_by(&:iid)
+
+    cross_project_filter = -> (merge_requests) do
+      merge_requests.select { |mr| mr.target_project == project }
+    end
+
+    Ability.merge_requests_readable_by_user(
+      merge_requests, current_user,
+      filters: {
+        read_cross_project: cross_project_filter
+      }
+    )
   end
 
   # All branches containing the current issue's ID, except for
diff --git a/app/models/notification_recipient.rb b/app/models/notification_recipient.rb
index 472b348a545..fd70e920c7e 100644
--- a/app/models/notification_recipient.rb
+++ b/app/models/notification_recipient.rb
@@ -85,6 +85,7 @@ class NotificationRecipient
       return false unless user.can?(:receive_notifications)
       return true if @skip_read_ability
 
+      return false if @target && !user.can?(:read_cross_project)
       return false if @project && !user.can?(:read_project, @project)
 
       return true unless read_ability
diff --git a/app/models/project.rb b/app/models/project.rb
index 4ad6f025e5c..ba278a49688 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1037,6 +1037,9 @@ class Project < ActiveRecord::Base
   end
 
   def user_can_push_to_empty_repo?(user)
+    return false unless empty_repo?
+    return false unless Ability.allowed?(user, :push_code, self)
+
     !ProtectedBranch.default_branch_protected? || team.max_member_access(user.id) > Gitlab::Access::DEVELOPER
   end