Merge branch '9102-hide-dismissed-be' into 'master'

Add scope filter to security dashboards

See merge request gitlab-org/gitlab!16692

ee/app/controllers/concerns/vulnerability_findings_actions.rb

@@ -35,8 +35,7 @@ module VulnerabilityFindingsActions
   private
 
   def filter_params
-    params.permit(report_type: [], confidence: [], project_id: [], severity: [])
-      .merge(hide_dismissed: ::Gitlab::Utils.to_boolean(params[:hide_dismissed]))
+    params.permit(:scope, report_type: [], confidence: [], project_id: [], severity: [])
   end
 
   def vulnerability_findings(collection = :latest)

ee/app/finders/security/vulnerability_findings_finder.rb

@@ -11,6 +11,7 @@
 #     confidence: Array<String>
 #     project: Array<String>
 #     report_type: Array<String>
+#     scope: String
 
 module Security
   class VulnerabilityFindingsFinder
@@ -22,12 +23,13 @@ module Security
       @params = params
     end
 
-    def execute(scope = :latest)
-      collection = init_collection(scope)
+    def execute(collection_scope = :latest)
+      collection = init_collection(collection_scope)
       collection = by_report_type(collection)
       collection = by_project(collection)
       collection = by_severity(collection)
       collection = by_confidence(collection)
+      collection = by_scope(collection) unless Feature.disabled?(:hide_dismissed_vulnerabilities)
       collection
     end
 
@@ -63,8 +65,16 @@ module Security
           *params[:confidence]).compact)
     end
 
-    def init_collection(scope)
-      case scope
+    def by_scope(items)
+      # We're using the same params as the public Vulnerabilities API because the frontend uses both
+      # https://gitlab.com/gitlab-org/gitlab/issues/33468 fixes issue with param name meaning
+      return items if params[:scope] == 'all'
+
+      items.undismissed
+    end
+
+    def init_collection(collection_scope)
+      case collection_scope
       when :all
         vulnerable.all_vulnerabilities
       when :with_sha
@@ -72,7 +82,7 @@ module Security
       when :latest
         vulnerable.latest_vulnerabilities
       else
-        raise ArgumentError, "invalid value for 'scope': #{scope}"
+        raise ArgumentError, "invalid value for 'collection_scope': #{collection_scope}"
       end
     end
   end

ee/changelogs/unreleased/9102-hide-dismissed-be.yml

+---
+title: Add filter for dismissed vulnerabilities on security dashboards.
+merge_request: 16692
+author:
+type: added

ee/spec/finders/security/vulnerability_findings_finder_spec.rb

@@ -81,7 +81,14 @@ describe Security::VulnerabilityFindingsFinder do
 
     context 'by all filters' do
       context 'with found entity' do
-        let(:params) { { severity: %w[high medium low], project_id: [project1.id, project2.id], report_type: %w[sast dast] } }
+        let(:params) { { severity: %w[high medium low], project_id: [project1.id, project2.id], report_type: %w[sast dast], scope: 'all' } }
+
+        before do
+          create(:vulnerability_feedback, :sast, :dismissal,
+             pipeline: pipeline1,
+             project: project1,
+             project_fingerprint: finding1.project_fingerprint)
+        end
 
         it 'filters by all params' do
           is_expected.to contain_exactly(finding1, finding3, finding4)
@@ -97,6 +104,45 @@ describe Security::VulnerabilityFindingsFinder do
       end
     end
 
+    context 'by scope' do
+      let!(:dismissal) do
+        create(:vulnerability_feedback, :sast, :dismissal,
+               pipeline: pipeline1,
+               project: project1,
+               project_fingerprint: finding1.project_fingerprint)
+      end
+      let!(:issue) do
+        create(:vulnerability_feedback, :sast, :issue,
+               pipeline: pipeline1,
+               project: project1,
+               project_fingerprint: finding4.project_fingerprint)
+      end
+
+      context 'when all' do
+        let(:params) { { scope: 'all' } }
+
+        it 'includes all vulnerabilities' do
+          is_expected.to contain_exactly(finding1, finding2, finding3, finding4)
+        end
+      end
+
+      context 'when not specified' do
+        let(:params) { {} }
+
+        it 'excludes dismissed vulnerabilities' do
+          is_expected.to contain_exactly(finding2, finding3, finding4)
+        end
+      end
+
+      context 'when dismissed' do
+        let(:params) { { scope: 'dismissed' } }
+
+        it 'excludes dismissed vulnerabilities' do
+          is_expected.to contain_exactly(finding2, finding3, finding4)
+        end
+      end
+    end
+
     context 'by some filters' do
       context 'with found entity' do
         let(:params) { { project_id: [project2.id], severity: %w[medium low] } }
@@ -132,10 +178,10 @@ describe Security::VulnerabilityFindingsFinder do
         end
       end
 
-      context 'with an invalid scope specifier' do
+      context 'with an invalid collection scope specifier' do
         it 'raises error' do
           expect { described_class.new(group).execute(:invalid) }.to(
-            raise_error(ArgumentError, "invalid value for 'scope': invalid")
+            raise_error(ArgumentError, "invalid value for 'collection_scope': invalid")
           )
         end
       end

ee/spec/models/vulnerabilities/occurrence_spec.rb

@@ -281,12 +281,26 @@ describe Vulnerabilities::Occurrence do
   end
 
   describe '.undismissed' do
-    it 'returns occurrences that do not have a corresponding dismissal feedback' do
-      undismissed_occurrence = create(:vulnerabilities_occurrence)
-      dismissed_occurrence = create(:vulnerabilities_occurrence)
-      create(:vulnerability_feedback, project_fingerprint: dismissed_occurrence.project_fingerprint)
+    set(:project) { create(:project) }
+    set(:project2) { create(:project) }
+    let!(:finding1) { create(:vulnerabilities_occurrence, project: project) }
+    let!(:finding2) { create(:vulnerabilities_occurrence, project: project) }
+    let!(:finding3) { create(:vulnerabilities_occurrence, project: project2) }
+
+    before do
+      create(
+        :vulnerability_feedback,
+        :dismissal,
+        project_fingerprint: finding1.project_fingerprint
+      )
+    end
+
+    it 'returns all non-dismissed occurrences' do
+      expect(described_class.undismissed).to contain_exactly(finding2, finding3)
+    end
 
-      expect(described_class.undismissed).to contain_exactly(undismissed_occurrence)
+    it 'returns non-dismissed occurrences for project' do
+      expect(project2.vulnerability_findings.undismissed).to contain_exactly(finding3)
     end
   end