summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2020-02-22 12:08:58 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2020-02-22 12:08:58 +0000
commited45528885b7b44c61f18175fe7cdbda12360669 (patch)
tree3d27c00a8a83d569cf238eaa05b7eb24b7a28a8d
parentab85af0f318ccbcfdd508e7a2f85788f26831785 (diff)
downloadgitlab-ce-ed45528885b7b44c61f18175fe7cdbda12360669.tar.gz
Add latest changes from gitlab-org/gitlab@master
-rw-r--r--app/controllers/projects/repositories_controller.rb8
-rw-r--r--changelogs/unreleased/sh-rate-limit-archive-endpoint.yml5
-rw-r--r--lib/api/repositories.rb6
-rw-r--r--lib/gitlab/application_rate_limiter.rb1
-rw-r--r--lib/gitlab/rate_limit_helpers.rb35
-rw-r--r--spec/controllers/projects/repositories_controller_spec.rb14
-rw-r--r--spec/lib/gitlab/rate_limit_helpers_spec.rb49
-rw-r--r--spec/requests/api/repositories_spec.rb12
8 files changed, 130 insertions, 0 deletions
diff --git a/app/controllers/projects/repositories_controller.rb b/app/controllers/projects/repositories_controller.rb
index d0fb814948f..1cb9e1d2c9b 100644
--- a/app/controllers/projects/repositories_controller.rb
+++ b/app/controllers/projects/repositories_controller.rb
@@ -3,11 +3,13 @@
class Projects::RepositoriesController < Projects::ApplicationController
include ExtractsPath
include StaticObjectExternalStorage
+ include Gitlab::RateLimitHelpers
prepend_before_action(only: [:archive]) { authenticate_sessionless_user!(:archive) }
# Authorize
before_action :require_non_empty_project, except: :create
+ before_action :archive_rate_limit!, only: :archive
before_action :assign_archive_vars, only: :archive
before_action :assign_append_sha, only: :archive
before_action :authorize_download_code!
@@ -34,6 +36,12 @@ class Projects::RepositoriesController < Projects::ApplicationController
private
+ def archive_rate_limit!
+ if archive_rate_limit_reached?(current_user, @project)
+ render plain: ::Gitlab::RateLimitHelpers::ARCHIVE_RATE_LIMIT_REACHED_MESSAGE, status: :too_many_requests
+ end
+ end
+
def repo_params
@repo_params ||= { ref: @ref, path: params[:path], format: params[:format], append_sha: @append_sha }
end
diff --git a/changelogs/unreleased/sh-rate-limit-archive-endpoint.yml b/changelogs/unreleased/sh-rate-limit-archive-endpoint.yml
new file mode 100644
index 00000000000..55c20d1a6e3
--- /dev/null
+++ b/changelogs/unreleased/sh-rate-limit-archive-endpoint.yml
@@ -0,0 +1,5 @@
+---
+title: Rate limit archive endpoint by user
+merge_request: 25750
+author:
+type: changed
diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb
index 00473db1ff1..62f5b67af1e 100644
--- a/lib/api/repositories.rb
+++ b/lib/api/repositories.rb
@@ -13,6 +13,8 @@ module API
end
resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
helpers do
+ include ::Gitlab::RateLimitHelpers
+
def handle_project_member_errors(errors)
if errors[:project_access].any?
error!(errors[:project_access], 422)
@@ -89,6 +91,10 @@ module API
optional :format, type: String, desc: 'The archive format'
end
get ':id/repository/archive', requirements: { format: Gitlab::PathRegex.archive_formats_regex } do
+ if archive_rate_limit_reached?(current_user, user_project)
+ render_api_error!({ error: ::Gitlab::RateLimitHelpers::ARCHIVE_RATE_LIMIT_REACHED_MESSAGE }, 429)
+ end
+
send_git_archive user_project.repository, ref: params[:sha], format: params[:format], append_sha: true
rescue
not_found!('File')
diff --git a/lib/gitlab/application_rate_limiter.rb b/lib/gitlab/application_rate_limiter.rb
index 49e1f1edfb9..8468c1c6601 100644
--- a/lib/gitlab/application_rate_limiter.rb
+++ b/lib/gitlab/application_rate_limiter.rb
@@ -21,6 +21,7 @@ module Gitlab
{
project_export: { threshold: 1, interval: 5.minutes },
project_download_export: { threshold: 10, interval: 10.minutes },
+ project_repositories_archive: { threshold: 5, interval: 1.minute },
project_generate_new_export: { threshold: 1, interval: 5.minutes },
project_import: { threshold: 30, interval: 10.minutes },
play_pipeline_schedule: { threshold: 1, interval: 1.minute },
diff --git a/lib/gitlab/rate_limit_helpers.rb b/lib/gitlab/rate_limit_helpers.rb
new file mode 100644
index 00000000000..a7cab650968
--- /dev/null
+++ b/lib/gitlab/rate_limit_helpers.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module RateLimitHelpers
+ ARCHIVE_RATE_LIMIT_REACHED_MESSAGE = 'This archive has been requested too many times. Try again later.'
+ ARCHIVE_RATE_ANONYMOUS_THRESHOLD = 100 # Allow 100 requests/min for anonymous users
+ ARCHIVE_RATE_THROTTLE_KEY = :project_repositories_archive
+
+ def archive_rate_limit_reached?(user, project)
+ return false unless Feature.enabled?(:archive_rate_limit, default_enabled: true)
+
+ key = ARCHIVE_RATE_THROTTLE_KEY
+
+ if rate_limiter.throttled?(key, scope: [project], threshold: archive_rate_threshold_by_user(user))
+ rate_limiter.log_request(request, "#{key}_request_limit".to_sym, user)
+
+ return true
+ end
+
+ false
+ end
+
+ def archive_rate_threshold_by_user(user)
+ if user
+ nil # Use the defaults
+ else
+ ARCHIVE_RATE_ANONYMOUS_THRESHOLD
+ end
+ end
+
+ def rate_limiter
+ ::Gitlab::ApplicationRateLimiter
+ end
+ end
+end
diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb
index d4a81f24d9c..2d39f0afaee 100644
--- a/spec/controllers/projects/repositories_controller_spec.rb
+++ b/spec/controllers/projects/repositories_controller_spec.rb
@@ -6,6 +6,10 @@ describe Projects::RepositoriesController do
let(:project) { create(:project, :repository) }
describe "GET archive" do
+ before do
+ allow(controller).to receive(:archive_rate_limit_reached?).and_return(false)
+ end
+
context 'as a guest' do
it 'responds with redirect in correct format' do
get :archive, params: { namespace_id: project.namespace, project_id: project, id: "master" }, format: "zip"
@@ -96,6 +100,16 @@ describe Projects::RepositoriesController do
end
end
+ describe 'rate limiting' do
+ it 'rate limits user when thresholds hit' do
+ expect(controller).to receive(:archive_rate_limit_reached?).and_return(true)
+
+ get :archive, params: { namespace_id: project.namespace, project_id: project, id: 'master' }, format: "html"
+
+ expect(response).to have_gitlab_http_status(:too_many_requests)
+ end
+ end
+
describe 'caching' do
it 'sets appropriate caching headers' do
get_archive
diff --git a/spec/lib/gitlab/rate_limit_helpers_spec.rb b/spec/lib/gitlab/rate_limit_helpers_spec.rb
new file mode 100644
index 00000000000..7eee30d60ca
--- /dev/null
+++ b/spec/lib/gitlab/rate_limit_helpers_spec.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::RateLimitHelpers, :clean_gitlab_redis_shared_state do
+ let(:limiter_class) do
+ Class.new do
+ include ::Gitlab::RateLimitHelpers
+
+ attr_reader :request
+
+ def initialize(request)
+ @request = request
+ end
+ end
+ end
+
+ let(:request) { instance_double(ActionDispatch::Request, request_method: 'GET', ip: '127.0.0.1', fullpath: '/') }
+ let(:class_instance) { limiter_class.new(request) }
+
+ let_it_be(:user) { create(:user) }
+ let_it_be(:project) { create(:project) }
+
+ describe '#archive_rate_limit_reached?' do
+ context 'with a user' do
+ it 'rate limits the user properly' do
+ 5.times do
+ expect(class_instance.archive_rate_limit_reached?(user, project)).to be_falsey
+ end
+
+ expect(class_instance.archive_rate_limit_reached?(user, project)).to be_truthy
+ end
+ end
+
+ context 'with an anonymous user' do
+ before do
+ stub_const('Gitlab::RateLimitHelpers::ARCHIVE_RATE_ANONYMOUS_THRESHOLD', 2)
+ end
+
+ it 'rate limits with higher limits' do
+ 2.times do
+ expect(class_instance.archive_rate_limit_reached?(nil, project)).to be_falsey
+ end
+
+ expect(class_instance.archive_rate_limit_reached?(nil, project)).to be_truthy
+ end
+ end
+ end
+end
diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb
index 8bca458bece..b1a65ded9ef 100644
--- a/spec/requests/api/repositories_spec.rb
+++ b/spec/requests/api/repositories_spec.rb
@@ -223,6 +223,10 @@ describe API::Repositories do
describe "GET /projects/:id/repository/archive(.:format)?:sha" do
let(:route) { "/projects/#{project.id}/repository/archive" }
+ before do
+ allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(false)
+ end
+
shared_examples_for 'repository archive' do
it 'returns the repository archive' do
get api(route, current_user)
@@ -263,6 +267,14 @@ describe API::Repositories do
let(:message) { '404 File Not Found' }
end
end
+
+ it 'rate limits user when thresholds hit' do
+ allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true)
+
+ get api("/projects/#{project.id}/repository/archive.tar.bz2", user)
+
+ expect(response).to have_gitlab_http_status(:too_many_requests)
+ end
end
context 'when unauthenticated', 'and project is public' do