Merge branch '34564-relate-issue-to-vulnerability' into 'master'

Create a Vulnerability-Issue link (API endpoint)

See merge request gitlab-org/gitlab!20485

ee/app/models/vulnerabilities/issue_link.rb

@@ -10,5 +10,12 @@ module Vulnerabilities
     enum link_type: { related: 1, created: 2 } # 'related' is the default value
 
     validates :vulnerability, :issue, presence: true
+    validates :issue_id, uniqueness: { scope: :vulnerability_id, message: N_('has already been linked to another vulnerability') }
+    validates :vulnerability_id,
+              uniqueness: {
+                conditions: -> { where(link_type: 'created') },
+                message: N_('already has a "created" issue link')
+              },
+              if: :created?
   end
 end

ee/app/policies/ee/project_policy.rb

@@ -165,6 +165,7 @@ module EE
         enable :read_project_security_dashboard
         enable :create_vulnerability
         enable :admin_vulnerability
+        enable :admin_vulnerability_issue_link
       end
 
       rule { threat_monitoring_enabled & (auditor | can?(:developer_access)) }.enable :read_threat_monitoring
@@ -219,6 +220,7 @@ module EE
       rule { auditor & ~developer }.policy do
         prevent :create_vulnerability
         prevent :admin_vulnerability
+        prevent :admin_vulnerability_issue_link
       end
 
       rule { auditor & ~guest }.policy do

ee/app/policies/vulnerabilities/issue_link_policy.rb

+# frozen_string_literal: true
+
+module Vulnerabilities
+  class IssueLinkPolicy < BasePolicy
+    delegate { @subject.vulnerability&.project }
+
+    with_scope :subject
+    condition(:cross_project_issue) { @subject.vulnerability&.project != @subject.issue&.project }
+
+    rule { cross_project_issue }.prevent :admin_vulnerability_issue_link
+  end
+end

ee/app/services/vulnerability_issue_links/create_service.rb

+# frozen_string_literal: true
+
+module VulnerabilityIssueLinks
+  class CreateService < BaseService
+    def initialize(user, vulnerability, issue, link_type: Vulnerabilities::IssueLink.link_types[:related])
+      @user = user
+      @vulnerability = vulnerability
+      @issue = issue
+      @link_type = link_type
+    end
+
+    def execute
+      raise Gitlab::Access::AccessDeniedError unless can?(@user, :admin_vulnerability_issue_link, issue_link)
+
+      if issue_link.save
+        success
+      else
+        error
+      end
+    end
+
+    private
+
+    def issue_link
+      @issue_link ||= Vulnerabilities::IssueLink.new(vulnerability: @vulnerability, issue: @issue, link_type: @link_type)
+    end
+
+    def success
+      ServiceResponse.success(payload: result_payload, http_status: 200)
+    end
+
+    def error
+      ServiceResponse.error(
+        message: issue_link.errors.full_messages.to_sentence,
+        payload: result_payload,
+        http_status: 422)
+    end
+
+    def result_payload
+      { record: issue_link }
+    end
+  end
+end

ee/lib/api/vulnerability_issue_links.rb

@@ -10,6 +10,14 @@ module API
       def find_vulnerability!
         Vulnerability.find(params[:id])
       end
+
+      def render_issue_link_response(response)
+        if response.success?
+          present(response.payload[:record], with: EE::API::Entities::VulnerabilityIssueLink)
+        else
+          render_api_error!(response.message, response.http_status)
+        end
+      end
     end
 
     params do
@@ -28,6 +36,23 @@ module API
                   .with_vulnerability_links,
                 with: EE::API::Entities::VulnerabilityRelatedIssue
       end
+
+      desc 'Relate an issue to a vulnerability' do
+        success EE::API::Entities::VulnerabilityIssueLink
+      end
+      params do
+        requires :target_issue_iid, type: Integer, desc: 'The IID of an issue to relate to'
+        optional :link_type, type: String, default: 'related', desc: 'Link type'
+      end
+      post ':id/issue_links' do
+        vulnerability = find_and_authorize_vulnerability!(:admin_vulnerability_issue_link)
+        issue = find_project_issue(params[:target_issue_iid], vulnerability.project_id)
+
+        response = ::VulnerabilityIssueLinks::CreateService.new(
+          current_user, vulnerability, issue, link_type: params[:link_type]).execute
+
+        render_issue_link_response(response)
+      end
     end
   end
 end

ee/lib/ee/api/entities.rb

@@ -982,6 +982,12 @@ module EE
           ::Vulnerabilities::IssueLink.link_types.key(related_issue.vulnerability_link_type)
         end
       end
+
+      class VulnerabilityIssueLink < Grape::Entity
+        expose :vulnerability, using: ::EE::API::Entities::Vulnerability
+        expose :issue, using: ::API::Entities::IssueBasic
+        expose :link_type
+      end
     end
   end
 end

ee/spec/fixtures/api/schemas/public_api/v4/vulnerability_issue_link.json

+{
+  "type": "object",
+  "additionalProperties": false,
+  "required": [
+    "issue",
+    "vulnerability",
+    "link_type"
+  ],
+  "properties": {
+    "issue": {
+      "oneOf": [
+        { "$ref": "../../../../../../../spec/fixtures/api/schemas/public_api/v4/issue.json" }
+      ]
+    },
+    "vulnerability": {
+      "oneOf": [
+        { "$ref": "vulnerability.json" }
+      ]
+    },
+    "link_type": { "type": "string" }
+  }
+}

ee/spec/models/vulnerabilities/issue_link_spec.rb

@@ -16,39 +16,66 @@ describe Vulnerabilities::IssueLink do
   describe 'validations' do
     it { is_expected.to validate_presence_of(:vulnerability) }
     it { is_expected.to validate_presence_of(:issue) }
-  end
 
-  context 'when there is a link between the same vulnerability and issue' do
-    let!(:existing_link) { create(:vulnerabilities_issue_link) }
+    describe 'uniqueness' do
+      before do
+        create(:vulnerabilities_issue_link)
+      end
 
-    it 'raises the uniqueness violation error' do
-      expect do
-        create(:vulnerabilities_issue_link,
-          issue: existing_link.issue,
-          vulnerability: existing_link.vulnerability)
-      end.to raise_error(ActiveRecord::RecordNotUnique)
+      it do
+        is_expected.to(
+          validate_uniqueness_of(:issue_id)
+            .scoped_to(:vulnerability_id)
+            .with_message('has already been linked to another vulnerability'))
+      end
+    end
+
+    describe 'only one "created" link allowed per vulnerability' do
+      let!(:existing_link) { create(:vulnerabilities_issue_link, :created) }
+
+      subject(:issue_link) do
+        build(:vulnerabilities_issue_link, :created, vulnerability: existing_link.vulnerability)
+      end
+
+      it do
+        is_expected.to(
+          validate_uniqueness_of(:vulnerability_id)
+            .with_message('already has a "created" issue link'))
+      end
     end
   end
 
-  context 'when there is an existing "created" issue link for vulnerability' do
-    let!(:existing_link) { create(:vulnerabilities_issue_link, :created) }
+  describe 'data consistency constraints' do
+    context 'when a link between the same vulnerability and issue already exists' do
+      let!(:existing_link) { create(:vulnerabilities_issue_link) }
 
-    it 'prevents the creation of a new "created" issue link' do
-      expect do
-        create(:vulnerabilities_issue_link,
-               :created,
-               vulnerability: existing_link.vulnerability,
-               issue: create(:issue))
-      end.to raise_error(ActiveRecord::RecordNotUnique)
+      it 'raises the uniqueness violation error' do
+        expect do
+          issue_link = build(
+            :vulnerabilities_issue_link,
+            issue_id: existing_link.issue_id,
+            vulnerability_id: existing_link.vulnerability_id)
+          issue_link.save(validate: false)
+        end.to raise_error(ActiveRecord::RecordNotUnique)
+      end
     end
 
-    it 'allows the creation of a new "related" issue link' do
-      expect do
-        create(:vulnerabilities_issue_link,
-               :related,
-               vulnerability: existing_link.vulnerability,
-               issue: create(:issue))
-      end.not_to raise_error
+    context 'when there is an existing "created" issue link for vulnerability' do
+      let!(:existing_link) { create(:vulnerabilities_issue_link, :created) }
+
+      it 'prevents the creation of a new "created" issue link' do
+        expect do
+          issue_link = build(:vulnerabilities_issue_link, :created, vulnerability: existing_link.vulnerability)
+          issue_link.save(validate: false)
+        end.to raise_error(ActiveRecord::RecordNotUnique)
+      end
+
+      it 'allows the creation of a new "related" issue link' do
+        expect do
+          issue_link = build(:vulnerabilities_issue_link, :related, vulnerability: existing_link.vulnerability)
+          issue_link.save(validate: false)
+        end.not_to raise_error
+      end
     end
   end
 end

ee/spec/policies/project_policy_spec.rb

@@ -36,6 +36,7 @@ describe ProjectPolicy do
       %i[
         admin_vulnerability_feedback read_project_security_dashboard read_feature_flag
         read_vulnerability create_vulnerability admin_vulnerability
+        admin_vulnerability_issue_link
       ]
     end
     let(:additional_maintainer_permissions) { %i[push_code_to_protected_branches admin_feature_flags_client] }

ee/spec/policies/vulnerabilities/issue_link_policy_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+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(:vulnerability_issue_link) { build(:vulnerabilities_issue_link, vulnerability: vulnerability, issue: issue) }
+
+  subject { described_class.new(user, vulnerability_issue_link) }
+
+  context 'with a user authorized to admin vulnerability-issue links' do
+    before do
+      stub_licensed_features(security_dashboard: true)
+
+      project.add_developer(user)
+    end
+
+    context 'with missing vulnerability' do
+      let(:vulnerability) { nil }
+      let(:issue) { create(:issue) }
+
+      it { is_expected.to be_disallowed(:admin_vulnerability_issue_link) }
+    end
+
+    context 'with missing issue' do
+      let(:issue) { nil }
+
+      it { is_expected.to be_disallowed(:admin_vulnerability_issue_link) }
+    end
+
+    context "when an issue to link to belongs to vulnerability's 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
+      let(:issue) { create(:issue) }
+
+      it { is_expected.to be_disallowed(:admin_vulnerability_issue_link) }
+    end
+  end
+end

ee/spec/requests/api/vulnerabilities_spec.rb

@@ -42,7 +42,7 @@ describe API::Vulnerabilities do
         end
       end
 
-      it_behaves_like 'forbids actions on vulnerability in case of disabled features'
+      it_behaves_like 'forbids access to vulnerability API endpoint in case of disabled features'
     end
 
     describe 'permissions' do
@@ -88,7 +88,7 @@ describe API::Vulnerabilities do
       end
 
       it_behaves_like 'responds with "not found" for an unknown vulnerability ID'
-      it_behaves_like 'forbids actions on vulnerability in case of disabled features'
+      it_behaves_like 'forbids access to vulnerability API endpoint in case of disabled features'
     end
 
     describe 'permissions' do
@@ -159,7 +159,7 @@ describe API::Vulnerabilities do
         end
       end
 
-      it_behaves_like 'forbids actions on vulnerability in case of disabled features'
+      it_behaves_like 'forbids access to vulnerability API endpoint in case of disabled features'
     end
 
     describe 'permissions' do
@@ -246,7 +246,7 @@ describe API::Vulnerabilities do
         end
       end
 
-      it_behaves_like 'forbids actions on vulnerability in case of disabled features'
+      it_behaves_like 'forbids access to vulnerability API endpoint in case of disabled features'
     end
 
     describe 'permissions' do
@@ -303,7 +303,7 @@ describe API::Vulnerabilities do
         end
       end
 
-      it_behaves_like 'forbids actions on vulnerability in case of disabled features'
+      it_behaves_like 'forbids access to vulnerability API endpoint in case of disabled features'
     end
 
     describe 'permissions' do

ee/spec/requests/api/vulnerability_issue_links_spec.rb

@@ -37,7 +37,7 @@ describe API::VulnerabilityIssueLinks do
 
       it_behaves_like 'responds with "not found" for an unknown vulnerability ID'
 
-      it_behaves_like 'forbids actions on vulnerability in case of disabled features'
+      it_behaves_like 'forbids access to vulnerability API endpoint in case of disabled features'
     end
 
     describe 'permissions' do
@@ -52,4 +52,97 @@ describe API::VulnerabilityIssueLinks do
       it { expect { get_issue_links }.to be_denied_for(:anonymous) }
     end
   end
+
+  describe 'POST /vulnerabilities/:id/issue_links' do
+    let_it_be(:issue) { create(:issue, project: project) }
+    let_it_be(:vulnerability) { create(:vulnerability, project: project) }
+    let(:vulnerability_id) { vulnerability.id }
+    let(:target_issue_iid) { issue.iid }
+    let(:params) { { target_issue_iid: target_issue_iid } }
+
+    subject(:create_issue_link) do
+      post api("/vulnerabilities/#{vulnerability_id}/issue_links", user), params: params
+    end
+
+    context 'with an authorized user with proper permissions' do
+      before do
+        project.add_developer(user)
+      end
+
+      context 'with valid params' do
+        it 'creates a new vulnerability-issue link' do
+          create_issue_link
+
+          expect(response).to have_gitlab_http_status(201)
+          expect(response).to match_response_schema('public_api/v4/vulnerability_issue_link', dir: 'ee')
+          expect(json_response['issue']['id']).to eq issue.id
+          expect(json_response['vulnerability']['id']).to eq vulnerability.id
+        end
+      end
+
+      context 'with unknown issue ID' do
+        let(:target_issue_iid) { 0 }
+
+        it 'responds with "not found" and specific error message' do
+          create_issue_link
+
+          expect(response).to have_gitlab_http_status(404)
+        end
+      end
+
+      context 'when a link between these issue and vulnerability already exists' do
+        before do
+          create(:vulnerabilities_issue_link, vulnerability: vulnerability, issue: issue)
+        end
+
+        it 'responds with "conflict" status code and specific error message' do
+          create_issue_link
+
+          expect(response).to have_gitlab_http_status(422)
+          expect(json_response['message']).to eq 'Issue has already been linked to another vulnerability'
+        end
+      end
+
+      context 'when a "created" link for a vulnerability already exists' do
+        before do
+          create(:vulnerabilities_issue_link, vulnerability: vulnerability, issue: create(:issue), link_type: 'created')
+        end
+
+        let(:params) { super().merge(link_type: 'created') }
+
+        it 'responds with "conflict" status code and specific error message' do
+          create_issue_link
+
+          expect(response).to have_gitlab_http_status(422)
+          expect(json_response['message']).to eq 'Vulnerability already has a "created" issue link'
+        end
+      end
+
+      context 'when trying to relate a confidential issue of the same project' do
+        let(:issue) { create(:issue, :confidential, project: project) }
+
+        it 'creates a new vulnerability-issue link' do
+          create_issue_link
+
+          expect(response).to have_gitlab_http_status(201)
+        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 { create_issue_link }.to be_allowed_for(:admin) }
+      it { expect { create_issue_link }.to be_allowed_for(:owner).of(project) }
+      it { expect { create_issue_link }.to be_allowed_for(:maintainer).of(project) }
+      it { expect { create_issue_link }.to be_allowed_for(:developer).of(project) }
+
+      it { expect { create_issue_link }.to be_denied_for(:auditor) }
+      it { expect { create_issue_link }.to be_denied_for(:reporter).of(project) }
+      it { expect { create_issue_link }.to be_denied_for(:guest).of(project) }
+      it { expect { create_issue_link }.to be_denied_for(:anonymous) }
+    end
+  end
 end

ee/spec/services/vulnerability_issue_links/create_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe VulnerabilityIssueLinks::CreateService do
+  include AccessMatchersGeneric
+
+  before do
+    stub_licensed_features(security_dashboard: true)
+  end
+
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let(:vulnerability) { create(:vulnerability, project: project) }
+  let(:issue) { create(:issue, project: vulnerability.project) }
+  let(:service) { described_class.new(user, vulnerability, issue) }
+
+  subject(:create_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 'creates a new vulnerability-issue link' do
+        expect { create_issue_link }.to change { Vulnerabilities::IssueLink.count }.by(1)
+
+        response = create_issue_link
+        expect(response).to be_success
+
+        issue_link = response.payload[:record]
+        expect(issue_link).to be_persisted
+        expect(issue_link).to have_attributes(vulnerability: vulnerability, issue: issue, link_type: 'related')
+      end
+    end
+
+    context 'with missing vulnerability' do
+      let(:service) { described_class.new(user, nil, issue) }
+
+      it 'responds with an error' do
+        expect { create_issue_link }.to raise_error(Gitlab::Access::AccessDeniedError)
+      end
+    end
+
+    context 'with missing issue' do
+      let(:service) { described_class.new(user, vulnerability, nil) }
+
+      it 'responds with an error' do
+        expect { create_issue_link }.to raise_error(Gitlab::Access::AccessDeniedError)
+      end
+    end
+
+    context 'when a link between these issue and vulnerability already exists' do
+      before do
+        create(:vulnerabilities_issue_link, vulnerability: vulnerability, issue: issue)
+      end
+
+      it 'responds with an error about a conflicting data' do
+        expect { create_issue_link }.not_to change { Vulnerabilities::IssueLink.count }
+
+        response = create_issue_link
+
+        expect(response).to be_error
+        expect(response.http_status).to eq 422
+        expect(response.message).to eq 'Issue has already been linked to another vulnerability'
+      end
+    end
+
+    context 'when a "created" link already exists for a vulnerability' do
+      before do
+        create(:vulnerabilities_issue_link, vulnerability: vulnerability, issue: create(:issue), link_type: 'created')
+      end
+
+      let(:service) { described_class.new(user, vulnerability, issue, link_type: 'created') }
+
+      it 'responds with an error about a conflicting data' do
+        expect { create_issue_link }.not_to change { Vulnerabilities::IssueLink.count }
+
+        response = create_issue_link
+
+        expect(response).to be_error
+        expect(response.http_status).to eq 422
+        expect(response.message).to eq 'Vulnerability already has a "created" issue link'
+      end
+    end
+
+    context 'when trying to relate an issue of a different project' do
+      let(:issue) { create(:issue) }
+
+      it 'raises an access error' do
+        expect { create_issue_link }.to raise_error(Gitlab::Access::AccessDeniedError)
+      end
+    end
+
+    context 'when trying to relate a confidential issue of the same project' do
+      it 'creates a vulnerability-issue link' do
+        expect { create_issue_link }.to change { Vulnerabilities::IssueLink.count }.by(1)
+      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 { create_issue_link }.to raise_error(Gitlab::Access::AccessDeniedError)
+      end
+    end
+  end
+
+  describe 'permissions' do
+    it { expect { create_issue_link }.to be_allowed_for(:admin) }
+    it { expect { create_issue_link }.to be_allowed_for(:owner).of(project) }
+    it { expect { create_issue_link }.to be_allowed_for(:maintainer).of(project) }
+    it { expect { create_issue_link }.to be_allowed_for(:developer).of(project) }
+
+    it { expect { create_issue_link }.to be_denied_for(:auditor) }
+    it { expect { create_issue_link }.to be_denied_for(:reporter).of(project) }
+    it { expect { create_issue_link }.to be_denied_for(:guest).of(project) }
+    it { expect { create_issue_link }.to be_denied_for(:anonymous) }
+  end
+end

ee/spec/support/shared_examples/requests/api/vulnerabilities_shared_examples.rb

 # frozen_string_literal: true
 
-shared_examples 'forbids actions on vulnerability in case of disabled features' do
+shared_examples 'forbids access to vulnerability API endpoint in case of disabled features' do
   context 'when "first-class vulnerabilities" feature is disabled' do
     before do
       stub_feature_flags(first_class_vulnerabilities: false)

locale/gitlab.pot

@@ -21237,6 +21237,9 @@ msgstr ""
 msgid "already being used for another group or project milestone."
 msgstr ""
 
+msgid "already has a \"created\" issue link"
+msgstr ""
+
 msgid "already shared with this group"
 msgstr ""
 
@@ -21694,6 +21697,9 @@ msgstr ""
 msgid "group"
 msgstr ""
 
+msgid "has already been linked to another vulnerability"
+msgstr ""
+
 msgid "has already been taken"
 msgstr ""