diff options
| -rw-r--r-- | app/finders/issues_finder.rb | 42 | ||||
| -rw-r--r-- | app/views/layouts/nav/_project.html.haml | 4 | ||||
| -rw-r--r-- | spec/finders/issues_finder_spec.rb | 125 |
3 files changed, 142 insertions, 29 deletions
diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb index 3da5508aefd..3455a75b8bc 100644 --- a/app/finders/issues_finder.rb +++ b/app/finders/issues_finder.rb @@ -16,14 +16,30 @@ # sort: string # class IssuesFinder < IssuableFinder + CONFIDENTIAL_ACCESS_LEVEL = Gitlab::Access::REPORTER + def klass Issue end + def not_restricted_by_confidentiality + return Issue.where('issues.confidential IS NOT TRUE') if user_cannot_see_confidential_issues? + return Issue.all if user_can_see_all_confidential_issues? + + Issue.where(' + issues.confidential IS NOT TRUE + OR (issues.confidential = TRUE + AND (issues.author_id = :user_id + OR EXISTS (SELECT TRUE FROM issue_assignees WHERE user_id = :user_id AND issue_id = issues.id) + OR issues.project_id IN(:project_ids)))', + user_id: current_user.id, + project_ids: current_user.authorized_projects(CONFIDENTIAL_ACCESS_LEVEL).select(:id)) + end + private def init_collection - IssuesFinder.not_restricted_by_confidentiality(current_user) + not_restricted_by_confidentiality end def by_assignee(items) @@ -38,22 +54,20 @@ class IssuesFinder < IssuableFinder end end - def self.not_restricted_by_confidentiality(user) - return Issue.where('issues.confidential IS NOT TRUE') if user.blank? + def item_project_ids(items) + items&.reorder(nil)&.select(:project_id) + end - return Issue.all if user.full_private_access? + def user_can_see_all_confidential_issues? + return false unless current_user + return true if current_user.full_private_access? - Issue.where(' - issues.confidential IS NOT TRUE - OR (issues.confidential = TRUE - AND (issues.author_id = :user_id - OR EXISTS (SELECT TRUE FROM issue_assignees WHERE user_id = :user_id AND issue_id = issues.id) - OR issues.project_id IN(:project_ids)))', - user_id: user.id, - project_ids: user.authorized_projects(Gitlab::Access::REPORTER).select(:id)) + project? && + project && + project.team.max_member_access(current_user.id) >= CONFIDENTIAL_ACCESS_LEVEL end - def item_project_ids(items) - items&.reorder(nil)&.select(:project_id) + def user_cannot_see_confidential_issues? + current_user.blank? end end diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml index 68024d782a6..a2c6e44425a 100644 --- a/app/views/layouts/nav/_project.html.haml +++ b/app/views/layouts/nav/_project.html.haml @@ -28,7 +28,7 @@ %span Issues - if @project.default_issues_tracker? - %span.badge.count.issue_counter= number_with_delimiter(IssuesFinder.new(current_user, project_id: @project.id).execute.opened.count) + %span.badge.count.issue_counter= number_with_delimiter(IssuesFinder.new(current_user, project_id: @project.id, state: :opened).execute.count) - if project_nav_tab? :merge_requests - controllers = [:merge_requests, 'projects/merge_requests/conflicts'] @@ -37,7 +37,7 @@ = link_to namespace_project_merge_requests_path(@project.namespace, @project), title: 'Merge Requests', class: 'shortcuts-merge_requests' do %span Merge Requests - %span.badge.count.merge_counter.js-merge-counter= number_with_delimiter(MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened.count) + %span.badge.count.merge_counter.js-merge-counter= number_with_delimiter(MergeRequestsFinder.new(current_user, project_id: @project.id, state: :opened).execute.count) - if project_nav_tab? :pipelines = nav_link(controller: [:pipelines, :builds, :environments, :artifacts]) do diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index 8ace1fb5751..dfa15e859a4 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -295,22 +295,121 @@ describe IssuesFinder do end end - describe '.not_restricted_by_confidentiality' do - let(:authorized_user) { create(:user) } - let(:project) { create(:empty_project, namespace: authorized_user.namespace) } - let!(:public_issue) { create(:issue, project: project) } - let!(:confidential_issue) { create(:issue, project: project, confidential: true) } - - it 'returns non confidential issues for nil user' do - expect(described_class.send(:not_restricted_by_confidentiality, nil)).to include(public_issue) - end + describe '#not_restricted_by_confidentiality' do + let(:guest) { create(:user) } + set(:authorized_user) { create(:user) } + set(:project) { create(:empty_project, namespace: authorized_user.namespace) } + set(:public_issue) { create(:issue, project: project) } + set(:confidential_issue) { create(:issue, project: project, confidential: true) } + + context 'when no project filter is given' do + let(:params) { {} } + + context 'for an anonymous user' do + subject { described_class.new(nil, params).not_restricted_by_confidentiality } + + it 'returns only public issues' do + expect(subject).to include(public_issue) + expect(subject).not_to include(confidential_issue) + end + end + + context 'for a user without project membership' do + subject { described_class.new(user, params).not_restricted_by_confidentiality } + + it 'returns only public issues' do + expect(subject).to include(public_issue) + expect(subject).not_to include(confidential_issue) + end + end + + context 'for a guest user' do + subject { described_class.new(guest, params).not_restricted_by_confidentiality } + + before do + project.add_guest(guest) + end + + it 'returns only public issues' do + expect(subject).to include(public_issue) + expect(subject).not_to include(confidential_issue) + end + end + + context 'for a project member with access to view confidential issues' do + subject { described_class.new(authorized_user, params).not_restricted_by_confidentiality } - it 'returns non confidential issues for user not authorized for the issues projects' do - expect(described_class.send(:not_restricted_by_confidentiality, user)).to include(public_issue) + it 'returns all issues' do + expect(subject).to include(public_issue, confidential_issue) + end + end end - it 'returns all issues for user authorized for the issues projects' do - expect(described_class.send(:not_restricted_by_confidentiality, authorized_user)).to include(public_issue, confidential_issue) + context 'when searching within a specific project' do + let(:params) { { project_id: project.id } } + + context 'for an anonymous user' do + subject { described_class.new(nil, params).not_restricted_by_confidentiality } + + it 'returns only public issues' do + expect(subject).to include(public_issue) + expect(subject).not_to include(confidential_issue) + end + + it 'does not filter by confidentiality' do + expect(Issue).not_to receive(:where).with(a_string_matching('confidential'), anything) + + subject + end + end + + context 'for a user without project membership' do + subject { described_class.new(user, params).not_restricted_by_confidentiality } + + it 'returns only public issues' do + expect(subject).to include(public_issue) + expect(subject).not_to include(confidential_issue) + end + + it 'filters by confidentiality' do + expect(Issue).to receive(:where).with(a_string_matching('confidential'), anything) + + subject + end + end + + context 'for a guest user' do + subject { described_class.new(guest, params).not_restricted_by_confidentiality } + + before do + project.add_guest(guest) + end + + it 'returns only public issues' do + expect(subject).to include(public_issue) + expect(subject).not_to include(confidential_issue) + end + + it 'filters by confidentiality' do + expect(Issue).to receive(:where).with(a_string_matching('confidential'), anything) + + subject + end + end + + context 'for a project member with access to view confidential issues' do + subject { described_class.new(authorized_user, params).not_restricted_by_confidentiality } + + it 'returns all issues' do + expect(subject).to include(public_issue, confidential_issue) + end + + it 'does not filter by confidentiality' do + expect(Issue).not_to receive(:where).with(a_string_matching('confidential'), anything) + + subject + end + end end end end |