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
- Removed unnecessary column from `SpamLog`
- Moved creation of SpamLogs out of its own service and into SpamCheckService
- Simplified code in SpamCheckService.
- Moved move spam related code into Spammable concern
- Merged `AkismetSubmittable` into `Spammable`
- Clean up `SpamCheckService`
- Added tests for `Spammable`
- Added submit (ham or spam) options to `AkismetHelper`
This was not a clear cut n+1 query, given that if you're directly subscribed to all issues
that the API is returning you never really need to check for the notes. However, if you're
subscribed to _all_ of them, then for each issuable you need to go once to `notes`, and
once to `users` (for the authors). By preemtively loading notes and authors, at worst you have
1 extra query, and at best you saved 2n extra queries. We also took advantage of this preloading
of notes when counting user notes.
Rename abilities to correspond contoller/model action names
write_ was renamed to create_
modify_ was renamed to update_
So now in update action we have next code
```
def create
can?(current_user, :create_issue, @issue)
end
def update
can?(current_user, :update_issue, @issue)
end
```
See merge request !896
write_ was renamed to create_
modify_ was renamed to update_
So now in update action we have next code
def create
can?(current_user, :create_issue, @issue)
end
def update
can?(current_user, :update_issue, @issue)
end
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
* users (#6878, #3526, #4209): Validation error messages are now exposed through 400 responses, 409 response are sent in case of duplicate email or username
* MRs (#5335): 409 responses are sent in case of duplicate merge request (source/target branches), 422 responses are sent when submiting MR fo/from unrelated forks
* issues
* labels
* projects
Douwe Maan
Rémy Coutable
Douglas Barbosa Alexandre
Sean McGivern
Robert Schilling
tiagonbotelho
Patricio Cano
Marc Siegfriedt
Alejandro Rodríguez
Ahmad Sherif
Stan Hu
Michael Greene
Zeger-Jan van de Weg
Dmitriy Zaporozhets
jubianchi
jubianchi