Merge branch '25934-project-snippet-vis' into 'security-9-2'

Fix visibility when referencing snippets

See merge request !2101

app/policies/project_snippet_policy.rb

 class ProjectSnippetPolicy < BasePolicy
   def rules
+    # We have to check both project feature visibility and a snippet visibility and take the stricter one
+    # This will be simplified - check https://gitlab.com/gitlab-org/gitlab-ce/issues/27573
+    return unless @subject.project.feature_available?(:snippets, @user)
+    return unless Ability.allowed?(@user, :read_project, @subject.project)
+
     can! :read_project_snippet if @subject.public?
     return unless @user

lib/banzai/reference_parser/base_parser.rb

@@ -62,7 +62,7 @@ module Banzai
         nodes.select do |node|
           if node.has_attribute?(project_attr)
-            can_read_reference?(user, projects[node])
+            can_read_reference?(user, projects[node], node)
           else
             true
           end
@@ -231,7 +231,7 @@ module Banzai
       # see reference comments.
       # Override this method on subclasses
       # to check if user can read resource
-      def can_read_reference?(user, ref_project)
+      def can_read_reference?(user, ref_project, node)
         raise NotImplementedError
       end

lib/banzai/reference_parser/commit_parser.rb

@@ -32,7 +32,7 @@ module Banzai
       private
-      def can_read_reference?(user, ref_project)
+      def can_read_reference?(user, ref_project, node)
         can?(user, :download_code, ref_project)
       end
     end

lib/banzai/reference_parser/commit_range_parser.rb

@@ -36,7 +36,7 @@ module Banzai
       private
-      def can_read_reference?(user, ref_project)
+      def can_read_reference?(user, ref_project, node)
         can?(user, :download_code, ref_project)
       end
     end

lib/banzai/reference_parser/external_issue_parser.rb

@@ -23,7 +23,7 @@ module Banzai
       private
-      def can_read_reference?(user, ref_project)
+      def can_read_reference?(user, ref_project, node)
         can?(user, :read_issue, ref_project)
       end
     end

lib/banzai/reference_parser/label_parser.rb

@@ -9,7 +9,7 @@ module Banzai
       private
-      def can_read_reference?(user, ref_project)
+      def can_read_reference?(user, ref_project, node)
         can?(user, :read_label, ref_project)
       end
     end

lib/banzai/reference_parser/merge_request_parser.rb

@@ -40,6 +40,10 @@ module Banzai
           self.class.data_attribute
         )
       end
+
+      def can_read_reference?(user, ref_project, node)
+        can?(user, :read_merge_request, ref_project)
+      end
     end
   end
 end

lib/banzai/reference_parser/milestone_parser.rb

@@ -9,7 +9,7 @@ module Banzai
       private
-      def can_read_reference?(user, ref_project)
+      def can_read_reference?(user, ref_project, node)
         can?(user, :read_milestone, ref_project)
       end
     end

lib/banzai/reference_parser/snippet_parser.rb

@@ -9,8 +9,8 @@ module Banzai
       private
-      def can_read_reference?(user, ref_project)
-        can?(user, :read_project_snippet, ref_project)
+      def can_read_reference?(user, ref_project, node)
+        can?(user, :read_project_snippet, referenced_by([node]).first)
       end
     end
   end

lib/banzai/reference_parser/user_parser.rb

@@ -103,7 +103,7 @@ module Banzai
           flat_map { |p| p.team.members.to_a }
       end
-      def can_read_reference?(user, ref_project)
+      def can_read_reference?(user, ref_project, node)
         can?(user, :read_project, ref_project)
       end
     end

spec/lib/banzai/reference_parser/base_parser_spec.rb

@@ -30,7 +30,7 @@ describe Banzai::ReferenceParser::BaseParser, lib: true do
       it 'checks if user can read the resource' do
         link['data-project'] = project.id.to_s
-        expect(subject).to receive(:can_read_reference?).with(user, project)
+        expect(subject).to receive(:can_read_reference?).with(user, project, link)
         subject.nodes_visible_to_user(user, [link])
       end

spec/lib/banzai/reference_parser/snippet_parser_spec.rb

@@ -4,20 +4,199 @@ describe Banzai::ReferenceParser::SnippetParser, lib: true do
   include ReferenceParserHelpers
   let(:project) { create(:empty_project, :public) }
+
   let(:user) { create(:user) }
-  let(:snippet) { create(:snippet, project: project) }
+  let(:external_user) { create(:user, :external) }
+  let(:project_member) { create(:user) }
+
   subject { described_class.new(project, user) }
   let(:link) { empty_html_link }
+  def visible_references(snippet_visibility, user = nil)
+    snippet = create(:project_snippet, snippet_visibility, project: project)
+    link['data-project'] = project.id.to_s
+    link['data-snippet'] = snippet.id.to_s
+
+    subject.nodes_visible_to_user(user, [link])
+  end
+
+  before do
+    project.add_user(project_member, :developer)
+  end
+
   describe '#nodes_visible_to_user' do
-    context 'when the link has a data-issue attribute' do
-      before { link['data-snippet'] = snippet.id.to_s }
-      it_behaves_like "referenced feature visibility", "snippets"
+    context 'when a project is public and the snippets feature is enabled for everyone' do
+      before do
+        project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::ENABLED)
+      end
+
+      it 'creates a reference for guest for a public snippet' do
+        expect(visible_references(:public)).to eq([link])
+      end
+
+      it 'creates a reference for a regular user for a public snippet' do
+        expect(visible_references(:public, user)).to eq([link])
+      end
+
+      it 'creates a reference for a regular user for an internal snippet' do
+        expect(visible_references(:internal, user)).to eq([link])
+      end
+
+      it 'does not create a reference for an external user for an internal snippet' do
+        expect(visible_references(:internal, external_user)).to be_empty
+      end
+
+      it 'creates a reference for a project member for a private snippet' do
+        expect(visible_references(:private, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for a regular user for a private snippet' do
+        expect(visible_references(:private, user)).to be_empty
+      end
+    end
+
+    context 'when a project is public and the snippets feature is enabled for project team members' do
+      before do
+        project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::PRIVATE)
+      end
+
+      it 'creates a reference for a project member for a public snippet' do
+        expect(visible_references(:public, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for guest for a public snippet' do
+        expect(visible_references(:public, nil)).to be_empty
+      end
+
+      it 'does not create a reference for a regular user for a public snippet' do
+        expect(visible_references(:public, user)).to be_empty
+      end
+
+      it 'creates a reference for a project member for an internal snippet' do
+        expect(visible_references(:internal, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for a regular user for an internal snippet' do
+        expect(visible_references(:internal, user)).to be_empty
+      end
+
+      it 'creates a reference for a project member for a private snippet' do
+        expect(visible_references(:private, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for a regular user for a private snippet' do
+        expect(visible_references(:private, user)).to be_empty
+      end
+    end
+
+    context 'when a project is internal and the snippets feature is enabled for everyone' do
+      before do
+        project.update_attribute(:visibility, Gitlab::VisibilityLevel::INTERNAL)
+        project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::ENABLED)
+      end
+
+      it 'does not create a reference for guest for a public snippet' do
+        expect(visible_references(:public)).to be_empty
+      end
+
+      it 'does not create a reference for an external user for a public snippet' do
+        expect(visible_references(:public, external_user)).to be_empty
+      end
+      it 'creates a reference for a regular user for a public snippet' do
+        expect(visible_references(:public, user)).to eq([link])
+      end
+
+      it 'creates a reference for a regular user for an internal snippet' do
+        expect(visible_references(:internal, user)).to eq([link])
+      end
+
+      it 'does not create a reference for an external user for an internal snippet' do
+        expect(visible_references(:internal, external_user)).to be_empty
+      end
+
+      it 'creates a reference for a project member for a private snippet' do
+        expect(visible_references(:private, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for a regular user for a private snippet' do
+        expect(visible_references(:private, user)).to be_empty
+      end
+    end
+
+    context 'when a project is internal and the snippets feature is enabled for project team members' do
+      before do
+        project.update_attribute(:visibility, Gitlab::VisibilityLevel::INTERNAL)
+        project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::PRIVATE)
+      end
+
+      it 'creates a reference for a project member for a public snippet' do
+        expect(visible_references(:public, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for guest for a public snippet' do
+        expect(visible_references(:public, nil)).to be_empty
+      end
+
+      it 'does not create reference for a regular user for a public snippet' do
+        expect(visible_references(:public, user)).to be_empty
+      end
+
+      it 'creates a reference for a project member for an internal snippet' do
+        expect(visible_references(:internal, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for a regular user for an internal snippet' do
+        expect(visible_references(:internal, user)).to be_empty
+      end
+
+      it 'creates a reference for a project member for a private snippet' do
+        expect(visible_references(:private, project_member)).to eq([link])
+      end
+
+      it 'does not create reference for a regular user for a private snippet' do
+        expect(visible_references(:private, user)).to be_empty
+      end
+    end
+
+    context 'when a project is private and the snippets feature is enabled for project team members' do
+      before do
+        project.update_attribute(:visibility, Gitlab::VisibilityLevel::PRIVATE)
+        project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::PRIVATE)
+      end
+
+      it 'creates a reference for a project member for a public snippet' do
+        expect(visible_references(:public, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for guest for a public snippet' do
+        expect(visible_references(:public, nil)).to be_empty
+      end
+
+      it 'does not create a reference for a regular user for a public snippet' do
+        expect(visible_references(:public, user)).to be_empty
+      end
+
+      it 'creates a reference for a project member for an internal snippet' do
+        expect(visible_references(:internal, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for a regular user for an internal snippet' do
+        expect(visible_references(:internal, user)).to be_empty
+      end
+
+      it 'creates a reference for a project member for a private snippet' do
+        expect(visible_references(:private, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for a regular user for a private snippet' do
+        expect(visible_references(:private, user)).to be_empty
+      end
     end
   end
   describe '#referenced_by' do
+    let(:snippet) { create(:snippet, project: project) }
     describe 'when the link has a data-snippet attribute' do
       context 'using an existing snippet ID' do
         it 'returns an Array of snippets' do
@@ -31,7 +210,7 @@ describe Banzai::ReferenceParser::SnippetParser, lib: true do
         it 'returns an empty Array' do
           link['data-snippet'] = ''
-          expect(subject.referenced_by([link])).to eq([])
+          expect(subject.referenced_by([link])).to be_empty
         end
       end
     end

spec/policies/project_snippet_policy_spec.rb

@@ -3,7 +3,7 @@ require 'spec_helper'
 describe ProjectSnippetPolicy, models: true do
   let(:regular_user) { create(:user) }
   let(:external_user) { create(:user, :external) }
-  let(:project) { create(:empty_project) }
+  let(:project) { create(:empty_project, :public) }
   let(:author_permissions) do
     [
@@ -107,7 +107,7 @@ describe ProjectSnippetPolicy, models: true do
     end
     context 'snippet author' do
-      let(:snippet) { create(:project_snippet, :private, author: regular_user) }
+      let(:snippet) { create(:project_snippet, :private, author: regular_user, project: project) }
       subject { described_class.abilities(regular_user, snippet).to_set }