Merge branch '34564-delete-vulnerability-issue-link' into 'master'

Delete a Vulnerability-Issue link API call

See merge request gitlab-org/gitlab!20545

ee/app/services/vulnerability_issue_links/delete_service.rb

+# frozen_string_literal: true
+
+module VulnerabilityIssueLinks
+  class DeleteService < BaseService
+    def initialize(user, vulnerability_issue_link)
+      @user = user
+      @link = vulnerability_issue_link
+    end
+
+    def execute
+      raise Gitlab::Access::AccessDeniedError unless can?(user, :admin_vulnerability_issue_link, link)
+
+      link.destroy!
+
+      success
+    end
+
+    private
+
+    attr_reader :user, :link
+
+    def success
+      ServiceResponse.success(payload: { record: link }, http_status: 200)
+    end
+  end
+end

ee/lib/api/helpers/vulnerabilities_helpers.rb

@@ -5,8 +5,12 @@ module API
     module VulnerabilitiesHelpers
       def find_and_authorize_vulnerability!(action)
         find_vulnerability!.tap do |vulnerability|
-          authorize! action, vulnerability.project
+          authorize_vulnerability!(vulnerability, action)
+        end
       end
+
+      def authorize_vulnerability!(vulnerability, action)
+        authorize! action, vulnerability.project
       end
     end
   end

ee/lib/api/vulnerabilities.rb

@@ -16,10 +16,6 @@ module API
         Vulnerability.with_findings.find(params[:id])
       end
 
-      def authorize_vulnerability!(vulnerability, action)
-        authorize! action, vulnerability.project
-      end
-
       def render_vulnerability(vulnerability)
         if vulnerability.valid?
           present vulnerability, with: EE::API::Entities::Vulnerability

ee/lib/api/vulnerability_issue_links.rb

@@ -18,6 +18,10 @@ module API
           render_api_error!(response.message, response.http_status)
         end
       end
+
+      def find_issue_link!
+        ::Vulnerabilities::IssueLink.find(params[:issue_link_id])
+      end
     end
 
     params do
@@ -53,6 +57,21 @@ module API
 
         render_issue_link_response(response)
       end
+
+      desc 'Delete a link between an issue and a vulnerability' do
+        success EE::API::Entities::VulnerabilityIssueLink
+      end
+      params do
+        requires :issue_link_id, type: Integer, desc: 'The ID of a vulnerability-issue-link to delete'
+      end
+      delete ':id/issue_links/:issue_link_id' do
+        find_and_authorize_vulnerability!(:admin_vulnerability_issue_link)
+        issue_link = find_issue_link!
+
+        service_response = ::VulnerabilityIssueLinks::DeleteService.new(current_user, issue_link).execute
+
+        render_issue_link_response(service_response)
+      end
     end
   end
 end

ee/spec/factories/vulnerabilities/issue_links.rb

@@ -12,5 +12,16 @@ FactoryBot.define do
     trait :related do
       link_type { :related }
     end
+
+    transient do
+      project { nil }
+    end
+
+    after(:build) do |link, evaluator|
+      if evaluator.project
+        link.vulnerability = create(:vulnerability, project: evaluator.project)
+        link.issue = create(:issue, project: evaluator.project)
+      end
+    end
   end
 end

ee/spec/policies/vulnerabilities/issue_link_policy_spec.rb

@@ -6,7 +6,7 @@ describe Vulnerabilities::IssueLinkPolicy do
   let_it_be(:user) { create(:user) }
   let_it_be(:project) { create(:project, namespace: user.namespace) }
   let(:vulnerability) { create(:vulnerability, project: project) }
-  let(:issue) { create(:issue, project: vulnerability.project) }
+  let(:issue) { create(:issue, project: project) }
   let(:vulnerability_issue_link) { build(:vulnerabilities_issue_link, vulnerability: vulnerability, issue: issue) }
 
   subject { described_class.new(user, vulnerability_issue_link) }
@@ -31,11 +31,11 @@ describe Vulnerabilities::IssueLinkPolicy do
       it { is_expected.to be_disallowed(:admin_vulnerability_issue_link) }
     end
 
-    context "when an issue to link to belongs to vulnerability's project" do
+    context 'when issue and link belong to the same project' do
       it { is_expected.to be_allowed(:admin_vulnerability_issue_link) }
     end
 
-    context "when an issue to link to doesn't belong to vulnerability's project" do
+    context "when issue and link don't belong to the same project" do
       let(:issue) { create(:issue) }
 
       it { is_expected.to be_disallowed(:admin_vulnerability_issue_link) }

ee/spec/requests/api/vulnerability_issue_links_spec.rb

@@ -145,4 +145,56 @@ describe API::VulnerabilityIssueLinks do
       it { expect { create_issue_link }.to be_denied_for(:anonymous) }
     end
   end
+
+  describe 'DELETE /vulnerabilities/:id/issue_links/:issue_link_id' do
+    let_it_be(:vulnerability_issue_link) { create(:vulnerabilities_issue_link, project: project) }
+    let_it_be(:vulnerability_id) { vulnerability_issue_link.vulnerability.id }
+    let_it_be(:issue_link_id) { vulnerability_issue_link.id }
+
+    subject(:delete_issue_link) do
+      delete api("/vulnerabilities/#{vulnerability_id}/issue_links/#{issue_link_id}", user)
+    end
+
+    context 'with an authorized user with proper permissions' do
+      before do
+        project.add_developer(user)
+      end
+
+      context 'with valid params' do
+        it 'deletes the specified vulnerability-issue link' do
+          delete_issue_link
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(response).to match_response_schema('public_api/v4/vulnerability_issue_link', dir: 'ee')
+          expect(json_response['issue']['id']).to eq vulnerability_issue_link.issue.id
+          expect(json_response['vulnerability']['id']).to eq vulnerability_id
+        end
+      end
+
+      context 'with unknown issue link ID' do
+        let(:issue_link_id) { 0 }
+
+        it 'responds with "not found" and specific error message' do
+          delete_issue_link
+
+          expect(response).to have_gitlab_http_status(404)
+        end
+      end
+
+      it_behaves_like 'responds with "not found" for an unknown vulnerability ID'
+      it_behaves_like 'forbids access to vulnerability API endpoint in case of disabled features'
+    end
+
+    describe 'permissions' do
+      it { expect { delete_issue_link }.to be_allowed_for(:admin) }
+      it { expect { delete_issue_link }.to be_allowed_for(:owner).of(project) }
+      it { expect { delete_issue_link }.to be_allowed_for(:maintainer).of(project) }
+      it { expect { delete_issue_link }.to be_allowed_for(:developer).of(project) }
+
+      it { expect { delete_issue_link }.to be_denied_for(:auditor) }
+      it { expect { delete_issue_link }.to be_denied_for(:reporter).of(project) }
+      it { expect { delete_issue_link }.to be_denied_for(:guest).of(project) }
+      it { expect { delete_issue_link }.to be_denied_for(:anonymous) }
+    end
+  end
 end

ee/spec/services/vulnerability_issue_links/delete_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe VulnerabilityIssueLinks::DeleteService do
+  include AccessMatchersGeneric
+
+  before do
+    stub_licensed_features(security_dashboard: true)
+  end
+
+  let_it_be(:project) { create(:project) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:vulnerability_issue_link, refind: true) { create(:vulnerabilities_issue_link, project: project) }
+  let(:service) { described_class.new(user, vulnerability_issue_link) }
+
+  subject(:delete_issue_link) { service.execute }
+
+  context 'with an authorized user with proper permissions' do
+    before do
+      project.add_developer(user)
+    end
+
+    context 'with valid params' do
+      it 'deletes the specified vulnerability-issue link' do
+        expect { delete_issue_link }.to change { Vulnerabilities::IssueLink.count }.by(-1)
+
+        response = delete_issue_link
+        expect(response).to be_success
+        expect(response.http_status).to eq 200
+
+        issue_link = response.payload[:record]
+        expect(issue_link).to have_attributes(vulnerability_issue_link.attributes)
+      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 { delete_issue_link }.to raise_error(Gitlab::Access::AccessDeniedError)
+      end
+    end
+  end
+
+  describe 'permissions' do
+    it { expect { delete_issue_link }.to be_allowed_for(:admin) }
+    it { expect { delete_issue_link }.to be_allowed_for(:owner).of(project) }
+    it { expect { delete_issue_link }.to be_allowed_for(:maintainer).of(project) }
+    it { expect { delete_issue_link }.to be_allowed_for(:developer).of(project) }
+
+    it { expect { delete_issue_link }.to be_denied_for(:auditor) }
+    it { expect { delete_issue_link }.to be_denied_for(:reporter).of(project) }
+    it { expect { delete_issue_link }.to be_denied_for(:guest).of(project) }
+    it { expect { delete_issue_link }.to be_denied_for(:anonymous) }
+  end
+end