Internalize private project minimum access level

Some feature allows GUEST to access only if project is not private.
This method returns access level when targeting private projects.

3 files changed, 37 insertions, 12 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index d16078b4fe0..83f362d2c9a 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -498,13 +498,11 @@ class Project < ApplicationRecord
 
   # This scope returns projects where user has access to both the project and the feature.
   def self.filter_by_feature_visibility(feature, user)
-    scope = with_feature_available_for_user(feature, user)
-
-    if ProjectFeature.guest_allowed_on_private_project?(feature)
-      scope.public_or_visible_to_user(user)
-    else
-      scope.public_or_visible_to_user(user, Gitlab::Access::REPORTER)
-    end
+    with_feature_available_for_user(feature, user)
+      .public_or_visible_to_user(
+        user,
+        ProjectFeature.required_minimum_access_level_for_private_project(feature)
+      )
   end
 
   scope :active, -> { joins(:issues, :notes, :merge_requests).order('issues.created_at, notes.created_at, merge_requests.created_at DESC') }
diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb
index 564e531c320..caa65d32c86 100644
--- a/app/models/project_feature.rb
+++ b/app/models/project_feature.rb
@@ -24,7 +24,7 @@ class ProjectFeature < ApplicationRecord
 
   FEATURES = %i(issues merge_requests wiki snippets builds repository pages).freeze
   PRIVATE_FEATURES_MIN_ACCESS_LEVEL = { merge_requests: Gitlab::Access::REPORTER }.freeze
-  FEATURES_ALLOWED_BY_GUEST_ON_PRIVATE_PROJECT = %i(issues wiki).freeze
+  PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT = { repository: Gitlab::Access::REPORTER }.freeze
   STRING_OPTIONS = HashWithIndifferentAccess.new({
     'disabled' => DISABLED,
     'private'  => PRIVATE,
@@ -46,16 +46,19 @@ class ProjectFeature < ApplicationRecord
       "#{table}.#{attribute}"
     end
 
-    def guest_allowed_on_private_project?(feature)
+    def required_minimum_access_level(feature)
       feature = ensure_feature!(feature)
 
-      FEATURES_ALLOWED_BY_GUEST_ON_PRIVATE_PROJECT.include?(feature)
+      PRIVATE_FEATURES_MIN_ACCESS_LEVEL.fetch(feature, Gitlab::Access::GUEST)
     end
 
-    def required_minimum_access_level(feature)
+    # Guest users can perform certain features on public and internal projects, but not private projects.
+    def required_minimum_access_level_for_private_project(feature)
       feature = ensure_feature!(feature)
 
-      PRIVATE_FEATURES_MIN_ACCESS_LEVEL.fetch(feature, Gitlab::Access::GUEST)
+      PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT.fetch(feature) do
+        required_minimum_access_level(feature)
+      end
     end
 
     def access_level_from_str(level)
diff --git a/spec/models/project_feature_spec.rb b/spec/models/project_feature_spec.rb
index 5b448c343a0..9ce1b8fd895 100644
--- a/spec/models/project_feature_spec.rb
+++ b/spec/models/project_feature_spec.rb
@@ -6,6 +6,16 @@ describe ProjectFeature do
   let(:project) { create(:project) }
   let(:user) { create(:user) }
 
+  describe 'PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT' do
+    it 'has higher level than that of PRIVATE_FEATURES_MIN_ACCESS_LEVEL' do
+      described_class::PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT.each do |feature, level|
+        if generic_level = described_class::PRIVATE_FEATURES_MIN_ACCESS_LEVEL[feature]
+          expect(level).to be >= generic_level
+        end
+      end
+    end
+  end
+
   describe '.quoted_access_level_column' do
     it 'returns the table name and quoted column name for a feature' do
       expected = '"project_features"."issues_access_level"'
@@ -246,10 +256,24 @@ describe ProjectFeature do
       expect(described_class.required_minimum_access_level('merge_requests')).to eq(Gitlab::Access::REPORTER)
     end
 
+    it 'handles repository' do
+      expect(described_class.required_minimum_access_level(:repository)).to eq(Gitlab::Access::GUEST)
+    end
+
     it 'raises error if feature is invalid' do
       expect do
         described_class.required_minimum_access_level(:foos)
       end.to raise_error
     end
   end
+
+  describe '.required_minimum_access_level_for_private_project' do
+    it 'returns higher permission for repository' do
+      expect(described_class.required_minimum_access_level_for_private_project(:repository)).to eq(Gitlab::Access::REPORTER)
+    end
+
+    it 'returns normal permission for issues' do
+      expect(described_class.required_minimum_access_level_for_private_project(:issues)).to eq(Gitlab::Access::GUEST)
+    end
+  end
 end