GitLab wird am Donnerstag, den 27. Januar, zwischen 08:00 und 12:00 Uhr wegen wichtigen Wartungsarbeiten nicht zur Verfügung stehen.

Commit b06a0923 authored by Alexandru Croitor's avatar Alexandru Croitor
Browse files

Add policy check if cross reference system notes are accessible

parent 4be19d5f
......@@ -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
......
---
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 @@
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 @@
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
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