Add User#full_private_access? to check if user has Private accesstc-refactor-projects-finder-init-collection

In CE only the admin has access to all private groups & projects. In EE also an
auditor can have full private access.

To overcome merge conflicts, or accidental incorrect access rights, abstract
this out in `User#full_private_access?`.

`User#admin?` now only should be used for admin-only features. For private
access-related features `User#full_private_access?` should be used.

Backported from gitlab-org/gitlab-ee!2199

7 files changed, 30 insertions, 6 deletions

diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb
index b4c074bc69c..3da5508aefd 100644
--- a/app/finders/issues_finder.rb
+++ b/app/finders/issues_finder.rb
@@ -41,7 +41,7 @@ class IssuesFinder < IssuableFinder
   def self.not_restricted_by_confidentiality(user)
     return Issue.where('issues.confidential IS NOT TRUE') if user.blank?
 
-    return Issue.all if user.admin?
+    return Issue.all if user.full_private_access?
 
     Issue.where('
       issues.confidential IS NOT TRUE
diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb
index dde2a11440d..48edd0738ee 100644
--- a/app/models/project_feature.rb
+++ b/app/models/project_feature.rb
@@ -90,7 +90,7 @@ class ProjectFeature < ActiveRecord::Base
     when DISABLED
       false
     when PRIVATE
-      user && (project.team.member?(user) || user.admin?)
+      user && (project.team.member?(user) || user.full_private_access?)
     when ENABLED
       true
     else
diff --git a/app/models/user.rb b/app/models/user.rb
index 954a30155f7..9971e43146a 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -984,6 +984,12 @@ class User < ActiveRecord::Base
     self.admin = (new_level == 'admin')
   end
 
+  # Does the user have access to all private groups & projects?
+  # Overridden in EE to also check auditor?
+  def full_private_access?
+    admin?
+  end
+
   def update_two_factor_requirement
     periods = expanded_groups_requiring_two_factor_authentication.pluck(:two_factor_grace_period)
 
diff --git a/changelogs/unreleased/tc-refactor-projects-finder-init-collection.yml b/changelogs/unreleased/tc-refactor-projects-finder-init-collection.yml
new file mode 100644
index 00000000000..7bcbd6468c7
--- /dev/null
+++ b/changelogs/unreleased/tc-refactor-projects-finder-init-collection.yml
@@ -0,0 +1,4 @@
+---
+title: Add User#full_private_access? to check if user has access to all private groups & projects
+merge_request: 12373
+author:
diff --git a/lib/gitlab/visibility_level.rb b/lib/gitlab/visibility_level.rb
index 36e5b5041a6..48f3d950779 100644
--- a/lib/gitlab/visibility_level.rb
+++ b/lib/gitlab/visibility_level.rb
@@ -28,7 +28,7 @@ module Gitlab
       def levels_for_user(user = nil)
         return [PUBLIC] unless user
 
-        if user.admin?
+        if user.full_private_access?
           [PRIVATE, INTERNAL, PUBLIC]
         elsif user.external?
           [PUBLIC]
diff --git a/spec/lib/gitlab/visibility_level_spec.rb b/spec/lib/gitlab/visibility_level_spec.rb
index 84d2484cc8a..db9d2807be6 100644
--- a/spec/lib/gitlab/visibility_level_spec.rb
+++ b/spec/lib/gitlab/visibility_level_spec.rb
@@ -21,7 +21,7 @@ describe Gitlab::VisibilityLevel, lib: true do
 
   describe '.levels_for_user' do
     it 'returns all levels for an admin' do
-      user = double(:user, admin?: true)
+      user = build(:user, :admin)
 
       expect(described_class.levels_for_user(user))
         .to eq([Gitlab::VisibilityLevel::PRIVATE,
@@ -30,7 +30,7 @@ describe Gitlab::VisibilityLevel, lib: true do
     end
 
     it 'returns INTERNAL and PUBLIC for internal users' do
-      user = double(:user, admin?: false, external?: false)
+      user = build(:user)
 
       expect(described_class.levels_for_user(user))
         .to eq([Gitlab::VisibilityLevel::INTERNAL,
@@ -38,7 +38,7 @@ describe Gitlab::VisibilityLevel, lib: true do
     end
 
     it 'returns PUBLIC for external users' do
-      user = double(:user, admin?: false, external?: true)
+      user = build(:user, :external)
 
       expect(described_class.levels_for_user(user))
         .to eq([Gitlab::VisibilityLevel::PUBLIC])
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 314f8781867..8e895ec6634 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1733,6 +1733,20 @@ describe User, models: true do
     end
   end
 
+  describe '#full_private_access?' do
+    it 'returns false for regular user' do
+      user = build(:user)
+
+      expect(user.full_private_access?).to be_falsy
+    end
+
+    it 'returns true for admin user' do
+      user = build(:user, :admin)
+
+      expect(user.full_private_access?).to be_truthy
+    end
+  end
+
   describe '.ghost' do
     it "creates a ghost user if one isn't already present" do
       ghost = User.ghost