Commit 546da471 authored by Alan (Maciej) Paruszewski's avatar Alan (Maciej) Paruszewski Committed by Heinrich Lee Yu

Add sorting to GraphQL Vulnerabilities API

This change adds sorting to Vulnerabilities GraphQL API to allow sorting
by severity.
parent 67ad08fd
......@@ -6927,6 +6927,11 @@ type Group {
"""
severity: [VulnerabilitySeverity!]
"""
List vulnerabilities by sort order
"""
sort: VulnerabilitySort = severity_desc
"""
Filter vulnerabilities by state
"""
......@@ -12689,6 +12694,11 @@ type Project {
"""
severity: [VulnerabilitySeverity!]
"""
List vulnerabilities by sort order
"""
sort: VulnerabilitySort = severity_desc
"""
Filter vulnerabilities by state
"""
......@@ -13451,6 +13461,11 @@ type Query {
"""
severity: [VulnerabilitySeverity!]
"""
List vulnerabilities by sort order
"""
sort: VulnerabilitySort = severity_desc
"""
Filter vulnerabilities by state
"""
......@@ -18389,6 +18404,21 @@ enum VulnerabilitySeverity {
UNKNOWN
}
"""
Vulnerability sort values
"""
enum VulnerabilitySort {
"""
Severity in ascending order
"""
severity_asc
"""
Severity in descending order
"""
severity_desc
}
"""
The state of the vulnerability.
"""
......
......@@ -19047,6 +19047,16 @@
},
"defaultValue": null
},
{
"name": "sort",
"description": "List vulnerabilities by sort order",
"type": {
"kind": "ENUM",
"name": "VulnerabilitySort",
"ofType": null
},
"defaultValue": "severity_desc"
},
{
"name": "after",
"description": "Returns the elements in the list that come after the specified cursor.",
......@@ -37132,6 +37142,16 @@
},
"defaultValue": null
},
{
"name": "sort",
"description": "List vulnerabilities by sort order",
"type": {
"kind": "ENUM",
"name": "VulnerabilitySort",
"ofType": null
},
"defaultValue": "severity_desc"
},
{
"name": "after",
"description": "Returns the elements in the list that come after the specified cursor.",
......@@ -39461,6 +39481,16 @@
},
"defaultValue": null
},
{
"name": "sort",
"description": "List vulnerabilities by sort order",
"type": {
"kind": "ENUM",
"name": "VulnerabilitySort",
"ofType": null
},
"defaultValue": "severity_desc"
},
{
"name": "after",
"description": "Returns the elements in the list that come after the specified cursor.",
......@@ -54034,6 +54064,29 @@
],
"possibleTypes": null
},
{
"kind": "ENUM",
"name": "VulnerabilitySort",
"description": "Vulnerability sort values",
"fields": null,
"inputFields": null,
"interfaces": null,
"enumValues": [
{
"name": "severity_desc",
"description": "Severity in descending order",
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "severity_asc",
"description": "Severity in ascending order",
"isDeprecated": false,
"deprecationReason": null
}
],
"possibleTypes": null
},
{
"kind": "ENUM",
"name": "VulnerabilityState",
......@@ -6,19 +6,20 @@
#
# Arguments:
# vulnerable: any object that has a #vulnerabilities method that returns a collection of `Vulnerability`s
# filters: optional! a hash with one or more of the following:
# params: optional! a hash with one or more of the following:
# project_ids: if `vulnerable` includes multiple projects (like a Group), this filter will restrict
# the vulnerabilities returned to those in the group's projects that also match these IDs
# report_types: only return vulnerabilities from these report types
# severities: only return vulnerabilities with these severities
# states: only return vulnerabilities in these states
# sort: return vulnerabilities ordered by severity_asc or severity_desc
module Security
class VulnerabilitiesFinder
include FinderMethods
def initialize(vulnerable, filters = {})
@filters = filters
def initialize(vulnerable, params = {})
@params = params
@vulnerabilities = vulnerable.vulnerabilities
end
......@@ -29,41 +30,45 @@ module Security
filter_by_states
filter_by_scanners
vulnerabilities
sort(vulnerabilities)
end
private
attr_reader :filters, :vulnerabilities
attr_reader :params, :vulnerabilities
def filter_by_projects
if filters[:project_id].present?
@vulnerabilities = vulnerabilities.for_projects(filters[:project_id])
if params[:project_id].present?
@vulnerabilities = vulnerabilities.for_projects(params[:project_id])
end
end
def filter_by_report_types
if filters[:report_type].present?
@vulnerabilities = vulnerabilities.with_report_types(filters[:report_type])
if params[:report_type].present?
@vulnerabilities = vulnerabilities.with_report_types(params[:report_type])
end
end
def filter_by_severities
if filters[:severity].present?
@vulnerabilities = vulnerabilities.with_severities(filters[:severity])
if params[:severity].present?
@vulnerabilities = vulnerabilities.with_severities(params[:severity])
end
end
def filter_by_states
if filters[:state].present?
@vulnerabilities = vulnerabilities.with_states(filters[:state])
if params[:state].present?
@vulnerabilities = vulnerabilities.with_states(params[:state])
end
end
def filter_by_scanners
if filters[:scanner].present?
@vulnerabilities = vulnerabilities.with_scanners(filters[:scanner])
if params[:scanner].present?
@vulnerabilities = vulnerabilities.with_scanners(params[:scanner])
end
end
def sort(items)
items.order_by(params[:sort])
end
end
end
......@@ -26,19 +26,23 @@ module Resolvers
required: false,
description: 'Filter vulnerabilities by scanner'
argument :sort, Types::VulnerabilitySortEnum,
required: false,
default_value: 'severity_desc',
description: 'List vulnerabilities by sort order'
def resolve(**args)
return Vulnerability.none unless vulnerable
vulnerabilities(args)
.with_findings_scanner_and_identifiers
.with_created_issue_links_and_issues
.ordered
end
private
def vulnerabilities(filters)
Security::VulnerabilitiesFinder.new(vulnerable, filters).execute
def vulnerabilities(params)
Security::VulnerabilitiesFinder.new(vulnerable, params).execute
end
end
end
# frozen_string_literal: true
module Types
class VulnerabilitySortEnum < BaseEnum
graphql_name 'VulnerabilitySort'
description 'Vulnerability sort values'
value 'severity_desc', 'Severity in descending order'
value 'severity_asc', 'Severity in ascending order'
end
end
......@@ -59,8 +59,6 @@ class Vulnerability < ApplicationRecord
validates :description, length: { maximum: Issuable::DESCRIPTION_LENGTH_MAX }, allow_blank: true
validates :description_html, length: { maximum: Issuable::DESCRIPTION_HTML_LENGTH_MAX }, allow_blank: true
scope :ordered, -> { order(severity: :desc) }
scope :with_findings, -> { includes(:findings) }
scope :with_findings_and_scanner, -> { includes(findings: :scanner) }
scope :with_findings_scanner_and_identifiers, -> { includes(findings: [:scanner, :identifiers, finding_identifiers: :identifier]) }
......@@ -71,7 +69,10 @@ class Vulnerability < ApplicationRecord
scope :with_severities, -> (severities) { where(severity: severities) }
scope :with_states, -> (states) { where(state: states) }
scope :with_scanners, -> (scanners) { joins(findings: :scanner).merge(Vulnerabilities::Scanner.with_external_id(scanners)) }
scope :grouped_by_severity, -> { group(:severity) }
scope :grouped_by_severity, -> { reorder(severity: :desc).group(:severity) }
scope :order_severity_asc, -> { reorder(severity: :asc, id: :desc) }
scope :order_severity_desc, -> { reorder(severity: :desc, id: :desc) }
class << self
def parent_class
......@@ -117,6 +118,15 @@ class Vulnerability < ApplicationRecord
def active_state_values
states.values_at(*active_states)
end
def order_by(method)
case method.to_s
when 'severity_desc' then order_severity_desc
when 'severity_asc' then order_severity_asc
else
order_severity_desc
end
end
end
# There will only be one finding associated with a vulnerability for the foreseeable future
......
---
title: Add ability to sort vulnerabilities by severity in GraphQL API
merge_request: 40856
author:
type: added
......@@ -10,11 +10,11 @@ RSpec.describe Security::VulnerabilitiesFinder do
end
let_it_be(:vulnerability2) do
create(:vulnerability, :with_findings, severity: :medium, report_type: :dast, state: :dismissed, project: project)
create(:vulnerability, :with_findings, severity: :high, report_type: :dependency_scanning, state: :confirmed, project: project)
end
let_it_be(:vulnerability3) do
create(:vulnerability, :with_findings, severity: :high, report_type: :dependency_scanning, state: :confirmed, project: project)
create(:vulnerability, :with_findings, severity: :medium, report_type: :dast, state: :dismissed, project: project)
end
let(:filters) { {} }
......@@ -38,7 +38,7 @@ RSpec.describe Security::VulnerabilitiesFinder do
let(:filters) { { report_type: %w[sast dast] } }
it 'only returns vulnerabilities matching the given report types' do
is_expected.to contain_exactly(vulnerability1, vulnerability2)
is_expected.to contain_exactly(vulnerability1, vulnerability3)
end
end
......@@ -46,7 +46,7 @@ RSpec.describe Security::VulnerabilitiesFinder do
let(:filters) { { severity: %w[medium high] } }
it 'only returns vulnerabilities matching the given severities' do
is_expected.to contain_exactly(vulnerability2, vulnerability3)
is_expected.to contain_exactly(vulnerability3, vulnerability2)
end
end
......@@ -54,15 +54,15 @@ RSpec.describe Security::VulnerabilitiesFinder do
let(:filters) { { state: %w[detected confirmed] } }
it 'only returns vulnerabilities matching the given states' do
is_expected.to contain_exactly(vulnerability1, vulnerability3)
is_expected.to contain_exactly(vulnerability1, vulnerability2)
end
end
context 'when filtered by scanner' do
let(:filters) { { scanner: [vulnerability1.finding_scanner_external_id, vulnerability3.finding_scanner_external_id] } }
let(:filters) { { scanner: [vulnerability1.finding_scanner_external_id, vulnerability2.finding_scanner_external_id] } }
it 'only returns vulnerabilities matching the given scanners' do
is_expected.to contain_exactly(vulnerability1, vulnerability3)
is_expected.to contain_exactly(vulnerability1, vulnerability2)
end
end
......@@ -82,6 +82,22 @@ RSpec.describe Security::VulnerabilitiesFinder do
end
end
context 'when sorted' do
let(:filters) { { sort: method } }
context 'ascending by severity' do
let(:method) { :severity_asc }
it { is_expected.to eq([vulnerability1, vulnerability3, vulnerability2]) }
end
context 'descending by severity' do
let(:method) { :severity_desc }
it { is_expected.to eq([vulnerability2, vulnerability3, vulnerability1]) }
end
end
context 'when filtered by more than one property' do
let_it_be(:vulnerability4) do
create(:vulnerability, severity: :medium, report_type: :sast, state: :detected, project: project)
......
......@@ -6,7 +6,7 @@ RSpec.describe Resolvers::VulnerabilitiesResolver do
include GraphqlHelpers
describe '#resolve' do
subject { resolve(described_class, obj: vulnerable, args: filters, ctx: { current_user: current_user }) }
subject { resolve(described_class, obj: vulnerable, args: params, ctx: { current_user: current_user }) }
let_it_be(:project) { create(:project) }
let_it_be(:user) { create(:user, security_dashboard_projects: [project]) }
......@@ -24,17 +24,37 @@ RSpec.describe Resolvers::VulnerabilitiesResolver do
end
let(:current_user) { user }
let(:filters) { {} }
let(:params) { {} }
let(:vulnerable) { project }
it 'orders results by severity' do
expect(subject.first).to eq(critical_vulnerability)
expect(subject.second).to eq(high_vulnerability)
expect(subject.third).to eq(low_vulnerability)
context 'when given sort' do
context 'when sorting descending by severity' do
let(:params) { { sort: :severity_desc } }
it { is_expected.to eq([critical_vulnerability, high_vulnerability, low_vulnerability]) }
end
context 'when sorting ascending by severity' do
let(:params) { { sort: :severity_asc } }
it { is_expected.to eq([low_vulnerability, high_vulnerability, critical_vulnerability]) }
end
context 'when sorting param is not provided' do
let(:params) { {} }
it { is_expected.to eq([critical_vulnerability, high_vulnerability, low_vulnerability]) }
end
context 'when sorting by invalid param' do
let(:params) { { sort: :invalid } }
it { is_expected.to eq([critical_vulnerability, high_vulnerability, low_vulnerability]) }
end
end
context 'when given severities' do
let(:filters) { { severity: ['low'] } }
let(:params) { { severity: ['low'] } }
it 'only returns vulnerabilities of the given severities' do
is_expected.to contain_exactly(low_vulnerability)
......@@ -42,7 +62,7 @@ RSpec.describe Resolvers::VulnerabilitiesResolver do
end
context 'when given states' do
let(:filters) { { state: ['dismissed'] } }
let(:params) { { state: ['dismissed'] } }
it 'only returns vulnerabilities of the given states' do
is_expected.to contain_exactly(high_vulnerability)
......@@ -50,7 +70,7 @@ RSpec.describe Resolvers::VulnerabilitiesResolver do
end
context 'when given scanner' do
let(:filters) { { scanner: [high_vulnerability.finding_scanner_external_id] } }
let(:params) { { scanner: [high_vulnerability.finding_scanner_external_id] } }
it 'only returns vulnerabilities of the given scanner' do
is_expected.to contain_exactly(high_vulnerability)
......@@ -58,7 +78,7 @@ RSpec.describe Resolvers::VulnerabilitiesResolver do
end
context 'when given report types' do
let(:filters) { { report_type: %i[dast sast] } }
let(:params) { { report_type: %i[dast sast] } }
it 'only returns vulnerabilities of the given report types' do
is_expected.to contain_exactly(critical_vulnerability, low_vulnerability)
......@@ -70,7 +90,7 @@ RSpec.describe Resolvers::VulnerabilitiesResolver do
let_it_be(:project2) { create(:project, namespace: group) }
let_it_be(:project2_vulnerability) { create(:vulnerability, project: project2) }
let(:filters) { { project_id: [project2.id] } }
let(:params) { { project_id: [project2.id] } }
let(:vulnerable) { group }
before do
......@@ -82,7 +102,7 @@ RSpec.describe Resolvers::VulnerabilitiesResolver do
end
context 'with multiple project IDs' do
let(:filters) { { project_id: [project.id, project2.id] } }
let(:params) { { project_id: [project.id, project2.id] } }
it 'avoids N+1 queries' do
control_count = ActiveRecord::QueryRecorder.new do
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe GitlabSchema.types['VulnerabilitySort'] do
it { expect(described_class.graphql_name).to eq('VulnerabilitySort') }
it 'exposes all the existing Vulnerability sort orders' do
expect(described_class.values.keys).to include(*%w[severity_desc severity_asc])
end
end
......@@ -156,8 +156,20 @@ RSpec.describe Vulnerability do
end
end
describe '.ordered' do
subject { described_class.ordered }
describe '.order_severity_asc' do
subject { described_class.order_severity_asc }
it 'returns vulnerabilities ordered by severity' do
low_vulnerability = create(:vulnerability, :low)
critical_vulnerability = create(:vulnerability, :critical)
medium_vulnerability = create(:vulnerability, :medium)
is_expected.to eq([low_vulnerability, medium_vulnerability, critical_vulnerability])
end
end
describe '.order_severity_desc' do
subject { described_class.order_severity_desc }
it 'returns vulnerabilities ordered by severity' do
low_vulnerability = create(:vulnerability, :low)
......@@ -168,6 +180,26 @@ RSpec.describe Vulnerability do
end
end
describe '.order_by' do
subject { described_class.order_by(method) }
let!(:low_vulnerability) { create(:vulnerability, :low) }
let!(:critical_vulnerability) { create(:vulnerability, :critical) }
let!(:medium_vulnerability) { create(:vulnerability, :medium) }
context 'when ordered by severity_desc' do
let(:method) { :severity_desc }
it { is_expected.to eq([critical_vulnerability, medium_vulnerability, low_vulnerability]) }
end
context 'when ordered by severity_asc' do
let(:method) { :severity_asc }
it { is_expected.to eq([low_vulnerability, medium_vulnerability, critical_vulnerability]) }
end
end
describe '.counts_by_day_and_severity' do
context 'when the vulnerability_history feature flag is disabled' do
before do
......
......@@ -38,7 +38,7 @@ RSpec.describe API::Vulnerabilities do
get_vulnerabilities
expect(response).to have_gitlab_http_status(:ok)
expect(json_response.map { |v| v['id'] }).to contain_exactly(project.vulnerabilities.second.id)
expect(json_response.map { |v| v['id'] }).to contain_exactly(project.vulnerabilities.order_severity_desc.second.id)
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