Commit e94e3137 authored by James Lopez's avatar James Lopez

Merge branch '320990_check_issue_permissions_for_vuln_issue_links' into 'master'

Authorize users to read `Vulnerabilities::IssueLink` records

See merge request gitlab-org/gitlab!62815
parents 95705b90 90692666
...@@ -2,11 +2,12 @@ ...@@ -2,11 +2,12 @@
module Types module Types
module Vulnerability module Vulnerability
# rubocop: disable Graphql/AuthorizeTypes
class IssueLinkType < BaseObject class IssueLinkType < BaseObject
graphql_name 'VulnerabilityIssueLink' graphql_name 'VulnerabilityIssueLink'
description 'Represents an issue link of a vulnerability' description 'Represents an issue link of a vulnerability'
authorize :read_issue_link
field :id, GraphQL::ID_TYPE, null: false, field :id, GraphQL::ID_TYPE, null: false,
description: 'GraphQL ID of the vulnerability.' description: 'GraphQL ID of the vulnerability.'
...@@ -16,6 +17,5 @@ module Types ...@@ -16,6 +17,5 @@ module Types
field :issue, ::Types::IssueType, null: false, field :issue, ::Types::IssueType, null: false,
description: 'The issue attached to issue link.' description: 'The issue attached to issue link.'
end end
# rubocop: enable Graphql/AuthorizeTypes
end end
end end
...@@ -3,5 +3,9 @@ ...@@ -3,5 +3,9 @@
module Vulnerabilities module Vulnerabilities
class IssueLinkPolicy < BasePolicy class IssueLinkPolicy < BasePolicy
delegate { @subject.vulnerability&.project } delegate { @subject.vulnerability&.project }
condition(:issue_readable?) { Ability.allowed?(@user, :read_issue, @subject.issue) }
rule { ~issue_readable? }.prevent :read_issue_link
end end
end end
...@@ -8,4 +8,5 @@ RSpec.describe GitlabSchema.types['VulnerabilityIssueLink'] do ...@@ -8,4 +8,5 @@ RSpec.describe GitlabSchema.types['VulnerabilityIssueLink'] do
subject { described_class } subject { described_class }
it { is_expected.to have_graphql_fields(expected_fields) } it { is_expected.to have_graphql_fields(expected_fields) }
it { is_expected.to require_graphql_authorizations(:read_issue_link) }
end end
...@@ -3,16 +3,16 @@ ...@@ -3,16 +3,16 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Vulnerabilities::IssueLinkPolicy do RSpec.describe Vulnerabilities::IssueLinkPolicy do
let(:vulnerability_issue_link) { build(:vulnerabilities_issue_link, vulnerability: vulnerability, issue: issue) }
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project, namespace: user.namespace) } let_it_be(:project) { create(:project, namespace: user.namespace) }
let_it_be(:vulnerability) { create(:vulnerability, project: project) }
let(:vulnerability) { create(:vulnerability, project: project) } let_it_be(:issue) { create(:issue, project: 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) } subject { described_class.new(user, vulnerability_issue_link) }
context 'with a user authorized to admin vulnerability-issue links' do describe ':admin_vulnerability_issue_link' do
before do before do
stub_licensed_features(security_dashboard: true) stub_licensed_features(security_dashboard: true)
...@@ -20,8 +20,8 @@ RSpec.describe Vulnerabilities::IssueLinkPolicy do ...@@ -20,8 +20,8 @@ RSpec.describe Vulnerabilities::IssueLinkPolicy do
end end
context 'with missing vulnerability' do context 'with missing vulnerability' do
let(:vulnerability) { nil } let_it_be(:vulnerability) { nil }
let(:issue) { create(:issue) } let_it_be(:issue) { create(:issue) }
it { is_expected.to be_disallowed(:admin_vulnerability_issue_link) } it { is_expected.to be_disallowed(:admin_vulnerability_issue_link) }
end end
...@@ -31,9 +31,27 @@ RSpec.describe Vulnerabilities::IssueLinkPolicy do ...@@ -31,9 +31,27 @@ RSpec.describe Vulnerabilities::IssueLinkPolicy do
end end
context "when issue and link don't belong to the same project" do context "when issue and link don't belong to the same project" do
let(:issue) { create(:issue) } let_it_be(:issue) { create(:issue) }
it { is_expected.to be_allowed(:admin_vulnerability_issue_link) } it { is_expected.to be_allowed(:admin_vulnerability_issue_link) }
end end
end end
describe ':read_issue_link' do
before do
allow(Ability).to receive(:allowed?).with(user, :read_issue, issue).and_return(allowed?)
end
context 'when the associated issue can not be read by the user' do
let(:allowed?) { false }
it { is_expected.to be_disallowed(:read_issue_link) }
end
context 'when the associated issue can be read by the user' do
let(:allowed?) { true }
it { is_expected.to be_allowed(:read_issue_link) }
end
end
end end
...@@ -8,11 +8,14 @@ RSpec.describe 'Query.vulnerabilities.issueLinks' do ...@@ -8,11 +8,14 @@ RSpec.describe 'Query.vulnerabilities.issueLinks' do
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
let_it_be(:user) { create(:user, security_dashboard_projects: [project]) } let_it_be(:user) { create(:user, security_dashboard_projects: [project]) }
let_it_be(:vulnerability) { create(:vulnerability, project: project) } let_it_be(:vulnerability) { create(:vulnerability, project: project) }
let_it_be(:related_issue) { create(:vulnerabilities_issue_link, :related, vulnerability: vulnerability) } let_it_be(:created_issue) { create(:issue, project: project) }
let_it_be(:created_issue) { create(:vulnerabilities_issue_link, :created, vulnerability: vulnerability) } let_it_be(:related_issue) { create(:issue, project: project) }
let_it_be(:related_issue_link) { create(:vulnerabilities_issue_link, :related, vulnerability: vulnerability, issue: related_issue) }
let_it_be(:created_issue_link) { create(:vulnerabilities_issue_link, :created, vulnerability: vulnerability, issue: created_issue) }
before do before do
project.add_developer(user) project.add_developer(user)
stub_licensed_features(security_dashboard: true) stub_licensed_features(security_dashboard: true)
end end
...@@ -46,13 +49,13 @@ RSpec.describe 'Query.vulnerabilities.issueLinks' do ...@@ -46,13 +49,13 @@ RSpec.describe 'Query.vulnerabilities.issueLinks' do
it 'returns a list of VulnerabilityIssueLink with `CREATED` linkType' do it 'returns a list of VulnerabilityIssueLink with `CREATED` linkType' do
query_issue_links('CREATED') query_issue_links('CREATED')
expect_issue_links_response(created_issue) expect_issue_links_response(created_issue_link)
end end
it 'returns a list of VulnerabilityIssueLink with `RELATED` linkType' do it 'returns a list of VulnerabilityIssueLink with `RELATED` linkType' do
query_issue_links('RELATED') query_issue_links('RELATED')
expect_issue_links_response(related_issue) expect_issue_links_response(related_issue_link)
end end
end end
...@@ -60,7 +63,7 @@ RSpec.describe 'Query.vulnerabilities.issueLinks' do ...@@ -60,7 +63,7 @@ RSpec.describe 'Query.vulnerabilities.issueLinks' do
it 'returns a list of all VulnerabilityIssueLink' do it 'returns a list of all VulnerabilityIssueLink' do
query_issue_links query_issue_links
expect_issue_links_response(related_issue, created_issue) expect_issue_links_response(related_issue_link, created_issue_link)
end end
end end
......
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