Commit 83e8f432 authored by GitLab Release Tools Bot's avatar GitLab Release Tools Bot

Merge branch 'security-filter-related-branches-from-activity-feed-12.5' into '12-5-stable'

Related Branches Visible to Guests in Issue Activity

See merge request gitlab/gitlabhq!3538
parents 7d028ae6 6324a099
......@@ -37,6 +37,10 @@ class Note < ApplicationRecord
redact_field :note
TYPES_RESTRICTED_BY_ABILITY = {
branch: :download_code
}.freeze
# Aliases to make application_helper#edited_time_ago_with_tooltip helper work properly with notes.
# See https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/10392/diffs#note_28719102
alias_attribute :last_edited_at, :updated_at
......@@ -341,7 +345,7 @@ class Note < ApplicationRecord
end
def visible_for?(user)
!cross_reference_not_visible_for?(user)
!cross_reference_not_visible_for?(user) && system_note_viewable_by?(user)
end
def award_emoji?
......@@ -493,6 +497,15 @@ class Note < ApplicationRecord
private
def system_note_viewable_by?(user)
return true unless system_note_metadata
restriction = TYPES_RESTRICTED_BY_ABILITY[system_note_metadata.action.to_sym]
return Ability.allowed?(user, restriction, project) if restriction
true
end
def keep_around_commit
project.repository.keep_around(self.commit_id)
end
......
---
title: Remove notes regarding Related Branches from Issue activity feeds for guest
users
merge_request:
author:
type: security
......@@ -1435,6 +1435,43 @@ describe Projects::IssuesController do
expect { get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } }.not_to exceed_query_limit(control_count)
end
end
context 'private project' do
let!(:branch_note) { create(:discussion_note_on_issue, :system, noteable: issue, project: project) }
let!(:commit_note) { create(:discussion_note_on_issue, :system, noteable: issue, project: project) }
let!(:branch_note_meta) { create(:system_note_metadata, note: branch_note, action: "branch") }
let!(:commit_note_meta) { create(:system_note_metadata, note: commit_note, action: "commit") }
context 'user is allowed access' do
before do
project.add_user(user, :maintainer)
end
it 'displays all available notes' do
get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid }
expect(json_response.length).to eq(3)
end
end
context 'user is a guest' do
let(:json_response_note_ids) do
json_response.collect { |discussion| discussion["notes"] }.flatten
.collect { |note| note["id"].to_i }
end
before do
project.add_guest(user)
end
it 'does not display notes w/type listed in TYPES_RESTRICTED_BY_ACCESS_LEVEL' do
get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid }
expect(json_response.length).to eq(2)
expect(json_response_note_ids).not_to include(branch_note.id)
end
end
end
end
end
......
......@@ -285,6 +285,70 @@ describe Note do
end
end
describe "#visible_for?" do
using RSpec::Parameterized::TableSyntax
let_it_be(:note) { create(:note) }
let_it_be(:user) { create(:user) }
where(:cross_reference_visible, :system_note_viewable, :result) do
true | true | false
false | true | true
false | false | false
end
with_them do
it "returns expected result" do
expect(note).to receive(:cross_reference_not_visible_for?).and_return(cross_reference_visible)
unless cross_reference_visible
expect(note).to receive(:system_note_viewable_by?)
.with(user).and_return(system_note_viewable)
end
expect(note.visible_for?(user)).to eq result
end
end
end
describe "#system_note_viewable_by?(user)" do
let_it_be(:note) { create(:note) }
let_it_be(:user) { create(:user) }
let!(:metadata) { create(:system_note_metadata, note: note, action: "branch") }
context "when system_note_metadata is not present" do
it "returns true" do
expect(note).to receive(:system_note_metadata).and_return(nil)
expect(note.send(:system_note_viewable_by?, user)).to be_truthy
end
end
context "system_note_metadata isn't of type 'branch'" do
before do
metadata.action = "not_a_branch"
end
it "returns true" do
expect(note.send(:system_note_viewable_by?, user)).to be_truthy
end
end
context "user doesn't have :download_code ability" do
it "returns false" do
expect(note.send(:system_note_viewable_by?, user)).to be_falsey
end
end
context "user has the :download_code ability" do
it "returns true" do
expect(Ability).to receive(:allowed?).with(user, :download_code, note.project).and_return(true)
expect(note.send(:system_note_viewable_by?, user)).to be_truthy
end
end
end
describe "cross_reference_not_visible_for?" do
let(:private_user) { create(:user) }
let(:private_project) { create(:project, namespace: private_user.namespace) { |p| p.add_maintainer(private_user) } }
......
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