Commit 35b09f59 authored by Alejandro Rodríguez's avatar Alejandro Rodríguez

Revert "Merge branch 'issue_23548_dev' into 'master'"

This reverts commit 311b59d9.
parent 295decbd
......@@ -241,27 +241,10 @@ class Issue < ActiveRecord::Base
# Returns `true` if the current issue can be viewed by either a logged in User
# or an anonymous user.
def visible_to_user?(user = nil)
return false unless project.feature_available?(:issues, user)
user ? readable_by?(user) : publicly_visible?
end
def overdue?
due_date.try(:past?) || false
end
# Only issues on public projects should be checked for spam
def check_for_spam?
project.public?
end
private
# Returns `true` if the given User can read the current Issue.
#
# This method duplicates the same check of issue_policy.rb
# for performance reasons, check commit: 002ad215818450d2cbbc5fa065850a953dc7ada8
# Make sure to sync this method with issue_policy.rb
def readable_by?(user)
if user.admin?
true
......@@ -282,4 +265,13 @@ class Issue < ActiveRecord::Base
def publicly_visible?
project.public? && !confidential?
end
def overdue?
due_date.try(:past?) || false
end
# Only issues on public projects should be checked for spam
def check_for_spam?
project.public?
end
end
class IssuePolicy < IssuablePolicy
# This class duplicates the same check of Issue#readable_by? for performance reasons
# Make sure to sync this class checks with issue.rb to avoid security problems.
# Check commit 002ad215818450d2cbbc5fa065850a953dc7ada8 for more information.
def issue
@subject
end
......
......@@ -63,7 +63,12 @@ module Banzai
nodes.select do |node|
if node.has_attribute?(project_attr)
node_id = node.attr(project_attr).to_i
can_read_reference?(user, projects[node_id])
if project && project.id == node_id
true
else
can?(user, :read_project, projects[node_id])
end
else
true
end
......@@ -217,15 +222,6 @@ module Banzai
attr_reader :current_user, :project
# When a feature is disabled or visible only for
# team members we should not allow team members
# see reference comments.
# Override this method on subclasses
# to check if user can read resource
def can_read_reference?(user, ref_project)
raise NotImplementedError
end
def lazy(&block)
Gitlab::Lazy.new(&block)
end
......
......@@ -29,12 +29,6 @@ module Banzai
commits
end
private
def can_read_reference?(user, ref_project)
can?(user, :download_code, ref_project)
end
end
end
end
......@@ -33,12 +33,6 @@ module Banzai
range.valid_commits? ? range : nil
end
private
def can_read_reference?(user, ref_project)
can?(user, :download_code, ref_project)
end
end
end
end
......@@ -20,12 +20,6 @@ module Banzai
def issue_ids_per_project(nodes)
gather_attributes_per_project(nodes, self.class.data_attribute)
end
private
def can_read_reference?(user, ref_project)
can?(user, :read_issue, ref_project)
end
end
end
end
......@@ -6,12 +6,6 @@ module Banzai
def references_relation
Label
end
private
def can_read_reference?(user, ref_project)
can?(user, :read_label, ref_project)
end
end
end
end
......@@ -6,12 +6,6 @@ module Banzai
def references_relation
MergeRequest.includes(:author, :assignee, :target_project)
end
private
def can_read_reference?(user, ref_project)
can?(user, :read_merge_request, ref_project)
end
end
end
end
......@@ -6,12 +6,6 @@ module Banzai
def references_relation
Milestone
end
private
def can_read_reference?(user, ref_project)
can?(user, :read_milestone, ref_project)
end
end
end
end
......@@ -6,12 +6,6 @@ module Banzai
def references_relation
Snippet
end
private
def can_read_reference?(user, ref_project)
can?(user, :read_project_snippet, ref_project)
end
end
end
end
......@@ -30,36 +30,22 @@ module Banzai
nodes.each do |node|
if node.has_attribute?(group_attr)
next unless can_read_group_reference?(node, user, groups)
visible << node
elsif can_read_project_reference?(node)
visible << node
node_group = groups[node.attr(group_attr).to_i]
if node_group &&
can?(user, :read_group, node_group)
visible << node
end
# Remaining nodes will be processed by the parent class'
# implementation of this method.
else
remaining << node
end
end
# If project does not belong to a group
# and does not have the same project id as the current project
# base class will check if user can read the project that contains
# the user reference.
visible + super(current_user, remaining)
end
# Check if project belongs to a group which
# user can read.
def can_read_group_reference?(node, user, groups)
node_group = groups[node.attr('data-group').to_i]
node_group && can?(user, :read_group, node_group)
end
def can_read_project_reference?(node)
node_id = node.attr('data-project').to_i
project && project.id == node_id
end
def nodes_user_can_reference(current_user, nodes)
project_attr = 'data-project'
author_attr = 'data-author'
......@@ -102,10 +88,6 @@ module Banzai
collection_objects_for_ids(Project, ids).
flat_map { |p| p.team.members.to_a }
end
def can_read_reference?(user, ref_project)
can?(user, :read_project, ref_project)
end
end
end
end
......@@ -22,7 +22,6 @@ feature 'Start new branch from an issue', feature: true do
create(:note, :on_issue, :system, project: project,
note: "Mentioned in !#{referenced_mr.iid}")
end
let(:referenced_mr) do
create(:merge_request, :simple, source_project: project, target_project: project,
description: "Fixes ##{issue.iid}", author: user)
......
......@@ -28,39 +28,31 @@ describe Banzai::Filter::RedactorFilter, lib: true do
and_return(parser_class)
end
context 'valid projects' do
before { allow_any_instance_of(Banzai::ReferenceParser::BaseParser).to receive(:can_read_reference?).and_return(true) }
it 'allows permitted Project references' do
user = create(:user)
project = create(:empty_project)
project.team << [user, :master]
it 'removes unpermitted Project references' do
user = create(:user)
project = create(:empty_project)
link = reference_link(project: project.id, reference_type: 'test')
doc = filter(link, current_user: user)
link = reference_link(project: project.id, reference_type: 'test')
doc = filter(link, current_user: user)
expect(doc.css('a').length).to eq 1
end
expect(doc.css('a').length).to eq 0
end
context 'invalid projects' do
before { allow_any_instance_of(Banzai::ReferenceParser::BaseParser).to receive(:can_read_reference?).and_return(false) }
it 'removes unpermitted references' do
user = create(:user)
project = create(:empty_project)
it 'allows permitted Project references' do
user = create(:user)
project = create(:empty_project)
project.team << [user, :master]
link = reference_link(project: project.id, reference_type: 'test')
doc = filter(link, current_user: user)
link = reference_link(project: project.id, reference_type: 'test')
doc = filter(link, current_user: user)
expect(doc.css('a').length).to eq 0
end
expect(doc.css('a').length).to eq 1
end
it 'handles invalid references' do
link = reference_link(project: 12345, reference_type: 'test')
it 'handles invalid Project references' do
link = reference_link(project: 12345, reference_type: 'test')
expect { filter(link) }.not_to raise_error
end
expect { filter(link) }.not_to raise_error
end
end
......
......@@ -27,12 +27,41 @@ describe Banzai::ReferenceParser::BaseParser, lib: true do
let(:link) { empty_html_link }
context 'when the link has a data-project attribute' do
it 'checks if user can read the resource' do
it 'returns the nodes if the attribute value equals the current project ID' do
link['data-project'] = project.id.to_s
expect(subject).to receive(:can_read_reference?).with(user, project)
expect(Ability).not_to receive(:allowed?)
expect(subject.nodes_visible_to_user(user, [link])).to eq([link])
end
it 'returns the nodes if the user can read the project' do
other_project = create(:empty_project, :public)
link['data-project'] = other_project.id.to_s
expect(Ability).to receive(:allowed?).
with(user, :read_project, other_project).
and_return(true)
expect(subject.nodes_visible_to_user(user, [link])).to eq([link])
end
it 'returns an empty Array when the attribute value is empty' do
link['data-project'] = ''
expect(subject.nodes_visible_to_user(user, [link])).to eq([])
end
it 'returns an empty Array when the user can not read the project' do
other_project = create(:empty_project, :public)
link['data-project'] = other_project.id.to_s
expect(Ability).to receive(:allowed?).
with(user, :read_project, other_project).
and_return(false)
subject.nodes_visible_to_user(user, [link])
expect(subject.nodes_visible_to_user(user, [link])).to eq([])
end
end
......
......@@ -8,14 +8,6 @@ describe Banzai::ReferenceParser::CommitParser, lib: true do
subject { described_class.new(project, user) }
let(:link) { empty_html_link }
describe '#nodes_visible_to_user' do
context 'when the link has a data-issue attribute' do
before { link['data-commit'] = 123 }
it_behaves_like "referenced feature visibility", "repository"
end
end
describe '#referenced_by' do
context 'when the link has a data-project attribute' do
before do
......
......@@ -8,14 +8,6 @@ describe Banzai::ReferenceParser::CommitRangeParser, lib: true do
subject { described_class.new(project, user) }
let(:link) { empty_html_link }
describe '#nodes_visible_to_user' do
context 'when the link has a data-issue attribute' do
before { link['data-commit-range'] = '123..456' }
it_behaves_like "referenced feature visibility", "repository"
end
end
describe '#referenced_by' do
context 'when the link has a data-project attribute' do
before do
......
......@@ -8,14 +8,6 @@ describe Banzai::ReferenceParser::ExternalIssueParser, lib: true do
subject { described_class.new(project, user) }
let(:link) { empty_html_link }
describe '#nodes_visible_to_user' do
context 'when the link has a data-issue attribute' do
before { link['data-external-issue'] = 123 }
it_behaves_like "referenced feature visibility", "issues"
end
end
describe '#referenced_by' do
context 'when the link has a data-project attribute' do
before do
......
......@@ -4,10 +4,10 @@ describe Banzai::ReferenceParser::IssueParser, lib: true do
include ReferenceParserHelpers
let(:project) { create(:empty_project, :public) }
let(:user) { create(:user) }
let(:issue) { create(:issue, project: project) }
let(:link) { empty_html_link }
subject { described_class.new(project, user) }
let(:user) { create(:user) }
let(:issue) { create(:issue, project: project) }
subject { described_class.new(project, user) }
let(:link) { empty_html_link }
describe '#nodes_visible_to_user' do
context 'when the link has a data-issue attribute' do
......@@ -15,8 +15,6 @@ describe Banzai::ReferenceParser::IssueParser, lib: true do
link['data-issue'] = issue.id.to_s
end
it_behaves_like "referenced feature visibility", "issues"
it 'returns the nodes when the user can read the issue' do
expect(Ability).to receive(:issues_readable_by_user).
with([issue], user).
......
......@@ -9,14 +9,6 @@ describe Banzai::ReferenceParser::LabelParser, lib: true do
subject { described_class.new(project, user) }
let(:link) { empty_html_link }
describe '#nodes_visible_to_user' do
context 'when the link has a data-issue attribute' do
before { link['data-label'] = label.id.to_s }
it_behaves_like "referenced feature visibility", "issues", "merge_requests"
end
end
describe '#referenced_by' do
describe 'when the link has a data-label attribute' do
context 'using an existing label ID' do
......
......@@ -8,19 +8,6 @@ describe Banzai::ReferenceParser::MergeRequestParser, lib: true do
subject { described_class.new(merge_request.target_project, user) }
let(:link) { empty_html_link }
describe '#nodes_visible_to_user' do
context 'when the link has a data-issue attribute' do
let(:project) { merge_request.target_project }
before do
project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC)
link['data-merge-request'] = merge_request.id.to_s
end
it_behaves_like "referenced feature visibility", "merge_requests"
end
end
describe '#referenced_by' do
describe 'when the link has a data-merge-request attribute' do
context 'using an existing merge request ID' do
......
......@@ -9,14 +9,6 @@ describe Banzai::ReferenceParser::MilestoneParser, lib: true do
subject { described_class.new(project, user) }
let(:link) { empty_html_link }
describe '#nodes_visible_to_user' do
context 'when the link has a data-issue attribute' do
before { link['data-milestone'] = milestone.id.to_s }
it_behaves_like "referenced feature visibility", "issues", "merge_requests"
end
end
describe '#referenced_by' do
describe 'when the link has a data-milestone attribute' do
context 'using an existing milestone ID' do
......
......@@ -9,14 +9,6 @@ describe Banzai::ReferenceParser::SnippetParser, lib: true do
subject { described_class.new(project, user) }
let(:link) { empty_html_link }
describe '#nodes_visible_to_user' do
context 'when the link has a data-issue attribute' do
before { link['data-snippet'] = snippet.id.to_s }
it_behaves_like "referenced feature visibility", "snippets"
end
end
describe '#referenced_by' do
describe 'when the link has a data-snippet attribute' do
context 'using an existing snippet ID' do
......
......@@ -103,8 +103,6 @@ describe Banzai::ReferenceParser::UserParser, lib: true do
it 'returns the nodes if the attribute value equals the current project ID' do
link['data-project'] = project.id.to_s
# Ensure that we dont call for Ability.allowed?
# When project_id in the node is equal to current project ID
expect(Ability).not_to receive(:allowed?)
expect(subject.nodes_visible_to_user(user, [link])).to eq([link])
......
......@@ -257,9 +257,8 @@ describe Gitlab::ClosingIssueExtractor, lib: true do
context 'with an external issue tracker reference' do
it 'extracts the referenced issue' do
jira_project = create(:jira_project, name: 'JIRA_EXT1')
jira_project.team << [jira_project.creator, :master]
jira_issue = ExternalIssue.new("#{jira_project.name}-1", project: jira_project)
closing_issue_extractor = described_class.new(jira_project, jira_project.creator)
closing_issue_extractor = described_class.new jira_project
message = "Resolve #{jira_issue.to_reference}"
expect(closing_issue_extractor.closed_by_message(message)).to eq([jira_issue])
......
......@@ -6,7 +6,7 @@ describe Gitlab::Gfm::ReferenceRewriter do
let(:new_project) { create(:project) }
let(:user) { create(:user) }
before { old_project.team << [user, :reporter] }
before { old_project.team << [user, :guest] }
describe '#rewrite' do
subject do
......
......@@ -3,8 +3,6 @@ require 'spec_helper'
describe Gitlab::ReferenceExtractor, lib: true do
let(:project) { create(:project) }
before { project.team << [project.creator, :developer] }
subject { Gitlab::ReferenceExtractor.new(project, project.creator) }
it 'accesses valid user objects' do
......@@ -44,6 +42,7 @@ describe Gitlab::ReferenceExtractor, lib: true do
end
it 'accesses valid issue objects' do
project.team << [project.creator, :developer]
@i0 = create(:issue, project: project)
@i1 = create(:issue, project: project)
......
......@@ -22,7 +22,7 @@ describe Issue, models: true do
it { is_expected.to have_db_index(:deleted_at) }
end
describe '.visible_to_user' do
describe 'visible_to_user' do
let(:user) { create(:user) }
let(:authorized_user) { create(:user) }
let(:project) { create(:project, namespace: authorized_user.namespace) }
......@@ -102,17 +102,11 @@ describe Issue, models: true do
it 'returns the merge request to close this issue' do
allow(mr).to receive(:closes_issue?).with(issue).and_return(true)
expect(issue.closed_by_merge_requests(mr.author)).to eq([mr])
end
it "returns an empty array when the merge request is closed already" do
closed_mr
expect(issue.closed_by_merge_requests(closed_mr.author)).to eq([])
expect(issue.closed_by_merge_requests).to eq([mr])
end
it "returns an empty array when the current issue is closed already" do
expect(closed_issue.closed_by_merge_requests(closed_issue.author)).to eq([])
expect(closed_issue.closed_by_merge_requests).to eq([])
end
end
......@@ -218,7 +212,7 @@ describe Issue, models: true do
source_project: subject.project,
source_branch: "#{subject.iid}-branch" })
merge_request.create_cross_references!(user)
expect(subject.referenced_merge_requests(user)).not_to be_empty
expect(subject.referenced_merge_requests).not_to be_empty
expect(subject.related_branches(user)).to eq([subject.to_branch_name])
end
......@@ -314,22 +308,6 @@ describe Issue, models: true do
end
describe '#visible_to_user?' do
context 'without a user' do
let(:issue) { build(:issue) }
it 'returns true when the issue is publicly visible' do
expect(issue).to receive(:publicly_visible?).and_return(true)
expect(issue.visible_to_user?).to eq(true)
end
it 'returns false when the issue is not publicly visible' do
expect(issue).to receive(:publicly_visible?).and_return(false)
expect(issue.visible_to_user?).to eq(false)
end
end
context 'with a user' do
let(:user) { build(:user) }
let(:issue) { build(:issue) }
......@@ -345,24 +323,26 @@ describe Issue, models: true do
expect(issue.visible_to_user?(user)).to eq(false)
end
end
it 'returns false when feature is disabled' do
expect(issue).not_to receive(:readable_by?)
context 'without a user' do
let(:issue) { build(:issue) }
issue.project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED)
it 'returns true when the issue is publicly visible' do
expect(issue).to receive(:publicly_visible?).and_return(true)
expect(issue.visible_to_user?(user)).to eq(false)
expect(issue.visible_to_user?).to eq(true)
end
it 'returns false when restricted for members' do
expect(issue).not_to receive(:readable_by?)
issue.project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PRIVATE)
it 'returns false when the issue is not publicly visible' do
expect(issue).to receive(:publicly_visible?).and_return(false)
expect(issue.visible_to_user?(user)).to eq(false)
expect(issue.visible_to_user?).to eq(false)
end
end
end
describe '#readable_by?' do
describe 'with a regular user that is not a team member' do
let(:user) { create(:user) }
......@@ -372,13 +352,13 @@ describe Issue, models: true do
it 'returns true for a regular issue' do
issue = build(:issue, project: project)
expect(issue.visible_to_user?(user)).to eq(true)
expect(issue).to be_readable_by(user)
end
it 'returns false for a confidential issue' do
issue = build(:issue, project: project, confidential: true)
expect(issue.visible_to_user?(user)).to eq(false)
expect(issue).not_to be_readable_by(user)
end
end
......@@ -389,13 +369,13 @@ describe Issue, models: true do
it 'returns true for a regular issue' do
issue = build(:issue, project: project)
expect(issue.visible_to_user?(user)).to eq(true)
expect(issue).to be_readable_by(user)
end
it 'returns false for a confidential issue' do
issue = build(:issue, :confidential, project: project)
expect(issue.visible_to_user?(user)).to eq(false)
expect(issue).not_to be_readable_by(user)
end
end
......@@ -407,13 +387,13 @@ describe Issue, models: true do
it 'returns false for a regular issue' do
issue = build(:issue, project: project)
expect(issue.visible_to_user?(user)).to eq(false)
expect(issue).not_to be_readable_by(user)
end
it 'returns false for a confidential issue' do
issue = build(:issue, :confidential, project: project)
expect(issue.visible_to_user?(user)).to eq(false)
expect(issue).not_to be_readable_by(user)
end
end
end
......@@ -424,28 +404,26 @@ describe Issue, models: true do
it 'returns false for a regular issue' do
issue = build(:issue, project: project)
expect(issue.visible_to_user?(user)).to eq(false)
expect(issue).not_to be_readable_by(user)
end
it 'returns false for a confidential issue' do
issue = build(:issue, :confidential, project: project)
expect(issue.visible_to_user?(user)).to eq(false)
expect(issue).not_to be_readable_by(user)
end
context 'when the user is the project owner' do
before { project.team << [user, :master] }
it 'returns true for a regular issue' do
issue = build(:issue, project: project)
expect(issue.visible_to_user?(user)).to eq(true)
expect(issue).not_to be_readable_by(user)
end
it 'returns true for a confidential issue' do
issue = build(:issue, :confidential, project: project)
expect(issue.visible_to_user?(user)).to eq(true)
expect(issue).not_to be_readable_by(user)
end
end
end
......@@ -463,13 +441,13 @@ describe Issue, models: true do
it 'returns true for a regular issue' do
issue = build(:issue, project: project)
expect(issue.visible_to_user?(user)).to eq(true)
expect(issue).to be_readable_by(user)
end
it 'returns true for a confidential issue' do
issue = build(:issue, :confidential, project: project)
expect(issue.visible_to_user?(user)).to eq(true)
expect(issue).to be_readable_by(user)
end
end
......@@ -483,13 +461,13 @@ describe Issue, models: true do
it 'returns true for a regular issue' do
issue = build(:issue, project: project)
expect(issue.visible_to_user?(user)).to eq(true)
expect(issue).to be_readable_by(user)
end
it 'returns true for a confidential issue' do
issue = build(:issue, :confidential, project: project)
expect(issue.visible_to_user?(user)).to eq(true)
expect(issue).to be_readable_by(user)
end
end
......@@ -503,13 +481,13 @@ describe Issue, models: true do
it 'returns true for a regular issue' do
issue = build(:issue, project: project)
expect(issue.visible_to_user?(user)).to eq(true)
expect(issue).to be_readable_by(user)
end
it 'returns true for a confidential issue' do
issue = build(:issue, :confidential, project: project)
expect(issue.visible_to_user?(user)).to eq(true)
expect(issue).to be_readable_by(user)
end
end
end
......@@ -521,13 +499,13 @@ describe Issue, models: true do
it 'returns true for a regular issue' do
issue = build(:issue, project: project)
expect(issue.visible_to_user?(user)).to eq(true)
expect(issue).to be_readable_by(user)
end
it 'returns true for a confidential issue' do
issue = build(:issue, :confidential, project: project)
expect(issue.visible_to_user?(user)).to eq(true)
expect(issue).to be_readable_by(user)
end
end
end
......@@ -539,13 +517,13 @@ describe Issue, models: true do
it 'returns true for a regular issue' do
issue = build(:issue, project: project)
expect(issue).to be_truthy
expect(issue).to be_publicly_visible
end
it 'returns false for a confidential issue' do
issue = build(:issue, :confidential, project: project)
expect(issue).not_to be_falsy
expect(issue).not_to be_publicly_visible
end
end
......@@ -555,13 +533,13 @@ describe Issue, models: true do
it 'returns false for a regular issue' do
issue = build(:issue, project: project)
expect(issue).not_to be_falsy
expect(issue).not_to be_publicly_visible
end
it 'returns false for a confidential issue' do
issue = build(:issue, :confidential, project: project)
expect(issue).not_to be_falsy
expect(issue).not_to be_publicly_visible
end
end
......@@ -571,13 +549,13 @@ describe Issue, models: true do
it 'returns false for a regular issue' do
issue = build(:issue, project: project)
expect(issue).not_to be_falsy
expect(issue).not_to be_publicly_visible
end
it 'returns false for a confidential issue' do
issue = build(:issue, :confidential, project: project)
expect(issue).not_to be_falsy
expect(issue).not_to be_publicly_visible
end
end
end
......
......@@ -49,8 +49,7 @@ module CycleAnalyticsHelpers
end
def merge_merge_requests_closing_issue(issue)
merge_requests = issue.closed_by_merge_requests(user)
merge_requests = issue.closed_by_merge_requests
merge_requests.each { |merge_request| MergeRequests::MergeService.new(project, user).execute(merge_request) }
end
......
RSpec.shared_examples "referenced feature visibility" do |*related_features|
let(:feature_fields) do
related_features.map { |feature| (feature + "_access_level").to_sym }
end
before { link['data-project'] = project.id.to_s }
context "when feature is disabled" do
it "does not create reference" do
set_features_fields_to(ProjectFeature::DISABLED)
expect(subject.nodes_visible_to_user(user, [link])).to eq([])
end
end
context "when feature is enabled only for team members" do
before { set_features_fields_to(ProjectFeature::PRIVATE) }
it "does not create reference for non member" do
non_member = create(:user)
expect(subject.nodes_visible_to_user(non_member, [link])).to eq([])
end
it "creates reference for member" do
project.team << [user, :developer]
expect(subject.nodes_visible_to_user(user, [link])).to eq([link])
end
end
context "when feature is enabled" do
# The project is public
it "creates reference" do
set_features_fields_to(ProjectFeature::ENABLED)
expect(subject.nodes_visible_to_user(user, [link])).to eq([link])
end
end
def set_features_fields_to(visibility_level)
feature_fields.each { |field| project.project_feature.update_attribute(field, visibility_level) }
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