12db4cc0e7
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
124 lines
3 KiB
Ruby
124 lines
3 KiB
Ruby
module Gitlab
|
|
class ProjectSearchResults < SearchResults
|
|
attr_reader :project, :repository_ref
|
|
|
|
def initialize(current_user, project, query, repository_ref = nil)
|
|
@current_user = current_user
|
|
@project = project
|
|
@repository_ref = repository_ref.presence || project.default_branch
|
|
@query = query
|
|
end
|
|
|
|
def objects(scope, page = nil)
|
|
case scope
|
|
when 'notes'
|
|
notes.page(page).per(per_page)
|
|
when 'blobs'
|
|
Kaminari.paginate_array(blobs).page(page).per(per_page)
|
|
when 'wiki_blobs'
|
|
Kaminari.paginate_array(wiki_blobs).page(page).per(per_page)
|
|
when 'commits'
|
|
Kaminari.paginate_array(commits).page(page).per(per_page)
|
|
else
|
|
super
|
|
end
|
|
end
|
|
|
|
def blobs_count
|
|
@blobs_count ||= blobs.count
|
|
end
|
|
|
|
def notes_count
|
|
@notes_count ||= notes.count
|
|
end
|
|
|
|
def wiki_blobs_count
|
|
@wiki_blobs_count ||= wiki_blobs.count
|
|
end
|
|
|
|
def commits_count
|
|
@commits_count ||= commits.count
|
|
end
|
|
|
|
def self.parse_search_result(result)
|
|
ref = nil
|
|
filename = nil
|
|
basename = nil
|
|
startline = 0
|
|
|
|
result.each_line.each_with_index do |line, index|
|
|
if line =~ /^.*:.*:\d+:/
|
|
ref, filename, startline = line.split(':')
|
|
startline = startline.to_i - index
|
|
extname = Regexp.escape(File.extname(filename))
|
|
basename = filename.sub(/#{extname}$/, '')
|
|
break
|
|
end
|
|
end
|
|
|
|
data = ""
|
|
|
|
result.each_line do |line|
|
|
data << line.sub(ref, '').sub(filename, '').sub(/^:-\d+-/, '').sub(/^::\d+:/, '')
|
|
end
|
|
|
|
OpenStruct.new(
|
|
filename: filename,
|
|
basename: basename,
|
|
ref: ref,
|
|
startline: startline,
|
|
data: data
|
|
)
|
|
end
|
|
|
|
private
|
|
|
|
def blobs
|
|
@blobs ||= begin
|
|
blobs = project.repository.search_files_by_content(query, repository_ref).first(100)
|
|
found_file_names = Set.new
|
|
|
|
results = blobs.map do |blob|
|
|
blob = self.class.parse_search_result(blob)
|
|
found_file_names << blob.filename
|
|
|
|
[blob.filename, blob]
|
|
end
|
|
|
|
project.repository.search_files_by_name(query, repository_ref).first(100).each do |filename|
|
|
results << [filename, nil] unless found_file_names.include?(filename)
|
|
end
|
|
|
|
results.sort_by(&:first)
|
|
end
|
|
end
|
|
|
|
def wiki_blobs
|
|
@wiki_blobs ||= begin
|
|
if project.wiki_enabled? && query.present?
|
|
project_wiki = ProjectWiki.new(project)
|
|
|
|
unless project_wiki.empty?
|
|
project_wiki.search_files(query)
|
|
else
|
|
[]
|
|
end
|
|
else
|
|
[]
|
|
end
|
|
end
|
|
end
|
|
|
|
def notes
|
|
@notes ||= NotesFinder.new(project, @current_user, search: query).execute.user.order('updated_at DESC')
|
|
end
|
|
|
|
def commits
|
|
@commits ||= project.repository.find_commits_by_message(query)
|
|
end
|
|
|
|
def project_ids_relation
|
|
project
|
|
end
|
|
end
|
|
end
|