Commit 9db2a8d4 authored by Alan (Maciej) Paruszewski's avatar Alan (Maciej) Paruszewski Committed by Fabio Pitino

Allow revert vulnerabilities to detected state

This change adds services necessary to revert Vulnerability back to
detected state.
parent 3ffa946e
......@@ -19,6 +19,7 @@ module EE
'weight' => 'weight',
'relate_epic' => 'epic',
'unrelate_epic' => 'epic',
'vulnerability_detected' => 'search-dot',
'vulnerability_confirmed' => 'shield',
'vulnerability_dismissed' => 'cancel',
'vulnerability_resolved' => 'status_closed',
......
......@@ -8,7 +8,7 @@ module EE
weight published
epic_issue_added issue_added_to_epic epic_issue_removed issue_removed_from_epic
epic_issue_moved issue_changed_epic epic_date_changed relate_epic unrelate_epic
vulnerability_confirmed vulnerability_dismissed vulnerability_resolved
vulnerability_confirmed vulnerability_dismissed vulnerability_resolved vulnerability_detected
iteration
].freeze
......
......@@ -40,6 +40,7 @@ module EE
has_one :group, through: :project
has_many :findings, class_name: '::Vulnerabilities::Finding', inverse_of: :vulnerability
has_many :dismissed_findings, -> { dismissed }, class_name: 'Vulnerabilities::Finding', inverse_of: :vulnerability
has_many :issue_links, class_name: '::Vulnerabilities::IssueLink', inverse_of: :vulnerability
has_many :created_issue_links, -> { created }, class_name: '::Vulnerabilities::IssueLink', inverse_of: :vulnerability
has_many :related_issues, through: :issue_links, source: :issue do
......
......@@ -147,15 +147,21 @@ module Vulnerabilities
end
end
def self.related_dismissal_feedback
Feedback
.where(arel_table[:report_type].eq(Feedback.arel_table[:category]))
.where(arel_table[:project_id].eq(Feedback.arel_table[:project_id]))
.where(Arel::Nodes::NamedFunction.new('ENCODE', [arel_table[:project_fingerprint], Arel::Nodes::SqlLiteral.new("'HEX'")]).eq(Feedback.arel_table[:project_fingerprint]))
.for_dismissal
end
private_class_method :related_dismissal_feedback
def self.dismissed
where('EXISTS (?)', related_dismissal_feedback.select(1))
end
def self.undismissed
where(
"NOT EXISTS (?)",
Feedback.select(1)
.where("#{table_name}.report_type = vulnerability_feedback.category")
.where("#{table_name}.project_id = vulnerability_feedback.project_id")
.where("ENCODE(#{table_name}.project_fingerprint, 'HEX') = vulnerability_feedback.project_fingerprint") # rubocop:disable GitlabSecurity/SqlInjection
.for_dismissal
)
where('NOT EXISTS (?)', related_dismissal_feedback.select(1))
end
def self.batch_count_by_project_and_severity(project_id, severity)
......
......@@ -5,7 +5,8 @@ module EE
class VulnerabilitiesService < ::SystemNotes::BaseService
# Called when state is changed for 'vulnerability'
def change_vulnerability_state
body = "changed vulnerability status to #{noteable.state}"
type = noteable.detected? ? 'reverted' : 'changed'
body = "#{type} vulnerability status to #{noteable.state}"
create_note(NoteSummary.new(noteable, project, author, body, action: "vulnerability_#{noteable.state}"))
end
......
# frozen_string_literal: true
module Vulnerabilities
class RevertToDetectedService < BaseService
include Gitlab::Allowable
REVERT_PARAMS = { resolved_by: nil, resolved_at: nil, dismissed_by: nil, dismissed_at: nil, confirmed_by: nil, confirmed_at: nil }.freeze
def execute
raise Gitlab::Access::AccessDeniedError unless authorized?
@vulnerability.transaction do
revert_result = revert_findings_to_detected_state
raise ActiveRecord::Rollback unless revert_result
update_with_note(@vulnerability, state: Vulnerability.states[:detected], **REVERT_PARAMS)
end
@vulnerability
end
private
def destroy_feedback_for(finding)
VulnerabilityFeedback::DestroyService
.new(@project, @user, finding.dismissal_feedback)
.execute
end
def revert_findings_to_detected_state
@vulnerability
.dismissed_findings
.each do |finding|
result = destroy_feedback_for(finding)
unless result
handle_finding_revert_error(finding)
return false
end
end
true
end
def handle_finding_revert_error(finding)
@vulnerability.errors.add(
:base,
:finding_revert_to_detected_error,
message: _("failed to revert associated finding(id=%{finding_id}) to detected") %
{
finding_id: finding.id
})
end
end
end
......@@ -4,7 +4,7 @@ require 'spec_helper'
RSpec.describe EE::SystemNoteMetadata do
%i[
vulnerability_confirmed vulnerability_dismissed vulnerability_resolved
vulnerability_confirmed vulnerability_dismissed vulnerability_resolved vulnerability_detected
].each do |action|
context 'when action type is valid' do
subject do
......
......@@ -33,6 +33,7 @@ RSpec.describe Vulnerability do
it { is_expected.to belong_to(:milestone) }
it { is_expected.to belong_to(:epic) }
it { is_expected.to have_many(:findings).class_name('Vulnerabilities::Finding').inverse_of(:vulnerability) }
it { is_expected.to have_many(:dismissed_findings).class_name('Vulnerabilities::Finding').inverse_of(:vulnerability) }
it { is_expected.to have_many(:issue_links).class_name('Vulnerabilities::IssueLink').inverse_of(:vulnerability) }
it { is_expected.to have_many(:created_issue_links).class_name('Vulnerabilities::IssueLink').inverse_of(:vulnerability).conditions(link_type: Vulnerabilities::IssueLink.link_types['created']) }
it { is_expected.to have_many(:related_issues).through(:issue_links).source(:issue) }
......
......@@ -252,6 +252,44 @@ RSpec.describe Vulnerabilities::Finding do
end
end
describe '.dismissed' do
let_it_be(:project) { create(:project) }
let_it_be(:project2) { create(:project) }
let!(:finding1) { create(:vulnerabilities_finding, project: project) }
let!(:finding2) { create(:vulnerabilities_finding, project: project, report_type: :dast) }
let!(:finding3) { create(:vulnerabilities_finding, project: project2) }
before do
create(
:vulnerability_feedback,
:dismissal,
project: finding1.project,
project_fingerprint: finding1.project_fingerprint
)
create(
:vulnerability_feedback,
:dismissal,
project_fingerprint: finding2.project_fingerprint,
project: project2
)
create(
:vulnerability_feedback,
:dismissal,
category: :sast,
project_fingerprint: finding2.project_fingerprint,
project: finding2.project
)
end
it 'returns all dismissed findings' do
expect(described_class.dismissed).to contain_exactly(finding1)
end
it 'returns dismissed findings for project' do
expect(project.vulnerability_findings.dismissed).to contain_exactly(finding1)
end
end
describe '.batch_count_by_project_and_severity' do
let(:pipeline) { create(:ci_pipeline, :success, project: project) }
let(:project) { create(:project) }
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Vulnerabilities::RevertToDetectedService do
include AccessMatchersGeneric
before do
stub_licensed_features(security_dashboard: true)
end
let_it_be(:user) { create(:user) }
let(:project) { create(:project) } # cannot use let_it_be here: caching causes problems with permission-related tests
let(:vulnerability) { create(:vulnerability, :with_findings, project: project) }
let(:service) { described_class.new(user, vulnerability) }
subject(:revert_vulnerability_to_detected) { service.execute }
shared_examples 'reverts vulnerability' do
it 'reverts a vulnerability and its associated findings to detected state' do
Timecop.freeze do
revert_vulnerability_to_detected
expect(vulnerability.reload).to(
have_attributes(state: 'detected', dismissed_by: nil, dismissed_at: nil, resolved_by: nil, resolved_at: nil, confirmed_by: nil, confirmed_at: nil))
expect(vulnerability.findings).to all not_have_vulnerability_dismissal_feedback
end
end
it 'creates note' do
expect(SystemNoteService).to receive(:change_vulnerability_state).with(vulnerability, user)
revert_vulnerability_to_detected
end
it_behaves_like 'calls vulnerability statistics utility services in order'
end
context 'with an authorized user with proper permissions' do
before do
project.add_developer(user)
end
context 'when vulnerability is dismissed' do
let(:vulnerability) { create(:vulnerability, :dismissed, :with_findings, project: project) }
include_examples 'reverts vulnerability'
end
context 'when vulnerability is confirmed' do
let(:vulnerability) { create(:vulnerability, :confirmed, :with_findings, project: project) }
include_examples 'reverts vulnerability'
end
context 'when vulnerability is resolved' do
let(:vulnerability) { create(:vulnerability, :resolved, :with_findings, project: project) }
include_examples 'reverts vulnerability'
end
context 'when there is an error' do
let(:broken_finding) { vulnerability.findings.first }
let!(:dismissal_feedback) do
create(:vulnerability_feedback, :dismissal, project: broken_finding.project, project_fingerprint: broken_finding.project_fingerprint)
end
before do
allow(service).to receive(:destroy_feedback_for).and_return(false)
end
it 'responds with error' do
expect(revert_vulnerability_to_detected.errors.messages).to eq(
base: ["failed to revert associated finding(id=#{broken_finding.id}) to detected"])
end
end
context 'when security dashboard feature is disabled' do
before do
stub_licensed_features(security_dashboard: false)
end
it 'raises an "access denied" error' do
expect { revert_vulnerability_to_detected }.to raise_error(Gitlab::Access::AccessDeniedError)
end
end
end
describe 'permissions' do
it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:admin) }
it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:owner).of(project) }
it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:maintainer).of(project) }
it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:developer).of(project) }
it { expect { revert_vulnerability_to_detected }.to be_denied_for(:auditor) }
it { expect { revert_vulnerability_to_detected }.to be_denied_for(:reporter).of(project) }
it { expect { revert_vulnerability_to_detected }.to be_denied_for(:guest).of(project) }
it { expect { revert_vulnerability_to_detected }.to be_denied_for(:anonymous) }
end
end
......@@ -7,3 +7,5 @@ RSpec::Matchers.define :have_vulnerability_dismissal_feedback do
project_fingerprint: finding.project_fingerprint)
end
end
RSpec::Matchers.define_negated_matcher :not_have_vulnerability_dismissal_feedback, :have_vulnerability_dismissal_feedback
......@@ -30124,6 +30124,9 @@ msgstr ""
msgid "failed to dismiss associated finding(id=%{finding_id}): %{message}"
msgstr ""
msgid "failed to revert associated finding(id=%{finding_id}) to detected"
msgstr ""
msgid "file"
msgid_plural "files"
msgstr[0] ""
......
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