Commit 3a3e04be authored by Michał Zając's avatar Michał Zając Committed by Kerri Miller

Add vulnerabilityFindingDismiss GraphQL mutation

* Add Vulnerabilities::FindingDismissService wrapping
  VulnerabilitiesFeedback::CreateService
* Add FindingPolicy

Changelog: added
EE: true
parent 72fa139f
...@@ -4931,6 +4931,27 @@ Input type: `VulnerabilityExternalIssueLinkDestroyInput` ...@@ -4931,6 +4931,27 @@ Input type: `VulnerabilityExternalIssueLinkDestroyInput`
| <a id="mutationvulnerabilityexternalissuelinkdestroyclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | | <a id="mutationvulnerabilityexternalissuelinkdestroyclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
| <a id="mutationvulnerabilityexternalissuelinkdestroyerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | | <a id="mutationvulnerabilityexternalissuelinkdestroyerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
### `Mutation.vulnerabilityFindingDismiss`
Input type: `VulnerabilityFindingDismissInput`
#### Arguments
| Name | Type | Description |
| ---- | ---- | ----------- |
| <a id="mutationvulnerabilityfindingdismissclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
| <a id="mutationvulnerabilityfindingdismisscomment"></a>`comment` | [`String`](#string) | Comment why finding should be dismissed. |
| <a id="mutationvulnerabilityfindingdismissdismissalreason"></a>`dismissalReason` | [`VulnerabilityDismissalReason`](#vulnerabilitydismissalreason) | Reason why finding should be dismissed. |
| <a id="mutationvulnerabilityfindingdismissid"></a>`id` | [`VulnerabilitiesFindingID!`](#vulnerabilitiesfindingid) | ID of the finding to be dismissed. |
#### Fields
| Name | Type | Description |
| ---- | ---- | ----------- |
| <a id="mutationvulnerabilityfindingdismissclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
| <a id="mutationvulnerabilityfindingdismisserrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
| <a id="mutationvulnerabilityfindingdismissfinding"></a>`finding` | [`PipelineSecurityReportFinding`](#pipelinesecurityreportfinding) | Finding after dismissal. |
### `Mutation.vulnerabilityResolve` ### `Mutation.vulnerabilityResolve`
Input type: `VulnerabilityResolveInput` Input type: `VulnerabilityResolveInput`
...@@ -18181,6 +18202,12 @@ A `VulnerabilitiesExternalIssueLinkID` is a global ID. It is encoded as a string ...@@ -18181,6 +18202,12 @@ A `VulnerabilitiesExternalIssueLinkID` is a global ID. It is encoded as a string
An example `VulnerabilitiesExternalIssueLinkID` is: `"gid://gitlab/Vulnerabilities::ExternalIssueLink/1"`. An example `VulnerabilitiesExternalIssueLinkID` is: `"gid://gitlab/Vulnerabilities::ExternalIssueLink/1"`.
### `VulnerabilitiesFindingID`
A `VulnerabilitiesFindingID` is a global ID. It is encoded as a string.
An example `VulnerabilitiesFindingID` is: `"gid://gitlab/Vulnerabilities::Finding/1"`.
### `VulnerabilitiesScannerID` ### `VulnerabilitiesScannerID`
A `VulnerabilitiesScannerID` is a global ID. It is encoded as a string. A `VulnerabilitiesScannerID` is a global ID. It is encoded as a string.
...@@ -31,6 +31,7 @@ module EE ...@@ -31,6 +31,7 @@ module EE
mount_mutation ::Mutations::RequirementsManagement::UpdateRequirement mount_mutation ::Mutations::RequirementsManagement::UpdateRequirement
mount_mutation ::Mutations::Vulnerabilities::Create mount_mutation ::Mutations::Vulnerabilities::Create
mount_mutation ::Mutations::Vulnerabilities::Dismiss mount_mutation ::Mutations::Vulnerabilities::Dismiss
mount_mutation ::Mutations::Vulnerabilities::FindingDismiss
mount_mutation ::Mutations::Vulnerabilities::Resolve mount_mutation ::Mutations::Vulnerabilities::Resolve
mount_mutation ::Mutations::Vulnerabilities::Confirm mount_mutation ::Mutations::Vulnerabilities::Confirm
mount_mutation ::Mutations::Vulnerabilities::RevertToDetected mount_mutation ::Mutations::Vulnerabilities::RevertToDetected
......
# frozen_string_literal: true
module Mutations
module Vulnerabilities
class FindingDismiss < BaseMutation
graphql_name 'VulnerabilityFindingDismiss'
authorize :admin_vulnerability
field :finding, Types::PipelineSecurityReportFindingType,
null: true,
description: 'Finding after dismissal.'
argument :id,
::Types::GlobalIDType[::Vulnerabilities::Finding],
required: true,
description: 'ID of the finding to be dismissed.'
argument :comment,
GraphQL::Types::String,
required: false,
description: 'Comment why finding should be dismissed.'
argument :dismissal_reason,
Types::Vulnerabilities::DismissalReasonEnum,
required: false,
description: 'Reason why finding should be dismissed.'
def resolve(id:, comment: nil, dismissal_reason: nil)
finding = authorized_find!(id: id)
result = dismiss_finding(finding, comment, dismissal_reason)
{
finding: result.success? ? result.payload[:finding] : nil,
errors: result.message || []
}
end
private
def dismiss_finding(finding, comment, dismissal_reason)
::Vulnerabilities::FindingDismissService.new(
current_user,
finding,
comment,
dismissal_reason
).execute
end
def find_object(id:)
id = ::Types::GlobalIDType[::Vulnerabilities::Finding].coerce_isolated_input(id)
GitlabSchema.find_by_gid(id)
end
end
end
end
# frozen_string_literal: true
module Vulnerabilities
class FindingPolicy < BasePolicy
delegate { @subject.project }
rule { ~can?(:read_security_resource) }.prevent :create_note
end
end
# frozen_string_literal: true
module Vulnerabilities
class FindingDismissService < BaseProjectService
include Gitlab::Allowable
def initialize(user, finding, comment = nil, dismissal_reason = nil)
super(project: finding.project, current_user: user)
@finding = finding
@comment = comment
@dismissal_reason = dismissal_reason
end
def execute
return ServiceResponse.error(message: "Access denied", http_status: :forbidden) unless authorized?
dismiss_finding
end
private
def authorized?
can?(@current_user, :admin_vulnerability, @project)
end
def dismiss_finding
result = ::VulnerabilityFeedback::CreateService.new(
@project,
@current_user,
feedback_params_for(@finding, @comment, @dismissal_reason)
).execute
case result[:status]
when :success
ServiceResponse.success(payload: { finding: @finding })
when :error
all_errors = result[:message].full_messages.join(",")
error_string = _("failed to dismiss finding: %{message}") % { message: all_errors }
ServiceResponse.error(message: error_string, http_status: :unprocessable_entity)
end
end
def feedback_params_for(finding, comment, dismissal_reason)
{
category: @finding.report_type,
feedback_type: 'dismissal',
project_fingerprint: @finding.project_fingerprint,
comment: @comment,
dismissal_reason: @dismissal_reason,
pipeline: @project.latest_pipeline_with_security_reports(only_successful: true),
finding_uuid: @finding.uuid_v5,
dismiss_vulnerability: false
}
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Mutations::Vulnerabilities::FindingDismiss do
include GraphqlHelpers
let(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
describe '#resolve' do
let_it_be(:finding) { create(:vulnerabilities_finding) }
let_it_be(:user) { create(:user) }
let_it_be(:finding_id) { GitlabSchema.id_from_object(finding).to_s }
let(:comment) { 'Dismissal Feedback' }
let(:mutated_finding) { subject[:finding] }
subject { mutation.resolve(id: finding_id, comment: comment, dismissal_reason: 'used_in_tests') }
context 'when the user can dismiss the finding' do
before do
stub_licensed_features(security_dashboard: true)
end
context 'when user does not have access to the project' do
it 'raises an error' do
expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
end
context 'with invalid params' do
let(:finding_id) { global_id_of(user) }
it 'raises an error' do
expect { subject }.to raise_error(::GraphQL::CoercionError)
end
end
context 'when user has access to the project' do
before do
finding.project.add_developer(user)
end
it 'returns the dismissed finding' do
expect(mutated_finding).to eq(finding)
expect(mutated_finding.state).to eq('dismissed')
expect(subject[:errors]).to be_empty
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'Dismissing a Vulnerabilities::Finding object' do
include GraphqlHelpers
let_it_be(:current_user) { create(:user) }
let_it_be(:project) { create(:project) }
let_it_be(:pipeline) { create(:ci_pipeline, :success, project: project) }
let_it_be(:finding) { create(:vulnerabilities_finding, :with_pipeline, project: project, severity: :high) }
let(:params) do
{
id: finding.to_global_id.to_s
}
end
let(:mutation) do
graphql_mutation(
:vulnerability_finding_dismiss,
params
)
end
let(:mutation_response) do
graphql_mutation_response(:vulnerability_finding_dismiss)
end
subject { post_graphql_mutation(mutation, current_user: current_user) }
context 'when the user does not have permission' do
before do
stub_licensed_features(security_dashboard: true)
end
it_behaves_like 'a mutation that returns a top-level access error'
it 'does not dismiss the Finding' do
expect { subject }.not_to change(finding, :state)
end
end
context 'when the user has permission' do
before do
finding.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 Finding' do
expect { subject }.to change(finding, :state).from('detected').to('dismissed')
end
context 'when comment is given' do
let(:comment) { "Used in tests" }
let(:params) do
{
id: finding.to_global_id.to_s,
comment: comment
}
end
let(:feedback) { finding.dismissal_feedback }
it 'saves the comment' do
expect { subject }.to change(finding, :state).from('detected').to('dismissed')
expect(feedback.comment).to eq(comment)
end
end
context 'when dismissal reason is given' do
let(:dismissal_reason) { "USED_IN_TESTS" }
let(:params) do
{
id: finding.to_global_id.to_s,
dismissal_reason: dismissal_reason
}
end
let(:feedback) { finding.dismissal_feedback }
it 'saves the dismissal reason' do
expect { subject }.to change(finding, :state).from('detected').to('dismissed')
expect(feedback.dismissal_reason).to eq('used_in_tests')
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Vulnerabilities::FindingDismissService 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!(:pipeline) { create(:ci_pipeline, :success, project: project) }
let!(:build) { create(:ee_ci_build, :sast, pipeline: pipeline) }
let!(:finding) { create(:vulnerabilities_finding, project: project) }
let(:service) { described_class.new(user, finding) }
subject(:dismiss_finding) { service.execute }
context 'with an authorized user with proper permissions' do
before do
project.add_developer(user)
end
context 'when comment is added' do
let(:comment) { 'Dismissal Comment' }
let(:service) { described_class.new(user, finding, comment) }
it 'dismisses a finding with comment', :aggregate_failures do
freeze_time do
dismiss_finding
aggregate_failures do
expect(finding.reload).to(have_attributes(state: 'dismissed'))
expect(finding.dismissal_feedback).to have_attributes(comment: comment, comment_author: user, comment_timestamp: be_like_time(Time.current), pipeline_id: pipeline.id)
end
end
end
end
context 'when the dismissal_reason is added' do
let(:dismissal_reason) { 'used_in_tests' }
let(:service) { described_class.new(user, finding, nil, dismissal_reason) }
it 'dismisses a finding', :aggregate_failures do
dismiss_finding
expect(finding.reload).to have_attributes(state: 'dismissed')
expect(finding.dismissal_feedback).to have_attributes(dismissal_reason: dismissal_reason)
end
end
context 'when Vulnerabilities::Feedback creation fails' do
let(:create_service_double) { instance_double("VulnerabilityFeedback::CreateService", execute: service_failure_payload) }
let(:service_failure_payload) do
{
status: :error,
message: errors_double
}
end
let(:errors_double) { instance_double("ActiveModel::Errors", full_messages: error_messages_array) }
let(:error_messages_array) { instance_double("Array", join: "something went wrong") }
before do
allow(VulnerabilityFeedback::CreateService).to receive(:new).and_return(create_service_double)
end
it 'returns the error' do
expect(create_service_double).to receive(:execute).once
result = dismiss_finding
expect(result).not_to be_success
expect(result.http_status).to be(:unprocessable_entity)
expect(result.message).to eq("failed to dismiss finding: something went wrong")
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
result = dismiss_finding
expect(result).not_to be_success
expect(result.http_status).to be(:forbidden)
expect(result.message).to eq("Access denied")
end
end
end
end
...@@ -42040,6 +42040,9 @@ msgstr "" ...@@ -42040,6 +42040,9 @@ msgstr ""
msgid "failed to dismiss associated finding(id=%{finding_id}): %{message}" msgid "failed to dismiss associated finding(id=%{finding_id}): %{message}"
msgstr "" msgstr ""
msgid "failed to dismiss finding: %{message}"
msgstr ""
msgid "failed to revert associated finding(id=%{finding_id}) to detected" msgid "failed to revert associated finding(id=%{finding_id}) to detected"
msgstr "" msgstr ""
......
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