Commit ff983fef authored by pburke's avatar pburke

Add scope filter to security dashboards

- Use same scope param behavior as Vulnerabilities API
- Hide dismissed vulnerabilities by default or when scope = "dismissed"
- Include all vulnerabilities when scope = "all"
parent f1f8ff31
......@@ -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)
......
......@@ -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
......
---
title: Add filter for dismissed vulnerabilities on security dashboards.
merge_request: 16692
author:
type: added
......@@ -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
......
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment