Add latest changes from gitlab-org/gitlab@master
This commit is contained in:
parent
7f8df34323
commit
4ae83df07e
32 changed files with 532 additions and 111 deletions
|
@ -8,8 +8,7 @@
|
|||
|
||||
- [ ] If your proposal includes changes to the top-level menu items within the left sidebar, engage the [Foundations Product Design Manager](https://about.gitlab.com/handbook/product/categories/#foundations-group) for approval. The Foundations DRI will work with UX partners in product design, research, and technical writing, as applicable.
|
||||
- [ ] Follow the [product development workflow](https://about.gitlab.com/handbook/product-development-flow/#validation-phase-2-problem-validation) validation process to ensure you are solving a well understood problem and that the proposed change is understandable and non-disruptive to users. Navigation-specific research is strongly encouraged.
|
||||
- [ ] Engage the [Editor](https://about.gitlab.com/handbook/engineering/development/dev/create-editor/) team to ensure your proposal is in alignment with holistic changes happening to the left side bar.
|
||||
- [ ] Engage the [Foundations](https://about.gitlab.com/handbook/product/categories/#foundations-group) team to ensure your proposal is in alignment with holistic changes happening to the left side bar.
|
||||
- [ ] Consider whether you need to communicate the change somehow, or if you will have an interim period in the UI where your nav item will live in more than one place.
|
||||
- [ ] Once implemented, update this [navigation map in Mural](https://app.mural.co/t/gitlab2474/m/gitlab2474/1589571490215/261462d0beb3043979374623710d3f2d6cfec1cb) with your navigation change.
|
||||
|
||||
/label ~UX ~"UI text" ~"documentation" ~"documentation" ~"Category:Navigation & Settings" ~"Category:Foundations" ~navigation
|
||||
|
|
|
@ -108,6 +108,12 @@ class ApplicationController < ActionController::Base
|
|||
head :forbidden, retry_after: Gitlab::Auth::UniqueIpsLimiter.config.unique_ips_limit_time_window
|
||||
end
|
||||
|
||||
rescue_from RateLimitedService::RateLimitedError do |e|
|
||||
e.log_request(request, current_user)
|
||||
response.headers.merge!(e.headers)
|
||||
render plain: e.message, status: :too_many_requests
|
||||
end
|
||||
|
||||
def redirect_back_or_default(default: root_path, options: {})
|
||||
redirect_back(fallback_location: default, **options)
|
||||
end
|
||||
|
|
|
@ -37,7 +37,7 @@ class Projects::IssuesController < Projects::ApplicationController
|
|||
before_action :authorize_download_code!, only: [:related_branches]
|
||||
|
||||
# Limit the amount of issues created per minute
|
||||
before_action :create_rate_limit, only: [:create]
|
||||
before_action :create_rate_limit, only: [:create], if: -> { Feature.disabled?('rate_limited_service_issues_create', project, default_enabled: :yaml) }
|
||||
|
||||
before_action do
|
||||
push_frontend_feature_flag(:tribute_autocomplete, @project)
|
||||
|
|
|
@ -125,15 +125,6 @@ module IntegrationsHelper
|
|||
!Gitlab.com?
|
||||
end
|
||||
|
||||
def integration_tabs(integration:)
|
||||
[
|
||||
{ key: 'edit', text: _('Settings'), href: scoped_edit_integration_path(integration) },
|
||||
(
|
||||
{ key: 'overrides', text: s_('Integrations|Projects using custom settings'), href: scoped_overrides_integration_path(integration) } if integration.instance_level?
|
||||
)
|
||||
].compact
|
||||
end
|
||||
|
||||
def jira_issue_breadcrumb_link(issue_reference)
|
||||
link_to '', { class: 'gl-display-flex gl-align-items-center gl-white-space-nowrap' } do
|
||||
icon = image_tag image_path('illustrations/logos/jira.svg'), width: 15, height: 15, class: 'gl-mr-2'
|
||||
|
|
93
app/services/concerns/rate_limited_service.rb
Normal file
93
app/services/concerns/rate_limited_service.rb
Normal file
|
@ -0,0 +1,93 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
module RateLimitedService
|
||||
extend ActiveSupport::Concern
|
||||
|
||||
RateLimitedNotSetupError = Class.new(StandardError)
|
||||
|
||||
class RateLimitedError < StandardError
|
||||
def initialize(key:, rate_limiter:)
|
||||
@key = key
|
||||
@rate_limiter = rate_limiter
|
||||
end
|
||||
|
||||
def headers
|
||||
# TODO: This will be fleshed out in https://gitlab.com/gitlab-org/gitlab/-/issues/342370
|
||||
{}
|
||||
end
|
||||
|
||||
def log_request(request, current_user)
|
||||
rate_limiter.class.log_request(request, "#{key}_request_limit".to_sym, current_user)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
attr_reader :key, :rate_limiter
|
||||
end
|
||||
|
||||
class RateLimiterScopedAndKeyed
|
||||
attr_reader :key, :opts, :rate_limiter_klass
|
||||
|
||||
def initialize(key:, opts:, rate_limiter_klass:)
|
||||
@key = key
|
||||
@opts = opts
|
||||
@rate_limiter_klass = rate_limiter_klass
|
||||
end
|
||||
|
||||
def rate_limit!(service)
|
||||
evaluated_scope = evaluated_scope_for(service)
|
||||
return if feature_flag_disabled?(evaluated_scope[:project])
|
||||
|
||||
rate_limiter = new_rate_limiter(evaluated_scope)
|
||||
if rate_limiter.throttled?
|
||||
raise RateLimitedError.new(key: key, rate_limiter: rate_limiter), _('This endpoint has been requested too many times. Try again later.')
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def users_allowlist
|
||||
@users_allowlist ||= opts[:users_allowlist] ? opts[:users_allowlist].call : []
|
||||
end
|
||||
|
||||
def evaluated_scope_for(service)
|
||||
opts[:scope].each_with_object({}) do |var, all|
|
||||
all[var] = service.public_send(var) # rubocop: disable GitlabSecurity/PublicSend
|
||||
end
|
||||
end
|
||||
|
||||
def feature_flag_disabled?(project)
|
||||
Feature.disabled?("rate_limited_service_#{key}", project, default_enabled: :yaml)
|
||||
end
|
||||
|
||||
def new_rate_limiter(evaluated_scope)
|
||||
rate_limiter_klass.new(key, **opts.merge(scope: evaluated_scope.values, users_allowlist: users_allowlist))
|
||||
end
|
||||
end
|
||||
|
||||
prepended do
|
||||
attr_accessor :rate_limiter_bypassed
|
||||
cattr_accessor :rate_limiter_scoped_and_keyed
|
||||
|
||||
def self.rate_limit(key:, opts:, rate_limiter_klass: ::Gitlab::ApplicationRateLimiter)
|
||||
self.rate_limiter_scoped_and_keyed = RateLimiterScopedAndKeyed.new(key: key,
|
||||
opts: opts,
|
||||
rate_limiter_klass: rate_limiter_klass)
|
||||
end
|
||||
end
|
||||
|
||||
def execute_without_rate_limiting(*args, **kwargs)
|
||||
self.rate_limiter_bypassed = true
|
||||
execute(*args, **kwargs)
|
||||
ensure
|
||||
self.rate_limiter_bypassed = false
|
||||
end
|
||||
|
||||
def execute(*args, **kwargs)
|
||||
raise RateLimitedNotSetupError if rate_limiter_scoped_and_keyed.nil?
|
||||
|
||||
rate_limiter_scoped_and_keyed.rate_limit!(self) unless rate_limiter_bypassed
|
||||
|
||||
super
|
||||
end
|
||||
end
|
|
@ -3,6 +3,10 @@
|
|||
module Issues
|
||||
class CreateService < Issues::BaseService
|
||||
include ResolveDiscussions
|
||||
prepend RateLimitedService
|
||||
|
||||
rate_limit key: :issues_create,
|
||||
opts: { scope: [:project, :current_user], users_allowlist: -> { [User.support_bot.username] } }
|
||||
|
||||
# NOTE: For Issues::CreateService, we require the spam_params and do not default it to nil, because
|
||||
# spam_checking is likely to be necessary. However, if there is not a request available in scope
|
||||
|
|
|
@ -1,18 +1,11 @@
|
|||
- active_tab = local_assigns.fetch(:active_tab, 'edit')
|
||||
- active_classes = 'gl-tab-nav-item-active gl-tab-nav-item-active-indigo active'
|
||||
- tabs = integration_tabs(integration: integration)
|
||||
|
||||
- if tabs.length <= 1
|
||||
= yield
|
||||
- else
|
||||
- if integration.instance_level?
|
||||
.tabs.gl-tabs
|
||||
%div
|
||||
%ul.nav.gl-tabs-nav{ role: 'tablist' }
|
||||
- tabs.each do |tab|
|
||||
%li.nav-item{ role: 'presentation' }
|
||||
%a.nav-link.gl-tab-nav-item{ role: 'tab', class: (active_classes if tab[:key] == active_tab), href: tab[:href] }
|
||||
= tab[:text]
|
||||
= gl_tabs_nav({ class: 'gl-mb-5' }) do
|
||||
= gl_tab_link_to _('Settings'), scoped_edit_integration_path(integration)
|
||||
= gl_tab_link_to s_('Integrations|Projects using custom settings'), scoped_overrides_integration_path(integration)
|
||||
|
||||
.tab-content.gl-tab-content
|
||||
.tab-pane.gl-pt-3.active{ role: 'tabpanel' }
|
||||
= yield
|
||||
= yield
|
||||
|
||||
- else
|
||||
= yield
|
||||
|
|
|
@ -0,0 +1,8 @@
|
|||
---
|
||||
name: rate_limited_service_issues_create
|
||||
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68526
|
||||
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/342677
|
||||
milestone: '14.4'
|
||||
type: development
|
||||
group: group::project management
|
||||
default_enabled: false
|
|
@ -245,7 +245,11 @@ end
|
|||
|
||||
## Prefer `aggregate_failures` when there are back-to-back expectations
|
||||
|
||||
In cases where there must be multiple (back-to-back) expectations within a test case, it is preferable to use `aggregate_failures`.
|
||||
See [Prefer aggregate failures when there are multiple expectations](#prefer-aggregate_failures-when-there-are-multiple-expectations)
|
||||
|
||||
## Prefer `aggregate_failures` when there are multiple expectations
|
||||
|
||||
In cases where there must be multiple expectations within a test case, it is preferable to use `aggregate_failures`.
|
||||
|
||||
This allows you to group a set of expectations and see all the failures altogether, rather than having the test being aborted on the first failure.
|
||||
|
||||
|
@ -270,6 +274,32 @@ Page::Search::Results.perform do |search|
|
|||
end
|
||||
```
|
||||
|
||||
Attach the `:aggregate_failures` metadata to the example if multiple expectations are separated by statements.
|
||||
|
||||
```ruby
|
||||
#=> Good
|
||||
it 'searches', :aggregate_failures do
|
||||
Page::Search::Results.perform do |search|
|
||||
expect(search).to have_file_in_project(template[:file_name], project.name)
|
||||
|
||||
search.switch_to_code
|
||||
|
||||
expect(search).to have_file_with_content(template[:file_name], content[0..33])
|
||||
end
|
||||
end
|
||||
|
||||
#=> Bad
|
||||
it 'searches' do
|
||||
Page::Search::Results.perform do |search|
|
||||
expect(search).to have_file_in_project(template[:file_name], project.name)
|
||||
|
||||
search.switch_to_code
|
||||
|
||||
expect(search).to have_file_with_content(template[:file_name], content[0..33])
|
||||
end
|
||||
end
|
||||
```
|
||||
|
||||
## Prefer to split tests across multiple files
|
||||
|
||||
Our framework includes a couple of parallelization mechanisms that work by executing spec files in parallel.
|
||||
|
|
|
@ -52,7 +52,8 @@ in your SAML IdP:
|
|||
|
||||
sudo -u git -H editor config/gitlab.yml
|
||||
```
|
||||
|
||||
|
||||
1. See [Initial OmniAuth Configuration](omniauth.md#initial-omniauth-configuration) for initial settings.
|
||||
1. To allow your users to use SAML to sign up without having to manually create
|
||||
an account first, add the following values to your configuration:
|
||||
|
||||
|
|
|
@ -75,10 +75,11 @@ You can also [view our language roadmap](https://about.gitlab.com/direction/secu
|
|||
| .NET Core | [Security Code Scan](https://security-code-scan.github.io) | 11.0 |
|
||||
| .NET Framework | [Security Code Scan](https://security-code-scan.github.io) | 13.0 |
|
||||
| Apex (Salesforce) | [PMD](https://pmd.github.io/pmd/index.html) | 12.1 |
|
||||
| C | [Semgrep](https://semgrep.dev) | 14.2 |
|
||||
| C | [Semgrep](https://semgrep.dev) | 14.2 |
|
||||
| C/C++ | [Flawfinder](https://github.com/david-a-wheeler/flawfinder) | 10.7 |
|
||||
| Elixir (Phoenix) | [Sobelow](https://github.com/nccgroup/sobelow) | 11.1 |
|
||||
| Go | [Gosec](https://github.com/securego/gosec) | 10.7 |
|
||||
| Go | [Semgrep](https://semgrep.dev) | 14.4 |
|
||||
| Groovy ([Ant](https://ant.apache.org/), [Gradle](https://gradle.org/), [Maven](https://maven.apache.org/), and [SBT](https://www.scala-sbt.org/)) | [SpotBugs](https://spotbugs.github.io/) with the [find-sec-bugs](https://find-sec-bugs.github.io/) plugin | 11.3 (Gradle) & 11.9 (Ant, Maven, SBT) |
|
||||
| Helm Charts | [Kubesec](https://github.com/controlplaneio/kubesec) | 13.1 |
|
||||
| Java ([Ant](https://ant.apache.org/), [Gradle](https://gradle.org/), [Maven](https://maven.apache.org/), and [SBT](https://www.scala-sbt.org/)) | [SpotBugs](https://spotbugs.github.io/) with the [find-sec-bugs](https://find-sec-bugs.github.io/) plugin | 10.6 (Maven), 10.8 (Gradle) & 11.9 (Ant, SBT) |
|
||||
|
|
Binary file not shown.
Before Width: | Height: | Size: 18 KiB |
Binary file not shown.
Before Width: | Height: | Size: 17 KiB |
|
@ -73,51 +73,41 @@ subgroups, the behavior is the same as when performing these actions at the
|
|||
|
||||
## Creating a subgroup
|
||||
|
||||
To create a subgroup you must either be an Owner or a Maintainer of the
|
||||
group, depending on the group's setting.
|
||||
|
||||
By default, groups created in:
|
||||
|
||||
- GitLab 12.2 or later allow both Owners and Maintainers to create subgroups.
|
||||
- GitLab 12.1 or earlier only allow Owners to create subgroups.
|
||||
|
||||
The setting can be changed for any group by:
|
||||
|
||||
- A group owner:
|
||||
1. Select the group.
|
||||
1. On the left sidebar, select **Settings > General**.
|
||||
1. Expand the **Permissions, LFS, 2FA** section.
|
||||
- An administrator:
|
||||
1. On the top bar, select **Menu > Admin**.
|
||||
1. On the left sidebar, select **Overview > Groups**.
|
||||
1. Select the group, and select **Edit**.
|
||||
|
||||
For:
|
||||
|
||||
- More information, check the [permissions table](../../permissions.md#group-members-permissions).
|
||||
- A list of words that are not allowed to be used as group names, see the
|
||||
[reserved names](../../reserved_names.md).
|
||||
|
||||
Users can always create subgroups if they are explicitly added as an Owner (or
|
||||
Users can create subgroups if they are explicitly added as an Owner (or
|
||||
Maintainer, if that setting is enabled) to an immediate parent group, even if group
|
||||
creation is disabled by an administrator in their settings.
|
||||
|
||||
To create a subgroup:
|
||||
|
||||
1. In the group's dashboard click the **New subgroup** button.
|
||||
1. On the top bar, select **Menu > Groups** and find the parent group.
|
||||
1. In the top right, select **New subgroup**.
|
||||
1. Select **Create group**.
|
||||
1. Fill in the fields. View a list of [reserved names](../../reserved_names.md)
|
||||
that cannot be used as group names.
|
||||
1. Select **Create group**.
|
||||
|
||||
![Subgroups page](img/create_subgroup_button_v13_6.png)
|
||||
### Change who can create subgroups
|
||||
|
||||
1. Create a new group like you would normally do. Notice that the immediate parent group
|
||||
namespace is fixed under **Group path**. The visibility level can differ from
|
||||
the immediate parent group.
|
||||
To create a subgroup you must either be an Owner or a Maintainer of the
|
||||
group, depending on the group's setting.
|
||||
|
||||
![Subgroups page](img/create_new_group.png)
|
||||
By default:
|
||||
|
||||
1. Click the **Create group** button to be redirected to the new group's
|
||||
dashboard page.
|
||||
- In GitLab 12.2 or later, both Owners and Maintainers can create subgroups.
|
||||
- In GitLab 12.1 or earlier, only Owners can create subgroups.
|
||||
|
||||
Follow the same process to create any subsequent groups.
|
||||
You can change this setting:
|
||||
|
||||
- As group owner:
|
||||
1. Select the group.
|
||||
1. On the left sidebar, select **Settings > General**.
|
||||
1. Expand **Permissions, LFS, 2FA**.
|
||||
- As an administrator:
|
||||
1. On the top bar, select **Menu > Admin**.
|
||||
1. On the left sidebar, select **Overview > Groups**.
|
||||
1. Select the group, and select **Edit**.
|
||||
|
||||
For more information, view the [permissions table](../../permissions.md#group-members-permissions).
|
||||
|
||||
## Membership
|
||||
|
||||
|
|
|
@ -124,6 +124,11 @@ module API
|
|||
handle_api_exception(exception)
|
||||
end
|
||||
|
||||
rescue_from RateLimitedService::RateLimitedError do |exception|
|
||||
exception.log_request(context.request, context.current_user)
|
||||
rack_response({ 'message' => { 'error' => exception.message } }.to_json, 429, exception.headers)
|
||||
end
|
||||
|
||||
format :json
|
||||
formatter :json, Gitlab::Json::GrapeFormatter
|
||||
content_type :json, 'application/json'
|
||||
|
@ -132,6 +137,7 @@ module API
|
|||
helpers ::API::Helpers
|
||||
helpers ::API::Helpers::CommonHelpers
|
||||
helpers ::API::Helpers::PerformanceBarHelpers
|
||||
helpers ::API::Helpers::RateLimiter
|
||||
|
||||
namespace do
|
||||
after do
|
||||
|
|
|
@ -2,8 +2,6 @@
|
|||
|
||||
module API
|
||||
class GroupExport < ::API::Base
|
||||
helpers Helpers::RateLimiter
|
||||
|
||||
before do
|
||||
not_found! unless Feature.enabled?(:group_import_export, user_group, default_enabled: true)
|
||||
|
||||
|
|
|
@ -4,7 +4,6 @@ module API
|
|||
class Issues < ::API::Base
|
||||
include PaginationParams
|
||||
helpers Helpers::IssuesHelpers
|
||||
helpers Helpers::RateLimiter
|
||||
|
||||
before { authenticate_non_get! }
|
||||
|
||||
|
@ -263,7 +262,7 @@ module API
|
|||
post ':id/issues' do
|
||||
Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/21140')
|
||||
|
||||
check_rate_limit! :issues_create, [current_user]
|
||||
check_rate_limit! :issues_create, [current_user] if Feature.disabled?("rate_limited_service_issues_create", user_project, default_enabled: :yaml)
|
||||
|
||||
authorize! :create_issue, user_project
|
||||
|
||||
|
|
|
@ -4,7 +4,6 @@ module API
|
|||
class Notes < ::API::Base
|
||||
include PaginationParams
|
||||
helpers ::API::Helpers::NotesHelpers
|
||||
helpers Helpers::RateLimiter
|
||||
|
||||
before { authenticate! }
|
||||
|
||||
|
|
|
@ -2,8 +2,6 @@
|
|||
|
||||
module API
|
||||
class ProjectExport < ::API::Base
|
||||
helpers Helpers::RateLimiter
|
||||
|
||||
feature_category :importers
|
||||
|
||||
before do
|
||||
|
|
|
@ -6,7 +6,6 @@ module API
|
|||
|
||||
helpers Helpers::ProjectsHelpers
|
||||
helpers Helpers::FileUploadHelpers
|
||||
helpers Helpers::RateLimiter
|
||||
|
||||
feature_category :importers
|
||||
|
||||
|
|
|
@ -11,6 +11,23 @@ module Gitlab
|
|||
# redirect_to(edit_project_path(@project), status: :too_many_requests)
|
||||
# end
|
||||
class ApplicationRateLimiter
|
||||
def initialize(key, **options)
|
||||
@key = key
|
||||
@options = options
|
||||
end
|
||||
|
||||
def throttled?
|
||||
self.class.throttled?(key, **options)
|
||||
end
|
||||
|
||||
def threshold_value
|
||||
options[:threshold] || self.class.threshold(key)
|
||||
end
|
||||
|
||||
def interval_value
|
||||
self.class.interval(key)
|
||||
end
|
||||
|
||||
class << self
|
||||
# Application rate limits
|
||||
#
|
||||
|
@ -154,5 +171,9 @@ module Gitlab
|
|||
scoped_user.username.downcase.in?(options[:users_allowlist])
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
attr_reader :key, :options
|
||||
end
|
||||
end
|
||||
|
|
|
@ -30,7 +30,8 @@ module Quality
|
|||
labels: labels.join(',')
|
||||
}
|
||||
params[:closed_at] = params[:created_at] + rand(35).days if params[:state] == 'closed'
|
||||
issue = ::Issues::CreateService.new(project: project, current_user: team.sample, params: params, spam_params: nil).execute
|
||||
|
||||
issue = ::Issues::CreateService.new(project: project, current_user: team.sample, params: params, spam_params: nil).execute_without_rate_limiting
|
||||
|
||||
if issue.persisted?
|
||||
created_issues_count += 1
|
||||
|
|
|
@ -66,7 +66,9 @@ class StaticAnalysis
|
|||
].compact.freeze
|
||||
|
||||
def run_tasks!(options = {})
|
||||
node_assignment = tasks_to_run((ENV['CI_NODE_TOTAL'] || 1).to_i)[(ENV['CI_NODE_INDEX'] || 1).to_i - 1]
|
||||
total_nodes = (ENV['CI_NODE_TOTAL'] || 1).to_i
|
||||
current_node_number = (ENV['CI_NODE_INDEX'] || 1).to_i
|
||||
node_assignment = tasks_to_run(total_nodes)[current_node_number - 1]
|
||||
|
||||
if options[:dry_run]
|
||||
puts "Dry-run mode!"
|
||||
|
|
|
@ -1411,39 +1411,42 @@ RSpec.describe Projects::IssuesController do
|
|||
stub_application_setting(issues_create_limit: 5)
|
||||
end
|
||||
|
||||
it 'prevents from creating more issues', :request_store do
|
||||
5.times { post_new_issue }
|
||||
context 'when issue creation limits imposed' do
|
||||
it 'prevents from creating more issues', :request_store do
|
||||
5.times { post_new_issue }
|
||||
|
||||
expect { post_new_issue }
|
||||
.to change { Gitlab::GitalyClient.get_request_count }.by(1) # creates 1 projects and 0 issues
|
||||
expect { post_new_issue }
|
||||
.to change { Gitlab::GitalyClient.get_request_count }.by(1) # creates 1 projects and 0 issues
|
||||
|
||||
post_new_issue
|
||||
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
|
||||
post_new_issue
|
||||
|
||||
it 'logs the event on auth.log' do
|
||||
attributes = {
|
||||
message: 'Application_Rate_Limiter_Request',
|
||||
env: :issues_create_request_limit,
|
||||
remote_ip: '0.0.0.0',
|
||||
request_method: 'POST',
|
||||
path: "/#{project.full_path}/-/issues",
|
||||
user_id: user.id,
|
||||
username: user.username
|
||||
}
|
||||
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
|
||||
|
||||
expect(Gitlab::AuthLogger).to receive(:error).with(attributes).once
|
||||
|
||||
project.add_developer(user)
|
||||
sign_in(user)
|
||||
|
||||
6.times do
|
||||
post :create, params: {
|
||||
namespace_id: project.namespace.to_param,
|
||||
project_id: project,
|
||||
issue: { title: 'Title', description: 'Description' }
|
||||
it 'logs the event on auth.log' do
|
||||
attributes = {
|
||||
message: 'Application_Rate_Limiter_Request',
|
||||
env: :issues_create_request_limit,
|
||||
remote_ip: '0.0.0.0',
|
||||
request_method: 'POST',
|
||||
path: "/#{project.full_path}/-/issues",
|
||||
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 do
|
||||
post :create, params: {
|
||||
namespace_id: project.namespace.to_param,
|
||||
project_id: project,
|
||||
issue: { title: 'Title', description: 'Description' }
|
||||
}
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -53,7 +53,11 @@ RSpec.describe Mutations::Issues::Create do
|
|||
stub_spam_services
|
||||
end
|
||||
|
||||
subject { mutation.resolve(**mutation_params) }
|
||||
def resolve
|
||||
mutation.resolve(**mutation_params)
|
||||
end
|
||||
|
||||
subject { resolve }
|
||||
|
||||
context 'when the user does not have permission to create an issue' do
|
||||
it 'raises an error' do
|
||||
|
@ -61,6 +65,15 @@ RSpec.describe Mutations::Issues::Create do
|
|||
end
|
||||
end
|
||||
|
||||
context 'when the user has exceeded the rate limit' do
|
||||
it 'raises an error' do
|
||||
allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true)
|
||||
project.add_developer(user)
|
||||
|
||||
expect { resolve }.to raise_error(RateLimitedService::RateLimitedError, _('This endpoint has been requested too many times. Try again later.'))
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the user can create an issue' do
|
||||
context 'when creating an issue a developer' do
|
||||
before do
|
||||
|
|
|
@ -136,6 +136,36 @@ RSpec.describe Gitlab::Email::Handler::CreateIssueHandler do
|
|||
expect { handler.execute }.to raise_error(Gitlab::Email::ProjectNotFound)
|
||||
end
|
||||
end
|
||||
|
||||
context 'rate limiting' do
|
||||
let(:rate_limited_service_feature_enabled) { nil }
|
||||
|
||||
before do
|
||||
stub_feature_flags(rate_limited_service_issues_create: rate_limited_service_feature_enabled)
|
||||
end
|
||||
|
||||
context 'when :rate_limited_service Feature is disabled' do
|
||||
let(:rate_limited_service_feature_enabled) { false }
|
||||
|
||||
it 'does not attempt to throttle' do
|
||||
expect(::Gitlab::ApplicationRateLimiter).not_to receive(:throttled?)
|
||||
|
||||
setup_attachment
|
||||
receiver.execute
|
||||
end
|
||||
end
|
||||
|
||||
context 'when :rate_limited_service Feature is enabled' do
|
||||
let(:rate_limited_service_feature_enabled) { true }
|
||||
|
||||
it 'raises a RateLimitedService::RateLimitedError' do
|
||||
allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true)
|
||||
|
||||
setup_attachment
|
||||
expect { receiver.execute }.to raise_error(RateLimitedService::RateLimitedError, _('This endpoint has been requested too many times. Try again later.'))
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def email_fixture(path)
|
||||
|
|
|
@ -243,6 +243,15 @@ RSpec.describe Gitlab::Email::Handler::ServiceDeskHandler do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when rate limiting is in effect' do
|
||||
it 'allows unlimited new issue creation' do
|
||||
stub_application_setting(issues_create_limit: 1)
|
||||
setup_attachment
|
||||
|
||||
expect { 2.times { receiver.execute } }.to change { Issue.count }.by(2)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#can_handle?' do
|
||||
|
|
|
@ -399,16 +399,15 @@ RSpec.describe API::Issues do
|
|||
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 issues' do
|
||||
allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true)
|
||||
|
||||
post api("/projects/#{project.id}/issues", user),
|
||||
params: { title: 'new issue', labels: 'label, label2', weight: 3, assignee_ids: [user2.id] }
|
||||
|
||||
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.')
|
||||
|
||||
expect(response).to have_gitlab_http_status(:too_many_requests)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
196
spec/services/concerns/rate_limited_service_spec.rb
Normal file
196
spec/services/concerns/rate_limited_service_spec.rb
Normal file
|
@ -0,0 +1,196 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require 'spec_helper'
|
||||
|
||||
RSpec.describe RateLimitedService do
|
||||
let(:key) { :issues_create }
|
||||
let(:scope) { [:project, :current_user] }
|
||||
let(:opts) { { scope: scope, users_allowlist: -> { [User.support_bot.username] } } }
|
||||
let(:rate_limiter_klass) { ::Gitlab::ApplicationRateLimiter }
|
||||
let(:rate_limiter_instance) { rate_limiter_klass.new(key, **opts) }
|
||||
|
||||
describe 'RateLimitedError' do
|
||||
subject { described_class::RateLimitedError.new(key: key, rate_limiter: rate_limiter_instance) }
|
||||
|
||||
describe '#headers' do
|
||||
it 'returns a Hash of HTTP headers' do
|
||||
# TODO: This will be fleshed out in https://gitlab.com/gitlab-org/gitlab/-/issues/342370
|
||||
expected_headers = {}
|
||||
|
||||
expect(subject.headers).to eq(expected_headers)
|
||||
end
|
||||
end
|
||||
|
||||
describe '#log_request' do
|
||||
it 'logs the request' do
|
||||
request = instance_double(Grape::Request)
|
||||
user = instance_double(User)
|
||||
|
||||
expect(rate_limiter_klass).to receive(:log_request).with(request, "#{key}_request_limit".to_sym, user)
|
||||
|
||||
subject.log_request(request, user)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'RateLimiterScopedAndKeyed' do
|
||||
subject { described_class::RateLimiterScopedAndKeyed.new(key: key, opts: opts, rate_limiter_klass: rate_limiter_klass) }
|
||||
|
||||
describe '#rate_limit!' do
|
||||
let(:project_with_feature_enabled) { create(:project) }
|
||||
let(:project_without_feature_enabled) { create(:project) }
|
||||
|
||||
let(:project) { nil }
|
||||
|
||||
let(:current_user) { create(:user) }
|
||||
let(:service) { instance_double(Issues::CreateService, project: project, current_user: current_user) }
|
||||
let(:evaluated_scope) { [project, current_user] }
|
||||
let(:evaluated_opts) { { scope: evaluated_scope, users_allowlist: %w[support-bot] } }
|
||||
let(:rate_limited_service_issues_create_feature_enabled) { nil }
|
||||
|
||||
before do
|
||||
allow(rate_limiter_klass).to receive(:new).with(key, **evaluated_opts).and_return(rate_limiter_instance)
|
||||
stub_feature_flags(rate_limited_service_issues_create: rate_limited_service_issues_create_feature_enabled)
|
||||
end
|
||||
|
||||
shared_examples 'a service that does not attempt to throttle' do
|
||||
it 'does not attempt to throttle' do
|
||||
expect(rate_limiter_instance).not_to receive(:throttled?)
|
||||
|
||||
expect(subject.rate_limit!(service)).to be_nil
|
||||
end
|
||||
end
|
||||
|
||||
shared_examples 'a service that does attempt to throttle' do
|
||||
before do
|
||||
allow(rate_limiter_instance).to receive(:throttled?).and_return(throttled)
|
||||
end
|
||||
|
||||
context 'when rate limiting is not in effect' do
|
||||
let(:throttled) { false }
|
||||
|
||||
it 'does not raise an exception' do
|
||||
expect(subject.rate_limit!(service)).to be_nil
|
||||
end
|
||||
end
|
||||
|
||||
context 'when rate limiting is in effect' do
|
||||
let(:throttled) { true }
|
||||
|
||||
it 'raises a RateLimitedError exception' do
|
||||
expect { subject.rate_limit!(service) }.to raise_error(described_class::RateLimitedError, 'This endpoint has been requested too many times. Try again later.')
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when :rate_limited_service_issues_create feature is globally disabled' do
|
||||
let(:rate_limited_service_issues_create_feature_enabled) { false }
|
||||
|
||||
it_behaves_like 'a service that does not attempt to throttle'
|
||||
end
|
||||
|
||||
context 'when :rate_limited_service_issues_create feature is globally enabled' do
|
||||
let(:throttled) { nil }
|
||||
let(:rate_limited_service_issues_create_feature_enabled) { true }
|
||||
let(:project) { project_without_feature_enabled }
|
||||
|
||||
it_behaves_like 'a service that does attempt to throttle'
|
||||
end
|
||||
|
||||
context 'when :rate_limited_service_issues_create feature is enabled for project_with_feature_enabled' do
|
||||
let(:throttled) { nil }
|
||||
let(:rate_limited_service_issues_create_feature_enabled) { project_with_feature_enabled }
|
||||
|
||||
context 'for project_without_feature_enabled' do
|
||||
let(:project) { project_without_feature_enabled }
|
||||
|
||||
it_behaves_like 'a service that does not attempt to throttle'
|
||||
end
|
||||
|
||||
context 'for project_with_feature_enabled' do
|
||||
let(:project) { project_with_feature_enabled }
|
||||
|
||||
it_behaves_like 'a service that does attempt to throttle'
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#execute_without_rate_limiting' do
|
||||
let(:rate_limiter_scoped_and_keyed) { instance_double(RateLimitedService::RateLimiterScopedAndKeyed) }
|
||||
let(:subject) do
|
||||
local_key = key
|
||||
local_opts = opts
|
||||
|
||||
Class.new do
|
||||
prepend RateLimitedService
|
||||
|
||||
rate_limit key: local_key, opts: local_opts
|
||||
|
||||
def execute(*args, **kwargs)
|
||||
'main logic here'
|
||||
end
|
||||
end.new
|
||||
end
|
||||
|
||||
before do
|
||||
allow(RateLimitedService::RateLimiterScopedAndKeyed).to receive(:new).with(key: key, opts: opts, rate_limiter_klass: rate_limiter_klass).and_return(rate_limiter_scoped_and_keyed)
|
||||
end
|
||||
|
||||
context 'bypasses rate limiting' do
|
||||
it 'calls super' do
|
||||
expect(rate_limiter_scoped_and_keyed).not_to receive(:rate_limit!).with(subject)
|
||||
|
||||
expect(subject.execute_without_rate_limiting).to eq('main logic here')
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#execute' do
|
||||
context 'when rate_limit has not been called' do
|
||||
let(:subject) { Class.new { prepend RateLimitedService }.new }
|
||||
|
||||
it 'raises an RateLimitedNotSetupError exception' do
|
||||
expect { subject.execute }.to raise_error(described_class::RateLimitedNotSetupError)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when rate_limit has been called' do
|
||||
let(:rate_limiter_scoped_and_keyed) { instance_double(RateLimitedService::RateLimiterScopedAndKeyed) }
|
||||
let(:subject) do
|
||||
local_key = key
|
||||
local_opts = opts
|
||||
|
||||
Class.new do
|
||||
prepend RateLimitedService
|
||||
|
||||
rate_limit key: local_key, opts: local_opts
|
||||
|
||||
def execute(*args, **kwargs)
|
||||
'main logic here'
|
||||
end
|
||||
end.new
|
||||
end
|
||||
|
||||
before do
|
||||
allow(RateLimitedService::RateLimiterScopedAndKeyed).to receive(:new).with(key: key, opts: opts, rate_limiter_klass: rate_limiter_klass).and_return(rate_limiter_scoped_and_keyed)
|
||||
end
|
||||
|
||||
context 'and applies rate limiting' do
|
||||
it 'raises an RateLimitedService::RateLimitedError exception' do
|
||||
expect(rate_limiter_scoped_and_keyed).to receive(:rate_limit!).with(subject).and_raise(RateLimitedService::RateLimitedError.new(key: key, rate_limiter: rate_limiter_instance))
|
||||
|
||||
expect { subject.execute }.to raise_error(RateLimitedService::RateLimitedError)
|
||||
end
|
||||
end
|
||||
|
||||
context 'but does not apply rate limiting' do
|
||||
it 'calls super' do
|
||||
expect(rate_limiter_scoped_and_keyed).to receive(:rate_limit!).with(subject).and_return(nil)
|
||||
|
||||
expect(subject.execute).to eq('main logic here')
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
|
@ -10,6 +10,25 @@ RSpec.describe Issues::CreateService do
|
|||
|
||||
let(:spam_params) { double }
|
||||
|
||||
describe '.rate_limiter_scoped_and_keyed' do
|
||||
it 'is set via the rate_limit call' do
|
||||
expect(described_class.rate_limiter_scoped_and_keyed).to be_a(RateLimitedService::RateLimiterScopedAndKeyed)
|
||||
|
||||
expect(described_class.rate_limiter_scoped_and_keyed.key).to eq(:issues_create)
|
||||
expect(described_class.rate_limiter_scoped_and_keyed.opts[:scope]).to eq(%i[project current_user])
|
||||
expect(described_class.rate_limiter_scoped_and_keyed.opts[:users_allowlist].call).to eq(%w[support-bot])
|
||||
expect(described_class.rate_limiter_scoped_and_keyed.rate_limiter_klass).to eq(Gitlab::ApplicationRateLimiter)
|
||||
end
|
||||
end
|
||||
|
||||
describe '#rate_limiter_bypassed' do
|
||||
let(:subject) { described_class.new(project: project, spam_params: {}) }
|
||||
|
||||
it 'is nil by default' do
|
||||
expect(subject.rate_limiter_bypassed).to be_nil
|
||||
end
|
||||
end
|
||||
|
||||
describe '#execute' do
|
||||
let_it_be(:assignee) { create(:user) }
|
||||
let_it_be(:milestone) { create(:milestone, project: project) }
|
||||
|
|
|
@ -9,11 +9,9 @@
|
|||
- "./ee/spec/finders/ee/namespaces/projects_finder_spec.rb"
|
||||
- "./ee/spec/graphql/ee/resolvers/namespace_projects_resolver_spec.rb"
|
||||
- "./ee/spec/lib/ee/gitlab/background_migration/migrate_approver_to_approval_rules_spec.rb"
|
||||
- "./ee/spec/lib/ee/gitlab/background_migration/migrate_security_scans_spec.rb"
|
||||
- "./ee/spec/lib/ee/gitlab/background_migration/populate_latest_pipeline_ids_spec.rb"
|
||||
- "./ee/spec/lib/ee/gitlab/background_migration/populate_resolved_on_default_branch_column_spec.rb"
|
||||
- "./ee/spec/lib/ee/gitlab/background_migration/populate_uuids_for_security_findings_spec.rb"
|
||||
- "./ee/spec/lib/ee/gitlab/background_migration/populate_vulnerability_feedback_pipeline_id_spec.rb"
|
||||
- "./ee/spec/lib/ee/gitlab/usage_data_spec.rb"
|
||||
- "./ee/spec/migrations/schedule_populate_resolved_on_default_branch_column_spec.rb"
|
||||
- "./ee/spec/models/ci/build_spec.rb"
|
||||
|
|
|
@ -80,6 +80,21 @@ RSpec.describe EmailReceiverWorker, :mailer do
|
|||
expect(email).to be_nil
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the error is RateLimitedService::RateLimitedError' do
|
||||
let(:error) { RateLimitedService::RateLimitedError.new(key: :issues_create, rate_limiter: Gitlab::ApplicationRateLimiter) }
|
||||
|
||||
it 'does not report the error to the sender' do
|
||||
expect(Gitlab::ErrorTracking).to receive(:track_exception).with(error).and_call_original
|
||||
|
||||
perform_enqueued_jobs do
|
||||
described_class.new.perform(raw_message)
|
||||
end
|
||||
|
||||
email = ActionMailer::Base.deliveries.last
|
||||
expect(email).to be_nil
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
|
Loading…
Reference in a new issue