Add policy check if cross reference system notes are accessible
This commit is contained in:
parent
e5fdd91318
commit
c99402c05f
|
@ -18,6 +18,7 @@ class Discussion
|
|||
:for_merge_request?,
|
||||
:to_ability_name,
|
||||
:editable?,
|
||||
:visible_for?,
|
||||
|
||||
to: :first_note
|
||||
|
||||
|
|
|
@ -11,6 +11,8 @@ class NotePolicy < BasePolicy
|
|||
|
||||
condition(:can_read_noteable) { can?(:"read_#{@subject.to_ability_name}") }
|
||||
|
||||
condition(:is_visible) { @subject.visible_for?(@user) }
|
||||
|
||||
rule { ~editable }.prevent :admin_note
|
||||
|
||||
# If user can't read the issue/MR/etc then they should not be allowed to do anything to their own notes
|
||||
|
@ -27,6 +29,13 @@ class NotePolicy < BasePolicy
|
|||
enable :resolve_note
|
||||
end
|
||||
|
||||
rule { ~is_visible }.policy do
|
||||
prevent :read_note
|
||||
prevent :admin_note
|
||||
prevent :resolve_note
|
||||
prevent :award_emoji
|
||||
end
|
||||
|
||||
rule { is_noteable_author }.policy do
|
||||
enable :resolve_note
|
||||
end
|
||||
|
|
|
@ -0,0 +1,6 @@
|
|||
---
|
||||
title: Add a policy check for system notes that may not be visible due to cross references
|
||||
to private items
|
||||
merge_request:
|
||||
author:
|
||||
type: security
|
|
@ -17,4 +17,83 @@ describe GitlabSchema.types['Issue'] do
|
|||
expect(described_class).to have_graphql_field(field_name)
|
||||
end
|
||||
end
|
||||
|
||||
describe "issue notes" do
|
||||
let(:user) { create(:user) }
|
||||
let(:project) { create(:project, :public) }
|
||||
let(:issue) { create(:issue, project: project) }
|
||||
let(:confidential_issue) { create(:issue, :confidential, project: project) }
|
||||
let(:private_note_body) { "mentioned in issue #{confidential_issue.to_reference(project)}" }
|
||||
let!(:note1) { create(:note, system: true, noteable: issue, author: user, project: project, note: private_note_body) }
|
||||
let!(:note2) { create(:note, system: true, noteable: issue, author: user, project: project, note: 'public note') }
|
||||
|
||||
let(:query) do
|
||||
%(
|
||||
query {
|
||||
project(fullPath:"#{project.full_path}"){
|
||||
issue(iid:"#{issue.iid}"){
|
||||
descriptionHtml
|
||||
notes{
|
||||
edges{
|
||||
node{
|
||||
bodyHtml
|
||||
author{
|
||||
username
|
||||
}
|
||||
body
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
)
|
||||
end
|
||||
|
||||
context 'query issue notes' do
|
||||
subject { GitlabSchema.execute(query, context: { current_user: current_user }).as_json }
|
||||
|
||||
shared_examples_for 'does not include private notes' do
|
||||
it "does not return private notes" do
|
||||
notes = subject.dig("data", "project", "issue", "notes", 'edges')
|
||||
notes_body = notes.map {|n| n.dig('node', 'body')}
|
||||
|
||||
expect(notes.size).to eq 1
|
||||
expect(notes_body).not_to include(private_note_body)
|
||||
expect(notes_body).to include('public note')
|
||||
end
|
||||
end
|
||||
|
||||
shared_examples_for 'includes private notes' do
|
||||
it "returns all notes" do
|
||||
notes = subject.dig("data", "project", "issue", "notes", 'edges')
|
||||
notes_body = notes.map {|n| n.dig('node', 'body')}
|
||||
|
||||
expect(notes.size).to eq 2
|
||||
expect(notes_body).to include(private_note_body)
|
||||
expect(notes_body).to include('public note')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when user signed in' do
|
||||
let(:current_user) { user }
|
||||
|
||||
it_behaves_like 'does not include private notes'
|
||||
|
||||
context 'when user member of the project' do
|
||||
before do
|
||||
project.add_developer(user)
|
||||
end
|
||||
|
||||
it_behaves_like 'includes private notes'
|
||||
end
|
||||
end
|
||||
|
||||
context 'when user is anonymous' do
|
||||
let(:current_user) { nil }
|
||||
|
||||
it_behaves_like 'does not include private notes'
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -152,6 +152,89 @@ describe NotePolicy do
|
|||
it_behaves_like 'a discussion with a private noteable'
|
||||
end
|
||||
end
|
||||
|
||||
context 'when it is a system note' do
|
||||
let(:developer) { create(:user) }
|
||||
let(:any_user) { create(:user) }
|
||||
|
||||
shared_examples_for 'user can read the note' do
|
||||
it 'allows the user to read the note' do
|
||||
expect(policy).to be_allowed(:read_note)
|
||||
end
|
||||
end
|
||||
|
||||
shared_examples_for 'user can act on the note' do
|
||||
it 'allows the user to read the note' do
|
||||
expect(policy).not_to be_allowed(:admin_note)
|
||||
expect(policy).to be_allowed(:resolve_note)
|
||||
expect(policy).to be_allowed(:award_emoji)
|
||||
end
|
||||
end
|
||||
|
||||
shared_examples_for 'user cannot read or act on the note' do
|
||||
it 'allows user to read the note' do
|
||||
expect(policy).not_to be_allowed(:admin_note)
|
||||
expect(policy).not_to be_allowed(:resolve_note)
|
||||
expect(policy).not_to be_allowed(:read_note)
|
||||
expect(policy).not_to be_allowed(:award_emoji)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when noteable is a public issue' do
|
||||
let(:note) { create(:note, system: true, noteable: noteable, author: user, project: project) }
|
||||
|
||||
before do
|
||||
project.add_developer(developer)
|
||||
end
|
||||
|
||||
context 'when user is project member' do
|
||||
let(:policy) { described_class.new(developer, note) }
|
||||
|
||||
it_behaves_like 'user can read the note'
|
||||
it_behaves_like 'user can act on the note'
|
||||
end
|
||||
|
||||
context 'when user is not project member' do
|
||||
let(:policy) { described_class.new(any_user, note) }
|
||||
|
||||
it_behaves_like 'user can read the note'
|
||||
end
|
||||
|
||||
context 'when user is anonymous' do
|
||||
let(:policy) { described_class.new(nil, note) }
|
||||
|
||||
it_behaves_like 'user can read the note'
|
||||
end
|
||||
end
|
||||
|
||||
context 'when it is a system note referencing a confidential issue' do
|
||||
let(:confidential_issue) { create(:issue, :confidential, project: project) }
|
||||
let(:note) { create(:note, system: true, noteable: issue, author: user, project: project, note: "mentioned in issue #{confidential_issue.to_reference(project)}") }
|
||||
|
||||
before do
|
||||
project.add_developer(developer)
|
||||
end
|
||||
|
||||
context 'when user is project member' do
|
||||
let(:policy) { described_class.new(developer, note) }
|
||||
|
||||
it_behaves_like 'user can read the note'
|
||||
it_behaves_like 'user can act on the note'
|
||||
end
|
||||
|
||||
context 'when user is not project member' do
|
||||
let(:policy) { described_class.new(any_user, note) }
|
||||
|
||||
it_behaves_like 'user cannot read or act on the note'
|
||||
end
|
||||
|
||||
context 'when user is anonymous' do
|
||||
let(:policy) { described_class.new(nil, note) }
|
||||
|
||||
it_behaves_like 'user cannot read or act on the note'
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue