Merge branch...

Merge branch '332066-be-bulk-status-change-for-findings-in-the-pipeline-security-dashboard' into 'master'

Add vulnerabilityFindingDismiss GraphQL mutation

See merge request gitlab-org/gitlab!77372

doc/api/graphql/reference/index.md

@@ -4935,6 +4935,27 @@ Input type: `VulnerabilityExternalIssueLinkDestroyInput`
 | <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. |
 
+### `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`
 
 Input type: `VulnerabilityResolveInput`
@@ -18186,6 +18207,12 @@ A `VulnerabilitiesExternalIssueLinkID` is a global ID. It is encoded as a string
 
 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`
 
 A `VulnerabilitiesScannerID` is a global ID. It is encoded as a string.

ee/app/graphql/ee/types/mutation_type.rb

@@ -31,6 +31,7 @@ module EE
         mount_mutation ::Mutations::RequirementsManagement::UpdateRequirement
         mount_mutation ::Mutations::Vulnerabilities::Create
         mount_mutation ::Mutations::Vulnerabilities::Dismiss
+        mount_mutation ::Mutations::Vulnerabilities::FindingDismiss
         mount_mutation ::Mutations::Vulnerabilities::Resolve
         mount_mutation ::Mutations::Vulnerabilities::Confirm
         mount_mutation ::Mutations::Vulnerabilities::RevertToDetected

ee/app/graphql/mutations/vulnerabilities/finding_dismiss.rb

+# 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

ee/app/policies/vulnerabilities/finding_policy.rb

+# frozen_string_literal: true
+module Vulnerabilities
+  class FindingPolicy < BasePolicy
+    delegate { @subject.project }
+
+    rule { ~can?(:read_security_resource) }.prevent :create_note
+  end
+end

ee/app/services/vulnerabilities/finding_dismiss_service.rb

+# 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

ee/spec/graphql/mutations/vulnerabilities/finding_dismiss_spec.rb

+# 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

ee/spec/requests/api/graphql/mutations/vulnerabilities/finding_dismiss_spec.rb

+# 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

ee/spec/services/vulnerabilities/finding_dismiss_service_spec.rb

+# 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

locale/gitlab.pot

@@ -42052,6 +42052,9 @@ msgstr ""
 msgid "failed to dismiss associated finding(id=%{finding_id}): %{message}"
 msgstr ""
 
+msgid "failed to dismiss finding: %{message}"
+msgstr ""
+
 msgid "failed to revert associated finding(id=%{finding_id}) to detected"
 msgstr ""