Commit 096e5186 authored by Victor Zagorodny's avatar Victor Zagorodny Committed by Douglas Barbosa Alexandre

Create a Vulnerability-Issue link (API endpoint)

Add new validations to Vulnerabilities::IssueLink
Add Vulnerabilities::CreateService service class
Add VulnerabilityIssueLinks API w/ tests
Add VulnerabilityIssueLinks API endpoint.
Add tests for it and all required files
(JSON schema fixture etc.) Extract common
helpers and test examples between this
API and Vulnerabilities API into shared
files.
parent ab5fab5e
...@@ -10,5 +10,12 @@ module Vulnerabilities ...@@ -10,5 +10,12 @@ module Vulnerabilities
enum link_type: { related: 1, created: 2 } # 'related' is the default value enum link_type: { related: 1, created: 2 } # 'related' is the default value
validates :vulnerability, :issue, presence: true 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
end end
...@@ -165,6 +165,7 @@ module EE ...@@ -165,6 +165,7 @@ module EE
enable :read_project_security_dashboard enable :read_project_security_dashboard
enable :create_vulnerability enable :create_vulnerability
enable :admin_vulnerability enable :admin_vulnerability
enable :admin_vulnerability_issue_link
end end
rule { threat_monitoring_enabled & (auditor | can?(:developer_access)) }.enable :read_threat_monitoring rule { threat_monitoring_enabled & (auditor | can?(:developer_access)) }.enable :read_threat_monitoring
...@@ -219,6 +220,7 @@ module EE ...@@ -219,6 +220,7 @@ module EE
rule { auditor & ~developer }.policy do rule { auditor & ~developer }.policy do
prevent :create_vulnerability prevent :create_vulnerability
prevent :admin_vulnerability prevent :admin_vulnerability
prevent :admin_vulnerability_issue_link
end end
rule { auditor & ~guest }.policy do rule { auditor & ~guest }.policy do
......
# 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
# 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
...@@ -10,6 +10,14 @@ module API ...@@ -10,6 +10,14 @@ module API
def find_vulnerability! def find_vulnerability!
Vulnerability.find(params[:id]) Vulnerability.find(params[:id])
end 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 end
params do params do
...@@ -28,6 +36,23 @@ module API ...@@ -28,6 +36,23 @@ module API
.with_vulnerability_links, .with_vulnerability_links,
with: EE::API::Entities::VulnerabilityRelatedIssue with: EE::API::Entities::VulnerabilityRelatedIssue
end 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 end
end end
...@@ -982,6 +982,12 @@ module EE ...@@ -982,6 +982,12 @@ module EE
::Vulnerabilities::IssueLink.link_types.key(related_issue.vulnerability_link_type) ::Vulnerabilities::IssueLink.link_types.key(related_issue.vulnerability_link_type)
end end
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 end
end end
{
"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" }
}
}
...@@ -16,39 +16,66 @@ describe Vulnerabilities::IssueLink do ...@@ -16,39 +16,66 @@ describe Vulnerabilities::IssueLink do
describe 'validations' do describe 'validations' do
it { is_expected.to validate_presence_of(:vulnerability) } it { is_expected.to validate_presence_of(:vulnerability) }
it { is_expected.to validate_presence_of(:issue) } it { is_expected.to validate_presence_of(:issue) }
end
context 'when there is a link between the same vulnerability and issue' do describe 'uniqueness' do
let!(:existing_link) { create(:vulnerabilities_issue_link) } before do
create(:vulnerabilities_issue_link)
end
it 'raises the uniqueness violation error' do it do
expect do is_expected.to(
create(:vulnerabilities_issue_link, validate_uniqueness_of(:issue_id)
issue: existing_link.issue, .scoped_to(:vulnerability_id)
vulnerability: existing_link.vulnerability) .with_message('has already been linked to another vulnerability'))
end.to raise_error(ActiveRecord::RecordNotUnique) 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
end end
context 'when there is an existing "created" issue link for vulnerability' do describe 'data consistency constraints' do
let!(:existing_link) { create(:vulnerabilities_issue_link, :created) } 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 it 'raises the uniqueness violation error' do
expect do expect do
create(:vulnerabilities_issue_link, issue_link = build(
:created, :vulnerabilities_issue_link,
vulnerability: existing_link.vulnerability, issue_id: existing_link.issue_id,
issue: create(:issue)) vulnerability_id: existing_link.vulnerability_id)
end.to raise_error(ActiveRecord::RecordNotUnique) issue_link.save(validate: false)
end.to raise_error(ActiveRecord::RecordNotUnique)
end
end end
it 'allows the creation of a new "related" issue link' do context 'when there is an existing "created" issue link for vulnerability' do
expect do let!(:existing_link) { create(:vulnerabilities_issue_link, :created) }
create(:vulnerabilities_issue_link,
:related, it 'prevents the creation of a new "created" issue link' do
vulnerability: existing_link.vulnerability, expect do
issue: create(:issue)) issue_link = build(:vulnerabilities_issue_link, :created, vulnerability: existing_link.vulnerability)
end.not_to raise_error 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 end
end end
...@@ -36,6 +36,7 @@ describe ProjectPolicy do ...@@ -36,6 +36,7 @@ describe ProjectPolicy do
%i[ %i[
admin_vulnerability_feedback read_project_security_dashboard read_feature_flag admin_vulnerability_feedback read_project_security_dashboard read_feature_flag
read_vulnerability create_vulnerability admin_vulnerability read_vulnerability create_vulnerability admin_vulnerability
admin_vulnerability_issue_link
] ]
end end
let(:additional_maintainer_permissions) { %i[push_code_to_protected_branches admin_feature_flags_client] } let(:additional_maintainer_permissions) { %i[push_code_to_protected_branches admin_feature_flags_client] }
......
# 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
...@@ -42,7 +42,7 @@ describe API::Vulnerabilities do ...@@ -42,7 +42,7 @@ describe API::Vulnerabilities do
end end
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 end
describe 'permissions' do describe 'permissions' do
...@@ -88,7 +88,7 @@ describe API::Vulnerabilities do ...@@ -88,7 +88,7 @@ describe API::Vulnerabilities do
end end
it_behaves_like 'responds with "not found" for an unknown vulnerability ID' 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 end
describe 'permissions' do describe 'permissions' do
...@@ -159,7 +159,7 @@ describe API::Vulnerabilities do ...@@ -159,7 +159,7 @@ describe API::Vulnerabilities do
end end
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 end
describe 'permissions' do describe 'permissions' do
...@@ -246,7 +246,7 @@ describe API::Vulnerabilities do ...@@ -246,7 +246,7 @@ describe API::Vulnerabilities do
end end
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 end
describe 'permissions' do describe 'permissions' do
...@@ -303,7 +303,7 @@ describe API::Vulnerabilities do ...@@ -303,7 +303,7 @@ describe API::Vulnerabilities do
end end
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 end
describe 'permissions' do describe 'permissions' do
......
...@@ -37,7 +37,7 @@ describe API::VulnerabilityIssueLinks do ...@@ -37,7 +37,7 @@ describe API::VulnerabilityIssueLinks do
it_behaves_like 'responds with "not found" for an unknown vulnerability ID' 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 end
describe 'permissions' do describe 'permissions' do
...@@ -52,4 +52,97 @@ describe API::VulnerabilityIssueLinks do ...@@ -52,4 +52,97 @@ describe API::VulnerabilityIssueLinks do
it { expect { get_issue_links }.to be_denied_for(:anonymous) } it { expect { get_issue_links }.to be_denied_for(:anonymous) }
end end
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 end
# 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
# frozen_string_literal: true # 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 context 'when "first-class vulnerabilities" feature is disabled' do
before do before do
stub_feature_flags(first_class_vulnerabilities: false) stub_feature_flags(first_class_vulnerabilities: false)
......
...@@ -21237,6 +21237,9 @@ msgstr "" ...@@ -21237,6 +21237,9 @@ msgstr ""
msgid "already being used for another group or project milestone." msgid "already being used for another group or project milestone."
msgstr "" msgstr ""
msgid "already has a \"created\" issue link"
msgstr ""
msgid "already shared with this group" msgid "already shared with this group"
msgstr "" msgstr ""
...@@ -21694,6 +21697,9 @@ msgstr "" ...@@ -21694,6 +21697,9 @@ msgstr ""
msgid "group" msgid "group"
msgstr "" msgstr ""
msgid "has already been linked to another vulnerability"
msgstr ""
msgid "has already been taken" msgid "has already been taken"
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