gitlab-org--gitlab-foss/spec/lib/gitlab/search_results_spec.rb

260 lines
9.4 KiB
Ruby
Raw Normal View History

require 'spec_helper'
describe Gitlab::SearchResults do
include ProjectForksHelper
let(:user) { create(:user) }
let!(:project) { create(:project, name: 'foo') }
let!(:issue) { create(:issue, project: project, title: 'foo') }
let!(:merge_request) do
create(:merge_request, source_project: project, title: 'foo')
end
let!(:milestone) { create(:milestone, project: project, title: 'foo') }
let(:results) { described_class.new(user, Project.all, 'foo') }
Merge branch 'jej-use-issuable-finder-instead-of-access-check' into 'security' Replace issue access checks with use of IssuableFinder Split from !2024 to partially solve https://gitlab.com/gitlab-org/gitlab-ce/issues/23867 ## Which fixes are in this MR? :warning: - Potentially untested :bomb: - No test coverage :traffic_light: - Test coverage of some sort exists (a test failed when error raised) :vertical_traffic_light: - Test coverage of return value (a test failed when nil used) :white_check_mark: - Permissions check tested ### Issue lookup with access check Using `visible_to_user` likely makes these security issues too. See [Code smells](#code-smells). - [x] :vertical_traffic_light: app/finders/notes_finder.rb:15 [`visible_to_user`] - [x] :traffic_light: app/views/layouts/nav/_project.html.haml:73 [`visible_to_user`] [`.count`] - [x] :white_check_mark: app/services/merge_requests/build_service.rb:84 [`issue.try(:confidential?)`] - [x] :white_check_mark: lib/api/issues.rb:112 [`visible_to_user`] - CHANGELOG: Prevented API returning issues set to 'Only team members' to everyone - [x] :white_check_mark: lib/api/helpers.rb:126 [`can?(current_user, :read_issue, issue)`] Maybe here too? - [x] :white_check_mark: lib/gitlab/search_results.rb:53 [`visible_to_user`] ### Previous discussions - [ ] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#b2ff264eddf9819d7693c14ae213d941494fe2b3_128_126 - [ ] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#7b6375270d22f880bdcb085e47b519b426a5c6c7_87_87 See merge request !2031
2016-11-22 10:25:04 +00:00
context 'as a user with access' do
before do
project.add_developer(user)
end
describe '#objects' do
2018-01-31 14:59:59 +00:00
it 'returns without_counts collection by default' do
expect(results.objects('projects')).to be_kind_of(Kaminari::PaginatableWithoutCount)
end
2018-01-31 14:59:59 +00:00
it 'returns with counts collection when requested' do
expect(results.objects('projects', 1, false)).not_to be_kind_of(Kaminari::PaginatableWithoutCount)
end
end
context "when count_limit is lower than total amount" do
before do
allow(results).to receive(:count_limit).and_return(1)
end
describe '#limited_projects_count' do
it 'returns the limited amount of projects' do
create(:project, name: 'foo2')
expect(results.limited_projects_count).to eq(1)
end
end
describe '#limited_merge_requests_count' do
it 'returns the limited amount of merge requests' do
create(:merge_request, :simple, source_project: project, title: 'foo2')
expect(results.limited_merge_requests_count).to eq(1)
end
end
describe '#limited_milestones_count' do
it 'returns the limited amount of milestones' do
create(:milestone, project: project, title: 'foo2')
expect(results.limited_milestones_count).to eq(1)
end
end
describe '#limited_issues_count' do
it 'runs single SQL query to get the limited amount of issues' do
create(:milestone, project: project, title: 'foo2')
expect(results).to receive(:issues).with(public_only: true).and_call_original
expect(results).not_to receive(:issues).with(no_args).and_call_original
expect(results.limited_issues_count).to eq(1)
end
end
end
context "when count_limit is higher than total amount" do
describe '#limited_issues_count' do
it 'runs multiple queries to get the limited amount of issues' do
expect(results).to receive(:issues).with(public_only: true).and_call_original
expect(results).to receive(:issues).with(no_args).and_call_original
expect(results.limited_issues_count).to eq(1)
end
end
end
Merge branch 'jej-23867-use-mr-finder-instead-of-access-check' into 'security' Replace MR access checks with use of MergeRequestsFinder Split from !2024 to partially solve https://gitlab.com/gitlab-org/gitlab-ce/issues/23867 :warning: - Potentially untested :bomb: - No test coverage :traffic_light: - Test coverage of some sort exists (a test failed when error raised) :vertical_traffic_light: - Test coverage of return value (a test failed when nil used) :white_check_mark: - Permissions check tested - [x] :bomb: app/finders/notes_finder.rb:17 - [x] :warning: app/views/layouts/nav/_project.html.haml:80 [`.count`] - [x] :bomb: app/controllers/concerns/creates_commit.rb:84 - [x] :traffic_light: app/controllers/projects/commits_controller.rb:24 - [x] :traffic_light: app/controllers/projects/compare_controller.rb:56 - [x] :vertical_traffic_light: app/controllers/projects/discussions_controller.rb:29 - [x] :white_check_mark: app/controllers/projects/todos_controller.rb:27 - [x] :vertical_traffic_light: app/models/commit.rb:268 - [x] :white_check_mark: lib/gitlab/search_results.rb:71 - [x] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#d1c10892daedb4d4dd3d4b12b6d071091eea83df_267_266 Memoize ` merged_merge_request(current_user)` - [x] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#d1c10892daedb4d4dd3d4b12b6d071091eea83df_248_247 Expected side effect for `merged_merge_request!`, consider `skip_authorization: true`. - [x] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#d1c10892daedb4d4dd3d4b12b6d071091eea83df_269_269 Scary use of unchecked `merged_merge_request?` See merge request !2033
2016-11-29 13:47:43 +00:00
it 'includes merge requests from source and target projects' do
forked_project = fork_project(project, user)
Merge branch 'jej-23867-use-mr-finder-instead-of-access-check' into 'security' Replace MR access checks with use of MergeRequestsFinder Split from !2024 to partially solve https://gitlab.com/gitlab-org/gitlab-ce/issues/23867 :warning: - Potentially untested :bomb: - No test coverage :traffic_light: - Test coverage of some sort exists (a test failed when error raised) :vertical_traffic_light: - Test coverage of return value (a test failed when nil used) :white_check_mark: - Permissions check tested - [x] :bomb: app/finders/notes_finder.rb:17 - [x] :warning: app/views/layouts/nav/_project.html.haml:80 [`.count`] - [x] :bomb: app/controllers/concerns/creates_commit.rb:84 - [x] :traffic_light: app/controllers/projects/commits_controller.rb:24 - [x] :traffic_light: app/controllers/projects/compare_controller.rb:56 - [x] :vertical_traffic_light: app/controllers/projects/discussions_controller.rb:29 - [x] :white_check_mark: app/controllers/projects/todos_controller.rb:27 - [x] :vertical_traffic_light: app/models/commit.rb:268 - [x] :white_check_mark: lib/gitlab/search_results.rb:71 - [x] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#d1c10892daedb4d4dd3d4b12b6d071091eea83df_267_266 Memoize ` merged_merge_request(current_user)` - [x] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#d1c10892daedb4d4dd3d4b12b6d071091eea83df_248_247 Expected side effect for `merged_merge_request!`, consider `skip_authorization: true`. - [x] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#d1c10892daedb4d4dd3d4b12b6d071091eea83df_269_269 Scary use of unchecked `merged_merge_request?` See merge request !2033
2016-11-29 13:47:43 +00:00
merge_request_2 = create(:merge_request, target_project: project, source_project: forked_project, title: 'foo')
results = described_class.new(user, Project.where(id: forked_project.id), 'foo')
expect(results.objects('merge_requests')).to include merge_request_2
end
describe '#merge_requests' do
it 'includes project filter by default' do
expect(results).to receive(:project_ids_relation).and_call_original
results.objects('merge_requests')
end
it 'it skips project filter if default project context is used' do
allow(results).to receive(:default_project_filter).and_return(true)
expect(results).not_to receive(:project_ids_relation)
results.objects('merge_requests')
end
end
describe '#issues' do
it 'includes project filter by default' do
expect(results).to receive(:project_ids_relation).and_call_original
results.objects('issues')
end
it 'it skips project filter if default project context is used' do
allow(results).to receive(:default_project_filter).and_return(true)
expect(results).not_to receive(:project_ids_relation)
results.objects('issues')
end
end
describe '#users' do
2018-12-18 12:51:26 +00:00
it 'does not call the UsersFinder when the current_user is not allowed to read users list' do
allow(Ability).to receive(:allowed?).and_return(false)
expect(UsersFinder).not_to receive(:new).with(user, search: 'foo').and_call_original
results.objects('users')
end
it 'calls the UsersFinder' do
expect(UsersFinder).to receive(:new).with(user, search: 'foo').and_call_original
results.objects('users')
end
end
end
Merge branch 'jej-use-issuable-finder-instead-of-access-check' into 'security' Replace issue access checks with use of IssuableFinder Split from !2024 to partially solve https://gitlab.com/gitlab-org/gitlab-ce/issues/23867 ## Which fixes are in this MR? :warning: - Potentially untested :bomb: - No test coverage :traffic_light: - Test coverage of some sort exists (a test failed when error raised) :vertical_traffic_light: - Test coverage of return value (a test failed when nil used) :white_check_mark: - Permissions check tested ### Issue lookup with access check Using `visible_to_user` likely makes these security issues too. See [Code smells](#code-smells). - [x] :vertical_traffic_light: app/finders/notes_finder.rb:15 [`visible_to_user`] - [x] :traffic_light: app/views/layouts/nav/_project.html.haml:73 [`visible_to_user`] [`.count`] - [x] :white_check_mark: app/services/merge_requests/build_service.rb:84 [`issue.try(:confidential?)`] - [x] :white_check_mark: lib/api/issues.rb:112 [`visible_to_user`] - CHANGELOG: Prevented API returning issues set to 'Only team members' to everyone - [x] :white_check_mark: lib/api/helpers.rb:126 [`can?(current_user, :read_issue, issue)`] Maybe here too? - [x] :white_check_mark: lib/gitlab/search_results.rb:53 [`visible_to_user`] ### Previous discussions - [ ] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#b2ff264eddf9819d7693c14ae213d941494fe2b3_128_126 - [ ] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#7b6375270d22f880bdcb085e47b519b426a5c6c7_87_87 See merge request !2031
2016-11-22 10:25:04 +00:00
it 'does not list issues on private projects' do
private_project = create(:project, :private)
Merge branch 'jej-use-issuable-finder-instead-of-access-check' into 'security' Replace issue access checks with use of IssuableFinder Split from !2024 to partially solve https://gitlab.com/gitlab-org/gitlab-ce/issues/23867 ## Which fixes are in this MR? :warning: - Potentially untested :bomb: - No test coverage :traffic_light: - Test coverage of some sort exists (a test failed when error raised) :vertical_traffic_light: - Test coverage of return value (a test failed when nil used) :white_check_mark: - Permissions check tested ### Issue lookup with access check Using `visible_to_user` likely makes these security issues too. See [Code smells](#code-smells). - [x] :vertical_traffic_light: app/finders/notes_finder.rb:15 [`visible_to_user`] - [x] :traffic_light: app/views/layouts/nav/_project.html.haml:73 [`visible_to_user`] [`.count`] - [x] :white_check_mark: app/services/merge_requests/build_service.rb:84 [`issue.try(:confidential?)`] - [x] :white_check_mark: lib/api/issues.rb:112 [`visible_to_user`] - CHANGELOG: Prevented API returning issues set to 'Only team members' to everyone - [x] :white_check_mark: lib/api/helpers.rb:126 [`can?(current_user, :read_issue, issue)`] Maybe here too? - [x] :white_check_mark: lib/gitlab/search_results.rb:53 [`visible_to_user`] ### Previous discussions - [ ] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#b2ff264eddf9819d7693c14ae213d941494fe2b3_128_126 - [ ] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#7b6375270d22f880bdcb085e47b519b426a5c6c7_87_87 See merge request !2031
2016-11-22 10:25:04 +00:00
issue = create(:issue, project: private_project, title: 'foo')
expect(results.objects('issues')).not_to include issue
end
describe 'confidential issues' do
let(:project_1) { create(:project, :internal) }
let(:project_2) { create(:project, :internal) }
let(:project_3) { create(:project, :internal) }
let(:project_4) { create(:project, :internal) }
let(:query) { 'issue' }
let(:limit_projects) { Project.where(id: [project_1.id, project_2.id, project_3.id]) }
let(:author) { create(:user) }
let(:assignee) { create(:user) }
let(:non_member) { create(:user) }
let(:member) { create(:user) }
let(:admin) { create(:admin) }
let!(:issue) { create(:issue, project: project_1, title: 'Issue 1') }
let!(:security_issue_1) { create(:issue, :confidential, project: project_1, title: 'Security issue 1', author: author) }
let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project_1, assignees: [assignee]) }
let!(:security_issue_3) { create(:issue, :confidential, project: project_2, title: 'Security issue 3', author: author) }
let!(:security_issue_4) { create(:issue, :confidential, project: project_3, title: 'Security issue 4', assignees: [assignee]) }
let!(:security_issue_5) { create(:issue, :confidential, project: project_4, title: 'Security issue 5') }
it 'does not list confidential issues for non project members' do
results = described_class.new(non_member, limit_projects, query)
issues = results.objects('issues')
expect(issues).to include issue
expect(issues).not_to include security_issue_1
expect(issues).not_to include security_issue_2
expect(issues).not_to include security_issue_3
expect(issues).not_to include security_issue_4
expect(issues).not_to include security_issue_5
expect(results.limited_issues_count).to eq 1
end
it 'does not list confidential issues for project members with guest role' do
project_1.add_guest(member)
project_2.add_guest(member)
results = described_class.new(member, limit_projects, query)
issues = results.objects('issues')
expect(issues).to include issue
expect(issues).not_to include security_issue_1
expect(issues).not_to include security_issue_2
expect(issues).not_to include security_issue_3
expect(issues).not_to include security_issue_4
expect(issues).not_to include security_issue_5
expect(results.limited_issues_count).to eq 1
end
it 'lists confidential issues for author' do
results = described_class.new(author, limit_projects, query)
issues = results.objects('issues')
expect(issues).to include issue
expect(issues).to include security_issue_1
expect(issues).not_to include security_issue_2
expect(issues).to include security_issue_3
expect(issues).not_to include security_issue_4
expect(issues).not_to include security_issue_5
expect(results.limited_issues_count).to eq 3
end
it 'lists confidential issues for assignee' do
results = described_class.new(assignee, limit_projects, query)
issues = results.objects('issues')
expect(issues).to include issue
expect(issues).not_to include security_issue_1
expect(issues).to include security_issue_2
expect(issues).not_to include security_issue_3
expect(issues).to include security_issue_4
expect(issues).not_to include security_issue_5
expect(results.limited_issues_count).to eq 3
end
it 'lists confidential issues for project members' do
project_1.add_developer(member)
project_2.add_developer(member)
results = described_class.new(member, limit_projects, query)
issues = results.objects('issues')
expect(issues).to include issue
expect(issues).to include security_issue_1
expect(issues).to include security_issue_2
expect(issues).to include security_issue_3
expect(issues).not_to include security_issue_4
expect(issues).not_to include security_issue_5
expect(results.limited_issues_count).to eq 4
end
it 'lists all issues for admin' do
results = described_class.new(admin, limit_projects, query)
issues = results.objects('issues')
expect(issues).to include issue
expect(issues).to include security_issue_1
expect(issues).to include security_issue_2
expect(issues).to include security_issue_3
expect(issues).to include security_issue_4
expect(issues).not_to include security_issue_5
expect(results.limited_issues_count).to eq 5
end
end
Merge branch 'jej-23867-use-mr-finder-instead-of-access-check' into 'security' Replace MR access checks with use of MergeRequestsFinder Split from !2024 to partially solve https://gitlab.com/gitlab-org/gitlab-ce/issues/23867 :warning: - Potentially untested :bomb: - No test coverage :traffic_light: - Test coverage of some sort exists (a test failed when error raised) :vertical_traffic_light: - Test coverage of return value (a test failed when nil used) :white_check_mark: - Permissions check tested - [x] :bomb: app/finders/notes_finder.rb:17 - [x] :warning: app/views/layouts/nav/_project.html.haml:80 [`.count`] - [x] :bomb: app/controllers/concerns/creates_commit.rb:84 - [x] :traffic_light: app/controllers/projects/commits_controller.rb:24 - [x] :traffic_light: app/controllers/projects/compare_controller.rb:56 - [x] :vertical_traffic_light: app/controllers/projects/discussions_controller.rb:29 - [x] :white_check_mark: app/controllers/projects/todos_controller.rb:27 - [x] :vertical_traffic_light: app/models/commit.rb:268 - [x] :white_check_mark: lib/gitlab/search_results.rb:71 - [x] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#d1c10892daedb4d4dd3d4b12b6d071091eea83df_267_266 Memoize ` merged_merge_request(current_user)` - [x] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#d1c10892daedb4d4dd3d4b12b6d071091eea83df_248_247 Expected side effect for `merged_merge_request!`, consider `skip_authorization: true`. - [x] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#d1c10892daedb4d4dd3d4b12b6d071091eea83df_269_269 Scary use of unchecked `merged_merge_request?` See merge request !2033
2016-11-29 13:47:43 +00:00
it 'does not list merge requests on projects with limited access' do
project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE)
expect(results.objects('merge_requests')).not_to include merge_request
end
end