kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Add latest changes from gitlab-org/gitlab@master

Browse source
This commit is contained in:
GitLab Bot 2021-02-09 21:09:19 +00:00
parent 16cfd85bcf
commit b5944525b0
44 changed files with 484 additions and 49 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
.gitlab/issue_templates/Dogfooding.md Normal file
View file

@ -0,0 +1,17 @@
<!--Lightweight issue template to encourage Dogfooding and educate team members about the importance of Dogfooding -->
/label ~"dogfooding" ~"group::" ~"section::" ~"Category::"
## Feature to Dogfood
<!--Link to Description of feature (Documentation, Epic, Opportunity Canvas, etc.) -->
## Goals
<!--Level of Dogfooding you are looking for: problem validation, testing, production usage, etc -->
## Progress Tracker
<!--List of tasks (e.g. a table with columns, project, status, issue links similar to what is [done here](https://gitlab.com/gitlab-com/www-gitlab-com/-/issues/8499))-->
## Why Dogfooding is Important
- https://about.gitlab.com/handbook/values/#dogfooding
- https://about.gitlab.com/handbook/product/product-processes/#dogfood-everything
- https://about.gitlab.com/handbook/engineering/#dogfooding

16
app/assets/javascripts/diffs/store/getters.js
View file

@ -4,6 +4,7 @@ import {
INLINE_DIFF_VIEW_TYPE,
INLINE_DIFF_LINES_KEY,
} from '../constants';
import { computeSuggestionCommitMessage } from '../utils/suggestions';
import { parallelizeDiffLines } from './utils';
export * from './getters_versions_dropdowns';
@ -154,3 +155,18 @@ export const diffLines = (state) => (file, unifiedDiffComponents) => {
state.diffViewType === INLINE_DIFF_VIEW_TYPE,
);
};
export function suggestionCommitMessage(state) {
return (values = {}) =>
computeSuggestionCommitMessage({
message: state.defaultSuggestionCommitMessage,
values: {
branch_name: state.branchName,
project_path: state.projectPath,
project_name: state.projectName,
username: state.username,
user_full_name: state.userFullName,
...values,
},
});
}

28
app/assets/javascripts/diffs/utils/suggestions.js Normal file
View file

@ -0,0 +1,28 @@
function removeEmptyProperties(dict) {
const noBlanks = Object.entries(dict).reduce((final, [key, value]) => {
const upd = { ...final };
// The number 0 shouldn't be falsey when we're printing variables
if (value || value === 0) {
upd[key] = value;
}
return upd;
}, {});
return noBlanks;
}
export function computeSuggestionCommitMessage({ message, values = {} } = {}) {
const noEmpties = removeEmptyProperties(values);
const matchPhrases = Object.keys(noEmpties)
.map((key) => `%{${key}}`)
.join('|');
const replacementExpression = new RegExp(`(${matchPhrases})`, 'gm');
return message.replace(replacementExpression, (match) => {
const key = match.replace(/(^%{|}$)/gm, '');
return noEmpties[key];
});
}

26
app/assets/javascripts/notes/components/note_body.vue
View file

@ -2,6 +2,8 @@
/* eslint-disable vue/no-v-html */
import { mapActions, mapGetters, mapState } from 'vuex';
import $ from 'jquery';
import { escape } from 'lodash';
import '~/behaviors/markdown/render_gfm';
import Suggestions from '~/vue_shared/components/markdown/suggestions.vue';
import autosave from '../mixins/autosave';
@ -29,6 +31,11 @@ export default {
required: false,
default: null,
},
file: {
type: Object,
required: false,
default: null,
},
canEdit: {
type: Boolean,
required: true,
@ -46,6 +53,7 @@ export default {
},
computed: {
...mapGetters(['getDiscussion', 'suggestionsCount']),
...mapGetters('diffs', ['suggestionCommitMessage']),
discussion() {
if (!this.note.isDraft) return {};
@ -54,7 +62,6 @@ export default {
...mapState({
batchSuggestionsInfo: (state) => state.notes.batchSuggestionsInfo,
}),
...mapState('diffs', ['defaultSuggestionCommitMessage']),
noteBody() {
return this.note.note;
},
@ -64,6 +71,21 @@ export default {
lineType() {
return this.line ? this.line.type : null;
},
commitMessage() {
// Please see this issue comment for why these
// are hard-coded to 1:
// https://gitlab.com/gitlab-org/gitlab/-/issues/291027#note_468308022
const suggestionsCount = 1;
const filesCount = 1;
const filePaths = this.file ? [this.file.file_path] : [];
const suggestion = this.suggestionCommitMessage({
file_paths: filePaths.join(', '),
suggestions_count: suggestionsCount,
files_count: filesCount,
});
return escape(suggestion);
},
},
mounted() {
this.renderGFM();
@ -135,7 +157,7 @@ export default {
:note-html="note.note_html"
:line-type="lineType"
:help-page-path="helpPagePath"
:default-commit-message="defaultSuggestionCommitMessage"
:default-commit-message="commitMessage"
@apply="applySuggestion"
@applyBatch="applySuggestionBatch"
@addToBatch="addSuggestionToBatch"

1
app/assets/javascripts/notes/components/noteable_note.vue
View file

@ -431,6 +431,7 @@ export default {
ref="noteBody"
:note="note"
:line="line"
:file="diffFile"
:can-edit="note.current_user.can_edit"
:is-editing="isEditing"
:help-page-path="helpPagePath"

1
app/controllers/admin/application_settings_controller.rb
View file

@ -243,6 +243,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
:domain_denylist_file,
:raw_blob_request_limit,
:issues_create_limit,
:notes_create_limit,
:default_branch_name,
disabled_oauth_sign_in_sources: [],
import_sources: [],

14
app/controllers/projects/notes_controller.rb
View file

@ -10,6 +10,7 @@ class Projects::NotesController < Projects::ApplicationController
before_action :authorize_read_note!
before_action :authorize_create_note!, only: [:create]
before_action :authorize_resolve_note!, only: [:resolve, :unresolve]
before_action :create_rate_limit, only: [:create]
feature_category :issue_tracking
@ -90,4 +91,17 @@ class Projects::NotesController < Projects::ApplicationController
def whitelist_query_limiting
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42383')
end
def create_rate_limit
key = :notes_create
return unless rate_limiter.throttled?(key, scope: [current_user])
rate_limiter.log_request(request, "#{key}_request_limit".to_sym, current_user)
render plain: _('This endpoint has been requested too many times. Try again later.'), status: :too_many_requests
end
def rate_limiter
::Gitlab::ApplicationRateLimiter
end
end

9
app/graphql/mutations/notes/create/base.rb
View file

@ -25,6 +25,7 @@ module Mutations
def resolve(args)
noteable = authorized_find!(id: args[:noteable_id])
verify_rate_limit!(current_user)
note = ::Notes::CreateService.new(
noteable.project,
@ -54,6 +55,14 @@ module Mutations
confidential: args[:confidential]
}
end
def verify_rate_limit!(current_user)
rate_limiter, key = ::Gitlab::ApplicationRateLimiter, :notes_create
return unless rate_limiter.throttled?(key, scope: [current_user])
raise Gitlab::Graphql::Errors::ResourceNotAvailable,
'This endpoint has been requested too many times. Try again later.'
end
end
end
end

1
app/helpers/application_settings_helper.rb
View file

@ -328,6 +328,7 @@ module ApplicationSettingsHelper
:email_restrictions_enabled,
:email_restrictions,
:issues_create_limit,
:notes_create_limit,
:raw_blob_request_limit,
:project_import_limit,
:project_export_limit,

3
app/models/application_setting.rb
View file

@ -444,6 +444,9 @@ class ApplicationSetting < ApplicationRecord
presence: true,
numericality: { only_integer: true, greater_than: 0 }
validates :notes_create_limit,
numericality: { only_integer: true, greater_than_or_equal_to: 0 }
attr_encrypted :asset_proxy_secret_key,
mode: :per_attribute_iv,
key: Settings.attr_encrypted_db_key_base_truncated,

1
app/models/application_setting_implementation.rb
View file

@ -93,6 +93,7 @@ module ApplicationSettingImplementation
import_sources: Settings.gitlab['import_sources'],
invisible_captcha_enabled: false,
issues_create_limit: 300,
notes_create_limit: 300,
local_markdown_version: 0,
login_recaptcha_protection_enabled: false,
max_artifacts_size: Settings.artifacts['max_size'],

2
app/services/concerns/alert_management/alert_processing.rb
View file

@ -125,3 +125,5 @@ module AlertManagement
end
end
end
AlertManagement::AlertProcessing.prepend_ee_mod

9
app/views/admin/application_settings/_note_limits.html.haml Normal file
View file

@ -0,0 +1,9 @@
= form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-note-limits-settings'), html: { class: 'fieldset-form' } do |f|
= form_errors(@application_setting)
%fieldset
.form-group
= f.label :notes_create_limit, _('Max requests per minute per user'), class: 'label-bold'
= f.number_field :notes_create_limit, class: 'form-control gl-form-input'
= f.submit _('Save changes'), class: "gl-button btn btn-success", data: { qa_selector: 'save_changes_button' }

11
app/views/admin/application_settings/network.html.haml
View file

@ -61,6 +61,17 @@
.settings-content
= render 'issue_limits'
%section.settings.as-note-limits.no-animate#js-note-limits-settings{ class: ('expanded' if expanded_by_default?) }
.settings-header
%h4
= _('Notes Rate Limits')
%button.btn.gl-button.btn-default.js-settings-toggle{ type: 'button' }
= expanded_by_default? ? _('Collapse') : _('Expand')
%p
= _('Configure limit for notes created per minute by web and API requests.')
.settings-content
= render 'note_limits'
%section.settings.as-import-export-limits.no-animate#js-import-export-limits-settings{ class: ('expanded' if expanded_by_default?) }
.settings-header
%h4

2
app/views/doorkeeper/applications/_delete_form.html.haml
View file

@ -2,7 +2,7 @@
= form_tag oauth_application_path(application) do
%input{ :name => "_method", :type => "hidden", :value => "delete" }/
- if defined? small
= button_tag type: "submit", class: "gl-button btn btn-transparent", data: { confirm: _("Are you sure?") } do
= button_tag type: "submit", class: "gl-button btn btn-default", data: { confirm: _("Are you sure?") } do
%span.sr-only
= _('Destroy')
= sprite_icon('remove')

4
app/views/doorkeeper/applications/index.html.haml
View file

@ -40,8 +40,8 @@
- application.redirect_uri.split.each do |uri|
%div= uri
%td= application.access_tokens.count
%td
= link_to edit_oauth_application_path(application), class: "gl-button btn btn-transparent gl-mr-2" do
%td.gl-display-flex
= link_to edit_oauth_application_path(application), class: "gl-button btn btn-default gl-mr-2" do
%span.sr-only
= _('Edit')
= sprite_icon('pencil')

5
changelogs/unreleased/231173-gl-button.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Apply GitLab UI button styles to buttons in gitlab_slack_application file
merge_request: 53478
author: Yogi (@yo)
type: other

5
changelogs/unreleased/320792-add-an-api-rate-limit-for-new-notes-using-application-rate-limits.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Add application rate limit for Notes creation
merge_request: 53637
author:
type: added

5
changelogs/unreleased/fix-button-alignment-applications.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Fix action button alignment for application inside the table in oauth/applications
merge_request: 52465
author: Yogi (@yo)
type: fixed

6
changelogs/unreleased/tor-defect-commit-message-placeholders.yml Normal file
View file

@ -0,0 +1,6 @@
---
title: Fill default commit message values in the placeholder instead of showing the
variable slugs
merge_request: 52851
author:
type: fixed

9
db/migrate/20210208161207_add_notes_create_limit_to_application_settings.rb Normal file
View file

@ -0,0 +1,9 @@
# frozen_string_literal: true
class AddNotesCreateLimitToApplicationSettings < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
add_column :application_settings, :notes_create_limit, :integer, default: 300, null: false
end
end

1
db/schema_migrations/20210208161207 Normal file
View file

@ -0,0 +1 @@
818fcf0f0fec9d2833b091ef380005a2d485486522fb63e2a7b2fd01dbf1ff79

1
db/structure.sql
View file

@ -9413,6 +9413,7 @@ CREATE TABLE application_settings (
git_two_factor_session_expiry integer DEFAULT 15 NOT NULL,
asset_proxy_allowlist text,
keep_latest_artifact boolean DEFAULT true NOT NULL,
notes_create_limit integer DEFAULT 300 NOT NULL,
CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)),
CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)),
CONSTRAINT check_17d9558205 CHECK ((char_length((kroki_url)::text) <= 1024)),

5
doc/api/notes.md
View file

@ -36,6 +36,11 @@ are paginated.
Read more on [pagination](README.md#pagination).
## Rate limits
To help avoid abuse, you can limit your users to a specific number of `Create` request per minute.
See [Notes rate limits](../user/admin_area/settings/rate_limit_on_notes_creation.md).
## Issues
### List project issue notes

13
doc/security/rate_limits.md
View file

@ -25,11 +25,14 @@ similarly mitigated by a rate limit.
## Admin Area settings
- [Issues rate limits](../user/admin_area/settings/rate_limit_on_issues_creation.md).
- [User and IP rate limits](../user/admin_area/settings/user_and_ip_rate_limits.md).
- [Raw endpoints rate limits](../user/admin_area/settings/rate_limits_on_raw_endpoints.md).
- [Protected paths](../user/admin_area/settings/protected_paths.md).
- [Import/Export rate limits](../user/admin_area/settings/import_export_rate_limits.md).
These are rate limits you can set in the Admin Area of your instance:
- [Import/Export rate limits](../user/admin_area/settings/import_export_rate_limits.md)
- [Issues rate limits](../user/admin_area/settings/rate_limit_on_issues_creation.md)
- [Notes rate limits](../user/admin_area/settings/rate_limit_on_notes_creation.md)
- [Protected paths](../user/admin_area/settings/protected_paths.md)
- [Raw endpoints rate limits](../user/admin_area/settings/rate_limits_on_raw_endpoints.md)
- [User and IP rate limits](../user/admin_area/settings/user_and_ip_rate_limits.md)
## Non-configurable limits

1
doc/user/admin_area/settings/index.md
View file

@ -94,6 +94,7 @@ Access the default page for admin area settings by navigating to **Admin Area >
| [Outbound requests](../../../security/webhooks.md) | Allow requests to the local network from hooks and services. |
| [Protected Paths](protected_paths.md) | Configure paths to be protected by Rack Attack. |
| [Incident Management](../../../operations/incident_management/index.md) Limits | Configure limits on the number of inbound alerts able to be sent to a project. |
| [Notes creation limit](rate_limit_on_notes_creation.md)| Set a rate limit on the note creation requests. |
## Geo

32
doc/user/admin_area/settings/rate_limit_on_notes_creation.md Normal file
View file

@ -0,0 +1,32 @@
---
type: reference
stage: Plan
group: Project Management
info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments
---
# Rate limits on note creation
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53637) in GitLab 13.9.
This setting allows you to rate limit the requests to the note creation endpoint.
To change the note creation rate limit:
1. Go to **Admin Area > Settings > Network**.
1. Expand the **Notes Rate Limits** section.
1. Enter the new value.
1. Select **Save changes**.
This limit is:
- Applied independently per user.
- Not applied per IP address.
The default value is `300`.
Requests over the rate limit are logged into the `auth.log` file.
For example, if you set a limit of 300, requests using the
[Projects::NotesController#create](https://gitlab.com/gitlab-org/gitlab/blob/master/app/controllers/projects/notes_controller.rb)
action exceeding a rate of 300 per minute are blocked. Access to the endpoint is allowed after one minute.

82
doc/user/profile/notifications.md
View file

@ -15,8 +15,9 @@ Notifications are sent via email.
You receive notifications for one of the following reasons:
- You participate in an issue, merge request, epic or design. In this context, _participate_ means comment, or edit.
- You enable notifications in an issue, merge request, or epic. To enable notifications, click the **Notifications** toggle in the sidebar to _on_.
- You participate in an issue, merge request, epic, or design. In this context, _participate_ means comment, or edit.
- You [enable notifications in an issue, merge request, or epic](#notifications-on-issues-merge-requests-and-epics).
- You configured notifications at the [project](#project-notifications) and/or [group](#group-notifications) level.
While notifications are enabled, you receive notification of actions occurring in that issue, merge request, or epic.
@ -25,7 +26,9 @@ Notifications can be blocked by an administrator, preventing them from being sen
## Tuning your notifications
The quantity of notifications can be overwhelming. GitLab allows you to tune the notifications you receive. For example, you may want to be notified about all activity in a specific project, but for others, only be notified when you are mentioned by name.
The number of notifications can be overwhelming. GitLab allows you to tune the notifications you receive.
For example, you might want to be notified about all activity in a specific project.
For other projects, you only need to be notified when you are mentioned by name.
You can tune the notifications you receive by combining your notification settings:
@ -159,49 +162,64 @@ Users are notified of the following events:
| Project moved | Project members (1) | (1) not disabled |
| New release | Project members | Custom notification |
## Issue / Epics / Merge request events
## Notifications on issues, merge requests, and epics
In most of the below cases, the notification is sent to:
To enable notifications on one specific issue, merge request or epic, you need to enable the **Notifications** toggle in the right sidebar.
- **Enable**: If you are not a participant in the discussion on that issue, but
want to receive notifications on each update, subscribe to it.
- **Disable**: If you are receiving notifications for updates to that issue but no
longer want to receive them, unsubscribe from it.
Configuring this notification on an epic doesn't make you automatically subscribed to the issue that are linked to the epic.
For most events, the notification is sent to:
- Participants:
- the author and assignee of the issue/merge request
- authors of comments on the issue/merge request
- anyone mentioned by `@username` in the title or description of the issue, merge request or epic **(ULTIMATE)**
- anyone with notification level "Participating" or higher that is mentioned by `@username` in any of the comments on the issue, merge request, or epic **(ULTIMATE)**
- Watchers: users with notification level "Watch"
- Subscribers: anyone who manually subscribed to the issue, merge request, or epic **(ULTIMATE)**
- Custom: Users with notification level "custom" who turned on notifications for any of the events present in the table below
- The author and assignee of the issue/merge request.
- Authors of comments on the issue/merge request.
- Anyone mentioned by `@username` in the title or description of the issue, merge request or epic.
- Anyone with notification level "Participating" or higher that is mentioned by `@username` in any of the comments on the issue, merge request, or epic.
- Watchers: users with notification level "Watch".
- Subscribers: anyone who manually subscribed to the issue, merge request, or epic.
- Custom: Users with notification level "custom" who turned on notifications for any of the events present in the table below.
NOTE:
To minimize the number of notifications that do not require any action, in [GitLab versions 12.9 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/616), eligible approvers are no longer notified for all the activities in their projects. To receive them they have to change their user notification settings to **Watch** instead.
To minimize the number of notifications that do not require any action, in
[GitLab versions 12.9 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/616), eligible
approvers are no longer notified for all the activities in their projects. To receive them they have
to change their user notification settings to **Watch** instead.
The following table presents the events that generate notifications for issues, merge requests, and
epics:
| Event | Sent to |
|------------------------|---------|
| New issue | |
| Close issue | |
| Reassign issue | The above, plus the old assignee |
| Reopen issue | |
| Due issue | Participants and Custom notification level with this event selected |
| Change milestone issue | Subscribers, participants mentioned, and Custom notification level with this event selected |
| Remove milestone issue | Subscribers, participants mentioned, and Custom notification level with this event selected |
| Change milestone merge request | Subscribers, participants mentioned, and Custom notification level with this event selected |
| Close epic | |
| Close issue | |
| Close merge request | |
| Due issue | Participants and Custom notification level with this event selected |
| Failed pipeline | The author of the pipeline |
| Fixed pipeline ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/24309) in GitLab 13.1.) | The author of the pipeline. Enabled by default. |
| Merge merge request | |
| Merge when pipeline succeeds ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/211961) in GitLab 13.4) | |
| New comment | Participants, Watchers, Subscribers, and Custom notification level with this event selected, plus anyone mentioned by `@username` in the comment, with notification level "Mention" or higher |
| New epic | |
| New issue | |
| New merge request | |
| Push to merge request | Participants and Custom notification level with this event selected |
| Reassign merge request | The above, plus the old assignee |
| Close merge request | |
| Reopen merge request | |
| Merge merge request | |
| Merge when pipeline succeeds ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/211961) in GitLab 13.4) | |
| Change milestone merge request | Subscribers, participants mentioned, and Custom notification level with this event selected |
| Reassign issue | Participants, Watchers, Subscribers, and Custom notification level with this event selected, plus the old assignee |
| Reassign merge request | Participants, Watchers, Subscribers, and Custom notification level with this event selected, plus the old assignee |
| Remove milestone issue | Subscribers, participants mentioned, and Custom notification level with this event selected |
| Remove milestone merge request | Subscribers, participants mentioned, and Custom notification level with this event selected |
| New comment | The above, plus anyone mentioned by `@username` in the comment, with notification level "Mention" or higher |
| Failed pipeline | The author of the pipeline |
| Fixed pipeline | The author of the pipeline. Enabled by default. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/24309) in GitLab 13.1. |
| Reopen epic | |
| Reopen issue | |
| Reopen merge request | |
| Successful pipeline | The author of the pipeline, if they have the custom notification setting for successful pipelines set. If the pipeline failed previously, a `Fixed pipeline` message is sent for the first successful pipeline after the failure, then a `Successful pipeline` message for any further successful pipelines. |
| New epic **(ULTIMATE)** | |
| Close epic **(ULTIMATE)** | |
| Reopen epic **(ULTIMATE)** | |
In addition, if the title or description of an Issue or Merge Request is
If the title or description of an issue or merge request is
changed, notifications are sent to any **new** mentions by `@username` as
if they had been mentioned in the original text.

7
doc/user/project/issues/issue_data_and_actions.md
View file

@ -161,14 +161,9 @@ or were mentioned in the description or threads.
### Notifications
Click on the icon to enable/disable [notifications](../../profile/notifications.md#issue--epics--merge-request-events)
Select the toggle to enable or disable [notifications](../../profile/notifications.md#notifications-on-issues-merge-requests-and-epics)
for the issue. Notifications are automatically enabled after you participate in the issue in any way.
- **Enable**: If you are not a participant in the discussion on that issue, but
want to receive notifications on each update, subscribe to it.
- **Disable**: If you are receiving notifications for updates to that issue but no
longer want to receive them, unsubscribe from it.
### Reference
- A quick "copy" button for that issue's reference, which looks like

2
lib/api/issues.rb
View file

@ -231,7 +231,7 @@ module API
post ':id/issues' do
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42320')
check_rate_limit! :issues_create, [current_user, :issues_create]
check_rate_limit! :issues_create, [current_user]
authorize! :create_issue, user_project

2
lib/api/notes.rb
View file

@ -4,6 +4,7 @@ module API
class Notes < ::API::Base
include PaginationParams
helpers ::API::Helpers::NotesHelpers
helpers Helpers::RateLimiter
before { authenticate! }
@ -72,6 +73,7 @@ module API
optional :created_at, type: String, desc: 'The creation date of the note'
end
post ":id/#{noteables_str}/:noteable_id/notes", feature_category: feature_category do
check_rate_limit! :notes_create, [current_user]
noteable = find_noteable(noteable_type, params[:noteable_id])
opts = {

1
lib/gitlab/application_rate_limiter.rb
View file

@ -20,6 +20,7 @@ module Gitlab
def rate_limits
{
issues_create: { threshold: -> { application_settings.issues_create_limit }, interval: 1.minute },
notes_create: { threshold: -> { application_settings.notes_create_limit }, interval: 1.minute },
project_export: { threshold: -> { application_settings.project_export_limit }, interval: 1.minute },
project_download_export: { threshold: -> { application_settings.project_download_export_limit }, interval: 1.minute },
project_repositories_archive: { threshold: 5, interval: 1.minute },

9
locale/gitlab.pot
View file

@ -7504,6 +7504,9 @@ msgstr ""
msgid "Configure limit for issues created per minute by web and API requests."
msgstr ""
msgid "Configure limit for notes created per minute by web and API requests."
msgstr ""
msgid "Configure limits for Project/Group Import/Export."
msgstr ""
@ -17964,6 +17967,9 @@ msgstr ""
msgid "Max file size is 200 KB."
msgstr ""
msgid "Max requests per minute per user"
msgstr ""
msgid "Max role"
msgstr ""
@ -20143,6 +20149,9 @@ msgstr ""
msgid "NoteForm|Note"
msgstr ""
msgid "Notes Rate Limits"
msgstr ""
msgid "Notes|Are you sure you want to cancel creating this comment?"
msgstr ""

36
spec/controllers/projects/notes_controller_spec.rb
View file

@ -727,6 +727,42 @@ RSpec.describe Projects::NotesController do
end
end
end
context 'when the endpoint receives requests above the limit' do
before do
stub_application_setting(notes_create_limit: 5)
end
it 'prevents from creating more notes', :request_store do
5.times { create! }
expect { create! }
.to change { Gitlab::GitalyClient.get_request_count }.by(0)
create!
expect(response.body).to eq(_('This endpoint has been requested too many times. Try again later.'))
expect(response).to have_gitlab_http_status(:too_many_requests)
end
it 'logs the event in auth.log' do
attributes = {
message: 'Application_Rate_Limiter_Request',
env: :notes_create_request_limit,
remote_ip: '0.0.0.0',
request_method: 'POST',
path: "/#{project.full_path}/notes",
user_id: user.id,
username: user.username
}
expect(Gitlab::AuthLogger).to receive(:error).with(attributes).once
project.add_developer(user)
sign_in(user)
6.times { create! }
end
end
end
describe 'PUT update' do

60
spec/frontend/diffs/store/getters_spec.js
View file

@ -375,4 +375,64 @@ describe('Diffs Module Getters', () => {
});
});
});
describe('suggestionCommitMessage', () => {
beforeEach(() => {
Object.assign(localState, {
defaultSuggestionCommitMessage:
'%{branch_name}%{project_path}%{project_name}%{username}%{user_full_name}%{file_paths}%{suggestions_count}%{files_count}',
branchName: 'branch',
projectPath: '/path',
projectName: 'name',
username: 'user',
userFullName: 'user userton',
});
});
it.each`
specialState | output
${{}} | ${'branch/pathnameuseruser userton%{file_paths}%{suggestions_count}%{files_count}'}
${{ userFullName: null }} | ${'branch/pathnameuser%{user_full_name}%{file_paths}%{suggestions_count}%{files_count}'}
${{ username: null }} | ${'branch/pathname%{username}user userton%{file_paths}%{suggestions_count}%{files_count}'}
${{ projectName: null }} | ${'branch/path%{project_name}useruser userton%{file_paths}%{suggestions_count}%{files_count}'}
${{ projectPath: null }} | ${'branch%{project_path}nameuseruser userton%{file_paths}%{suggestions_count}%{files_count}'}
${{ branchName: null }} | ${'%{branch_name}/pathnameuseruser userton%{file_paths}%{suggestions_count}%{files_count}'}
`(
'provides the correct "base" default commit message based on state ($specialState)',
({ specialState, output }) => {
Object.assign(localState, specialState);
expect(getters.suggestionCommitMessage(localState)()).toBe(output);
},
);
it.each`
stateOverrides | output
${{}} | ${'branch/pathnameuseruser userton%{file_paths}%{suggestions_count}%{files_count}'}
${{ user_full_name: null }} | ${'branch/pathnameuser%{user_full_name}%{file_paths}%{suggestions_count}%{files_count}'}
${{ username: null }} | ${'branch/pathname%{username}user userton%{file_paths}%{suggestions_count}%{files_count}'}
${{ project_name: null }} | ${'branch/path%{project_name}useruser userton%{file_paths}%{suggestions_count}%{files_count}'}
${{ project_path: null }} | ${'branch%{project_path}nameuseruser userton%{file_paths}%{suggestions_count}%{files_count}'}
${{ branch_name: null }} | ${'%{branch_name}/pathnameuseruser userton%{file_paths}%{suggestions_count}%{files_count}'}
`(
"properly overrides state values ($stateOverrides) if they're provided",
({ stateOverrides, output }) => {
expect(getters.suggestionCommitMessage(localState)(stateOverrides)).toBe(output);
},
);
it.each`
providedValues | output
${{ file_paths: 'path1, path2', suggestions_count: 1, files_count: 1 }} | ${'branch/pathnameuseruser usertonpath1, path211'}
${{ suggestions_count: 1, files_count: 1 }} | ${'branch/pathnameuseruser userton%{file_paths}11'}
${{ file_paths: 'path1, path2', files_count: 1 }} | ${'branch/pathnameuseruser usertonpath1, path2%{suggestions_count}1'}
${{ file_paths: 'path1, path2', suggestions_count: 1 }} | ${'branch/pathnameuseruser usertonpath1, path21%{files_count}'}
${{ something_unused: 'CrAzY TeXt' }} | ${'branch/pathnameuseruser userton%{file_paths}%{suggestions_count}%{files_count}'}
`(
"fills in any missing interpolations ($providedValues) when they're provided at the getter callsite",
({ providedValues, output }) => {
expect(getters.suggestionCommitMessage(localState)(providedValues)).toBe(output);
},
);
});
});

15
spec/frontend/diffs/utils/suggestions_spec.js Normal file
View file

@ -0,0 +1,15 @@
import { computeSuggestionCommitMessage } from '~/diffs/utils/suggestions';
describe('Diff Suggestions utilities', () => {
describe('computeSuggestionCommitMessage', () => {
it.each`
description | input | values | output
${'makes the appropriate replacements'} | ${'%{foo} %{bar}'} | ${{ foo: 'foo', bar: 'bar' }} | ${'foo bar'}
${"skips replacing values that aren't passed"} | ${'%{foo} %{bar}'} | ${{ foo: 'foo' }} | ${'foo %{bar}'}
${'treats the number 0 as a valid value (not falsey)'} | ${'%{foo} %{bar}'} | ${{ foo: 'foo', bar: 0 }} | ${'foo 0'}
${"works when the variables don't have any space between them"} | ${'%{foo}%{bar}'} | ${{ foo: 'foo', bar: 'bar' }} | ${'foobar'}
`('$description', ({ input, output, values }) => {
expect(computeSuggestionCommitMessage({ message: input, values })).toBe(output);
});
});
});

54
spec/frontend/notes/components/note_body_spec.js
View file

@ -1,6 +1,14 @@
import Vue from 'vue';
import { shallowMount } from '@vue/test-utils';
import Vuex from 'vuex';
import notes from '~/notes/stores/modules/index';
import createStore from '~/notes/stores';
import { suggestionCommitMessage } from '~/diffs/store/getters';
import Suggestions from '~/vue_shared/components/markdown/suggestions.vue';
import noteBody from '~/notes/components/note_body.vue';
import { noteableDataMock, notesDataMock, note } from '../mock_data';
describe('issue_note_body component', () => {
@ -54,4 +62,50 @@ describe('issue_note_body component', () => {
expect(vm.autosave.key).toEqual(autosaveKey);
});
});
describe('commitMessage', () => {
let wrapper;
Vue.use(Vuex);
beforeEach(() => {
const notesStore = notes();
notesStore.state.notes = {};
store = new Vuex.Store({
modules: {
notes: notesStore,
diffs: {
namespaced: true,
state: {
defaultSuggestionCommitMessage:
'%{branch_name}%{project_path}%{project_name}%{username}%{user_full_name}%{file_paths}%{suggestions_count}%{files_count}',
branchName: 'branch',
projectPath: '/path',
projectName: 'name',
username: 'user',
userFullName: 'user userton',
},
getters: { suggestionCommitMessage },
},
},
});
wrapper = shallowMount(noteBody, {
store,
propsData: {
note: { ...note, suggestions: [12345] },
canEdit: true,
file: { file_path: 'abc' },
},
});
});
it('passes the correct default placeholder commit message for a suggestion to the suggestions component', () => {
const commitMessage = wrapper.find(Suggestions).attributes('defaultcommitmessage');
expect(commitMessage).toBe('branch/pathnameuseruser usertonabc11');
});
});
});

1
spec/lib/gitlab/import_export/all_models.yml
View file

@ -561,6 +561,7 @@ project:
- alert_management_http_integrations
- exported_protected_branches
- incident_management_oncall_schedules
- incident_management_oncall_rotations
- debian_distributions
- merge_request_metrics
award_emoji:

6
spec/models/application_setting_spec.rb
View file

@ -114,6 +114,12 @@ RSpec.describe ApplicationSetting do
it { is_expected.to allow_value(nil).for(:repository_storages_weighted_default) }
it { is_expected.not_to allow_value({ default: 100, shouldntexist: 50 }).for(:repository_storages_weighted) }
it { is_expected.to allow_value(400).for(:notes_create_limit) }
it { is_expected.not_to allow_value('two').for(:notes_create_limit) }
it { is_expected.not_to allow_value(nil).for(:notes_create_limit) }
it { is_expected.not_to allow_value(5.5).for(:notes_create_limit) }
it { is_expected.not_to allow_value(-2).for(:notes_create_limit) }
context 'help_page_documentation_base_url validations' do
it { is_expected.to allow_value(nil).for(:help_page_documentation_base_url) }
it { is_expected.to allow_value('https://docs.gitlab.com').for(:help_page_documentation_base_url) }

2
spec/requests/api/graphql/mutations/notes/create/diff_note_spec.rb
View file

@ -43,6 +43,8 @@ RSpec.describe 'Adding a DiffNote' do
it_behaves_like 'a Note mutation when there are active record validation errors', model: DiffNote
it_behaves_like 'a Note mutation when there are rate limit validation errors'
context do
let(:diff_refs) { build(:commit).diff_refs } # Allow fake diff refs so arguments are valid

2
spec/requests/api/graphql/mutations/notes/create/image_diff_note_spec.rb
View file

@ -46,6 +46,8 @@ RSpec.describe 'Adding an image DiffNote' do
it_behaves_like 'a Note mutation when there are active record validation errors', model: DiffNote
it_behaves_like 'a Note mutation when there are rate limit validation errors'
context do
let(:diff_refs) { build(:commit).diff_refs } # Allow fake diff refs so arguments are valid

2
spec/requests/api/graphql/mutations/notes/create/note_spec.rb
View file

@ -37,6 +37,8 @@ RSpec.describe 'Adding a Note' do
it_behaves_like 'a Note mutation when the given resource id is not for a Noteable'
it_behaves_like 'a Note mutation when there are rate limit validation errors'
it 'returns the note' do
post_graphql_mutation(mutation, current_user: current_user)

11
spec/support/shared_examples/graphql/notes_creation_shared_examples.rb
View file

@ -64,3 +64,14 @@ RSpec.shared_examples 'a Note mutation when the given resource id is not for a N
let(:match_errors) { include(/does not represent an instance of Note/) }
end
end
RSpec.shared_examples 'a Note mutation when there are rate limit validation errors' do
before do
stub_application_setting(notes_create_limit: 3)
3.times { post_graphql_mutation(mutation, current_user: current_user) }
end
it_behaves_like 'a Note mutation that does not create a Note'
it_behaves_like 'a mutation that returns top-level errors',
errors: ['This endpoint has been requested too many times. Try again later.']
end

13
spec/support/shared_examples/requests/api/notes_shared_examples.rb
View file

@ -274,6 +274,19 @@ RSpec.shared_examples 'noteable API' do |parent_type, noteable_type, id_name|
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'when request exceeds the rate limit' do
before do
allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true)
end
it 'prevents users from creating more notes' do
post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user), params: { body: 'hi!' }
expect(response).to have_gitlab_http_status(:too_many_requests)
expect(json_response['message']['error']).to eq('This endpoint has been requested too many times. Try again later.')
end
end
end
describe "PUT /#{parent_type}/:id/#{noteable_type}/:noteable_id/notes/:note_id" do