Skip to content
Snippets Groups Projects
Commit debb3649 authored by Kerri Miller's avatar Kerri Miller
Browse files

Restrict branches visible to guests in Issue feed

Notes related to branch creation should not be shown in an issue's
activity feed when the user doesn't have access to :download_code.
parent 9d3adee8
No related branches found
No related tags found
No related merge requests found
Loading
Loading
@@ -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
Loading
Loading
@@ -330,7 +334,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?
Loading
Loading
@@ -483,6 +487,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
Loading
Loading
---
title: Remove notes regarding Related Branches from Issue activity feeds for guest
users
merge_request:
author:
type: security
Loading
Loading
@@ -1380,6 +1380,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
 
Loading
Loading
Loading
Loading
@@ -252,6 +252,70 @@ describe Note do
end
end
 
describe "#visible_for?" do
using RSpec::Parameterized::TableSyntax
let(:note) { create(:note) }
let(: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(:note) { create(:note) }
let(: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) } }
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment