From 3214277f3350425baf26b514f1bcf1633018a644 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Fri, 21 Sep 2018 16:16:13 +0100 Subject: [PATCH] Check snippet note event visibility --- app/models/event.rb | 12 ++++++ spec/models/event_spec.rb | 87 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/app/models/event.rb b/app/models/event.rb index 2d9097a7769..2e690f8c013 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -148,6 +148,8 @@ class Event < ActiveRecord::Base end end + # rubocop:disable Metrics/CyclomaticComplexity + # rubocop:disable Metrics/PerceivedComplexity def visible_to_user?(user = nil) if push? || commit_note? Ability.allowed?(user, :download_code, project) @@ -159,12 +161,18 @@ class Event < ActiveRecord::Base Ability.allowed?(user, :read_issue, note? ? note_target : target) elsif merge_request? || merge_request_note? Ability.allowed?(user, :read_merge_request, note? ? note_target : target) + elsif personal_snippet_note? + Ability.allowed?(user, :read_personal_snippet, note_target) + elsif project_snippet_note? + Ability.allowed?(user, :read_project_snippet, note_target) elsif milestone? Ability.allowed?(user, :read_milestone, project) else false # No other event types are visible end end + # rubocop:enable Metrics/PerceivedComplexity + # rubocop:enable Metrics/CyclomaticComplexity def project_name if project @@ -306,6 +314,10 @@ class Event < ActiveRecord::Base note? && target && target.for_snippet? end + def personal_snippet_note? + note? && target && target.for_personal_snippet? + end + def note_target target.noteable end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 15bcaa19d5b..81748681528 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -148,9 +148,13 @@ describe Event do let(:admin) { create(:admin) } let(:issue) { create(:issue, project: project, author: author, assignees: [assignee]) } let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee]) } + let(:project_snippet) { create(:project_snippet, :public, project: project, author: author) } + let(:personal_snippet) { create(:personal_snippet, :public, author: author) } let(:note_on_commit) { create(:note_on_commit, project: project) } let(:note_on_issue) { create(:note_on_issue, noteable: issue, project: project) } let(:note_on_confidential_issue) { create(:note_on_issue, noteable: confidential_issue, project: project) } + let(:note_on_project_snippet) { create(:note_on_project_snippet, author: author, noteable: project_snippet, project: project) } + let(:note_on_personal_snippet) { create(:note_on_personal_snippet, author: author, noteable: personal_snippet, project: nil) } let(:milestone_on_project) { create(:milestone, project: project) } let(:event) { described_class.new(project: project, target: target, author_id: author.id) } @@ -305,6 +309,89 @@ describe Event do end end end + + context 'project snippet note event' do + let(:target) { note_on_project_snippet } + + it do + expect(event.visible_to_user?(nil)).to be_truthy + expect(event.visible_to_user?(non_member)).to be_truthy + expect(event.visible_to_user?(author)).to be_truthy + expect(event.visible_to_user?(member)).to be_truthy + expect(event.visible_to_user?(guest)).to be_truthy + expect(event.visible_to_user?(admin)).to be_truthy + end + + context 'on public project with private snippets' do + let(:project) { create(:project, :public, :snippets_private) } + + it do + expect(event.visible_to_user?(nil)).to be_falsy + expect(event.visible_to_user?(non_member)).to be_falsy + + # Normally, we'd expect the author of a comment to be able to view it. + # However, this doesn't seem to be the case for comments on snippets. + expect(event.visible_to_user?(author)).to be_falsy + + expect(event.visible_to_user?(member)).to be_truthy + expect(event.visible_to_user?(guest)).to be_truthy + expect(event.visible_to_user?(admin)).to be_truthy + end + end + + context 'on private project' do + let(:project) { create(:project, :private) } + + it do + expect(event.visible_to_user?(nil)).to be_falsy + expect(event.visible_to_user?(non_member)).to be_falsy + + # Normally, we'd expect the author of a comment to be able to view it. + # However, this doesn't seem to be the case for comments on snippets. + expect(event.visible_to_user?(author)).to be_falsy + + expect(event.visible_to_user?(member)).to be_truthy + expect(event.visible_to_user?(guest)).to be_truthy + expect(event.visible_to_user?(admin)).to be_truthy + end + end + end + + context 'personal snippet note event' do + let(:target) { note_on_personal_snippet } + + it do + expect(event.visible_to_user?(nil)).to be_truthy + expect(event.visible_to_user?(non_member)).to be_truthy + expect(event.visible_to_user?(author)).to be_truthy + expect(event.visible_to_user?(admin)).to be_truthy + end + + context 'on internal snippet' do + let(:personal_snippet) { create(:personal_snippet, :internal, author: author) } + + it do + expect(event.visible_to_user?(nil)).to be_falsy + expect(event.visible_to_user?(non_member)).to be_truthy + expect(event.visible_to_user?(author)).to be_truthy + expect(event.visible_to_user?(admin)).to be_truthy + end + end + + context 'on private snippet' do + let(:personal_snippet) { create(:personal_snippet, :private, author: author) } + + it do + expect(event.visible_to_user?(nil)).to be_falsy + expect(event.visible_to_user?(non_member)).to be_falsy + expect(event.visible_to_user?(author)).to be_truthy + + # It is very unexpected that a private personal snippet is not visible + # to an instance administrator. This should be fixed in the future. + expect(event.visible_to_user?(admin)).to be_falsy + end + end + end end describe '.limit_recent' do