diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-22 15:08:58 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-22 15:08:58 +0300 |
commit | ed45528885b7b44c61f18175fe7cdbda12360669 (patch) | |
tree | 3d27c00a8a83d569cf238eaa05b7eb24b7a28a8d | |
parent | ab85af0f318ccbcfdd508e7a2f85788f26831785 (diff) |
Add latest changes from gitlab-org/gitlab@master
-rw-r--r-- | app/controllers/projects/repositories_controller.rb | 8 | ||||
-rw-r--r-- | changelogs/unreleased/sh-rate-limit-archive-endpoint.yml | 5 | ||||
-rw-r--r-- | lib/api/repositories.rb | 6 | ||||
-rw-r--r-- | lib/gitlab/application_rate_limiter.rb | 1 | ||||
-rw-r--r-- | lib/gitlab/rate_limit_helpers.rb | 35 | ||||
-rw-r--r-- | spec/controllers/projects/repositories_controller_spec.rb | 14 | ||||
-rw-r--r-- | spec/lib/gitlab/rate_limit_helpers_spec.rb | 49 | ||||
-rw-r--r-- | spec/requests/api/repositories_spec.rb | 12 |
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 |