3bf34face4
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? ⚠️ - 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 ### Issue lookup with access check Using `visible_to_user` likely makes these security issues too. See [Code smells](#code-smells). - [x] 🚦 app/finders/notes_finder.rb:15 [`visible_to_user`] - [x] 🚥 app/views/layouts/nav/_project.html.haml:73 [`visible_to_user`] [`.count`] - [x] ✅ app/services/merge_requests/build_service.rb:84 [`issue.try(:confidential?)`] - [x] ✅ lib/api/issues.rb:112 [`visible_to_user`] - CHANGELOG: Prevented API returning issues set to 'Only team members' to everyone - [x] ✅ lib/api/helpers.rb:126 [`can?(current_user, :read_issue, issue)`] Maybe here too? - [x] ✅ 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 |
||
|---|---|---|
| .. | ||
| access_requests_finder.rb | ||
| branches_finder.rb | ||
| contributed_projects_finder.rb | ||
| group_projects_finder.rb | ||
| groups_finder.rb | ||
| issuable_finder.rb | ||
| issues_finder.rb | ||
| joined_groups_finder.rb | ||
| labels_finder.rb | ||
| merge_requests_finder.rb | ||
| milestones_finder.rb | ||
| move_to_project_finder.rb | ||
| notes_finder.rb | ||
| personal_projects_finder.rb | ||
| pipelines_finder.rb | ||
| projects_finder.rb | ||
| README.md | ||
| snippets_finder.rb | ||
| tags_finder.rb | ||
| todos_finder.rb | ||
| union_finder.rb | ||
Finders
This type of classes responsible for collection items based on different conditions. To prevent lookup methods in models like this:
class Project
def issues_for_user_filtered_by(user, filter)
# A lot of logic not related to project model itself
end
end
issues = project.issues_for_user_filtered_by(user, params)
Better use this:
issues = IssuesFinder.new(project, user, filter).execute
It will help keep models thiner.