From 3d1de3329a7a3df8a02adf210f4a12c436aed038 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Fri, 7 Jul 2023 15:02:39 +0000
Subject: [PATCH] Add mutation to dismiss multiple vulnerabilities
This change adds a new mutation that can be used to dismiss multiple
vulnerabilities at the same time. The name of the new mutation is
`Mutation.vulnerabilitiesDismiss`.
---
doc/api/graphql/reference/index.md | 25 +++
.../security/vulnerabilities_controller.rb | 4 +
.../security/vulnerabilities_controller.rb | 1 +
.../vulnerability_report_controller.rb | 4 +
.../security/application_controller.rb | 4 +
ee/app/graphql/ee/types/mutation_type.rb | 1 +
.../mutations/vulnerabilities/bulk_dismiss.rb | 62 +++++++
ee/app/models/ee/vulnerability.rb | 5 +
.../system_notes/vulnerabilities_service.rb | 50 +++---
.../vulnerabilities/bulk_dismiss_service.rb | 127 +++++++++++++++
.../dismiss_multiple_vulnerabilities.yml | 8 +
.../security/vulnerability_report_spec.rb | 44 +++++
.../vulnerabilities/bulk_dismiss_spec.rb | 53 ++++++
ee/spec/models/ee/vulnerability_spec.rb | 21 +++
.../vulnerabilities/bulk_dismiss_spec.rb | 125 ++++++++++++++
.../bulk_dismiss_service_spec.rb | 153 ++++++++++++++++++
16 files changed, 669 insertions(+), 18 deletions(-)
create mode 100644 ee/app/graphql/mutations/vulnerabilities/bulk_dismiss.rb
create mode 100644 ee/app/services/vulnerabilities/bulk_dismiss_service.rb
create mode 100644 ee/config/feature_flags/development/dismiss_multiple_vulnerabilities.yml
create mode 100644 ee/spec/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb
create mode 100644 ee/spec/requests/api/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb
create mode 100644 ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 65c3072fa8ba4..8c21767f583e5 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -6881,6 +6881,31 @@ Input type: `UserSetNamespaceCommitEmailInput`
| <a id="mutationusersetnamespacecommitemailerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
| <a id="mutationusersetnamespacecommitemailnamespacecommitemail"></a>`namespaceCommitEmail` | [`NamespaceCommitEmail`](#namespacecommitemail) | User namespace commit email after mutation. |
+### `Mutation.vulnerabilitiesDismiss`
+
+WARNING:
+**Introduced** in 16.2.
+This feature is an Experiment. It can be changed or removed at any time.
+
+Input type: `VulnerabilitiesDismissInput`
+
+#### Arguments
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="mutationvulnerabilitiesdismissclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
+| <a id="mutationvulnerabilitiesdismisscomment"></a>`comment` | [`String`](#string) | Comment why vulnerability was dismissed (maximum 50,000 characters). |
+| <a id="mutationvulnerabilitiesdismissdismissalreason"></a>`dismissalReason` | [`VulnerabilityDismissalReason`](#vulnerabilitydismissalreason) | Reason why vulnerability should be dismissed. |
+| <a id="mutationvulnerabilitiesdismissvulnerabilityids"></a>`vulnerabilityIds` | [`[VulnerabilityID!]!`](#vulnerabilityid) | IDs of the vulnerabilities to be dismissed. |
+
+#### Fields
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="mutationvulnerabilitiesdismissclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
+| <a id="mutationvulnerabilitiesdismisserrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
+| <a id="mutationvulnerabilitiesdismissvulnerabilities"></a>`vulnerabilities` **{warning-solid}** | [`[Vulnerability!]!`](#vulnerability) | **Deprecated:** This feature is an Experiment. It can be changed or removed at any time. Introduced in 16.2. |
+
### `Mutation.vulnerabilityConfirm`
Input type: `VulnerabilityConfirmInput`
diff --git a/ee/app/controllers/groups/security/vulnerabilities_controller.rb b/ee/app/controllers/groups/security/vulnerabilities_controller.rb
index a3a334b8a3732..c6c8749358cae 100644
--- a/ee/app/controllers/groups/security/vulnerabilities_controller.rb
+++ b/ee/app/controllers/groups/security/vulnerabilities_controller.rb
@@ -5,6 +5,10 @@ module Security
class VulnerabilitiesController < Groups::ApplicationController
layout 'group'
+ before_action do
+ push_frontend_feature_flag(:dismiss_multiple_vulnerabilities, @project)
+ end
+
feature_category :vulnerability_management
urgency :low
diff --git a/ee/app/controllers/projects/security/vulnerabilities_controller.rb b/ee/app/controllers/projects/security/vulnerabilities_controller.rb
index 2b0fe774f0354..b431bc3d29fce 100644
--- a/ee/app/controllers/projects/security/vulnerabilities_controller.rb
+++ b/ee/app/controllers/projects/security/vulnerabilities_controller.rb
@@ -7,6 +7,7 @@ class VulnerabilitiesController < Projects::ApplicationController
before_action do
push_frontend_feature_flag(:create_vulnerability_jira_issue_via_graphql, @project)
+ push_frontend_feature_flag(:dismiss_multiple_vulnerabilities, @project)
push_frontend_feature_flag(:openai_experimentation, @project)
end
diff --git a/ee/app/controllers/projects/security/vulnerability_report_controller.rb b/ee/app/controllers/projects/security/vulnerability_report_controller.rb
index 92bf3223965d1..165ea0fd4ac4d 100644
--- a/ee/app/controllers/projects/security/vulnerability_report_controller.rb
+++ b/ee/app/controllers/projects/security/vulnerability_report_controller.rb
@@ -3,6 +3,10 @@
module Projects
module Security
class VulnerabilityReportController < Projects::ApplicationController
+ before_action do
+ push_frontend_feature_flag(:dismiss_multiple_vulnerabilities, @project)
+ end
+
before_action :authorize_read_vulnerability!
feature_category :vulnerability_management
diff --git a/ee/app/controllers/security/application_controller.rb b/ee/app/controllers/security/application_controller.rb
index b507f9829b855..30bf5218454dd 100644
--- a/ee/app/controllers/security/application_controller.rb
+++ b/ee/app/controllers/security/application_controller.rb
@@ -4,6 +4,10 @@ module Security
class ApplicationController < ::ApplicationController
include SecurityDashboardsPermissions
+ before_action do
+ push_frontend_feature_flag(:dismiss_multiple_vulnerabilities, @project)
+ end
+
feature_category :vulnerability_management
urgency :low
diff --git a/ee/app/graphql/ee/types/mutation_type.rb b/ee/app/graphql/ee/types/mutation_type.rb
index 35181a8e2fd36..5f50e2ba192e8 100644
--- a/ee/app/graphql/ee/types/mutation_type.rb
+++ b/ee/app/graphql/ee/types/mutation_type.rb
@@ -37,6 +37,7 @@ module MutationType
mount_mutation ::Mutations::Security::Finding::Dismiss
mount_mutation ::Mutations::Security::Finding::RevertToDetected
mount_mutation ::Mutations::Vulnerabilities::Create
+ mount_mutation ::Mutations::Vulnerabilities::BulkDismiss, alpha: { milestone: '16.2' }
mount_mutation ::Mutations::Vulnerabilities::Dismiss
mount_mutation ::Mutations::Vulnerabilities::Resolve
mount_mutation ::Mutations::Vulnerabilities::Confirm
diff --git a/ee/app/graphql/mutations/vulnerabilities/bulk_dismiss.rb b/ee/app/graphql/mutations/vulnerabilities/bulk_dismiss.rb
new file mode 100644
index 0000000000000..adbf92f0db108
--- /dev/null
+++ b/ee/app/graphql/mutations/vulnerabilities/bulk_dismiss.rb
@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+module Mutations
+ module Vulnerabilities
+ class BulkDismiss < BaseMutation
+ graphql_name 'VulnerabilitiesDismiss'
+ authorize :admin_vulnerability
+
+ argument :vulnerability_ids,
+ [::Types::GlobalIDType[::Vulnerability]],
+ required: true,
+ prepare: ->(vulnerability_ids, ctx) {
+ ::Mutations::Vulnerabilities::BulkDismiss.prepare(vulnerability_ids, ctx)
+ },
+ description: "IDs of the vulnerabilities to be dismissed."
+
+ argument :comment,
+ GraphQL::Types::String,
+ required: false,
+ description: "Comment why vulnerability was dismissed (maximum 50,000 characters)."
+
+ argument :dismissal_reason,
+ Types::Vulnerabilities::DismissalReasonEnum,
+ required: false,
+ description: 'Reason why vulnerability should be dismissed.'
+
+ field :vulnerabilities, [Types::VulnerabilityType],
+ alpha: { milestone: '16.2' },
+ null: false,
+ description: 'Vulnerabilities after state change.'
+
+ def resolve(comment: nil, dismissal_reason: nil, vulnerability_ids: [])
+ ids = vulnerability_ids.map(&:model_id).uniq
+
+ response = ::Vulnerabilities::BulkDismissService.new(
+ current_user,
+ ids,
+ comment,
+ dismissal_reason
+ ).execute
+
+ {
+ vulnerabilities: response[:vulnerabilities] || [],
+ errors: response.success? ? [] : [response.message]
+ }
+ rescue Gitlab::Access::AccessDeniedError
+ raise_resource_not_available_error!
+ end
+
+ def self.prepare(vulnerability_ids, _ctx)
+ max_vulnerabilities = ::Vulnerabilities::BulkDismissService::MAX_BATCH
+ if vulnerability_ids.length > max_vulnerabilities
+ raise GraphQL::ExecutionError, "Maximum vulnerability_ids exceeded (#{max_vulnerabilities})"
+ elsif vulnerability_ids.empty?
+ raise GraphQL::ExecutionError, "At least 1 value must be provided for vulnerability_ids"
+ end
+
+ vulnerability_ids
+ end
+ end
+ end
+end
diff --git a/ee/app/models/ee/vulnerability.rb b/ee/app/models/ee/vulnerability.rb
index f09370d153266..341db402c7a67 100644
--- a/ee/app/models/ee/vulnerability.rb
+++ b/ee/app/models/ee/vulnerability.rb
@@ -91,6 +91,7 @@ def with_vulnerability_links
scope :with_report_types, -> (report_types) { where(report_type: report_types) }
scope :with_severities, -> (severities) { where(severity: severities) }
scope :with_states, -> (states) { where(state: states) }
+ scope :without_states, -> (states) { where.not(state: states) }
scope :with_scanner_external_ids, -> (scanner_external_ids) { joins(findings: :scanner).merge(::Vulnerabilities::Scanner.with_external_id(scanner_external_ids)) }
scope :grouped_by_severity, -> { reorder(severity: :desc).group(:severity) }
scope :by_primary_identifier_ids, -> (identifier_ids) do
@@ -289,6 +290,10 @@ def parent_class
::Project
end
+ def projects
+ ::Project.id_in(select(:project_id))
+ end
+
def to_ability_name
model_name.singular
end
diff --git a/ee/app/services/system_notes/vulnerabilities_service.rb b/ee/app/services/system_notes/vulnerabilities_service.rb
index 6346b3bbf7b8e..d73ca41c0244d 100644
--- a/ee/app/services/system_notes/vulnerabilities_service.rb
+++ b/ee/app/services/system_notes/vulnerabilities_service.rb
@@ -13,16 +13,42 @@ def change_vulnerability_state(body = nil)
create_note(NoteSummary.new(noteable, project, author, body, action: "vulnerability_#{to_state}"))
end
+ class << self
+ def formatted_note(from_state, to_state, dismissal_reason, comment)
+ format(
+ "%{from_state} vulnerability status to %{to_state}%{reason}%{comment}",
+ from_state: from_state,
+ to_state: to_state.to_s.titleize,
+ comment: formatted_comment(comment),
+ reason: formatted_reason(dismissal_reason, to_state)
+ )
+ end
+
+ private
+
+ def formatted_reason(dismissal_reason, to_state)
+ return if to_state.to_sym != :dismissed
+ return if dismissal_reason.blank?
+
+ ": #{dismissal_reason.titleize}"
+ end
+
+ def formatted_comment(comment)
+ return unless comment.present?
+
+ format(' and the following comment: "%{comment}"', comment: comment)
+ end
+ end
+
private
def state_change_body
if state_transition.present?
- format(
- "%{from_status} vulnerability status to %{to_status}%{dismissal_reason}%{state_comment}",
- from_status: from_status,
- to_status: to_state.titleize,
- dismissal_reason: dismissal_reason,
- state_comment: state_comment
+ self.class.formatted_note(
+ from_status,
+ to_state,
+ state_transition.dismissal_reason,
+ state_transition.comment
)
else
"changed vulnerability status to Detected"
@@ -37,18 +63,6 @@ def to_state
@to_state ||= state_transition&.to_state || 'detected'
end
- def state_comment
- return unless state_transition.comment.present?
-
- format(' and the following comment: "%{comment}"', comment: state_transition.comment)
- end
-
- def dismissal_reason
- return unless state_transition.to_state_dismissed? && state_transition.dismissal_reason.present?
-
- ": #{state_transition.dismissal_reason.titleize}"
- end
-
def state_transition
@state_transition ||= noteable.latest_state_transition
end
diff --git a/ee/app/services/vulnerabilities/bulk_dismiss_service.rb b/ee/app/services/vulnerabilities/bulk_dismiss_service.rb
new file mode 100644
index 0000000000000..ee2fb76e65279
--- /dev/null
+++ b/ee/app/services/vulnerabilities/bulk_dismiss_service.rb
@@ -0,0 +1,127 @@
+# frozen_string_literal: true
+
+module Vulnerabilities
+ class BulkDismissService
+ include Gitlab::Allowable
+ MAX_BATCH = 100
+
+ def initialize(current_user, vulnerability_ids, comment, dismissal_reason)
+ @user = current_user
+ @vulnerability_ids = vulnerability_ids
+ @comment = comment
+ @dismissal_reason = dismissal_reason
+ @project_ids = {}
+ end
+
+ def execute
+ ensure_authorized_projects!
+
+ vulnerability_ids.each_slice(MAX_BATCH).each do |ids|
+ dismiss(Vulnerability.id_in(ids).without_states(:dismissed))
+ end
+ refresh_statistics
+
+ ServiceResponse.success(payload: {
+ vulnerabilities: Vulnerability.id_in(vulnerability_ids)
+ })
+ rescue ActiveRecord::ActiveRecordError
+ ServiceResponse.error(message: "Could not dismiss vulnerabilities")
+ end
+
+ private
+
+ attr_reader :vulnerability_ids, :user, :comment, :dismissal_reason, :project_ids
+
+ def ensure_authorized_projects!
+ raise Gitlab::Access::AccessDeniedError unless authorized_and_ff_enabled?
+ end
+
+ def authorized_and_ff_enabled?
+ Vulnerability.id_in(vulnerability_ids)
+ .projects
+ .with_group
+ .with_namespace
+ .include_project_feature
+ .all? do |project|
+ Feature.enabled?(:dismiss_multiple_vulnerabilities, project) &&
+ can?(user, :admin_vulnerability, project)
+ end
+ end
+
+ def dismiss(vulnerabilities)
+ vulnerability_attrs = vulnerabilities.pluck(:id, :state, :project_id) # rubocop:disable CodeReuse/ActiveRecord
+ return if vulnerability_attrs.empty?
+
+ state_transitions = transition_attributes_for(vulnerability_attrs)
+ system_notes = system_note_attributes_for(vulnerability_attrs)
+
+ ApplicationRecord.transaction do
+ Note.insert_all!(system_notes)
+ Vulnerabilities::StateTransition.insert_all!(state_transitions)
+ # The `insert_or_update_vulnerability_reads` database trigger does not
+ # update the dismissal_reason and we are moving away from using
+ # database triggers to keep tables up to date.
+ Vulnerabilities::Read
+ .by_vulnerabilities(vulnerabilities)
+ .update_all(dismissal_reason: dismissal_reason)
+
+ vulnerabilities.update_all(
+ state: :dismissed,
+ dismissed_by_id: user.id,
+ dismissed_at: now,
+ updated_at: now
+ )
+ end
+ end
+
+ def transition_attributes_for(attrs)
+ attrs.map do |id, state, _|
+ {
+ vulnerability_id: id,
+ from_state: state,
+ to_state: :dismissed,
+ comment: comment,
+ dismissal_reason: dismissal_reason,
+ author_id: user.id,
+ created_at: now,
+ updated_at: now
+ }
+ end
+ end
+
+ def system_note_attributes_for(attrs)
+ attrs.map do |id, _, project_id|
+ project_ids[project_id] = true
+ {
+ noteable_type: "Vulnerability",
+ noteable_id: id,
+ project_id: project_id,
+ system: true,
+ note: ::SystemNotes::VulnerabilitiesService.formatted_note(
+ 'changed',
+ :dismissed,
+ dismissal_reason.to_s.titleize,
+ comment
+ ),
+ author_id: user.id,
+ created_at: now,
+ updated_at: now
+ }
+ end
+ end
+
+ def refresh_statistics
+ return if project_ids.empty?
+
+ Vulnerabilities::Statistics::AdjustmentWorker.perform_async(project_ids.keys)
+ end
+
+ # We use this for setting the created_at and updated_at timestamps
+ # for the various records created by this service.
+ # The time is memoized on the first call to this method so all of the
+ # created records will have the same timestamps.
+ def now
+ @now ||= Time.current.utc
+ end
+ end
+end
diff --git a/ee/config/feature_flags/development/dismiss_multiple_vulnerabilities.yml b/ee/config/feature_flags/development/dismiss_multiple_vulnerabilities.yml
new file mode 100644
index 0000000000000..30bf22d56739d
--- /dev/null
+++ b/ee/config/feature_flags/development/dismiss_multiple_vulnerabilities.yml
@@ -0,0 +1,8 @@
+---
+name: dismiss_multiple_vulnerabilities
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/122815
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/415405
+milestone: '16.2'
+type: development
+group: group::threat insights
+default_enabled: false
diff --git a/ee/spec/features/projects/security/vulnerability_report_spec.rb b/ee/spec/features/projects/security/vulnerability_report_spec.rb
index 4a20bbe3f7325..db0d5a8e573c0 100644
--- a/ee/spec/features/projects/security/vulnerability_report_spec.rb
+++ b/ee/spec/features/projects/security/vulnerability_report_spec.rb
@@ -132,4 +132,48 @@
expect(page).to have_content secret_detection_vulnerability.title
end
end
+
+ context "when dismissing a vulnerability" do
+ let_it_be(:vulnerabilities) { create_list(:vulnerability, 5, :with_findings, project: project) }
+
+ it "dismisses the vulnerability" do
+ visit project_security_vulnerability_report_index_path(project)
+ wait_for_all_requests
+
+ vulnerabilities.each do |vulnerability|
+ expect(page).to have_content(vulnerability.title)
+ end
+
+ change_all(state: :dismissed, reason: :used_in_tests, comment: "A comment")
+
+ vulnerabilities.each do |vulnerability|
+ expect(page).to have_no_content(vulnerability.title)
+ end
+
+ filter_by(status: :dismissed)
+
+ vulnerabilities.each do |vulnerability|
+ expect(page).to have_content(vulnerability.title)
+ end
+ end
+ end
+
+ def filter_by(status:)
+ click_on "Needs triage +1 more"
+ find("li[data-testid='listbox-item-#{status.to_s.upcase}']").click
+ find("h2[data-testid='vulnerability-report-header']").click
+ wait_for_all_requests
+ end
+
+ def change_all(state:, reason:, comment:)
+ find('input[data-testid="vulnerability-checkbox-all"]').check
+ click_on 'Set status'
+
+ find("li[data-testid='listbox-item-#{state}']").click
+ find("div[data-testid='dismissal-reason-listbox']").click
+ find("li[data-testid='listbox-item-#{reason}']").click
+ find("[data-testid='change-status-comment-textbox']").fill_in(with: comment)
+ click_button 'Change status'
+ wait_for_all_requests
+ end
end
diff --git a/ee/spec/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb b/ee/spec/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb
new file mode 100644
index 0000000000000..9536bd5da967d
--- /dev/null
+++ b/ee/spec/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb
@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Mutations::Vulnerabilities::BulkDismiss, feature_category: :vulnerability_management do
+ include GraphqlHelpers
+
+ let_it_be(:project) { create(:project) }
+ let_it_be(:user) { create(:user) }
+ let_it_be(:vulnerabilities) { create_list(:vulnerability, 2, :with_findings, project: project) }
+
+ let(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
+ let(:vulnerability_ids) { vulnerabilities.map(&:to_global_id) }
+ let(:comment) { 'Dismissal Feedback' }
+ let(:dismissal_reason) { 'used_in_tests' }
+
+ before do
+ stub_feature_flags(dismiss_multiple_vulnerabilities: [project])
+ stub_licensed_features(security_dashboard: true)
+ end
+
+ before_all do
+ project.add_developer(user)
+ end
+
+ subject do
+ mutation.resolve(
+ vulnerability_ids: vulnerability_ids,
+ comment: comment,
+ dismissal_reason: dismissal_reason
+ )
+ end
+
+ describe '#resolve' do
+ it 'does not introduce N+1 errors' do
+ queries = ActiveRecord::QueryRecorder.new do
+ subject
+ end
+
+ expect(
+ queries.occurrences_starting_with('INSERT INTO "vulnerability_state_transitions"').values.sum
+ ).to eq(1)
+ expect(queries.occurrences_starting_with('INSERT INTO "notes"').values.sum).to eq(1)
+ expect(queries.occurrences_starting_with('SELECT "namespaces"').values.sum).to eq(2)
+ expect(queries.occurrences_starting_with('SELECT "project_features"').values.sum).to eq(1)
+ expect(queries.occurrences_starting_with('SELECT "vulnerabilities"').values.sum).to eq(1)
+ expect(queries.occurrences_starting_with('SELECT "projects"').values.sum).to eq(1)
+ expect(queries.occurrences_starting_with('UPDATE "vulnerabilities"').values.sum).to eq(1)
+ expect(queries.occurrences_starting_with('UPDATE "vulnerability_reads"').values.sum).to eq(1)
+ expect(queries.count).to be <= 15
+ end
+ end
+end
diff --git a/ee/spec/models/ee/vulnerability_spec.rb b/ee/spec/models/ee/vulnerability_spec.rb
index eb71b58c6e832..caf8a2adf05df 100644
--- a/ee/spec/models/ee/vulnerability_spec.rb
+++ b/ee/spec/models/ee/vulnerability_spec.rb
@@ -232,6 +232,18 @@
end
end
+ describe '.without_states' do
+ let_it_be(:detected_vulnerability) { create(:vulnerability, :detected) }
+ let_it_be(:dismissed_vulnerability) { create(:vulnerability, :dismissed) }
+ let_it_be(:confirmed_vulnerability) { vulnerability }
+
+ subject { described_class.without_states(:dismissed) }
+
+ it 'returns vulnerabilities matching the given states' do
+ is_expected.to contain_exactly(detected_vulnerability, confirmed_vulnerability)
+ end
+ end
+
describe '.with_scanner_external_ids' do
let!(:vulnerability_1) { create(:vulnerability, :with_findings) }
let!(:vulnerability_2) { create(:vulnerability, :with_findings) }
@@ -674,6 +686,15 @@
it { is_expected.to eq(::Project) }
end
+ describe '.projects' do
+ let!(:project_2) { create(:project) }
+ let!(:vulnerability_2) { create(:vulnerability, :sast, :confirmed, :low, project: project_2) }
+
+ subject(:projects) { ::Vulnerability.all.projects }
+
+ it { is_expected.to contain_exactly(project, project_2) }
+ end
+
describe '.to_ability_name' do
subject(:ability_name) { ::Vulnerability.to_ability_name }
diff --git a/ee/spec/requests/api/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb b/ee/spec/requests/api/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb
new file mode 100644
index 0000000000000..d8574fc8c6e9d
--- /dev/null
+++ b/ee/spec/requests/api/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb
@@ -0,0 +1,125 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+RSpec.describe "Mutation.vulnerabilitiesDismiss", feature_category: :vulnerability_management do
+ include GraphqlHelpers
+
+ let_it_be(:project) { create(:project) }
+ let_it_be(:current_user) { create(:user) }
+ let_it_be(:vulnerability_1) { create(:vulnerability, :with_findings, project: project) }
+ let_it_be(:vulnerability_2) { create(:vulnerability, :with_findings, project: project) }
+
+ let(:vulnerabilities) { [vulnerability_1, vulnerability_2] }
+ let(:vulnerability_ids) { vulnerabilities.map { |v| v.to_global_id.to_s } }
+ let(:comment) { 'Dismissal Feedback' }
+ let(:dismissal_reason) { 'USED_IN_TESTS' }
+ let(:arguments) do
+ {
+ vulnerability_ids: vulnerability_ids,
+ comment: comment,
+ dismissal_reason: dismissal_reason
+ }
+ end
+
+ subject(:mutation) { graphql_mutation(:vulnerabilities_dismiss, arguments) }
+
+ def mutation_response
+ graphql_mutation_response(:vulnerabilities_dismiss)
+ end
+
+ context "when the user does not have access" do
+ it_behaves_like "a mutation that returns a top-level access error"
+ end
+
+ context "when the user has access" do
+ before_all do
+ project.add_developer(current_user)
+ end
+
+ context "when security_dashboard is disabled" do
+ before do
+ stub_licensed_features(security_dashboard: false)
+ end
+
+ it_behaves_like 'a mutation that returns top-level errors',
+ errors: ['The resource that you are attempting to access does not ' \
+ 'exist or you don\'t have permission to perform this action']
+ end
+
+ context "when security_dashboard is enabled" do
+ before do
+ stub_licensed_features(security_dashboard: true)
+ end
+
+ it "dismisses the vulnerabilities" do
+ post_graphql_mutation(mutation, current_user: current_user)
+
+ expect(vulnerability_1.reload).to be_dismissed
+ expect(vulnerability_2.reload).to be_dismissed
+ expect(mutation_response["errors"]).to be_empty
+ expect(mutation_response["vulnerabilities"].count).to eq(2)
+ mutation_response["vulnerabilities"].each do |vulnerability|
+ expect(vulnerability["state"]).to eq("DISMISSED")
+ expect(vulnerability["stateComment"]).to eq(comment)
+ expect(vulnerability["dismissedBy"]["id"]).to eq(current_user.to_global_id.to_s)
+ end
+ end
+
+ context "without a comment" do
+ let(:arguments) do
+ {
+ vulnerability_ids: vulnerability_ids,
+ dismissal_reason: dismissal_reason
+ }
+ end
+
+ it "dismisses the vulnerabilities" do
+ post_graphql_mutation(mutation, current_user: current_user)
+
+ expect(vulnerability_1.reload).to be_dismissed
+ expect(vulnerability_2.reload).to be_dismissed
+ expect(mutation_response["errors"]).to be_empty
+ end
+ end
+
+ context "without a dismissal reason" do
+ let(:arguments) do
+ {
+ vulnerability_ids: vulnerability_ids,
+ comment: comment
+ }
+ end
+
+ it "dismisses the vulnerabilities" do
+ post_graphql_mutation(mutation, current_user: current_user)
+
+ expect(vulnerability_1.reload).to be_dismissed
+ expect(vulnerability_2.reload).to be_dismissed
+ expect(mutation_response["errors"]).to be_empty
+ end
+ end
+
+ context "when too many vulnerabilities are passed" do
+ before do
+ stub_const("::Vulnerabilities::BulkDismissService::MAX_BATCH", 1)
+ end
+
+ it_behaves_like 'a mutation that returns top-level errors', errors: [/Maximum vulnerability_ids exceeded \(1\)/]
+ end
+
+ context "when vulnerability_id is nil" do
+ let(:vulnerability_ids) { [nil] }
+
+ it_behaves_like 'a mutation that returns top-level errors', errors: [/Expected value to not be null/]
+ end
+
+ context "when vulnerability_ids are empty" do
+ let(:vulnerability_ids) { [] }
+
+ it_behaves_like 'a mutation that returns top-level errors',
+ errors: ["At least 1 value must be provided for vulnerability_ids"]
+ end
+ end
+ end
+end
diff --git a/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb b/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb
new file mode 100644
index 0000000000000..223b71f6c809c
--- /dev/null
+++ b/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb
@@ -0,0 +1,153 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Vulnerabilities::BulkDismissService, feature_category: :vulnerability_management do
+ let_it_be(:user) { create(:user) }
+ let_it_be(:project) { create(:project) }
+ let_it_be(:vulnerability) { create(:vulnerability, :with_findings, :detected, :high_severity, project: project) }
+ let(:vulnerability_ids) { [vulnerability.id] }
+ let(:comment) { "i prefer lowercase." }
+ let(:dismissal_reason) { 'used_in_tests' }
+
+ subject(:service) { described_class.new(user, vulnerability_ids, comment, dismissal_reason) }
+
+ describe '#execute' do
+ before_all do
+ project.add_developer(user)
+ end
+
+ before do
+ stub_licensed_features(security_dashboard: true)
+ end
+
+ context 'when the user is not authorized to dismiss vulnerabilities from one of the projects' do
+ let_it_be(:other_project) { create(:project) }
+ let_it_be(:other_vulnerability) { create(:vulnerability, :with_findings, project: other_project) }
+ let(:vulnerability_ids) { [vulnerability.id, other_vulnerability.id] }
+
+ it 'raises an error' do
+ expect { service.execute }.to raise_error(Gitlab::Access::AccessDeniedError)
+ end
+ end
+
+ context 'when the user is authorized' do
+ it 'dismisses each vulnerability', :freeze_time do
+ service.execute
+
+ vulnerability.reload
+ expect(vulnerability).to be_dismissed
+ expect(vulnerability.dismissed_by).to eq(user)
+ expect(vulnerability.dismissed_at).to eq(Time.current)
+ end
+
+ it 'inserts a state transition for each vulnerability' do
+ service.execute
+
+ vulnerability.reload
+ last_state = vulnerability.state_transitions.last
+ expect(last_state.from_state).to eq('detected')
+ expect(last_state.to_state).to eq('dismissed')
+ expect(last_state.comment).to eq(comment)
+ expect(last_state.dismissal_reason).to eq(dismissal_reason)
+ expect(last_state.author).to eq(user)
+ end
+
+ it 'inserts a system note for each vulnerability' do
+ service.execute
+
+ last_note = Note.last
+
+ expect(last_note.noteable).to eq(vulnerability)
+ expect(last_note.author).to eq(user)
+ expect(last_note.project).to eq(project)
+ expect(last_note.note).to eq(
+ "changed vulnerability status to Dismissed: Used In Tests and the following comment: \"#{comment}\""
+ )
+ expect(last_note).to be_system
+ end
+
+ it 'updates the dismissal reason for each vulnerability read record' do
+ service.execute
+
+ reads = Vulnerabilities::Read.by_vulnerabilities(vulnerability_ids)
+ expect(reads.pluck(:dismissal_reason)).to match_array([dismissal_reason])
+ end
+
+ it 'updates the statistics', :sidekiq_inline do
+ _active_vulnerability = create(:vulnerability, :high_severity, project: project)
+
+ service.execute
+
+ expect(project.vulnerability_statistic).to be_present
+ expect(project.vulnerability_statistic.total).to eq(1)
+ expect(project.vulnerability_statistic.critical).to eq(0)
+ expect(project.vulnerability_statistic.high).to eq(1)
+ expect(project.vulnerability_statistic.medium).to eq(0)
+ expect(project.vulnerability_statistic.low).to eq(0)
+ expect(project.vulnerability_statistic.unknown).to eq(0)
+ expect(project.vulnerability_statistic.letter_grade).to eq('d')
+ end
+
+ it 'returns a service response' do
+ result = service.execute
+
+ expect(result.payload).to have_key(:vulnerabilities)
+ expect(result.payload[:vulnerabilities].count).to eq(vulnerability_ids.count)
+ end
+
+ context 'when an error occurs' do
+ before do
+ allow(Note).to receive(:insert_all!).and_raise(ActiveRecord::RecordNotUnique)
+ end
+
+ it 'does not bubble up the error' do
+ expect { service.execute }.not_to raise_error
+ end
+
+ it 'returns an appropriate service response' do
+ result = service.execute
+
+ expect(result).to be_error
+ expect(result.errors).to eq(['Could not dismiss vulnerabilities'])
+ end
+
+ it 'does not commit any changes' do
+ service.execute
+
+ expect(vulnerability.reload).not_to be_dismissed
+ end
+ end
+
+ context 'when updating a large # of vulnerabilities' do
+ let_it_be(:vulnerabilities) { create_list(:vulnerability, 2, :with_findings, project: project) }
+ let_it_be(:vulnerability_ids) { vulnerabilities.map(&:id) }
+
+ it 'does not introduce N+1 queries' do
+ queries = ActiveRecord::QueryRecorder.new do
+ service.execute
+ end
+
+ expect(queries.count).to eq(13)
+ end
+ end
+
+ context 'when a vulnerability has already been dismissed' do
+ let_it_be(:dismissed_vulnerability) { create(:vulnerability, :with_findings, :dismissed, project: project) }
+ let(:vulnerability_ids) { [dismissed_vulnerability.id] }
+
+ it 'does not update the vulnerability' do
+ expect { service.execute }.not_to change { dismissed_vulnerability.reload.dismissed_at }
+ end
+
+ it 'does not insert a system note' do
+ expect { service.execute }.not_to change { Note.count }
+ end
+
+ it 'does not insert a state transition' do
+ expect { service.execute }.not_to change { dismissed_vulnerability.state_transitions.count }
+ end
+ end
+ end
+ end
+end
--
GitLab