gitlab-org--gitlab-foss/spec/lib/gitlab/project_search_results_spec.rb
Douwe Maan 12db4cc0e7 Merge branch 'jej-note-search-uses-finder' into 'security'
Fix missing Note access checks in by moving Note#search to updated NoteFinder

Split from !2024 to partially solve https://gitlab.com/gitlab-org/gitlab-ce/issues/23867

## Which fixes are in this MR?

⚠️ - Potentially untested  
💣 - No test coverage  
🚥 - Test coverage of some sort exists (a test failed when error raised)  
🚦 - Test coverage of return value (a test failed when nil used)  
 - Permissions check tested

### Note lookup without access check

- [x]  app/finders/notes_finder.rb:13 :download_code check
- [x]  app/finders/notes_finder.rb:19 `SnippetsFinder`
- [x]  app/models/note.rb:121 [`Issue#visible_to_user`]
- [x]  lib/gitlab/project_search_results.rb:113
  - This is the only use of `app/models/note.rb:121` above, but importantly has no access checks at all. This means it leaks MR comments and snippets when those features are `team-only` in addition to the issue comments which would be fixed by `app/models/note.rb:121`.
  - It is only called from SearchController where `can?(current_user, :download_code, @project)` is checked, so commit comments are not leaked.

### Previous discussions
- [x] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#b915c5267a63628b0bafd23d37792ae73ceae272_13_13 `: download_code` check on commit
- [x] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#b915c5267a63628b0bafd23d37792ae73ceae272_19_19 `SnippetsFinder` should be used
  - `SnippetsFinder` should check if the snippets feature is enabled -> https://gitlab.com/gitlab-org/gitlab-ce/issues/25223

###  Acceptance criteria met?
- [x] Tests added for new code
- [x] TODO comments removed
- [x] Squashed and removed skipped tests
- [x] Changelog entry
- [ ] State Gitlab versions affected and issue severity in description
- [ ] Create technical debt issue for NotesFinder.
  - Either split into `NotesFinder::ForTarget` and `NotesFinder::Search` or consider object per notable type such as `NotesFinder::OnIssue`. For the first option could create `NotesFinder::Base` which is either inherited from or which can be included in the other two.
  - Avoid case statement anti-pattern in this finder with use of `NotesFinder::OnCommit` etc. Consider something on the finder for this? `Model.finder(user, project)`
  - Move `inc_author` to the controller, and implement `related_notes` to replace `non_diff_notes`/`mr_and_commit_notes`

See merge request !2035
2016-12-15 11:40:12 -03:00

181 lines
6.2 KiB
Ruby

require 'spec_helper'
describe Gitlab::ProjectSearchResults, lib: true do
let(:user) { create(:user) }
let(:project) { create(:project) }
let(:query) { 'hello world' }
describe 'initialize with empty ref' do
let(:results) { described_class.new(user, project, query, '') }
it { expect(results.project).to eq(project) }
it { expect(results.query).to eq('hello world') }
end
describe 'initialize with ref' do
let(:ref) { 'refs/heads/test' }
let(:results) { described_class.new(user, project, query, ref) }
it { expect(results.project).to eq(project) }
it { expect(results.repository_ref).to eq(ref) }
it { expect(results.query).to eq('hello world') }
end
describe 'blob search' do
let(:results) { described_class.new(user, project, 'files').objects('blobs') }
it 'finds by name' do
expect(results).to include(["files/images/wm.svg", nil])
end
it 'finds by content' do
blob = results.select { |result| result.first == "CHANGELOG" }.flatten.last
expect(blob.filename).to eq("CHANGELOG")
end
describe 'parsing results' do
let(:results) { project.repository.search_files_by_content('feature', 'master') }
let(:search_result) { results.first }
subject { described_class.parse_search_result(search_result) }
it "returns a valid OpenStruct object" do
is_expected.to be_an OpenStruct
expect(subject.filename).to eq('CHANGELOG')
expect(subject.basename).to eq('CHANGELOG')
expect(subject.ref).to eq('master')
expect(subject.startline).to eq(188)
expect(subject.data.lines[2]).to eq(" - Feature: Replace teams with group membership\n")
end
context "when filename has extension" do
let(:search_result) { "master:CONTRIBUTE.md:5:- [Contribute to GitLab](#contribute-to-gitlab)\n" }
it { expect(subject.filename).to eq('CONTRIBUTE.md') }
it { expect(subject.basename).to eq('CONTRIBUTE') }
end
context "when file under directory" do
let(:search_result) { "master:a/b/c.md:5:a b c\n" }
it { expect(subject.filename).to eq('a/b/c.md') }
it { expect(subject.basename).to eq('a/b/c') }
end
end
end
it 'does not list issues on private projects' do
issue = create(:issue, project: project)
results = described_class.new(user, project, issue.title)
expect(results.objects('issues')).not_to include issue
end
describe 'confidential issues' do
let(:query) { 'issue' }
let(:author) { create(:user) }
let(:assignee) { create(:user) }
let(:non_member) { create(:user) }
let(:member) { create(:user) }
let(:admin) { create(:admin) }
let(:project) { create(:empty_project, :internal) }
let!(:issue) { create(:issue, project: project, title: 'Issue 1') }
let!(:security_issue_1) { create(:issue, :confidential, project: project, title: 'Security issue 1', author: author) }
let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project, assignee: assignee) }
it 'does not list project confidential issues for non project members' do
results = described_class.new(non_member, project, 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(results.issues_count).to eq 1
end
it 'does not list project confidential issues for project members with guest role' do
project.team << [member, :guest]
results = described_class.new(member, project, 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(results.issues_count).to eq 1
end
it 'lists project confidential issues for author' do
results = described_class.new(author, project, 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(results.issues_count).to eq 2
end
it 'lists project confidential issues for assignee' do
results = described_class.new(assignee, project, 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(results.issues_count).to eq 2
end
it 'lists project confidential issues for project members' do
project.team << [member, :developer]
results = described_class.new(member, project, 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(results.issues_count).to eq 3
end
it 'lists all project issues for admin' do
results = described_class.new(admin, project, 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(results.issues_count).to eq 3
end
end
describe 'notes search' do
it 'lists notes' do
project = create(:empty_project, :public)
note = create(:note, project: project)
results = described_class.new(user, project, note.note)
expect(results.objects('notes')).to include note
end
it "doesn't list issue notes when access is restricted" do
project = create(:empty_project, :public, issues_access_level: ProjectFeature::PRIVATE)
note = create(:note_on_issue, project: project)
results = described_class.new(user, project, note.note)
expect(results.objects('notes')).not_to include note
end
it "doesn't list merge_request notes when access is restricted" do
project = create(:empty_project, :public, merge_requests_access_level: ProjectFeature::PRIVATE)
note = create(:note_on_merge_request, project: project)
results = described_class.new(user, project, note.note)
expect(results.objects('notes')).not_to include note
end
end
end