This is to be more consistent as there is already a :read_note policy in
NotePolicy. To keep other behaviour the same we've introduced a
Note#noteable_ability_name that is used anywhere this was expected.
Because we don't have any destroy callbacks (or other logic
triggered on event destroy), there is no reason for deleting events
inefficiently one by one, instead we can use :delete_all.
Limiting the size of issuable description and comments to 1_000_000,
which is close to ~1MB of ASCII characters, which represents 99.9% of
all descriptions and comments we have in DB at the moment. This should
help prevent DoS attacks when comments contain refference strings.
Also this change updates regexp matching the namespaces paths by
limiting the namespaces paths to Namespace::NUMBER_OF_ANCESTORS_ALLOWED,
as we allow 20 levels deep groups.
see https://gitlab.com/gitlab-org/gitlab-ce/issues/61974#note_191274234
When used with a Hash, `.keys.include?` is bad because:
1. It performs a O(n) search instead of the efficient `.has_key?`
2. It clones all keys into separate array.
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/64975
This exposes `Note`s on Issues & MergeRequests using a
`Types::Notes::NoteableType` in GraphQL.
Exposing notes on a new type can be done by implementing the
`NoteableType` interface on the type. The presented object should
be a `Noteable`.
There is a combination of few strategies implemented here:
1. Few relations were eager loaded
2. Changed few polymorphic routes to specific ones so we don't have to
use `#becomes(Namespace)` which doesn't preserve association cache
It's possible that user pastes accidentally also unsubscribe link
which is included in footer of notification emails. This unsubscribe
link contains personal token which attacker then use to act as the
original user (e.g. for sending comments under his/her identity).
This commit adds the module `FromUnion`, which provides the class method
`from_union`. This simplifies the process of selecting data from the
result of a UNION, and reduces the likelihood of making mistakes. As a
result, instead of this:
union = Gitlab::SQL::Union.new([foo, bar])
Foo.from("(#{union.to_sql}) #{Foo.table_name}")
We can now write this instead:
Foo.from_union([foo, bar])
This commit also includes some changes to make this new setup work
properly. For example, a bug in Rails 4
(https://github.com/rails/rails/issues/24193) would break the use of
`from("sub-query-here").includes(:relation)` in certain cases. There was
also a CI query which appeared to repeat a lot of conditions from an
outer query on an inner query, which isn't necessary.
Finally, we include a RuboCop cop to ensure developers use this new
module, instead of using Gitlab::SQL::Union directly.
Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/51307
The status is shown for
- The author of a commit when viewing a commit
- Notes on a commit (regular/diff)
- The user that triggered a pipeline when viewing a pipeline
- The author of a merge request when viewing a merge request
- The author of notes on a merge request (regular/diff)
- The author of an issue when viewing an issue
- The author of notes on an issue
- The author of a snippet when viewing a snippet
- The author of notes on a snippet
- A user's profile page
- The list of members of a group/user
This is tricky: the query was being run in
`ObjectStorage::Extension::RecordsUploads#retrieve_from_store!`, but we can't
just add batch loading there, because the `#upload=` method there would use the
result immediately, making the batch only have one item.
Instead, we can pre-emptively add an item to the batch whenever an avatarable
object is initialized, and then reuse that batch item in
`#retrieve_from_store!`. However, this also has problems:
1. There is a lot of logic in `Avatarable#retrieve_upload_from_batch`.
2. Some of that logic constructs a 'fake' model for the batch key. This should
be fine, because of ActiveRecord's override of `#==`, but it relies on that
staying the same.
We request Gitaly in a N+1 manner to build discussion diffs. Once the diffs are from different revisions, it's hard to make a single request to the service in order to build the whole response.
With this change we solve this problem and simplify a lot fetching this piece of info.
Updates notes-related services and rendering so this code can be
easily used for group-scoped resources (specifically Epics).
Related to gitlab-ee!5205
[10.6] Prevent notes on confidential issues from being sent to chat
See merge request gitlab/gitlabhq!2366
# Conflicts:
# app/helpers/services_helper.rb
* adds basic discussions API for issues and snippets
* reorganizes notes specs (so same tests can be used for all noteable types - issues, MRs, snippets)
When a note is part of a discussion, the email sent out will be
`In-Reply-To` the previous note in that discussion. It also
`References` all the previous notes in that discussion, and the
original issue.
Closesgitlab-org/gitlab-ce#36054.
This throttles the number of UPDATE queries that can be triggered by
calling "touch" on a Note, Issue, or MergeRequest. For Note objects we
also take care of updating the associated "noteable" relation in a
smarter way than Rails does by default.
this will remove the need make N queries (per-note) at the
cost of having to mark notes with an attribute
this opens up the possibility for other special roles for notes
* master: (42 commits)
alternative route for download archive
Add missing command to compile GetText files
Prevent user from changing username with container registry tags
Rename RPC 'Exists' to 'RepositoryExists'
Fix the sticky changes bar on commits page
Fix ee_compat_check when EE branch uses a prefix
Removed display styles when hiding the fly out navigation
Explain why we use select all for project_url_constrainer.rb
Fix pikaday being undefined
Add a helper to stub storage settings with defaults
Enable the Layout/SpaceBeforeBlockBraces cop
update Install from Source instructions
Translations can be picked without asking for exceptions
Ask for exceptions in advance
Don't require stackprof in Gemfile
Synchronous Korean translation in zanata
Use full path of user's avatar in webhooks
Update icon color on hover
Align all nav items in sidebar
Fix height of collapsed sidebar items
...
This is allowed for existing instances so we don't end up 76 offenses
right away, but for new code one should _only_ use this if they _have_
to remove non database data. Even then it's usually better to do this in
a service class as this gives you more control over how to remove the
data (e.g. in bulk).
Fix https://gitlab.com/gitlab-org/gitlab-ce/issues/27070
Deprecate "chat commands" in favor of "slash commands"
We looked for things like:
- `slash commmand`
- `slash_command`
- `slash-command`
- `SlashCommand`
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
GitLab Bot
GitLab Release Tools Bot
Kerri Miller
Dylan Griffith
Dmitriy Zaporozhets
Jan Provaznik
Michael Kozono
Stan Hu
dineshpanda
Patrick Derichs
Alexandru Croitor
Luke Duncalfe
Takuya Noguchi
Mario de la Ossa
Bob Van Landuyt
Nick Thomas
Heinrich Lee Yu
Jarka Košanová
Oswaldo Ferreira
Gabriel Mazetto
Cindy Pallares
Felipe Artur
Brett Walker
Yorick Peterse
Sean McGivern
Rémy Coutable
gfyoung
Douwe Maan
Sean McGivern
Satish Perala
Fatih Acet
Andreas Brandl
Mario de la Ossa
Micaël Bergeron
Toon Claes
micael.bergeron
Douwe Maan
Jen-Shin Lin
micael.bergeron
Filipa Lacerda
Lin Jen-Shin
Grzegorz Bizon
Eric Eastwood
Luke "Jared" Bennett
blackst0ne
Adam Niedzielski
mhasbini
Alfredo Sumaran
Felipe Artur