Commit ddda5851 authored by Igor Drozdov's avatar Igor Drozdov

Rate limit Gitlab Shell operations

When a user performs an unusual number of a particular operation
to a particular project, they receive 429 response code

Rate limit is 60 operations per minute

Changelog: added
parent 8b5b39ce
---
name: rate_limit_gitlab_shell
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78373
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/350465
milestone: '14.7'
type: development
group: group::source code
default_enabled: false
......@@ -43,6 +43,10 @@ module API
# This is a separate method so that EE can alter its behaviour more
# easily.
if Feature.enabled?(:rate_limit_gitlab_shell, default_enabled: :yaml)
check_rate_limit!(:gitlab_shell_operation, scope: [params[:action], params[:project], actor.key_or_user])
end
# Stores some Git-specific env thread-safely
env = parse_env
Gitlab::Git::HookEnv.set(gl_repository, env) if container
......
......@@ -56,7 +56,8 @@ module Gitlab
profile_update_username: { threshold: 10, interval: 1.minute },
update_environment_canary_ingress: { threshold: 1, interval: 1.minute },
auto_rollback_deployment: { threshold: 1, interval: 3.minutes },
user_email_lookup: { threshold: -> { application_settings.user_email_lookup_limit }, interval: 1.minute }
user_email_lookup: { threshold: -> { application_settings.user_email_lookup_limit }, interval: 1.minute },
gitlab_shell_operation: { threshold: 600, interval: 1.minute }
}.freeze
end
......@@ -64,7 +65,7 @@ module Gitlab
# be throttled.
#
# @param key [Symbol] Key attribute registered in `.rate_limits`
# @param scope [Array<ActiveRecord>] Array of ActiveRecord models to scope throttling to a specific request (e.g. per user per project)
# @param scope [Array<ActiveRecord>] Array of ActiveRecord models, Strings or Symbols to scope throttling to a specific request (e.g. per user per project)
# @param threshold [Integer] Optional threshold value to override default one registered in `.rate_limits`
# @param users_allowlist [Array<String>] Optional list of usernames to exclude from the limit. This param will only be functional if Scope includes a current user.
# @param peek [Boolean] Optional. When true the key will not be incremented but the current throttled state will be returned.
......
......@@ -372,7 +372,38 @@ RSpec.describe API::Internal::Base do
end
end
describe "POST /internal/allowed", :clean_gitlab_redis_shared_state do
describe "POST /internal/allowed", :clean_gitlab_redis_shared_state, :clean_gitlab_redis_rate_limiting do
shared_examples 'rate limited request' do
let(:action) { 'git-upload-pack' }
let(:actor) { key }
it 'is throttled by rate limiter' do
allow(::Gitlab::ApplicationRateLimiter).to receive(:threshold).and_return(1)
expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:gitlab_shell_operation, scope: [action, project.full_path, actor]).twice.and_call_original
request
expect(response).to have_gitlab_http_status(:ok)
request
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
context 'when rate_limit_gitlab_shell feature flag is disabled' do
before do
stub_feature_flags(rate_limit_gitlab_shell: false)
end
it 'is not throttled by rate limiter' do
expect(::Gitlab::ApplicationRateLimiter).not_to receive(:throttled?)
subject
end
end
end
context "access granted" do
let(:env) { {} }
......@@ -530,6 +561,32 @@ RSpec.describe API::Internal::Base do
expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-mep-mep' => 'true')
expect(user.reload.last_activity_on).to eql(Date.today)
end
it_behaves_like 'rate limited request' do
def request
pull(key, project)
end
end
context 'when user_id is passed' do
it_behaves_like 'rate limited request' do
let(:actor) { user }
def request
post(
api("/internal/allowed"),
params: {
user_id: user.id,
project: full_path_for(project),
gl_repository: gl_repository_for(project),
action: 'git-upload-pack',
secret_token: secret_token,
protocol: 'ssh'
}
)
end
end
end
end
context "with a feature flag enabled for a project" do
......@@ -576,6 +633,14 @@ RSpec.describe API::Internal::Base do
expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage))
expect(user.reload.last_activity_on).to be_nil
end
it_behaves_like 'rate limited request' do
let(:action) { 'git-receive-pack' }
def request
push(key, project)
end
end
end
context 'when receive_max_input_size has been updated' do
......@@ -838,6 +903,14 @@ RSpec.describe API::Internal::Base do
expect(json_response["gitaly"]["address"]).to eq(Gitlab::GitalyClient.address(project.repository_storage))
expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage))
end
it_behaves_like 'rate limited request' do
let(:action) { 'git-upload-archive' }
def request
archive(key, project)
end
end
end
context "not added to project" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment